[PATCH v2 0/3] sysfb: Fix display output on Lenovo D330 (and others)

[PATCH v2 0/3] sysfb: Fix display output on Lenovo D330 (and others)

[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.

v2:
- reorder patches to have the immediate fix first
- detect and handle broken EDID better (Sashiko)
- fix width/height pointer check (Sashiko)

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

 drivers/firmware/efi/sysfb_efi.c |  9 -------
 drivers/gpu/drm/drm_edid.c       | 46 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/sysfb/efidrm.c   | 17 +++++++++++-
 drivers/gpu/drm/sysfb/ofdrm.c    | 18 +++++++++++--
 drivers/gpu/drm/sysfb/vesadrm.c  | 17 +++++++++++-
 include/drm/drm_edid.h           |  2 ++
 6 files changed, 96 insertions(+), 13 deletions(-)


base-commit: edb0a89a038079d0d86e768b30a72d053d3e39e0
-- 
2.55.0

[PATCH v2 1/3] firmware/sysfb: Remove rotation quirk for Lenovo D330

[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>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
---
 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

Re: [PATCH v2 1/3] firmware/sysfb: Remove rotation quirk for Lenovo D330

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the sysfb swap quirk entirely will break display rendering and panel orientation detection for Lenovo D330 devices running older firmware.
--

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

firmware/sysfb: Remove rotation quirk for Lenovo D330

This commit removes the DMI match for the Lenovo D330 in sysfb_efi.c, which
previously swapped display width and height. The intent is to keep the
display as-is so DRM drivers can pick the correct panel orientation.

> diff --git a/drivers/firmware/efi/sysfb_efi.c b/drivers/firmware/efi/sysfb_efi.c
> --- 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,
> -	},

[Severity: High]
Will removing this DMI match entirely break display rendering for Lenovo D330
devices running older firmware?

While newer BIOS versions correctly report the native portrait geometry
(making the previous unconditional swap incorrect), older BIOS versions
genuinely reported the wrong landscape EFI geometry and relied on this quirk
to fix the lfb_width, lfb_height, and lfb_linelength fields. 

Unconditionally removing the DMI match appears to fix newer BIOSes but
introduces a regression for older BIOSes where early boot framebuffers
(sysfb/simpledrm) might use the incorrect EFI-reported dimensions. This can
result in severely corrupted display output (garbage on screen) until a
native DRM driver loads.

Since later patches in this series updating the panel orientation quirk logic
do not correct the underlying fb_mode and fb_pitch used by drm_sysfb_mode(),
would it be safer to retain the quirk but modify the callback to only swap if
the reported width is greater than the height (e.g., if
si->lfb_width > si->lfb_height)?

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

Re: [PATCH v2 1/3] firmware/sysfb: Remove rotation quirk for Lenovo D330

[Jani Nikula]

On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> The Lenovo D330 is a notebook with unmountable display. The display

Please elaborate on what you mean by "unmountable display".

No comments on the patch itself, I just haven't heard that before.

> 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>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  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

-- 
Jani Nikula, Intel

Re: [PATCH v2 1/3] firmware/sysfb: Remove rotation quirk for Lenovo D330

[Thomas Zimmermann]

Am 31.08.26 um 14:49 schrieb Jani Nikula:
> On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> The Lenovo D330 is a notebook with unmountable display. The display
> Please elaborate on what you mean by "unmountable display".
>
> No comments on the patch itself, I just haven't heard that before.

Sorry, that word doesn't exist; maybe "mountable".  I think "detachable" 
is the correct word.  The notebook's display can be removed from the 
keyboard to act like a tablet.

Best regards
Thomas

>
>> 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>
>> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>> ---
>>   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

-- 
--
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)

Re: [PATCH v2 1/3] firmware/sysfb: Remove rotation quirk for Lenovo D330

[Jani Nikula]

On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Am 31.08.26 um 14:49 schrieb Jani Nikula:
>> On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>> The Lenovo D330 is a notebook with unmountable display. The display
>> Please elaborate on what you mean by "unmountable display".
>>
>> No comments on the patch itself, I just haven't heard that before.
>
> Sorry, that word doesn't exist; maybe "mountable".  I think "detachable" 
> is the correct word.  The notebook's display can be removed from the 
> keyboard to act like a tablet.

Ah, *lightbulb*. :)

Though arguably it's the keyboard that's detachable, as the display is
the main part that keeps working without the keyboard, not the other way
round.

BR,
Jani.


-- 
Jani Nikula, Intel

[PATCH v2 2/3] drm/edid: Add drm_edid_get_preferred_size()

[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.

v2:
- handle EDID without pixel timing descriptor (Sashiko)
- fix checks for width and height pointers

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/gpu/drm/drm_edid.c | 46 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_edid.h     |  2 ++
 2 files changed, 48 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 9990a836d0b6..53c7eed7a73e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -2872,6 +2872,52 @@ 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_timing *dt;
+	const struct detailed_pixel_timing *pt;
+
+	if (drm_edid->size < EDID_LENGTH)
+		return -EINVAL;
+
+	/*
+	 * Use display mode from the Preferred Timing Descriptor. 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 */
+
+	dt = &edid->detailed_timings[0];
+
+	if (!le16_to_cpu(dt->pixel_clock))
+		return -EINVAL;
+
+	pt = &dt->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

Re: [PATCH v2 2/3] drm/edid: Add drm_edid_get_preferred_size()

[Jani Nikula]

On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 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.
>
> v2:
> - handle EDID without pixel timing descriptor (Sashiko)
> - fix checks for width and height pointers
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/gpu/drm/drm_edid.c | 46 ++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_edid.h     |  2 ++
>  2 files changed, 48 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 9990a836d0b6..53c7eed7a73e 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -2872,6 +2872,52 @@ 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.

I think the special use case for this function should be mentioned. In
the regular EDID parsing case, the connector will include the modes, one
of them will be preferred, and that should be, uh, preferred.

I dislike adding multiple functions for parsing the same thing, but
looks like it can't be helped here. Let's at least avoid adding users
for this in the paths that can use the normal stuff?

> + *
> + * 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_timing *dt;
> +	const struct detailed_pixel_timing *pt;
> +
> +	if (drm_edid->size < EDID_LENGTH)
> +		return -EINVAL;

This is guaranteed. drm_edid_alloc() intentionally won't hand you out a
struct drm_edid with size smaller than EDID_LENGTH. Maybe check for
!drm_edid instead.

> +
> +	/*
> +	 * Use display mode from the Preferred Timing Descriptor. For old
> +	 * and obscure displays, we might need better heuristics.
> +	 */
> +
> +	if (edid->revision < 4 && !(edid->features & DRM_EDID_FEATURE_PREFERRED_TIMING))

The preferred timing mode should always be there starting from EDID
1.3. The bit doesn't indicate whether it's there or not. It should be
always set for EDID 1.3, and for EDID 1.4 it indicates whether the
preferred timing includes the native pixel format and the preferred
refresh rate.

I think just if (edid->revision < 3) should be enough.

> +		return -EINVAL; /* no Preferred Timing Descriptor */
> +
> +	dt = &edid->detailed_timings[0];
> +
> +	if (!le16_to_cpu(dt->pixel_clock))

There's is_detailed_timing_descriptor() for this.

> +		return -EINVAL;
> +
> +	pt = &dt->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);

-- 
Jani Nikula, Intel

Re: [PATCH v2 2/3] drm/edid: Add drm_edid_get_preferred_size()

[Thomas Zimmermann]

Hi

Am 31.08.26 um 15:13 schrieb Jani Nikula:
> On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> 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.
>>
>> v2:
>> - handle EDID without pixel timing descriptor (Sashiko)
>> - fix checks for width and height pointers
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>> ---
>>   drivers/gpu/drm/drm_edid.c | 46 ++++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_edid.h     |  2 ++
>>   2 files changed, 48 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 9990a836d0b6..53c7eed7a73e 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -2872,6 +2872,52 @@ 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.
> I think the special use case for this function should be mentioned. In
> the regular EDID parsing case, the connector will include the modes, one
> of them will be preferred, and that should be, uh, preferred.
>
> I dislike adding multiple functions for parsing the same thing, but
> looks like it can't be helped here. Let's at least avoid adding users
> for this in the paths that can use the normal stuff?

Absolutely.  The only reason I added this helper is that the driver 
needs the size for setting up the display pipeline before the connector 
exists. I looked into using the existing code without the connector, but 
that refactoring seemed to do more harm than good. Hence the new helper.

