Re: [PATCH 13/13] drm/i915: Disable pipe gamma when C8 pixel format is used

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

On Thu, Jan 17, 2019 at 11:13:58AM -0800, Matt Roper wrote:
> On Fri, Jan 11, 2019 at 07:08:23PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Planes scanning out C8 will want to use the legacy lut as
> > their palette. That means the LUT content are unikely to
> > be useful for gamma correction on other planes. Thus we
> > should disable pipe gamma for all the other planes. And
> > we should reject any non legacy LUT configurations when
> > C8 planes are present.
> > 
> > Fixes the appearance of the hw cursor when running
> > X -depth 8.
> > 
> > Note that CHV with it's independent CGM degamma/gamma LUTs
> > could probably use the CGM for gamma correction even when
> > the legacy LUT is used for C8. But that would require a
> > new uapi for configuring the legacy LUT and CGM LUTs at
> > the same time. Totally not worth it.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_atomic_plane.c | 5 +++++
> >  drivers/gpu/drm/i915/intel_color.c        | 8 +++++++-
> >  drivers/gpu/drm/i915/intel_drv.h          | 1 +
> >  3 files changed, 13 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > index 50be2c5dd76e..f311763867c4 100644
> > --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
> > @@ -119,6 +119,7 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
> >  
> >  	new_crtc_state->active_planes &= ~BIT(plane->id);
> >  	new_crtc_state->nv12_planes &= ~BIT(plane->id);
> > +	new_crtc_state->c8_planes &= ~BIT(plane->id);
> >  	new_plane_state->base.visible = false;
> >  
> >  	if (!new_plane_state->base.crtc && !old_plane_state->base.crtc)
> > @@ -136,6 +137,10 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
> >  	    new_plane_state->base.fb->format->format == DRM_FORMAT_NV12)
> >  		new_crtc_state->nv12_planes |= BIT(plane->id);
> >  
> > +	if (new_plane_state->base.visible &&
> > +	    new_plane_state->base.fb->format->format == DRM_FORMAT_C8)
> > +		new_crtc_state->c8_planes |= BIT(plane->id);
> > +
> >  	if (new_plane_state->base.visible || old_plane_state->base.visible)
> >  		new_crtc_state->update_planes |= BIT(plane->id);
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > index 789b04bb51d2..c8d12653d77f 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -691,7 +691,13 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
> >  	degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> >  	gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> >  
> > -	crtc_state->gamma_enable = gamma_lut || degamma_lut;
> > +	/* C8 needs the legacy LUT all to itself */
> > +	if (crtc_state->c8_planes &&
> > +	    !crtc_state_is_legacy_gamma(crtc_state))
> > +		return -EINVAL;
> > +
> > +	crtc_state->gamma_enable = (gamma_lut || degamma_lut) &&
> > +		!crtc_state->c8_planes;
> 
> It's not really clear to me from the bspec...the legacy gamma will still
> get used as the c8 palette even when the pipe gamma enable bit is off
> for the plane?

Yes. At least it did on all the platforms I tried, which IIRC
were quite a few.

> If so,
> 
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> 
> >  
> >  	if (INTEL_GEN(dev_priv) >= 9 ||
> >  	    IS_BROADWELL(dev_priv) || IS_HASWELL(dev_priv))
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index a4496f799af3..4d9ea05a6825 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -930,6 +930,7 @@ struct intel_crtc_state {
> >  	/* bitmask of visible planes (enum plane_id) */
> >  	u8 active_planes;
> >  	u8 nv12_planes;
> > +	u8 c8_planes;
> >  
> >  	/* bitmask of planes that will be updated during the commit */
> >  	u8 update_planes;
> > -- 
> > 2.19.2
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

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