diff mbox series

[v7,2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion

Message ID 1541060341-31948-3-git-send-email-uma.shankar@intel.com (mailing list archive)
State New, archived
Headers show
Series Enable Plane Input CSC for ICL | expand

Commit Message

Shankar, Uma Nov. 1, 2018, 8:19 a.m. UTC 
Plane input CSC needs to be enabled to convert frambuffers from
YUV to RGB. This is needed for bottom 3 planes on ICL, rest of
the planes have hardcoded conversion and taken care by the legacy
code.

This patch defines the co-efficient values for YUV to RGB conversion
in BT709 and BT601 formats. It programs the coefficients and enables
the plane input csc unit in hardware.

This has been verified and tested by Maarten and the change is working
as expecpted.

v2: Addressed Maarten's and Ville's review comments and added the
coefficients in a 2D array instead of independent Macros.

v3: Added individual coefficient matrix (9 values) instead of 6
register values as per Maarten's comment. Also addresed a shift
issue with B channel coefficient.

v4: Added support for Limited Range Color Handling

v5: Fixed Matt and Maarten's review comments.

v6: Added human readable matrix values for YUV to RGB Conversion along
with just the bspec register values, as per Matt's suggestion.

v7: Refactored the code, move csc coefficient programming function to
intel_sprite.c and made it static as per Ville's review comment.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

---
 drivers/gpu/drm/i915/intel_display.c | 17 +++++--
 drivers/gpu/drm/i915/intel_sprite.c  | 99 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_color_mgmt.h         |  9 ++++
 3 files changed, 120 insertions(+), 5 deletions(-)

Comments

Ville Syrjälä Nov. 1, 2018, 9:46 a.m. UTC | #1 
On Thu, Nov 01, 2018 at 01:49:01PM +0530, Uma Shankar wrote:
> Plane input CSC needs to be enabled to convert frambuffers from
> YUV to RGB. This is needed for bottom 3 planes on ICL, rest of
> the planes have hardcoded conversion and taken care by the legacy
> code.
> 
> This patch defines the co-efficient values for YUV to RGB conversion
> in BT709 and BT601 formats. It programs the coefficients and enables
> the plane input csc unit in hardware.
> 
> This has been verified and tested by Maarten and the change is working
> as expecpted.
> 
> v2: Addressed Maarten's and Ville's review comments and added the
> coefficients in a 2D array instead of independent Macros.
> 
> v3: Added individual coefficient matrix (9 values) instead of 6
> register values as per Maarten's comment. Also addresed a shift
> issue with B channel coefficient.
> 
> v4: Added support for Limited Range Color Handling
> 
> v5: Fixed Matt and Maarten's review comments.
> 
> v6: Added human readable matrix values for YUV to RGB Conversion along
> with just the bspec register values, as per Matt's suggestion.
> 
> v7: Refactored the code, move csc coefficient programming function to
> intel_sprite.c and made it static as per Ville's review comment.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 17 +++++--
>  drivers/gpu/drm/i915/intel_sprite.c  | 99 ++++++++++++++++++++++++++++++++++++
>  include/drm/drm_color_mgmt.h         |  9 ++++
>  3 files changed, 120 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fe045ab..daec1fd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3666,6 +3666,7 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  	struct drm_i915_private *dev_priv =
>  		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;
>  
>  	if (INTEL_GEN(dev_priv) < 11) {
> @@ -3675,14 +3676,20 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
>  	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
>  
> -	if (fb->format->is_yuv) {
> -		if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> -			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> +	if (fb->format->is_yuv && !icl_is_hdr_plane(plane)) {
> +		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;
> +			plane_color_ctl |=
> +				PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
>  
>  		if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> -			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> +			plane_color_ctl |=
> +			PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> +	} else if (fb->format->is_yuv) {
> +		plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
>  	}
>  
>  	return plane_color_ctl;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index c604f6b..711502455 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -40,6 +40,7 @@
>  #include "intel_frontbuffer.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> +#include <drm/drm_color_mgmt.h>
>  
>  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>  			     int usecs)
> @@ -458,11 +459,106 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  }
>  
> +static void
> +icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
> +			    const struct intel_plane_state *plane_state)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(plane_state->base.plane->dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	enum pipe pipe = crtc->pipe;
> +	struct intel_plane *intel_plane =
> +		to_intel_plane(plane_state->base.plane);
> +	enum plane_id plane = intel_plane->id;
> +
> +	static const u16 input_csc_matrix[][9] = {
> +		/*
> +		 * BT.601 full range YCbCr -> full range RGB
> +		 * The matrix required is :
> +		 * [1.000, 0.000, 1.371,
> +		 *  1.000, -0.336, -0.698,
> +		 *  1.000, 1.732, 0.0000]
> +		 */
> +		[DRM_COLOR_YCBCR_BT601] = {
> +			0x7AF8, 0x7800, 0x0,
> +			0x8B28, 0x7800, 0x9AC0,
> +			0x0, 0x7800, 0x7DD8,
> +		},
> +		/*
> +		 * BT.709 full range YCbCr -> full range RGB
> +		 * The matrix required is :
> +		 * [1.000, 0.000, 1.574,
> +		 *  1.000, -0.187, -0.468,
> +		 *  1.000, 1.855, 0.0000]
> +		 */
> +		[DRM_COLOR_YCBCR_BT709] = {
> +			0x7C98, 0x7800, 0x0,
> +			0x9EF8, 0x7800, 0xABF8,
> +			0x0, 0x7800,  0x7ED8,
> +		},
> +	};
> +
> +	/* Matrix for Limited Range to Full Range Conversion */
> +	static const u16 input_csc_matrix_lr[][9] = {
> +		/*
> +		 * BT.601 Limted range YCbCr -> full range RGB
> +		 * The matrix required is :
> +		 * [1.164384, 0.000, 1.596370,
> +		 *  1.138393, -0.382500, -0.794598,
> +		 *  1.138393, 1.971696, 0.0000]
> +		 */
> +		[DRM_COLOR_YCBCR_BT601] = {
> +			0x7CC8, 0x7950, 0x0,
> +			0x8CB8, 0x7918, 0x9C40,
> +			0x0, 0x7918, 0x7FC8,
> +		},
> +		/*
> +		 * BT.709 Limited range YCbCr -> full range RGB
> +		 * The matrix required is :
> +		 * [1.164, 0.000, 1.833671,
> +		 *  1.138393, -0.213249, -0.532909,
> +		 *  1.138393, 2.112402, 0.0000]
> +		 */
> +		[DRM_COLOR_YCBCR_BT709] = {
> +			0x7EA8, 0x7950, 0x0,
> +			0x8888, 0x7918, 0xADA8,
> +			0x0, 0x7918,  0x6870,
> +		},
> +	};
> +	const u16 *csc;
> +
> +	if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> +		csc = input_csc_matrix[plane_state->base.color_encoding];
> +	else
> +		csc = input_csc_matrix_lr[plane_state->base.color_encoding];
> +
> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) |
> +		   GOFF(csc[1]));

Make all of these I915_WRITE_FW() and ...

> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2]));
> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) |
> +		   GOFF(csc[4]));
> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5]));
> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) |
> +		   GOFF(csc[7]));
> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8]));
> +
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0),
> +		   PREOFF_YUV_TO_RGB_HI);
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1),
> +		   PREOFF_YUV_TO_RGB_ME);
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2),
> +		   PREOFF_YUV_TO_RGB_LO);
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0);
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0);
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 0x0);
> +}
> +
>  void
>  skl_update_plane(struct intel_plane *plane,
>  		 const struct intel_crtc_state *crtc_state,
>  		 const struct intel_plane_state *plane_state)
>  {
> +	const struct drm_framebuffer *fb = plane_state->base.fb;
>  	int color_plane = 0;
>  
>  	if (plane_state->linked_plane) {
> @@ -470,6 +566,9 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
>  		color_plane = 1;
>  	}
>  
> +	if (fb->format->is_yuv && icl_is_hdr_plane(plane))
> +		icl_program_input_csc_coeff(crtc_state, plane_state);

... call this from within the spinlocked section of skl_program_plane().

> +
>  	skl_program_plane(plane, crtc_state, plane_state,
>  			  color_plane, false, plane_state->ctl);
>  }
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 90ef999..3e17381 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -29,6 +29,15 @@
>  struct drm_crtc;
>  struct drm_plane;
>  
> +/* Preoffset values for YUV to RGB Conversion */
> +#define PREOFF_YUV_TO_RGB_HI		0x1800
> +#define PREOFF_YUV_TO_RGB_ME		0x1F00
> +#define PREOFF_YUV_TO_RGB_LO		0x1800
> +
> +#define  ROFF(x)          (((x) & 0xffff) << 16)
> +#define  GOFF(x)          (((x) & 0xffff) << 0)
> +#define  BOFF(x)          (((x) & 0xffff) << 16)
> +
>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
>  
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Nov. 1, 2018, 9:50 a.m. UTC | #2 
On Thu, Nov 01, 2018 at 01:49:01PM +0530, Uma Shankar wrote:
> Plane input CSC needs to be enabled to convert frambuffers from
> YUV to RGB. This is needed for bottom 3 planes on ICL, rest of
> the planes have hardcoded conversion and taken care by the legacy
> code.
> 
> This patch defines the co-efficient values for YUV to RGB conversion
> in BT709 and BT601 formats. It programs the coefficients and enables
> the plane input csc unit in hardware.
> 
> This has been verified and tested by Maarten and the change is working
> as expecpted.
> 
> v2: Addressed Maarten's and Ville's review comments and added the
> coefficients in a 2D array instead of independent Macros.
> 
> v3: Added individual coefficient matrix (9 values) instead of 6
> register values as per Maarten's comment. Also addresed a shift
> issue with B channel coefficient.
> 
> v4: Added support for Limited Range Color Handling
> 
> v5: Fixed Matt and Maarten's review comments.
> 
> v6: Added human readable matrix values for YUV to RGB Conversion along
> with just the bspec register values, as per Matt's suggestion.
> 
> v7: Refactored the code, move csc coefficient programming function to
> intel_sprite.c and made it static as per Ville's review comment.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 17 +++++--
>  drivers/gpu/drm/i915/intel_sprite.c  | 99 ++++++++++++++++++++++++++++++++++++
>  include/drm/drm_color_mgmt.h         |  9 ++++
>  3 files changed, 120 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fe045ab..daec1fd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3666,6 +3666,7 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  	struct drm_i915_private *dev_priv =
>  		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;
>  
>  	if (INTEL_GEN(dev_priv) < 11) {
> @@ -3675,14 +3676,20 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
>  	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
>  
> -	if (fb->format->is_yuv) {
> -		if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> -			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> +	if (fb->format->is_yuv && !icl_is_hdr_plane(plane)) {
> +		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;
> +			plane_color_ctl |=
> +				PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
>  
>  		if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> -			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> +			plane_color_ctl |=
> +			PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> +	} else if (fb->format->is_yuv) {
> +		plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
>  	}
>  
>  	return plane_color_ctl;
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index c604f6b..711502455 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -40,6 +40,7 @@
>  #include "intel_frontbuffer.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> +#include <drm/drm_color_mgmt.h>
>  
>  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>  			     int usecs)
> @@ -458,11 +459,106 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
>  }
>  
> +static void
> +icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
> +			    const struct intel_plane_state *plane_state)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(plane_state->base.plane->dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	enum pipe pipe = crtc->pipe;
> +	struct intel_plane *intel_plane =
> +		to_intel_plane(plane_state->base.plane);
> +	enum plane_id plane = intel_plane->id;
> +
> +	static const u16 input_csc_matrix[][9] = {
> +		/*
> +		 * BT.601 full range YCbCr -> full range RGB
> +		 * The matrix required is :
> +		 * [1.000, 0.000, 1.371,
> +		 *  1.000, -0.336, -0.698,
> +		 *  1.000, 1.732, 0.0000]
> +		 */
> +		[DRM_COLOR_YCBCR_BT601] = {
> +			0x7AF8, 0x7800, 0x0,
> +			0x8B28, 0x7800, 0x9AC0,
> +			0x0, 0x7800, 0x7DD8,
> +		},
> +		/*
> +		 * BT.709 full range YCbCr -> full range RGB
> +		 * The matrix required is :
> +		 * [1.000, 0.000, 1.574,
> +		 *  1.000, -0.187, -0.468,
> +		 *  1.000, 1.855, 0.0000]
> +		 */
> +		[DRM_COLOR_YCBCR_BT709] = {
> +			0x7C98, 0x7800, 0x0,
> +			0x9EF8, 0x7800, 0xABF8,
> +			0x0, 0x7800,  0x7ED8,
> +		},
> +	};
> +
> +	/* Matrix for Limited Range to Full Range Conversion */
> +	static const u16 input_csc_matrix_lr[][9] = {
> +		/*
> +		 * BT.601 Limted range YCbCr -> full range RGB
> +		 * The matrix required is :
> +		 * [1.164384, 0.000, 1.596370,
> +		 *  1.138393, -0.382500, -0.794598,
> +		 *  1.138393, 1.971696, 0.0000]
> +		 */
> +		[DRM_COLOR_YCBCR_BT601] = {
> +			0x7CC8, 0x7950, 0x0,
> +			0x8CB8, 0x7918, 0x9C40,
> +			0x0, 0x7918, 0x7FC8,
> +		},
> +		/*
> +		 * BT.709 Limited range YCbCr -> full range RGB
> +		 * The matrix required is :
> +		 * [1.164, 0.000, 1.833671,
> +		 *  1.138393, -0.213249, -0.532909,
> +		 *  1.138393, 2.112402, 0.0000]
> +		 */
> +		[DRM_COLOR_YCBCR_BT709] = {
> +			0x7EA8, 0x7950, 0x0,
> +			0x8888, 0x7918, 0xADA8,
> +			0x0, 0x7918,  0x6870,
> +		},
> +	};
> +	const u16 *csc;
> +
> +	if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> +		csc = input_csc_matrix[plane_state->base.color_encoding];
> +	else
> +		csc = input_csc_matrix_lr[plane_state->base.color_encoding];
> +
> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) |
> +		   GOFF(csc[1]));
> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2]));
> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) |
> +		   GOFF(csc[4]));
> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5]));
> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) |
> +		   GOFF(csc[7]));
> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8]));
> +
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0),
> +		   PREOFF_YUV_TO_RGB_HI);
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1),
> +		   PREOFF_YUV_TO_RGB_ME);
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2),
> +		   PREOFF_YUV_TO_RGB_LO);
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0);
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0);
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 0x0);
> +}
> +
>  void
>  skl_update_plane(struct intel_plane *plane,
>  		 const struct intel_crtc_state *crtc_state,
>  		 const struct intel_plane_state *plane_state)
>  {
> +	const struct drm_framebuffer *fb = plane_state->base.fb;
>  	int color_plane = 0;
>  
>  	if (plane_state->linked_plane) {
> @@ -470,6 +566,9 @@ int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
>  		color_plane = 1;
>  	}
>  
> +	if (fb->format->is_yuv && icl_is_hdr_plane(plane))
> +		icl_program_input_csc_coeff(crtc_state, plane_state);
> +
>  	skl_program_plane(plane, crtc_state, plane_state,
>  			  color_plane, false, plane_state->ctl);
>  }
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 90ef999..3e17381 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -29,6 +29,15 @@
>  struct drm_crtc;
>  struct drm_plane;
>  
> +/* Preoffset values for YUV to RGB Conversion */
> +#define PREOFF_YUV_TO_RGB_HI		0x1800
> +#define PREOFF_YUV_TO_RGB_ME		0x1F00
> +#define PREOFF_YUV_TO_RGB_LO		0x1800

