dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others)
@ 2026-08-27 14:01 Thomas Zimmermann
  2026-08-27 14:01 ` [PATCH 1/3] drm/edid: Add drm_edid_get_preferred_size() Thomas Zimmermann
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2026-08-27 14:01 UTC (permalink / raw)
  To: javierm, ardb, ilias.apalodimas, jani.nikula, maarten.lankhorst,
	mripard, simona, airlied
  Cc: dri-devel, linux-efi, sashiko-reviews, Thomas Zimmermann

The kernel mis-interprets the display geometry given by the Lenovo
D330 as incorrect. The values are correct, but the display output
is rotated by 90°. With the kernel's firmware quirk applied the output
is garbled.

Remove the firmware quirk for the Lenovo D330 and let DRM's sysfb
drivers set the correct panel orientation instead. To make this work
for any display mode, use the panel size for looking up the correct
orientation.

Tested on a Lenovo D330-10IGM IdeaPad.

Thomas Zimmermann (3):
  drm/edid: Add drm_edid_get_preferred_size()
  drm/sysfb: Use preferred panel size for panel orientation quirks
  firmware/sysfb: Remove rotation quirk for Lenovo D330

 drivers/firmware/efi/sysfb_efi.c |  9 -------
 drivers/gpu/drm/drm_edid.c       | 40 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/sysfb/efidrm.c   | 17 +++++++++++++-
 drivers/gpu/drm/sysfb/ofdrm.c    | 16 ++++++++++++-
 drivers/gpu/drm/sysfb/vesadrm.c  | 17 +++++++++++++-
 include/drm/drm_edid.h           |  2 ++
 6 files changed, 89 insertions(+), 12 deletions(-)


base-commit: dc462ab791b686c48545c160ecc81be64f77a846
-- 
2.55.0


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] drm/edid: Add drm_edid_get_preferred_size()
  2026-08-27 14:01 [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others) Thomas Zimmermann
@ 2026-08-27 14:01 ` Thomas Zimmermann
  2026-08-27 14:19   ` sashiko-bot
  2026-08-27 14:01 ` [PATCH 2/3] drm/sysfb: Use preferred panel size for panel orientation quirks Thomas Zimmermann
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Thomas Zimmermann @ 2026-08-27 14:01 UTC (permalink / raw)
  To: javierm, ardb, ilias.apalodimas, jani.nikula, maarten.lankhorst,
	mripard, simona, airlied
  Cc: dri-devel, linux-efi, sashiko-reviews, Thomas Zimmermann

Add drm_edid_get_preferred_size() to extract the panel's preferred
display resolution from a given EDID. Required for DRM's panel
orientation quirks.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/drm_edid.c | 40 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_edid.h     |  2 ++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 07970e5b5f65..02cbf8de1128 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2844,6 +2844,46 @@ u32 drm_edid_get_panel_id(const struct drm_edid *drm_edid)
 }
 EXPORT_SYMBOL(drm_edid_get_panel_id);
 
