From: Thomas Zimmermann <tzimmermann@suse.de>
To: Rayyan Ansari <rayyan@ansari.sh>, dri-devel@lists.freedesktop.org
Cc: devicetree@vger.kernel.org, linux-fbdev@vger.kernel.org,
janne@jannau.net,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Javier Martinez Canillas <javierm@redhat.com>,
linux-kernel@vger.kernel.org, Hans de Goede <hdegoede@redhat.com>,
Rob Herring <robh+dt@kernel.org>,
~postmarketos/upstreaming@lists.sr.ht, asahi@lists.linux.dev
Subject: Re: [PATCH v2 1/2] drm/simpledrm: Allow physical width and height configuration via DT
Date: Thu, 26 Jan 2023 14:05:18 +0100 [thread overview]
Message-ID: <00e614e8-4395-0aac-d613-b70b7005bd0b@suse.de> (raw)
In-Reply-To: <20230121153544.467126-2-rayyan@ansari.sh>
[-- Attachment #1.1: Type: text/plain, Size: 4408 bytes --]
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;
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]
next prev parent reply other threads:[~2023-01-26 13:05 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-21 15:35 [PATCH v2 0/2] SimpleDRM: allow configuring physical width and height Rayyan Ansari
2023-01-21 15:35 ` [PATCH v2 1/2] drm/simpledrm: Allow physical width and height configuration via DT Rayyan Ansari
2023-01-26 13:05 ` Thomas Zimmermann [this message]
2023-01-21 15:35 ` [PATCH v2 2/2] dt-bindings: display: simple-framebuffer: Document physical width and height properties Rayyan Ansari
2023-01-22 15:31 ` Rob Herring
2023-01-22 17:28 ` Rayyan Ansari
2023-01-22 15:36 ` Rob Herring
2023-01-22 15:42 ` Hans de Goede
2023-01-22 17:25 ` Rayyan Ansari
2023-01-23 17:53 ` Rob Herring
2023-01-24 22:19 ` Rayyan Ansari
2023-01-24 22:40 ` Mark Kettenis
2023-01-25 7:28 ` Krzysztof Kozlowski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=00e614e8-4395-0aac-d613-b70b7005bd0b@suse.de \
--to=tzimmermann@suse.de \
--cc=asahi@lists.linux.dev \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=hdegoede@redhat.com \
--cc=janne@jannau.net \
--cc=javierm@redhat.com \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linux-fbdev@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rayyan@ansari.sh \
--cc=robh+dt@kernel.org \
--cc=~postmarketos/upstreaming@lists.sr.ht \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).