Re: [PATCH 12/13] drm/i915: Use ilk_lut_write*() for all ilk+ gamma modes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ville Syrjala
> Sent: Wednesday, November 23, 2022 8:57 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject:  [PATCH 12/13] drm/i915: Use ilk_lut_write*() for all ilk+ gamma
> modes
> 
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> 
> We could use the dsb to load the LUT in any gamma mode, not just when using the
> multi-segment mode. So replace the direct mmio on all ilk+ paths with the wrapper.
> 
> There are a few functions (ilk_load_lut_10(), ivb_load_lut_10()) that would never be
> used on a platform with dsb so we could skip those, but probably better to keep all
> this 100% consistent to avoid people getting confused and copy pasting the wrong
> thing when adding a new gamma mode.
> 
> The gmch stuff I left with direct mmio since those are fairly distinct and shouldn't
> cause too much confusion. Although I've also pondered about converting everything
> over to dsb command buffers and just executing it on the CPU when the real hw is
> not available. But dunno if that would actually be a good idea or not...

Current approach looks good to me. Converting all to DSB command buffers would be
ideal for consistency, but otherwise also is fine.

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

> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 106 ++++++++++-----------
>  1 file changed, 50 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 9978d21f1634..d57631b0bb9a 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -858,10 +858,10 @@ static void ilk_lut_write_indexed(const struct
> intel_crtc_state *crtc_state,
>  		intel_de_write_fw(i915, reg, val);
>  }
> 
> -static void ilk_load_lut_8(struct intel_crtc *crtc,
> +static void ilk_load_lut_8(const struct intel_crtc_state *crtc_state,
>  			   const struct drm_property_blob *blob)  {
> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	const struct drm_color_lut *lut;
>  	enum pipe pipe = crtc->pipe;
>  	int i;
> @@ -872,36 +872,35 @@ static void ilk_load_lut_8(struct intel_crtc *crtc,
>  	lut = blob->data;
> 
>  	for (i = 0; i < 256; i++)
> -		intel_de_write_fw(i915, LGC_PALETTE(pipe, i),
> -				  i9xx_lut_8(&lut[i]));
> +		ilk_lut_write(crtc_state, LGC_PALETTE(pipe, i),
> +			      i9xx_lut_8(&lut[i]));
>  }
> 
> -static void ilk_load_lut_10(struct intel_crtc *crtc,
> +static void ilk_load_lut_10(const struct intel_crtc_state *crtc_state,
>  			    const struct drm_property_blob *blob)  {
> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	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; i++)
> -		intel_de_write_fw(i915, PREC_PALETTE(pipe, i),
> -				  ilk_lut_10(&lut[i]));
> +		ilk_lut_write(crtc_state, PREC_PALETTE(pipe, i),
> +			      ilk_lut_10(&lut[i]));
>  }
> 
>  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 *post_csc_lut = crtc_state->post_csc_lut;
>  	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
>  	const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		ilk_load_lut_8(crtc, blob);
> +		ilk_load_lut_8(crtc_state, blob);
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -		ilk_load_lut_10(crtc, blob);
> +		ilk_load_lut_10(crtc_state, blob);
>  		break;
>  	default:
>  		MISSING_CASE(crtc_state->gamma_mode);
> @@ -922,56 +921,56 @@ static int ivb_lut_10_size(u32 prec_index)
>   * "Restriction : Index auto increment mode is not
>   *  supported and must not be enabled."
>   */
> -static void ivb_load_lut_10(struct intel_crtc *crtc,
> +static void ivb_load_lut_10(const struct intel_crtc_state *crtc_state,
>  			    const struct drm_property_blob *blob,
>  			    u32 prec_index)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	const struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	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; i++) {
> -		intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> -				  prec_index + i);
> -		intel_de_write_fw(i915, PREC_PAL_DATA(pipe),
> -				  ilk_lut_10(&lut[i]));
> +		ilk_lut_write(crtc_state, PREC_PAL_INDEX(pipe),
> +			      prec_index + i);
> +		ilk_lut_write(crtc_state, PREC_PAL_DATA(pipe),
> +			      ilk_lut_10(&lut[i]));
>  	}
> 
>  	/*
>  	 * Reset the index, otherwise it prevents the legacy palette to be
>  	 * written properly.
>  	 */
> -	intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> -			  PAL_PREC_INDEX_VALUE(0));
> +	ilk_lut_write(crtc_state, PREC_PAL_INDEX(pipe),
> +		      PAL_PREC_INDEX_VALUE(0));
>  }
> 
>  /* On BDW+ the index auto increment mode actually works */ -static void
> bdw_load_lut_10(struct intel_crtc *crtc,
> +static void bdw_load_lut_10(const struct intel_crtc_state *crtc_state,
>  			    const struct drm_property_blob *blob,
>  			    u32 prec_index)
>  {
> -	struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	const struct drm_color_lut *lut = blob->data;
>  	int i, lut_size = drm_color_lut_size(blob);
>  	enum pipe pipe = crtc->pipe;
> 
> -	intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> -			  prec_index);
> -	intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> -			  PAL_PREC_AUTO_INCREMENT |
> -			  prec_index);
> +	ilk_lut_write(crtc_state, PREC_PAL_INDEX(pipe),
> +		      prec_index);
> +	ilk_lut_write(crtc_state, PREC_PAL_INDEX(pipe),
> +		      PAL_PREC_AUTO_INCREMENT |
> +		      prec_index);
> 
>  	for (i = 0; i < lut_size; i++)
> -		intel_de_write_fw(i915, PREC_PAL_DATA(pipe),
> -				  ilk_lut_10(&lut[i]));
> +		ilk_lut_write_indexed(crtc_state, PREC_PAL_DATA(pipe),
> +				      ilk_lut_10(&lut[i]));
> 
>  	/*
>  	 * Reset the index, otherwise it prevents the legacy palette to be
>  	 * written properly.
>  	 */
> -	intel_de_write_fw(i915, PREC_PAL_INDEX(pipe),
> -			  PAL_PREC_INDEX_VALUE(0));
> +	ilk_lut_write(crtc_state, PREC_PAL_INDEX(pipe),
> +		      PAL_PREC_INDEX_VALUE(0));
>  }
> 
>  static void ivb_load_lut_ext_max(const struct intel_crtc_state *crtc_state) @@ -
> 998,24 +997,23 @@ static void glk_load_lut_ext2_max(const struct intel_crtc_state
> *crtc_state)
> 
>  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 *post_csc_lut = crtc_state->post_csc_lut;
>  	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
>  	const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		ilk_load_lut_8(crtc, blob);
> +		ilk_load_lut_8(crtc_state, blob);
>  		break;
>  	case GAMMA_MODE_MODE_SPLIT:
> -		ivb_load_lut_10(crtc, pre_csc_lut, PAL_PREC_SPLIT_MODE |
> +		ivb_load_lut_10(crtc_state, pre_csc_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
> -		ivb_load_lut_10(crtc, post_csc_lut, PAL_PREC_SPLIT_MODE |
> +		ivb_load_lut_10(crtc_state, post_csc_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(512));
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -		ivb_load_lut_10(crtc, blob,
> +		ivb_load_lut_10(crtc_state, blob,
>  				PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
>  		break;
> @@ -1027,25 +1025,23 @@ static void ivb_load_luts(const struct intel_crtc_state
> *crtc_state)
> 
>  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 *post_csc_lut = crtc_state->post_csc_lut;
>  	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
>  	const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		ilk_load_lut_8(crtc, blob);
> +		ilk_load_lut_8(crtc_state, blob);
>  		break;
>  	case GAMMA_MODE_MODE_SPLIT:
> -		bdw_load_lut_10(crtc, pre_csc_lut, PAL_PREC_SPLIT_MODE |
> +		bdw_load_lut_10(crtc_state, pre_csc_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
> -		bdw_load_lut_10(crtc, post_csc_lut, PAL_PREC_SPLIT_MODE |
> +		bdw_load_lut_10(crtc_state, post_csc_lut, PAL_PREC_SPLIT_MODE
> |
>  				PAL_PREC_INDEX_VALUE(512));
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -
> -		bdw_load_lut_10(crtc, blob,
> +		bdw_load_lut_10(crtc_state, blob,
>  				PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
>  		break;
> @@ -1077,11 +1073,11 @@ static void glk_load_degamma_lut(const struct
> intel_crtc_state *crtc_state,
>  	 * ignore the index bits, so we need to reset it to index 0
>  	 * separately.
>  	 */
> -	intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe),
> -			  PRE_CSC_GAMC_INDEX_VALUE(0));
> -	intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe),
> -			  PRE_CSC_GAMC_AUTO_INCREMENT |
> -			  PRE_CSC_GAMC_INDEX_VALUE(0));
> +	ilk_lut_write(crtc_state, PRE_CSC_GAMC_INDEX(pipe),
> +		      PRE_CSC_GAMC_INDEX_VALUE(0));
> +	ilk_lut_write(crtc_state, PRE_CSC_GAMC_INDEX(pipe),
> +		      PRE_CSC_GAMC_AUTO_INCREMENT |
> +		      PRE_CSC_GAMC_INDEX_VALUE(0));
> 
>  	for (i = 0; i < lut_size; i++) {
>  		/*
> @@ -1097,32 +1093,31 @@ static void glk_load_degamma_lut(const struct
> intel_crtc_state *crtc_state,
>  		 * ToDo: Extend to max 7.0. Enable 32 bit input value
>  		 * as compared to just 16 to achieve this.
>  		 */
> -		intel_de_write_fw(i915, PRE_CSC_GAMC_DATA(pipe),
> -				  lut[i].green);
> +		ilk_lut_write_indexed(crtc_state, PRE_CSC_GAMC_DATA(pipe),
> +				      lut[i].green);
>  	}
> 
>  	/* Clamp values > 1.0. */
>  	while (i++ < glk_degamma_lut_size(i915))
> -		intel_de_write_fw(i915, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
> +		ilk_lut_write_indexed(crtc_state, PRE_CSC_GAMC_DATA(pipe), 1 <<
> 16);
> 
> -	intel_de_write_fw(i915, PRE_CSC_GAMC_INDEX(pipe), 0);
> +	ilk_lut_write(crtc_state, PRE_CSC_GAMC_INDEX(pipe), 0);
>  }
> 
>  static void glk_load_luts(const struct intel_crtc_state *crtc_state)  {
>  	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
>  	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> 
>  	if (pre_csc_lut)
>  		glk_load_degamma_lut(crtc_state, pre_csc_lut);
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		ilk_load_lut_8(crtc, post_csc_lut);
> +		ilk_load_lut_8(crtc_state, post_csc_lut);
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -		bdw_load_lut_10(crtc, post_csc_lut, PAL_PREC_INDEX_VALUE(0));
> +		bdw_load_lut_10(crtc_state, post_csc_lut,
> PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
>  		glk_load_lut_ext2_max(crtc_state);
>  		break;
> @@ -1248,14 +1243,13 @@ static void icl_load_luts(const struct intel_crtc_state
> *crtc_state)  {
>  	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
>  	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> 
>  	if (pre_csc_lut)
>  		glk_load_degamma_lut(crtc_state, pre_csc_lut);
> 
>  	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		ilk_load_lut_8(crtc, post_csc_lut);
> +		ilk_load_lut_8(crtc_state, post_csc_lut);
>  		break;
>  	case GAMMA_MODE_MODE_12BIT_MULTI_SEG:
>  		icl_program_gamma_superfine_segment(crtc_state);
> @@ -1264,7 +1258,7 @@ static void icl_load_luts(const struct intel_crtc_state
> *crtc_state)
>  		glk_load_lut_ext2_max(crtc_state);
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -		bdw_load_lut_10(crtc, post_csc_lut, PAL_PREC_INDEX_VALUE(0));
> +		bdw_load_lut_10(crtc_state, post_csc_lut,
> PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
>  		glk_load_lut_ext2_max(crtc_state);
>  		break;
> --
> 2.37.4





[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux