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

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3426 bytes --]

On Wed, 25 Jan 2023 12:59:53 +0000
Joshua Ashton <joshua@froggi.es> wrote:

> On 1/23/23 20: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.  
> 
> I am happy with treating 2020_YCC and 2020_RGB as the same.
> 
> I think having the YCC/RGB split here in Colorspace is a mistake.
> Pixel encoding should be completely separate from colorspace from a uAPI 
> perspective when we want to expose that.
> It's just a design flaw from when it was added as it just mirrors the 
> values in the colorimetry packets 1:1. I understand why this happened, 
> and I don't think it's a big deal for us to correct ourselves now:
> 
> I suggest we deprecate the _YCC variants, treat them the same as the RGB 
> enum to avoid any potential uAPI breakage and key the split entirely off 
> the pixel_encoding value.
> 
> That way when we do want to plumb more explicit pixel encoding down the 
> line, userspace only has to manage one thing. There's no advantage for 
> anything more here.

Sounds good to me!


Thanks,
pq

> 
> - Joshie 🐸✨
> 
> > 
> > On Fri, Jan 13, 2023 at 5:26 PM Harry Wentland <harry.wentland@amd.com> wrote:  
> >>
> >> From: Joshua Ashton <joshua@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@froggi.es>
> >> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> >> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> >> Cc: Sebastian Wick <sebastian.wick@redhat.com>
> >> Cc: Vitaly.Prosyak@amd.com
> >> Cc: Joshua Ashton <joshua@froggi.es>
> >> Cc: dri-devel@lists.freedesktop.org
> >> Cc: amd-gfx@lists.freedesktop.org
> >> Reviewed-by: Harry Wentland <harry.wentland@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
> >>  
> >   


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]