+/**
+ * drm_edid_get_preferred_size - Get a panel's preferred size from EDID
+ * @drm_edid: EDID that contains panel ID.
+ * @width: Returns the panel's width in pixels per scanline, if given
+ * @height: Returns the panel's height in scanlines, if given
+ *
+ * This function detects the preferred size of a panel from the given
+ * EDID. There is no such information stored in the EDID block directly,
+ * but the preferred mode often corresponds to the panel's native geometry.
+ *
+ * Return: Zero on success, or a negative errno code otherwise.
+ */
+int drm_edid_get_preferred_size(const struct drm_edid *drm_edid,
+				unsigned int *width, unsigned int *height)
+{
+	const struct edid *edid = drm_edid->edid;
+	const struct detailed_pixel_timing *pt;
+
+	if (drm_edid->size < EDID_LENGTH)
+		return -EINVAL;
+
+	/*
+	 * Use whatever the Preferred Timing Descriptor tells us. For old
+	 * and obscure displays, we might need better heuristics.
+	 */
+
+	if (edid->revision < 4 && !(edid->features & DRM_EDID_FEATURE_PREFERRED_TIMING))
+		return -EINVAL; /* no Preferred Timing Descriptor */
+
+	pt = &edid->detailed_timings[0].data.pixel_data;
+
+	if (*width)
+		*width = (pt->hactive_hblank_hi & 0xf0) << 4 | pt->hactive_lo;
+	if (*height)
+		*height = (pt->vactive_vblank_hi & 0xf0) << 4 | pt->vactive_lo;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_edid_get_preferred_size);
+
 /**
  * drm_edid_read_base_block - Get a panel's EDID base block
  * @adapter: I2C adapter to use for DDC
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 04f7a7f1f108..981aeeb6ddf9 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -487,6 +487,8 @@ void drm_edid_get_product_id(const struct drm_edid *drm_edid,
 void drm_edid_print_product_id(struct drm_printer *p,
 			       const struct drm_edid_product_id *id, bool raw);
 u32 drm_edid_get_panel_id(const struct drm_edid *drm_edid);
+int drm_edid_get_preferred_size(const struct drm_edid *drm_edid,
+				unsigned int *width, unsigned int *height);
 bool drm_edid_match(const struct drm_edid *drm_edid,
 		    const struct drm_edid_ident *ident);
 bool drm_edid_has_quirk(struct drm_connector *connector, enum drm_edid_quirk quirk);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] drm/sysfb: Use preferred panel size for panel orientation quirks
  2026-08-27 14:01 [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others) Thomas Zimmermann
  2026-08-27 14:01 ` [PATCH 1/3] drm/edid: Add drm_edid_get_preferred_size() Thomas Zimmermann
@ 2026-08-27 14:01 ` Thomas Zimmermann
  2026-08-27 14:21   ` sashiko-bot
  2026-08-27 14:01 ` [PATCH 3/3] firmware/sysfb: Remove rotation quirk for Lenovo D330 Thomas Zimmermann
  2026-08-27 16:56 ` [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others) Ard Biesheuvel
  3 siblings, 1 reply; 10+ messages in thread
From: Thomas Zimmermann @ 2026-08-27 14:01 UTC (permalink / raw)
  To: javierm, ardb, ilias.apalodimas, jani.nikula, maarten.lankhorst,
	mripard, simona, airlied
  Cc: dri-devel, linux-efi, sashiko-reviews, Thomas Zimmermann

Sysfb drivers currently use the given display mode for looking up the
panel orientation. But the look-up table stores the native geometry of
the panels, so the lookup fails if the current mode sizes differs.

Get the panel's native geometry with drm_edid_get_preferred_size() from
the EDID and use it for looking up the panel orientation.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/sysfb/efidrm.c  | 17 ++++++++++++++++-
 drivers/gpu/drm/sysfb/ofdrm.c   | 16 +++++++++++++++-
 drivers/gpu/drm/sysfb/vesadrm.c | 17 ++++++++++++++++-
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
index 3f9cd5d03efb..7a7d12003304 100644
--- a/drivers/gpu/drm/sysfb/efidrm.c
+++ b/drivers/gpu/drm/sysfb/efidrm.c
@@ -152,6 +152,7 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
 	const struct screen_info *si;
 	const struct drm_format_info *format;
 	int width, height, stride;
+	unsigned int panel_width, panel_height;
 	s64 vsize;
 	u64 mem_flags;
 	struct resource resbuf;
@@ -217,6 +218,20 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
 	if (drm_edid_header_is_valid(dpy->edid.dummy) == 8)
 		sysfb->edid = dpy->edid.dummy;
 #endif
+
+	panel_width = width;
+	panel_height = height;
+
+	if (sysfb->edid) {
+		const struct drm_edid *drm_edid;
+
+		drm_edid = drm_edid_alloc(sysfb->edid, EDID_LENGTH);
+		if (drm_edid) {
+			drm_edid_get_preferred_size(drm_edid, &panel_width, &panel_height);
+			drm_edid_free(drm_edid);
+		}
+	}
+
 	sysfb->fb_mode = drm_sysfb_mode(width, height, 0, 0);
 	sysfb->fb_format = format;
 	sysfb->fb_pitch = stride;
@@ -340,7 +355,7 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
 	drm_connector_helper_add(connector, &efidrm_connector_helper_funcs);
 	drm_connector_set_panel_orientation_with_quirk(connector,
 						       DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
-						       width, height);
+						       panel_width, panel_height);
 	if (sysfb->edid)
 		drm_connector_attach_edid_property(connector);
 
diff --git a/drivers/gpu/drm/sysfb/ofdrm.c b/drivers/gpu/drm/sysfb/ofdrm.c
index 819aed466727..6cf2440d7d9e 100644
--- a/drivers/gpu/drm/sysfb/ofdrm.c
+++ b/drivers/gpu/drm/sysfb/ofdrm.c
@@ -827,6 +827,7 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv,
 	enum ofdrm_model model;
 	bool big_endian;
 	int width, height, depth, linebytes;
+	unsigned int panel_width, panel_height;
 	const struct drm_format_info *format;
 	u64 address;
 	const u8 *edid;
@@ -994,6 +995,19 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv,
 		sysfb->fb_gamma_lut_size = OFDRM_GAMMA_LUT_SIZE;
 	sysfb->edid = edid;
 
+	panel_width = width;
+	panel_height = height;
+
+	if (sysfb->edid) {
+		const struct drm_edid *drm_edid;
+
+		drm_edid = drm_edid_alloc(sysfb->edid, EDID_LENGTH);
+		if (drm_edid) {
+			drm_edid_get_preferred_size(drm_edid, &panel_width, &panel_height);
+			drm_edid_free(drm_edid);
+		}
+	}
+
 	drm_dbg(dev, "display mode={" DRM_MODE_FMT "}\n", DRM_MODE_ARG(&sysfb->fb_mode));
 	drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, linebytes=%d byte\n",
 		&format->format, width, height, linebytes);
@@ -1065,7 +1079,7 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv,
 	drm_connector_helper_add(connector, &ofdrm_connector_helper_funcs);
 	drm_connector_set_panel_orientation_with_quirk(connector,
 						       DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
-						       width, height);
+						       panel_width, panel_height);
 	if (edid)
 		drm_connector_attach_edid_property(connector);
 
diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
index 6a67b2d2e451..8c52bbac6d2f 100644
--- a/drivers/gpu/drm/sysfb/vesadrm.c
+++ b/drivers/gpu/drm/sysfb/vesadrm.c
@@ -402,6 +402,7 @@ static struct vesadrm_device *vesadrm_device_create(struct drm_driver *drv,
 	const struct screen_info *si;
 	const struct drm_format_info *format;
 	int width, height, stride;
+	unsigned int panel_width, panel_height;
 	s64 vsize;
 	struct resource resbuf;
 	struct resource *res;
@@ -484,6 +485,20 @@ static struct vesadrm_device *vesadrm_device_create(struct drm_driver *drv,
 	if (drm_edid_header_is_valid(dpy->edid.dummy) == 8)
 		sysfb->edid = dpy->edid.dummy;
 #endif
+
+	panel_width = width;
+	panel_height = height;
+
+	if (sysfb->edid) {
+		const struct drm_edid *drm_edid;
+
+		drm_edid = drm_edid_alloc(sysfb->edid, EDID_LENGTH);
+		if (drm_edid) {
+			drm_edid_get_preferred_size(drm_edid, &panel_width, &panel_height);
+			drm_edid_free(drm_edid);
+		}
+	}
+
 	sysfb->fb_mode = drm_sysfb_mode(width, height, 0, 0);
 	sysfb->fb_format = format;
 	sysfb->fb_pitch = stride;
@@ -585,7 +600,7 @@ static struct vesadrm_device *vesadrm_device_create(struct drm_driver *drv,
 	drm_connector_helper_add(connector, &vesadrm_connector_helper_funcs);
 	drm_connector_set_panel_orientation_with_quirk(connector,
 						       DRM_MODE_PANEL_ORIENTATION_UNKNOWN,
-						       width, height);
+						       panel_width, panel_height);
 	if (sysfb->edid)
 		drm_connector_attach_edid_property(connector);
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] firmware/sysfb: Remove rotation quirk for Lenovo D330
  2026-08-27 14:01 [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others) Thomas Zimmermann
  2026-08-27 14:01 ` [PATCH 1/3] drm/edid: Add drm_edid_get_preferred_size() Thomas Zimmermann
  2026-08-27 14:01 ` [PATCH 2/3] drm/sysfb: Use preferred panel size for panel orientation quirks Thomas Zimmermann
@ 2026-08-27 14:01 ` Thomas Zimmermann
  2026-08-27 16:56 ` [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others) Ard Biesheuvel
  3 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2026-08-27 14:01 UTC (permalink / raw)
  To: javierm, ardb, ilias.apalodimas, jani.nikula, maarten.lankhorst,
	mripard, simona, airlied
  Cc: dri-devel, linux-efi, sashiko-reviews, Thomas Zimmermann

The Lenovo D330 is a notebook with unmountable display. The display
is installed in landscape orientation, but the display mode is in
portrait orientation. By default, the display output is rotated to
the left by 90°.

The sysfb quirk breaks the display by swapping display width and
height; and thus mixing up the display geometry. Fix this by keeping
the display as-is. The sysfb DRM drivers, usually efidrm, will pick
the correct panel orientation for the display.

Tested on a Lenovo D330-10IGM IdeaPad.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/firmware/efi/sysfb_efi.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/firmware/efi/sysfb_efi.c b/drivers/firmware/efi/sysfb_efi.c
index 685283bb7327..2338de73c62c 100644
--- a/drivers/firmware/efi/sysfb_efi.c
+++ b/drivers/firmware/efi/sysfb_efi.c
@@ -301,15 +301,6 @@ static const struct dmi_system_id efifb_dmi_swap_width_height[] __initconst = {
 		},
 		.callback = efifb_swap_width_height,
 	},
-	{
-		/* Lenovo D330 with 800x1280 or 1200x1920 portrait screen */
-		.matches = {
-			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"),
-			DMI_EXACT_MATCH(DMI_PRODUCT_VERSION,
-					"Lenovo ideapad D330-10IGM"),
-		},
-		.callback = efifb_swap_width_height,
-	},
 	{
 		/*
 		 * Lenovo IdeaPad Duet 3 10IGL5 and 10IGL5-LTE with
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] drm/edid: Add drm_edid_get_preferred_size()
  2026-08-27 14:01 ` [PATCH 1/3] drm/edid: Add drm_edid_get_preferred_size() Thomas Zimmermann
@ 2026-08-27 14:19   ` sashiko-bot
  2026-08-27 14:40     ` Thomas Zimmermann
  0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-08-27 14:19 UTC (permalink / raw)
  To: Thomas Zimmermann; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Pointer Dereference and Logic Error in drm_edid_get_preferred_size
- [High] Missing EDID Hardware Validation for Detailed Timing Descriptor
--

commit 2d529ec582fce7d82757afd45c986da44f50ceb6
Author: Thomas Zimmermann <tzimmermann@suse.de>
Subject: drm/edid: Add drm_edid_get_preferred_size()

This commit adds drm_edid_get_preferred_size() to extract the panel's
preferred display resolution from a given EDID. This is required for
DRM's panel orientation quirks.

> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index df3c25bac761d..650ccd05977be 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c

[ ... ]

> @@ -2844,6 +2844,46 @@ u32 drm_edid_get_panel_id(const struct drm_edid *drm_edid)
>  }
>  EXPORT_SYMBOL(drm_edid_get_panel_id);
>  
> +/**
> + * drm_edid_get_preferred_size - Get a panel's preferred size from EDID
> + * @drm_edid: EDID that contains panel ID.
> + * @width: Returns the panel's width in pixels per scanline, if given
> + * @height: Returns the panel's height in scanlines, if given
> + *
> + * This function detects the preferred size of a panel from the given
> + * EDID. There is no such information stored in the EDID block directly,
> + * but the preferred mode often corresponds to the panel's native geometry.
> + *
> + * Return: Zero on success, or a negative errno code otherwise.
> + */
> +int drm_edid_get_preferred_size(const struct drm_edid *drm_edid,
> +				unsigned int *width, unsigned int *height)
> +{
> +	const struct edid *edid = drm_edid->edid;
> +	const struct detailed_pixel_timing *pt;
> +
> +	if (drm_edid->size < EDID_LENGTH)
> +		return -EINVAL;
> +
> +	/*
> +	 * Use whatever the Preferred Timing Descriptor tells us. For old
> +	 * and obscure displays, we might need better heuristics.
> +	 */
> +
> +	if (edid->revision < 4 && !(edid->features & DRM_EDID_FEATURE_PREFERRED_TIMING))
> +		return -EINVAL; /* no Preferred Timing Descriptor */
> +
> +	pt = &edid->detailed_timings[0].data.pixel_data;

[Severity: High]
Does this safely handle non-compliant or corrupted EDIDs where the first
detailed timing block might be a Display Descriptor rather than a pixel
timing descriptor?

In the EDID specification, a detailed timing block is only a Detailed Timing
Descriptor if its pixel clock is non-zero. If the pixel clock is 0, the block
is a Display Descriptor like an ASCII monitor name. By unconditionally
accessing it as pixel data without validating it, could we end up reading
overlapping union fields and returning garbage display geometry data to the
driver?

> +
> +	if (*width)
> +		*width = (pt->hactive_hblank_hi & 0xf0) << 4 | pt->hactive_lo;
> +	if (*height)
> +		*height = (pt->vactive_vblank_hi & 0xf0) << 4 | pt->vactive_lo;

[Severity: High]
Does this logic incorrectly dereference the pointers before checking if they
are valid?

The kernel doc mentions that width and height are optional, which implies
callers might pass a NULL pointer. If a caller passes NULL for width or
height, evaluating if (*width) will cause a NULL pointer dereference crash.

Additionally, if a caller passes a valid pointer to a 0-initialized variable,
evaluating if (*width) will result in false, skipping the assignment entirely
and leaving the output as 0. Should these conditions check if (width) and
if (height) instead?

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_edid_get_preferred_size);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827140726.420734-1-tzimmermann@suse.de?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] drm/sysfb: Use preferred panel size for panel orientation quirks
  2026-08-27 14:01 ` [PATCH 2/3] drm/sysfb: Use preferred panel size for panel orientation quirks Thomas Zimmermann
@ 2026-08-27 14:21   ` sashiko-bot
  2026-08-27 14:44     ` Thomas Zimmermann
  0 siblings, 1 reply; 10+ messages in thread
From: sashiko-bot @ 2026-08-27 14:21 UTC (permalink / raw)
  To: Thomas Zimmermann; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The sysfb drivers parse unvalidated EDID data to extract the preferred size.

Pre-existing issues:
- [High] drm_edid_get_preferred_size() incorrectly checks the value pointed to by width/height instead of validating the pointers themselves.
--

commit df008c5490e15b30036ebe81983d7a6c0c6e5b33
Author: Thomas Zimmermann <tzimmermann@suse.de>

drm/sysfb: Use preferred panel size for panel orientation quirks

Sysfb drivers currently use the given display mode for looking up the
panel orientation. But the look-up table stores the native geometry of
the panels, so the lookup fails if the current mode sizes differs.

Get the panel's native geometry with drm_edid_get_preferred_size() from
the EDID and use it for looking up the panel orientation.

> diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
> index 3f9cd5d03efb2..7a7d12003304f 100644
> --- a/drivers/gpu/drm/sysfb/efidrm.c
> +++ b/drivers/gpu/drm/sysfb/efidrm.c
[ ... ]
> @@ -217,6 +218,20 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
>  	if (drm_edid_header_is_valid(dpy->edid.dummy) == 8)
>  		sysfb->edid = dpy->edid.dummy;
>  #endif
> +
> +	panel_width = width;
> +	panel_height = height;
> +
> +	if (sysfb->edid) {
> +		const struct drm_edid *drm_edid;
> +
> +		drm_edid = drm_edid_alloc(sysfb->edid, EDID_LENGTH);
> +		if (drm_edid) {
> +			drm_edid_get_preferred_size(drm_edid, &panel_width, &panel_height);

[Severity: High]
This isn't a bug introduced by this patch, but I wanted to ask about the
semantics of drm_edid_get_preferred_size(). In drivers/gpu/drm/drm_edid.c,
it does:

	if (*width)
		*width = (pt->hactive_hblank_hi & 0xf0) << 4 | pt->hactive_lo;
	if (*height)
		*height = (pt->vactive_vblank_hi & 0xf0) << 4 | pt->vactive_lo;

This incorrectly checks the values pointed to by width/height instead of
validating the pointers themselves. A caller passing a NULL pointer for
either width or height will cause a kernel panic (NULL pointer dereference).

Furthermore, passing a pointer to a zero-initialized variable evaluates to
false, causing the function to silently skip populating the preferred size.
While this patch bypasses the crash by passing non-NULL pointers initialized
to >0, is it worth fixing the generic API so other callers do not trip over
this?

[Severity: Medium]
Does parsing an unvalidated EDID block pose a risk here? The sysfb drivers
parse unvalidated EDID data to extract the preferred size.

If the firmware provides a corrupted EDID where only the 8-byte header is
valid, this code extracts geometry from potentially corrupted firmware data,
which can result in bogus resolutions being passed to the panel orientation
quirk lookup.

> +			drm_edid_free(drm_edid);
> +		}
> +	}
> +

[ ... ]
> diff --git a/drivers/gpu/drm/sysfb/ofdrm.c b/drivers/gpu/drm/sysfb/ofdrm.c
> index 819aed466727e..6cf2440d7d9e2 100644
> --- a/drivers/gpu/drm/sysfb/ofdrm.c
> +++ b/drivers/gpu/drm/sysfb/ofdrm.c
[ ... ]
> @@ -994,6 +995,19 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv,
>  		sysfb->fb_gamma_lut_size = OFDRM_GAMMA_LUT_SIZE;
>  	sysfb->edid = edid;
>  
> +	panel_width = width;
> +	panel_height = height;
> +
> +	if (sysfb->edid) {
> +		const struct drm_edid *drm_edid;
> +
> +		drm_edid = drm_edid_alloc(sysfb->edid, EDID_LENGTH);
> +		if (drm_edid) {
> +			drm_edid_get_preferred_size(drm_edid, &panel_width, &panel_height);

[Severity: Medium]
Similarly here in ofdrm_device_create(), unvalidated EDID data is parsed.
Could this lead to bogus geometry being extracted from a corrupted EDID?

> +			drm_edid_free(drm_edid);
> +		}
> +	}
> +

[ ... ]
> diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
> index 6a67b2d2e451b..8c52bbac6d2ff 100644
> --- a/drivers/gpu/drm/sysfb/vesadrm.c
> +++ b/drivers/gpu/drm/sysfb/vesadrm.c
[ ... ]
> @@ -484,6 +485,20 @@ static struct vesadrm_device *vesadrm_device_create(struct drm_driver *drv,
>  	if (drm_edid_header_is_valid(dpy->edid.dummy) == 8)
>  		sysfb->edid = dpy->edid.dummy;
>  #endif
> +
> +	panel_width = width;
> +	panel_height = height;
> +
> +	if (sysfb->edid) {
> +		const struct drm_edid *drm_edid;
> +
> +		drm_edid = drm_edid_alloc(sysfb->edid, EDID_LENGTH);
> +		if (drm_edid) {
> +			drm_edid_get_preferred_size(drm_edid, &panel_width, &panel_height);

[Severity: Medium]
Likewise in vesadrm_device_create(), parsing unvalidated EDID data may
result in bogus resolutions being passed to the panel orientation quirk
lookup. Should the EDID be fully validated first?

> +			drm_edid_free(drm_edid);
> +		}
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827140726.420734-1-tzimmermann@suse.de?part=2

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] drm/edid: Add drm_edid_get_preferred_size()
  2026-08-27 14:19   ` sashiko-bot
@ 2026-08-27 14:40     ` Thomas Zimmermann
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2026-08-27 14:40 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: dri-devel



Am 27.08.26 um 16:19 schrieb sashiko-bot@kernel.org:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Pointer Dereference and Logic Error in drm_edid_get_preferred_size

This is just a pretty stupid typo. Will be fixed.

> - [High] Missing EDID Hardware Validation for Detailed Timing Descriptor

Will also be fixed.


> --
>
> commit 2d529ec582fce7d82757afd45c986da44f50ceb6
> Author: Thomas Zimmermann <tzimmermann@suse.de>
> Subject: drm/edid: Add drm_edid_get_preferred_size()
>
> This commit adds drm_edid_get_preferred_size() to extract the panel's
> preferred display resolution from a given EDID. This is required for
> DRM's panel orientation quirks.
>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index df3c25bac761d..650ccd05977be 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
> [ ... ]
>
>> @@ -2844,6 +2844,46 @@ u32 drm_edid_get_panel_id(const struct drm_edid *drm_edid)
>>   }
>>   EXPORT_SYMBOL(drm_edid_get_panel_id);
>>   
>> +/**
>> + * drm_edid_get_preferred_size - Get a panel's preferred size from EDID
>> + * @drm_edid: EDID that contains panel ID.
>> + * @width: Returns the panel's width in pixels per scanline, if given
>> + * @height: Returns the panel's height in scanlines, if given
>> + *
>> + * This function detects the preferred size of a panel from the given
>> + * EDID. There is no such information stored in the EDID block directly,
>> + * but the preferred mode often corresponds to the panel's native geometry.
>> + *
>> + * Return: Zero on success, or a negative errno code otherwise.
>> + */
>> +int drm_edid_get_preferred_size(const struct drm_edid *drm_edid,
>> +				unsigned int *width, unsigned int *height)
>> +{
>> +	const struct edid *edid = drm_edid->edid;
>> +	const struct detailed_pixel_timing *pt;
>> +
>> +	if (drm_edid->size < EDID_LENGTH)
>> +		return -EINVAL;
>> +
>> +	/*
>> +	 * Use whatever the Preferred Timing Descriptor tells us. For old
>> +	 * and obscure displays, we might need better heuristics.
>> +	 */
>> +
>> +	if (edid->revision < 4 && !(edid->features & DRM_EDID_FEATURE_PREFERRED_TIMING))
>> +		return -EINVAL; /* no Preferred Timing Descriptor */
>> +
>> +	pt = &edid->detailed_timings[0].data.pixel_data;
> [Severity: High]
> Does this safely handle non-compliant or corrupted EDIDs where the first
> detailed timing block might be a Display Descriptor rather than a pixel
> timing descriptor?
>
> In the EDID specification, a detailed timing block is only a Detailed Timing
> Descriptor if its pixel clock is non-zero. If the pixel clock is 0, the block
> is a Display Descriptor like an ASCII monitor name. By unconditionally
> accessing it as pixel data without validating it, could we end up reading
> overlapping union fields and returning garbage display geometry data to the
> driver?
>
>> +
>> +	if (*width)
>> +		*width = (pt->hactive_hblank_hi & 0xf0) << 4 | pt->hactive_lo;
>> +	if (*height)
>> +		*height = (pt->vactive_vblank_hi & 0xf0) << 4 | pt->vactive_lo;
> [Severity: High]
> Does this logic incorrectly dereference the pointers before checking if they
> are valid?
>
> The kernel doc mentions that width and height are optional, which implies
> callers might pass a NULL pointer. If a caller passes NULL for width or
> height, evaluating if (*width) will cause a NULL pointer dereference crash.
>
> Additionally, if a caller passes a valid pointer to a 0-initialized variable,
> evaluating if (*width) will result in false, skipping the assignment entirely
> and leaving the output as 0. Should these conditions check if (width) and
> if (height) instead?
>
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(drm_edid_get_preferred_size);

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/3] drm/sysfb: Use preferred panel size for panel orientation quirks
  2026-08-27 14:21   ` sashiko-bot
@ 2026-08-27 14:44     ` Thomas Zimmermann
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2026-08-27 14:44 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: dri-devel



Am 27.08.26 um 16:21 schrieb sashiko-bot@kernel.org:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Medium] The sysfb drivers parse unvalidated EDID data to extract the preferred size.

If we read bogus values, the panel orientation could be wrong. Nothing 
worse happens.  Apart from that, most drivers call 
drm_edid_header_is_valid(). I'll add this call to ofdrm, which is 
currently missing it.
>
> Pre-existing issues:
> - [High] drm_edid_get_preferred_size() incorrectly checks the value pointed to by width/height instead of validating the pointers themselves.

That's just fallout from the typo in patch 1.

> --
>
> commit df008c5490e15b30036ebe81983d7a6c0c6e5b33
> Author: Thomas Zimmermann <tzimmermann@suse.de>
>
> drm/sysfb: Use preferred panel size for panel orientation quirks
>
> Sysfb drivers currently use the given display mode for looking up the
> panel orientation. But the look-up table stores the native geometry of
> the panels, so the lookup fails if the current mode sizes differs.
>
> Get the panel's native geometry with drm_edid_get_preferred_size() from
> the EDID and use it for looking up the panel orientation.
>
>> diff --git a/drivers/gpu/drm/sysfb/efidrm.c b/drivers/gpu/drm/sysfb/efidrm.c
>> index 3f9cd5d03efb2..7a7d12003304f 100644
>> --- a/drivers/gpu/drm/sysfb/efidrm.c
>> +++ b/drivers/gpu/drm/sysfb/efidrm.c
> [ ... ]
>> @@ -217,6 +218,20 @@ static struct efidrm_device *efidrm_device_create(struct drm_driver *drv,
>>   	if (drm_edid_header_is_valid(dpy->edid.dummy) == 8)
>>   		sysfb->edid = dpy->edid.dummy;
>>   #endif
>> +
>> +	panel_width = width;
>> +	panel_height = height;
>> +
>> +	if (sysfb->edid) {
>> +		const struct drm_edid *drm_edid;
>> +
>> +		drm_edid = drm_edid_alloc(sysfb->edid, EDID_LENGTH);
>> +		if (drm_edid) {
>> +			drm_edid_get_preferred_size(drm_edid, &panel_width, &panel_height);
> [Severity: High]
> This isn't a bug introduced by this patch, but I wanted to ask about the
> semantics of drm_edid_get_preferred_size(). In drivers/gpu/drm/drm_edid.c,
> it does:
>
> 	if (*width)
> 		*width = (pt->hactive_hblank_hi & 0xf0) << 4 | pt->hactive_lo;
> 	if (*height)
> 		*height = (pt->vactive_vblank_hi & 0xf0) << 4 | pt->vactive_lo;
>
> This incorrectly checks the values pointed to by width/height instead of
> validating the pointers themselves. A caller passing a NULL pointer for
> either width or height will cause a kernel panic (NULL pointer dereference).
>
> Furthermore, passing a pointer to a zero-initialized variable evaluates to
> false, causing the function to silently skip populating the preferred size.
> While this patch bypasses the crash by passing non-NULL pointers initialized
> to >0, is it worth fixing the generic API so other callers do not trip over
> this?
>
> [Severity: Medium]
> Does parsing an unvalidated EDID block pose a risk here? The sysfb drivers
> parse unvalidated EDID data to extract the preferred size.
>
> If the firmware provides a corrupted EDID where only the 8-byte header is
> valid, this code extracts geometry from potentially corrupted firmware data,
> which can result in bogus resolutions being passed to the panel orientation
> quirk lookup.
>
>> +			drm_edid_free(drm_edid);
>> +		}
>> +	}
>> +
> [ ... ]
>> diff --git a/drivers/gpu/drm/sysfb/ofdrm.c b/drivers/gpu/drm/sysfb/ofdrm.c
>> index 819aed466727e..6cf2440d7d9e2 100644
>> --- a/drivers/gpu/drm/sysfb/ofdrm.c
>> +++ b/drivers/gpu/drm/sysfb/ofdrm.c
> [ ... ]
>> @@ -994,6 +995,19 @@ static struct ofdrm_device *ofdrm_device_create(struct drm_driver *drv,
>>   		sysfb->fb_gamma_lut_size = OFDRM_GAMMA_LUT_SIZE;
>>   	sysfb->edid = edid;
>>   
>> +	panel_width = width;
>> +	panel_height = height;
>> +
>> +	if (sysfb->edid) {
>> +		const struct drm_edid *drm_edid;
>> +
>> +		drm_edid = drm_edid_alloc(sysfb->edid, EDID_LENGTH);
>> +		if (drm_edid) {
>> +			drm_edid_get_preferred_size(drm_edid, &panel_width, &panel_height);
> [Severity: Medium]
> Similarly here in ofdrm_device_create(), unvalidated EDID data is parsed.
> Could this lead to bogus geometry being extracted from a corrupted EDID?
>
>> +			drm_edid_free(drm_edid);
>> +		}
>> +	}
>> +
> [ ... ]
>> diff --git a/drivers/gpu/drm/sysfb/vesadrm.c b/drivers/gpu/drm/sysfb/vesadrm.c
>> index 6a67b2d2e451b..8c52bbac6d2ff 100644
>> --- a/drivers/gpu/drm/sysfb/vesadrm.c
>> +++ b/drivers/gpu/drm/sysfb/vesadrm.c
> [ ... ]
>> @@ -484,6 +485,20 @@ static struct vesadrm_device *vesadrm_device_create(struct drm_driver *drv,
>>   	if (drm_edid_header_is_valid(dpy->edid.dummy) == 8)
>>   		sysfb->edid = dpy->edid.dummy;
>>   #endif
>> +
>> +	panel_width = width;
>> +	panel_height = height;
>> +
>> +	if (sysfb->edid) {
>> +		const struct drm_edid *drm_edid;
>> +
>> +		drm_edid = drm_edid_alloc(sysfb->edid, EDID_LENGTH);
>> +		if (drm_edid) {
>> +			drm_edid_get_preferred_size(drm_edid, &panel_width, &panel_height);
> [Severity: Medium]
> Likewise in vesadrm_device_create(), parsing unvalidated EDID data may
> result in bogus resolutions being passed to the panel orientation quirk
> lookup. Should the EDID be fully validated first?
>
>> +			drm_edid_free(drm_edid);
>> +		}
>> +	}
>> +

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others)
  2026-08-27 14:01 [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others) Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2026-08-27 14:01 ` [PATCH 3/3] firmware/sysfb: Remove rotation quirk for Lenovo D330 Thomas Zimmermann
