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