diff mbox series

[10/11] drm/i915: Share {csc, gamma}_enable calculation for ilk/snb vs. ivb+

Message ID 20221026113906.10551-11-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: More gamma work | expand

Commit Message

Ville Syrjälä Oct. 26, 2022, 11:39 a.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

ilk/snb vs. ivb+ hardware is mostly identical except for the addition
of the split gamma mode on ivb. Thus we can share the csc_enable
and gamma_enable calculation for both variants. Pull that stuff
into a few helpers.

Note that this also fills in the missing ctm/degamma stuff into
ilk_color_check() pretty much, so for good measure let's also
add a few extra checks relating to that, although we still don't
expose ctm/degamma to userspace. But now it'll be trivial to do
so if we wish.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 49 ++++++++++++++--------
 1 file changed, 32 insertions(+), 17 deletions(-)

Comments

Nautiyal, Ankit K Nov. 3, 2022, 10:25 a.m. UTC | #1 
LGTM.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

On 10/26/2022 5:09 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> ilk/snb vs. ivb+ hardware is mostly identical except for the addition
> of the split gamma mode on ivb. Thus we can share the csc_enable
> and gamma_enable calculation for both variants. Pull that stuff
> into a few helpers.
>
> Note that this also fills in the missing ctm/degamma stuff into
> ilk_color_check() pretty much, so for good measure let's also
> add a few extra checks relating to that, although we still don't
> expose ctm/degamma to userspace. But now it'll be trivial to do
> so if we wish.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_color.c | 49 ++++++++++++++--------
>   1 file changed, 32 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 926784f266f2..33871bfacee7 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1442,6 +1442,20 @@ static int chv_color_check(struct intel_crtc_state *crtc_state)
>   	return 0;
>   }
>   
> +static bool ilk_gamma_enable(const struct intel_crtc_state *crtc_state)
> +{
> +	return (crtc_state->hw.gamma_lut ||
> +		crtc_state->hw.degamma_lut) &&
> +		!crtc_state->c8_planes;
> +}
> +
> +static bool ilk_csc_enable(const struct intel_crtc_state *crtc_state)
> +{
> +	return crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
> +		ilk_csc_limited_range(crtc_state) ||
> +		crtc_state->hw.ctm;
> +}
> +
>   static u32 ilk_gamma_mode(const struct intel_crtc_state *crtc_state)
>   {
>   	if (!crtc_state->gamma_enable ||
> @@ -1487,22 +1501,29 @@ static void ilk_assign_luts(struct intel_crtc_state *crtc_state)
>   
>   static int ilk_color_check(struct intel_crtc_state *crtc_state)
>   {
> +	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
>   	int ret;
>   
>   	ret = check_luts(crtc_state);
>   	if (ret)
>   		return ret;
>   
> -	crtc_state->gamma_enable =
> -		crtc_state->hw.gamma_lut &&
> -		!crtc_state->c8_planes;
> +	if (crtc_state->hw.degamma_lut && crtc_state->hw.gamma_lut) {
> +		drm_dbg_kms(&i915->drm,
> +			    "Degamma and gamma together are not possible\n");
> +		return -EINVAL;
> +	}
>   
> -	/*
> -	 * We don't expose the ctm on ilk/snb currently, also RGB
> -	 * limited range output is handled by the hw automagically.
> -	 */
> -	crtc_state->csc_enable =
> -		crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB;
> +	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB &&
> +	    crtc_state->hw.ctm) {
> +		drm_dbg_kms(&i915->drm,
> +			    "YCbCr and CTM together are not possible\n");
> +		return -EINVAL;
> +	}
> +
> +	crtc_state->gamma_enable = ilk_gamma_enable(crtc_state);
> +
> +	crtc_state->csc_enable = ilk_csc_enable(crtc_state);
>   
>   	crtc_state->gamma_mode = ilk_gamma_mode(crtc_state);
>   
> @@ -1546,7 +1567,6 @@ static u32 ivb_csc_mode(const struct intel_crtc_state *crtc_state)
>   static int ivb_color_check(struct intel_crtc_state *crtc_state)
>   {
>   	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> -	bool limited_color_range = ilk_csc_limited_range(crtc_state);
>   	int ret;
>   
>   	ret = check_luts(crtc_state);
> @@ -1567,14 +1587,9 @@ static int ivb_color_check(struct intel_crtc_state *crtc_state)
>   		return -EINVAL;
>   	}
>   
> -	crtc_state->gamma_enable =
> -		(crtc_state->hw.gamma_lut ||
> -		 crtc_state->hw.degamma_lut) &&
> -		!crtc_state->c8_planes;
> +	crtc_state->gamma_enable = ilk_gamma_enable(crtc_state);
>   
> -	crtc_state->csc_enable =
> -		crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
> -		crtc_state->hw.ctm || limited_color_range;
> +	crtc_state->csc_enable = ilk_csc_enable(crtc_state);
>   
>   	crtc_state->gamma_mode = ivb_gamma_mode(crtc_state);
>
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 926784f266f2..33871bfacee7 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1442,6 +1442,20 @@  static int chv_color_check(struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
+static bool ilk_gamma_enable(const struct intel_crtc_state *crtc_state)
+{
+	return (crtc_state->hw.gamma_lut ||
+		crtc_state->hw.degamma_lut) &&
+		!crtc_state->c8_planes;
+}
+
+static bool ilk_csc_enable(const struct intel_crtc_state *crtc_state)
+{
+	return crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
+		ilk_csc_limited_range(crtc_state) ||
+		crtc_state->hw.ctm;
+}
+
 static u32 ilk_gamma_mode(const struct intel_crtc_state *crtc_state)
 {
 	if (!crtc_state->gamma_enable ||
@@ -1487,22 +1501,29 @@  static void ilk_assign_luts(struct intel_crtc_state *crtc_state)
 
 static int ilk_color_check(struct intel_crtc_state *crtc_state)
 {
+	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
 	int ret;
 
 	ret = check_luts(crtc_state);
 	if (ret)
 		return ret;
 
-	crtc_state->gamma_enable =
-		crtc_state->hw.gamma_lut &&
-		!crtc_state->c8_planes;
+	if (crtc_state->hw.degamma_lut && crtc_state->hw.gamma_lut) {
+		drm_dbg_kms(&i915->drm,
+			    "Degamma and gamma together are not possible\n");
+		return -EINVAL;
+	}
 
-	/*
-	 * We don't expose the ctm on ilk/snb currently, also RGB
-	 * limited range output is handled by the hw automagically.
-	 */
-	crtc_state->csc_enable =
-		crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB;
+	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB &&
+	    crtc_state->hw.ctm) {
+		drm_dbg_kms(&i915->drm,
+			    "YCbCr and CTM together are not possible\n");
+		return -EINVAL;
+	}
+
+	crtc_state->gamma_enable = ilk_gamma_enable(crtc_state);
+
+	crtc_state->csc_enable = ilk_csc_enable(crtc_state);
 
 	crtc_state->gamma_mode = ilk_gamma_mode(crtc_state);
 
@@ -1546,7 +1567,6 @@  static u32 ivb_csc_mode(const struct intel_crtc_state *crtc_state)
 static int ivb_color_check(struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
-	bool limited_color_range = ilk_csc_limited_range(crtc_state);
 	int ret;
 
 	ret = check_luts(crtc_state);
@@ -1567,14 +1587,9 @@  static int ivb_color_check(struct intel_crtc_state *crtc_state)
 		return -EINVAL;
 	}
 
-	crtc_state->gamma_enable =
-		(crtc_state->hw.gamma_lut ||
-		 crtc_state->hw.degamma_lut) &&
-		!crtc_state->c8_planes;
+	crtc_state->gamma_enable = ilk_gamma_enable(crtc_state);
 
-	crtc_state->csc_enable =
-		crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
-		crtc_state->hw.ctm || limited_color_range;
+	crtc_state->csc_enable = ilk_csc_enable(crtc_state);
 
 	crtc_state->gamma_mode = ivb_gamma_mode(crtc_state);