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

[Matt Roper]

On Fri, Jan 11, 2019 at 07:08:23PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at 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 at 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?  If so,

Reviewed-by: Matt Roper <matthew.d.roper at 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