* [PATCH] drm/i915/bxt: fix incorrect BIOS backlight PWM frequency setting @ 2015-12-10 13:52 Imre Deak 2015-12-10 15:00 ` Jani Nikula 0 siblings, 1 reply; 3+ messages in thread From: Imre Deak @ 2015-12-10 13:52 UTC (permalink / raw) To: intel-gfx; +Cc: Jani Nikula There seem to be BIOSes with that set an invalid PWM frequency, assuming the wrong base frequency. This base frequency is 19.2MHz on BXT except for A stepping where it's the current CD clock frequency. The BIOSes in question don't take this difference into account and program the frequency assuming it's based on the CD clock frequency even on B stepping. Since we don't care about A stepping any more, simply assume the 19.2MHz everywhere and require a minimum of 100Hz PWM frequency, regard anything below this as incorrect. In case we correct the frequency we also have to disable fastboot, that being the simplest way to reprogram the backlight settings. Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 5 +++++ drivers/gpu/drm/i915/intel_drv.h | 1 + drivers/gpu/drm/i915/intel_panel.c | 7 +++++++ 3 files changed, 13 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e9bb860..bdd8506 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13271,6 +13271,11 @@ static void calc_watermark_data(struct drm_atomic_state *state) } } +void intel_disable_fastboot(void) +{ + i915.fastboot = 0; +} + /** * intel_atomic_check - validate state object * @dev: drm device diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 76dfa28..e957494 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1057,6 +1057,7 @@ int intel_pch_rawclk(struct drm_device *dev); int intel_hrawclk(struct drm_device *dev); void intel_mark_busy(struct drm_device *dev); void intel_mark_idle(struct drm_device *dev); +void intel_disable_fastboot(void); void intel_crtc_restore_mode(struct drm_crtc *crtc); int intel_display_suspend(struct drm_device *dev); void intel_encoder_destroy(struct drm_encoder *encoder); diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 72183a0..c598b6b 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1647,6 +1647,13 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused) panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY; panel->backlight.max = I915_READ(BXT_BLC_PWM_FREQ(panel->backlight.controller)); + if (panel->backlight.hz_to_pwm && + panel->backlight.max > panel->backlight.hz_to_pwm(connector, 100)) { + DRM_INFO("Correcting innvalid BIOS backlight PWM frequency (%d), disabling fastboot\n", + panel->backlight.max); + intel_disable_fastboot(); + panel->backlight.max = 0; + } if (!panel->backlight.max) panel->backlight.max = get_backlight_max_vbt(connector); -- 2.5.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915/bxt: fix incorrect BIOS backlight PWM frequency setting 2015-12-10 13:52 [PATCH] drm/i915/bxt: fix incorrect BIOS backlight PWM frequency setting Imre Deak @ 2015-12-10 15:00 ` Jani Nikula 2015-12-10 15:08 ` Imre Deak 0 siblings, 1 reply; 3+ messages in thread From: Jani Nikula @ 2015-12-10 15:00 UTC (permalink / raw) To: Imre Deak, intel-gfx; +Cc: Lankhorst, Maarten On Thu, 10 Dec 2015, Imre Deak <imre.deak@intel.com> wrote: > There seem to be BIOSes with that set an invalid PWM frequency, assuming the > wrong base frequency. This base frequency is 19.2MHz on BXT except for A > stepping where it's the current CD clock frequency. The BIOSes in question > don't take this difference into account and program the frequency > assuming it's based on the CD clock frequency even on B stepping. > > Since we don't care about A stepping any more, simply assume the 19.2MHz > everywhere and require a minimum of 100Hz PWM frequency, regard anything > below this as incorrect. In case we correct the frequency we also have to > disable fastboot, that being the simplest way to reprogram the backlight > settings. > > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 5 +++++ > drivers/gpu/drm/i915/intel_drv.h | 1 + > drivers/gpu/drm/i915/intel_panel.c | 7 +++++++ > 3 files changed, 13 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index e9bb860..bdd8506 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13271,6 +13271,11 @@ static void calc_watermark_data(struct drm_atomic_state *state) > } > } > > +void intel_disable_fastboot(void) > +{ > + i915.fastboot = 0; > +} > + > /** > * intel_atomic_check - validate state object > * @dev: drm device > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 76dfa28..e957494 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1057,6 +1057,7 @@ int intel_pch_rawclk(struct drm_device *dev); > int intel_hrawclk(struct drm_device *dev); > void intel_mark_busy(struct drm_device *dev); > void intel_mark_idle(struct drm_device *dev); > +void intel_disable_fastboot(void); > void intel_crtc_restore_mode(struct drm_crtc *crtc); > int intel_display_suspend(struct drm_device *dev); > void intel_encoder_destroy(struct drm_encoder *encoder); > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 72183a0..c598b6b 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -1647,6 +1647,13 @@ bxt_setup_backlight(struct intel_connector *connector, enum pipe unused) > panel->backlight.active_low_pwm = pwm_ctl & BXT_BLC_PWM_POLARITY; > panel->backlight.max = > I915_READ(BXT_BLC_PWM_FREQ(panel->backlight.controller)); > + if (panel->backlight.hz_to_pwm && > + panel->backlight.max > panel->backlight.hz_to_pwm(connector, 100)) { > + DRM_INFO("Correcting innvalid BIOS backlight PWM frequency (%d), disabling fastboot\n", > + panel->backlight.max); > + intel_disable_fastboot(); > + panel->backlight.max = 0; > + } What a mess, again. This should be reported to the BIOS folks, they've probably written this against BXT A0. Maarten wants to kill off i915.fastboot again [1], and I need to fix the Chromebook backlight. This ties in a bit. The basic premise is the same; BIOS gave us a broken state. We should probably try to sanitize what we can, but there are limits to that. If the backlight code observes broken state, we'd need to call back to eDP code (which isn't necessarily ready yet because we're initializing) to switch off backlight in the power sequencer, reinit backlight, switch backlight back on in the power sequencer. This doesn't make sense. There are probably other scenarios where sanitization is too hard. I think your general approach of forcing a modeset and driver load in some cases is a good one. But I'd like to hear Maarten's comments on the implementation of it. BR, Jani. [1] http://mid.gmane.org/5666907C.2040501@linux.intel.com > > if (!panel->backlight.max) > panel->backlight.max = get_backlight_max_vbt(connector); -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915/bxt: fix incorrect BIOS backlight PWM frequency setting 2015-12-10 15:00 ` Jani Nikula @ 2015-12-10 15:08 ` Imre Deak 0 siblings, 0 replies; 3+ messages in thread From: Imre Deak @ 2015-12-10 15:08 UTC (permalink / raw) To: Jani Nikula, intel-gfx; +Cc: Lankhorst, Maarten On to, 2015-12-10 at 17:00 +0200, Jani Nikula wrote: > On Thu, 10 Dec 2015, Imre Deak <imre.deak@intel.com> wrote: > > There seem to be BIOSes with that set an invalid PWM frequency, > > assuming the > > wrong base frequency. This base frequency is 19.2MHz on BXT except > > for A > > stepping where it's the current CD clock frequency. The BIOSes in > > question > > don't take this difference into account and program the frequency > > assuming it's based on the CD clock frequency even on B stepping. > > > > Since we don't care about A stepping any more, simply assume the > > 19.2MHz > > everywhere and require a minimum of 100Hz PWM frequency, regard > > anything > > below this as incorrect. In case we correct the frequency we also > > have to > > disable fastboot, that being the simplest way to reprogram the > > backlight > > settings. > > > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 5 +++++ > > drivers/gpu/drm/i915/intel_drv.h | 1 + > > drivers/gpu/drm/i915/intel_panel.c | 7 +++++++ > > 3 files changed, 13 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index e9bb860..bdd8506 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13271,6 +13271,11 @@ static void calc_watermark_data(struct > > drm_atomic_state *state) > > } > > } > > > > +void intel_disable_fastboot(void) > > +{ > > + i915.fastboot = 0; > > +} > > + > > /** > > * intel_atomic_check - validate state object > > * @dev: drm device > > diff --git a/drivers/gpu/drm/i915/intel_drv.h > > b/drivers/gpu/drm/i915/intel_drv.h > > index 76dfa28..e957494 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -1057,6 +1057,7 @@ int intel_pch_rawclk(struct drm_device *dev); > > int intel_hrawclk(struct drm_device *dev); > > void intel_mark_busy(struct drm_device *dev); > > void intel_mark_idle(struct drm_device *dev); > > +void intel_disable_fastboot(void); > > void intel_crtc_restore_mode(struct drm_crtc *crtc); > > int intel_display_suspend(struct drm_device *dev); > > void intel_encoder_destroy(struct drm_encoder *encoder); > > diff --git a/drivers/gpu/drm/i915/intel_panel.c > > b/drivers/gpu/drm/i915/intel_panel.c > > index 72183a0..c598b6b 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -1647,6 +1647,13 @@ bxt_setup_backlight(struct intel_connector > > *connector, enum pipe unused) > > panel->backlight.active_low_pwm = pwm_ctl & > > BXT_BLC_PWM_POLARITY; > > panel->backlight.max = > > I915_READ(BXT_BLC_PWM_FREQ(panel- > > >backlight.controller)); > > + if (panel->backlight.hz_to_pwm && > > + panel->backlight.max > panel- > > >backlight.hz_to_pwm(connector, 100)) { > > + DRM_INFO("Correcting innvalid BIOS backlight PWM > > frequency (%d), disabling fastboot\n", > > + panel->backlight.max); > > + intel_disable_fastboot(); > > + panel->backlight.max = 0; > > + } > > What a mess, again. This should be reported to the BIOS folks, > they've > probably written this against BXT A0. > > Maarten wants to kill off i915.fastboot again [1], and I need to fix > the > Chromebook backlight. This ties in a bit. > > The basic premise is the same; BIOS gave us a broken state. We should > probably try to sanitize what we can, but there are limits to that. > If > the backlight code observes broken state, we'd need to call back to > eDP > code (which isn't necessarily ready yet because we're initializing) > to > switch off backlight in the power sequencer, reinit backlight, switch > backlight back on in the power sequencer. This doesn't make sense. > There > are probably other scenarios where sanitization is too hard. > > I think your general approach of forcing a modeset and driver load in > some cases is a good one. But I'd like to hear Maarten's comments on > the > implementation of it. Note that this is also a problem without fastboot, since atm we'll just use the BIOS setting if it programmed the frequency. In fact fastboot is disabled now by default and I added the disabling of it only in case someone tries to enable it or it becomes enabled by default in the future. Forgot to mention all this in the commit message. --Imre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-12-10 15:08 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-10 13:52 [PATCH] drm/i915/bxt: fix incorrect BIOS backlight PWM frequency setting Imre Deak 2015-12-10 15:00 ` Jani Nikula 2015-12-10 15:08 ` Imre Deak
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox