[v2,0/2] SimpleDRM: allow configuring physical width and height

Hello,

The following patches:
- Add support for configuring the width-mm and height-mm DRM mode
  properties in the SimpleDRM driver via Device Tree
- Document these two new Device Tree properties

This is useful for allowing interfaces such as Phosh to calculate       
proper scaling values.

Changes since RFC:
- Switch to using 32-bit DT property
- Report errors for return values of of_property_read_u32 except -EINVAL
- Calculate default value during probe
- Add documentation

Rayyan Ansari (2):
  drm/simpledrm: Allow physical width and height configuration via DT
  dt-bindings: display: simple-framebuffer: Document physical width and
    height properties

 .../bindings/display/simple-framebuffer.yaml  |  8 +++
 drivers/gpu/drm/tiny/simpledrm.c              | 60 ++++++++++++++++---
 2 files changed, 59 insertions(+), 9 deletions(-)

Hi,

I just want to comment that the code in this patch is fine, but I'm not 
going to merge it until the other discussion about using the panel's DT 
nodes has been resolved. IMHO, the panel-based solution seems preferable 
to the new properties.

Best regards
Thomas

Am 21.01.23 um 16:35 schrieb Rayyan Ansari:
> Signed-off-by: Rayyan Ansari <rayyan@ansari.sh>
> ---
>   drivers/gpu/drm/tiny/simpledrm.c | 60 +++++++++++++++++++++++++++-----
>   1 file changed, 51 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
> index 162eb44dcba8..7aab7fa572f0 100644
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> @@ -128,6 +128,23 @@ simplefb_read_u32_of(struct drm_device *dev, struct device_node *of_node,
>   	return ret;
>   }
>   
> +static int
> +simplefb_read_u32_of_opt(struct drm_device *dev, struct device_node *of_node,
> +			 const char *name, u32 *value)
> +{
> +	int ret = of_property_read_u32(of_node, name, value);
> +
> +	if (ret == -EINVAL) {
> +		*value = 0;
> +		ret = 0;
> +	} else if (ret) {
> +		drm_err(dev, "simplefb: cannot parse framebuffer %s: error %d\n",
> +			name, ret);
> +	}
> +
> +	return ret;
> +}
> +
>   static int
>   simplefb_read_string_of(struct drm_device *dev, struct device_node *of_node,
>   			const char *name, const char **value)
> @@ -184,6 +201,19 @@ simplefb_get_format_of(struct drm_device *dev, struct device_node *of_node)
>   	return simplefb_get_validated_format(dev, format);
>   }
>   
> +static int
> +simplefb_get_mm_of(struct drm_device *dev, struct device_node *of_node,
> +		   const char *name)
> +{
> +	int mm;
> +	int ret = simplefb_read_u32_of_opt(dev, of_node, name, &mm);
> +
> +	if (ret)
> +		return ret;
> +	return simplefb_get_validated_int(dev, name, mm);
> +}
> +
> +
>   /*
>    * Simple Framebuffer device
>    */
> @@ -599,16 +629,12 @@ static const struct drm_mode_config_funcs simpledrm_mode_config_funcs = {
>    */
>   
>   static struct drm_display_mode simpledrm_mode(unsigned int width,
> -					      unsigned int height)
> +					      unsigned int height,
> +					      unsigned int width_mm,
> +					      unsigned int height_mm)
>   {
> -	/*
> -	 * Assume a monitor resolution of 96 dpi to
> -	 * get a somewhat reasonable screen size.
> -	 */
>   	const struct drm_display_mode mode = {
> -		DRM_MODE_INIT(60, width, height,
> -			      DRM_MODE_RES_MM(width, 96ul),
> -			      DRM_MODE_RES_MM(height, 96ul))
> +		DRM_MODE_INIT(60, width, height, width_mm, height_mm)
>   	};
>   
>   	return mode;
> @@ -622,6 +648,7 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   	struct simpledrm_device *sdev;
>   	struct drm_device *dev;
>   	int width, height, stride;
> +	int width_mm = 0, height_mm = 0;
>   	const struct drm_format_info *format;
>   	struct resource *res, *mem;
>   	void __iomem *screen_base;
> @@ -676,6 +703,12 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   		format = simplefb_get_format_of(dev, of_node);
>   		if (IS_ERR(format))
>   			return ERR_CAST(format);
> +		width_mm = simplefb_get_mm_of(dev, of_node, "width-mm");
> +		if (width_mm < 0)
> +			return ERR_PTR(width_mm);
> +		height_mm = simplefb_get_mm_of(dev, of_node, "height-mm");
> +		if (height_mm < 0)
> +			return ERR_PTR(height_mm);
>   	} else {
>   		drm_err(dev, "no simplefb configuration found\n");
>   		return ERR_PTR(-ENODEV);
> @@ -686,7 +719,16 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv,
>   			return ERR_PTR(-EINVAL);
>   	}
>   
> -	sdev->mode = simpledrm_mode(width, height);
> +	/*
> +	 * Assume a monitor resolution of 96 dpi if physical dimensions
> +	 * are not specified to get a somewhat reasonable screen size.
> +	 */
> +	if (!width_mm)
> +		width_mm = DRM_MODE_RES_MM(width, 96ul);
> +	if (!height_mm)
> +		height_mm = DRM_MODE_RES_MM(height, 96ul);
> +
> +	sdev->mode = simpledrm_mode(width, height, width_mm, height_mm);
>   	sdev->format = format;
>   	sdev->pitch = stride;
>