Why are these here when all the other numbers are in 
icl_program_input_csc_coeff()? I'd rather just put
everything into icl_program_input_csc_coeff().

> +
> +#define  ROFF(x)          (((x) & 0xffff) << 16)
> +#define  GOFF(x)          (((x) & 0xffff) << 0)
> +#define  BOFF(x)          (((x) & 0xffff) << 16)
> +
>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
>  
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Shankar, Uma Nov. 1, 2018, 5:45 p.m. UTC | #3 
>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Thursday, November 1, 2018 3:17 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>;
>Lankhorst, Maarten <maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [v7 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to
>RGB Conversion
>
>On Thu, Nov 01, 2018 at 01:49:01PM +0530, Uma Shankar wrote:
>> Plane input CSC needs to be enabled to convert frambuffers from YUV to
>> RGB. This is needed for bottom 3 planes on ICL, rest of the planes
>> have hardcoded conversion and taken care by the legacy code.
>>
>> This patch defines the co-efficient values for YUV to RGB conversion
>> in BT709 and BT601 formats. It programs the coefficients and enables
>> the plane input csc unit in hardware.
>>
>> This has been verified and tested by Maarten and the change is working
>> as expecpted.
>>
>> v2: Addressed Maarten's and Ville's review comments and added the
>> coefficients in a 2D array instead of independent Macros.
>>
>> v3: Added individual coefficient matrix (9 values) instead of 6
>> register values as per Maarten's comment. Also addresed a shift issue
>> with B channel coefficient.
>>
>> v4: Added support for Limited Range Color Handling
>>
>> v5: Fixed Matt and Maarten's review comments.
>>
>> v6: Added human readable matrix values for YUV to RGB Conversion along
>> with just the bspec register values, as per Matt's suggestion.
>>
>> v7: Refactored the code, move csc coefficient programming function to
>> intel_sprite.c and made it static as per Ville's review comment.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 17 +++++--
>> drivers/gpu/drm/i915/intel_sprite.c  | 99
>++++++++++++++++++++++++++++++++++++
>>  include/drm/drm_color_mgmt.h         |  9 ++++
>>  3 files changed, 120 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index fe045ab..daec1fd 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3666,6 +3666,7 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state
>*crtc_state,
>>  	struct drm_i915_private *dev_priv =
>>  		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;
>>
>>  	if (INTEL_GEN(dev_priv) < 11) {
>> @@ -3675,14 +3676,20 @@ u32 glk_plane_color_ctl(const struct
>intel_crtc_state *crtc_state,
>>  	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
>>  	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
>>
>> -	if (fb->format->is_yuv) {
>> -		if (plane_state->base.color_encoding ==
>DRM_COLOR_YCBCR_BT709)
>> -			plane_color_ctl |=
>PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
>> +	if (fb->format->is_yuv && !icl_is_hdr_plane(plane)) {
>> +		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;
>> +			plane_color_ctl |=
>> +
>	PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
>>
>>  		if (plane_state->base.color_range ==
>DRM_COLOR_YCBCR_FULL_RANGE)
>> -			plane_color_ctl |=
>PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>> +			plane_color_ctl |=
>> +			PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>> +	} else if (fb->format->is_yuv) {
>> +		plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
>>  	}
>>
>>  	return plane_color_ctl;
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>> b/drivers/gpu/drm/i915/intel_sprite.c
>> index c604f6b..711502455 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -40,6 +40,7 @@
>>  #include "intel_frontbuffer.h"
>>  #include <drm/i915_drm.h>
>>  #include "i915_drv.h"
>> +#include <drm/drm_color_mgmt.h>
>>
>>  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>>  			     int usecs)
>> @@ -458,11 +459,106 @@ int intel_plane_check_src_coordinates(struct
>intel_plane_state *plane_state)
>>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);  }
>>
>> +static void
>> +icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
>> +			    const struct intel_plane_state *plane_state) {
>> +	struct drm_i915_private *dev_priv =
>> +		to_i915(plane_state->base.plane->dev);
>> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> +	enum pipe pipe = crtc->pipe;
>> +	struct intel_plane *intel_plane =
>> +		to_intel_plane(plane_state->base.plane);
>> +	enum plane_id plane = intel_plane->id;
>> +
>> +	static const u16 input_csc_matrix[][9] = {
>> +		/*
>> +		 * BT.601 full range YCbCr -> full range RGB
>> +		 * The matrix required is :
>> +		 * [1.000, 0.000, 1.371,
>> +		 *  1.000, -0.336, -0.698,
>> +		 *  1.000, 1.732, 0.0000]
>> +		 */
>> +		[DRM_COLOR_YCBCR_BT601] = {
>> +			0x7AF8, 0x7800, 0x0,
>> +			0x8B28, 0x7800, 0x9AC0,
>> +			0x0, 0x7800, 0x7DD8,
>> +		},
>> +		/*
>> +		 * BT.709 full range YCbCr -> full range RGB
>> +		 * The matrix required is :
>> +		 * [1.000, 0.000, 1.574,
>> +		 *  1.000, -0.187, -0.468,
>> +		 *  1.000, 1.855, 0.0000]
>> +		 */
>> +		[DRM_COLOR_YCBCR_BT709] = {
>> +			0x7C98, 0x7800, 0x0,
>> +			0x9EF8, 0x7800, 0xABF8,
>> +			0x0, 0x7800,  0x7ED8,
>> +		},
>> +	};
>> +
>> +	/* Matrix for Limited Range to Full Range Conversion */
>> +	static const u16 input_csc_matrix_lr[][9] = {
>> +		/*
>> +		 * BT.601 Limted range YCbCr -> full range RGB
>> +		 * The matrix required is :
>> +		 * [1.164384, 0.000, 1.596370,
>> +		 *  1.138393, -0.382500, -0.794598,
>> +		 *  1.138393, 1.971696, 0.0000]
>> +		 */
>> +		[DRM_COLOR_YCBCR_BT601] = {
>> +			0x7CC8, 0x7950, 0x0,
>> +			0x8CB8, 0x7918, 0x9C40,
>> +			0x0, 0x7918, 0x7FC8,
>> +		},
>> +		/*
>> +		 * BT.709 Limited range YCbCr -> full range RGB
>> +		 * The matrix required is :
>> +		 * [1.164, 0.000, 1.833671,
>> +		 *  1.138393, -0.213249, -0.532909,
>> +		 *  1.138393, 2.112402, 0.0000]
>> +		 */
>> +		[DRM_COLOR_YCBCR_BT709] = {
>> +			0x7EA8, 0x7950, 0x0,
>> +			0x8888, 0x7918, 0xADA8,
>> +			0x0, 0x7918,  0x6870,
>> +		},
>> +	};
>> +	const u16 *csc;
>> +
>> +	if (plane_state->base.color_range ==
>DRM_COLOR_YCBCR_FULL_RANGE)
>> +		csc = input_csc_matrix[plane_state->base.color_encoding];
>> +	else
>> +		csc = input_csc_matrix_lr[plane_state->base.color_encoding];
>> +
>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) |
>> +		   GOFF(csc[1]));
>
>Make all of these I915_WRITE_FW() and ...

