| Message ID | 1546933053-10731-3-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for Gen 11 pipe color features | expand |
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@intel.com> > Signed-off-by: Uma Shankar <uma.shankar@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@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 >
>-----Original Message----- >From: Roper, Matthew D >Sent: Friday, January 11, 2019 4:08 AM >To: Shankar, Uma <uma.shankar@intel.com> >Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten ><maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Sharma, >Shashank <shashank.sharma@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.roper@intel.com> >> Signed-off-by: Uma Shankar <uma.shankar@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.roper@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
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. + */ + if (IS_HASWELL(dev_priv)) { + if (crtc_state->ips_enabled) + 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))
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@intel.com> Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)