From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH 11.5/13] drm/i915: remove QUIRK_NO_PCH_PWM_ENABLE Date: Thu, 14 Nov 2013 12:50:29 +0200 Message-ID: <1384426229.30482.2.camel@intelbox> References: <1384424069-15078-1-git-send-email-jani.nikula@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1820972083==" Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 42C12FB428 for ; Thu, 14 Nov 2013 02:50:32 -0800 (PST) In-Reply-To: <1384424069-15078-1-git-send-email-jani.nikula@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1820972083== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-29RPFFMEtvRr6Gi4z2Uh" --=-29RPFFMEtvRr6Gi4z2Uh Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2013-11-14 at 12:14 +0200, Jani Nikula wrote: > The quirk was added as what I'd say was a stopgap measure in >=20 > commit e85843bec6c2ea7c10ec61238396891cc2b753a9 > Author: Kamal Mostafa > Date: Fri Jul 19 15:02:01 2013 -0700 >=20 > drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight >=20 > without really digging into what was going on. >=20 > Also, as mentioned in the related bug [1], having the quirk regressed > some of the machines it was supposed to fix to begin with, and there > were patches posted to disable the quirk on such machines [2]! >=20 > The fact is, we do need the BLM_PCH_PWM_ENABLE bit set to have > backlight. With the quirk, we've relied on BIOS to have set it, and our > save/restore code to retain it. With the full backlight setup at enable, > we have no place for things that rely on previous state. >=20 > With the per platform hooks, we've also made a change in the PCH > platform enable order: setting the backlight duty cycle between CPU and > PCH PWM enable. Some experimenting and >=20 > commit 770c12312ad617172b1a65b911d3e6564fc5aca8 > Author: Takashi Iwai > Date: Sat Aug 11 08:56:42 2012 +0200 >=20 > drm/i915: Fix blank panel at reopening lid >=20 > indicate that we can't set the backlight before enabling CPU PWM; the > value just won't stick. But AFAICT we should do it before enabling the > PCH PWM. >=20 > Finally, any fallout we should fix properly, preferrably without quirks, > and absolutely without quirks that rely on existing state. With the per > platform hooks have much more flexibility to adjust the sequence as > required by platforms. >=20 > [1] https://bugzilla.kernel.org/show_bug.cgi?id=3D47941 > [2] http://lkml.kernel.org/r/1378229848-29113-1-git-send-email-kamal@cano= nical.com >=20 > Signed-off-by: Jani Nikula Thanks for the explanation: Reviewed-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_drv.h | 1 - > drivers/gpu/drm/i915/intel_display.c | 16 ---------------- > drivers/gpu/drm/i915/intel_panel.c | 4 ---- > 3 files changed, 21 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index c243b8e..e726ab9 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -717,7 +717,6 @@ enum intel_sbi_destination { > #define QUIRK_PIPEA_FORCE (1<<0) > #define QUIRK_LVDS_SSC_DISABLE (1<<1) > #define QUIRK_INVERT_BRIGHTNESS (1<<2) > -#define QUIRK_NO_PCH_PWM_ENABLE (1<<3) > =20 > struct intel_fbdev; > struct intel_fbc_work; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index 25ef080..b9f763c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10456,17 +10456,6 @@ static void quirk_invert_brightness(struct drm_d= evice *dev) > DRM_INFO("applying inverted panel brightness quirk\n"); > } > =20 > -/* > - * Some machines (Dell XPS13) suffer broken backlight controls if > - * BLM_PCH_PWM_ENABLE is set. > - */ > -static void quirk_no_pcm_pwm_enable(struct drm_device *dev) > -{ > - struct drm_i915_private *dev_priv =3D dev->dev_private; > - dev_priv->quirks |=3D QUIRK_NO_PCH_PWM_ENABLE; > - DRM_INFO("applying no-PCH_PWM_ENABLE quirk\n"); > -} > - > struct intel_quirk { > int device; > int subsystem_vendor; > @@ -10526,11 +10515,6 @@ static struct intel_quirk intel_quirks[] =3D { > * seem to use inverted backlight PWM. > */ > { 0x2a42, 0x1025, PCI_ANY_ID, quirk_invert_brightness }, > - > - /* Dell XPS13 HD Sandy Bridge */ > - { 0x0116, 0x1028, 0x052e, quirk_no_pcm_pwm_enable }, > - /* Dell XPS13 HD and XPS13 FHD Ivy Bridge */ > - { 0x0166, 0x1028, 0x058b, quirk_no_pcm_pwm_enable }, > }; > =20 > static void intel_init_quirks(struct drm_device *dev) > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/in= tel_panel.c > index 0986472..da088e3 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -749,10 +749,6 @@ static void pch_enable_backlight(struct intel_connec= tor *connector) > pch_ctl2 =3D panel->backlight.max << 16; > I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2); > =20 > - /* XXX: transitional */ > - if (dev_priv->quirks & QUIRK_NO_PCH_PWM_ENABLE) > - return; > - > pch_ctl1 =3D 0; > if (panel->backlight.active_low_pwm) > pch_ctl1 |=3D BLM_PCH_POLARITY; --=-29RPFFMEtvRr6Gi4z2Uh 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) iQEcBAABAgAGBQJShKr1AAoJEORIIAnNuWDF+9MH/3yq+PqoPymLsR6OZLP1xO1j bp/VyJQ4lP6IBvcuGGTNjGK/qb0Bsc7E1yw4fcYSeECf/iGfUp3p63PmXXN3BeP3 GDQs5/2mTINEguCrT7qUy920LfUrJ6FbwEcc898OpQGzwrPTqXmaumrqxZXMNuCw pJjx7YlGVQbVfm/EezfltgOUkBFoCtqYSL6Ptsye4Ypjym2eP4jp9Q6Ffw29+7No G7l+DaKjf3PriKlBpspBoCjvlxxCgvV3+U84T8ufnyQbNtpaJjNcGFkvU4P5TjL1 uvkCybegPRKH+/lY9sFkyITLk5xSjKg5vsG56cLPXuklTAZJt9w1NygBGsECVNA= =BZpS -----END PGP SIGNATURE----- --=-29RPFFMEtvRr6Gi4z2Uh-- --===============1820972083== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1820972083==--