Re: [PATCH v6 2/7] drm/edid: prepare for VESA vendor-specific data block extension

[Yaroslav]

On 2025-11-26 16:06, Yaroslav wrote:
> On 2025-11-26 15:59, Jani Nikula wrote:
> > On Wed, 26 Nov 2025, Yaroslav <iam@xxxxxx> wrote:
> > > On 2025-11-26 15:08, Jani Nikula wrote:
> > > > On Wed, 26 Nov 2025, Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
> > > > > On Wed, 26 Nov 2025, Yaroslav Bolyukin <iam@xxxxxxx> wrote:
> > > > > > Current VESA vendor-specific block parsing expects real block size 
> > > > > > to be
> > > > > > the same as the defined struct size, use real offsets in conditionals
> > > > > > instead to add struct fields in future commits.
> > > > > >
> > > > > > Signed-off-by: Yaroslav Bolyukin <iam@xxxxxxx>
> > > > >
> > > > > I think this is something we want to backport, since MSO would break
> > > > > with bigger vendor-specific blocks, and that leads to black screens on
> > > > > MSO displays.
> > > > >
> > > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > > Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> > > >
> > > > Oops, I'll take that back.
> > > >
> > > > > > ---
> > > > > >    drivers/gpu/drm/drm_edid.c | 28 ++++++++++++----------------
> > > > > >    1 file changed, 12 insertions(+), 16 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > > > > index 64f7a94dd9e4..a52fd6de9327 100644
> > > > > > --- a/drivers/gpu/drm/drm_edid.c
> > > > > > +++ b/drivers/gpu/drm/drm_edid.c
> > > > > > @@ -6544,7 +6544,7 @@ static void 
> > > > > > drm_parse_vesa_specific_block(struct drm_connector *connector,
> > > > > >        if (oui(vesa->oui[0], vesa->oui[1], vesa->oui[2]) != 
> > > > > > VESA_IEEE_OUI)
> > > > > >            return;
> > > > > > -    if (sizeof(*vesa) != sizeof(*block) + block->num_bytes) {
> > > > > > +    if (block->num_bytes < 5) {
> > > > > >            drm_dbg_kms(connector->dev,
> > > > > >                    "[CONNECTOR:%d:%s] Unexpected VESA vendor block 
> > > > > > size\n",
> > > > > >                    connector->base.id, connector->name);
> > > > > > @@ -6567,24 +6567,20 @@ static void 
> > > > > > drm_parse_vesa_specific_block(struct drm_connector *connector,
> > > > > >            break;
> > > > > >        }
> > > > > > -    if (!info->mso_stream_count) {
> > > > > > -        info->mso_pixel_overlap = 0;
> > > >
> > > > This is no longer cleared for !info->mso_stream_count.
> > > >
> > > > Perhaps the code could be reorganized to handle it better.
> > >
> > > It defaults to zero due to drm_reset_display_info()
> >
> > Yes, and the code above the context initializes it from the vendor
> > block.
>
> It is wrapped in `if (info->mso_stream_count)` so we get
>
> /* From drm_reset_display_info */
> info->mso_pixel_overlap = 0;
>
> /* This part was removed, as we don't want the early return here anymore 
> due to extended structure length checks
> if (!info->mso_stream_count) {
>    info->mso_pixel_overlap = 0;
>    return;
> } */
>
>
> /* This code is right below your comment */
> if (info->mso_stream_count) {
>    info->mso_pixel_overlap = ...;
>    ...
> }
>
> /* I can reinsert the value reset explicitly here, if that's what you 
> suggesting to do:
>
> else {
>    info->mso_pixel_overlap = 0;
> }*/

For reference, repository state at the point after this patch:

https://github.com/CertainLach/linux-1/blob/01b0097171ebe85ae124a7b535dce0b46a1dd8e2/drivers/gpu/drm/drm_edid.c#L6561-L6593

Commit: 
https://github.com/CertainLach/linux-1/commit/01b0097171ebe85ae124a7b535dce0b46a1dd8e2

(Note that the fix for DP/EDP mistake is already here, but nothing else 
was changed in the github version of this patch)

> > > > > > -        return;
> > > > > > -    }
> > > > > > -
> > > > > > -    info->mso_pixel_overlap = 
> > > > > > FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, vesa->mso);
> > > > > > -    if (info->mso_pixel_overlap > 8) {
> > > > > > +    if (info->mso_stream_count) {
> > > > > > +        info->mso_pixel_overlap = 
> > > > > > FIELD_GET(DISPLAYID_VESA_MSO_OVERLAP, vesa->mso);
> > > > > > +        if (info->mso_pixel_overlap > 8) {
> > > > > > +            drm_dbg_kms(connector->dev,
> > > > > > +                    "[CONNECTOR:%d:%s] Reserved MSO pixel overlap 
> > > > > > value %u\n",
> > > > > > +                    connector->base.id, connector->name,
> > > > > > +                    info->mso_pixel_overlap);
> > > > > > +            info->mso_pixel_overlap = 8;
> > > > > > +        }
> > > > > >            drm_dbg_kms(connector->dev,
> > > > > > -                "[CONNECTOR:%d:%s] Reserved MSO pixel overlap 
> > > > > > value %u\n",
> > > > > > +                "[CONNECTOR:%d:%s] MSO stream count %u, pixel 
> > > > > > overlap %u\n",
> > > > > >                    connector->base.id, connector->name,
> > > > > > -                info->mso_pixel_overlap);
> > > > > > -        info->mso_pixel_overlap = 8;
> > > > > > +                info->mso_stream_count, info->mso_pixel_overlap);
> > > > > >        }
> > > > > > -
> > > > > > -    drm_dbg_kms(connector->dev,
> > > > > > -            "[CONNECTOR:%d:%s] MSO stream count %u, pixel overlap 
> > > > > > %u\n",
> > > > > > -            connector->base.id, connector->name,
> > > > > > -            info->mso_stream_count, info->mso_pixel_overlap);
> > > > > >    }
> > > > > >    static void drm_update_vesa_specific_block(struct drm_connector 
> > > > > > *connector,