[PATCH] drm/i915: Fix readout degamma_lut mismatch on ilk/snb

[Shankar, Uma]

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: Wednesday, July 10, 2024 6:12 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: stable at vger.kernel.org
> Subject: [PATCH] drm/i915: Fix readout degamma_lut mismatch on ilk/snb
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> On ilk/snb the pipe may be configured to place the LUT before or after the CSC
> depending on various factors, but as there is only one LUT (no split mode like on
> IVB+) we only advertise a gamma_lut and no degamma_lut in the uapi to avoid
> confusing userspace.
> 
> This can cause a problem during readout if the VBIOS/GOP enabled the LUT in the
> pre CSC configuration. The current code blindly assigns the results of the readout
> to the degamma_lut, which will cause a failure during the next atomic_check() as
> we aren't expecting anything to be in degamma_lut since it's not visible to
> userspace.
> 
> Fix the problem by assigning whatever LUT we read out from the hardware into
> gamma_lut.

Looks Good to me.
Reviewed-by: Uma Shankar <uma.shankar at intel.com>

> Cc: stable at vger.kernel.org
> Fixes: d2559299d339 ("drm/i915: Make ilk_read_luts() capable of degamma
> readout")
> Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/11608
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  .../drm/i915/display/intel_modeset_setup.c    | 31 ++++++++++++++++---
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index 7602cb30ebf1..e1213f3d93cc 100644
> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> @@ -326,6 +326,8 @@ static void
> intel_modeset_update_connector_atomic_state(struct drm_i915_private
> 
>  static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state)
> {
> +	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +
>  	if (intel_crtc_is_joiner_secondary(crtc_state))
>  		return;
> 
> @@ -337,11 +339,30 @@ static void intel_crtc_copy_hw_to_uapi_state(struct
> intel_crtc_state *crtc_state
>  	crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
>  	crtc_state->uapi.scaling_filter = crtc_state->hw.scaling_filter;
> 
> -	/* assume 1:1 mapping */
> -	drm_property_replace_blob(&crtc_state->hw.degamma_lut,
> -				  crtc_state->pre_csc_lut);
> -	drm_property_replace_blob(&crtc_state->hw.gamma_lut,
> -				  crtc_state->post_csc_lut);
> +	if (DISPLAY_INFO(i915)->color.degamma_lut_size) {
> +		/* assume 1:1 mapping */
> +		drm_property_replace_blob(&crtc_state->hw.degamma_lut,
> +					  crtc_state->pre_csc_lut);
> +		drm_property_replace_blob(&crtc_state->hw.gamma_lut,
> +					  crtc_state->post_csc_lut);
> +	} else {
> +		/*
> +		 * ilk/snb hw may be configured for either pre_csc_lut
> +		 * or post_csc_lut, but we don't advertise degamma_lut as
> +		 * being available in the uapi since there is only one
> +		 * hardware LUT. Always assign the result of the readout
> +		 * to gamma_lut as that is the only valid source of LUTs
> +		 * in the uapi.
> +		 */
> +		drm_WARN_ON(&i915->drm, crtc_state->post_csc_lut &&
> +			    crtc_state->pre_csc_lut);
> +
> +		drm_property_replace_blob(&crtc_state->hw.degamma_lut,
> +					  NULL);
> +		drm_property_replace_blob(&crtc_state->hw.gamma_lut,
> +					  crtc_state->post_csc_lut ?:
> +					  crtc_state->pre_csc_lut);
> +	}
> 
>  	drm_property_replace_blob(&crtc_state->uapi.degamma_lut,
>  				  crtc_state->hw.degamma_lut);
> --
> 2.44.2