Re: [PATCH] drm/edid: Always set RGB444

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux