On Wed, 23 Mar 2022, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > On Wed, Mar 23, 2022 at 12:04:38PM +0200, Jani Nikula wrote: >> Only an EDID CEA extension has byte #3, while the CTA DisplayID Data >> Block does not. Don't interpret bogus data for color formats. > > I think what we might want eventually is a cleaner split between > the CTA data blocks vs. the rest of the EDID CTA ext block. Only > the former is relevant for DisplayID. Well, I just abstracted it all away in the CEA data block iteration series [1]. It'll be possible to add a different iteration initializer depending on what you want to iterate and where. BR, Jani. > > But for a bugfix we want to keep it simple. > > Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >> >> For most displays it's probably an unlikely scenario you'd have a CTA >> DisplayID Data Block without a CEA extension, but they do exist. >> >> Fixes: e28ad544f462 ("drm/edid: parse CEA blocks embedded in DisplayID") >> Cc: <stable@xxxxxxxxxxxxxxx> # v4.15 >> Cc: Shawn C Lee <shawn.c.lee@xxxxxxxxx> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx> >> >> --- >> >> commit e28ad544f462 was merged in v5.3, but it has Cc: stable for v4.15. >> >> This is also fixed in my CEA data block iteration series [1], but we'll >> want the simple fix for stable first. >> >> Hum, CTA is formerly CEA, I and the code seem to use both, should we use >> only one or the other? > > And before CEA it was called EIA (IIRC). Dunno if we also use that name > somewhere. > > If someone cares enough I guess we could rename everything to "cta". > >> >> [1] https://patchwork.freedesktop.org/series/101659/ >> --- >> drivers/gpu/drm/drm_edid.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c >> index 561f53831e29..ccf7031a6797 100644 >> --- a/drivers/gpu/drm/drm_edid.c >> +++ b/drivers/gpu/drm/drm_edid.c >> @@ -5187,10 +5187,14 @@ static void drm_parse_cea_ext(struct drm_connector *connector, >> >> /* The existence of a CEA block should imply RGB support */ >> info->color_formats = DRM_COLOR_FORMAT_RGB444; >> - if (edid_ext[3] & EDID_CEA_YCRCB444) >> - info->color_formats |= DRM_COLOR_FORMAT_YCBCR444; >> - if (edid_ext[3] & EDID_CEA_YCRCB422) >> - info->color_formats |= DRM_COLOR_FORMAT_YCBCR422; >> + >> + /* CTA DisplayID Data Block does not have byte #3 */ >> + if (edid_ext[0] == CEA_EXT) { >> + if (edid_ext[3] & EDID_CEA_YCRCB444) >> + info->color_formats |= DRM_COLOR_FORMAT_YCBCR444; >> + if (edid_ext[3] & EDID_CEA_YCRCB422) >> + info->color_formats |= DRM_COLOR_FORMAT_YCBCR422; >> + } >> >> if (cea_db_offsets(edid_ext, &start, &end)) >> return; >> -- >> 2.30.2 -- Jani Nikula, Intel Open Source Graphics Center