On Mon, 31 Aug 2026, Thomas Zimmermann <[email protected]> wrote:
> Hi
>
> Am 31.08.26 um 15:23 schrieb Jani Nikula:
>> On Mon, 31 Aug 2026, Thomas Zimmermann <[email protected]> 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 <[email protected]>
>>> Acked-by: Ard Biesheuvel <[email protected]>
>>> ---
>>> 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
