Re: [PATCH] drm/i915/color: Use correct variable to load lut

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Oct 09, 2024 at 08:46:20AM +0000, Kandpal, Suraj wrote:
> 
> 
> > -----Original Message-----
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Sent: Wednesday, October 9, 2024 12:25 PM
> > To: Kandpal, Suraj <suraj.kandpal@intel.com>
> > Cc: intel-xe@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Shankar,
> > Uma <uma.shankar@intel.com>; Borah, Chaitanya Kumar
> > <chaitanya.kumar.borah@intel.com>
> > Subject: Re: [PATCH] drm/i915/color: Use correct variable to load lut
> > 
> > On Wed, Oct 09, 2024 at 12:07:53PM +0530, Suraj Kandpal wrote:
> > > Use the blob variable instead of post_csc_lut as it may end up being
> > > null.
> > 
> > Not possible. We always have a LUT for each half in split gamma mode, and
> > if we don't then someone screwed up much earlier before we end up here.
> > 
> 
> If that is the case then the code line
> const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
> wouldn't be needed but it has been added  presumably because there are times

That is for the non-split gamma cases.

> that we reach here with post csc as null and in that case we at least make sure we are
> loading the correct lut hence the fix.

post_csc_lut is the correct one to load here. Trying to load
the pre_csc_lut twice is just complete nonsense. And if
post_csc_lut could somehow be NULL so could pre_csc_lut.

> 
> Regards,
> Suraj Kandpal
> 
> > >
> > > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_color.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > > b/drivers/gpu/drm/i915/display/intel_color.c
> > > index caf1af039960..22b7090c4f6f 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > > @@ -1503,7 +1503,7 @@ static void ivb_load_luts(const struct
> > intel_crtc_state *crtc_state)
> > >  		ivb_load_lut_10(crtc_state, pre_csc_lut,
> > PAL_PREC_SPLIT_MODE |
> > >  				PAL_PREC_INDEX_VALUE(0));
> > >  		ivb_load_lut_ext_max(crtc_state);
> > > -		ivb_load_lut_10(crtc_state, post_csc_lut,
> > PAL_PREC_SPLIT_MODE |
> > > +		ivb_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
> > >  				PAL_PREC_INDEX_VALUE(512));
> > >  		break;
> > >  	case GAMMA_MODE_MODE_10BIT:
> > > @@ -1531,7 +1531,7 @@ static void bdw_load_luts(const struct
> > intel_crtc_state *crtc_state)
> > >  		bdw_load_lut_10(crtc_state, pre_csc_lut,
> > PAL_PREC_SPLIT_MODE |
> > >  				PAL_PREC_INDEX_VALUE(0));
> > >  		ivb_load_lut_ext_max(crtc_state);
> > > -		bdw_load_lut_10(crtc_state, post_csc_lut,
> > PAL_PREC_SPLIT_MODE |
> > > +		bdw_load_lut_10(crtc_state, blob, PAL_PREC_SPLIT_MODE |
> > >  				PAL_PREC_INDEX_VALUE(512));
> > >  		break;
> > >  	case GAMMA_MODE_MODE_10BIT:
> > > --
> > > 2.43.2
> > 
> > --
> > Ville Syrjälä
> > Intel

-- 
Ville Syrjälä
Intel