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

[Thomas Zimmermann <tzimmermann@suse.de>]

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)