Re: [PATCH] drm: parse color format support for digital displays

[Adam Jackson <ajax@redhat.com>]

On 4/15/11 2:40 PM, Jesse Barnes wrote:

> @@ -1461,6 +1462,15 @@ static void drm_add_display_info(struct edid *edid,
>   		info->bpp = 0;
>   		break;
>   	}
> +
> +	if (edid->features & DRM_EDID_FEATURE_RGB)
> +		info->color_formats = DRM_COLOR_FORMAT_RGB444;
> +	else if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB444)
> +		info->color_formats = DRM_COLOR_FORMAT_RGB444 |
> +			DRM_COLOR_FORMAT_YCRCB444;
> +	else if (edid->features & DRM_EDID_FEATURE_RGB_YCRCB)
> +		info->color_formats = DRM_COLOR_FORMAT_RGB444 |
> +			DRM_COLOR_FORMAT_YCRCB444 | DRM_COLOR_FORMAT_YCRCB422;

Overcomplicated (RGB is numerically 0, YCRCB is just two other values 
or'd together) and wrong (doesn't handle YCbCr 4:2:2 alone).  You want:

	info->color_formats = DRM_COLOR_FORMAT_RGB444;
	if (edid->features & DRM_EDID_FEATURE_YCRCB444)
		info->color_formats |= DRM_COLOR_FORMAT_YCBCR444;
	if (edid->features & DRM_EDID_FEATURE_YCRCB422)
		info->color_formats |= DRM_COLOR_FORMAT_YCBCR422;

... which is corrected to not include RGB uselessly in the 
DRM_EDID_FEATURE_* tokens.  I should have noticed that in your first 
patch, whoops.

- ajax