From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kamal Mostafa Subject: Re: [Intel-gfx] [PATCH 15/19] drm/i915: i915.quirks_set/quirks_mask overrides dmi match Date: Tue, 17 Sep 2013 09:42:08 -0700 Message-ID: <1379436128.3371.198.camel@fourier> References: <1378852608-30281-1-git-send-email-rodrigo.vivi@gmail.com> <1378852608-30281-16-git-send-email-rodrigo.vivi@gmail.com> <87k3inydie.fsf@intel.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg="pgp-sha256"; protocol="application/pgp-signature"; boundary="=-maMTAKFh2qJtA7rGlWer" Return-path: In-Reply-To: <87k3inydie.fsf@intel.com> Sender: stable-owner@vger.kernel.org To: Jani Nikula Cc: Rodrigo Vivi , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org, Daniel Vetter List-Id: intel-gfx@lists.freedesktop.org --=-maMTAKFh2qJtA7rGlWer Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2013-09-11 at 11:21 +0300, Jani Nikula wrote: > On Wed, 11 Sep 2013, Rodrigo Vivi wrote: > > From: Kamal Mostafa > > > > Boot params quirks_set and quirks_mask allow the user to enable or > > inhibit any dmi-matched quirks, overriding the dmi match table. Exampl= es: > > > > i915.quirks_set=3D0x2 - enables QUIRK_LVDS_SSC_DISABLE > > i915.quirks_set=3D0x8 - enables QUIRK_NO_PCH_PWM_ENABLE > > i915.quirks_mask=3D0x8 - disables QUIRK_NO_PCH_PWM_ENABLE > > i915.quirks_mask=3D0xFF - disables all quirks Thanks for reviewing this, Jani. I hope you're open to a little more debate on the topic... > While I admit this can be used to workaround issues with certain > machines, this is a hack that exposes an implementation detail to the > userspace, and carves it into the stone. Any user of it would have to > look at the kernel source to make a smart choice of parameters; more > likely forums would start filling with tips what to set. All of the above is true of all boot params. Why is that a problem? Consider that if this quirks_set/mask feature were available: - Users and developers could trivially check whether any future machine needs to be added to any of the existing dmi-match quirk tables. Currently we have to build a test kernel and get the user to install it, but this would allow us to just ask "Does i915.quirks_set=3D0x2 fix it?". That feature alone makes me want quirks_set/mask. - We would unblock some black-screen-on-boot users who currently have no other solution. And we would unblock future users who face future dmi/quirk-mismatch issues (of which I am sure there will be more). - The existing i915.invert_brightness override param would not have been needed. (It is worth nothing also that my previously proposed i915.disable_pch_pwm override patch was rejected, even though its implementation is identical to invert_brightness.) > So NAK from me. >=20 > That said, I think we still have a few stones unturned wrt backlight and > what BIOS does in UEFI mode. I would be thrilled to see the UEFI/BIOS/backlight issue get fixed, but in my opinion i915.quirks_set/mask would be very useful (a) anyway, and (b) immediately. -Kamal >=20 > Cheers, > Jani. >=20 > > > > Signed-off-by: Kamal Mostafa > > Cc: stable@vger.kernel.org > > Signed-off-by: Rodrigo Vivi > > --- > > drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i91= 5/intel_display.c > > index 1eabca3..12607f2 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -10043,8 +10043,19 @@ static struct intel_quirk intel_quirks[] =3D { > > { 0x0166, 0x1028, 0x058b, quirk_no_pcm_pwm_enable }, > > }; > > =20 > > +unsigned long i915_quirks_set __read_mostly =3D 0; > > +module_param_named(quirks_set, i915_quirks_set, ulong, 0600); > > +MODULE_PARM_DESC(quirks_set, > > + "Enable specified quirks bits."); > > + > > +unsigned long i915_quirks_mask __read_mostly =3D 0; > > +module_param_named(quirks_mask, i915_quirks_mask, ulong, 0600); > > +MODULE_PARM_DESC(quirks_mask, > > + "Disable specified quirks bits (override dmi match)."); > > + > > static void intel_init_quirks(struct drm_device *dev) > > { > > + struct drm_i915_private *dev_priv =3D dev->dev_private; > > struct pci_dev *d =3D dev->pdev; > > int i; > > =20 > > @@ -10062,6 +10073,10 @@ static void intel_init_quirks(struct drm_devic= e *dev) > > if (dmi_check_system(*intel_dmi_quirks[i].dmi_id_list) !=3D 0) > > intel_dmi_quirks[i].hook(dev); > > } > > + > > + /* handle user-specified quirks overrides */ > > + dev_priv->quirks |=3D i915_quirks_set; > > + dev_priv->quirks &=3D ~i915_quirks_mask; > > } > > =20 > > /* Disable the VGA plane that we never use */ > > --=20 > > 1.8.1.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >=20 --=-maMTAKFh2qJtA7rGlWer Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iQIcBAABCAAGBQJSOIZgAAoJEHqwmdxYrXhZ/4sP/2ucEXkY94Tw6oqlymkNkkZl m6jCf8vZIBMYYXwv+KDERl5h4FeN9IcErzi+zscbgJcTvBwpA9S7FgFS9o70DYmo ShuvWnsqKP1HHK/W/tW4wIJHRpw3K+H4Z8V1ztlNRnfTtPLTKlYZImRLq6QbY8GO h+TVc+65S7aHckeAh/0673nRpJOm0N8aiybLRCfeklRVQNb338Lq7HaMsamwiA7R jctOrMwNlMWgXgipXC4O6T07g/WWYRbCqmU5BLRisaBYqYQVjSwcRXxzO6lxsSKx 2qAVc+tOwwBjU5ApAdCph9U7Na94m/W7jiMevBrLkfvF2syrgeMhqnqsDcry4C2o FYvMkBBUDlBx7AHVlNgd0J4gz+yQszWhZJMdP/gDtJc29BC6cAn54N+xGgC9XG4A QkXIBZx/c24SEzUjDcLlyg+AdDlVBTfN5GrPusNOcd18fCWeow4PUFV/JLpan8Uz W7Q0QykNf/SEU74mUw9+QWEPAZoWugdYrSJjgWWwXJO9lKSzSTOGldQCLmH334aq tk2QgkHmittRaDzzUPOfRDfyvHGSdEf6pUh8+BR7sVplpXzE64+Ne7HozW5gC7JX j8P5JMF+GMII84lWcbK5jxjlDPalH18RIDcXJW45iIOWoQlhtNfNd3xXXOOfbE8K dmDxuVZkUYHn5nkYmmjU =gAm8 -----END PGP SIGNATURE----- --=-maMTAKFh2qJtA7rGlWer--