Re: [PATCH] drm/i915/color: fix broken display in icl+

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org, ankit.k.nautiyal@intel.com
Subject: Re: [PATCH] drm/i915/color: fix broken display in icl+
Date: Tue, 1 Oct 2019 17:21:25 +0300	[thread overview]
Message-ID: <20191001142125.GL1208@intel.com> (raw)
In-Reply-To: <87pnjgdhab.fsf@intel.com>

On Tue, Oct 01, 2019 at 11:03:08AM +0300, Jani Nikula wrote:
> On Tue, 01 Oct 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
> > Premature gamma lut prepration and loading which was getting
> > reflected in first modeset causing different colors on
> > screen during boot.
> >
> > Issue: In BIOS, gamma is disabled by default. However,
> > legacy_read_luts() was getting called even before the legacy_load_luts()
> > which was setting crtc_state->base.gamma_lut and gamma_lut was
> > programmed with junk values which led to visual artifacts (different
> > colored screens instead of usual black during boot).
> >
> > Fix: Calling read_luts() only when gamma is enabled which will happen
> > after first modeset.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111809
> 
> I'm confused. Is there a current problem upstream after the revert
> 1b8588741fdc ("Revert "drm/i915/color: Extract icl_read_luts()"")?
> 
> Or does this fix a problem that only occurs in conjunction with the
> reverted commit? Then say so.
> 
> Note inline.
> 
> > Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index f1328c08f4ad..f89aa4bb9f42 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -10528,7 +10528,9 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >  		i9xx_get_pipe_color_config(pipe_config);
> >  	}
> >  
> > -	intel_color_get_config(pipe_config);
> > +	if ((INTEL_GEN(dev_priv) >= 11 && (pipe_config->gamma_mode & POST_CSC_GAMMA_ENABLE)) ||
> > +	   (INTEL_GEN(dev_priv) >= 9 && (pipe_config->gamma_enable)))
> > +			intel_color_get_config(pipe_config);
> 
> Put all of the conditions inside intel_color_get_config().

In fact inside the .read_luts() since these checks are platform
specific.

Also this check is wrong for CHV since it has a separate
enable knob for the CGM LUT (gamma_enable only deals with the
legacy LUT).

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx