Re: [Intel-gfx] [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips
Hello! I know this is an old thread, but i am having this problem on my macbook air. Ideas for a quick fix? Thanks, James ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips
On Tue, 22 Nov 2011 17:40:25 +0100, Daniel Mack zon...@gmail.com wrote: Just curious - is this patch queued somewhere for mainline yet? I'm waiting for airlied to pull drm-fixes-intel before I push more stuff there. -- keith.pack...@intel.com pgpn9lOMl3zqH.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips
On Sun, 20 Nov 2011 12:22:50 +0100, Takashi Iwai ti...@suse.de wrote: So, writing bit-0 caused a problem, as it seems. Thanks. I've noted that in the patch message. -- keith.pack...@intel.com pgpCXCnhRKCYp.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips
At Sat, 19 Nov 2011 10:34:12 -0800, Keith Packard wrote: [1 text/plain (quoted-printable)] On Sat, 19 Nov 2011 11:05:05 +0100, Takashi Iwai ti...@suse.de wrote: Maybe it'd be better to mention that actually setting bit-0 caused a blank screen on some machines. Was that caused by *just* setting bit zero? Or was it caused by setting the duty cycle to 0x, in which case it would be larger than the maximum value? I'll clean up the commit log message with your answer and then push this out. According to Daniels' original post: On 11/04/2011 03:36 PM, Daniel Mack wrote: I'm facing a bug on a Samsung X20 notebook which features an i915 chipset (output of 'lspci -v' attached). The effect is that setting the backlight to odd values causes the value to be misinterpreted. Harald Hoyer (cc:) had the same thing on a Netbook (I don't recall which model it was). So this will turn the backlight to full brightness: # cat /sys/class/backlight/intel_backlight/max_brightness 29750 # echo 29750 /sys/class/backlight/intel_backlight/brightness However, writing 29749 will turn the display backlight off, and 29748 appears to be the next valid lower value. So, writing bit-0 caused a problem, as it seems. Takashi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips
On Sat, 19 Nov 2011 11:05:05 +0100, Takashi Iwai ti...@suse.de wrote: Maybe it'd be better to mention that actually setting bit-0 caused a blank screen on some machines. Was that caused by *just* setting bit zero? Or was it caused by setting the duty cycle to 0x, in which case it would be larger than the maximum value? I'll clean up the commit log message with your answer and then push this out. -- keith.pack...@intel.com pgpHsSu2riBkU.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips
On Thu, 17 Nov 2011 17:33:50 +0100, Takashi Iwai ti...@suse.de wrote:
If it's only with 915GM, we'll just need to change IS_PINEVEW() to
IS_PINEVIEW() || IS_I915GM(). This might be a safer option at this
moment unless we check all cases or specs...
I read through the hardware docs yesterday and figured out what was
going on. On all pre-gen4 hardware, the maximum backlight value has the
lowest bit (of 16) hard-wired to zero. This means that the possible
backlight duty cycle values are limited to 0 .. 0xfffe.
On older hardware, this was managed by reporting this true range. This
meant that the set_backlight path didn't need any special code; simply
setting the values as provided should have worked just fine.
On Pineview, this was changed (for reasons unknown) to use only 15 bits
for backlight levels. The range of possible values is then
0 .. 0x7fff. In this case, values have to be shifted by one to convert
between the advertised range of 0 .. 0x7fff and the hardware range of
0 .. 0xfffe.
Exposing the range of 0 .. 0xfffe introduces a potential problem --
write a value of 0x and the hardware gets mightily confused,
and probably ends up turning the backlight off. Using the range of
0 .. 0x7fff avoids this issue completely.
Using the narrower range does limit the precision of the backlight level
setting, but it seems like 32767 possible values is more than sufficient...
Here's a patch which just uses the pineview version everywhere (and
cleans that up at the same time).
From e06789f688dc7ab1331f5f15ad3d60326b5139b4 Mon Sep 17 00:00:00 2001
From: Keith Packard kei...@keithp.com
Date: Fri, 18 Nov 2011 11:09:24 -0800
Subject: [PATCH] drm/i915: Treat pre-gen4 backlight duty cycle value
consistently
For i945 and earlier chips, the backlight frequency value had the low
bit (of 16) fixed to zero. The Pineview code path handled this by just
exposing the backlight range as 15 bits while other chips had the
backlight range limited to 0 .. 0xfffe.
This patch makes everyone take the pineview code path, providing 15
bits of backlight duty cycle range which seems more than sufficient to me.
Signed-off-by: Keith Packard kei...@keithp.com
---
drivers/gpu/drm/i915/intel_panel.c | 16 +---
1 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 21f60b7..04d79fd 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -178,13 +178,10 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev)
if (HAS_PCH_SPLIT(dev)) {
max = 16;
} else {
- if (IS_PINEVIEW(dev)) {
+ if (INTEL_INFO(dev)-gen 4)
max = 17;
- } else {
+ else
max = 16;
- if (INTEL_INFO(dev)-gen 4)
- max = ~1;
- }
if (is_backlight_combination_mode(dev))
max *= 0xff;
@@ -203,13 +200,12 @@ 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;
- if (IS_PINEVIEW(dev))
+ if (INTEL_INFO(dev)-gen 4)
val = 1;
if (is_backlight_combination_mode(dev)) {
u8 lbpc;
- val = ~1;
pci_read_config_byte(dev-pdev, PCI_LBPC, lbpc);
val *= lbpc;
}
@@ -246,11 +242,9 @@ static void intel_panel_actually_set_backlight(struct
drm_device *dev, u32 level
}
tmp = I915_READ(BLC_PWM_CTL);
- if (IS_PINEVIEW(dev)) {
- tmp = ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
+ if (INTEL_INFO(dev)-gen 4)
level = 1;
- } else
- tmp = ~BACKLIGHT_DUTY_CYCLE_MASK;
+ tmp = ~BACKLIGHT_DUTY_CYCLE_MASK;
I915_WRITE(BLC_PWM_CTL, tmp | level);
}
--
1.7.7.3
--
keith.pack...@intel.com
pgphIVM6LQ6Q1.pgp
Description: PGP signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips
On Wed, 16 Nov 2011 18:14:55 +0100, Takashi Iwai ti...@suse.de wrote: While refactoring of backlight control code in commit [a95735569: drm/i915: Refactor panel backlight controls], the handling of the bit 0 of duty-cycle was gone except for pineview. This resulted in invalid register values for old chips like 915GM. When the bit 0 is set, the backlight is turned off suddenly. I'm looking at the mentioned patch and I don't see how that managed to correctly handle bit 0; is this the patch that managed to break this? -- keith.pack...@intel.com pgpNfnYzlHW6a.pgp Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips
At Wed, 16 Nov 2011 22:15:41 -0800, Keith Packard wrote: On Wed, 16 Nov 2011 18:14:55 +0100, Takashi Iwai ti...@suse.de wrote: While refactoring of backlight control code in commit [a95735569: drm/i915: Refactor panel backlight controls], the handling of the bit 0 of duty-cycle was gone except for pineview. This resulted in invalid register values for old chips like 915GM. When the bit 0 is set, the backlight is turned off suddenly. I'm looking at the mentioned patch and I don't see how that managed to correctly handle bit 0; is this the patch that managed to break this? Hrm, then I must have been confused. The gen 4 check (formerly !IS_I965G()) was firstly introduced in this refactoring. I thought this was done before, but apparently not. Looking more carefully again, actually the IS_PINEVIEW() part (former IS_GND()) was introduced in the commit [078a033f: drm/i915: fix opregion backlight chip detect and range]. Thus the same bug must have been present even in the earlier version, but didn't come up by some reason. So, I'll have to correct the patch commit log. Sorry for that. Now, a question is whether the condition (gen 4) is really correct. I *guess* the original code in the refactoring was intended to cover it: if (!IS_I965G(dev)) max = ~1; But this rule wasn't applied to set_backlight(). The bit-0-clear above implies that it's a similar behavior like pineview. And indeed Daniel's machine with 915GM hits this problem. If the above rule should be applied to all places, it'd make more sense to handle both in the same way like pineview. Unfortunately I have no old machines to test this, and the only material to judge was Daniel's case. (Is Harald's machine also 915GM?) If it's only with 915GM, we'll just need to change IS_PINEVEW() to IS_PINEVIEW() || IS_I915GM(). This might be a safer option at this moment unless we check all cases or specs... thanks, Takashi ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Fix invalid backpanel values for GEN3 or older chips
While refactoring of backlight control code in commit [a95735569:
drm/i915: Refactor panel backlight controls], the handling of the bit
0 of duty-cycle was gone except for pineview. This resulted in invalid
register values for old chips like 915GM. When the bit 0 is set, the
backlight is turned off suddenly.
This patch changes the bit-0 check by replacing with the condition of
gen 4 (pineview is included in this condition, too).
Reported-and-tested-by: Daniel Mack zon...@gmail.com
Cc: sta...@kernel.org
Signed-off-by: Takashi Iwai ti...@suse.de
---
drivers/gpu/drm/i915/intel_panel.c |8 +++-
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_panel.c
index 499d4c0..737d00f 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -178,12 +178,10 @@ u32 intel_panel_get_max_backlight(struct drm_device *dev)
if (HAS_PCH_SPLIT(dev)) {
max = 16;
} else {
- if (IS_PINEVIEW(dev)) {
+ if (INTEL_INFO(dev)-gen 4) {
max = 17;
} else {
max = 16;
- if (INTEL_INFO(dev)-gen 4)
- max = ~1;
}
if (is_backlight_combination_mode(dev))
@@ -203,7 +201,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;
- if (IS_PINEVIEW(dev))
+ if (INTEL_INFO(dev)-gen 4)
val = 1;
if (is_backlight_combination_mode(dev)) {
@@ -246,7 +244,7 @@ static void intel_panel_actually_set_backlight(struct
drm_device *dev, u32 level
}
tmp = I915_READ(BLC_PWM_CTL);
- if (IS_PINEVIEW(dev)) {
+ if (INTEL_INFO(dev)-gen 4) {
tmp = ~(BACKLIGHT_DUTY_CYCLE_MASK - 1);
level = 1;
} else
--
1.7.7
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
