[Intel-gfx] [v5 2/6] drm/i915: Sanitize crtc gamma mode

[Matt Roper]

On Tue, Jan 08, 2019 at 01:07:29PM +0530, Uma Shankar wrote:
> Sanitize crtc gamma mode and update the mode in driver in case
> BIOS has setup a different gamma mode as to what is expected by
> driver. There is restriction on HSW platform not to read/write
> color LUT's if ips is enabled. Handled the same accordingly.
> 
> Credits-to: Matt Roper <matthew.d.roper at intel.com>
> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 696e6f5..03c8f68 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15401,6 +15401,23 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  		}
>  	}
>  
> +	/*
> +	 * Sanitize gamma mode incase BIOS leaves it in SPLIT GAMMA MODE
> +	 * Workaround HSW : Do not read or write the pipe palette/gamma data
> +	 * while GAMMA_MODE is configured for split gamma and IPS_CTL has IPS
> +	 * enabled.
> +	 */

The other thing that might be worth noting here is that we don't
actually try to read out the LUT's and CTM that the BIOS setup, so at
the moment they stick around for a while until the get unexpectedly
clobbered by a subsequent modeset or fastset.   The change here will
basically force them to be reset to standard/linear values at startup.

Maybe in the future we'll try to actually read out and preserve the
contents of the actual LUT's and CTM that the BIOS had setup, but we
don't do that yet today, so the change here at least makes the behavior
a little bit more consistent than what it has been.

Up to you whether you want to try to describe that in either the comment
and/or commit message.

> +	if (IS_HASWELL(dev_priv)) {
> +		if (crtc_state->ips_enabled)

It looks like both hsw_disable_ips() and hsw_enable_ips() have this test
inside of them already, so we can just call them unconditionally here.

Aside from that,

Reviewed-by: Matt Roper <matthew.d.roper at intel.com>

> +			hsw_disable_ips(crtc_state);
> +
> +		intel_color_set_csc(crtc_state);
> +		intel_color_load_luts(crtc_state);
> +
> +		if (crtc_state->ips_enabled)
> +			hsw_enable_ips(crtc_state);
> +	}
> +
>  	/* Adjust the state of the output pipe according to whether we
>  	 * have active connectors/encoders. */
>  	if (crtc_state->base.active && !intel_crtc_has_encoders(crtc))
> -- 
> 1.9.1
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795