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

[Yaroslav <iam@0la.ch>]

On 2025-11-26 16:06, Yaroslav wrote:
> 
> 
> On 2025-11-26 15:59, Jani Nikula wrote:
>> On Wed, 26 Nov 2025, Yaroslav <iam@0la.ch> wrote:
>>> On 2025-11-26 15:08, Jani Nikula wrote:
>>>> On Wed, 26 Nov 2025, Jani Nikula <jani.nikula@linux.intel.com> wrote:
>>>>> On Wed, 26 Nov 2025, Yaroslav Bolyukin <iam@lach.pw> 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@lach.pw>
>>>>>
>>>>> 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@vger.kernel.org
>>>>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>>>>
>>>> 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,
>>>>
>>>
>>