LinuxLists.cc - Linux 2.6.39-rc5

[permalink] [raw]

Subject: Re: Linux 2.6.39-rc5

* Linus Torvalds -- Wednesday 27 April 2011:
> Go forth and test,

Doesn't work on my notebook with i915/KMS. But then again, neither did
2.6.38 or any of its stable releases. The last working version was
2.6.38-rc8. The problem has been reported[1], but the bug got closed
in the wrong assumption that the bug is fixed. I reported that, too.
No reactions to that.

The bug was introduced with commit ba3820ade317ee36e496b9b40d2ec3987dd4aef0
by Takashi Iwai. The result is that when using KMS my notebook's[2] screen
remains dark, because the backlight isn't turned on.

Reverting commit ba3820ade317ee36e496b9b40d2ec3987dd4aef0 makes my
notebook work correctly again as it used to in 2.6.38-rc8 and before.

m.

[1] https://bugzilla.kernel.org/show_bug.cgi?id=31522
[2] Acer TravelMate 5735Z-452G32Mnss, using Intel GM45

2011-04-29 17:09:57

[permalink] [raw]

Subject: Re: Linux 2.6.39-rc5

At Fri, 29 Apr 2011 17:58:06 +0200,
Melchior FRANZ wrote:
>
> * Linus Torvalds -- Wednesday 27 April 2011:
> > Go forth and test,
>
> Doesn't work on my notebook with i915/KMS. But then again, neither did
> 2.6.38 or any of its stable releases. The last working version was
> 2.6.38-rc8. The problem has been reported[1], but the bug got closed
> in the wrong assumption that the bug is fixed. I reported that, too.
> No reactions to that.
>
> The bug was introduced with commit ba3820ade317ee36e496b9b40d2ec3987dd4aef0
> by Takashi Iwai. The result is that when using KMS my notebook's[2] screen
> remains dark, because the backlight isn't turned on.
>
> Reverting commit ba3820ade317ee36e496b9b40d2ec3987dd4aef0 makes my
> notebook work correctly again as it used to in 2.6.38-rc8 and before.

Could you check whether the patch below changes the behavior?
If this cures, it means that the backlight-combo mode doesn't work on
your machine.

Takashi

---
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index a06ff07..bf61e02 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -116,11 +116,13 @@ static int is_backlight_combination_mode(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;

+#if 0
if (INTEL_INFO(dev)->gen >= 4)
return I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE;

if (IS_GEN2(dev))
return I915_READ(BLC_PWM_CTL) & BLM_LEGACY_MODE;
+#endif

return 0;
}

2011-04-29 17:41:58

[permalink] [raw]

Subject: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

* Takashi Iwai -- Friday 29 April 2011:
> Melchior FRANZ wrote:
> > The bug was introduced with commit ba3820ade317ee36e496b9b40d2ec3987dd4aef0
> > [...] when using KMS my notebook's[2] screen remains dark, because the
> > backlight isn't turned on.

> Could you check whether the patch below changes the behavior?
> If this cures, it means that the backlight-combo mode doesn't work on
> your machine.

Yes, that works. (Test was with fafc9929c668f8bae6dd1f109f33a86d2cb3c460,
which is current HEAD.)

m.

2011-04-29 19:47:07

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

At Fri, 29 Apr 2011 19:41:53 +0200,
Melchior FRANZ wrote:
>
> * Takashi Iwai -- Friday 29 April 2011:
> > Melchior FRANZ wrote:
> > > The bug was introduced with commit ba3820ade317ee36e496b9b40d2ec3987dd4aef0
> > > [...] when using KMS my notebook's[2] screen remains dark, because the
> > > backlight isn't turned on.
>
> > Could you check whether the patch below changes the behavior?
> > If this cures, it means that the backlight-combo mode doesn't work on
> > your machine.
>
> Yes, that works. (Test was with fafc9929c668f8bae6dd1f109f33a86d2cb3c460,
> which is current HEAD.)

Looking at bugzilla, the problem seems like the case lbpc=0.
What about the patch below instead?

Takashi

---
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index a06ff07..ba60218 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -207,7 +207,8 @@ u32 intel_panel_get_backlight(struct drm_device *dev)

val &= ~1;
pci_read_config_byte(dev->pdev, PCI_LBPC, &lbpc);
- val *= lbpc;
+ if (lbpc)
+ val *= lbpc;
}
}

2011-04-29 20:09:59

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

