On Thu, Feb 03, 2022 at 12:54:16PM +0100, Maxime Ripard wrote:
> In order to fill the drm_display_info structure each time an EDID is
> read, the code currently will call drm_add_display_info with the parsed
> EDID.
>
> drm_add_display_info will then call drm_reset_display_info to reset all
> the fields to 0, and then set them to the proper value depending on the
> EDID.
>
> In the color_formats case, we will thus report that we don't support any
> color format, and then fill it back with RGB444 plus the additional
> formats described in the EDID Feature Support byte.
>
> However, since that byte only contains format-related bits since the 1.4
> specification, this doesn't happen if the EDID is following an earlier
> specification. In turn, it means that for one of these EDID, we end up
> with color_formats set to 0.
>
> The EDID 1.3 specification never really specifies what it means by RGB
> exactly, but since both HDMI and DVI will use RGB444, it's fairly safe
> to assume it's supposed to be RGB444.
>
> Let's move the addition of RGB444 to color_formats earlier in
> drm_add_display_info() so that it's always set for a digital display.
>
> Fixes: da05a5a71ad8 ("drm: parse color format support for digital displays")
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Reported-by: Matthias Reichl <hias@xxxxxxxxx>
> Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
Looks OK to me. I guess the DFP1.x == 0 case might allow some
other encoding to be used but the EDID spec makes not mention
how to figure that out.
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> 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..f5f5de362ff2 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -5345,6 +5345,7 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi
> if (!(edid->input & DRM_EDID_INPUT_DIGITAL))
> return quirks;
>
> + info->color_formats |= DRM_COLOR_FORMAT_RGB444;
> drm_parse_cea_ext(connector, edid);
>
> /*
> @@ -5393,7 +5394,6 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi
> DRM_DEBUG("%s: Assigning EDID-1.4 digital sink color depth as %d bpc.\n",
> connector->name, info->bpc);
>
> - info->color_formats |= DRM_COLOR_FORMAT_RGB444;
> if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB444)
> info->color_formats |= DRM_COLOR_FORMAT_YCRCB444;
> if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB422)
> --
> 2.34.1
--
Ville Syrjälä
Intel
