Re: [Intel-gfx] [PATCH for stable 3.14 only 1/1] drm/i915: restore QUIRK_NO_PCH_PWM_ENABLE

[Kamal Mostafa]

On Mon, 2014-04-28 at 13:10 +0300, Jani Nikula wrote:
> This reverts the bisected regressing
> 
> commit bc0bb9fd1c7810407ab810d204bbaecb255fddde
> Author: Jani Nikula 
> Date:   Thu Nov 14 12:14:29 2013 +0200
> 
> drm/i915: remove QUIRK_NO_PCH_PWM_ENABLE
> 
> restoring QUIRK_NO_PCH_PWM_ENABLE for a couple of Dell XPS models which
> broke in 3.14.


Confirmed: This patch (restore QUIRK_NO_PCH_PWM_ENABLE) on top of 3.14.2
does fix it again for the two affected models.

Tested-by: Kamal Mostafa 

Thanks, Jani!

 -Kamal


> There is no such revert upstream. We have root caused and fixed the
> issue upstream, without the quirk, with:
> 
> commit 39fbc9c8f6765959b55e0b127dd5c57df5a47d67
> Author: Jani Nikula 
> Date:   Wed Apr 9 11:22:06 2014 +0300
> 
> drm/i915: check VBT for supported backlight type
> 
> and
> 
> commit c675949ec58ca50d5a3ae3c757892f1560f6e896
> Author: Jani Nikula 
> Date:   Wed Apr 9 11:31:37 2014 +0300
> 
> drm/i915: do not setup backlight if not available according to VBT
> 
> While the commits are within the stable rules otherwise, and fix more
> machines than just the regressed Dell XPS models, we feel backporting
> them to stable may be too risky. The revert is limited to the broken
> machines, and the impact should be effectively the same as what the
> upstream commits do more generally.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76276
> Reported-by: Romain Francoise 
> CC: Kamal Mostafa 
> CC: Daniel Vetter 
> CC: sta...@vger.kernel.org (3.14 only)
> Signed-off-by: Jani Nikula 
> ---
>  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 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index df77e20e3c3d..697f2150a997 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -734,6 +734,7 @@ 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)
>  
>  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 9b8a7c7ea7fc..963639d9049b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10771,6 +10771,17 @@ static void quirk_invert_brightness(struct 
> drm_device *dev)
>   DRM_INFO("applying inverted panel brightness quirk\n");
>  }
>  
> +/*
> + * 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 = dev->dev_private;
> + dev_priv->quirks |= QUIRK_NO_PCH_PWM_ENABLE;
> + DRM_INFO("applying no-PCH_PWM_ENABLE quirk\n");
> +}
> +
>  struct intel_quirk {
>   int device;
>   int subsystem_vendor;
> @@ -10839,6 +10850,11 @@ static struct intel_quirk intel_quirks[] = {
>  
>   /* Acer Aspire 4736Z */
>   { 0x2a42, 0x1025, 0x0260, 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 },
>  };
>  
>  static void intel_init_quirks(struct drm_device *dev)
> diff --git a/drivers/gpu/drm/i915/intel_panel.c 
> b/drivers/gpu/drm/i915/intel_panel.c
> index 079ea38f14d9..9f1d7a9300e8 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -671,6 +671,10 @@ static void pch_enable_backlight(struct intel_connector 
> *connector)
>   pch_ctl2 = panel->backlight.max << 16;
>   I915_WRITE(BLC_PWM_PCH_CTL2, pch_ctl2);
>  
> + /* XXX: transitional */
> + if (dev_priv->quirks & QUIRK_NO_PCH_PWM_ENABLE)
> + return;
> +
>   pch_ctl1 = 0;
>   if (panel->backlight.active_low_pwm)
>   pch_ctl1 |= BLM_PCH_POLARITY;



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: do not setup backlight if not available according to VBT

[Kamal Mostafa]

On Wed, 2014-04-09 at 13:35 +0300, Jani Nikula wrote:
> Some machines use an external EC for controlling the backlight. Info
> about this is present in the VBT. Do not setup native backlight control
> if no PWM backlight is available or supported according to VBT. The
> acpi_backlight interface appears to work for the EC control.
> 
> In most cases there has been no harm done, but it looks like there are
> machines out there that have both an EC and our PWM line connected to
> the same wire. This, obviously, does not end well.
> 
> This should fix the regression caused by
> commit bc0bb9fd1c7810407ab810d204bbaecb255fddde
> Author: Jani Nikula 
> Date:   Thu Nov 14 12:14:29 2013 +0200
> 
> drm/i915: remove QUIRK_NO_PCH_PWM_ENABLE


Works great!  These patches do indeed re-fix backlight control in v3.14,
on both of the two XPS13 models that had needed the NO_PCH_PWM_ENABLE
quirk.

Tested-by: Kamal Mostafa 

Thanks Jani!

 -Kamal


> AFAICT the quirk removed by the above commit effectively resulted in
> i915 not driving the backlight PWM output, thus not messing things up.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76276
> Reference: https://bugzilla.kernel.org/show_bug.cgi?id=47941
> CC: Aaron Lu 
> CC: Kamal Mostafa 
> CC: Eric Griffith 
> CC: Kent Baxley 
> CC: sta...@vger.kernel.org [v3.14+]
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/i915/intel_panel.c |5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_panel.c 
> b/drivers/gpu/drm/i915/intel_panel.c
> index cb058408c70e..0eead16aeda7 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -1065,6 +1065,11 @@ int intel_panel_setup_backlight(struct drm_connector 
> *connector)
>   unsigned long flags;
>   int ret;
>  
> + if (!dev_priv->vbt.backlight.present) {
> + DRM_DEBUG_KMS("native backlight control not available per 
> VBT\n");
> + return 0;
> + }
> +
>   /* set level and max in panel struct */
>   spin_lock_irqsave(&dev_priv->backlight_lock, flags);
>   ret = dev_priv->display.setup_backlight(intel_connector);



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 15/19] drm/i915: i915.quirks_set/quirks_mask overrides dmi match

[Kamal Mostafa]

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.  Examples:
> >
> >   i915.quirks_set=0x2   - enables QUIRK_LVDS_SSC_DISABLE
> >   i915.quirks_set=0x8   - enables QUIRK_NO_PCH_PWM_ENABLE
> >   i915.quirks_mask=0x8  - disables QUIRK_NO_PCH_PWM_ENABLE
> >   i915.quirks_mask=0xFF - 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=0x2 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.
> 
> 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


> 
> Cheers,
> Jani.
> 
> >
> > Signed-off-by: Kamal Mostafa 
> > Cc: sta...@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/i915/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[] = {
> > { 0x0166, 0x1028, 0x058b, quirk_no_pcm_pwm_enable },
> >  };
> >  
> > +unsigned long i915_quirks_set __read_mostly = 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 = 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 = dev->dev_private;
> > struct pci_dev *d = dev->pdev;
> > int i;
> >  
> > @@ -10062,6 +10073,10 @@ 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);
> > }
> > +
> > +   /* handle user-specified quirks overrides */
> > +   dev_priv->quirks |= i915_quirks_set;
> > +   dev_priv->quirks &= ~i915_quirks_mask;
> >  }
> >  
> >  /* Disable the VGA plane that we never use */
> > -- 
> > 1.8.1.4
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: i915.quirks_set/quirks_mask overrides dmi match

[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.  Examples:

  i915.quirks_set=0x2   - enables QUIRK_LVDS_SSC_DISABLE
  i915.quirks_set=0x8   - enables QUIRK_NO_PCH_PWM_ENABLE
  i915.quirks_mask=0x8  - disables QUIRK_NO_PCH_PWM_ENABLE
  i915.quirks_mask=0xFF - disables all quirks

Signed-off-by: Kamal Mostafa 
Cc: sta...@vger.kernel.org
---
 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/i915/intel_display.c
index d88057e..a6af11c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10028,8 +10028,19 @@ static struct intel_quirk intel_quirks[] = {
{ 0x0166, 0x1028, 0x058b, quirk_no_pcm_pwm_enable },
 };
 
+unsigned long i915_quirks_set __read_mostly = 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 = 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 = dev->dev_private;
struct pci_dev *d = dev->pdev;
int i;
 
@@ -10047,6 +10058,10 @@ 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);
}
+
+   /* handle user-specified quirks overrides */
+   dev_priv->quirks |= i915_quirks_set;
+   dev_priv->quirks &= ~i915_quirks_mask;
 }
 
 /* Disable the VGA plane that we never use */
-- 
1.8.1.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: i915.disable_pch_pwm overrides PCH_PWM_ENABLE quirk

[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  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 
> 
> 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


signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: i915.disable_pch_pwm overrides PCH_PWM_ENABLE quirk

[Kamal Mostafa]

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 
---
 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

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Linux 3.11-rc2 (acpi backlight, revert)

[Kamal Mostafa]

On Thu, 2013-07-25 at 16:46 +0200, Daniel Vetter wrote:
> On Thu, Jul 25, 2013 at 07:43:17AM -0700, Kamal Mostafa wrote:
> > On Thu, 2013-07-25 at 15:00 +0200, Rafael J. Wysocki wrote:
> > > On Monday, July 22, 2013 09:54:21 PM Rafael J. Wysocki wrote:
> > > > On Monday, July 22, 2013 11:11:54 AM Linus Torvalds wrote:
> > > > > On Mon, Jul 22, 2013 at 6:02 AM, Rafael J. Wysocki  
> > > > > wrote:
> > > > > >
> > > > > > Linus, do you want me to send a pull request reverting 8c5bd7a and 
> > > > > > efaa14c?
> > > > > 
> > > > > Yes, but let's wait a while. Not because I think we'll fix the problem
> > > > > (hey, miracles might happen), but because I think it would be useful
> > > > > to couple the reverts with information about the particular machines
> > > > > that broke (and the people who reported it). So that when we
> > > > > inevitably try again, we can perhaps get some testing effort with
> > > > > those machines/people.
> > > > > 
> > > > > It doesn't seem to be a show-stopped for a large number of people, so
> > > > > there's no huge hurry. I'd suggest doing the revert just in time for
> > > > > rc3, but waiting until then to gather info about people who see
> > > > > breakage.
> > > > > 
> > > > > Sound like a plan?
> > > > 
> > > > Yes, it does.
> > > 
> > > OK, time to revert I guess.
> > > 
> > > James, Kamal, Steven, Jörg, Martin, Kalle, please check if the apppended 
> > > patch
> > > fixes the backlight for you.
> > 
> > Yes, this revert patch does re-enable backlight control for the affected
> > Dell XPS13 models.
> 
> Are these the same models that neeed the special quirk to not write
> PCH_PWM_ENABLE? Or do they need both?

Hi Daniel-

Yes, these are the same models (Dell XPS13) that need the PCH_PWM_ENABLE
quirk, but that's not related to this ACPI problem...

All of the XPS13 models still need the PCH_PWM_ENABLE quirk which is now
present in mainline (e85843b "drm/i915: quirk no PCH_PWM_ENABLE for Dell
XPS13 backlight").

Separately from that, some of the XPS13 models were _also_ adversely
affected (as were some other machines) by the ACPI changes that are
about to be reverted.

 -Kamal



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] Linux 3.11-rc2 (acpi backlight, revert)

[Kamal Mostafa]

On Thu, 2013-07-25 at 15:00 +0200, Rafael J. Wysocki wrote:
> On Monday, July 22, 2013 09:54:21 PM Rafael J. Wysocki wrote:
> > On Monday, July 22, 2013 11:11:54 AM Linus Torvalds wrote:
> > > On Mon, Jul 22, 2013 at 6:02 AM, Rafael J. Wysocki  wrote:
> > > >
> > > > Linus, do you want me to send a pull request reverting 8c5bd7a and 
> > > > efaa14c?
> > > 
> > > Yes, but let's wait a while. Not because I think we'll fix the problem
> > > (hey, miracles might happen), but because I think it would be useful
> > > to couple the reverts with information about the particular machines
> > > that broke (and the people who reported it). So that when we
> > > inevitably try again, we can perhaps get some testing effort with
> > > those machines/people.
> > > 
> > > It doesn't seem to be a show-stopped for a large number of people, so
> > > there's no huge hurry. I'd suggest doing the revert just in time for
> > > rc3, but waiting until then to gather info about people who see
> > > breakage.
> > > 
> > > Sound like a plan?
> > 
> > Yes, it does.
> 
> OK, time to revert I guess.
> 
> James, Kamal, Steven, Jörg, Martin, Kalle, please check if the apppended patch
> fixes the backlight for you.

Yes, this revert patch does re-enable backlight control for the affected
Dell XPS13 models.

 -Kamal


> Aaron, please double check if acpi_video_backlight_quirks() will still work as
> needed.
> 
> Thanks,
> Rafael
> 
> 
> ---
> From: Rafael J. Wysocki 
> Subject: Revert "ACPI / video / i915: No ACPI backlight if firmware expects 
> Windows 8"
> 
> We attempted to address a regression introduced by commit a57f7f9
> (ACPICA: Add Windows8/Server2012 string for _OSI method.) after which
> ACPI video backlight support doesn't work on a number of systems,
> because the relevant AML methods in the ACPI tables in their BIOSes
> become useless after the BIOS has been told that the OS is compatible
> with Windows 8.  That problem is tracked by the bug entry at:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=51231
> 
> Commit 8c5bd7a (ACPI / video / i915: No ACPI backlight if firmware
> expects Windows 8) introduced for this purpose essentially prevented
> the ACPI backlight support from being used if the BIOS had been told
> that the OS was compatible with Windows 8 and the i915 driver was
> loaded, in which case the backlight would always be handled by i915.
> Unfortunately, however, that turned out to cause problems with
> backlight to appear on multiple systems with symptoms indicating that
> i915 was unable to control the backlight on those systems as
> expected.
> 
> For this reason, revert commit 8c5bd7a, but leave the function
> acpi_video_backlight_quirks() introduced by it, because another
> commit on top of it uses that function.
> 
> References: https://lkml.org/lkml/2013/7/21/119
> References: https://lkml.org/lkml/2013/7/22/261
> References: https://lkml.org/lkml/2013/7/23/429
> References: https://lkml.org/lkml/2013/7/23/459
> References: https://lkml.org/lkml/2013/7/23/81
> References: https://lkml.org/lkml/2013/7/24/27
> Reported-by: James Hogan 
> Reported-by: Steven Newbury 
> Reported-by: Kamal Mostafa 
> Reported-by: Martin Steigerwald 
> Reported-by: Jörg Otte 
> Reported-by: Kalle Valo 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  drivers/acpi/internal.h |2 -
>  drivers/acpi/video.c|   67 
> 
>  drivers/acpi/video_detect.c |   15 
>  drivers/gpu/drm/i915/i915_dma.c |2 -
>  include/acpi/video.h|   11 --
>  include/linux/acpi.h|1 
>  6 files changed, 11 insertions(+), 87 deletions(-)
> 
> Index: linux-pm/drivers/acpi/video.c
> ===
> --- linux-pm.orig/drivers/acpi/video.c
> +++ linux-pm/drivers/acpi/video.c
> @@ -897,7 +897,7 @@ static void acpi_video_device_find_cap(s
>   if (acpi_video_init_brightness(device))
>   return;
>  
> - if (acpi_video_verify_backlight_support()) {
> + if (acpi_video_backlight_support()) {
>   struct backlight_properties props;
>   struct pci_dev *pdev;
>   acpi_handle acpi_parent;
> @@ -1344,8 +1344,8 @@ acpi_video_switch_brightness(struct acpi
>   unsigned long long level_current, level_next;
>   int result = -EINVAL;
>  
> - /* no warning message if acpi_backlight=vendor or a quirk is used */
> - if (!acpi_video_verify_backlight_support())
> + /* no 

[Intel-gfx] [PATCH] drm/i915: quirk no PCH_PWM_ENABLE for Dell XPS13 backlight

[Kamal Mostafa]

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=47941
BugLink: https://bugs.launchpad.net/bugs/1163720
BugLink: https://bugs.launchpad.net/bugs/1162026

Some machines suffer from non-functional backlight controls if
BLM_PCH_PWM_ENABLE is set, so provide a quirk to avoid doing so.
Apply this quirk to Dell XPS 13 models.

Tested-by: Eric Griffith 
Tested-by: Kent Baxley 
Cc:  # v3.8+
Signed-off-by: Kamal Mostafa 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_display.c | 16 
 drivers/gpu/drm/i915/intel_panel.c   |  3 ++-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a416645..204c3ec 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -555,6 +555,7 @@ 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)
 
 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 85f3eb7..42e207e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9398,6 +9398,17 @@ static void quirk_invert_brightness(struct drm_device 
*dev)
DRM_INFO("applying inverted panel brightness quirk\n");
 }
 
+/*
+ * 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 = dev->dev_private;
+   dev_priv->quirks |= QUIRK_NO_PCH_PWM_ENABLE;
+   DRM_INFO("applying no-PCH_PWM_ENABLE quirk\n");
+}
+
 struct intel_quirk {
int device;
int subsystem_vendor;
@@ -9467,6 +9478,11 @@ static struct intel_quirk intel_quirks[] = {
 
/* Acer Aspire 4736Z */
{ 0x2a42, 0x1025, 0x0260, 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 },
 };
 
 static void intel_init_quirks(struct drm_device *dev)
diff --git a/drivers/gpu/drm/i915/intel_panel.c 
b/drivers/gpu/drm/i915/intel_panel.c
index 80bea1d..01b5a51 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -580,7 +580,8 @@ void intel_panel_enable_backlight(struct drm_device *dev,
POSTING_READ(reg);
I915_WRITE(reg, tmp | BLM_PWM_ENABLE);
 
-   if (HAS_PCH_SPLIT(dev)) {
+   if (HAS_PCH_SPLIT(dev) &&
+   !(dev_priv->quirks & QUIRK_NO_PCH_PWM_ENABLE)) {
tmp = I915_READ(BLC_PWM_PCH_CTL1);
tmp |= BLM_PCH_PWM_ENABLE;
tmp &= ~BLM_PCH_OVERRIDE_ENABLE;
-- 
1.8.1.2

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] i915 PCH backlight vs. Dell XPS13

[Kamal Mostafa]

On Fri, Feb 8, 2013 at 8:36 PM, Kamal Mostafa  wrote:
> > Daniel's backlight fixes which landed in 3.6 did enable working
> > backlight controls on the XPS13, but it appears that this commit from
> > Paolo broke it again:
> > a4f32fc drm/i915: don't forget the PCH backlight registers

On Fri, 2013-02-08 at 20:55 +0100, Daniel Vetter wrote:
> You might want to try the latest drm-intel-nightly git branch from
> http://cgit.freedesktop.org/~danvet/drm-intel This has a few more
> tricks which helped on some similar platforsm to yours (and similar
> resume issues).


Yes indeed, this commit from drm-intel-nightly does fix the problem:
"drm/i915: write backlight harder".

Thanks as always, for your help Daniel!

 -Kamal



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: quirk disable i915 backlight on Dell XPS 13z

[Kamal Mostafa]

Hi Daniel-

Reminding you of this issue...  I've sent the register dump you asked
for... You said you had an idea?

Thanks very much for looking into it,

 -Kamal


On Fri, 2012-04-27 at 14:55 -0700, Kamal Mostafa wrote:
> On Fri, 2012-04-27 at 23:25 +0200, Daniel Vetter wrote:
> > Hm. Can you please install intel-gpu-tools and attach the output of
> > intel_reg_dumper? I have an idea ...
> 
> Here it is with intel_backlight/brightness set to 1 ("strobe light"
> behavior occurring):
> 
> PGETBL_CTL: 0x
>GEN6_INSTDONE_1: 0xfffb
>GEN6_INSTDONE_2: 0x03305e3f
>   CPU_VGACNTRL: 0x8000 (disabled)
> DIGITAL_PORT_HOTPLUG_CNTRL: 0x
>  RR_HW_CTL: 0x (low 0, high 0)
> FDI_PLL_BIOS_0: 0x
> FDI_PLL_BIOS_1: 0x
> FDI_PLL_BIOS_2: 0x
>DISPLAY_PORT_PLL_BIOS_0: 0x
>DISPLAY_PORT_PLL_BIOS_1: 0x
>DISPLAY_PORT_PLL_BIOS_2: 0x
>   FDI_PLL_FREQ_CTL: 0x
>  PIPEACONF: 0xc050 (enabled, active, 6bpc)
>   HTOTAL_A: 0x05d90555 (1366 active, 1498 total)
>   HBLANK_A: 0x05d90555 (1366 start, 1498 end)
>HSYNC_A: 0x05a50585 (1414 start, 1446 end)
>   VTOTAL_A: 0x031502ff (768 active, 790 total)
>   VBLANK_A: 0x031502ff (768 start, 790 end)
>VSYNC_A: 0x03040300 (769 start, 773 end)
>   VSYNCSHIFT_A: 0x
>   PIPEASRC: 0x055502ff (1366, 768)
>  PIPEA_DATA_M1: 0x7e1380e4 (TU 64, val 0x1380e4 1278180)
>  PIPEA_DATA_N1: 0x0020f580 (val 0x20f580 216)
>  PIPEA_DATA_M2: 0x (TU 1, val 0x0 0)
>  PIPEA_DATA_N2: 0x (val 0x0 0)
>  PIPEA_LINK_M1: 0x00011562 (val 0x11562 71010)
>  PIPEA_LINK_N1: 0x00041eb0 (val 0x41eb0 27)
>  PIPEA_LINK_M2: 0x (val 0x0 0)
>  PIPEA_LINK_N2: 0x (val 0x0 0)
>   DSPACNTR: 0xd8004400 (enabled)
>   DSPABASE: 0x
> DSPASTRIDE: 0x1600 (88)
>   DSPASURF: 0x009bf000
>DSPATILEOFF: 0x (0, 0)
>  PIPEBCONF: 0x (disabled, inactive, 8bpc)
>   HTOTAL_B: 0x (1 active, 1 total)
>   HBLANK_B: 0x (1 start, 1 end)
>HSYNC_B: 0x (1 start, 1 end)
>   VTOTAL_B: 0x (1 active, 1 total)
>   VBLANK_B: 0x (1 start, 1 end)
>VSYNC_B: 0x (1 start, 1 end)
>   VSYNCSHIFT_B: 0x
>   DSPBCNTR: 0x4000 (disabled)
>   DSPBBASE: 0x
> DSPBSTRIDE: 0x (0)
>   DSPBSURF: 0x
>DSPBTILEOFF: 0x (0, 0)
>   PIPEBSRC: 0x (1, 1)
>  PIPEB_DATA_M1: 0x (TU 1, val 0x0 0)
>  PIPEB_DATA_N1: 0x (val 0x0 0)
>  PIPEB_DATA_M2: 0x (TU 1, val 0x0 0)
>  PIPEB_DATA_N2: 0x (val 0x0 0)
>  PIPEB_LINK_M1: 0x (val 0x0 0)
>  PIPEB_LINK_N1: 0x (val 0x0 0)
>  PIPEB_LINK_M2: 0x (val 0x0 0)
>  PIPEB_LINK_N2: 0x (val 0x0 0)
>  PFA_CTL_1: 0x (disable, auto_scale yes, 
> auto_scale_cal no, v_filter enable, vadapt disable, mode least, filter_sel 
> programmed,chroma pre-filter disable, vert3tap auto, v_inter_invert field 1)
>  PFA_CTL_2: 0x7e80 (vscale 0.988281)
>  PFA_CTL_3: 0x3f40 (vscale initial phase 0.494141)
>  PFA_CTL_4: 0x7d54 (hscale 0.979126)
>PFA_WIN_POS: 0x (0, 0)
>   PFA_WIN_SIZE: 0x (0, 0)
>  PFB_CTL_1: 0x (disable, auto_scale yes, 
> auto_scale_cal no, v_filter enable, vadapt disable, mode least, filter_sel 
> programmed,chroma pre-filter disable, vert3tap auto, v_inter_invert field 1)
>  PFB_CTL_2: 0x (vscale 0.00)
>  PFB_CTL_3: 0x (vscale initial phase 0.00)
>  PFB_CTL_4: 0x (hscale 0.00)
>PFB_WIN_POS: 0x (0, 0)
>   

Re: [Intel-gfx] [PATCH 1/2] drm/i915: i915.enable_backlight=0 disables intel_backlight

[Kamal Mostafa]

Hi Daniel-

Reminding you of this patch...

The new i915.enable_backlight modparam introduced in patch 1/2 is
generally useful, regardless of whether we quirk-enable as done in patch
2/2.  Its already been used successfully for a different machine[0] with
a different backlight problem.

Please consider applying this patch 1/2.

Thanks,

 -Kamal

[0] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/954661/comments/27


On Thu, 2012-04-26 at 22:07 +0200, Daniel Vetter wrote:
> On Wed, Apr 25, 2012 at 10:28:41AM -0700, Kamal Mostafa wrote:
> > i915.enable_backlight=0 can be used to disable i915 backlight control
> > and the /sys/class/backlight/intel_backlight interface -- useful for
> > systems where intel_backight conflicts with BIOS backlight control.
> > 
> > BugLink: https://launchpad.net/bugs/954661
> > Signed-off-by: Kamal Mostafa 
> 
> Ok, I've just gone through the fun of merging a set of backlight quirks a
> few weeks back. Then noticed that an awful lot of machines seem to be
> affected and later on read about a few interesting bits in the
> documentation. Turns out the hw is all good, it's just the driver totally
> mishandling the backlight.
> 
> To cut things short: This time around I want more justification for the
> quirk than just "this makes this one machine work somehow".
> -Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c|6 ++
> >  drivers/gpu/drm/i915/i915_drv.h|1 +
> >  drivers/gpu/drm/i915/intel_panel.c |   12 
> >  3 files changed, 19 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index ae8a64f..ddb947b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -112,6 +112,12 @@ module_param_named(i915_enable_ppgtt, 
> > i915_enable_ppgtt, int, 0600);
> >  MODULE_PARM_DESC(i915_enable_ppgtt,
> > "Enable PPGTT (default: true)");
> >  
> > +int i915_enable_backlight __read_mostly = -1;
> > +module_param_named(enable_backlight, i915_enable_backlight, int, 0644);
> > +MODULE_PARM_DESC(enable_backlight,
> > +   "Enable backlight control and the intel_backlight interface. "
> > +   "(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 5fabc6c..6e52a42 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1087,6 +1087,7 @@ extern int i915_enable_rc6 __read_mostly;
> >  extern int i915_enable_fbc __read_mostly;
> >  extern bool i915_enable_hangcheck __read_mostly;
> >  extern int i915_enable_ppgtt __read_mostly;
> > +extern int i915_enable_backlight __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_panel.c 
> > b/drivers/gpu/drm/i915/intel_panel.c
> > index 48177ec..fcecbd2 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -259,6 +259,9 @@ void intel_panel_disable_backlight(struct drm_device 
> > *dev)
> >  {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > +   if (!i915_enable_backlight)
> > +   return;
> > +
> > dev_priv->backlight_enabled = false;
> > intel_panel_actually_set_backlight(dev, 0);
> >  }
> > @@ -267,6 +270,9 @@ void intel_panel_enable_backlight(struct drm_device 
> > *dev)
> >  {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > +   if (!i915_enable_backlight)
> > +   return;
> > +
> > if (dev_priv->backlight_level == 0)
> > dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
> >  
> > @@ -333,6 +339,9 @@ int intel_panel_setup_backlight(struct drm_device *dev)
> > struct backlight_properties props;
> > struct drm_connector *connector;
> >  
> > +   if (!i915_enable_backlight)
> > +   return 0;
> > +
> > intel_panel_init_backlight(dev);
> >  
> > if (dev_priv->int_lvds_connector)
> > @@ -368,6 +377,9 @@ void intel_panel_destroy_backlight(struct drm_device 
> > *dev)
> >  #else
> >  int intel_panel_setup_backlight(struct drm_device *dev)
> >  {
> > +   if (!i915_enable_backlight)
> > +   return;
> > +
> > intel_panel_init_backlight(dev);
> > return 0;
> >  }
> > -- 
> > 1.7.5.4
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/2] drm/i915: quirk disable i915 backlight on Dell XPS 13z

[Kamal Mostafa]

On Fri, 2012-04-27 at 23:25 +0200, Daniel Vetter wrote:
> Hm. Can you please install intel-gpu-tools and attach the output of
> intel_reg_dumper? I have an idea ...

Here it is with intel_backlight/brightness set to 1 ("strobe light"
behavior occurring):

PGETBL_CTL: 0x
   GEN6_INSTDONE_1: 0xfffb
   GEN6_INSTDONE_2: 0x03305e3f
  CPU_VGACNTRL: 0x8000 (disabled)
DIGITAL_PORT_HOTPLUG_CNTRL: 0x
 RR_HW_CTL: 0x (low 0, high 0)
FDI_PLL_BIOS_0: 0x
FDI_PLL_BIOS_1: 0x
FDI_PLL_BIOS_2: 0x
   DISPLAY_PORT_PLL_BIOS_0: 0x
   DISPLAY_PORT_PLL_BIOS_1: 0x
   DISPLAY_PORT_PLL_BIOS_2: 0x
  FDI_PLL_FREQ_CTL: 0x
 PIPEACONF: 0xc050 (enabled, active, 6bpc)
  HTOTAL_A: 0x05d90555 (1366 active, 1498 total)
  HBLANK_A: 0x05d90555 (1366 start, 1498 end)
   HSYNC_A: 0x05a50585 (1414 start, 1446 end)
  VTOTAL_A: 0x031502ff (768 active, 790 total)
  VBLANK_A: 0x031502ff (768 start, 790 end)
   VSYNC_A: 0x03040300 (769 start, 773 end)
  VSYNCSHIFT_A: 0x
  PIPEASRC: 0x055502ff (1366, 768)
 PIPEA_DATA_M1: 0x7e1380e4 (TU 64, val 0x1380e4 1278180)
 PIPEA_DATA_N1: 0x0020f580 (val 0x20f580 216)
 PIPEA_DATA_M2: 0x (TU 1, val 0x0 0)
 PIPEA_DATA_N2: 0x (val 0x0 0)
 PIPEA_LINK_M1: 0x00011562 (val 0x11562 71010)
 PIPEA_LINK_N1: 0x00041eb0 (val 0x41eb0 27)
 PIPEA_LINK_M2: 0x (val 0x0 0)
 PIPEA_LINK_N2: 0x (val 0x0 0)
  DSPACNTR: 0xd8004400 (enabled)
  DSPABASE: 0x
DSPASTRIDE: 0x1600 (88)
  DSPASURF: 0x009bf000
   DSPATILEOFF: 0x (0, 0)
 PIPEBCONF: 0x (disabled, inactive, 8bpc)
  HTOTAL_B: 0x (1 active, 1 total)
  HBLANK_B: 0x (1 start, 1 end)
   HSYNC_B: 0x (1 start, 1 end)
  VTOTAL_B: 0x (1 active, 1 total)
  VBLANK_B: 0x (1 start, 1 end)
   VSYNC_B: 0x (1 start, 1 end)
  VSYNCSHIFT_B: 0x
  DSPBCNTR: 0x4000 (disabled)
  DSPBBASE: 0x
DSPBSTRIDE: 0x (0)
  DSPBSURF: 0x
   DSPBTILEOFF: 0x (0, 0)
  PIPEBSRC: 0x (1, 1)
 PIPEB_DATA_M1: 0x (TU 1, val 0x0 0)
 PIPEB_DATA_N1: 0x (val 0x0 0)
 PIPEB_DATA_M2: 0x (TU 1, val 0x0 0)
 PIPEB_DATA_N2: 0x (val 0x0 0)
 PIPEB_LINK_M1: 0x (val 0x0 0)
 PIPEB_LINK_N1: 0x (val 0x0 0)
 PIPEB_LINK_M2: 0x (val 0x0 0)
 PIPEB_LINK_N2: 0x (val 0x0 0)
 PFA_CTL_1: 0x (disable, auto_scale yes, 
auto_scale_cal no, v_filter enable, vadapt disable, mode least, filter_sel 
programmed,chroma pre-filter disable, vert3tap auto, v_inter_invert field 1)
 PFA_CTL_2: 0x7e80 (vscale 0.988281)
 PFA_CTL_3: 0x3f40 (vscale initial phase 0.494141)
 PFA_CTL_4: 0x7d54 (hscale 0.979126)
   PFA_WIN_POS: 0x (0, 0)
  PFA_WIN_SIZE: 0x (0, 0)
 PFB_CTL_1: 0x (disable, auto_scale yes, 
auto_scale_cal no, v_filter enable, vadapt disable, mode least, filter_sel 
programmed,chroma pre-filter disable, vert3tap auto, v_inter_invert field 1)
 PFB_CTL_2: 0x (vscale 0.00)
 PFB_CTL_3: 0x (vscale initial phase 0.00)
 PFB_CTL_4: 0x (hscale 0.00)
   PFB_WIN_POS: 0x (0, 0)
  PFB_WIN_SIZE: 0x (0, 0)
  PCH_DREF_CONTROL: 0x1402 (cpu source disable, ssc_source 
enable, nonspread_source enable, superspread_source disable, ssc4_mode 
downspread, ssc1 enable, ssc4 disable)
   PCH_RAWCLK_FREQ: 0x007d (FDL_TP1 timer 0.5us, FDL_TP2 timer 
1.5us, freq 125)
  PCH_DPLL_TMR_CFG: 0x0271186a
PCH_SSC4_PARMS: 0x01204860
PCH_SSC4_AUX_PARMS: 0x29c5
  PCH_DPLL_SEL: 0x0008 (TransA DPLL enable (DPLL A), TransB 
DPLL disable (DPLL (null)))
   PCH_DPLL_ANALOG_CTL: 0x8000
PCH_DPLL_A: 0x88046004 (

Re: [Intel-gfx] [PATCH 2/2] drm/i915: quirk disable i915 backlight on Dell XPS 13z

[Kamal Mostafa]

On Thu, 2012-04-26 at 22:07 +0200, Daniel Vetter wrote:
> ...
> To cut things short: This time around I want more justification for the
> quirk than just "this makes this one machine work somehow".


A bit more detail...

On this Dell XPS 13z "Ultrabook" (sandybridge_m 0x0116) when
intel_backlight/brightness gets stuffed with any value except 0 or
max_brightness, the backlight cycles between flashing (like a strobe
light!) and then pulsating (bright to dim to bright).  That
flashing/pulsating cycle repeats continuously, about every 2 seconds.
The behavior is affected by the value stuffed into brightness more or
less along the lines of:

   0: very dim, totally stable
   1: flashes like crazy for about 1.5 sec, then pulsates for 0.5 sec
1000: flashes for about 0.5 sec, then pulsates for about 1.5 sec
2000: flashes very briefly, then pulsates for about 2 sec
3000: flickers, then pulsates for about 2 sec
4000  pulsates continuously, every 2 sec
4882: (max_brightness) full brightness, stable

This behavior manifests both in X and in a text VT, and occurs with or
without the presence of other backlight interfaces besides
intel_backlight.  It does not appear to me to be a userspace problem.

The additional wrinkle is that this machine presents an acpi_video0
backlight interface as well, and it even works properly -- but *only*
after you specifically stuff 0 into intel_backlight/brightness (or if
intel_backlight is disabled by the proposed quirk).

Any non-zero intel_backlight/brightness value prevents acpi_video0 from
working.  When intel_backlight is set to max_brightness (like at boot),
acpi_video0/brightness seems to have no effect at all; when
intel_backlight is set to other non-zero values, the flashing/pulsating
behavior occurs.

I'd be (quite) happy to test a proper intel_backlight fix, but in the
meantime disabling it by quirk seems appropriate for this machine, since
that allows the acpi_video0 interface to work out of the box.

Thanks for considering it,

 -Kamal


On Wed, 2012-04-25 at 10:28 -0700, Kamal Mostafa wrote:
> From: Robert Hooker 
> 
> Dell XPS 13z exhibits problems (backlight flashing/pulsating) when
> intel_backlight is enabled at all, so disable it.
> 
> BugLink: https://launchpad.net/bugs/954661
> Signed-off-by: Robert Hooker 
> Signed-off-by: Kamal Mostafa 
> ---
>  drivers/gpu/drm/i915/intel_display.c |   17 +
>  1 files changed, 17 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index 5908cd5..0c4cac4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9100,6 +9100,19 @@ static void quirk_ssc_force_disable(struct drm_device 
> *dev)
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   dev_priv->quirks |= QUIRK_LVDS_SSC_DISABLE;
>  }
> + 
> +/*
> + * Some machines (Dell XPS 13z) exhibit problems with i915 control of the
> + * backlight registers; Others may need the intel_backlight interface
> + * disabled for some other reason.
> + */
> +static void quirk_backlight_disable(struct drm_device *dev)
> +{
> + if (i915_enable_backlight == -1) {
> + i915_enable_backlight = 0;
> + DRM_DEBUG_DRIVER("disabling intel_backlight interface via quirk\n");
> + }
> +}
>  
>  struct intel_quirk {
>   int device;
> @@ -9133,6 +9146,10 @@ struct intel_quirk intel_quirks[] = {
>  
>   /* Sony Vaio Y cannot use SSC on LVDS */
>   { 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
> +
> + /* Dell XPS 13z needs to disable the intel_backlight interface
> +(LP: #954661) */
> + { 0x0116, 0x1028, 0x052e, quirk_backlight_disable },
>  };
>  
>  static void intel_init_quirks(struct drm_device *dev)


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/2] drm/i915: i915.enable_backlight=0 disables intel_backlight

[Kamal Mostafa]

On Thu, 2012-04-26 at 22:07 +0200, Daniel Vetter wrote:
> On Wed, Apr 25, 2012 at 10:28:41AM -0700, Kamal Mostafa wrote:
> > i915.enable_backlight=0 can be used to disable i915 backlight control
> > and the /sys/class/backlight/intel_backlight interface -- useful for
> > systems where intel_backight conflicts with BIOS backlight control.
> > 
> > BugLink: https://launchpad.net/bugs/954661
> > Signed-off-by: Kamal Mostafa 
> 
> Ok, I've just gone through the fun of merging a set of backlight quirks a
> few weeks back. Then noticed that an awful lot of machines seem to be
> affected and later on read about a few interesting bits in the
> documentation. Turns out the hw is all good, it's just the driver totally
> mishandling the backlight.
> 
> To cut things short: This time around I want more justification for the
> quirk than just "this makes this one machine work somehow".


The enable_backlight modparam (this PATCH 1/2) would be generally useful
to have, whether or not we quirk-disable it for that particular machine.
Consider accepting this first patch 1/2 on its own?

 -Kamal


> > ---
> >  drivers/gpu/drm/i915/i915_drv.c|6 ++
> >  drivers/gpu/drm/i915/i915_drv.h|1 +
> >  drivers/gpu/drm/i915/intel_panel.c |   12 
> >  3 files changed, 19 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c 
> > b/drivers/gpu/drm/i915/i915_drv.c
> > index ae8a64f..ddb947b 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -112,6 +112,12 @@ module_param_named(i915_enable_ppgtt, 
> > i915_enable_ppgtt, int, 0600);
> >  MODULE_PARM_DESC(i915_enable_ppgtt,
> > "Enable PPGTT (default: true)");
> >  
> > +int i915_enable_backlight __read_mostly = -1;
> > +module_param_named(enable_backlight, i915_enable_backlight, int, 0644);
> > +MODULE_PARM_DESC(enable_backlight,
> > +   "Enable backlight control and the intel_backlight interface. "
> > +   "(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 5fabc6c..6e52a42 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1087,6 +1087,7 @@ extern int i915_enable_rc6 __read_mostly;
> >  extern int i915_enable_fbc __read_mostly;
> >  extern bool i915_enable_hangcheck __read_mostly;
> >  extern int i915_enable_ppgtt __read_mostly;
> > +extern int i915_enable_backlight __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_panel.c 
> > b/drivers/gpu/drm/i915/intel_panel.c
> > index 48177ec..fcecbd2 100644
> > --- a/drivers/gpu/drm/i915/intel_panel.c
> > +++ b/drivers/gpu/drm/i915/intel_panel.c
> > @@ -259,6 +259,9 @@ void intel_panel_disable_backlight(struct drm_device 
> > *dev)
> >  {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > +   if (!i915_enable_backlight)
> > +   return;
> > +
> > dev_priv->backlight_enabled = false;
> > intel_panel_actually_set_backlight(dev, 0);
> >  }
> > @@ -267,6 +270,9 @@ void intel_panel_enable_backlight(struct drm_device 
> > *dev)
> >  {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > +   if (!i915_enable_backlight)
> > +   return;
> > +
> > if (dev_priv->backlight_level == 0)
> > dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
> >  
> > @@ -333,6 +339,9 @@ int intel_panel_setup_backlight(struct drm_device *dev)
> > struct backlight_properties props;
> > struct drm_connector *connector;
> >  
> > +   if (!i915_enable_backlight)
> > +   return 0;
> > +
> > intel_panel_init_backlight(dev);
> >  
> > if (dev_priv->int_lvds_connector)
> > @@ -368,6 +377,9 @@ void intel_panel_destroy_backlight(struct drm_device 
> > *dev)
> >  #else
> >  int intel_panel_setup_backlight(struct drm_device *dev)
> >  {
> > +   if (!i915_enable_backlight)
> > +   return;
> > +
> > intel_panel_init_backlight(dev);
> > return 0;
> >  }
> > -- 
> > 1.7.5.4
> > 
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 2/2] drm/i915: quirk disable i915 backlight on Dell XPS 13z

[Kamal Mostafa]

From: Robert Hooker 

Dell XPS 13z exhibits problems (backlight flashing/pulsating) when
intel_backlight is enabled at all, so disable it.

BugLink: https://launchpad.net/bugs/954661
Signed-off-by: Robert Hooker 
Signed-off-by: Kamal Mostafa 
---
 drivers/gpu/drm/i915/intel_display.c |   17 +
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 5908cd5..0c4cac4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9100,6 +9100,19 @@ static void quirk_ssc_force_disable(struct drm_device 
*dev)
struct drm_i915_private *dev_priv = dev->dev_private;
dev_priv->quirks |= QUIRK_LVDS_SSC_DISABLE;
 }
+ 
+/*
+ * Some machines (Dell XPS 13z) exhibit problems with i915 control of the
+ * backlight registers; Others may need the intel_backlight interface
+ * disabled for some other reason.
+ */
+static void quirk_backlight_disable(struct drm_device *dev)
+{
+   if (i915_enable_backlight == -1) {
+   i915_enable_backlight = 0;
+   DRM_DEBUG_DRIVER("disabling intel_backlight interface via quirk\n");
+   }
+}
 
 struct intel_quirk {
int device;
@@ -9133,6 +9146,10 @@ struct intel_quirk intel_quirks[] = {
 
/* Sony Vaio Y cannot use SSC on LVDS */
{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
+
+   /* Dell XPS 13z needs to disable the intel_backlight interface
+  (LP: #954661) */
+   { 0x0116, 0x1028, 0x052e, quirk_backlight_disable },
 };
 
 static void intel_init_quirks(struct drm_device *dev)
-- 
1.7.5.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 1/2] drm/i915: i915.enable_backlight=0 disables intel_backlight

[Kamal Mostafa]

i915.enable_backlight=0 can be used to disable i915 backlight control
and the /sys/class/backlight/intel_backlight interface -- useful for
systems where intel_backight conflicts with BIOS backlight control.

BugLink: https://launchpad.net/bugs/954661
Signed-off-by: Kamal Mostafa 
---
 drivers/gpu/drm/i915/i915_drv.c|6 ++
 drivers/gpu/drm/i915/i915_drv.h|1 +
 drivers/gpu/drm/i915/intel_panel.c |   12 
 3 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index ae8a64f..ddb947b 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -112,6 +112,12 @@ module_param_named(i915_enable_ppgtt, i915_enable_ppgtt, 
int, 0600);
 MODULE_PARM_DESC(i915_enable_ppgtt,
"Enable PPGTT (default: true)");
 
+int i915_enable_backlight __read_mostly = -1;
+module_param_named(enable_backlight, i915_enable_backlight, int, 0644);
+MODULE_PARM_DESC(enable_backlight,
+   "Enable backlight control and the intel_backlight interface. "
+   "(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 5fabc6c..6e52a42 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1087,6 +1087,7 @@ extern int i915_enable_rc6 __read_mostly;
 extern int i915_enable_fbc __read_mostly;
 extern bool i915_enable_hangcheck __read_mostly;
 extern int i915_enable_ppgtt __read_mostly;
+extern int i915_enable_backlight __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_panel.c 
b/drivers/gpu/drm/i915/intel_panel.c
index 48177ec..fcecbd2 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -259,6 +259,9 @@ void intel_panel_disable_backlight(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
 
+   if (!i915_enable_backlight)
+   return;
+
dev_priv->backlight_enabled = false;
intel_panel_actually_set_backlight(dev, 0);
 }
@@ -267,6 +270,9 @@ void intel_panel_enable_backlight(struct drm_device *dev)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
 
+   if (!i915_enable_backlight)
+   return;
+
if (dev_priv->backlight_level == 0)
dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
 
@@ -333,6 +339,9 @@ int intel_panel_setup_backlight(struct drm_device *dev)
struct backlight_properties props;
struct drm_connector *connector;
 
+   if (!i915_enable_backlight)
+   return 0;
+
intel_panel_init_backlight(dev);
 
if (dev_priv->int_lvds_connector)
@@ -368,6 +377,9 @@ void intel_panel_destroy_backlight(struct drm_device *dev)
 #else
 int intel_panel_setup_backlight(struct drm_device *dev)
 {
+   if (!i915_enable_backlight)
+   return;
+
intel_panel_init_backlight(dev);
return 0;
 }
-- 
1.7.5.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] i915: Fix bug where screen brightness is not restored

[Kamal Mostafa]

On Tue, 2011-11-15 at 21:28 +0100, Takashi Iwai wrote:
> My rough guess is the inconsistency of property taken during the
> backlight disabled.  How about the patch below (untested!) in addition
> to the fix in 3.2?
> 
> 
> Takashi

Yes Takashi, your patch below (plus the already committed fix[0]) does
indeed fix the problem[1] for me.  Thanks!

Tested-by: Kamal Mostafa 

 -Kamal

[0] f52c619a590fa75276c07dfcaf380dee53e4ea4c
drm/i915/panel: Always record the backlight level again (but cleverly)

[1] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/872652



> ---
> diff --git a/drivers/gpu/drm/i915/intel_panel.c 
> b/drivers/gpu/drm/i915/intel_panel.c
> index 499d4c0..21f60b7 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -326,7 +326,8 @@ static int intel_panel_update_status(struct 
> backlight_device *bd)
>  static int intel_panel_get_brightness(struct backlight_device *bd)
>  {
>   struct drm_device *dev = bl_get_data(bd);
> - return intel_panel_get_backlight(dev);
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + return dev_priv->backlight_level;
>  }
>  
>  static const struct backlight_ops intel_panel_bl_ops = {
> 



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] i915: Fix bug where screen brightness is not restored

[Kamal Mostafa]

On Mon, 2011-11-14 at 18:42 -0800, Alex Davis wrote:
> From: Alex Davis
> 
> This patch fixes an issue where the screen would remain dark when
> 
> a key was pressed when the laptop lid was reopened or after the
> laptop had gone into power-save mode.

[cross-posting to intel-gfx]

Keith, also note that Alex and I did respond to your request to test the
already committed patch "drm/i915/panel: Always record the backlight
level again (but cleverly)".  We both determined that patch does *not*
fix the problem.  Alex's patch below does fix it.

Matthew, any comment on the question that I fielded?:

>> Why does intel_panel_disable_backlight even want to set the
>> brightness to 0 anyway?...  Its only caller is just about to turn
>> off power to the panel.  Is that call to
>> intel_panel_{,actually_}set_backlight(dev, 0) really necessary
>> or useful on *any* systems?

 -Kamal


> It seems that there are a number of people with different machines
> that have this problem:
> 
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/872652
> https://launchpad.net/~kamalmostafa/+archive/stuck-backlight
> and https://bugs.freedesktop.org/show_bug.cgi?id=41926
> 
> This patch is against Linux 3.1
> 
> Putting printk's in ./drivers/gpu/drm/i915/intel_panel.c showed that 
> intel_get_brightness was being called after the panel was disabled,
> which caused a 0 to be saved as the value to restore the brightness. 
> intel_panel_disable_backlight merely sets the brightness to 0. Commenting
> out this call allows the correct brightness value to be saved.
> 
> Signed-off-by: Alex Davis 
> Tested-by: Kamal Mostafa 
> -
> diff --git a/drivers/gpu/drm/i915/intel_panel.c 
> b/drivers/gpu/drm/i915/intel_panel.c
> index a9e0c7b..6f56676 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -262,8 +262,6 @@ void intel_panel_disable_backlight(struct drm_device *dev)
> dev_priv->backlight_level = intel_panel_get_backlight(dev);
> dev_priv->backlight_enabled = false;
> }
> -
> -   intel_panel_set_backlight(dev, 0);
>  }
> 
>  void intel_panel_enable_backlight(struct drm_device *dev)
> 
>  
> 
> I code, therefore I am
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] i915: do not setup intel_backlight twice

[Kamal Mostafa]

The commit "Not all systems expose a firmware or platform mechanism for
changing the backlight intensity on i915, so add native driver support"
adds calls to  intel_panel_setup_backlight() from intel_{lvds,dp}_init
so do not call it again from intel_setup_outputs().

BugLink: http://bugs.launchpad.net/bugs/831542

Signed-off-by: Kamal Mostafa 
---
 drivers/gpu/drm/i915/intel_display.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index ee1d701..5a1ae9f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7238,8 +7238,6 @@ static void intel_setup_outputs(struct drm_device *dev)
intel_encoder_clones(dev, encoder->clone_mask);
}
 
-   intel_panel_setup_backlight(dev);
-
/* disable all the possible outputs/crtcs before entering KMS mode */
drm_helper_disable_unused_functions(dev);
 }
-- 
1.7.4.1

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] i915 native backlight never got merged

[Kamal Mostafa]

On Fri, 2011-08-12 at 06:52 -0700, Keith Packard wrote:
> On Fri, 12 Aug 2011 12:11:33 +0200, Michel Alexandre Salim 
>  wrote:
> 
> > Matthew's last patch from July 16th applies without modification on top
> > of Linux 3.0 and 3.1-rc1 [...]
>> From fa7419eee713b989e2c268c7b06ec9a544a2b647 Mon Sep 17 00:00:00 2001
> 
> I'll merge this version, with the change to make
> intel_panel_init_backlight static (it's not declared in a header, and
> isn't used outside of intel_panel.c).

That version works also works fine for me.

Tested-by: Kamal Mostafa 

 -Kamal



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] i915 native backlight never got merged

[Kamal Mostafa]

On Thu, 2011-08-11 at 15:28 -0700, Keith Packard wrote:
> I've had to amend the patch a bit to get it to apply on top of
> drm-intel-fixes; anyone care to take a look and see if it still looks
> reasonable (and/or actually works?)

Hi Keith-

I confirm that this does indeed work (Dell Studio 1558).  My thanks to
you and Matthew!

Tested-by: Kamal Mostafa 

 -Kamal

> From 2c17b1ae587289501029daa5c0692818b88d21a6 Mon Sep 17 00:00:00 2001
> From: Matthew Garrett 
> Date: Fri, 14 Jan 2011 14:24:22 -0500
> Subject: [PATCH] i915: Add native backlight control
> 
> Not all systems expose a firmware or platform mechanism for changing the
> backlight intensity on i915, so add native driver support.
> 
> Signed-off-by: Matthew Garrett 
> Cc: intel-gfx 
> Tested-by: Michel Alexandre Salim 
> Signed-off-by: Keith Packard 






signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] i915 native backlight never got merged

[Kamal Mostafa]

Several months ago, Matthew Garrett submitted a set of backlight
patches[1], all but one of which landed in 2.6.39-rc1.  The one that
never did get merged is the bit that exposes the "intel_backlight"
interface:

[PATCH 2/5] i915: Add native backlight control
http://lists.freedesktop.org/archives/intel-gfx/2011-January/009207.html

So what happened to that patch?  Did it get lost or is it stuck
somewhere?  I humbly ask that it be re-reviewed and pushed upstream.

 -Kamal

[1] 
[PATCH 1/5] Backlight: Add backlight type
http://lists.freedesktop.org/archives/intel-gfx/2011-January/009208.html
commit ef22f6a70c9186c8e25f757b0e8f7374b37f69bf




signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC PATCH 0/2] i915 brightness control

[Kamal Mostafa]

On Mon, 2010-09-13 at 18:48 +0100, Matthew Garrett wrote:
> I've looked into this issue more closely and think I've worked out the 
> underlying problem. The system in question appears to have two GPUs and 
> exposes two ACPI backlight devices. Both of these are associated with 
> existing PCI devices, so we don't ignore either of them because of that. 
> Further, one of them (the AMD one) implements the spec properly and 
> should work. We don't seem to perform a more fine-grained check to 
> identify whether every ACPI backlight has all the required methods, and 
> so as a result we provide both the working one and the non-working one.
> 
> Having thought about this some more, I don't think this is the right 
> approach. We should be ensuring that every backlight ahs all the 
> required methods and then dropping the one that doesn't. This should be 
> replaced with a native i915 backlight, and I sent patches to do that 
> last week.


I agree.  Your proposed design is good, and I have successfully tested
your proposed patches[1] (after minor porting changes to Ubuntu
Maverick's 2.6.35).  Thanks very much Matthew!

FYI, I have published an experimental Ubuntu Maverick PPA kernel[2]
which includes your patches, plus my dell_laptop tweaks to inhibit the
broken dell_backlight by a module param or dmi blacklist table (in lieu
of a yet to be implemented more fine-grained check).

 -Kamal Mostafa 


[1] 2010-09-08 [Intel-gfx] [PATCH] i915: Add native backlight control
2010-09-08 [Intel-gfx] [PATCH] Backlight: Add backlight type

[2] https://launchpad.net/~kamalmostafa/+archive/linux-kamal-mjgbacklight



signature.asc
Description: This is a digitally signed message part
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [RFC PATCH 2/2] drm/i915: override acpi brightness control

[Kamal Mostafa]

BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/568611

Override acpi brightness control with i915-opregion method.

Signed-off-by: Kamal Mostafa 
---
 drivers/gpu/drm/i915/i915_opregion.c |   19 +++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_opregion.c 
b/drivers/gpu/drm/i915/i915_opregion.c
index 7cc8410..a69033e 100644
--- a/drivers/gpu/drm/i915/i915_opregion.c
+++ b/drivers/gpu/drm/i915/i915_opregion.c
@@ -32,6 +32,9 @@
 #include "i915_drm.h"
 #include "i915_drv.h"
 
+unsigned int i915_brightness = 1;
+module_param_named(brightness, i915_brightness, int, 0400);
+
 #define PCI_ASLE 0xe4
 #define PCI_LBPC 0xf4
 #define PCI_ASLS 0xfc
@@ -422,6 +425,18 @@ static void intel_didl_outputs(struct drm_device *dev)
opregion->acpi->didl[i] = 0;
 }
 
+static unsigned int i915_set_brightness(void *dev, unsigned int brightness)
+{
+/* Nb. brightness value range is 0 to 255. */
+   u32 bclp = ASLE_BCLP_VALID | brightness;
+   struct drm_device *drmdev = dev;
+
+   if (IS_IRONLAKE(drmdev))
+   return asle_set_backlight_ironlake(drmdev, bclp);
+   else
+   return asle_set_backlight(drmdev, bclp);
+}
+
 int intel_opregion_init(struct drm_device *dev, int resume)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
@@ -469,6 +484,10 @@ int intel_opregion_init(struct drm_device *dev, int resume)
DRM_DEBUG_DRIVER("ASLE supported\n");
opregion->asle = base + OPREGION_ASLE_OFFSET;
opregion_enable_asle(dev);
+   /* Register the i915-based brightness control with ACPI */
+   if (!resume && i915_brightness)
+   acpi_brightness_hook_register("i915",
+   i915_set_brightness, dev, 255);
}
 
if (!resume)
-- 
1.7.0.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [RFC PATCH 1/2] acpi/video: acpi_brightness_hook API

[Kamal Mostafa]

BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/568611

New acpi_brightness_hook_register lets another driver (e.g. i915) override
the often-dysfunctional native acpi brightness control methods.

Signed-off-by: Kamal Mostafa 
---
 drivers/acpi/video.c |  149 ++
 include/acpi/video.h |   10 +++
 2 files changed, 147 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index bd9843a..331fdcc 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -481,6 +481,12 @@ acpi_video_device_set_state(struct acpi_video_device 
*device, int state)
return status;
 }
 
+static unsigned int (*acpi_brightness_hook_routine)
+   (void *dev, unsigned int brightness) = NULL;
+static char *acpi_brightness_hook_driver;
+static void *acpi_brightness_hook_dev;
+static unsigned int acpi_brightness_hook_max;
+
 static int
 acpi_video_device_lcd_query_levels(struct acpi_video_device *device,
   union acpi_object **levels)
@@ -520,13 +526,25 @@ acpi_video_device_lcd_set_level(struct acpi_video_device 
*device, int level)
struct acpi_object_list args = { 1, &arg0 };
int state;
 
-   arg0.integer.value = level;
+   /* If another driver has registered a brightness hook override,
+* set the brightness using that method, otherwise set the brightness
+* using the acpi _BCM method. */
+   if ( acpi_brightness_hook_routine ) {
+   status = acpi_brightness_hook_routine(acpi_brightness_hook_dev,
+   level * acpi_brightness_hook_max / 100);
+   if ( status != 0 ) {
+   ACPI_ERROR((AE_INFO, "brightness hook failed"));
+   return -EIO;
+   }
+   } else {
+   arg0.integer.value = level;
 
-   status = acpi_evaluate_object(device->dev->handle, "_BCM",
- &args, NULL);
-   if (ACPI_FAILURE(status)) {
-   ACPI_ERROR((AE_INFO, "Evaluating _BCM failed"));
-   return -EIO;
+   status = acpi_evaluate_object(device->dev->handle, "_BCM",
+ &args, NULL);
+   if (ACPI_FAILURE(status)) {
+   ACPI_ERROR((AE_INFO, "Evaluating _BCM failed"));
+   return -EIO;
+   }
}
 
device->brightness->curr = level;
@@ -607,7 +625,10 @@ acpi_video_device_lcd_get_level_current(struct 
acpi_video_device *device,
acpi_status status = AE_OK;
int i;
 
-   if (device->cap._BQC || device->cap._BCQ) {
+   /* Try to get the current brightness using the _BQC/_BCQ method, only
+* if another driver has not registered a brightness hook override. */
+   if (!acpi_brightness_hook_routine
+   && (device->cap._BQC || device->cap._BCQ)) {
char *buf = device->cap._BQC ? "_BQC" : "_BCQ";
 
status = acpi_evaluate_integer(device->dev->handle, buf,
@@ -784,7 +805,7 @@ acpi_video_cmp_level(const void *a, const void *b)
  * device  : video output device (LCD, CRT, ..)
  *
  *  Return Value:
- * Maximum brightness level
+ * 0 on success, error code on failure.
  *
  *  Allocate and initialize device->brightness.
  */
@@ -944,6 +965,99 @@ out:
  * device  : video output device (LCD, CRT, ..)
  *
  *  Return Value:
+ * 0 on success, error code on failure.
+ *
+ *  Allocate and initialize device->brightness. when a driver has registered
+ *  a brightness hook override via acpi_brightness_hook_register.
+ *
+ *  Cobbles up a fake brightness 'levels' array (emulating a _BCL list) and
+ *  sets max brightness -- effectively what acpi_video_init_brightness does
+ *  for the native acpi brightness methods
+ */
+static int
+acpi_brightness_hook_init(struct acpi_video_device *device)
+{
+   struct acpi_video_device_brightness *br = NULL;
+   int result = -EINVAL;
+   int nsteps = 10;
+   int count, i;
+   static int initialized = 0;
+
+   if ( initialized )
+   return 1;
+   initialized = 1;
+
+   device->brightness = NULL;
+
+   br = kzalloc(sizeof(*br), GFP_KERNEL);
+   if (!br) {
+   printk(KERN_ERR "can't allocate memory\n");
+   result = -ENOMEM;
+   return result;
+   }
+   br->levels = kmalloc((nsteps + 2) * sizeof *(br->levels),
+   GFP_KERNEL);
+   if (!br->levels) {
+   result = -ENOMEM;
+   kfree(br);
+   return result;
+   }
+
+   for (count=2, i = 1; i <= nsteps; i++)
+   br->levels[count++] = (

[Intel-gfx] [RFC PATCH 0/2] i915 brightness control

[Kamal Mostafa]

Requesting comments on this idea (proposed by Matthew Garrett) and my
implementation.  Note especially the KNOWN PROBLEM with a "GM45" GPU.

 -Kamal

...

BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/568611

In order to avoid the often-dysfunctional native acpi brightness control
methods, a new acpi_brightness_hook interface is made available.

The i915 driver uses the new hook to take over brightness control.
Boot with i915.brightness=0 to disable.

KNOWN PROBLEM:
  - i915 opregion brightness control silently fails on GM45 (pci-id 0x2a42)

[RFC PATCH 1/2] acpi/video: acpi_brightness_hook API
[RFC PATCH 2/2] drm/i915: override acpi brightness control

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx