Re: [Intel-gfx] [PATCH 05/13] drm/i915: Standardize auto-increment LUT load procedure

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

LGTM.

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

On 11/23/2022 8:56 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Various gamma units on various platforms have some problems loading
> the LUT index and auto-increment bit at the same time. We have to
> do this in two steps. The first known case was the glk degamma LUT,
> but at least ADL has another known case.
>
> We're not going to suffer too badly from a couple of extra register
> writes here, so let's just standardize on this practice for all
> auto-increment LUT loads/reads. This way we never have to worry about
> this specific issue again. And for good measure always reset the
> index back to zero at the end (we already did this in a few places).
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_color.c | 19 ++++++++++++++++++-
>   1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index c960c2aaf328..bd7e781d9d07 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -934,6 +934,8 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
>   	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);
> @@ -1138,7 +1140,10 @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
>   	 * 2/(8 * 128 * 256) ... 8/(8 * 128 * 256).
>   	 */
>   	intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
> -			    PAL_PREC_AUTO_INCREMENT);
> +			    PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
> +	intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
> +			    PAL_PREC_AUTO_INCREMENT |
> +			    PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
>   
>   	for (i = 0; i < 9; i++) {
>   		const struct drm_color_lut *entry = &lut[i];
> @@ -1148,6 +1153,9 @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
>   		intel_dsb_indexed_reg_write(crtc_state, PREC_PAL_MULTI_SEG_DATA(pipe),
>   					    ilk_lut_12p4_udw(entry));
>   	}
> +
> +	intel_dsb_reg_write(crtc_state, PREC_PAL_MULTI_SEG_INDEX(pipe),
> +			    PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
>   }
>   
>   static void
> @@ -1170,6 +1178,8 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
>   	 * PAL_PREC_INDEX[0] and PAL_PREC_INDEX[1] map to seg2[1],
>   	 * seg2[0] being unused by the hardware.
>   	 */
> +	intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
> +			    PAL_PREC_INDEX_VALUE(0));
>   	intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
>   			    PAL_PREC_AUTO_INCREMENT |
>   			    PAL_PREC_INDEX_VALUE(0));
> @@ -1202,6 +1212,9 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
>   					    ilk_lut_12p4_udw(entry));
>   	}
>   
> +	intel_dsb_reg_write(crtc_state, PREC_PAL_INDEX(pipe),
> +			    PAL_PREC_INDEX_VALUE(0));
> +
>   	/* The last entry in the LUT is to be programmed in GCMAX */
>   	entry = &lut[256 * 8 * 128];
>   	ivb_load_lut_max(crtc_state, entry);
> @@ -2819,6 +2832,8 @@ static struct drm_property_blob *bdw_read_lut_10(struct intel_crtc *crtc,
>   
>   	lut = blob->data;
>   
> +	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);
> @@ -2947,6 +2962,8 @@ icl_read_lut_multi_segment(struct intel_crtc *crtc)
>   
>   	lut = blob->data;
>   
> +	intel_de_write_fw(i915, PREC_PAL_MULTI_SEG_INDEX(pipe),
> +			  PAL_PREC_MULTI_SEG_INDEX_VALUE(0));
>   	intel_de_write_fw(i915, PREC_PAL_MULTI_SEG_INDEX(pipe),
>   			  PAL_PREC_MULTI_SEG_AUTO_INCREMENT |
>   			  PAL_PREC_MULTI_SEG_INDEX_VALUE(0));