Re: [PATCH v3 11/17] drm/amd/display: Send correct DP colorspace infopacket

2023-03-08 Thread Sebastian Wick
On Tue, Mar 7, 2023 at 4:12 PM Harry Wentland  wrote:
>
> Look at connector->colorimetry to determine output colorspace.
>
> We don't want to impact current SDR behavior, so
> DRM_MODE_COLORIMETRY_DEFAULT preserves current behavior.
>
> Signed-off-by: Harry Wentland 
> Cc: Pekka Paalanen 
> Cc: Sebastian Wick 
> Cc: vitaly.pros...@amd.com
> Cc: Joshua Ashton 
> Cc: dri-de...@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> Reviewed-By: Joshua Ashton 
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 38 +++
>  1 file changed, 22 insertions(+), 16 deletions(-)
>
> 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 58fc719bec8d..cdfd09d50ee6 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -5302,21 +5302,21 @@ get_aspect_ratio(const struct drm_display_mode 
> *mode_in)
>  }
>
>  static enum dc_color_space
> -get_output_color_space(const struct dc_crtc_timing *dc_crtc_timing)
> +get_output_color_space(const struct dc_crtc_timing *dc_crtc_timing,
> +  const struct drm_connector_state *connector_state)
>  {
> enum dc_color_space color_space = COLOR_SPACE_SRGB;
>
> -   switch (dc_crtc_timing->pixel_encoding) {
> -   case PIXEL_ENCODING_YCBCR422:
> -   case PIXEL_ENCODING_YCBCR444:
> -   case PIXEL_ENCODING_YCBCR420:
> -   {
> +   switch (connector_state->colorspace) {
> +   case DRM_MODE_COLORIMETRY_DEFAULT: // ITU601

So, I do get random behavior with DRM_MODE_COLORIMETRY_DEFAULT instead
of the colorimetry that the EDID specifies? That doesn't sound good at
all.

> +   if (dc_crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB) {
> +   color_space = COLOR_SPACE_SRGB;
> /*
>  * 27030khz is the separation point between HDTV and SDTV
>  * according to HDMI spec, we use YCbCr709 and YCbCr601
>  * respectively
>  */
> -   if (dc_crtc_timing->pix_clk_100hz > 270300) {
> +   } else if (dc_crtc_timing->pix_clk_100hz > 270300) {
> if (dc_crtc_timing->flags.Y_ONLY)
> color_space =
> COLOR_SPACE_YCBCR709_LIMITED;
> @@ -5329,15 +5329,21 @@ get_output_color_space(const struct dc_crtc_timing 
> *dc_crtc_timing)
> else
> color_space = COLOR_SPACE_YCBCR601;
> }
> -
> -   }
> -   break;
> -   case PIXEL_ENCODING_RGB:
> -   color_space = COLOR_SPACE_SRGB;
> break;
> -
> -   default:
> -   WARN_ON(1);
> +   case DRM_MODE_COLORIMETRY_BT709_YCC:
> +   if (dc_crtc_timing->flags.Y_ONLY)
> +   color_space = COLOR_SPACE_YCBCR709_LIMITED;
> +   else
> +   color_space = COLOR_SPACE_YCBCR709;
> +   break;
> +   case DRM_MODE_COLORIMETRY_OPRGB:
> +   color_space = COLOR_SPACE_ADOBERGB;
> +   break;
> +   case DRM_MODE_COLORIMETRY_BT2020:
> +   color_space = COLOR_SPACE_2020_RGB_FULLRANGE;
> +   break;
> +   case DRM_MODE_COLORIMETRY_BT2020_DEPRECATED:
> +   color_space = COLOR_SPACE_2020_YCBCR;
> break;
> }
>
> @@ -5476,7 +5482,7 @@ static void 
> fill_stream_properties_from_drm_display_mode(
> }
> }
>
> -   stream->output_color_space = get_output_color_space(timing_out);
> +   stream->output_color_space = get_output_color_space(timing_out, 
> connector_state);
>  }
>
>  static void fill_audio_info(struct audio_info *audio_info,
> --
> 2.39.2
>



[PATCH v3 11/17] drm/amd/display: Send correct DP colorspace infopacket

2023-03-07 Thread Harry Wentland
Look at connector->colorimetry to determine output colorspace.

We don't want to impact current SDR behavior, so
DRM_MODE_COLORIMETRY_DEFAULT preserves current behavior.

Signed-off-by: Harry Wentland 
Cc: Pekka Paalanen 
Cc: Sebastian Wick 
Cc: vitaly.pros...@amd.com
Cc: Joshua Ashton 
Cc: dri-de...@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
Reviewed-By: Joshua Ashton 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 38 +++
 1 file changed, 22 insertions(+), 16 deletions(-)

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 58fc719bec8d..cdfd09d50ee6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5302,21 +5302,21 @@ get_aspect_ratio(const struct drm_display_mode *mode_in)
 }
 
 static enum dc_color_space
-get_output_color_space(const struct dc_crtc_timing *dc_crtc_timing)
+get_output_color_space(const struct dc_crtc_timing *dc_crtc_timing,
+  const struct drm_connector_state *connector_state)
 {
enum dc_color_space color_space = COLOR_SPACE_SRGB;
 
-   switch (dc_crtc_timing->pixel_encoding) {
-   case PIXEL_ENCODING_YCBCR422:
-   case PIXEL_ENCODING_YCBCR444:
-   case PIXEL_ENCODING_YCBCR420:
-   {
+   switch (connector_state->colorspace) {
+   case DRM_MODE_COLORIMETRY_DEFAULT: // ITU601
+   if (dc_crtc_timing->pixel_encoding == PIXEL_ENCODING_RGB) {
+   color_space = COLOR_SPACE_SRGB;
/*
 * 27030khz is the separation point between HDTV and SDTV
 * according to HDMI spec, we use YCbCr709 and YCbCr601
 * respectively
 */
-   if (dc_crtc_timing->pix_clk_100hz > 270300) {
+   } else if (dc_crtc_timing->pix_clk_100hz > 270300) {
if (dc_crtc_timing->flags.Y_ONLY)
color_space =
COLOR_SPACE_YCBCR709_LIMITED;
@@ -5329,15 +5329,21 @@ get_output_color_space(const struct dc_crtc_timing 
*dc_crtc_timing)
else
color_space = COLOR_SPACE_YCBCR601;
}
-
-   }
-   break;
-   case PIXEL_ENCODING_RGB:
-   color_space = COLOR_SPACE_SRGB;
break;
-
-   default:
-   WARN_ON(1);
+   case DRM_MODE_COLORIMETRY_BT709_YCC:
+   if (dc_crtc_timing->flags.Y_ONLY)
+   color_space = COLOR_SPACE_YCBCR709_LIMITED;
+   else
+   color_space = COLOR_SPACE_YCBCR709;
+   break;
+   case DRM_MODE_COLORIMETRY_OPRGB:
+   color_space = COLOR_SPACE_ADOBERGB;
+   break;
+   case DRM_MODE_COLORIMETRY_BT2020:
+   color_space = COLOR_SPACE_2020_RGB_FULLRANGE;
+   break;
+   case DRM_MODE_COLORIMETRY_BT2020_DEPRECATED:
+   color_space = COLOR_SPACE_2020_YCBCR;
break;
}
 
@@ -5476,7 +5482,7 @@ static void fill_stream_properties_from_drm_display_mode(
}
}
 
-   stream->output_color_space = get_output_color_space(timing_out);
+   stream->output_color_space = get_output_color_space(timing_out, 
connector_state);
 }
 
 static void fill_audio_info(struct audio_info *audio_info,
-- 
2.39.2