2013-09-03 17:37:41

by Kamal Mostafa

[permalink] [raw]
Subject: [PATCH] drm/i915: i915.disable_pch_pwm overrides PCH_PWM_ENABLE quirk

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 <[email protected]>
---
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(-)

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 */
--
1.8.1.2


2013-09-03 17:50:03

by Daniel Vetter

[permalink] [raw]
Subject: Re: [PATCH] drm/i915: i915.disable_pch_pwm overrides PCH_PWM_ENABLE quirk

On Tue, Sep 3, 2013 at 7:37 PM, Kamal Mostafa <[email protected]> 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 <[email protected]>

Nack. Piling quirk over quirk isn't the right approach and I think I
should just revert the pch_pwm enable quirk again.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

2013-09-03 18:52:10

by Kamal Mostafa

[permalink] [raw]
Subject: Re: [PATCH] drm/i915: i915.disable_pch_pwm overrides PCH_PWM_ENABLE quirk

On Tue, 2013-09-03 at 19:50 +0200, Daniel Vetter wrote:
> On Tue, Sep 3, 2013 at 7:37 PM, Kamal Mostafa <[email protected]> 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 <[email protected]>
>
> 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


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part