[PATCH] drm/edid: Refine HDMI VSDB detect

[Lee, Shawn C]

On Thursday, December 23, 2021 1:02 PM, Ville Syrjälä wrote:
>On Sun, Dec 12, 2021 at 11:33:31PM +0800, Lee Shawn C wrote:
>> According to CEA-861-F chapter 7.5.4. It says "The VSDB shall contain 
>> the
>> 3 bytes of the IEEE OUI as well as any additional payload bytes needed."
>> Now DRM driver check HDMI OUI but VSDB payload size at least five bytes.
>> That may caused some HDMI monitors' audio feature can't be enabled.
>> Because of they only have three bytes payload (OUI only) in VSDB.
>> 
>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>> Cc: Adam Jackson <ajax at redhat.com>
>> Cc: Dave Airlie <airlied at redhat.com>
>> Signed-off-by: Lee Shawn C <shawn.c.lee at intel.com>
>> ---
>>  drivers/gpu/drm/drm_edid.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c 
>> index 12893e7be89b..5aa4a6bf4a13 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -4205,7 +4205,7 @@ static bool cea_db_is_hdmi_vsdb(const u8 *db)
>>  	if (cea_db_tag(db) != VENDOR_BLOCK)
>>  		return false;
>>  
>> -	if (cea_db_payload_len(db) < 5)
>> +	if (cea_db_payload_len(db) < 3)
>>  		return false;
>
>I was a a bit worried that we are now assuming that we can parse some
>stuff without further length checks, but looks like that's just the
>"source physical address" stuff which we do not parse in drm_edid.c,
>and the other fields we do parse are actually optional and so already
>have the require length checks. So this seems fine.
>

Thanks for the comments! We will share the information to customer.

Best regards,
Shawn

>Closes: https://gitlab.freedesktop.org/drm/misc/-/issues/28
>Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
>Note that there is a second edid parser in cec_get_edid_spa_location()
>that does parse the source physical address. You may want to double
>check that it doesn't make any bad assumptions about the length
>of the vsdb either. I think we should probably get rid of that
>second parser and just have drm_edid.c extract the source physical
>address and pass that on directly to the cec code instead. But I
>guess the cec code still couldn't remove the second parser
>since some media drivers need it :( Though it could then perhaps
>be moved into the media code instead of having it as a massive
>inline function in the cec headers.
>
>--
>Ville Syrjälä
>Intel