| Message ID | 1378229848-29113-1-git-send-email-kamal@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 3, 2013 at 7:37 PM, Kamal Mostafa <kamal@canonical.com> wrote: > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=47941 > > Some BIOS configurations of Dell XPS13 are adversely affected by e85843b > ("drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight") so provide a > boot param to inhibit the quirk, or force it on. > > i915.disable_pch_pwm can be set to > -1: auto (default; allows the disabling of pch_pwm by dmi quirk table match) > 0: inhibits the disabling of pch_pwm (overrides dmi quirk table match) > 1: forces the disabling of pch_pwm > > Signed-off-by: Kamal Mostafa <kamal@canonical.com> Nack. Piling quirk over quirk isn't the right approach and I think I should just revert the pch_pwm enable quirk again. -Daniel
On Tue, 2013-09-03 at 19:50 +0200, Daniel Vetter wrote: > On Tue, Sep 3, 2013 at 7:37 PM, Kamal Mostafa <kamal@canonical.com> wrote: > > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=47941 > > > > Some BIOS configurations of Dell XPS13 are adversely affected by e85843b > > ("drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight") so provide a > > boot param to inhibit the quirk, or force it on. > > > > i915.disable_pch_pwm can be set to > > -1: auto (default; allows the disabling of pch_pwm by dmi quirk table match) > > 0: inhibits the disabling of pch_pwm (overrides dmi quirk table match) > > 1: forces the disabling of pch_pwm > > > > Signed-off-by: Kamal Mostafa <kamal@canonical.com> > > Nack. Piling quirk over quirk isn't the right approach I understand your reluctance, but this isn't actually any new quirk functionality, just a way to manually enable/disable the original PCH_PWM_ENABLE quirk. I think this is the least crazy approach, because: Most XPS13 configurations do need the quirk (and maybe some other yet to be identified machines also), but dmi matching cannot discern the one particular XPS13 configuration ("Ivy Bridge booting UEFI mode without Legacy Option ROM") that is adversely affected by it. We could alternately consider trying to detect that specific configuration with code in i915, but that seemed a lot crazier (and less generally useful) than just providing an override switch for rare or yet-to-be-discovered configurations. Hmmm. What if we had a pair of boot params "i915.quirks_set" and "i915.quirks_mask" boot params that could be used to manually set or mask _all_ the bits in dev_priv->quirks? Such params would surely come in handy for cases just like this one, and would be useful for testing future machines easily. (Would you take that if I submitted it?) > and I think I > should just revert the pch_pwm enable quirk again. > -Daniel But reverting the original quirk would break ALL the XPS13 configurations, which nobody is requesting. Please don't revert the quirk. At most, you might want to disable the Ivy Bridge dmi match (but I don't recommend this either): /* Dell XPS13 HD and XPS13 FHD Ivy Bridge */ { 0x0166, 0x1028, 0x058b, quirk_no_pcm_pwm_enable }, -Kamal
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 72e2be7..fee05df 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -154,6 +154,10 @@ module_param_named(prefault_disable, i915_prefault_disable, bool, 0600); MODULE_PARM_DESC(prefault_disable, "Disable page prefaulting for pread/pwrite/reloc (default:false). For developers only."); +int i915_disable_pch_pwm __read_mostly = -1; +module_param_named(disable_pch_pwm, i915_disable_pch_pwm, int, 0600); +MODULE_PARM_DESC(disable_pch_pwm, "disable PCH_PWM (default: -1 (auto))"); + static struct drm_driver driver; extern int intel_agp_enabled; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 769c138..e6f2a34 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1726,6 +1726,7 @@ extern bool i915_fastboot __read_mostly; extern int i915_enable_pc8 __read_mostly; extern int i915_pc8_timeout __read_mostly; extern bool i915_prefault_disable __read_mostly; +extern int i915_disable_pch_pwm __read_mostly; extern int i915_suspend(struct drm_device *dev, pm_message_t state); extern int i915_resume(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 30e5946..86fa722 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9948,9 +9948,8 @@ static void quirk_invert_brightness(struct drm_device *dev) */ static void quirk_no_pcm_pwm_enable(struct drm_device *dev) { - struct drm_i915_private *dev_priv = dev->dev_private; - dev_priv->quirks |= QUIRK_NO_PCH_PWM_ENABLE; - DRM_INFO("applying no-PCH_PWM_ENABLE quirk\n"); + if (i915_disable_pch_pwm < 0) + i915_disable_pch_pwm = 1; } struct intel_quirk { @@ -10048,6 +10047,12 @@ static void intel_init_quirks(struct drm_device *dev) if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) != 0) intel_dmi_quirks[i].hook(dev); } + + if (i915_disable_pch_pwm == 1) { + struct drm_i915_private *dev_priv = dev->dev_private; + dev_priv->quirks |= QUIRK_NO_PCH_PWM_ENABLE; + DRM_INFO("applying no-PCH_PWM_ENABLE quirk\n"); + } } /* Disable the VGA plane that we never use */
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=47941 Some BIOS configurations of Dell XPS13 are adversely affected by e85843b ("drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight") so provide a boot param to inhibit the quirk, or force it on. i915.disable_pch_pwm can be set to -1: auto (default; allows the disabling of pch_pwm by dmi quirk table match) 0: inhibits the disabling of pch_pwm (overrides dmi quirk table match) 1: forces the disabling of pch_pwm Signed-off-by: Kamal Mostafa <kamal@canonical.com> --- drivers/gpu/drm/i915/i915_drv.c | 4 ++++ drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_display.c | 11 ++++++++--- 3 files changed, 13 insertions(+), 3 deletions(-)