[PATCH v2 18/21] drm/amd/display: Fallback to 2020_YCBCR if the pixel encoding is not RGB

[Harry Wentland]

On 1/24/23 10:37, Harry Wentland wrote:
> 
> 
> On 1/23/23 15:30, Sebastian Wick wrote:
>> A new property to control YCC and subsampling would be the more
>> complete path here. If we actually want to fix this in the short-term
>> though, we should handle the YCC and RGB Colorspace values as
>> equivalent, everywhere. Technically we're breaking the user space API
>> here so it should be documented on the KMS property and other drivers
>> must be adjusted accordingly as well.
>>
> 
> Could someone point me to a userspace that uses this currently?
> 

To elaborate a bit more...

A driver has always had the ability to pick the wire format, whether
it'd be RGB or YCbCr (444, or 420). In some cases that selection
is required in order to satisfy bandwidth requirements. In others
we follow a certain policy to ensure similar behaviors between our
Windows and Linux drivers. I don't think it makes sense for userspace
to control this.

Based on what I see I am not convinced the entirety of the
colorspace definition has a corresponding implementation in an
upstream, canonical userspace, hence my question. Not even an IGT
test existed when I started looking at this. In the absence of a
missing userspace implementation I am not convinced we're breaking
anything. Even then, this was never implemented in amdgpu so
there is no way this regresses any existing behavior.

Harry

> Harry
> 
>> On Fri, Jan 13, 2023 at 5:26 PM Harry Wentland <harry.wentland at amd.com> wrote:
>>>
>>> From: Joshua Ashton <joshua at froggi.es>
>>>
>>> Userspace might not aware whether we're sending RGB or YCbCr
>>> data to the display. If COLOR_SPACE_2020_RGB_FULLRANGE is
>>> requested but the output encoding is YCbCr we should
>>> send COLOR_SPACE_2020_YCBCR.
>>>
>>> Signed-off-by: Joshua Ashton <joshua at froggi.es>
>>> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
>>> Cc: Pekka Paalanen <ppaalanen at gmail.com>
>>> Cc: Sebastian Wick <sebastian.wick at redhat.com>
>>> Cc: Vitaly.Prosyak at amd.com
>>> Cc: Joshua Ashton <joshua at froggi.es>
>>> Cc: dri-devel at lists.freedesktop.org
>>> Cc: amd-gfx at lists.freedesktop.org
>>> Reviewed-by: Harry Wentland <harry.wentland at amd.com>
>>> ---
>>>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 ++++-
>>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> index f74b125af31f..16940ea61b59 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -5184,7 +5184,10 @@ get_output_color_space(const struct dc_crtc_timing *dc_crtc_timing,
>>>                 color_space = COLOR_SPACE_ADOBERGB;
>>>                 break;
>>>         case DRM_MODE_COLORIMETRY_BT2020_RGB:
>>> -               color_space = COLOR_SPACE_2020_RGB_FULLRANGE;
>>> +               if (dc_crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB)
>>> +                       color_space = COLOR_SPACE_2020_RGB_FULLRANGE;
>>> +               else
>>> +                       color_space = COLOR_SPACE_2020_YCBCR;
>>>                 break;
>>>         case DRM_MODE_COLORIMETRY_BT2020_YCC:
>>>                 color_space = COLOR_SPACE_2020_YCBCR;
>>> --
>>> 2.39.0
>>>
>>
>