Should the new function be named drm_edid_detect_panel_size() to signal 
the use case?


>
>> + *
>> + * 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_timing *dt;
>> +	const struct detailed_pixel_timing *pt;
>> +
>> +	if (drm_edid->size < EDID_LENGTH)
>> +		return -EINVAL;
> This is guaranteed. drm_edid_alloc() intentionally won't hand you out a
> struct drm_edid with size smaller than EDID_LENGTH. Maybe check for
> !drm_edid instead.

Ok.

>
>> +
>> +	/*
>> +	 * Use display mode from the Preferred Timing Descriptor. For old
>> +	 * and obscure displays, we might need better heuristics.
>> +	 */
>> +
>> +	if (edid->revision < 4 && !(edid->features & DRM_EDID_FEATURE_PREFERRED_TIMING))
> The preferred timing mode should always be there starting from EDID
> 1.3. The bit doesn't indicate whether it's there or not. It should be
> always set for EDID 1.3, and for EDID 1.4 it indicates whether the
> preferred timing includes the native pixel format and the preferred
> refresh rate.
>
> I think just if (edid->revision < 3) should be enough.

Ok

>
>> +		return -EINVAL; /* no Preferred Timing Descriptor */
>> +
>> +	dt = &edid->detailed_timings[0];
>> +
>> +	if (!le16_to_cpu(dt->pixel_clock))
> There's is_detailed_timing_descriptor() for this.

Great!

Best regards
Thomas

>
>> +		return -EINVAL;
>> +
>> +	pt = &dt->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);

-- 
--
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)

Re: [PATCH v2 2/3] drm/edid: Add drm_edid_get_preferred_size()

[Jani Nikula]

On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Absolutely.  The only reason I added this helper is that the driver 
> needs the size for setting up the display pipeline before the connector 
> exists. I looked into using the existing code without the connector, but 
> that refactoring seemed to do more harm than good. Hence the new helper.

Heh, I looked into that too while reviewing, and agreed.

> Should the new function be named drm_edid_detect_panel_size() to signal 
> the use case?

Surprisingly, no strong feelings either way, both work for me.


BR,
Jani.


-- 
Jani Nikula, Intel

Re: [PATCH v2 2/3] drm/edid: Add drm_edid_get_preferred_size()

[Jani Nikula]

On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi
>
> Am 31.08.26 um 15:13 schrieb Jani Nikula:
>> On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>> +
>>> +	/*
>>> +	 * Use display mode from the Preferred Timing Descriptor. For old
>>> +	 * and obscure displays, we might need better heuristics.
>>> +	 */
>>> +
>>> +	if (edid->revision < 4 && !(edid->features & DRM_EDID_FEATURE_PREFERRED_TIMING))
>> The preferred timing mode should always be there starting from EDID
>> 1.3. The bit doesn't indicate whether it's there or not. It should be
>> always set for EDID 1.3, and for EDID 1.4 it indicates whether the
>> preferred timing includes the native pixel format and the preferred
>> refresh rate.
>>
>> I think just if (edid->revision < 3) should be enough.
>
> Ok

I think I may have gotten this one wrong, and your original was correct.

I didn't double check what the EDID spec definition of pixel format is:
"Native Pixel Format is defined as the number of physical pixels along
the horizontal axis by the number of physical pixels along the vertical
axis of the display device."

i.e. exactly what you're looking for. Usually, I would not consider
"pixel format" to include resolution.

The preferred timing will still be there as I described, and I would
*guess* preferred most likely equals native, but your original is the
more cautious approach.

My bad, sorry for the noise.


BR,
Jani.


-- 
Jani Nikula, Intel

[PATCH v2 3/3] drm/sysfb: Use preferred panel size for panel orientation quirks