Sure, will update these.

>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2]));
>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) |
>> +		   GOFF(csc[4]));
>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5]));
>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) |
>> +		   GOFF(csc[7]));
>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8]));
>> +
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0),
>> +		   PREOFF_YUV_TO_RGB_HI);
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1),
>> +		   PREOFF_YUV_TO_RGB_ME);
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2),
>> +		   PREOFF_YUV_TO_RGB_LO);
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0);
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0);
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 0x0); }
>> +
>>  void
>>  skl_update_plane(struct intel_plane *plane,
>>  		 const struct intel_crtc_state *crtc_state,
>>  		 const struct intel_plane_state *plane_state)  {
>> +	const struct drm_framebuffer *fb = plane_state->base.fb;
>>  	int color_plane = 0;
>>
>>  	if (plane_state->linked_plane) {
>> @@ -470,6 +566,9 @@ int intel_plane_check_src_coordinates(struct
>intel_plane_state *plane_state)
>>  		color_plane = 1;
>>  	}
>>
>> +	if (fb->format->is_yuv && icl_is_hdr_plane(plane))
>> +		icl_program_input_csc_coeff(crtc_state, plane_state);
>
>... call this from within the spinlocked section of skl_program_plane().

Ok, will update and resend.

>> +
>>  	skl_program_plane(plane, crtc_state, plane_state,
>>  			  color_plane, false, plane_state->ctl);  } diff --git
>> a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h index
>> 90ef999..3e17381 100644
>> --- a/include/drm/drm_color_mgmt.h
>> +++ b/include/drm/drm_color_mgmt.h
>> @@ -29,6 +29,15 @@
>>  struct drm_crtc;
>>  struct drm_plane;
>>
>> +/* Preoffset values for YUV to RGB Conversion */
>> +#define PREOFF_YUV_TO_RGB_HI		0x1800
>> +#define PREOFF_YUV_TO_RGB_ME		0x1F00
>> +#define PREOFF_YUV_TO_RGB_LO		0x1800
>> +
>> +#define  ROFF(x)          (((x) & 0xffff) << 16)
>> +#define  GOFF(x)          (((x) & 0xffff) << 0)
>> +#define  BOFF(x)          (((x) & 0xffff) << 16)
>> +
>>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t
>> bit_precision);
>>
>>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel
Shankar, Uma Nov. 1, 2018, 5:45 p.m. UTC | #4 
>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Thursday, November 1, 2018 3:21 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>;
>Lankhorst, Maarten <maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [v7 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to
>RGB Conversion
>
>On Thu, Nov 01, 2018 at 01:49:01PM +0530, Uma Shankar wrote:
>> Plane input CSC needs to be enabled to convert frambuffers from YUV to
>> RGB. This is needed for bottom 3 planes on ICL, rest of the planes
>> have hardcoded conversion and taken care by the legacy code.
>>
>> This patch defines the co-efficient values for YUV to RGB conversion
>> in BT709 and BT601 formats. It programs the coefficients and enables
>> the plane input csc unit in hardware.
>>
>> This has been verified and tested by Maarten and the change is working
>> as expecpted.
>>
>> v2: Addressed Maarten's and Ville's review comments and added the
>> coefficients in a 2D array instead of independent Macros.
>>
>> v3: Added individual coefficient matrix (9 values) instead of 6
>> register values as per Maarten's comment. Also addresed a shift issue
>> with B channel coefficient.
>>
>> v4: Added support for Limited Range Color Handling
>>
>> v5: Fixed Matt and Maarten's review comments.
>>
>> v6: Added human readable matrix values for YUV to RGB Conversion along
>> with just the bspec register values, as per Matt's suggestion.
>>
>> v7: Refactored the code, move csc coefficient programming function to
>> intel_sprite.c and made it static as per Ville's review comment.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 17 +++++--
>> drivers/gpu/drm/i915/intel_sprite.c  | 99
>++++++++++++++++++++++++++++++++++++
>>  include/drm/drm_color_mgmt.h         |  9 ++++
>>  3 files changed, 120 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index fe045ab..daec1fd 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3666,6 +3666,7 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state
>*crtc_state,
>>  	struct drm_i915_private *dev_priv =
>>  		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;
>>
>>  	if (INTEL_GEN(dev_priv) < 11) {
>> @@ -3675,14 +3676,20 @@ u32 glk_plane_color_ctl(const struct
>intel_crtc_state *crtc_state,
>>  	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
>>  	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
>>
>> -	if (fb->format->is_yuv) {
>> -		if (plane_state->base.color_encoding ==
>DRM_COLOR_YCBCR_BT709)
>> -			plane_color_ctl |=
>PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
>> +	if (fb->format->is_yuv && !icl_is_hdr_plane(plane)) {
>> +		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;
>> +			plane_color_ctl |=
>> +
>	PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
>>
>>  		if (plane_state->base.color_range ==
>DRM_COLOR_YCBCR_FULL_RANGE)
>> -			plane_color_ctl |=
>PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>> +			plane_color_ctl |=
>> +			PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>> +	} else if (fb->format->is_yuv) {
>> +		plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
>>  	}
>>
>>  	return plane_color_ctl;
>> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
>> b/drivers/gpu/drm/i915/intel_sprite.c
>> index c604f6b..711502455 100644
>> --- a/drivers/gpu/drm/i915/intel_sprite.c
>> +++ b/drivers/gpu/drm/i915/intel_sprite.c
>> @@ -40,6 +40,7 @@
>>  #include "intel_frontbuffer.h"
>>  #include <drm/i915_drm.h>
>>  #include "i915_drv.h"
>> +#include <drm/drm_color_mgmt.h>
>>
>>  int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
>>  			     int usecs)
>> @@ -458,11 +459,106 @@ int intel_plane_check_src_coordinates(struct
>intel_plane_state *plane_state)
>>  	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);  }
>>
>> +static void
>> +icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
>> +			    const struct intel_plane_state *plane_state) {
>> +	struct drm_i915_private *dev_priv =
>> +		to_i915(plane_state->base.plane->dev);
>> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> +	enum pipe pipe = crtc->pipe;
>> +	struct intel_plane *intel_plane =
>> +		to_intel_plane(plane_state->base.plane);
>> +	enum plane_id plane = intel_plane->id;
>> +
>> +	static const u16 input_csc_matrix[][9] = {
>> +		/*
>> +		 * BT.601 full range YCbCr -> full range RGB
>> +		 * The matrix required is :
>> +		 * [1.000, 0.000, 1.371,
>> +		 *  1.000, -0.336, -0.698,
>> +		 *  1.000, 1.732, 0.0000]
>> +		 */
>> +		[DRM_COLOR_YCBCR_BT601] = {
>> +			0x7AF8, 0x7800, 0x0,
>> +			0x8B28, 0x7800, 0x9AC0,
>> +			0x0, 0x7800, 0x7DD8,
>> +		},
>> +		/*
>> +		 * BT.709 full range YCbCr -> full range RGB
>> +		 * The matrix required is :
>> +		 * [1.000, 0.000, 1.574,
>> +		 *  1.000, -0.187, -0.468,
>> +		 *  1.000, 1.855, 0.0000]
>> +		 */
>> +		[DRM_COLOR_YCBCR_BT709] = {
>> +			0x7C98, 0x7800, 0x0,
>> +			0x9EF8, 0x7800, 0xABF8,
>> +			0x0, 0x7800,  0x7ED8,
>> +		},
>> +	};
>> +
>> +	/* Matrix for Limited Range to Full Range Conversion */
>> +	static const u16 input_csc_matrix_lr[][9] = {
>> +		/*
>> +		 * BT.601 Limted range YCbCr -> full range RGB
>> +		 * The matrix required is :
>> +		 * [1.164384, 0.000, 1.596370,
>> +		 *  1.138393, -0.382500, -0.794598,
>> +		 *  1.138393, 1.971696, 0.0000]
>> +		 */
>> +		[DRM_COLOR_YCBCR_BT601] = {
>> +			0x7CC8, 0x7950, 0x0,
>> +			0x8CB8, 0x7918, 0x9C40,
>> +			0x0, 0x7918, 0x7FC8,
>> +		},
>> +		/*
>> +		 * BT.709 Limited range YCbCr -> full range RGB
>> +		 * The matrix required is :
>> +		 * [1.164, 0.000, 1.833671,
>> +		 *  1.138393, -0.213249, -0.532909,
>> +		 *  1.138393, 2.112402, 0.0000]
>> +		 */
>> +		[DRM_COLOR_YCBCR_BT709] = {
>> +			0x7EA8, 0x7950, 0x0,
>> +			0x8888, 0x7918, 0xADA8,
>> +			0x0, 0x7918,  0x6870,
>> +		},
>> +	};
>> +	const u16 *csc;
>> +
>> +	if (plane_state->base.color_range ==
>DRM_COLOR_YCBCR_FULL_RANGE)
>> +		csc = input_csc_matrix[plane_state->base.color_encoding];
>> +	else
>> +		csc = input_csc_matrix_lr[plane_state->base.color_encoding];
>> +
>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) |
>> +		   GOFF(csc[1]));
>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2]));
>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) |
>> +		   GOFF(csc[4]));
>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5]));
>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) |
>> +		   GOFF(csc[7]));
>> +	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8]));
>> +
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0),
>> +		   PREOFF_YUV_TO_RGB_HI);
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1),
>> +		   PREOFF_YUV_TO_RGB_ME);
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2),
>> +		   PREOFF_YUV_TO_RGB_LO);
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0);
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0);
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 0x0); }
>> +
>>  void
>>  skl_update_plane(struct intel_plane *plane,
>>  		 const struct intel_crtc_state *crtc_state,
>>  		 const struct intel_plane_state *plane_state)  {
>> +	const struct drm_framebuffer *fb = plane_state->base.fb;
>>  	int color_plane = 0;
>>
>>  	if (plane_state->linked_plane) {
>> @@ -470,6 +566,9 @@ int intel_plane_check_src_coordinates(struct
>intel_plane_state *plane_state)
>>  		color_plane = 1;
>>  	}
>>
>> +	if (fb->format->is_yuv && icl_is_hdr_plane(plane))
>> +		icl_program_input_csc_coeff(crtc_state, plane_state);
>> +
>>  	skl_program_plane(plane, crtc_state, plane_state,
>>  			  color_plane, false, plane_state->ctl);  } diff --git
>> a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h index
>> 90ef999..3e17381 100644
>> --- a/include/drm/drm_color_mgmt.h
>> +++ b/include/drm/drm_color_mgmt.h
>> @@ -29,6 +29,15 @@
>>  struct drm_crtc;
>>  struct drm_plane;
>>
>> +/* Preoffset values for YUV to RGB Conversion */
>> +#define PREOFF_YUV_TO_RGB_HI		0x1800
>> +#define PREOFF_YUV_TO_RGB_ME		0x1F00
>> +#define PREOFF_YUV_TO_RGB_LO		0x1800
>
>Why are these here when all the other numbers are in
>icl_program_input_csc_coeff()? I'd rather just put everything into
>icl_program_input_csc_coeff().

Sure, will move this around. Thanks..

Regards,
Uma Shankar

>> +
>> +#define  ROFF(x)          (((x) & 0xffff) << 16)
>> +#define  GOFF(x)          (((x) & 0xffff) << 0)
>> +#define  BOFF(x)          (((x) & 0xffff) << 16)
>> +
>>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t
>> bit_precision);
>>
>>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fe045ab..daec1fd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3666,6 +3666,7 @@  u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 	struct drm_i915_private *dev_priv =
 		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;
 
 	if (INTEL_GEN(dev_priv) < 11) {
@@ -3675,14 +3676,20 @@  u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 	plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
 	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
 
-	if (fb->format->is_yuv) {
-		if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
-			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
+	if (fb->format->is_yuv && !icl_is_hdr_plane(plane)) {
+		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;
+			plane_color_ctl |=
+				PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
 
 		if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
-			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
+			plane_color_ctl |=
+			PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
+	} else if (fb->format->is_yuv) {
+		plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
 	}
 
 	return plane_color_ctl;
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index c604f6b..711502455 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -40,6 +40,7 @@ 
 #include "intel_frontbuffer.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
+#include <drm/drm_color_mgmt.h>
 
 int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
 			     int usecs)
@@ -458,11 +459,106 @@  int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
 	spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 }
 
