>-----Original Message-----
>From: Roper, Matthew D
>Sent: Friday, January 11, 2019 4:08 AM
>To: Shankar, Uma <uma.shan...@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten
><maarten.lankho...@intel.com>; Syrjala, Ville <ville.syrj...@intel.com>;
>Sharma,
>Shashank <shashank.sha...@intel.com>
>Subject: Re: [v5 2/6] drm/i915: Sanitize crtc gamma mode
>
>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.ro...@intel.com>
>> Signed-off-by: Uma Shankar <uma.shan...@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.
Sure Matt, I will update the commit message to reflect this as well.
>> + 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.
>
Yes, this can be dropped. Will do that.
>Aside from that,
>
>Reviewed-by: Matt Roper <matthew.d.ro...@intel.com>
>
Thanks Matt for the review and valuable comments.
Regards,
Uma Shankar
>> + 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