[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.

v2:
- ofdrm: validate EDID header before using it (Sashiko)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/gpu/drm/sysfb/efidrm.c  | 17 ++++++++++++++++-
 drivers/gpu/drm/sysfb/ofdrm.c   | 18 ++++++++++++++++--
 drivers/gpu/drm/sysfb/vesadrm.c | 17 ++++++++++++++++-
 3 files changed, 48 insertions(+), 4 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 9d60db45139c..a0a865aee098 100644
--- a/drivers/gpu/drm/sysfb/ofdrm.c
+++ b/drivers/gpu/drm/sysfb/ofdrm.c
@@ -229,7 +229,7 @@ static const u8 *display_get_edid_of(struct drm_device *dev, struct device_node
 {
 	int ret = of_property_read_u8_array(of_node, "EDID", buf, EDID_LENGTH);
 
-	if (ret)
+	if (ret || drm_edid_header_is_valid(buf) != 8)
 		return NULL;
 	return buf;
 }
@@ -828,6 +828,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;
@@ -998,6 +999,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);
@@ -1069,7 +1083,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

Re: [PATCH v2 3/3] drm/sysfb: Use preferred panel size for panel orientation quirks

[Jani Nikula]

On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> 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.
>
> v2:
> - ofdrm: validate EDID header before using it (Sashiko)
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/gpu/drm/sysfb/efidrm.c  | 17 ++++++++++++++++-
>  drivers/gpu/drm/sysfb/ofdrm.c   | 18 ++++++++++++++++--
>  drivers/gpu/drm/sysfb/vesadrm.c | 17 ++++++++++++++++-
>  3 files changed, 48 insertions(+), 4 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);
> +		}
> +	}
> +

Unrelated to the patch at hand, but does all of this code assume EDID is
always just one block? Scary. The whole sysfs->edid being a u8 pointer
without size feels like a footgun.

Here, the above should work, but I've been kind of hoping to eradicate
EDID_LENGTH assumptions and calculations from everywhere outside of
drm_edid.c, because it's complicated.


BR,
Jani.


>  	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 9d60db45139c..a0a865aee098 100644
> --- a/drivers/gpu/drm/sysfb/ofdrm.c
> +++ b/drivers/gpu/drm/sysfb/ofdrm.c
> @@ -229,7 +229,7 @@ static const u8 *display_get_edid_of(struct drm_device *dev, struct device_node
>  {
>  	int ret = of_property_read_u8_array(of_node, "EDID", buf, EDID_LENGTH);
>  
> -	if (ret)
> +	if (ret || drm_edid_header_is_valid(buf) != 8)
>  		return NULL;
>  	return buf;
>  }
> @@ -828,6 +828,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;
> @@ -998,6 +999,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);
> @@ -1069,7 +1083,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);

-- 
Jani Nikula, Intel

Re: [PATCH v2 3/3] drm/sysfb: Use preferred panel size for panel orientation quirks

[Thomas Zimmermann]

Hi

Am 31.08.26 um 15:23 schrieb Jani Nikula:
> On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> 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.
>>
>> v2:
>> - ofdrm: validate EDID header before using it (Sashiko)
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>> ---
>>   drivers/gpu/drm/sysfb/efidrm.c  | 17 ++++++++++++++++-
>>   drivers/gpu/drm/sysfb/ofdrm.c   | 18 ++++++++++++++++--
>>   drivers/gpu/drm/sysfb/vesadrm.c | 17 ++++++++++++++++-
>>   3 files changed, 48 insertions(+), 4 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);
>> +		}
>> +	}
>> +
> Unrelated to the patch at hand, but does all of this code assume EDID is
> always just one block? Scary. The whole sysfs->edid being a u8 pointer
> without size feels like a footgun.

For UEFI and VESA it's always one block of 128 bytes in size. The boot 
parameters hard-code this value and there's no space for additional 
blocks. [1]  On OpenFirmware, ofdrm checks the size to be EDID_LENGTH. 
[2] I guess there could more blocks there, but AFAIK the DT property 
only exists on ancient PPC MACs. I have no means of testing, but the 
test file I obtained from such a system is 128 bytes.

[1] 
https://elixir.bootlin.com/linux/v7.2.2/source/arch/x86/include/uapi/asm/bootparam.h#L133
[2] 
https://elixir.bootlin.com/linux/v7.2.2/source/drivers/gpu/drm/sysfb/ofdrm.c#L229


>
> Here, the above should work, but I've been kind of hoping to eradicate
> EDID_LENGTH assumptions and calculations from everywhere outside of
> drm_edid.c, because it's complicated.

We need some way of importing the raw data block into drm_edid.c, which 
in turn needs the buffer size. The drivers could do this without 
EDID_LENGTH if that helps. Note that the drivers use the regular 
machinery from drm_edid.c for dissecting the EDID block. See [3].

[3] 
https://elixir.bootlin.com/linux/v7.2.2/source/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c#L594

