| Message ID | 20240710124137.16773-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fix readout degamma_lut mismatch on ilk/snb | expand |
> -----Original Message----- > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville > Syrjala > Sent: Wednesday, July 10, 2024 6:12 PM > To: intel-gfx@lists.freedesktop.org > Cc: stable@vger.kernel.org > Subject: [PATCH] drm/i915: Fix readout degamma_lut mismatch on ilk/snb > > From: Ville Syrjälä <ville.syrjala@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@intel.com> > Cc: stable@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@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
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);