@ 2026-08-27 16:56 ` Ard Biesheuvel
  2026-08-31 10:34   ` Thomas Zimmermann
  3 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2026-08-27 16:56 UTC (permalink / raw)
  To: Thomas Zimmermann, Javier Martinez Canillas, Ilias Apalodimas,
	Jani Nikula, maarten.lankhorst, mripard, Simona Vetter,
	David Airlie
  Cc: dri-devel, linux-efi, sashiko-reviews

Hello Thomas,

On Thu, 27 Aug 2026, at 16:01, Thomas Zimmermann wrote:
> The kernel mis-interprets the display geometry given by the Lenovo
> D330 as incorrect. The values are correct, but the display output
> is rotated by 90°. With the kernel's firmware quirk applied the output
> is garbled.
>
> Remove the firmware quirk for the Lenovo D330 and let DRM's sysfb
> drivers set the correct panel orientation instead. To make this work
> for any display mode, use the panel size for looking up the correct
> orientation.
>
> Tested on a Lenovo D330-10IGM IdeaPad.
>
> Thomas Zimmermann (3):
>   drm/edid: Add drm_edid_get_preferred_size()
>   drm/sysfb: Use preferred panel size for panel orientation quirks
>   firmware/sysfb: Remove rotation quirk for Lenovo D330
>

Acked-by: Ard Biesheuvel <ardb@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others)
  2026-08-27 16:56 ` [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others) Ard Biesheuvel
@ 2026-08-31 10:34   ` Thomas Zimmermann
  0 siblings, 0 replies; 10+ messages in thread
From: Thomas Zimmermann @ 2026-08-31 10:34 UTC (permalink / raw)
  To: Ard Biesheuvel, Javier Martinez Canillas, Ilias Apalodimas,
	Jani Nikula, maarten.lankhorst, mripard, Simona Vetter,
	David Airlie
  Cc: dri-devel, linux-efi, sashiko-reviews

Hi Ard

Am 27.08.26 um 18:56 schrieb Ard Biesheuvel:
> Hello Thomas,
>
> On Thu, 27 Aug 2026, at 16:01, Thomas Zimmermann wrote:
>> The kernel mis-interprets the display geometry given by the Lenovo
>> D330 as incorrect. The values are correct, but the display output
>> is rotated by 90°. With the kernel's firmware quirk applied the output
>> is garbled.
>>
>> Remove the firmware quirk for the Lenovo D330 and let DRM's sysfb
>> drivers set the correct panel orientation instead. To make this work
>> for any display mode, use the panel size for looking up the correct
>> orientation.
>>
>> Tested on a Lenovo D330-10IGM IdeaPad.
>>
>> Thomas Zimmermann (3):
>>    drm/edid: Add drm_edid_get_preferred_size()
>>    drm/sysfb: Use preferred panel size for panel orientation quirks
>>    firmware/sysfb: Remove rotation quirk for Lenovo D330
>>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>

The Sashiko bot warns that the quirk is required on the device's older 
UEFI firmware revisions.  I can confirm that the quirk breaks display 
output for the latest firmware (ver 8NCN43WW).  How is this usually 
being handled? Can I expect affected users to update their firmware?

Best regards
Thomas


-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-08-31 10:34 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-27 14:01 [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others) Thomas Zimmermann
2026-08-27 14:01 ` [PATCH 1/3] drm/edid: Add drm_edid_get_preferred_size() Thomas Zimmermann
2026-08-27 14:19   ` sashiko-bot
2026-08-27 14:40     ` Thomas Zimmermann
2026-08-27 14:01 ` [PATCH 2/3] drm/sysfb: Use preferred panel size for panel orientation quirks Thomas Zimmermann
2026-08-27 14:21   ` sashiko-bot
2026-08-27 14:44     ` Thomas Zimmermann
2026-08-27 14:01 ` [PATCH 3/3] firmware/sysfb: Remove rotation quirk for Lenovo D330 Thomas Zimmermann
2026-08-27 16:56 ` [PATCH 0/3] sysfb: Fix display output on Lenovo D330 (and others) Ard Biesheuvel
2026-08-31 10:34   ` Thomas Zimmermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox