Re: [PATCH] drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Shashank Sharma <shashank.sharma@intel.com>
Cc: intel-gfx@lists.freedesktop.org,
	Ville Syrjala <ville.syrjala@intel.com>,
	Maarten Lankhorst <maarten.lankhorst@intel.com>
Subject: Re: [PATCH] drm/i915/GLK: Properly handle plane CSC for BT2020 framebuffers
Date: Thu, 2 May 2019 13:15:05 +0300	[thread overview]
Message-ID: <20190502101505.GK24299@intel.com> (raw)
In-Reply-To: <20190502094942.21115-1-shashank.sharma@intel.com>

On Thu, May 02, 2019 at 03:19:42PM +0530, Shashank Sharma wrote:
> Framebuffer formats P01x are supported by GLK, but the function which
> handles CSC on plane color control register, still expectes the input
> buffer to be REC709. This can cause inaccurate output for direct P01x
> flips.
> 
> This patch checks if the color_encoding property is set to YCBCR_2020,
> and enables the corresponding color conversion mode on plane CSC.
> 
> PS: renamed variable plane_color_ctl to color_ctl for 80 char stuff.
> 
> Cc: Ville Syrjala <ville.syrjala@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++----------
>  1 file changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index dd65d7c521c1..2d4d3128bf1f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3868,24 +3868,30 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  		to_i915(plane_state->base.plane->dev);
>  	const struct drm_framebuffer *fb = plane_state->base.fb;
>  	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> -	u32 plane_color_ctl = 0;
> +	u32 color_ctl = 0;
>  
> -	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
> -	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
> +	color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
> +	color_ctl |= glk_plane_color_ctl_alpha(plane_state);
>  
>  	if (fb->format->is_yuv && !icl_is_hdr_plane(dev_priv, plane->id)) {
> -		if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> -			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> -		else
> -			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> +		switch (plane_state->base.color_encoding) {
> +		case DRM_COLOR_YCBCR_BT709:
> +			color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> +			break;
> +		case DRM_COLOR_YCBCR_BT2020:
> +			color_ctl |= PLANE_COLOR_CSC_MODE_YUV2020_TO_RGB2020;
> +			break;
> +		default:
> +			color_ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> +		}

This isn't going to do anything without adjusting the property supported
encodings as well.

>  
>  		if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> -			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> +			color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>  	} else if (fb->format->is_yuv) {
> -		plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> +		color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
>  	}
>  
> -	return plane_color_ctl;
> +	return color_ctl;
>  }
>  
>  static int
> -- 
> 2.17.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx