Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [Intel-gfx] [PATCH 01/20] drm/i915: Fix state checker hw.active/hw.enable readout
       [not found] ` <20200717211345.26851-2-ville.syrjala@linux.intel.com>
@ 2020-09-17 19:20   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 19:20 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:43 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 01/20] drm/i915: Fix state checker
> hw.active/hw.enable readout
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Previously intel_dump_pipe_config() used to dump the full crtc state whether or
> not the crtc was logically enabled or not. As that meant occasionally dumping
> confusing stale garbage I changed it to check whether the crtc is logically enabled
> or not. However I did not realize that the state checker readout code does not
> populate crtc_state.hw.{active,enabled}. Hence the state checker dump would
> only give us a full dump of the sw state but not the hw state. Fix that by
> populating those bits of the hw state as well.

Looks good.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Fixes: 10d75f5428fd ("drm/i915: Fix plane state dumps")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 729ec6e0d43a..ae0af452d776 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -14244,7 +14244,6 @@ verify_crtc_state(struct intel_crtc *crtc,
>  	struct intel_encoder *encoder;
>  	struct intel_crtc_state *pipe_config = old_crtc_state;
>  	struct drm_atomic_state *state = old_crtc_state->uapi.state;
> -	bool active;
> 
>  	__drm_atomic_helper_crtc_destroy_state(&old_crtc_state->uapi);
>  	intel_crtc_free_hw_state(old_crtc_state);
> @@ -14254,16 +14253,19 @@ verify_crtc_state(struct intel_crtc *crtc,
>  	drm_dbg_kms(&dev_priv->drm, "[CRTC:%d:%s]\n", crtc->base.base.id,
>  		    crtc->base.name);
> 
> -	active = dev_priv->display.get_pipe_config(crtc, pipe_config);
> +	pipe_config->hw.enable = new_crtc_state->hw.enable;
> +
> +	pipe_config->hw.active =
> +		dev_priv->display.get_pipe_config(crtc, pipe_config);
> 
>  	/* we keep both pipes enabled on 830 */
> -	if (IS_I830(dev_priv))
> -		active = new_crtc_state->hw.active;
> +	if (IS_I830(dev_priv) && pipe_config->hw.active)
> +		pipe_config->hw.active = new_crtc_state->hw.active;
> 
> -	I915_STATE_WARN(new_crtc_state->hw.active != active,
> +	I915_STATE_WARN(new_crtc_state->hw.active != pipe_config->hw.active,
>  			"crtc active state doesn't match with hw state "
>  			"(expected %i, found %i)\n",
> -			new_crtc_state->hw.active, active);
> +			new_crtc_state->hw.active, pipe_config->hw.active);
> 
>  	I915_STATE_WARN(crtc->active != new_crtc_state->hw.active,
>  			"transitional active state does not match atomic hw state
> "
> @@ -14272,6 +14274,7 @@ verify_crtc_state(struct intel_crtc *crtc,
> 
>  	for_each_encoder_on_crtc(dev, &crtc->base, encoder) {
>  		enum pipe pipe;
> +		bool active;
> 
>  		active = encoder->get_hw_state(encoder, &pipe);
>  		I915_STATE_WARN(active != new_crtc_state->hw.active,
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 02/20] drm/i915: Move MST master transcoder dump earlier
       [not found] ` <20200717211345.26851-3-ville.syrjala@linux.intel.com>
@ 2020-09-17 19:24   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 19:24 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:43 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 02/20] drm/i915: Move MST master transcoder dump
> earlier
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move the MST master transcoder dump next to the other transcoder bits.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index ae0af452d776..c36379cf07fc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13031,6 +13031,9 @@ static void intel_dump_pipe_config(const struct
> intel_crtc_state *pipe_config,
>  		    transcoder_name(pipe_config->cpu_transcoder),
>  		    pipe_config->pipe_bpp, pipe_config->dither);
> 
> +	drm_dbg_kms(&dev_priv->drm, "MST master transcoder: %s\n",
> +		    transcoder_name(pipe_config->mst_master_transcoder));
> +
>  	drm_dbg_kms(&dev_priv->drm,
>  		    "port sync: master transcoder: %s, slave transcoder bitmask =
> 0x%x\n",
>  		    transcoder_name(pipe_config->master_transcoder),
> @@ -13128,9 +13131,6 @@ static void intel_dump_pipe_config(const struct
> intel_crtc_state *pipe_config,
>  			    pipe_config->csc_mode, pipe_config->gamma_mode,
>  			    pipe_config->gamma_enable, pipe_config-
> >csc_enable);
> 
> -	drm_dbg_kms(&dev_priv->drm, "MST master transcoder: %s\n",
> -		    transcoder_name(pipe_config->mst_master_transcoder));
> -
>  dump_planes:
>  	if (!state)
>  		return;
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 03/20] drm/i915: Include the LUT sizes in the state dump
       [not found] ` <20200717211345.26851-4-ville.syrjala@linux.intel.com>
@ 2020-09-17 19:27   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 19:27 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:43 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 03/20] drm/i915: Include the LUT sizes in the state
> dump
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Dump the sizes of the software LUTs in the state dump. Makes it a bit easier to
> see which is is present without having to decode it from the gamma_mode and

Nitpick: Drop an extra "is"
With this,
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> other bits of state.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index c36379cf07fc..9279df7757fc 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13131,6 +13131,12 @@ static void intel_dump_pipe_config(const struct
> intel_crtc_state *pipe_config,
>  			    pipe_config->csc_mode, pipe_config->gamma_mode,
>  			    pipe_config->gamma_enable, pipe_config-
> >csc_enable);
> 
> +	drm_dbg_kms(&dev_priv->drm, "degamma lut: %d entries, gamma lut:
> %d entries\n",
> +		    pipe_config->hw.degamma_lut ?
> +		    drm_color_lut_size(pipe_config->hw.degamma_lut) : 0,
> +		    pipe_config->hw.gamma_lut ?
> +		    drm_color_lut_size(pipe_config->hw.gamma_lut) : 0);
> +
>  dump_planes:
>  	if (!state)
>  		return;
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 04/20] drm/i915: s/glk_read_lut_10/bdw_read_lut_10/
       [not found] ` <20200717211345.26851-5-ville.syrjala@linux.intel.com>
@ 2020-09-17 19:29   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 19:29 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:43 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 04/20] drm/i915:
> s/glk_read_lut_10/bdw_read_lut_10/
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> glk_read_lut_10() works just find for all bdw+ platforms, so rename it.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 945bb03bdd4d..77c103a86a30 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1919,7 +1919,8 @@ static void ilk_read_luts(struct intel_crtc_state
> *crtc_state)
>  		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc);  }
> 
> -static struct drm_property_blob *glk_read_lut_10(struct intel_crtc *crtc,
> +/* On BDW+ the index auto increment mode actually works */ static
> +struct drm_property_blob *bdw_read_lut_10(struct intel_crtc *crtc,
>  						 u32 prec_index)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1960,7
> +1961,7 @@ static void glk_read_luts(struct intel_crtc_state *crtc_state)
>  	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
>  		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
>  	else
> -		crtc_state->hw.gamma_lut = glk_read_lut_10(crtc,
> PAL_PREC_INDEX_VALUE(0));
> +		crtc_state->hw.gamma_lut = bdw_read_lut_10(crtc,
> +PAL_PREC_INDEX_VALUE(0));
>  }
> 
>  static struct drm_property_blob *
> @@ -2016,7 +2017,7 @@ static void icl_read_luts(struct intel_crtc_state
> *crtc_state)
>  		crtc_state->hw.gamma_lut = icl_read_lut_multi_segment(crtc);
>  		break;
>  	default:
> -		crtc_state->hw.gamma_lut = glk_read_lut_10(crtc,
> PAL_PREC_INDEX_VALUE(0));
> +		crtc_state->hw.gamma_lut = bdw_read_lut_10(crtc,
> +PAL_PREC_INDEX_VALUE(0));
>  	}
>  }
> 
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 05/20] drm/i915: Reset glk degamma index after programming/readout
       [not found] ` <20200717211345.26851-6-ville.syrjala@linux.intel.com>
@ 2020-09-17 19:39   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 19:39 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 05/20] drm/i915: Reset glk degamma index after
> programming/readout
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Just for some extra consistency let's reset the glk degamma LUT index back to 0
> after we're dong trawling the LUT.

We do set this to 0 in the beginning, but I think good to leave in a clean state.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 77c103a86a30..37a4fede7bc0 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -818,12 +818,14 @@ static void glk_load_degamma_lut(const struct
> intel_crtc_state *crtc_state)
>  		 * as compared to just 16 to achieve this.
>  		 */
>  		intel_de_write(dev_priv, PRE_CSC_GAMC_DATA(pipe),
> -		               lut[i].green);
> +			       lut[i].green);
>  	}
> 
>  	/* Clamp values > 1.0. */
>  	while (i++ < 35)
>  		intel_de_write(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
> +
> +	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
>  }
> 
>  static void glk_load_degamma_lut_linear(const struct intel_crtc_state
> *crtc_state) @@ -851,6 +853,8 @@ static void
> glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_stat
>  	/* Clamp values > 1.0. */
>  	while (i++ < 35)
>  		intel_de_write(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
> +
> +	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
>  }
> 
>  static void glk_load_luts(const struct intel_crtc_state *crtc_state)
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 06/20] drm/i915: Shuffle chv_cgm_gamma_pack() around a bit
       [not found] ` <20200717211345.26851-7-ville.syrjala@linux.intel.com>
@ 2020-09-17 19:42   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 19:42 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 06/20] drm/i915: Shuffle chv_cgm_gamma_pack()
> around a bit
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Move chv_cgm_gamma_pack() next to the other CGM gamma functions.
> Right now it's stuck in the middle of the CGM degamma functions.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 37a4fede7bc0..260bbbd5bbf2 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1030,13 +1030,6 @@ static u32 chv_cgm_degamma_udw(const struct
> drm_color_lut *color)
>  	return drm_color_lut_extract(color->red, 14);  }
> 
> -static void chv_cgm_gamma_pack(struct drm_color_lut *entry, u32 ldw, u32
> udw) -{
> -	entry->green =
> intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_GREEN_MASK, ldw),
> 10);
> -	entry->blue =
> intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_BLUE_MASK, ldw), 10);
> -	entry->red =
> intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_RED_MASK, udw), 10);
> -}
> -
>  static void chv_load_cgm_degamma(struct intel_crtc *crtc,
>  				 const struct drm_property_blob *blob)  { @@ -
> 1064,6 +1057,13 @@ static u32 chv_cgm_gamma_udw(const struct
> drm_color_lut *color)
>  	return drm_color_lut_extract(color->red, 10);  }
> 
> +static void chv_cgm_gamma_pack(struct drm_color_lut *entry, u32 ldw,
> +u32 udw) {
> +	entry->green =
> intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_GREEN_MASK, ldw),
> 10);
> +	entry->blue =
> intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_BLUE_MASK, ldw), 10);
> +	entry->red =
> +intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_RED_MASK, udw),
> 10);
> +}
> +
>  static void chv_load_cgm_gamma(struct intel_crtc *crtc,
>  			       const struct drm_property_blob *blob)  {
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 07/20] drm/i915: Relocate CHV CGM gamma masks
       [not found] ` <20200717211345.26851-8-ville.syrjala@linux.intel.com>
@ 2020-09-17 19:46   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 19:46 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 07/20] drm/i915: Relocate CHV CGM gamma masks
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> CGM_PIPE_GAMMA_RED_MASK & co. are misplaced. Move then below the
> relevant register. And while at it add the degamma counterparts.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b9607ac3620d..428ef06b8084 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10999,14 +10999,17 @@ enum skl_power_gate {
>  #define _CGM_PIPE_A_CSC_COEFF67	(VLV_DISPLAY_BASE + 0x6790C)
>  #define _CGM_PIPE_A_CSC_COEFF8	(VLV_DISPLAY_BASE + 0x67910)
>  #define _CGM_PIPE_A_DEGAMMA	(VLV_DISPLAY_BASE + 0x66000)
> +#define   CGM_PIPE_DEGAMMA_RED_MASK	REG_GENMASK(13, 0)
> +#define   CGM_PIPE_DEGAMMA_GREEN_MASK	REG_GENMASK(29, 16)
> +#define   CGM_PIPE_DEGAMMA_BLUE_MASK	REG_GENMASK(13, 0)
>  #define _CGM_PIPE_A_GAMMA	(VLV_DISPLAY_BASE + 0x67000)
> +#define   CGM_PIPE_GAMMA_RED_MASK	REG_GENMASK(9, 0)
> +#define   CGM_PIPE_GAMMA_GREEN_MASK	REG_GENMASK(25, 16)
> +#define   CGM_PIPE_GAMMA_BLUE_MASK	REG_GENMASK(9, 0)
>  #define _CGM_PIPE_A_MODE	(VLV_DISPLAY_BASE + 0x67A00)
>  #define   CGM_PIPE_MODE_GAMMA	(1 << 2)
>  #define   CGM_PIPE_MODE_CSC	(1 << 1)
>  #define   CGM_PIPE_MODE_DEGAMMA	(1 << 0)
> -#define   CGM_PIPE_GAMMA_RED_MASK   REG_GENMASK(9, 0)
> -#define   CGM_PIPE_GAMMA_GREEN_MASK REG_GENMASK(25, 16)
> -#define   CGM_PIPE_GAMMA_BLUE_MASK  REG_GENMASK(9, 0)
> 
>  #define _CGM_PIPE_B_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x69900)
>  #define _CGM_PIPE_B_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x69904)
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 08/20] drm/i915: Add glk+ degamma readout
       [not found] ` <20200717211345.26851-9-ville.syrjala@linux.intel.com>
@ 2020-09-17 19:58   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 19:58 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 08/20] drm/i915: Add glk+ degamma readout
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Read out the degamma LUT on glk+. No state cheker as of yet since it requires
> dealing with he glk csc vs. degamma mess.

s/he/the
With this,
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 44 ++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 260bbbd5bbf2..437cc56925ab 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1955,10 +1955,51 @@ static struct drm_property_blob
> *bdw_read_lut_10(struct intel_crtc *crtc,
>  	return blob;
>  }
> 
> +static struct drm_property_blob *glk_read_degamma_lut(struct intel_crtc
> +*crtc) {
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	int i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> +	enum pipe pipe = crtc->pipe;
> +	struct drm_property_blob *blob;
> +	struct drm_color_lut *lut;
> +
> +	blob = drm_property_create_blob(&dev_priv->drm,
> +					sizeof(struct drm_color_lut) * lut_size,
> +					NULL);
> +	if (IS_ERR(blob))
> +		return NULL;
> +
> +	lut = blob->data;
> +
> +	/*
> +	 * When setting the auto-increment bit, the hardware seems to
> +	 * ignore the index bits, so we need to reset it to index 0
> +	 * separately.
> +	 */
> +	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> +	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
> +		       PRE_CSC_GAMC_AUTO_INCREMENT);
> +
> +	for (i = 0; i < lut_size; i++) {
> +		u32 val = intel_de_read(dev_priv, PRE_CSC_GAMC_DATA(pipe));
> +
> +		lut[i].red = val;
> +		lut[i].green = val;
> +		lut[i].blue = val;
> +	}
> +
> +	intel_de_write(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> +
> +	return blob;
> +}
> +
>  static void glk_read_luts(struct intel_crtc_state *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> 
> +	if (crtc_state->csc_enable)
> +		crtc_state->hw.degamma_lut = glk_read_degamma_lut(crtc);
> +
>  	if (!crtc_state->gamma_enable)
>  		return;
> 
> @@ -2010,6 +2051,9 @@ static void icl_read_luts(struct intel_crtc_state
> *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> 
> +	if (crtc_state->gamma_mode & PRE_CSC_GAMMA_ENABLE)
> +		crtc_state->hw.degamma_lut = glk_read_degamma_lut(crtc);
> +
>  	if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
>  		return;
> 
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 09/20] drm/i915: Read out CHV CGM degamma
       [not found] ` <20200717211345.26851-10-ville.syrjala@linux.intel.com>
@ 2020-09-17 20:06   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 20:06 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 09/20] drm/i915: Read out CHV CGM degamma
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since CHV has the dedicate CGM degamma unit readout is trivial.
> Just do it.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 36 ++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 437cc56925ab..6842f5c0356d 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1030,6 +1030,13 @@ static u32 chv_cgm_degamma_udw(const struct
> drm_color_lut *color)
>  	return drm_color_lut_extract(color->red, 14);  }
> 
> +static void chv_cgm_degamma_pack(struct drm_color_lut *entry, u32 ldw,
> +u32 udw) {
> +	entry->green =
> intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_DEGAMMA_GREEN_MASK, ldw),
> 14);
> +	entry->blue =
> intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_DEGAMMA_BLUE_MASK, ldw),
> 14);
> +	entry->red =
> +intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_DEGAMMA_RED_MASK, udw),
> +14); }
> +
>  static void chv_load_cgm_degamma(struct intel_crtc *crtc,
>  				 const struct drm_property_blob *blob)  { @@ -
> 1821,6 +1828,32 @@ static void i965_read_luts(struct intel_crtc_state
> *crtc_state)
>  		crtc_state->hw.gamma_lut = i965_read_lut_10p6(crtc);  }
> 
> +static struct drm_property_blob *chv_read_cgm_degamma(struct intel_crtc
> +*crtc) {
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	int i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> +	enum pipe pipe = crtc->pipe;
> +	struct drm_property_blob *blob;
> +	struct drm_color_lut *lut;
> +
> +	blob = drm_property_create_blob(&dev_priv->drm,
> +					sizeof(struct drm_color_lut) * lut_size,
> +					NULL);
> +	if (IS_ERR(blob))
> +		return NULL;
> +
> +	lut = blob->data;
> +
> +	for (i = 0; i < lut_size; i++) {
> +		u32 ldw = intel_de_read(dev_priv, CGM_PIPE_DEGAMMA(pipe, i,
> 0));
> +		u32 udw = intel_de_read(dev_priv, CGM_PIPE_DEGAMMA(pipe, i,
> 1));
> +
> +		chv_cgm_degamma_pack(&lut[i], ldw, udw);
> +	}
> +
> +	return blob;
> +}
> +
>  static struct drm_property_blob *chv_read_cgm_gamma(struct intel_crtc *crtc)
> {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1851,6
> +1884,9 @@ static void chv_read_luts(struct intel_crtc_state *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> 
> +	if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)
> +		crtc_state->hw.degamma_lut = chv_read_cgm_degamma(crtc);
> +
>  	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
>  		crtc_state->hw.gamma_lut = chv_read_cgm_gamma(crtc);
>  	else
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 10/20] drm/i915: Add gamma/degamma readout for bdw+
       [not found] ` <20200717211345.26851-11-ville.syrjala@linux.intel.com>
@ 2020-09-17 20:15   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 20:15 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 10/20] drm/i915: Add gamma/degamma readout for
> bdw+
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Read out the gamme/degamma LUT on bdw+. Not entirely complete as we need

s/gamme/gamma
With this,
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> a bit more special sauce to handle the split gamma mode.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 27 ++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 6842f5c0356d..5742ac1af862 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1991,6 +1991,32 @@ static struct drm_property_blob
> *bdw_read_lut_10(struct intel_crtc *crtc,
>  	return blob;
>  }
> 
> +static void bdw_read_luts(struct intel_crtc_state *crtc_state) {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_property_blob **blob =
> +		crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA ?
> +		&crtc_state->hw.gamma_lut : &crtc_state->hw.degamma_lut;
> +
> +	if (!crtc_state->gamma_enable)
> +		return;
> +
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		*blob = ilk_read_lut_8(crtc);
> +		break;
> +	case GAMMA_MODE_MODE_SPLIT:
> +		/* FIXME */
> +		break;
> +	case GAMMA_MODE_MODE_10BIT:
> +		*blob = bdw_read_lut_10(crtc, PAL_PREC_INDEX_VALUE(0));
> +		break;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		break;
> +	}
> +}
> +
>  static struct drm_property_blob *glk_read_degamma_lut(struct intel_crtc *crtc)
> {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -2154,6
> +2180,7 @@ void intel_color_init(struct intel_crtc *crtc)
>  			dev_priv->display.read_luts = glk_read_luts;
>  		} else if (INTEL_GEN(dev_priv) >= 8) {
>  			dev_priv->display.load_luts = bdw_load_luts;
> +			dev_priv->display.read_luts = bdw_read_luts;
>  		} else if (INTEL_GEN(dev_priv) >= 7) {
>  			dev_priv->display.load_luts = ivb_load_luts;
>  		} else {
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 11/20] drm/i915: Do degamma+gamma readout in bdw+ split gamma mode
       [not found] ` <20200717211345.26851-12-ville.syrjala@linux.intel.com>
@ 2020-09-17 20:40   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 20:40 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 11/20] drm/i915: Do degamma+gamma readout in
> bdw+ split gamma mode
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Read out both gamma and degamma when usng the split gamma mode on

s/usng/using
With this,
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> bdw+. We can't use the auto increment mode to iterate the LUTs since we want
> to read out less entries than what we stuff into the software LUT.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 52 +++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 5742ac1af862..f34257922e4d 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1959,6 +1959,46 @@ static void ilk_read_luts(struct intel_crtc_state
> *crtc_state)
>  		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc);  }
> 
> +/*
> + * IVB/HSW Bspec / PAL_PREC_INDEX:
> + * "Restriction : Index auto increment mode is not
> + *  supported and must not be enabled."
> + */
> +static struct drm_property_blob *ivb_read_lut_10(struct intel_crtc *crtc,
> +						 u32 prec_index)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	int i, hw_lut_size = ivb_lut_10_size(prec_index);
> +	int lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> +	enum pipe pipe = crtc->pipe;
> +	struct drm_property_blob *blob;
> +	struct drm_color_lut *lut;/
> +
> +	blob = drm_property_create_blob(&dev_priv->drm,
> +					sizeof(struct drm_color_lut) * lut_size,
> +					NULL);
> +	if (IS_ERR(blob))
> +		return NULL;
> +
> +	lut = blob->data;
> +
> +	for (i = 0; i < lut_size; i++) {
> +		/* We discard half the user entries in split gamma mode */
> +		int index = DIV_ROUND_UP(i * (hw_lut_size - 1), lut_size - 1);
> +		u32 val;
> +
> +		intel_de_write(dev_priv, PREC_PAL_INDEX(pipe),
> +			       prec_index + index);
> +		val = intel_de_read(dev_priv, PREC_PAL_DATA(pipe));
> +
> +		ilk_lut_10_pack(&lut[i], val);
> +	}
> +
> +	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe), 0);
> +
> +	return blob;
> +}
> +
>  /* On BDW+ the index auto increment mode actually works */  static struct
> drm_property_blob *bdw_read_lut_10(struct intel_crtc *crtc,
>  						 u32 prec_index)
> @@ -2006,7 +2046,17 @@ static void bdw_read_luts(struct intel_crtc_state
> *crtc_state)
>  		*blob = ilk_read_lut_8(crtc);
>  		break;
>  	case GAMMA_MODE_MODE_SPLIT:
> -		/* FIXME */
> +		/*
> +		 * Can't use bdw_read_lut_10() with its auto-increment
> +		 * mode here since we want to generate 1024 entry
> +		 * software LUTs from the 512 entry hardware LUTs.
> +		 */
> +		crtc_state->hw.degamma_lut =
> +			ivb_read_lut_10(crtc, PAL_PREC_SPLIT_MODE |
> +					PAL_PREC_INDEX_VALUE(0));
> +		crtc_state->hw.gamma_lut =
> +			ivb_read_lut_10(crtc, PAL_PREC_SPLIT_MODE |
> +					PAL_PREC_INDEX_VALUE(512));
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
>  		*blob = bdw_read_lut_10(crtc, PAL_PREC_INDEX_VALUE(0));
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 12/20] drm/i915: Polish bdw_read_lut_10() a bit
       [not found] ` <20200717211345.26851-13-ville.syrjala@linux.intel.com>
@ 2020-09-17 20:43   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 20:43 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 12/20] drm/i915: Polish bdw_read_lut_10() a bit
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since bdw_read_lut_10() uses the auto-increment mode we must have an equal
> number of entries in the software LUT and the hardware LUT. WARN if that is
> not the case.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index f34257922e4d..9f01fb316efa 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -2005,12 +2005,15 @@ static struct drm_property_blob
> *bdw_read_lut_10(struct intel_crtc *crtc,  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	int i, hw_lut_size = ivb_lut_10_size(prec_index);
> +	int lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
>  	enum pipe pipe = crtc->pipe;
>  	struct drm_property_blob *blob;
>  	struct drm_color_lut *lut;
> 
> +	drm_WARN_ON(&dev_priv->drm, lut_size != hw_lut_size);
> +
>  	blob = drm_property_create_blob(&dev_priv->drm,
> -					sizeof(struct drm_color_lut) *
> hw_lut_size,
> +					sizeof(struct drm_color_lut) * lut_size,
>  					NULL);
>  	if (IS_ERR(blob))
>  		return NULL;
> @@ -2020,7 +2023,7 @@ static struct drm_property_blob
> *bdw_read_lut_10(struct intel_crtc *crtc,
>  	intel_de_write(dev_priv, PREC_PAL_INDEX(pipe),
>  		       prec_index | PAL_PREC_AUTO_INCREMENT);
> 
> -	for (i = 0; i < hw_lut_size; i++) {
> +	for (i = 0; i < lut_size; i++) {
>  		u32 val = intel_de_read(dev_priv, PREC_PAL_DATA(pipe));
> 
>  		ilk_lut_10_pack(&lut[i], val);
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 13/20] drm/i915: Add gamma/degamm readout for ivb/hsw
       [not found] ` <20200717211345.26851-14-ville.syrjala@linux.intel.com>
@ 2020-09-17 20:46   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 20:46 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 13/20] drm/i915: Add gamma/degamm readout for

Typo in degamma.
With this fixed,
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> ivb/hsw
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We now have all the code necessary for gamma/degamma readout on ivb/hsw.
> Plug it all in.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 32 ++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 9f01fb316efa..886f3f0d873a 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1999,6 +1999,37 @@ static struct drm_property_blob
> *ivb_read_lut_10(struct intel_crtc *crtc,
>  	return blob;
>  }
> 
> +static void ivb_read_luts(struct intel_crtc_state *crtc_state) {
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_property_blob **blob =
> +		crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA ?
> +		&crtc_state->hw.gamma_lut : &crtc_state->hw.degamma_lut;
> +
> +	if (!crtc_state->gamma_enable)
> +		return;
> +
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		*blob = ilk_read_lut_8(crtc);
> +		break;
> +	case GAMMA_MODE_MODE_SPLIT:
> +		crtc_state->hw.degamma_lut =
> +			ivb_read_lut_10(crtc, PAL_PREC_SPLIT_MODE |
> +					PAL_PREC_INDEX_VALUE(0));
> +		crtc_state->hw.gamma_lut =
> +			ivb_read_lut_10(crtc, PAL_PREC_SPLIT_MODE |
> +					PAL_PREC_INDEX_VALUE(512));
> +		break;
> +	case GAMMA_MODE_MODE_10BIT:
> +		*blob = ivb_read_lut_10(crtc, PAL_PREC_INDEX_VALUE(0));
> +		break;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		break;
> +	}
> +}
> +
>  /* On BDW+ the index auto increment mode actually works */  static struct
> drm_property_blob *bdw_read_lut_10(struct intel_crtc *crtc,
>  						 u32 prec_index)
> @@ -2236,6 +2267,7 @@ void intel_color_init(struct intel_crtc *crtc)
>  			dev_priv->display.read_luts = bdw_read_luts;
>  		} else if (INTEL_GEN(dev_priv) >= 7) {
>  			dev_priv->display.load_luts = ivb_load_luts;
> +			dev_priv->display.read_luts = ivb_read_luts;
>  		} else {
>  			dev_priv->display.load_luts = ilk_load_luts;
>  			dev_priv->display.read_luts = ilk_read_luts;
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 14/20] drm/i915: Replace some gamma_mode ifs with switches
       [not found] ` <20200717211345.26851-15-ville.syrjala@linux.intel.com>
@ 2020-09-17 20:52   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 20:52 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 14/20] drm/i915: Replace some gamma_mode ifs with
> switches
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since gamma_mode can have more than two values on ilk+ let's use switch
> statemnts when interpreting them.

Nit: Typo in statements
With this fixed,
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 93 ++++++++++++++++------
>  1 file changed, 70 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 886f3f0d873a..d5ce58c3bc11 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -638,10 +638,17 @@ static void ilk_load_luts(const struct intel_crtc_state
> *crtc_state)
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	const struct drm_property_blob *gamma_lut = crtc_state-
> >hw.gamma_lut;
> 
> -	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
>  		ilk_load_lut_8(crtc, gamma_lut);
> -	else
> +		break;
> +	case GAMMA_MODE_MODE_10BIT:
>  		ilk_load_lut_10(crtc, gamma_lut);
> +		break;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		break;
> +	}
>  }
> 
>  static int ivb_lut_10_size(u32 prec_index) @@ -745,21 +752,27 @@ static void
> ivb_load_luts(const struct intel_crtc_state *crtc_state)
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	const struct drm_property_blob *gamma_lut = crtc_state-
> >hw.gamma_lut;
>  	const struct drm_property_blob *degamma_lut = crtc_state-
> >hw.degamma_lut;
> +	const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
> 
> -	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
> -		ilk_load_lut_8(crtc, gamma_lut);
> -	} else if (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		ilk_load_lut_8(crtc, blob);
> +		break;
> +	case GAMMA_MODE_MODE_SPLIT:
>  		ivb_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
>  		ivb_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(512));
> -	} else {
> -		const struct drm_property_blob *blob = gamma_lut ?:
> degamma_lut;
> -
> +		break;
> +	case GAMMA_MODE_MODE_10BIT:
>  		ivb_load_lut_10(crtc, blob,
>  				PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
> +		break;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		break;
>  	}
>  }
> 
> @@ -768,21 +781,27 @@ static void bdw_load_luts(const struct intel_crtc_state
> *crtc_state)
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	const struct drm_property_blob *gamma_lut = crtc_state-
> >hw.gamma_lut;
>  	const struct drm_property_blob *degamma_lut = crtc_state-
> >hw.degamma_lut;
> +	const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
> 
> -	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
> -		ilk_load_lut_8(crtc, gamma_lut);
> -	} else if (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		ilk_load_lut_8(crtc, blob);
> +		break;
> +	case GAMMA_MODE_MODE_SPLIT:
>  		bdw_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
>  		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(512));
> -	} else {
> -		const struct drm_property_blob *blob = gamma_lut ?:
> degamma_lut;
> -
> +		break;
> +	case GAMMA_MODE_MODE_10BIT:
>  		bdw_load_lut_10(crtc, blob,
>  				PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
> +		break;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		break;
>  	}
>  }
> 
> @@ -875,11 +894,17 @@ static void glk_load_luts(const struct intel_crtc_state
> *crtc_state)
>  	else
>  		glk_load_degamma_lut_linear(crtc_state);
> 
> -	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
>  		ilk_load_lut_8(crtc, gamma_lut);
> -	} else {
> +		break;
> +	case GAMMA_MODE_MODE_10BIT:
>  		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
> +		break;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		break;
>  	}
>  }
> 
> @@ -1011,9 +1036,13 @@ static void icl_load_luts(const struct intel_crtc_state
> *crtc_state)
>  		icl_program_gamma_superfine_segment(crtc_state);
>  		icl_program_gamma_multi_segment(crtc_state);
>  		break;
> -	default:
> +	case GAMMA_MODE_MODE_10BIT:
>  		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
> +		break;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		break;
>  	}
> 
>  	intel_dsb_commit(crtc_state);
> @@ -1744,7 +1773,7 @@ bool intel_color_lut_equal(struct drm_property_blob
> *blob1,
>  		break;
>  	default:
>  		MISSING_CASE(gamma_mode);
> -			return false;
> +		return false;
>  	}
> 
>  	return true;
> @@ -1953,10 +1982,17 @@ static void ilk_read_luts(struct intel_crtc_state
> *crtc_state)
>  	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
>  		return;
> 
> -	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
>  		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
> -	else
> +		break;
> +	case GAMMA_MODE_MODE_10BIT:
>  		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc);
> +		break;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		break;
> +	}
>  }
> 
>  /*
> @@ -2149,10 +2185,17 @@ static void glk_read_luts(struct intel_crtc_state
> *crtc_state)
>  	if (!crtc_state->gamma_enable)
>  		return;
> 
> -	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
>  		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
> -	else
> +		break;
> +	case GAMMA_MODE_MODE_10BIT:
>  		crtc_state->hw.gamma_lut = bdw_read_lut_10(crtc,
> PAL_PREC_INDEX_VALUE(0));
> +		break;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		break;
> +	}
>  }
> 
>  static struct drm_property_blob *
> @@ -2207,11 +2250,15 @@ static void icl_read_luts(struct intel_crtc_state
> *crtc_state)
>  	case GAMMA_MODE_MODE_8BIT:
>  		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
>  		break;
> +	case GAMMA_MODE_MODE_10BIT:
> +		crtc_state->hw.gamma_lut = bdw_read_lut_10(crtc,
> PAL_PREC_INDEX_VALUE(0));
> +		break;
>  	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>  		crtc_state->hw.gamma_lut = icl_read_lut_multi_segment(crtc);
>  		break;
>  	default:
> -		crtc_state->hw.gamma_lut = bdw_read_lut_10(crtc,
> PAL_PREC_INDEX_VALUE(0));
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		break;
>  	}
>  }
> 
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 15/20] drm/i915: Make ilk_load_luts() deal with degamma
       [not found] ` <20200717211345.26851-16-ville.syrjala@linux.intel.com>
@ 2020-09-17 20:56   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 20:56 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 15/20] drm/i915: Make ilk_load_luts() deal with
> degamma
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make ilk_load_luts() ready for a degamma lut. Currently we never have one, but
> soon we may get one from readout, and I think we may want to change the state
> computation such that we may end up with one even when userspace has simply
> supplied a gamma lut.
> 
> At least the code now follows the path laid out by the ivb/bdw counterpars.

Sounds good.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index d5ce58c3bc11..12a41fb4a98c 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -637,13 +637,15 @@ static void ilk_load_luts(const struct intel_crtc_state
> *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	const struct drm_property_blob *gamma_lut = crtc_state-
> >hw.gamma_lut;
> +	const struct drm_property_blob *degamma_lut = crtc_state-
> >hw.degamma_lut;
> +	const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		ilk_load_lut_8(crtc, gamma_lut);
> +		ilk_load_lut_8(crtc, blob);
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -		ilk_load_lut_10(crtc, gamma_lut);
> +		ilk_load_lut_10(crtc, blob);
>  		break;
>  	default:
>  		MISSING_CASE(crtc_state->gamma_mode);
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 16/20] drm/i915: Make ilk_read_luts() capable of degamma readout
       [not found] ` <20200717211345.26851-17-ville.syrjala@linux.intel.com>
@ 2020-09-17 20:58   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 20:58 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 16/20] drm/i915: Make ilk_read_luts() capable of
> degamma readout
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Just like ivb+, ilk/snb can select whether the hw lut acts as gamma or degamma.
> Make the readout cognizant of that fact.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 12a41fb4a98c..7f9e26419b56 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1977,19 +1977,19 @@ static struct drm_property_blob
> *ilk_read_lut_10(struct intel_crtc *crtc)  static void ilk_read_luts(struct
> intel_crtc_state *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +	struct drm_property_blob **blob =
> +		crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA ?
> +		&crtc_state->hw.gamma_lut : &crtc_state->hw.degamma_lut;
> 
>  	if (!crtc_state->gamma_enable)
>  		return;
> 
> -	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
> -		return;
> -
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
> +		*blob = ilk_read_lut_8(crtc);
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc);
> +		*blob = ilk_read_lut_10(crtc);
>  		break;
>  	default:
>  		MISSING_CASE(crtc_state->gamma_mode);
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 17/20] drm/i915: Make .read_luts() mandatory
       [not found] ` <20200717211345.26851-18-ville.syrjala@linux.intel.com>
@ 2020-09-17 21:00   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 21:00 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 17/20] drm/i915: Make .read_luts() mandatory
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Every platform now implemnts .read_luts(). Make it not optional.
Nit: Typo in implements.
With this,
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 7f9e26419b56..acf3d152edfe 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1215,8 +1215,7 @@ void intel_color_get_config(struct intel_crtc_state
> *crtc_state)  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> 
> -	if (dev_priv->display.read_luts)
> -		dev_priv->display.read_luts(crtc_state);
> +	dev_priv->display.read_luts(crtc_state);
>  }
> 
>  static bool need_plane_update(struct intel_plane *plane,
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 18/20] drm/i915: Extract ilk_crtc_has_gamma() & co.
       [not found] ` <20200717211345.26851-19-ville.syrjala@linux.intel.com>
@ 2020-09-17 21:03   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 21:03 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 18/20] drm/i915: Extract ilk_crtc_has_gamma() & co.
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Extract a few helpers to check whether the hw degamma or gamma LUT is
> enabled.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 27 ++++++++++++++++------
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index acf3d152edfe..f714c87d8e42 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1628,12 +1628,15 @@ static int i9xx_gamma_precision(const struct
> intel_crtc_state *crtc_state)
>  	}
>  }
> 
> +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_gamma_precision(const struct intel_crtc_state *crtc_state)  {
> -	if (!crtc_state->gamma_enable)
> -		return 0;
> -
> -	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
> +	if (!ilk_crtc_has_gamma(crtc_state))
>  		return 0;
> 
>  	switch (crtc_state->gamma_mode) {
> @@ -1671,9 +1674,19 @@ static int glk_gamma_precision(const struct
> intel_crtc_state *crtc_state)
>  	}
>  }
> 
> +static bool icl_crtc_has_degamma(const struct intel_crtc_state
> +*crtc_state) {
> +	return crtc_state->gamma_mode & PRE_CSC_GAMMA_ENABLE; }
> +
> +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_gamma_precision(const struct intel_crtc_state *crtc_state)  {
> -	if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
> +	if (!icl_crtc_has_gamma(crtc_state))
>  		return 0;
> 
>  	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
> @@ -2241,10 +2254,10 @@ static void icl_read_luts(struct intel_crtc_state
> *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> 
> -	if (crtc_state->gamma_mode & PRE_CSC_GAMMA_ENABLE)
> +	if (icl_crtc_has_degamma(crtc_state))
>  		crtc_state->hw.degamma_lut = glk_read_degamma_lut(crtc);
> 
> -	if ((crtc_state->gamma_mode & POST_CSC_GAMMA_ENABLE) == 0)
> +	if (!icl_crtc_has_gamma(crtc_state))
>  		return;
> 
>  	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 19/20] drm/i915: Complete the gamma/degamma state checking
       [not found] ` <20200717211345.26851-20-ville.syrjala@linux.intel.com>
@ 2020-09-17 21:52   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-17 21:52 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> 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ä <ville.syrjala@linux.intel.com>
> 
> 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 <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  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 *crtc_state)  {
>  	if (!icl_crtc_has_gamma(crtc_state))
> @@ -1702,97 +1755,310 @@ static int icl_gamma_precision(const struct
> intel_crtc_state *crtc_state)
>  	}
>  }
> 
> -int intel_color_get_gamma_bit_precision(const struct intel_crtc_state
> *crtc_state) -{
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -
> -	if (HAS_GMCH(dev_priv)) {
> -		if (IS_CHERRYVIEW(dev_priv))
> -			return chv_gamma_precision(crtc_state);
> -		else
> -			return i9xx_gamma_precision(crtc_state);
> -	} else {
> -		if (INTEL_GEN(dev_priv) >= 11)
> -			return icl_gamma_precision(crtc_state);
> -		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> -			return glk_gamma_precision(crtc_state);
> -		else if (IS_IRONLAKE(dev_priv))
> -			return ilk_gamma_precision(crtc_state);
> -	}
> -
> -	return 0;
> -}
> -
> -static bool err_check(struct drm_color_lut *lut1,
> -		      struct drm_color_lut *lut2, u32 err)
> +static bool lut_entry_equal(const struct drm_color_lut *lut1,
> +			    const struct drm_color_lut *lut2, u32 err)
>  {
>  	return ((abs((long)lut2->red - lut1->red)) <= err) &&
>  		((abs((long)lut2->blue - lut1->blue)) <= err) &&
>  		((abs((long)lut2->green - lut1->green)) <= err);  }
> 
> -static bool intel_color_lut_entries_equal(struct drm_color_lut *lut1,
> -					  struct drm_color_lut *lut2,
> -					  int lut_size, u32 err)
> +static int intel_color_lut_size(const struct drm_property_blob *blob)
>  {
> -	int i;
> +	return blob ? drm_color_lut_size(blob) : 0; }
> +
> +static bool intel_color_lut_sizes_equal(const struct drm_property_blob *blob1,
> +					const struct drm_property_blob *blob2) {
> +	return intel_color_lut_size(blob1) == intel_color_lut_size(blob2); }
> +
> +static bool intel_lut_equal(const struct drm_property_blob *blob1,
> +			    const struct drm_property_blob *blob2,
> +			    int bit_precision)
> +{
> +	const struct drm_color_lut *lut1, *lut2;
> +	int i, lut_size;
> +
> +	if (!intel_color_lut_sizes_equal(blob1, blob2))
> +		return false;
> +
> +	if (!blob1)
> +		return true;
> +
> +	if (!bit_precision)
> +		return false;
> +
> +	lut_size = drm_color_lut_size(blob1);
> +	lut1 = blob1->data;
> +	lut2 = blob2->data;
> 
>  	for (i = 0; i < lut_size; i++) {
> -		if (!err_check(&lut1[i], &lut2[i], err))
> +		if (!lut_entry_equal(&lut1[i], &lut2[i],
> +				     0xffff >> bit_precision))
>  			return false;
>  	}
> 
>  	return true;
>  }
> 
> -bool intel_color_lut_equal(struct drm_property_blob *blob1,
> -			   struct drm_property_blob *blob2,
> -			   u32 gamma_mode, u32 bit_precision)
> +static bool i9xx_degamma_equal(const struct intel_crtc_state *crtc_state1,
> +			       const struct intel_crtc_state *crtc_state2,
> +			       bool fastset)
>  {
> -	struct drm_color_lut *lut1, *lut2;
> -	int lut_size1, lut_size2;
> -	u32 err;
> +	const struct drm_property_blob *degamma_lut1 = crtc_state1-
> >hw.degamma_lut;
> +	const struct drm_property_blob *degamma_lut2 =
> +crtc_state2->hw.degamma_lut;
> 
> -	if (!blob1 != !blob2)
> +	/* no degamma */
> +	return !degamma_lut1 && !degamma_lut2; }
> +
> +static bool i9xx_gamma_equal(const struct intel_crtc_state *crtc_state1,
> +			     const struct intel_crtc_state *crtc_state2,
> +			     bool fastset)
> +{
> +	const struct drm_property_blob *gamma_lut1 = crtc_state1-
> >hw.gamma_lut;
> +	const struct drm_property_blob *gamma_lut2 =
> +crtc_state2->hw.gamma_lut;
> +
> +	return intel_lut_equal(gamma_lut1, gamma_lut2,
> +			       i9xx_gamma_precision(crtc_state1));
> +}
> +
> +static bool chv_degamma_equal(const struct intel_crtc_state *crtc_state1,
> +			      const struct intel_crtc_state *crtc_state2,
> +			      bool fastset)
> +{
> +	const struct drm_property_blob *degamma_lut1 = crtc_state1-
> >hw.degamma_lut;
> +	const struct drm_property_blob *degamma_lut2 =
> +crtc_state2->hw.degamma_lut;
> +
> +	return intel_lut_equal(degamma_lut1, degamma_lut2,
> +			       chv_degamma_precision(crtc_state1));
> +}
> +
> +static bool chv_gamma_equal(const struct intel_crtc_state *crtc_state1,
> +			    const struct intel_crtc_state *crtc_state2,
> +			    bool fastset)
> +{
> +	const struct drm_property_blob *gamma_lut1 = crtc_state1-
> >hw.gamma_lut;
> +	const struct drm_property_blob *gamma_lut2 =
> +crtc_state2->hw.gamma_lut;
> +
> +	return intel_lut_equal(gamma_lut1, gamma_lut2,
> +			       chv_gamma_precision(crtc_state1));
> +}
> +
> +static bool ilk_degamma_equal(const struct intel_crtc_state *crtc_state1,
> +			      const struct intel_crtc_state *crtc_state2,
> +			      bool fastset)
> +{
> +	const struct drm_property_blob *degamma_lut1 = crtc_state2-
> >hw.degamma_lut;
> +	const struct drm_property_blob *degamma_lut2 =
> +crtc_state2->hw.degamma_lut;
> +
> +	if (!fastset) {
> +		const struct drm_property_blob *gamma_lut2 =
> +crtc_state2->hw.gamma_lut;
> +
> +		/*
> +		 * For readout crtc_state1 is the software state,
> +		 * crtc_state2 is the hardware state.
> +		 *
> +		 * On ILK-BDW the single LUT may appear before or after the
> +		 * CSC due to various reasons. Readout does not know whether
> +		 * the user supplied it as a gamma LUT or degamma LUT. If the
> +		 * software and hardware states look inconsistent assume
> +		 * readout got things the wrong way around.
> +		 *
> +		 * FIXME think about assigning these consistently from the
> hardware
> +		 * POV already when computing the state. Would avoid this
> hideous
> +		 * hack, but possibly not entirely trivial to do since we currently
> +		 * just blindly copy those from the uapi state.
> +		 */
> +		if (!!degamma_lut1 != !!degamma_lut2)
> +			swap(gamma_lut2, degamma_lut2);
> +	}
> +
> +	return intel_lut_equal(degamma_lut1, degamma_lut2,
> +			       ilk_degamma_precision(crtc_state1));
> +}
> +
> +static bool ilk_gamma_equal(const struct intel_crtc_state *crtc_state1,
> +			    const struct intel_crtc_state *crtc_state2,
> +			    bool fastset)
> +{
> +	const struct drm_property_blob *gamma_lut1 = crtc_state1-
> >hw.gamma_lut;
> +	const struct drm_property_blob *gamma_lut2 =
> +crtc_state2->hw.gamma_lut;
> +
> +	if (!fastset) {
> +		const struct drm_property_blob *degamma_lut2 =
> +crtc_state2->hw.degamma_lut;
> +
> +		/*
> +		 * For readout crtc_state1 is the software state,
> +		 * crtc_state2 is the hardware state.
> +		 *
> +		 * On ILK-BDW the single LUT may appear before or after the
> +		 * CSC due to various reasons. Readout does not know whether
> +		 * the user supplied it as a gamma LUT or degamma LUT. If the
> +		 * software and hardware states look inconsistent assume
> +		 * readout got things the wrong way around.
> +		 *
> +		 * FIXME think about assigning these consistently from the
> hardware
> +		 * POV already when computing the state. Would avoid this
> hideous
> +		 * hack, but possibly not entirely trivial to do since we currently
> +		 * just blindly copy those from the uapi state.
> +		 */
> +		if (!!gamma_lut1 != !!gamma_lut2)
> +			swap(gamma_lut2, degamma_lut2);
> +	}
> +
> +	return intel_lut_equal(gamma_lut1, gamma_lut2,
> +			       ilk_gamma_precision(crtc_state1));
> +}
> +
> +static bool ivb_lut_split_equal(const struct drm_property_blob *blob1,
> +				const struct drm_property_blob *blob2,
> +				int bit_precision)
> +{
> +	const struct drm_color_lut *lut1, *lut2;
> +	int i, lut_size, hw_lut_size;
> +
> +	if (!intel_color_lut_sizes_equal(blob1, blob2))
>  		return false;
> 
>  	if (!blob1)
>  		return true;
> 
> -	lut_size1 = drm_color_lut_size(blob1);
> -	lut_size2 = drm_color_lut_size(blob2);
> -
> -	/* check sw and hw lut size */
> -	if (lut_size1 != lut_size2)
> +	if (!bit_precision)
>  		return false;
> 
> +	lut_size = drm_color_lut_size(blob1);
> +	hw_lut_size = ivb_lut_10_size(PAL_PREC_SPLIT_MODE);
>  	lut1 = blob1->data;
>  	lut2 = blob2->data;
> 
> -	err = 0xffff >> bit_precision;
> +	for (i = 0; i < hw_lut_size; i++) {
> +		/* We discard half the user entries in split gamma mode */
> +		const struct drm_color_lut *entry1 =
> +			&lut1[i * (lut_size - 1) / (hw_lut_size - 1)];
> +		const struct drm_color_lut *entry2 =
> +			&lut2[i * (lut_size - 1) / (hw_lut_size - 1)];
> 
> -	/* check sw and hw lut entry to be equal */
> -	switch (gamma_mode & GAMMA_MODE_MODE_MASK) {
> -	case GAMMA_MODE_MODE_8BIT:
> -	case GAMMA_MODE_MODE_10BIT:
> -		if (!intel_color_lut_entries_equal(lut1, lut2,
> -						   lut_size2, err))
> +		if (!lut_entry_equal(entry1, entry2, 0xffff >> bit_precision))
>  			return false;
> -		break;
> -	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
> -		if (!intel_color_lut_entries_equal(lut1, lut2,
> -						   9, err))
> -			return false;
> -		break;
> -	default:
> -		MISSING_CASE(gamma_mode);
> -		return false;
>  	}
> 
>  	return true;
>  }
> 
> +static bool ivb_degamma_equal(const struct intel_crtc_state *crtc_state1,
> +			      const struct intel_crtc_state *crtc_state2,
> +			      bool fastset)
> +{
> +	if (crtc_state1->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
> +		const struct drm_property_blob *degamma_lut1 = crtc_state1-
> >hw.degamma_lut;
> +		const struct drm_property_blob *degamma_lut2 =
> +crtc_state2->hw.degamma_lut;
> +
> +		return ivb_lut_split_equal(degamma_lut1, degamma_lut2,
> +					   ivb_degamma_precision(crtc_state1));
> +	} else {
> +		return ilk_degamma_equal(crtc_state1, crtc_state2, fastset);
> +	}
> +}
> +
> +static bool ivb_gamma_equal(const struct intel_crtc_state *crtc_state1,
> +			    const struct intel_crtc_state *crtc_state2,
> +			    bool fastset)
> +{
> +	if (crtc_state1->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
> +		const struct drm_property_blob *gamma_lut1 = crtc_state1-
> >hw.gamma_lut;
> +		const struct drm_property_blob *gamma_lut2 =
> +crtc_state2->hw.gamma_lut;
> +
> +		return ivb_lut_split_equal(gamma_lut1, gamma_lut2,
> +					   ivb_gamma_precision(crtc_state1));
> +	} else {
> +		return ilk_gamma_equal(crtc_state1, crtc_state2, fastset);
> +	}
> +}
> +
> +static bool glk_degamma_equal(const struct intel_crtc_state *crtc_state1,
> +			      const struct intel_crtc_state *crtc_state2,
> +			      bool fastset)
> +{
> +	const struct drm_property_blob *degamma_lut1 = crtc_state1-
> >hw.degamma_lut;
> +	const struct drm_property_blob *degamma_lut2 =
> +crtc_state2->hw.degamma_lut;
> +
> +	if (!fastset) {
> +		/*
> +		 * For readout crtc_state1 is the software state,
> +		 * crtc_state2 is the hardware state.
> +		 *
> +		 * Readout can't tell the difference between an actual
> +		 * degamma LUT and the linear degamma LUT we have to load
> +		 * whenever the pipe CSC is active. Ignore the hardware
> +		 * degamma LUT when we don't have a software degamma LUT.
> +		 *
> +		 * FIXME think about assigning an internal linear lut already
> +		 * when computing the state. Would avoid this hideous hack,
> +		 * but possibly not entirely trivial to do since we currently
> +		 * just blindly copy this from the uapi state.
> +		 */
> +		if (!degamma_lut1)
> +			degamma_lut2 = NULL;
> +	}
> +
> +	return intel_lut_equal(degamma_lut1, degamma_lut2,
> +			       glk_degamma_precision(crtc_state1));
> +}
> +
> +static bool glk_gamma_equal(const struct intel_crtc_state *crtc_state1,
> +			    const struct intel_crtc_state *crtc_state2,
> +			    bool fastset)
> +{
> +	const struct drm_property_blob *gamma_lut1 = crtc_state1-
> >hw.gamma_lut;
> +	const struct drm_property_blob *gamma_lut2 =
> +crtc_state2->hw.gamma_lut;
> +
> +	return intel_lut_equal(gamma_lut1, gamma_lut2,
> +			       glk_gamma_precision(crtc_state1));
> +}
> +
> +static bool icl_degamma_equal(const struct intel_crtc_state *crtc_state1,
> +			      const struct intel_crtc_state *crtc_state2,
> +			      bool fastset)
> +{
> +	const struct drm_property_blob *degamma_lut1 = crtc_state1-
> >hw.degamma_lut;
> +	const struct drm_property_blob *degamma_lut2 =
> +crtc_state2->hw.degamma_lut;
> +
> +	return intel_lut_equal(degamma_lut1, degamma_lut2,
> +			       icl_degamma_precision(crtc_state1));
> +}
> +
> +static bool icl_gamma_equal(const struct intel_crtc_state *crtc_state1,
> +			    const struct intel_crtc_state *crtc_state2,
> +			    bool fastset)
> +{
> +	const struct drm_property_blob *gamma_lut1 = crtc_state1-
> >hw.gamma_lut;
> +	const struct drm_property_blob *gamma_lut2 =
> +crtc_state2->hw.gamma_lut;
> +
> +	return intel_lut_equal(gamma_lut1, gamma_lut2,
> +			       icl_gamma_precision(crtc_state1));
> +}
> +
> +bool intel_color_degamma_lut_equal(const struct intel_crtc_state *crtc_state1,
> +				   const struct intel_crtc_state *crtc_state2,
> +				   bool fastset)
> +{
> +	struct drm_i915_private *dev_priv =
> +to_i915(crtc_state1->uapi.crtc->dev);
> +
> +	return dev_priv->display.degamma_equal(crtc_state1, crtc_state2,
> +fastset); }
> +
> +bool intel_color_gamma_lut_equal(const struct intel_crtc_state *crtc_state1,
> +				 const struct intel_crtc_state *crtc_state2,
> +				 bool fastset)
> +{
> +	struct drm_i915_private *dev_priv =
> +to_i915(crtc_state1->uapi.crtc->dev);
> +
> +	return dev_priv->display.gamma_equal(crtc_state1, crtc_state2,
> +fastset); }
> +
>  static struct drm_property_blob *i9xx_read_lut_8(struct intel_crtc *crtc)  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -
> 2289,16 +2555,22 @@ void intel_color_init(struct intel_crtc *crtc)
>  			dev_priv->display.color_commit = i9xx_color_commit;
>  			dev_priv->display.load_luts = chv_load_luts;
>  			dev_priv->display.read_luts = chv_read_luts;
> +			dev_priv->display.gamma_equal = chv_gamma_equal;
> +			dev_priv->display.degamma_equal =
> chv_degamma_equal;
>  		} else if (INTEL_GEN(dev_priv) >= 4) {
>  			dev_priv->display.color_check = i9xx_color_check;
>  			dev_priv->display.color_commit = i9xx_color_commit;
>  			dev_priv->display.load_luts = i965_load_luts;
>  			dev_priv->display.read_luts = i965_read_luts;
> +			dev_priv->display.gamma_equal = i9xx_gamma_equal;
> +			dev_priv->display.degamma_equal =
> i9xx_degamma_equal;
>  		} else {
>  			dev_priv->display.color_check = i9xx_color_check;
>  			dev_priv->display.color_commit = i9xx_color_commit;
>  			dev_priv->display.load_luts = i9xx_load_luts;
>  			dev_priv->display.read_luts = i9xx_read_luts;
> +			dev_priv->display.gamma_equal = i9xx_gamma_equal;
> +			dev_priv->display.degamma_equal =
> i9xx_degamma_equal;
>  		}
>  	} else {
>  		if (INTEL_GEN(dev_priv) >= 11)
> @@ -2320,19 +2592,30 @@ void intel_color_init(struct intel_crtc *crtc)
>  		if (INTEL_GEN(dev_priv) >= 11) {
>  			dev_priv->display.load_luts = icl_load_luts;
>  			dev_priv->display.read_luts = icl_read_luts;
> +			dev_priv->display.gamma_equal = icl_gamma_equal;
> +			dev_priv->display.degamma_equal = icl_degamma_equal;
>  		} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> {
>  			dev_priv->display.load_luts = glk_load_luts;
>  			dev_priv->display.read_luts = glk_read_luts;
> +			dev_priv->display.gamma_equal = glk_gamma_equal;
> +			dev_priv->display.degamma_equal =
> glk_degamma_equal;
>  		} else if (INTEL_GEN(dev_priv) >= 8) {
>  			dev_priv->display.load_luts = bdw_load_luts;
>  			dev_priv->display.read_luts = bdw_read_luts;
> +			dev_priv->display.gamma_equal = ivb_gamma_equal;
> +			dev_priv->display.degamma_equal =
> ivb_degamma_equal;
>  		} else if (INTEL_GEN(dev_priv) >= 7) {
>  			dev_priv->display.load_luts = ivb_load_luts;
>  			dev_priv->display.read_luts = ivb_read_luts;
> +			dev_priv->display.gamma_equal = ivb_gamma_equal;
> +			dev_priv->display.degamma_equal =
> ivb_degamma_equal;
>  		} else {
>  			dev_priv->display.load_luts = ilk_load_luts;
>  			dev_priv->display.read_luts = ilk_read_luts;
> +			dev_priv->display.gamma_equal = ilk_gamma_equal;
> +			dev_priv->display.degamma_equal =
> ilk_degamma_equal;
>  		}
> +
>  	}
> 
>  	drm_crtc_enable_color_mgmt(&crtc->base,
> diff --git a/drivers/gpu/drm/i915/display/intel_color.h
> b/drivers/gpu/drm/i915/display/intel_color.h
> index 173727aaa24d..079ea90c22c8 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.h
> +++ b/drivers/gpu/drm/i915/display/intel_color.h
> @@ -17,9 +17,11 @@ int intel_color_check(struct intel_crtc_state *crtc_state);
> void intel_color_commit(const struct intel_crtc_state *crtc_state);  void
> intel_color_load_luts(const struct intel_crtc_state *crtc_state);  void
> intel_color_get_config(struct intel_crtc_state *crtc_state); -int
> intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state); -
> bool intel_color_lut_equal(struct drm_property_blob *blob1,
> -			   struct drm_property_blob *blob2,
> -			   u32 gamma_mode, u32 bit_precision);
> +bool intel_color_gamma_lut_equal(const struct intel_crtc_state *crtc_state1,
> +				 const struct intel_crtc_state *crtc_state2,
> +				 bool fastset);
> +bool intel_color_degamma_lut_equal(const struct intel_crtc_state *crtc_state1,
> +				   const struct intel_crtc_state *crtc_state2,
> +				   bool fastset);
> 
>  #endif /* __INTEL_COLOR_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 9279df7757fc..e80b4cd8eea1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -13635,7 +13635,6 @@ intel_pipe_config_compare(const struct
> intel_crtc_state *current_config,
>  	struct drm_i915_private *dev_priv = to_i915(current_config->uapi.crtc-
> >dev);
>  	struct intel_crtc *crtc = to_intel_crtc(pipe_config->uapi.crtc);
>  	bool ret = true;
> -	u32 bp_gamma = 0;
>  	bool fixup_inherited = fastset &&
>  		current_config->inherited && !pipe_config->inherited;
> 
> @@ -13798,21 +13797,10 @@ intel_pipe_config_compare(const struct
> intel_crtc_state *current_config,
>  	} \
>  } while (0)
> 
> -#define PIPE_CONF_CHECK_COLOR_LUT(name1, name2, bit_precision) do { \
> -	if (current_config->name1 != pipe_config->name1) { \
> -		pipe_config_mismatch(fastset, crtc, __stringify(name1), \
> -				"(expected %i, found %i, won't compare lut
> values)", \
> -				current_config->name1, \
> -				pipe_config->name1); \
> -		ret = false;\
> -	} else { \
> -		if (!intel_color_lut_equal(current_config->name2, \
> -					pipe_config->name2, pipe_config-
> >name1, \
> -					bit_precision)) { \
> -			pipe_config_mismatch(fastset, crtc, __stringify(name2), \
> -					"hw_state doesn't match sw_state"); \
> -			ret = false; \
> -		} \
> +#define PIPE_CONF_CHECK_COLOR_LUT(name) do { \
> +	if (!intel_color_##name##_equal(current_config, pipe_config, fastset)) { \
> +		pipe_config_mismatch(fastset, crtc, "hw." __stringify(name), " ");
> \
> +		ret = false; \
>  	} \
>  } while (0)
> 
> @@ -13918,9 +13906,8 @@ intel_pipe_config_compare(const struct
> intel_crtc_state *current_config,
>  		PIPE_CONF_CHECK_I(linetime);
>  		PIPE_CONF_CHECK_I(ips_linetime);
> 
> -		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
> -		if (bp_gamma)
> -			PIPE_CONF_CHECK_COLOR_LUT(gamma_mode,
> hw.gamma_lut, bp_gamma);
> +		PIPE_CONF_CHECK_COLOR_LUT(gamma_lut);
> +		PIPE_CONF_CHECK_COLOR_LUT(degamma_lut);
>  	}
> 
>  	PIPE_CONF_CHECK_BOOL(double_wide);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e4f7f6518945..54d224be256a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -256,6 +256,7 @@ struct sdvo_device_mapping {
>  	u8 ddc_pin;
>  };
> 
> +struct drm_property_blob;
>  struct intel_connector;
>  struct intel_encoder;
>  struct intel_atomic_state;
> @@ -334,6 +335,12 @@ struct drm_i915_display_funcs {
>  	 */
>  	void (*load_luts)(const struct intel_crtc_state *crtc_state);
>  	void (*read_luts)(struct intel_crtc_state *crtc_state);
> +	bool (*gamma_equal)(const struct intel_crtc_state *crtc_state1,
> +			    const struct intel_crtc_state *crtc_state2,
> +			    bool fastset);
> +	bool (*degamma_equal)(const struct intel_crtc_state *crtc_state1,
> +			      const struct intel_crtc_state *crtc_state2,
> +			      bool fastset);
>  };
> 
>  struct intel_csr {
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH 20/20] drm/i915: Add 10bit gamma mode for gen2/3
       [not found] ` <20200717211345.26851-21-ville.syrjala@linux.intel.com>
@ 2020-09-21 19:40   ` Shankar, Uma
  0 siblings, 0 replies; 20+ messages in thread
From: Shankar, Uma @ 2020-09-21 19:40 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx@lists.freedesktop.org



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Saturday, July 18, 2020 2:44 AM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 20/20] drm/i915: Add 10bit gamma mode for gen2/3
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Some gen2/gen3 parts have a 10bit gamma mode, on some pipes.
> Expose it.
> 
> The format is different to the later i965+ style in that we store a 10bit value and
> a 6 bit floating point slope for each entry. Ie. the hardware extrapolates the
> intermediate steps from the current LUT entry, instead of interpolating between
> the current and next LUT entries. This also means we don't store the last LUT
> entry in any register as it is defined by the previous LUT entry's value+slope.
> 
> The slope has limited precision though (2 bit exponent + 4 bit mantissa), so we
> have to allow for more error in the state checker for the last entry, and we have
> to make sure userspace doesn't pass in something where the slope is simply to
> steep. In theory we should perhaps check the slope for every interval, but we
> don't do that for any other interpolated gamma mode and I suspect they may
> also have some internal limit on the slope. I haven't confirmed that theory
> though. Anyways, only the last entry provides a slight challenge for the state
> checker since all the other entries do have an explicit value stored in a register.

Couldn't verify the details from spec. But with the description, was not able to spot any issue
with the implementation.	
Acked-by: Uma Shankar <uma.shankar@intel.com>

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 248 ++++++++++++++++++++-
>  drivers/gpu/drm/i915/i915_pci.c            |  10 +-
>  drivers/gpu/drm/i915/i915_reg.h            |  38 +++-
>  3 files changed, 279 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index ca6c6679368c..055a49ed4e3a 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -414,6 +414,79 @@ static void i9xx_lut_8_pack(struct drm_color_lut *entry,
> u32 val)
>  	entry->blue =
> intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);  }
> 
> +/* i8xx/i9xx+ 10bit slope format "even DW" (low 8 bits) */ static u32
> +_i9xx_lut_10_ldw(u16 a) {
> +	return drm_color_lut_extract(a, 10) & 0xff; }
> +
> +static u32 i9xx_lut_10_ldw(const struct drm_color_lut *color) {
> +	return _i9xx_lut_10_ldw(color[0].red) << 16 |
> +		_i9xx_lut_10_ldw(color[0].green) << 8 |
> +		_i9xx_lut_10_ldw(color[0].blue);
> +}
> +
> +/* i8xx/i9xx+ 10bit slope format "odd DW" (high 2 bits + slope) */
> +static u32 _i9xx_lut_10_udw(u16 a, u16 b) {
> +	unsigned int mantissa, exponent;
> +
> +	a = drm_color_lut_extract(a, 10);
> +	b = drm_color_lut_extract(b, 10);
> +
> +	/* b = a + 8 * m * 2 ^ -e */
> +	mantissa = clamp(b - a, 0, 0x7f);
> +	exponent = 3;
> +	while (mantissa > 0xf) {
> +		mantissa >>= 1;
> +		exponent--;
> +	}
> +
> +	return (exponent << 6) |
> +		(mantissa << 2) |
> +		(a >> 8);
> +}
> +
> +static u32 i9xx_lut_10_udw(const struct drm_color_lut *color) {
> +	return _i9xx_lut_10_udw(color[0].red, color[1].red) << 16 |
> +		_i9xx_lut_10_udw(color[0].green, color[1].green) << 8 |
> +		_i9xx_lut_10_udw(color[0].blue, color[1].blue); }
> +
> +static void i9xx_lut_10_pack(struct drm_color_lut *color,
> +			     u32 ldw, u32 udw)
> +{
> +	u16 red = REG_FIELD_GET(PALETTE_10BIT_RED_LDW_MASK, ldw) |
> +		REG_FIELD_GET(PALETTE_10BIT_RED_UDW_MASK, udw) << 8;
> +	u16 green = REG_FIELD_GET(PALETTE_10BIT_GREEN_LDW_MASK, ldw) |
> +		REG_FIELD_GET(PALETTE_10BIT_GREEN_UDW_MASK, udw) << 8;
> +	u16 blue = REG_FIELD_GET(PALETTE_10BIT_BLUE_LDW_MASK, ldw) |
> +		REG_FIELD_GET(PALETTE_10BIT_BLUE_UDW_MASK, udw) << 8;
> +
> +	color->red = intel_color_lut_pack(red, 10);
> +	color->green = intel_color_lut_pack(green, 10);
> +	color->blue = intel_color_lut_pack(blue, 10); }
> +
> +static void i9xx_lut_10_pack_slope(struct drm_color_lut *color,
> +				   u32 ldw, u32 udw)
> +{
> +	int r_exp = REG_FIELD_GET(PALETTE_10BIT_RED_EXP_MASK, udw);
> +	int r_mant = REG_FIELD_GET(PALETTE_10BIT_RED_MANT_MASK, udw);
> +	int g_exp = REG_FIELD_GET(PALETTE_10BIT_GREEN_EXP_MASK, udw);
> +	int g_mant = REG_FIELD_GET(PALETTE_10BIT_GREEN_MANT_MASK,
> udw);
> +	int b_exp = REG_FIELD_GET(PALETTE_10BIT_BLUE_EXP_MASK, udw);
> +	int b_mant = REG_FIELD_GET(PALETTE_10BIT_BLUE_MANT_MASK, udw);
> +
> +	i9xx_lut_10_pack(color, ldw, udw);
> +
> +	color->red += r_mant << (3 - r_exp);
> +	color->green += g_mant << (3 - g_exp);
> +	color->blue += b_mant << (3 - b_exp);
> +}
> +
>  /* i965+ "10.6" bit interpolated format "even DW" (low 8 bits) */  static u32
> i965_lut_10p6_ldw(const struct drm_color_lut *color)  { @@ -554,6 +627,22 @@
> static void i9xx_load_lut_8(struct intel_crtc *crtc,
>  			       i9xx_lut_8(&lut[i]));
>  }
> 
> +static void i9xx_load_lut_10(struct intel_crtc *crtc,
> +			     const struct drm_property_blob *blob) {
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	const struct drm_color_lut *lut = blob->data;
> +	int i, lut_size = drm_color_lut_size(blob);
> +	enum pipe pipe = crtc->pipe;
> +
> +	for (i = 0; i < lut_size - 1; i++) {
> +		intel_de_write(dev_priv, PALETTE(pipe, 2 * i + 0),
> +			       i9xx_lut_10_ldw(&lut[i]));
> +		intel_de_write(dev_priv, PALETTE(pipe, 2 * i + 1),
> +			       i9xx_lut_10_udw(&lut[i]));
> +	}
> +}
> +
>  static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> @@ -562,7 +651,10 @@ static void i9xx_load_luts(const struct intel_crtc_state
> *crtc_state)
> 
>  	assert_pll_enabled(dev_priv, crtc->pipe);
> 
> -	i9xx_load_lut_8(crtc, gamma_lut);
> +	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
> +		i9xx_load_lut_8(crtc, gamma_lut);
> +	else
> +		i9xx_load_lut_10(crtc, gamma_lut);
>  }
> 
>  static void i965_load_lut_10p6(struct intel_crtc *crtc, @@ -1326,11 +1418,36
> @@ static u32 i9xx_gamma_mode(struct intel_crtc_state *crtc_state)
>  	    crtc_state_is_legacy_gamma(crtc_state))
>  		return GAMMA_MODE_MODE_8BIT;
>  	else
> -		return GAMMA_MODE_MODE_10BIT; /* i965+ only */
> +		return GAMMA_MODE_MODE_10BIT;
> +}
> +
> +static int i9xx_lut_10_diff(u16 a, u16 b) {
> +	return drm_color_lut_extract(a, 10) -
> +		drm_color_lut_extract(b, 10);
> +}
> +
> +static int i9xx_check_lut_10(struct drm_i915_private *dev_priv,
> +			     const struct drm_property_blob *blob) {
> +	const struct drm_color_lut *lut = blob->data;
> +	int lut_size = drm_color_lut_size(blob);
> +	const struct drm_color_lut *a = &lut[lut_size - 2];
> +	const struct drm_color_lut *b = &lut[lut_size - 1];
> +
> +	if (i9xx_lut_10_diff(b->red, a->red) > 0x7f ||
> +	    i9xx_lut_10_diff(b->green, a->green) > 0x7f ||
> +	    i9xx_lut_10_diff(b->blue, a->blue) > 0x7f) {
> +		drm_dbg_kms(&dev_priv->drm, "Last gamma LUT entry exceeds
> max slope\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
>  }
> 
>  static int i9xx_color_check(struct intel_crtc_state *crtc_state)  {
> +	struct drm_i915_private *dev_priv =
> +to_i915(crtc_state->uapi.crtc->dev);
>  	int ret;
> 
>  	ret = check_luts(crtc_state);
> @@ -1343,6 +1460,13 @@ static int i9xx_color_check(struct intel_crtc_state
> *crtc_state)
> 
>  	crtc_state->gamma_mode = i9xx_gamma_mode(crtc_state);
> 
> +	if (INTEL_GEN(dev_priv) < 4 &&
> +	    crtc_state->gamma_mode == GAMMA_MODE_MODE_10BIT) {
> +		ret = i9xx_check_lut_10(dev_priv, crtc_state->hw.gamma_lut);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	ret = intel_color_add_affected_planes(crtc_state);
>  	if (ret)
>  		return ret;
> @@ -1613,6 +1737,22 @@ static int icl_color_check(struct intel_crtc_state
> *crtc_state)  }
> 
>  static int i9xx_gamma_precision(const struct intel_crtc_state *crtc_state)
> +{
> +	if (!crtc_state->gamma_enable)
> +		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 int i965_gamma_precision(const struct intel_crtc_state
> +*crtc_state)
>  {
>  	if (!crtc_state->gamma_enable)
>  		return 0;
> @@ -1700,7 +1840,7 @@ static int chv_gamma_precision(const struct
> intel_crtc_state *crtc_state)
>  	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
>  		return 10;
>  	else
> -		return i9xx_gamma_precision(crtc_state);
> +		return i965_gamma_precision(crtc_state);
>  }
> 
>  static int glk_degamma_precision(const struct intel_crtc_state *crtc_state) @@ -
> 1803,6 +1943,37 @@ static bool intel_lut_equal(const struct drm_property_blob
> *blob1,
>  	return true;
>  }
> 
> +static bool i9xx_lut_10_equal(const struct drm_property_blob *blob1,
> +			      const struct drm_property_blob *blob2,
> +			      int bit_precision)
> +{
> +	const struct drm_color_lut *lut1, *lut2;
> +	int i, lut_size;
> +
> +	if (!intel_color_lut_sizes_equal(blob1, blob2))
> +		return false;
> +
> +	if (!blob1)
> +		return true;
> +
> +	if (!bit_precision)
> +		return false;
> +
> +	lut_size = drm_color_lut_size(blob1);
> +	lut1 = blob1->data;
> +	lut2 = blob2->data;
> +
> +	for (i = 0; i < lut_size - 1; i++) {
> +		if (!lut_entry_equal(&lut1[i], &lut2[i],
> +				     0xffff >> bit_precision))
> +			return false;
> +	}
> +
> +	/* limited precision for the last entry due to floating point slope */
> +	return lut_entry_equal(&lut1[i], &lut2[i],
> +			       0xffff >> (bit_precision - 3)); }
> +
>  static bool i9xx_degamma_equal(const struct intel_crtc_state *crtc_state1,
>  			       const struct intel_crtc_state *crtc_state2,
>  			       bool fastset)
> @@ -1821,8 +1992,23 @@ static bool i9xx_gamma_equal(const struct
> intel_crtc_state *crtc_state1,
>  	const struct drm_property_blob *gamma_lut1 = crtc_state1-
> >hw.gamma_lut;
>  	const struct drm_property_blob *gamma_lut2 = crtc_state2-
> >hw.gamma_lut;
> 
> +	if (crtc_state1->gamma_mode == GAMMA_MODE_MODE_10BIT)
> +		return i9xx_lut_10_equal(gamma_lut1, gamma_lut2,
> +					 i9xx_gamma_precision(crtc_state1));
> +	else
> +		return intel_lut_equal(gamma_lut1, gamma_lut2,
> +				       i9xx_gamma_precision(crtc_state1));
> +}
> +
> +static bool i965_gamma_equal(const struct intel_crtc_state *crtc_state1,
> +			     const struct intel_crtc_state *crtc_state2,
> +			     bool fastset)
> +{
> +	const struct drm_property_blob *gamma_lut1 = crtc_state1-
> >hw.gamma_lut;
> +	const struct drm_property_blob *gamma_lut2 =
> +crtc_state2->hw.gamma_lut;
> +
>  	return intel_lut_equal(gamma_lut1, gamma_lut2,
> -			       i9xx_gamma_precision(crtc_state1));
> +			       i965_gamma_precision(crtc_state1));
>  }
> 
>  static bool chv_degamma_equal(const struct intel_crtc_state *crtc_state1, @@ -
> 2084,6 +2270,36 @@ static struct drm_property_blob *i9xx_read_lut_8(struct
> intel_crtc *crtc)
>  	return blob;
>  }
> 
> +static struct drm_property_blob *
> +i9xx_read_lut_10(struct intel_crtc *crtc) {
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> +	enum pipe pipe = crtc->pipe;
> +	struct drm_property_blob *blob;
> +	struct drm_color_lut *lut;
> +	u32 ldw, udw;
> +	int i;
> +
> +	blob = drm_property_create_blob(&dev_priv->drm,
> +					lut_size * sizeof(lut[0]), NULL);
> +	if (IS_ERR(blob))
> +		return NULL;
> +
> +	lut = blob->data;
> +
> +	for (i = 0; i < lut_size - 1; i++) {
> +		ldw = intel_de_read(dev_priv, PALETTE(pipe, 2 * i + 0));
> +		udw = intel_de_read(dev_priv, PALETTE(pipe, 2 * i + 1));
> +
> +		i9xx_lut_10_pack(&lut[i], ldw, udw);
> +	}
> +
> +	i9xx_lut_10_pack_slope(&lut[i], ldw, udw);
> +
> +	return blob;
> +}
> +
>  static void i9xx_read_luts(struct intel_crtc_state *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> @@ -2091,7 +2307,10 @@ static void i9xx_read_luts(struct intel_crtc_state
> *crtc_state)
>  	if (!crtc_state->gamma_enable)
>  		return;
> 
> -	crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc);
> +	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
> +		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc);
> +	else
> +		crtc_state->hw.gamma_lut = i9xx_read_lut_10(crtc);
>  }
> 
>  static struct drm_property_blob *i965_read_lut_10p6(struct intel_crtc *crtc)
> @@ -2546,6 +2765,7 @@ void intel_color_init(struct intel_crtc *crtc)  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	bool has_ctm = INTEL_INFO(dev_priv)->color.degamma_lut_size != 0;
> +	int gamma_lut_size;
> 
>  	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
> 
> @@ -2562,7 +2782,7 @@ void intel_color_init(struct intel_crtc *crtc)
>  			dev_priv->display.color_commit = i9xx_color_commit;
>  			dev_priv->display.load_luts = i965_load_luts;
>  			dev_priv->display.read_luts = i965_read_luts;
> -			dev_priv->display.gamma_equal = i9xx_gamma_equal;
> +			dev_priv->display.gamma_equal = i965_gamma_equal;
>  			dev_priv->display.degamma_equal =
> i9xx_degamma_equal;
>  		} else {
>  			dev_priv->display.color_check = i9xx_color_check; @@ -
> 2618,8 +2838,20 @@ void intel_color_init(struct intel_crtc *crtc)
> 
>  	}
> 
> +	/*
> +	 * "DPALETTE_A: NOTE: The 8-bit (non-10-bit) mode is the
> +	 *  only mode supported by Alviso and Grantsdale."
> +	 *
> +	 * Actually looks like this affects all of gen3.
> +	 * Confirmed on alv,cst,pnv. Mobile gen2 parts (alm,mgm)
> +	 * are confirmed not to suffer from this restriction.
> +	 */
> +	if (IS_GEN(dev_priv, 3) && crtc->pipe == PIPE_A)
> +		gamma_lut_size = 256;
> +	else
> +		gamma_lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> +
>  	drm_crtc_enable_color_mgmt(&crtc->base,
>  				   INTEL_INFO(dev_priv)-
> >color.degamma_lut_size,
> -				   has_ctm,
> -				   INTEL_INFO(dev_priv)->color.gamma_lut_size);
> +				   has_ctm, gamma_lut_size);
>  }
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 2338f92ce490..baa24e4691c0 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -127,9 +127,9 @@
>  		[PIPE_D] = TGL_CURSOR_D_OFFSET, \
>  	}
> 
> -#define I9XX_COLORS \
> +#define I845_COLORS \
>  	.color = { .gamma_lut_size = 256 }
> -#define I965_COLORS \
> +#define I9XX_COLORS \
>  	.color = { .gamma_lut_size = 129, \
>  		   .gamma_lut_tests = DRM_COLOR_LUT_NON_DECREASING, \
>  	}
> @@ -194,7 +194,7 @@
>  	.dma_mask_size = 32, \
>  	I845_PIPE_OFFSETS, \
>  	I845_CURSOR_OFFSETS, \
> -	I9XX_COLORS, \
> +	I845_COLORS, \
>  	GEN_DEFAULT_PAGE_SIZES, \
>  	GEN_DEFAULT_REGIONS
> 
> @@ -323,7 +323,7 @@ static const struct intel_device_info pnv_m_info = {
>  	.dma_mask_size = 36, \
>  	I9XX_PIPE_OFFSETS, \
>  	I9XX_CURSOR_OFFSETS, \
> -	I965_COLORS, \
> +	I9XX_COLORS, \
>  	GEN_DEFAULT_PAGE_SIZES, \
>  	GEN_DEFAULT_REGIONS
> 
> @@ -524,7 +524,7 @@ static const struct intel_device_info vlv_info = {
>  	.display_mmio_offset = VLV_DISPLAY_BASE,
>  	I9XX_PIPE_OFFSETS,
>  	I9XX_CURSOR_OFFSETS,
> -	I965_COLORS,
> +	I9XX_COLORS,
>  	GEN_DEFAULT_PAGE_SIZES,
>  	GEN_DEFAULT_REGIONS,
>  };
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 428ef06b8084..0c34d62f22f4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3708,10 +3708,40 @@ static inline bool i915_mmio_reg_valid(i915_reg_t
> reg)
>  #define _PALETTE_A		0xa000
>  #define _PALETTE_B		0xa800
>  #define _CHV_PALETTE_C		0xc000
> -#define PALETTE_RED_MASK        REG_GENMASK(23, 16)
> -#define PALETTE_GREEN_MASK      REG_GENMASK(15, 8)
> -#define PALETTE_BLUE_MASK       REG_GENMASK(7, 0)
> -#define PALETTE(pipe, i)	_MMIO(DISPLAY_MMIO_BASE(dev_priv) + \
> +/* 8bit palette mode */
> +#define  PALETTE_RED_MASK		REG_GENMASK(23, 16)
> +#define  PALETTE_RED(x)
> 	REG_FIELD_PREP(PALETTE_RED_MASK, (x))
> +#define  PALETTE_GREEN_MASK		REG_GENMASK(15, 8)
> +#define  PALETTE_GREEN(x)		REG_FIELD_PREP(PALETTE_GREEN_MASK,
> (x))
> +#define  PALETTE_BLUE_MASK		REG_GENMASK(7, 0)
> +#define  PALETTE_BLUE(x)		REG_FIELD_PREP(PALETTE_BLUE_MASK,
> (x))
> +/* i9xx 10bit interpolated ldw */
> +#define  PALETTE_10BIT_RED_LDW_MASK	REG_GENMASK(23, 16)
> +#define  PALETTE_10BIT_RED_LDW(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_RED_LDW_MASK, (x))
> +#define  PALETTE_10BIT_GREEN_LDW_MASK	REG_GENMASK(15, 8)
> +#define  PALETTE_10BIT_GREEN_LDW(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_GREEN_LDW_MASK, (x))
> +#define  PALETTE_10BIT_BLUE_LDW_MASK	REG_GENMASK(7, 0)
> +#define  PALETTE_10BIT_BLUE_LDW(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_BLUE_LDW_MASK, (x))
> +/* i9xx 10bit interpolated udw */
> +#define  PALETTE_10BIT_RED_EXP_MASK	REG_GENMASK(23, 22)
> +#define  PALETTE_10BIT_RED_EXP(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_RED_EXP_MASK, (x))
> +#define  PALETTE_10BIT_RED_MANT_MASK	REG_GENMASK(21, 18)
> +#define  PALETTE_10BIT_RED_MANT(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_RED_MANT_MASK, (x))
> +#define  PALETTE_10BIT_RED_UDW_MASK	REG_GENMASK(17, 16)
> +#define  PALETTE_10BIT_RED_UDW(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_RED_UDW_MASK, (X))
> +#define  PALETTE_10BIT_GREEN_EXP_MASK	REG_GENMASK(15, 14)
> +#define  PALETTE_10BIT_GREEN_EXP(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_GREEN_EXP_MASK, (x))
> +#define  PALETTE_10BIT_GREEN_MANT_MASK	REG_GENMASK(13, 10)
> +#define  PALETTE_10BIT_GREEN_MANT(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_GREEN_MANT_MASK, (x))
> +#define  PALETTE_10BIT_GREEN_UDW_MASK	REG_GENMASK(9, 8)
> +#define  PALETTE_10BIT_GREEN_UDW(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_GREEN_UDW_MASK, (x))
> +#define  PALETTE_10BIT_BLUE_EXP_MASK	REG_GENMASK(7, 6)
> +#define  PALETTE_10BIT_BLUE_EXP(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_BLUE_EXP_MASK, (x))
> +#define  PALETTE_10BIT_BLUE_MANT_MASK	REG_GENMASK(5, 2)
> +#define  PALETTE_10BIT_BLUE_MANT(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_BLUE_MANT_MASK, (x))
> +#define  PALETTE_10BIT_BLUE_UDW_MASK	REG_GENMASK(1, 0)
> +#define  PALETTE_10BIT_BLUE_UDW(x)
> 	REG_FIELD_PREP(PALETTE_10BIT_BLUE_UDW_MASK, (x))
> +#define PALETTE(pipe, i)	_MMIO(DISPLAY_MMIO_BASE(dev_priv) +	\
>  				      _PICK((pipe), _PALETTE_A,		\
>  					    _PALETTE_B, _CHV_PALETTE_C) + \
>  				      (i) * 4)
> --
> 2.26.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-09-21 19:40 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20200717211345.26851-1-ville.syrjala@linux.intel.com>
     [not found] ` <20200717211345.26851-2-ville.syrjala@linux.intel.com>
2020-09-17 19:20   ` [Intel-gfx] [PATCH 01/20] drm/i915: Fix state checker hw.active/hw.enable readout Shankar, Uma
     [not found] ` <20200717211345.26851-3-ville.syrjala@linux.intel.com>
2020-09-17 19:24   ` [Intel-gfx] [PATCH 02/20] drm/i915: Move MST master transcoder dump earlier Shankar, Uma
     [not found] ` <20200717211345.26851-4-ville.syrjala@linux.intel.com>
2020-09-17 19:27   ` [Intel-gfx] [PATCH 03/20] drm/i915: Include the LUT sizes in the state dump Shankar, Uma
     [not found] ` <20200717211345.26851-5-ville.syrjala@linux.intel.com>
2020-09-17 19:29   ` [Intel-gfx] [PATCH 04/20] drm/i915: s/glk_read_lut_10/bdw_read_lut_10/ Shankar, Uma
     [not found] ` <20200717211345.26851-6-ville.syrjala@linux.intel.com>
2020-09-17 19:39   ` [Intel-gfx] [PATCH 05/20] drm/i915: Reset glk degamma index after programming/readout Shankar, Uma
     [not found] ` <20200717211345.26851-7-ville.syrjala@linux.intel.com>
2020-09-17 19:42   ` [Intel-gfx] [PATCH 06/20] drm/i915: Shuffle chv_cgm_gamma_pack() around a bit Shankar, Uma
     [not found] ` <20200717211345.26851-8-ville.syrjala@linux.intel.com>
2020-09-17 19:46   ` [Intel-gfx] [PATCH 07/20] drm/i915: Relocate CHV CGM gamma masks Shankar, Uma
     [not found] ` <20200717211345.26851-9-ville.syrjala@linux.intel.com>
2020-09-17 19:58   ` [Intel-gfx] [PATCH 08/20] drm/i915: Add glk+ degamma readout Shankar, Uma
     [not found] ` <20200717211345.26851-10-ville.syrjala@linux.intel.com>
2020-09-17 20:06   ` [Intel-gfx] [PATCH 09/20] drm/i915: Read out CHV CGM degamma Shankar, Uma
     [not found] ` <20200717211345.26851-11-ville.syrjala@linux.intel.com>
2020-09-17 20:15   ` [Intel-gfx] [PATCH 10/20] drm/i915: Add gamma/degamma readout for bdw+ Shankar, Uma
     [not found] ` <20200717211345.26851-12-ville.syrjala@linux.intel.com>
2020-09-17 20:40   ` [Intel-gfx] [PATCH 11/20] drm/i915: Do degamma+gamma readout in bdw+ split gamma mode Shankar, Uma
     [not found] ` <20200717211345.26851-13-ville.syrjala@linux.intel.com>
2020-09-17 20:43   ` [Intel-gfx] [PATCH 12/20] drm/i915: Polish bdw_read_lut_10() a bit Shankar, Uma
     [not found] ` <20200717211345.26851-14-ville.syrjala@linux.intel.com>
2020-09-17 20:46   ` [Intel-gfx] [PATCH 13/20] drm/i915: Add gamma/degamm readout for ivb/hsw Shankar, Uma
     [not found] ` <20200717211345.26851-15-ville.syrjala@linux.intel.com>
2020-09-17 20:52   ` [Intel-gfx] [PATCH 14/20] drm/i915: Replace some gamma_mode ifs with switches Shankar, Uma
     [not found] ` <20200717211345.26851-16-ville.syrjala@linux.intel.com>
2020-09-17 20:56   ` [Intel-gfx] [PATCH 15/20] drm/i915: Make ilk_load_luts() deal with degamma Shankar, Uma
     [not found] ` <20200717211345.26851-17-ville.syrjala@linux.intel.com>
2020-09-17 20:58   ` [Intel-gfx] [PATCH 16/20] drm/i915: Make ilk_read_luts() capable of degamma readout Shankar, Uma
     [not found] ` <20200717211345.26851-18-ville.syrjala@linux.intel.com>
2020-09-17 21:00   ` [Intel-gfx] [PATCH 17/20] drm/i915: Make .read_luts() mandatory Shankar, Uma
     [not found] ` <20200717211345.26851-19-ville.syrjala@linux.intel.com>
2020-09-17 21:03   ` [Intel-gfx] [PATCH 18/20] drm/i915: Extract ilk_crtc_has_gamma() & co Shankar, Uma
     [not found] ` <20200717211345.26851-20-ville.syrjala@linux.intel.com>
2020-09-17 21:52   ` [Intel-gfx] [PATCH 19/20] drm/i915: Complete the gamma/degamma state checking Shankar, Uma
     [not found] ` <20200717211345.26851-21-ville.syrjala@linux.intel.com>
2020-09-21 19:40   ` [Intel-gfx] [PATCH 20/20] drm/i915: Add 10bit gamma mode for gen2/3 Shankar, Uma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox