Re: [PATCH] drm/i915: i915.disable_pch_pwm overrides PCH_PWM_ENABLEquirk

[Kamal Mostafa]

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

Attachment: signature.asc
Description: This is a digitally signed message part