diff mbox series

[v2] drm/i915: support two CSC module on gen11 and later

Message ID 20210115054604.27726-1-shawn.c.lee@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2] drm/i915: support two CSC module on gen11 and later | expand

Commit Message

Lee, Shawn C Jan. 15, 2021, 5:46 a.m. UTC
There are two CSC on pipeline on gen11 and later platform.
User space application is allowed to enable CTM and RGB
to YCbCr coversion at the same time now.

v2: check csc capability in {}_color_check function.

Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Shankar Uma <uma.shankar@intel.com>

Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c   | 45 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_display.c | 13 ------
 2 files changed, 45 insertions(+), 13 deletions(-)

Comments

Ville Syrjälä Jan. 15, 2021, 4:04 p.m. UTC | #1
On Fri, Jan 15, 2021 at 01:46:04PM +0800, Lee Shawn C wrote:
> There are two CSC on pipeline on gen11 and later platform.
> User space application is allowed to enable CTM and RGB
> to YCbCr coversion at the same time now.
> 
> v2: check csc capability in {}_color_check function.
> 
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Shankar Uma <uma.shankar@intel.com>
> 
> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c   | 45 ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_display.c | 13 ------
>  2 files changed, 45 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 172d398081ee..22edcd0c9ad5 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1322,10 +1322,35 @@ static u32 i9xx_gamma_mode(struct intel_crtc_state *crtc_state)
>  		return GAMMA_MODE_MODE_10BIT; /* i965+ only */
>  }
>  
> +static int check_csc(const struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(pipe_config->uapi.crtc->dev);
> +
> +	if ((INTEL_GEN(dev_priv) < 11) &&
> +	    (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
> +	     pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) &&
> +	     pipe_config->hw.ctm) {

IMO trying to make this generic to all platforms is just making it
more complicated for no good reason. For now I'd just inline this
directly into the two places where we need it.

And if we write it as 

if (crtc_state->output_format != RGB &&
    crtc_state->hw.ctm) {
	drm_dbg_kms(...);
	return -EINVAL;
}

it will nicely match the code that calculates csc_enable.

> +		/*
> +		 * There is only one pipe CSC unit per pipe, and we need that
> +		 * for output conversion from RGB->YCBCR. So if CTM is already
> +		 * applied we can't support YCBCR420 output.
> +		 */
> +		drm_dbg_kms(&dev_priv->drm,
> +			    "YCBCR420 and CTM together are not possible\n");

Should just say YCbCr here since it's not necessarily 4:2:0 subsampled.

> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int i9xx_color_check(struct intel_crtc_state *crtc_state)
>  {
>  	int ret;
>  
> +	ret = check_csc(crtc_state);
> +	if (ret)
> +		return ret;
> +
>  	ret = check_luts(crtc_state);
>  	if (ret)
>  		return ret;
> @@ -1374,6 +1399,10 @@ static int chv_color_check(struct intel_crtc_state *crtc_state)
>  {
>  	int ret;
>  
> +	ret = check_csc(crtc_state);
> +	if (ret)
> +		return ret;
> +
>  	ret = check_luts(crtc_state);
>  	if (ret)
>  		return ret;
> @@ -1427,6 +1456,10 @@ static int ilk_color_check(struct intel_crtc_state *crtc_state)
>  {
>  	int ret;
>  
> +	ret = check_csc(crtc_state);
> +	if (ret)
> +		return ret;
> +
>  	ret = check_luts(crtc_state);
>  	if (ret)
>  		return ret;
> @@ -1488,6 +1521,10 @@ static int ivb_color_check(struct intel_crtc_state *crtc_state)
>  	bool limited_color_range = ilk_csc_limited_range(crtc_state);
>  	int ret;
>  
> +	ret = check_csc(crtc_state);
> +	if (ret)
> +		return ret;
> +
>  	ret = check_luts(crtc_state);
>  	if (ret)
>  		return ret;
> @@ -1527,6 +1564,10 @@ static int glk_color_check(struct intel_crtc_state *crtc_state)
>  {
>  	int ret;
>  
> +	ret = check_csc(crtc_state);
> +	if (ret)
> +		return ret;
> +
>  	ret = check_luts(crtc_state);
>  	if (ret)
>  		return ret;
> @@ -1592,6 +1633,10 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
>  {
>  	int ret;
>  
> +	ret = check_csc(crtc_state);
> +	if (ret)
> +		return ret;
> +
>  	ret = check_luts(crtc_state);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 66990e48c0d4..e60cbe8b0203 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -7835,19 +7835,6 @@ static int intel_crtc_compute_config(struct intel_crtc *crtc,
>  		return -EINVAL;
>  	}
>  
> -	if ((pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
> -	     pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) &&
> -	     pipe_config->hw.ctm) {
> -		/*
> -		 * There is only one pipe CSC unit per pipe, and we need that
> -		 * for output conversion from RGB->YCBCR. So if CTM is already
> -		 * applied we can't support YCBCR420 output.
> -		 */
> -		drm_dbg_kms(&dev_priv->drm,
> -			    "YCBCR420 and CTM together are not possible\n");
> -		return -EINVAL;
> -	}
> -
>  	/*
>  	 * Pipe horizontal size must be even in:
>  	 * - DVO ganged mode
> -- 
> 2.17.1
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 172d398081ee..22edcd0c9ad5 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1322,10 +1322,35 @@  static u32 i9xx_gamma_mode(struct intel_crtc_state *crtc_state)
 		return GAMMA_MODE_MODE_10BIT; /* i965+ only */
 }
 
+static int check_csc(const struct intel_crtc_state *pipe_config)
+{
+	struct drm_i915_private *dev_priv = to_i915(pipe_config->uapi.crtc->dev);
+
+	if ((INTEL_GEN(dev_priv) < 11) &&
+	    (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
+	     pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) &&
+	     pipe_config->hw.ctm) {
+		/*
+		 * There is only one pipe CSC unit per pipe, and we need that
+		 * for output conversion from RGB->YCBCR. So if CTM is already
+		 * applied we can't support YCBCR420 output.
+		 */
+		drm_dbg_kms(&dev_priv->drm,
+			    "YCBCR420 and CTM together are not possible\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int i9xx_color_check(struct intel_crtc_state *crtc_state)
 {
 	int ret;
 
+	ret = check_csc(crtc_state);
+	if (ret)
+		return ret;
+
 	ret = check_luts(crtc_state);
 	if (ret)
 		return ret;
@@ -1374,6 +1399,10 @@  static int chv_color_check(struct intel_crtc_state *crtc_state)
 {
 	int ret;
 
+	ret = check_csc(crtc_state);
+	if (ret)
+		return ret;
+
 	ret = check_luts(crtc_state);
 	if (ret)
 		return ret;
@@ -1427,6 +1456,10 @@  static int ilk_color_check(struct intel_crtc_state *crtc_state)
 {
 	int ret;
 
+	ret = check_csc(crtc_state);
+	if (ret)
+		return ret;
+
 	ret = check_luts(crtc_state);
 	if (ret)
 		return ret;
@@ -1488,6 +1521,10 @@  static int ivb_color_check(struct intel_crtc_state *crtc_state)
 	bool limited_color_range = ilk_csc_limited_range(crtc_state);
 	int ret;
 
+	ret = check_csc(crtc_state);
+	if (ret)
+		return ret;
+
 	ret = check_luts(crtc_state);
 	if (ret)
 		return ret;
@@ -1527,6 +1564,10 @@  static int glk_color_check(struct intel_crtc_state *crtc_state)
 {
 	int ret;
 
+	ret = check_csc(crtc_state);
+	if (ret)
+		return ret;
+
 	ret = check_luts(crtc_state);
 	if (ret)
 		return ret;
@@ -1592,6 +1633,10 @@  static int icl_color_check(struct intel_crtc_state *crtc_state)
 {
 	int ret;
 
+	ret = check_csc(crtc_state);
+	if (ret)
+		return ret;
+
 	ret = check_luts(crtc_state);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 66990e48c0d4..e60cbe8b0203 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -7835,19 +7835,6 @@  static int intel_crtc_compute_config(struct intel_crtc *crtc,
 		return -EINVAL;
 	}
 
-	if ((pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
-	     pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR444) &&
-	     pipe_config->hw.ctm) {
-		/*
-		 * There is only one pipe CSC unit per pipe, and we need that
-		 * for output conversion from RGB->YCBCR. So if CTM is already
-		 * applied we can't support YCBCR420 output.
-		 */
-		drm_dbg_kms(&dev_priv->drm,
-			    "YCBCR420 and CTM together are not possible\n");
-		return -EINVAL;
-	}
-
 	/*
 	 * Pipe horizontal size must be even in:
 	 * - DVO ganged mode