> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ville
> Syrjala
> Sent: Wednesday, July 10, 2024 6:12 PM
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: stable@xxxxxxxxxxxxxxx
> Subject: [PATCH] drm/i915: Fix readout degamma_lut mismatch on ilk/snb
>
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxx>
> ---
> .../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
