Re: [Intel-gfx] [PATCH 19/20] drm/i915: Complete the gamma/degamma state checking

2020-09-17 Thread Shankar, Uma


> -Original Message-
> From: Intel-gfx  On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 19/20] drm/i915: Complete the gamma/degamma
> state checking
> 
> From: Ville Syrjälä 
> 
> Add new .gamma_equal() and .degamma_equal() vfuncs to be used by the state
> checker to verify the gamma/degamma LUTs. We need somewhat custom
> behaviour for some platforms due to sometimes swapping the roles of the
> gamma and degamma LUTs (due to RGB limited color range or YCbCr output CSC
> usage). Also glk has a special relationship between the CSC enable bit and the
> degamma LUT so it also needs customized behaviour.
> 
> We could pontially compute the state in a better way to make these state check
> hacks unnecessary, but that's going to require some actual thought so we'll 
> leave
> it for the future.

Like the idea of how the anomalies of various platforms are balanced.
Reviewed-by: Uma Shankar 

> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/display/intel_color.c   | 435 +++
>  drivers/gpu/drm/i915/display/intel_color.h   |  10 +-
>  drivers/gpu/drm/i915/display/intel_display.c |  25 +-
>  drivers/gpu/drm/i915/i915_drv.h  |   7 +
>  4 files changed, 378 insertions(+), 99 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index f714c87d8e42..ca6c6679368c 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1628,26 +1628,71 @@ static int i9xx_gamma_precision(const struct
> intel_crtc_state *crtc_state)
>   }
>  }
> 
> +static bool ilk_crtc_has_degamma(const struct intel_crtc_state
> +*crtc_state) {
> + return crtc_state->gamma_enable &&
> + (crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0; }
> +
>  static bool ilk_crtc_has_gamma(const struct intel_crtc_state *crtc_state)  {
>   return crtc_state->gamma_enable &&
>   (crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) != 0;  }
> 
> +static int ilk_lut_precision(const struct intel_crtc_state *crtc_state)
> +{
> + switch (crtc_state->gamma_mode) {
> + case GAMMA_MODE_MODE_8BIT:
> + return 8;
> + case GAMMA_MODE_MODE_10BIT:
> + return 10;
> + default:
> + MISSING_CASE(crtc_state->gamma_mode);
> + return 0;
> + }
> +}
> +
> +static int ilk_degamma_precision(const struct intel_crtc_state
> +*crtc_state) {
> + if (ilk_crtc_has_degamma(crtc_state))
> + return ilk_lut_precision(crtc_state);
> + else
> + return 0;
> +}
> +
>  static int ilk_gamma_precision(const struct intel_crtc_state *crtc_state)  {
> - if (!ilk_crtc_has_gamma(crtc_state))
> + if (ilk_crtc_has_gamma(crtc_state))
> + return ilk_lut_precision(crtc_state);
> + else
>   return 0;
> +}
> 
> - switch (crtc_state->gamma_mode) {
> - case GAMMA_MODE_MODE_8BIT:
> - return 8;
> - case GAMMA_MODE_MODE_10BIT:
> +static int ivb_degamma_precision(const struct intel_crtc_state
> +*crtc_state) {
> + if (crtc_state->gamma_enable &&
> + crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)
>   return 10;
> - default:
> - MISSING_CASE(crtc_state->gamma_mode);
> + else
> + return ilk_degamma_precision(crtc_state);
> +}
> +
> +static int ivb_gamma_precision(const struct intel_crtc_state
> +*crtc_state) {
> + if (crtc_state->gamma_enable &&
> + crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)
> + return 10;
> + else
> + return ilk_gamma_precision(crtc_state); }
> +
> +static int chv_degamma_precision(const struct intel_crtc_state
> +*crtc_state) {
> + if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)
> + return 14;
> + else
>   return 0;
> - }
>  }
> 
>  static int chv_gamma_precision(const struct intel_crtc_state *crtc_state) @@ 
> -
> 1658,20 +1703,20 @@ static int chv_gamma_precision(const struct
> intel_crtc_state *crtc_state)
>   return i9xx_gamma_precision(crtc_state);  }
> 
> +static int glk_degamma_precision(const struct intel_crtc_state
> +*crtc_state) {
> + if (crtc_state->csc_enable)
> + return 16;
> + else
> + return 0;
> +}
> +
>  static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)  {
> - 

[Intel-gfx] [PATCH 19/20] drm/i915: Complete the gamma/degamma state checking

2020-07-17 Thread Ville Syrjala
From: Ville Syrjälä 

Add new .gamma_equal() and .degamma_equal() vfuncs to be used
by the state checker to verify the gamma/degamma LUTs. We need
somewhat custom behaviour for some platforms due to sometimes
swapping the roles of the gamma and degamma LUTs (due to
RGB limited color range or YCbCr output CSC usage). Also glk
has a special relationship between the CSC enable bit and
the degamma LUT so it also needs customized behaviour.

We could pontially compute the state in a better way to make these
state check hacks unnecessary, but that's going to require some
actual thought so we'll leave it for the future.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/i915/display/intel_color.c   | 435 +++
 drivers/gpu/drm/i915/display/intel_color.h   |  10 +-
 drivers/gpu/drm/i915/display/intel_display.c |  25 +-
 drivers/gpu/drm/i915/i915_drv.h  |   7 +
 4 files changed, 378 insertions(+), 99 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
b/drivers/gpu/drm/i915/display/intel_color.c
index f714c87d8e42..ca6c6679368c 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1628,26 +1628,71 @@ static int i9xx_gamma_precision(const struct 
intel_crtc_state *crtc_state)
}
 }
 
+static bool ilk_crtc_has_degamma(const struct intel_crtc_state *crtc_state)
+{
+   return crtc_state->gamma_enable &&
+   (crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0;
+}
+
 static bool ilk_crtc_has_gamma(const struct intel_crtc_state *crtc_state)
 {
return crtc_state->gamma_enable &&
(crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) != 0;
 }
 
+static int ilk_lut_precision(const struct intel_crtc_state *crtc_state)
+{
+   switch (crtc_state->gamma_mode) {
+   case GAMMA_MODE_MODE_8BIT:
+   return 8;
+   case GAMMA_MODE_MODE_10BIT:
+   return 10;
+   default:
+   MISSING_CASE(crtc_state->gamma_mode);
+   return 0;
+   }
+}
+
+static int ilk_degamma_precision(const struct intel_crtc_state *crtc_state)
+{
+   if (ilk_crtc_has_degamma(crtc_state))
+   return ilk_lut_precision(crtc_state);
+   else
+   return 0;
+}
+
 static int ilk_gamma_precision(const struct intel_crtc_state *crtc_state)
 {
-   if (!ilk_crtc_has_gamma(crtc_state))
+   if (ilk_crtc_has_gamma(crtc_state))
+   return ilk_lut_precision(crtc_state);
+   else
return 0;
+}
 
-   switch (crtc_state->gamma_mode) {
-   case GAMMA_MODE_MODE_8BIT:
-   return 8;
-   case GAMMA_MODE_MODE_10BIT:
+static int ivb_degamma_precision(const struct intel_crtc_state *crtc_state)
+{
+   if (crtc_state->gamma_enable &&
+   crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)
return 10;
-   default:
-   MISSING_CASE(crtc_state->gamma_mode);
+   else
+   return ilk_degamma_precision(crtc_state);
+}
+
+static int ivb_gamma_precision(const struct intel_crtc_state *crtc_state)
+{
+   if (crtc_state->gamma_enable &&
+   crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)
+   return 10;
+   else
+   return ilk_gamma_precision(crtc_state);
+}
+
+static int chv_degamma_precision(const struct intel_crtc_state *crtc_state)
+{
+   if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)
+   return 14;
+   else
return 0;
-   }
 }
 
 static int chv_gamma_precision(const struct intel_crtc_state *crtc_state)
@@ -1658,20 +1703,20 @@ static int chv_gamma_precision(const struct 
intel_crtc_state *crtc_state)
return i9xx_gamma_precision(crtc_state);
 }
 
+static int glk_degamma_precision(const struct intel_crtc_state *crtc_state)
+{
+   if (crtc_state->csc_enable)
+   return 16;
+   else
+   return 0;
+}
+
 static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)
 {
-   if (!crtc_state->gamma_enable)
+   if (crtc_state->gamma_enable)
+   return ilk_lut_precision(crtc_state);
+   else
return 0;
-
-   switch (crtc_state->gamma_mode) {
-   case GAMMA_MODE_MODE_8BIT:
-   return 8;
-   case GAMMA_MODE_MODE_10BIT:
-   return 10;
-   default:
-   MISSING_CASE(crtc_state->gamma_mode);
-   return 0;
-   }
 }
 
 static bool icl_crtc_has_degamma(const struct intel_crtc_state *crtc_state)
@@ -1684,6 +1729,14 @@ static bool icl_crtc_has_gamma(const struct 
intel_crtc_state *crtc_state)
return crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE;
 }
 
+static int icl_degamma_precision(const struct intel_crtc_state *crtc_state)
+{
+   if (icl_crtc_has_degamma(crtc_state))
+   return 16;
+   else
+   return 0;
+}
+
 static int icl_gamma_precision(const struct intel_crtc_state