+static void
+icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
+			    const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	enum pipe pipe = crtc->pipe;
+	struct intel_plane *intel_plane =
+		to_intel_plane(plane_state->base.plane);
+	enum plane_id plane = intel_plane->id;
+
+	static const u16 input_csc_matrix[][9] = {
+		/*
+		 * BT.601 full range YCbCr -> full range RGB
+		 * The matrix required is :
+		 * [1.000, 0.000, 1.371,
+		 *  1.000, -0.336, -0.698,
+		 *  1.000, 1.732, 0.0000]
+		 */
+		[DRM_COLOR_YCBCR_BT601] = {
+			0x7AF8, 0x7800, 0x0,
+			0x8B28, 0x7800, 0x9AC0,
+			0x0, 0x7800, 0x7DD8,
+		},
+		/*
+		 * BT.709 full range YCbCr -> full range RGB
+		 * The matrix required is :
+		 * [1.000, 0.000, 1.574,
+		 *  1.000, -0.187, -0.468,
+		 *  1.000, 1.855, 0.0000]
+		 */
+		[DRM_COLOR_YCBCR_BT709] = {
+			0x7C98, 0x7800, 0x0,
+			0x9EF8, 0x7800, 0xABF8,
+			0x0, 0x7800,  0x7ED8,
+		},
+	};
+
+	/* Matrix for Limited Range to Full Range Conversion */
+	static const u16 input_csc_matrix_lr[][9] = {
+		/*
+		 * BT.601 Limted range YCbCr -> full range RGB
+		 * The matrix required is :
+		 * [1.164384, 0.000, 1.596370,
+		 *  1.138393, -0.382500, -0.794598,
+		 *  1.138393, 1.971696, 0.0000]
+		 */
+		[DRM_COLOR_YCBCR_BT601] = {
+			0x7CC8, 0x7950, 0x0,
+			0x8CB8, 0x7918, 0x9C40,
+			0x0, 0x7918, 0x7FC8,
+		},
+		/*
+		 * BT.709 Limited range YCbCr -> full range RGB
+		 * The matrix required is :
+		 * [1.164, 0.000, 1.833671,
+		 *  1.138393, -0.213249, -0.532909,
+		 *  1.138393, 2.112402, 0.0000]
+		 */
+		[DRM_COLOR_YCBCR_BT709] = {
+			0x7EA8, 0x7950, 0x0,
+			0x8888, 0x7918, 0xADA8,
+			0x0, 0x7918,  0x6870,
+		},
+	};
+	const u16 *csc;
+
+	if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
+		csc = input_csc_matrix[plane_state->base.color_encoding];
+	else
+		csc = input_csc_matrix_lr[plane_state->base.color_encoding];
+
+	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 0), ROFF(csc[0]) |
+		   GOFF(csc[1]));
+	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 1), BOFF(csc[2]));
+	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 2), ROFF(csc[3]) |
+		   GOFF(csc[4]));
+	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 3), BOFF(csc[5]));
+	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 4), ROFF(csc[6]) |
+		   GOFF(csc[7]));
+	I915_WRITE(PLANE_INPUT_CSC_COEFF(pipe, plane, 5), BOFF(csc[8]));
+
+	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 0),
+		   PREOFF_YUV_TO_RGB_HI);
+	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 1),
+		   PREOFF_YUV_TO_RGB_ME);
+	I915_WRITE(PLANE_INPUT_CSC_PREOFF(pipe, plane, 2),
+		   PREOFF_YUV_TO_RGB_LO);
+	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 0), 0x0);
+	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 1), 0x0);
+	I915_WRITE(PLANE_INPUT_CSC_POSTOFF(pipe, plane, 2), 0x0);
+}
+
 void
 skl_update_plane(struct intel_plane *plane,
 		 const struct intel_crtc_state *crtc_state,
 		 const struct intel_plane_state *plane_state)
 {
+	const struct drm_framebuffer *fb = plane_state->base.fb;
 	int color_plane = 0;
 
 	if (plane_state->linked_plane) {
@@ -470,6 +566,9 @@  int intel_plane_check_src_coordinates(struct intel_plane_state *plane_state)
 		color_plane = 1;
 	}
 
+	if (fb->format->is_yuv && icl_is_hdr_plane(plane))
+		icl_program_input_csc_coeff(crtc_state, plane_state);
+
 	skl_program_plane(plane, crtc_state, plane_state,
 			  color_plane, false, plane_state->ctl);
 }
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index 90ef999..3e17381 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -29,6 +29,15 @@ 
 struct drm_crtc;
 struct drm_plane;
 
+/* Preoffset values for YUV to RGB Conversion */
+#define PREOFF_YUV_TO_RGB_HI		0x1800
+#define PREOFF_YUV_TO_RGB_ME		0x1F00
+#define PREOFF_YUV_TO_RGB_LO		0x1800
+
+#define  ROFF(x)          (((x) & 0xffff) << 16)
+#define  GOFF(x)          (((x) & 0xffff) << 0)
+#define  BOFF(x)          (((x) & 0xffff) << 16)
+
 uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
 
 void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,