Best regards
Thomas


>
>
> BR,
> Jani.
>
>
>>   	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 9d60db45139c..a0a865aee098 100644
>> --- a/drivers/gpu/drm/sysfb/ofdrm.c
>> +++ b/drivers/gpu/drm/sysfb/ofdrm.c
>> @@ -229,7 +229,7 @@ static const u8 *display_get_edid_of(struct drm_device *dev, struct device_node
>>   {
>>   	int ret = of_property_read_u8_array(of_node, "EDID", buf, EDID_LENGTH);
>>   
>> -	if (ret)
>> +	if (ret || drm_edid_header_is_valid(buf) != 8)
>>   		return NULL;
>>   	return buf;
>>   }
>> @@ -828,6 +828,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;
>> @@ -998,6 +999,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);
>> @@ -1069,7 +1083,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);

-- 
--
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)

Re: [PATCH v2 3/3] drm/sysfb: Use preferred panel size for panel orientation quirks

[Jani Nikula]

On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi
>
> Am 31.08.26 um 15:23 schrieb Jani Nikula:
>> On Mon, 31 Aug 2026, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>> 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.
>>>
>>> v2:
>>> - ofdrm: validate EDID header before using it (Sashiko)
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Acked-by: Ard Biesheuvel <ardb@kernel.org>
>>> ---
>>>   drivers/gpu/drm/sysfb/efidrm.c  | 17 ++++++++++++++++-
>>>   drivers/gpu/drm/sysfb/ofdrm.c   | 18 ++++++++++++++++--
>>>   drivers/gpu/drm/sysfb/vesadrm.c | 17 ++++++++++++++++-
>>>   3 files changed, 48 insertions(+), 4 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);
>>> +		}
>>> +	}
>>> +
>> Unrelated to the patch at hand, but does all of this code assume EDID is
>> always just one block? Scary. The whole sysfs->edid being a u8 pointer
>> without size feels like a footgun.
>
> For UEFI and VESA it's always one block of 128 bytes in size. The boot 
> parameters hard-code this value and there's no space for additional 
> blocks. [1]  On OpenFirmware, ofdrm checks the size to be EDID_LENGTH. 
> [2] I guess there could more blocks there, but AFAIK the DT property 
> only exists on ancient PPC MACs. I have no means of testing, but the 
> test file I obtained from such a system is 128 bytes.
>
> [1] 
> https://elixir.bootlin.com/linux/v7.2.2/source/arch/x86/include/uapi/asm/bootparam.h#L133
> [2] 
> https://elixir.bootlin.com/linux/v7.2.2/source/drivers/gpu/drm/sysfb/ofdrm.c#L229
>
>
>>
>> Here, the above should work, but I've been kind of hoping to eradicate
>> EDID_LENGTH assumptions and calculations from everywhere outside of
>> drm_edid.c, because it's complicated.
>
> We need some way of importing the raw data block into drm_edid.c, which 
> in turn needs the buffer size. The drivers could do this without 
> EDID_LENGTH if that helps. Note that the drivers use the regular 
> machinery from drm_edid.c for dissecting the EDID block. See [3].
>
> [3] 
> https://elixir.bootlin.com/linux/v7.2.2/source/drivers/gpu/drm/sysfb/drm_sysfb_modeset.c#L594

Okay, thanks for the pointers.

BR,
Jani.


>
> Best regards
> Thomas
>
>
>>
>>
>> BR,
>> Jani.
>>
>>
>>>   	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 9d60db45139c..a0a865aee098 100644
>>> --- a/drivers/gpu/drm/sysfb/ofdrm.c
>>> +++ b/drivers/gpu/drm/sysfb/ofdrm.c
>>> @@ -229,7 +229,7 @@ static const u8 *display_get_edid_of(struct drm_device *dev, struct device_node
>>>   {
>>>   	int ret = of_property_read_u8_array(of_node, "EDID", buf, EDID_LENGTH);
>>>   
>>> -	if (ret)
>>> +	if (ret || drm_edid_header_is_valid(buf) != 8)
>>>   		return NULL;
>>>   	return buf;
>>>   }
>>> @@ -828,6 +828,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;
>>> @@ -998,6 +999,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);
>>> @@ -1069,7 +1083,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);

-- 
Jani Nikula, Intel