* Takashi Iwai -- Friday 29 April 2011:
[https://bugzilla.kernel.org/show_bug.cgi?id=31522]
> Looking at bugzilla, the problem seems like the case lbpc=0.
> What about the patch below instead?

> - val *= lbpc;
> + if (lbpc)
> + val *= lbpc;

Yes, that works as well. (I had assumed that this was the problem,
but wasn't sure if a zero was even a legitimate value, or rather
a sign for a problem or wrong assumption elsewhere.)

m.

2011-04-30 08:12:00

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

At Fri, 29 Apr 2011 22:09:54 +0200,
Melchior FRANZ wrote:
>
> * Takashi Iwai -- Friday 29 April 2011:
> [https://bugzilla.kernel.org/show_bug.cgi?id=31522]
> > Looking at bugzilla, the problem seems like the case lbpc=0.
> > What about the patch below instead?
>
> > - val *= lbpc;
> > + if (lbpc)
> > + val *= lbpc;
>
> Yes, that works as well. (I had assumed that this was the problem,
> but wasn't sure if a zero was even a legitimate value, or rather
> a sign for a problem or wrong assumption elsewhere.)

Well, this was just my wild guess. I remember vaguely that the value
zero could be interpreted as the max. It might be depending on the
hardware.

So, the patch below may work better in your case.

Also, with the patch, does the backlight level can be adjusted
correctly to different values? I wonder whether LBPC adjustment
really works or not on your machine.

Takashi

---
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index a06ff07..aaa1f9e 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -207,6 +207,8 @@ u32 intel_panel_get_backlight(struct drm_device *dev)

val &= ~1;
pci_read_config_byte(dev->pdev, PCI_LBPC, &lbpc);
+ if (!lbpc)
+ lbpc = 0xff; /* max value */
val *= lbpc;
}
}

2011-04-30 08:32:13

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

* Takashi Iwai -- Saturday 30 April 2011:
> I remember vaguely that the value zero could be interpreted as the max.

> Also, with the patch, does the backlight level can be adjusted
> correctly to different values? I wonder whether LBPC adjustment
> really works or not on your machine.

> + if (!lbpc)
> + lbpc = 0xff; /* max value */

This change does *not* work on my machine. The screen stays black
again.

Yes, backlight adjustment generally works on this notebook, but only
with "acpi_osi=Linux" on the command line.

m.

2011-04-30 10:27:28

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

At Sat, 30 Apr 2011 10:32:04 +0200,
Melchior FRANZ wrote:
>
> * Takashi Iwai -- Saturday 30 April 2011:
> > I remember vaguely that the value zero could be interpreted as the max.
>
> > Also, with the patch, does the backlight level can be adjusted
> > correctly to different values? I wonder whether LBPC adjustment
> > really works or not on your machine.
>
> > + if (!lbpc)
> > + lbpc = 0xff; /* max value */
>
> This change does *not* work on my machine. The screen stays black
> again.

Hrm, then it's really hard to say how exactly the system behaves when
lbpc=0... I guess we should avoid controlling LBPC in such a case,
e.g. something like the patch below (totally untested).

But Intel guys must know of this better... I leave this to their
hands.

>
> Yes, backlight adjustment generally works on this notebook, but only
> with "acpi_osi=Linux" on the command line.

acpi_osi quirk should be better added statically, then.

thanks,

Takashi

---
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1c1b27c..c0ab771 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -328,6 +328,7 @@ typedef struct drm_i915_private {
struct intel_overlay *overlay;

/* LVDS info */
+ int backlight_combination_mode; /* 0=unknown, -1=no, 1=yes */
int backlight_level; /* restore backlight to this value */
bool backlight_enabled;
struct drm_display_mode *panel_fixed_mode;
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index a06ff07..320dd5f 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -115,14 +115,24 @@ done:
static int is_backlight_combination_mode(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
-
- if (INTEL_INFO(dev)->gen >= 4)
- return I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE;
-
- if (IS_GEN2(dev))
- return I915_READ(BLC_PWM_CTL) & BLM_LEGACY_MODE;
-
- return 0;
+ int combo_mode;
+ u8 lbpc;
+
+ if (dev_priv->backlight_combination_mode)
+ return dev_priv->backlight_combination_mode > 0;
+
+ pci_read_config_byte(dev->pdev, PCI_LBPC, &lbpc);
+ if (!lbpc)
+ combo_mode = 0;
+ else if (INTEL_INFO(dev)->gen >= 4)
+ combo_mode = I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE;
+ else if (IS_GEN2(dev))
+ combo_mode = I915_READ(BLC_PWM_CTL) & BLM_LEGACY_MODE;
+ else
+ combo_mode = 0;
+
+ dev_priv->backlight_combination_mode = combo_mode ? -1 : 1;
+ return combo_mode;
}

static u32 i915_read_blc_pwm_ctl(struct drm_i915_private *dev_priv)

2011-04-30 11:34:59

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

Dropping Linus from the CC.

* Takashi Iwai -- Saturday 30 April 2011:
* * At Sat, 30 Apr 2011 10:32:04 +0200, Melchior FRANZ wrote:
> > Yes, backlight adjustment generally works on this notebook, but only
> > with "acpi_osi=Linux" on the command line.
>
> acpi_osi quirk should be better added statically, then.

No, I guess the problem here is that acer_wmi doesn't support this
machine yet. I told the ACER WMI maintainer and sent a DSDT image,
but this message was thoroughly ignored.

$ dmesg|grep -i acer
[ 0.000000] DMI: Acer TM5735/BA51_MV, BIOS V1.04 09/23/2010
[ 71.850534] acer_wmi: Acer Laptop ACPI-WMI Extras
[ 72.350278] acer_wmi: Unable to detect available WMID devices

Machine: Acer Travelmate 5735Z-452G32Mnss

m.

2011-04-30 14:38:07

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

Hi Melchior,

於六，2011-04-30 於 13:34 +0200，Melchior FRANZ 提到：
> Dropping Linus from the CC.
>
>
> * Takashi Iwai -- Saturday 30 April 2011:
> * * At Sat, 30 Apr 2011 10:32:04 +0200, Melchior FRANZ wrote:
> > > Yes, backlight adjustment generally works on this notebook, but only
> > > with "acpi_osi=Linux" on the command line.
> >
> > acpi_osi quirk should be better added statically, then.
>
> No, I guess the problem here is that acer_wmi doesn't support this
> machine yet. I told the ACER WMI maintainer and sent a DSDT image,
> but this message was thoroughly ignored.
>
>
> $ dmesg|grep -i acer
> [ 0.000000] DMI: Acer TM5735/BA51_MV, BIOS V1.04 09/23/2010
> [ 71.850534] acer_wmi: Acer Laptop ACPI-WMI Extras
> [ 72.350278] acer_wmi: Unable to detect available WMID devices
>
> Machine: Acer Travelmate 5735Z-452G32Mnss
>
> m.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

Could you please attach DSDT on this mail or share your kernel bugzilla
number to us?

I found cann't access BKO now?

Thank's
Joey Lee

2011-05-02 12:09:05

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

At Sat, 30 Apr 2011 13:34:51 +0200,
Melchior FRANZ wrote:
>
> Dropping Linus from the CC.
>
>
> * Takashi Iwai -- Saturday 30 April 2011:
> * * At Sat, 30 Apr 2011 10:32:04 +0200, Melchior FRANZ wrote:
> > > Yes, backlight adjustment generally works on this notebook, but only
> > > with "acpi_osi=Linux" on the command line.
> >
> > acpi_osi quirk should be better added statically, then.
>
> No, I guess the problem here is that acer_wmi doesn't support this
> machine yet. I told the ACER WMI maintainer and sent a DSDT image,
> but this message was thoroughly ignored.
>
>
> $ dmesg|grep -i acer
> [ 0.000000] DMI: Acer TM5735/BA51_MV, BIOS V1.04 09/23/2010
> [ 71.850534] acer_wmi: Acer Laptop ACPI-WMI Extras
> [ 72.350278] acer_wmi: Unable to detect available WMID devices
>
> Machine: Acer Travelmate 5735Z-452G32Mnss

Hm, but the backlight control is done via the standard ACPI, no?
If so, the fact that acer_wmi doesn't work sounds irrelevant.

BTW, did you try my previous patch?

Takashi

2011-05-02 13:12:46

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

* Takashi Iwai -- Monday 02 May 2011:
* > At Sat, 30 Apr 2011 13:34:51 +0200, Melchior FRANZ wrote:
> > * Takashi Iwai -- Saturday 30 April 2011:
> > > acpi_osi quirk should be better added statically, then.
> >
> > No, I guess the problem here is that acer_wmi doesn't support this
> > machine yet.

> Hm, but the backlight control is done via the standard ACPI, no?

I have no idea. If I were an expert, I would just have sent a
patch, not a bug report. And I prefer fixes to "quirks". :-P

> BTW, did you try my previous patch?

Works. I hadn't tested it before, because you seemed to wait
for the opinion of some Intel people, and so did I. But thanks
for that code!

m.

2011-05-04 07:07:10

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

Hi Melchior,

於一，2011-05-02 於 14:08 +0200，Takashi Iwai 提到：
> At Sat, 30 Apr 2011 13:34:51 +0200,
> Melchior FRANZ wrote:
> >
> > Dropping Linus from the CC.
> >
> >
> > * Takashi Iwai -- Saturday 30 April 2011:
> > * * At Sat, 30 Apr 2011 10:32:04 +0200, Melchior FRANZ wrote:
> > > > Yes, backlight adjustment generally works on this notebook, but only
> > > > with "acpi_osi=Linux" on the command line.
> > >
> > > acpi_osi quirk should be better added statically, then.
> >
> > No, I guess the problem here is that acer_wmi doesn't support this
> > machine yet. I told the ACER WMI maintainer and sent a DSDT image,
> > but this message was thoroughly ignored.
> >
> >
> > $ dmesg|grep -i acer
> > [ 0.000000] DMI: Acer TM5735/BA51_MV, BIOS V1.04 09/23/2010
> > [ 71.850534] acer_wmi: Acer Laptop ACPI-WMI Extras
> > [ 72.350278] acer_wmi: Unable to detect available WMID devices
> >
> > Machine: Acer Travelmate 5735Z-452G32Mnss
>
> Hm, but the backlight control is done via the standard ACPI, no?
> If so, the fact that acer_wmi doesn't work sounds irrelevant.
>
> BTW, did you try my previous patch?
>
>
> Takashi

Yes, please try Takashi's previous patch.

I am tracing bko#34142, but this issue does not related to your
backlight control, because there have _BCM, _BQC declare in your DSDT,
your machine MUST SUPPORTED by standard acpi backlight interface.

Even the acer-wmi can probed on your machine, it cann't help anything
for your backlight control.

Thank's a lot!
Joey Lee

2011-05-05 20:42:27

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem (was: Re: Linux 2.6.39-rc5)

* Takashi Iwai -- Thursday 05 May 2011:
> Try the fixed patch below.

> ---
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 456f404..4c6e187 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -332,6 +332,7 @@ typedef struct drm_i915_private {
> struct intel_overlay *overlay;
>
> /* LVDS info */
> + int backlight_combination_mode; /* 0=unknown, -1=no, 1=yes */
> int backlight_level; /* restore backlight to this value */
> bool backlight_enabled;
> struct drm_display_mode *panel_fixed_mode;
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index f8f86e5..5a6be04 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -115,14 +115,24 @@ done:
> static int is_backlight_combination_mode(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> -
> - if (INTEL_INFO(dev)->gen >= 4)
> - return I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE;
> -
> - if (IS_GEN2(dev))
> - return I915_READ(BLC_PWM_CTL) & BLM_LEGACY_MODE;
> -
> - return 0;
> + int combo_mode;
> + u8 lbpc;
> +
> + if (dev_priv->backlight_combination_mode)
> + return dev_priv->backlight_combination_mode > 0;
> +
> + pci_read_config_byte(dev->pdev, PCI_LBPC, &lbpc);
> + if (!lbpc)
> + combo_mode = 0;
> + else if (INTEL_INFO(dev)->gen >= 4)
> + combo_mode = I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE;
> + else if (IS_GEN2(dev))
> + combo_mode = I915_READ(BLC_PWM_CTL) & BLM_LEGACY_MODE;
> + else
> + combo_mode = 0;
> +
> + dev_priv->backlight_combination_mode = combo_mode ? 1 : -1;
> + return combo_mode;
> }
>
> static u32 i915_read_blc_pwm_ctl(struct drm_i915_private *dev_priv)

The first patch had a problem. Without the lbpc check for zero the backlight
still stayed off and could only be activated with the brightness-down key.
This revised patch works correctly for me. Thanks!

m.

2011-05-06 08:52:11

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem

* Melchior FRANZ -- Thursday 05 May 2011:
> This revised patch works correctly for me.

... and changing backlight brightness worked with acpi_osi=Linux, too.
Unfortunately, that does no longer work now, although I haven't changed
anything. Pressing the brightness buttons causes some ACPI debug output,
but it doesn't adjust anything.

m.

2011-05-06 12:42:06

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem

Hi Melchior,

於五，2011-05-06 於 10:52 +0200，Melchior FRANZ 提到：
> * Melchior FRANZ -- Thursday 05 May 2011:
> > This revised patch works correctly for me.
>
> ... and changing backlight brightness worked with acpi_osi=Linux, too.
> Unfortunately, that does no longer work now, although I haven't changed
> anything. Pressing the brightness buttons causes some ACPI debug output,
> but it doesn't adjust anything.
>
> m.

hmm........ Takashi's patch works before, but doesn't work now?

Does that because my new patch for bko#34142 ? Could you please add
acer-wmi to /etc/modprobe.d/blacklist then reboot machine ?

Step:
- add acer-wmi to /etc/modprobe.d/blacklist
- shutdown your machine
- remove AC power and notebook's battery
- plugin AC power then boot your notebook
- check the brightness control

Thank's
Joey Lee

2011-05-06 16:23:12

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem

* Joey Lee -- Friday 06 May 2011:
> 於五，2011-05-06 於 10:52 +0200，Melchior FRANZ 提到：
> > ... and changing backlight brightness worked with acpi_osi=Linux, too.
> > Unfortunately, that does no longer work now, although I haven't changed
> > anything. Pressing the brightness buttons causes some ACPI debug output,
> > but it doesn't adjust anything.

> hmm........ Takashi's patch works before, but doesn't work now?

Argh ... sorry Takashi. Joey is right. I thought I had tested that,
but apparently not. Indeed, your last patch prevents the backlight
from being turned off, but it also breaks the brightness adjustment
keys at runtime with acpi_osi=Linux. And without that it doesn't
work, anyway, just like before. I'll add some more debug messages ...

> Does that because my new patch for bko#34142 ? Could you please add
> acer-wmi to /etc/modprobe.d/blacklist then reboot machine ?

Done. No, your patch is not involved. But backing out Takashi's
patch and re-applying confirmed that it's that patch.

m.

2011-05-07 20:22:49

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem

* Melchior FRANZ -- Friday 06 May 2011:
> last patch prevents the backlight from being turned off, but it also
> breaks the brightness adjustment keys at runtime with acpi_osi=Linux.

It has turned out that acpi key events seem to be handled correctly
and even the state of /sys/class/backlight/acer-wmi/brightness is
updated accordingly. The only problem is that this maintained
brightness state isn't applied to the actual backlight. It remains
at highest level. Google pointed me to this workaround for another
Acer notebook:

https://help.ubuntu.com/community/AspireTimeline/Fixes#Alternative%20fix%20for%2010.10

This uses the acpid to write the brightness value to the display
using setpci. And this works on my notebook as well (Acer Travelmate
5735Z-452G32Mnss). Not pretty but better than nothing. Is Acer
not able to build proper notebooks? Or is it a kernel bug?

m.

2011-05-08 13:41:45

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem

於六，2011-05-07 於 22:22 +0200，Melchior FRANZ 提到：
> * Melchior FRANZ -- Friday 06 May 2011:
> > last patch prevents the backlight from being turned off, but it also
> > breaks the brightness adjustment keys at runtime with acpi_osi=Linux.
>
> It has turned out that acpi key events seem to be handled correctly
> and even the state of /sys/class/backlight/acer-wmi/brightness is

That's interesting for acer-wmi generated brightness interface when this
machine declare acpi brightness interface in DSDT.

> updated accordingly. The only problem is that this maintained

Does it work to you direct control brightness by access
by /sys/class/backlight/acer-wmi/brightness ?

> brightness state isn't applied to the actual backlight. It remains
> at highest level. Google pointed me to this workaround for another
> Acer notebook:
>
> https://help.ubuntu.com/community/AspireTimeline/Fixes#Alternative%20fix%20for%2010.10
>
> This uses the acpid to write the brightness value to the display
> using setpci. And this works on my notebook as well (Acer Travelmate

As I remember, use setpci to control brightness is not recommended
because BIOS or ACPI will also touch brightness level. That will be
better control brightness by the function that was provided by BIOS.
e.g. ACPI or WMI interface, or direct control by EC.

That means that will be better fix your Fn key control brightness like
before, you just need press Fn key to change brightness and don't need
have any workaround.

> 5735Z-452G32Mnss). Not pretty but better than nothing. Is Acer
> not able to build proper notebooks? Or is it a kernel bug?
>
> m.

Looks like current status is we try to fix bko#31522 but the patch
causes your brightness no work by press Fn key even with acpi_osi=Linux.
Does it right?

Could you please also help to check:
replace acpi_osi=Linux by acpi_osi="!Windows 2006"

Does it also works to you for backlight control?

Thank's
Joey Lee

2011-05-08 14:05:51

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem

* Joey Lee -- Sunday 08 May 2011:
> 於六，2011-05-07 於 22:22 +0200，Melchior FRANZ 提到：
> > It has turned out that acpi key events seem to be handled correctly
> > and even the state of /sys/class/backlight/acer-wmi/brightness is
>
> That's interesting for acer-wmi generated brightness interface when this
> machine declare acpi brightness interface in DSDT.

I'm sorry to give contradicting info. Yesterday I saw a subdir acer-wmi
(or acer_wmi) in /sys/class/backlight/ during one of my tests (with/without
acer_wmi, with/without acpi_osi=Linux). Now I left acpi_osi=Linux
away and allow acer_wmi, and it's subdir acpi_video0. Next time I'll
make notes.

> Does it work to you direct control brightness by access
> by /sys/class/backlight/acer-wmi/brightness ?

No. A number written to this virtual file is accepted and remembered,
but it doesn't actually change the brightness. It takes setpci to do
that.

> As I remember, use setpci to control brightness is not recommended
> because BIOS or ACPI will also touch brightness level. That will be
> better control brightness by the function that was provided by BIOS.
> e.g. ACPI or WMI interface, or direct control by EC.

Well, sounds plausible. And I wouldn't do it if it weren't the only
way at the moment. :-)

> That means that will be better fix your Fn key control brightness like
> before, you just need press Fn key to change brightness and don't need
> have any workaround.

OK. I have added a lot of debug messages to intel_panel.c yesterday.
All it told me was that it seems to work correctly wiht acpi_osi=Linux.
Except that it doesn't actually change the brightness. Without acpi_osi
the functions aren't called at all and none of my messages showed up.

> Looks like current status is we try to fix bko#31522 but the patch
> causes your brightness no work by press Fn key even with acpi_osi=Linux.
> Does it right?

The history is: with acpi_osi=Linux everything worked with 2.6.38-rc8.
With 2.6.38 the screen stayed black. The patch that only ignored lbpc=0
worked (IIRC) including key adjustment. Later patches broke keys.

> replace acpi_osi=Linux by acpi_osi="!Windows 2006"
>
> Does it also works to you for backlight control?

No, doesn't work.

m.

2011-05-09 08:50:57

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem

Add Cc. Michael Chang for he is our i915 expert.

Hi Melchior,

於日，2011-05-08 於 16:05 +0200，Melchior FRANZ 提到：
>
> > Does it work to you direct control brightness by access
> > by /sys/class/backlight/acer-wmi/brightness ?
>
> No. A number written to this virtual file is accepted and remembered,
> but it doesn't actually change the brightness. It takes setpci to do
> that.
>

I thought the video driver still is the KEY component for backlight
issues, need fix the problem in video driver first.

>
>
> > As I remember, use setpci to control brightness is not recommended
> > because BIOS or ACPI will also touch brightness level. That will be
> > better control brightness by the function that was provided by BIOS.
> > e.g. ACPI or WMI interface, or direct control by EC.
>
> Well, sounds plausible. And I wouldn't do it if it weren't the only
> way at the moment. :-)
>
>
>
> > That means that will be better fix your Fn key control brightness like
> > before, you just need press Fn key to change brightness and don't need
> > have any workaround.
>
> OK. I have added a lot of debug messages to intel_panel.c yesterday.
> All it told me was that it seems to work correctly wiht acpi_osi=Linux.
> Except that it doesn't actually change the brightness. Without acpi_osi
> the functions aren't called at all and none of my messages showed up.
>

I traced _Q event in your DSDT, when acpi_osi=Linux, it run the Intel
OpRegion logic for change brightness. And, finally, intel_opregion will
access the function the were provided by intel_panel. So, the problem
still back to intel_panel.

After discuss with Michael chang, we thought there have problem in your
brightness level after add combination mode:

vi driver/gpu/drm/i915/intel_panel.c

void intel_panel_set_backlight(struct drm_device *dev, u32 level)
{
struct drm_i915_private *dev_priv = dev->dev_private;
u32 tmp;

DRM_DEBUG_DRIVER("set backlight PWM = %d\n", level);

if (HAS_PCH_SPLIT(dev))
return intel_pch_panel_set_backlight(dev, level);

if (is_backlight_combination_mode(dev)){
u32 max = intel_panel_get_max_backlight(dev);
u8 lbpc;

lbpc = level * 0xfe / max + 1;
level /= lbpc; /* maybe the level changed by lbpc */
pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc);
}

tmp = I915_READ(BLC_PWM_CTL);
if (IS_PINEVIEW(dev)) {
tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
level <<= 1;
} else
tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
I915_WRITE(BLC_PWM_CTL, tmp | level);
}

We need to know some run time value when intel_panel_set_backlight call by funciton key.
Please help to apply the attached debug patch to intel_panel.c then attached dmesg.

>
>
> > Looks like current status is we try to fix bko#31522 but the patch
> > causes your brightness no work by press Fn key even with acpi_osi=Linux.
> > Does it right?
>
> The history is: with acpi_osi=Linux everything worked with 2.6.38-rc8.
> With 2.6.38 the screen stayed black. The patch that only ignored lbpc=0
> worked (IIRC) including key adjustment. Later patches broke keys.
>
>
>
> > replace acpi_osi=Linux by acpi_osi="!Windows 2006"
> >
> > Does it also works to you for backlight control?
>
> No, doesn't work.
>

We can test the acpi_osi="!Windows 2006" again after we fix the i915's
problem.

Thank's
Joey Lee

The following is debug patch, and please add kernel parameter
drm.debug=0x02 :

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index f8f86e5..f62dbd9 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -236,17 +236,22 @@ void intel_panel_set_backlight(struct drm_device *dev, u32 level)
u32 max = intel_panel_get_max_backlight(dev);
u8 lbpc;

+ DRM_DEBUG_DRIVER("set backlight max = % lbpc = level * 0xfe / max + 1;
+ DRM_DEBUG_DRIVER("set backlight lbpc = %d\n", lbpc);
level /= lbpc;
pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc);
}

tmp = I915_READ(BLC_PWM_CTL);
+ DRM_DEBUG_DRIVER("set backlight tmp(1) = %d\n", tmp);
if (IS_PINEVIEW(dev)) {
tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
level <<= 1;
} else
tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
+ DRM_DEBUG_DRIVER("set backlight tmp(2) = %d\n", tmp);
+ DRM_DEBUG_DRIVER("set backlight level = %d\n", level);
I915_WRITE(BLC_PWM_CTL, tmp | level);
}

2011-05-09 08:58:21

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem

At Sat, 7 May 2011 22:22:40 +0200,
Melchior FRANZ wrote:
>
> * Melchior FRANZ -- Friday 06 May 2011:
> > last patch prevents the backlight from being turned off, but it also
> > breaks the brightness adjustment keys at runtime with acpi_osi=Linux.
>
> It has turned out that acpi key events seem to be handled correctly
> and even the state of /sys/class/backlight/acer-wmi/brightness is
> updated accordingly. The only problem is that this maintained
> brightness state isn't applied to the actual backlight. It remains
> at highest level. Google pointed me to this workaround for another
> Acer notebook:
>
> https://help.ubuntu.com/community/AspireTimeline/Fixes#Alternative%20fix%20for%2010.10
>
> This uses the acpid to write the brightness value to the display
> using setpci. And this works on my notebook as well (Acer Travelmate
> 5735Z-452G32Mnss).

Then we miss something. With the hack above, you are doing nothing
but writing LBPC register value externally from setpci. It implies
that the write to LBPC basically works on your machine.

Takashi

2011-05-09 09:00:12

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem

At Mon, 09 May 2011 02:50:50 -0600,
Joey Lee wrote:
>
> Add Cc. Michael Chang for he is our i915 expert.
>
> Hi Melchior,
>
> 於日，2011-05-08 於 16:05 +0200，Melchior FRANZ 提到：
> >
> > > Does it work to you direct control brightness by access
> > > by /sys/class/backlight/acer-wmi/brightness ?
> >
> > No. A number written to this virtual file is accepted and remembered,
> > but it doesn't actually change the brightness. It takes setpci to do
> > that.
> >
>
> I thought the video driver still is the KEY component for backlight
> issues, need fix the problem in video driver first.
>
> >
> >
> > > As I remember, use setpci to control brightness is not recommended
> > > because BIOS or ACPI will also touch brightness level. That will be
> > > better control brightness by the function that was provided by BIOS.
> > > e.g. ACPI or WMI interface, or direct control by EC.
> >
> > Well, sounds plausible. And I wouldn't do it if it weren't the only
> > way at the moment. :-)
> >
> >
> >
> > > That means that will be better fix your Fn key control brightness like
> > > before, you just need press Fn key to change brightness and don't need
> > > have any workaround.
> >
> > OK. I have added a lot of debug messages to intel_panel.c yesterday.
> > All it told me was that it seems to work correctly wiht acpi_osi=Linux.
> > Except that it doesn't actually change the brightness. Without acpi_osi
> > the functions aren't called at all and none of my messages showed up.
> >
>
> I traced _Q event in your DSDT, when acpi_osi=Linux, it run the Intel
> OpRegion logic for change brightness. And, finally, intel_opregion will
> access the function the were provided by intel_panel. So, the problem
> still back to intel_panel.
>
> After discuss with Michael chang, we thought there have problem in your
> brightness level after add combination mode:
>
> vi driver/gpu/drm/i915/intel_panel.c
>
> void intel_panel_set_backlight(struct drm_device *dev, u32 level)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> u32 tmp;
>
> DRM_DEBUG_DRIVER("set backlight PWM = %d\n", level);
>
> if (HAS_PCH_SPLIT(dev))
> return intel_pch_panel_set_backlight(dev, level);
>
> if (is_backlight_combination_mode(dev)){
> u32 max = intel_panel_get_max_backlight(dev);
> u8 lbpc;
>
> lbpc = level * 0xfe / max + 1;
> level /= lbpc; /* maybe the level changed by lbpc */
> pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc);
> }
>
> tmp = I915_READ(BLC_PWM_CTL);
> if (IS_PINEVIEW(dev)) {
> tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
> level <<= 1;
> } else
> tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
> I915_WRITE(BLC_PWM_CTL, tmp | level);
> }
>
> We need to know some run time value when intel_panel_set_backlight call by funciton key.

Yes, that'll help understanding.

> Please help to apply the attached debug patch to intel_panel.c then attached dmesg.

The patch has an obvious typo :)
Also, we should track the value in intel_panel_get_backlight(), too.

Takashi

> > > Looks like current status is we try to fix bko#31522 but the patch
> > > causes your brightness no work by press Fn key even with acpi_osi=Linux.
> > > Does it right?
> >
> > The history is: with acpi_osi=Linux everything worked with 2.6.38-rc8.
> > With 2.6.38 the screen stayed black. The patch that only ignored lbpc=0
> > worked (IIRC) including key adjustment. Later patches broke keys.
> >
> >
> >
> > > replace acpi_osi=Linux by acpi_osi="!Windows 2006"
> > >
> > > Does it also works to you for backlight control?
> >
> > No, doesn't work.
> >
>
> We can test the acpi_osi="!Windows 2006" again after we fix the i915's
> problem.
>
>
> Thank's
> Joey Lee
>
> The following is debug patch, and please add kernel parameter
> drm.debug=0x02 :
>
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index f8f86e5..f62dbd9 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -236,17 +236,22 @@ void intel_panel_set_backlight(struct drm_device *dev, u32 level)
> u32 max = intel_panel_get_max_backlight(dev);
> u8 lbpc;
>
> + DRM_DEBUG_DRIVER("set backlight max = % lbpc = level * 0xfe / max + 1;
> + DRM_DEBUG_DRIVER("set backlight lbpc = %d\n", lbpc);
> level /= lbpc;
> pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc);
> }
>
> tmp = I915_READ(BLC_PWM_CTL);
> + DRM_DEBUG_DRIVER("set backlight tmp(1) = %d\n", tmp);
> if (IS_PINEVIEW(dev)) {
> tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
> level <<= 1;
> } else
> tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
> + DRM_DEBUG_DRIVER("set backlight tmp(2) = %d\n", tmp);
> + DRM_DEBUG_DRIVER("set backlight level = %d\n", level);
> I915_WRITE(BLC_PWM_CTL, tmp | level);
> }
>
>
>

2011-05-09 09:35:29

[permalink] [raw]

Subject: Re: i915/kms/backlight-combo mode problem

於一，2011-05-09 於 11:00 +0200，Takashi Iwai 提到：
> At Mon, 09 May 2011 02:50:50 -0600,
> Joey Lee wrote:
> >
> > We need to know some run time value when intel_panel_set_backlight call by funciton key.
>
> Yes, that'll help understanding.
>
> > Please help to apply the attached debug patch to intel_panel.c then attached dmesg.
>
> The patch has an obvious typo :)
> Also, we should track the value in intel_panel_get_backlight(), too.
>
>
> Takashi
>

Thank's for Takashi's review and sorry for my typo.
Follow Takashi's suggestion, I added a debug message in get_backlight,
the following is new debug patch:

diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index f8f86e5..9695840 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -199,6 +199,7 @@ u32 intel_panel_get_backlight(struct drm_device *dev)
val = I915_READ(BLC_PWM_CPU_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
} else {
val = I915_READ(BLC_PWM_CTL) & BACKLIGHT_DUTY_CYCLE_MASK;
+ DRM_DEBUG_DRIVER("get backlight val = %d\n", val);
if (IS_PINEVIEW(dev))
val >>= 1;

@@ -236,17 +237,22 @@ void intel_panel_set_backlight(struct drm_device *dev, u32 level)
u32 max = intel_panel_get_max_backlight(dev);
u8 lbpc;

+ DRM_DEBUG_DRIVER("set backlight max = %d\n", max);
lbpc = level * 0xfe / max + 1;
+ DRM_DEBUG_DRIVER("set backlight lbpc = %d\n", lbpc);
level /= lbpc;
pci_write_config_byte(dev->pdev, PCI_LBPC, lbpc);
}

tmp = I915_READ(BLC_PWM_CTL);
+ DRM_DEBUG_DRIVER("set backlight tmp(1) = %d\n", tmp);
if (IS_PINEVIEW(dev)) {
tmp &= ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
level <<= 1;
} else
tmp &= ~BACKLIGHT_DUTY_CYCLE_MASK;
+ DRM_DEBUG_DRIVER("set backlight tmp(2) = %d\n", tmp);
+ DRM_DEBUG_DRIVER("set backlight level = %d\n", level);
I915_WRITE(BLC_PWM_CTL, tmp | level);
}

2011-05-09 10:08:17