[PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Daniel Vetter]

There's a bit in the docs for gen4 only that says whether the
backlight control is inverted. And both the quirk we have and
all bugs only concern i965gm and gm45 (and mostly Acer) afaics.

So lets drop the quirk and use the bit instead.

Also clean up the BLC register definitions a bit by correctly
grouping the CTL and CTL2 definitions together.

This quirk was originally added in

commit 5a15ab5b93e4a3ebcd4fa6c76cf646a45e9cf806
Author: Carsten Emde <C.Emde@osadl.org>
Date:   Thu Mar 15 15:56:27 2012 +0100

    drm/i915: panel: invert brightness acer aspire 5734z

Cc: Carsten Emde <C.Emde@osadl.org>
References: https://bugzilla.kernel.org/show_bug.cgi?id=31522
References: https://bugs.freedesktop.org/show_bug.cgi?id=37986
References: https://bugs.freedesktop.org/show_bug.cgi?id=40455
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h      |    7 ++++---
 drivers/gpu/drm/i915/intel_display.c |    3 ---
 drivers/gpu/drm/i915/intel_panel.c   |    4 ++++
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5ac9837..67c4ca0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1783,16 +1783,17 @@
 #define PFIT_AUTO_RATIOS 0x61238
 
 /* Backlight control */
-#define BLC_PWM_CTL		0x61254
-#define   BACKLIGHT_MODULATION_FREQ_SHIFT		(17)
 #define BLC_PWM_CTL2		0x61250 /* 965+ only */
-#define   BLM_COMBINATION_MODE (1 << 30)
+#define   BLM_COMBINATION_MODE	(1 << 30)
+#define   BLM_POLARITY_I965	(1 << 28) /* gen4 only */
+#define BLC_PWM_CTL		0x61254
 /*
  * This is the most significant 15 bits of the number of backlight cycles in a
  * complete cycle of the modulated backlight control.
  *
  * The actual value is this field multiplied by two.
  */
+#define   BACKLIGHT_MODULATION_FREQ_SHIFT		(17)
 #define   BACKLIGHT_MODULATION_FREQ_MASK		(0x7fff << 17)
 #define   BLM_LEGACY_MODE				(1 << 16)
 /*
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4c844c6..5d215f0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6482,9 +6482,6 @@ static struct intel_quirk intel_quirks[] = {
 
 	/* Sony Vaio Y cannot use SSC on LVDS */
 	{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
-
-	/* Acer Aspire 5734Z must invert backlight brightness */
-	{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness },
 };
 
 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 cad45ff..1874517 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -212,6 +212,10 @@ static u32 intel_panel_compute_brightness(struct drm_device *dev, u32 val)
 	    dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS)
 		return intel_panel_get_max_backlight(dev) - val;
 
+	/* gen4 has a polarity bit */
+	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965))
+			return intel_panel_get_max_backlight(dev) - val;
+
 	return val;
 }
 
-- 
1.7.10

[PATCH 2/2] drm/i915: pnv has a backlight polarity control bit, too

[Daniel Vetter]

So let's use it.

We already correctly ignore bit0 on gen < 4, now we also now why ;-)
I've decided that losing that single bit of precision isn't worth the
trouble to sprinkle IS_PINEVIEW checks all over the backlight control
code - that code is way too fragile imo.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h    |    2 ++
 drivers/gpu/drm/i915/intel_panel.c |    5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 67c4ca0..45d5568 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1805,6 +1805,8 @@
  */
 #define   BACKLIGHT_DUTY_CYCLE_SHIFT		(0)
 #define   BACKLIGHT_DUTY_CYCLE_MASK		(0xffff)
+#define   BACKLIGHT_DUTY_CYCLE_MASK_PNV		(0xfffe)
+#define   BLM_POLARITY_PNV			(1 << 0) /* pnv only */
 
 #define BLC_HIST_CTL		0x61260
 
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 1874517..e519221 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -212,8 +212,9 @@ static u32 intel_panel_compute_brightness(struct drm_device *dev, u32 val)
 	    dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS)
 		return intel_panel_get_max_backlight(dev) - val;
 
-	/* gen4 has a polarity bit */
-	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965))
+	/* gen4/pnv has a polarity bit */
+	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965) ||
+	    IS_PINEVIEW(dev) && (I915_READ(BLC_PWM_CTL) & BLM_POLARITY_PNV))
 			return intel_panel_get_max_backlight(dev) - val;
 
 	return val;
-- 
1.7.10

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Chris Wilson]

On Mon, 23 Apr 2012 11:32:14 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> There's a bit in the docs for gen4 only that says whether the
> backlight control is inverted. And both the quirk we have and
> all bugs only concern i965gm and gm45 (and mostly Acer) afaics.
> 
> So lets drop the quirk and use the bit instead.
> 
> Also clean up the BLC register definitions a bit by correctly
> grouping the CTL and CTL2 definitions together.
> 
> This quirk was originally added in
> 
> commit 5a15ab5b93e4a3ebcd4fa6c76cf646a45e9cf806
> Author: Carsten Emde <C.Emde@osadl.org>
> Date:   Thu Mar 15 15:56:27 2012 +0100
> 
>     drm/i915: panel: invert brightness acer aspire 5734z
> 
> Cc: Carsten Emde <C.Emde@osadl.org>
> References: https://bugzilla.kernel.org/show_bug.cgi?id=31522
> References: https://bugs.freedesktop.org/show_bug.cgi?id=37986
> References: https://bugs.freedesktop.org/show_bug.cgi?id=40455
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Oh, light dawns. That explains the polarity bit.

Hmm, on PCH this bit is moved to 29 (expect for early IBX silicon)
according to my specs.

Btw, you introduce too many tabs.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/2] drm/i915: pnv has a backlight polarity control bit, too

[Chris Wilson]

On Mon, 23 Apr 2012 11:32:15 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> So let's use it.
> 
> We already correctly ignore bit0 on gen < 4, now we also now why ;-)
> I've decided that losing that single bit of precision isn't worth the
> trouble to sprinkle IS_PINEVIEW checks all over the backlight control
> code - that code is way too fragile imo.
> 
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |    2 ++
>  drivers/gpu/drm/i915/intel_panel.c |    5 +++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 67c4ca0..45d5568 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1805,6 +1805,8 @@
>   */
>  #define   BACKLIGHT_DUTY_CYCLE_SHIFT		(0)
>  #define   BACKLIGHT_DUTY_CYCLE_MASK		(0xffff)
> +#define   BACKLIGHT_DUTY_CYCLE_MASK_PNV		(0xfffe)
> +#define   BLM_POLARITY_PNV			(1 << 0) /* pnv only */
>  
>  #define BLC_HIST_CTL		0x61260
>  
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 1874517..e519221 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -212,8 +212,9 @@ static u32 intel_panel_compute_brightness(struct drm_device *dev, u32 val)
>  	    dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS)
>  		return intel_panel_get_max_backlight(dev) - val;
>  
> -	/* gen4 has a polarity bit */
> -	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965))
> +	/* gen4/pnv has a polarity bit */
> +	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965) ||
> +	    IS_PINEVIEW(dev) && (I915_READ(BLC_PWM_CTL) & BLM_POLARITY_PNV))

Now that is just getting ugly. Break this out into a simple and readable
predicate function intel_panel_backlight_is_inverted().

>  			return intel_panel_get_max_backlight(dev) - val;
Tabtastic.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Daniel Vetter]

On Mon, Apr 23, 2012 at 10:53:31AM +0100, Chris Wilson wrote:
> On Mon, 23 Apr 2012 11:32:14 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > There's a bit in the docs for gen4 only that says whether the
> > backlight control is inverted. And both the quirk we have and
> > all bugs only concern i965gm and gm45 (and mostly Acer) afaics.
> > 
> > So lets drop the quirk and use the bit instead.
> > 
> > Also clean up the BLC register definitions a bit by correctly
> > grouping the CTL and CTL2 definitions together.
> > 
> > This quirk was originally added in
> > 
> > commit 5a15ab5b93e4a3ebcd4fa6c76cf646a45e9cf806
> > Author: Carsten Emde <C.Emde@osadl.org>
> > Date:   Thu Mar 15 15:56:27 2012 +0100
> > 
> >     drm/i915: panel: invert brightness acer aspire 5734z
> > 
> > Cc: Carsten Emde <C.Emde@osadl.org>
> > References: https://bugzilla.kernel.org/show_bug.cgi?id=31522
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=37986
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=40455
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Oh, light dawns. That explains the polarity bit.
> 
> Hmm, on PCH this bit is moved to 29 (expect for early IBX silicon)
> according to my specs.

On my specs bit29 is pipe assignement, we should set it if the panel is on
pipe B (well, it just takes the pll to do the modulation from that pipe
then).
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Chris Wilson]

On Mon, 23 Apr 2012 14:21:20 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> On my specs bit29 is pipe assignement, we should set it if the panel is on
> pipe B (well, it just takes the pll to do the modulation from that pipe
> then).

On CTL1 rather than CTL2, if that makes a difference. Listed in both the
IBX and CPT specs as bit 29, backlight polarity. The joy of ever
changing documentation.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Daniel Vetter]

On Mon, Apr 23, 2012 at 01:54:23PM +0200, Carsten Emde wrote:
> On 04/23/2012 11:32 AM, Daniel Vetter wrote:
> >There's a bit in the docs for gen4 only that says whether the
> >backlight control is inverted. And both the quirk we have and
> >all bugs only concern i965gm and gm45 (and mostly Acer) afaics.
> >
> >So lets drop the quirk and use the bit instead.
> >
> >Also clean up the BLC register definitions a bit by correctly
> >grouping the CTL and CTL2 definitions together.
> >[..]
> >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >index 4c844c6..5d215f0 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -6482,9 +6482,6 @@ static struct intel_quirk intel_quirks[] = {
> >
> >  	/* Sony Vaio Y cannot use SSC on LVDS */
> >  	{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
> >-
> >-	/* Acer Aspire 5734Z must invert backlight brightness */
> >-	{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness },
> Made a first test with these lines removed and the new mechanism
> added.  Unfortunately, the screen is now dark again. Works after
> re-adding the quirk.
> 
> >+	/* gen4 has a polarity bit */
> >+	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965))
> >+			return intel_panel_get_max_backlight(dev) - val;
> >+
> Apparently, these conditions are not met on the Acer Aspire 5734Z.
> Will add some debug output and evaluate further. For the time being,
> please leave the quirk in place.

Hm, I've tried this and when I set this bit, panel brightness is inverted
on my gm45. Can you install intel-gpu-tools and read out the 2 backlight
control registers with 

# intel_reg_read 0x61254
# intel_reg_read 0x61250

Thanks, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Daniel Vetter]

On Mon, Apr 23, 2012 at 02:32:57PM +0200, Daniel Vetter wrote:
> On Mon, Apr 23, 2012 at 01:54:23PM +0200, Carsten Emde wrote:
> > On 04/23/2012 11:32 AM, Daniel Vetter wrote:
> > >There's a bit in the docs for gen4 only that says whether the
> > >backlight control is inverted. And both the quirk we have and
> > >all bugs only concern i965gm and gm45 (and mostly Acer) afaics.
> > >
> > >So lets drop the quirk and use the bit instead.
> > >
> > >Also clean up the BLC register definitions a bit by correctly
> > >grouping the CTL and CTL2 definitions together.
> > >[..]
> > >diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > >index 4c844c6..5d215f0 100644
> > >--- a/drivers/gpu/drm/i915/intel_display.c
> > >+++ b/drivers/gpu/drm/i915/intel_display.c
> > >@@ -6482,9 +6482,6 @@ static struct intel_quirk intel_quirks[] = {
> > >
> > >  	/* Sony Vaio Y cannot use SSC on LVDS */
> > >  	{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
> > >-
> > >-	/* Acer Aspire 5734Z must invert backlight brightness */
> > >-	{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness },
> > Made a first test with these lines removed and the new mechanism
> > added.  Unfortunately, the screen is now dark again. Works after
> > re-adding the quirk.
> > 
> > >+	/* gen4 has a polarity bit */
> > >+	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965))
> > >+			return intel_panel_get_max_backlight(dev) - val;
> > >+
> > Apparently, these conditions are not met on the Acer Aspire 5734Z.
> > Will add some debug output and evaluate further. For the time being,
> > please leave the quirk in place.
> 
> Hm, I've tried this and when I set this bit, panel brightness is inverted
> on my gm45. Can you install intel-gpu-tools and read out the 2 backlight
> control registers with 
> 
> # intel_reg_read 0x61254
> # intel_reg_read 0x61250

I've forgotten to add: Also please grab these register values when booting
with nomodeset, so that we can compare the values the bios sets with
whatever i915 sets.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Carsten Emde]

On 04/23/2012 02:36 PM, Daniel Vetter wrote:
> On Mon, Apr 23, 2012 at 02:32:57PM +0200, Daniel Vetter wrote:
>> On Mon, Apr 23, 2012 at 01:54:23PM +0200, Carsten Emde wrote:
>>> On 04/23/2012 11:32 AM, Daniel Vetter wrote:
>>>> There's a bit in the docs for gen4 only that says whether the
>>>> backlight control is inverted. And both the quirk we have and
>>>> all bugs only concern i965gm and gm45 (and mostly Acer) afaics.
>>>>
>>>> So lets drop the quirk and use the bit instead.
>>>>
>>>> Also clean up the BLC register definitions a bit by correctly
>>>> grouping the CTL and CTL2 definitions together.
>>>> [..]
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>> index 4c844c6..5d215f0 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -6482,9 +6482,6 @@ static struct intel_quirk intel_quirks[] = {
>>>>
>>>>   	/* Sony Vaio Y cannot use SSC on LVDS */
>>>>   	{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
>>>> -
>>>> -	/* Acer Aspire 5734Z must invert backlight brightness */
>>>> -	{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness },
>>> Made a first test with these lines removed and the new mechanism
>>> added.  Unfortunately, the screen is now dark again. Works after
>>> re-adding the quirk.
>>>
>>>> +	/* gen4 has a polarity bit */
>>>> +	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965))
>>>> +			return intel_panel_get_max_backlight(dev) - val;
>>>> +
>>> Apparently, these conditions are not met on the Acer Aspire 5734Z.
>>> Will add some debug output and evaluate further. For the time being,
>>> please leave the quirk in place.
>>
>> Hm, I've tried this and when I set this bit, panel brightness is inverted
>> on my gm45. Can you install intel-gpu-tools and read out the 2 backlight
>> control registers with
>>
>> # intel_reg_read 0x61254
>> # intel_reg_read 0x61250
>
> I've forgotten to add: Also please grab these register values when booting
> with nomodeset, so that we can compare the values the bios sets with
> whatever i915 sets.

Without nomodeset:
# intel_reg_read 0x61254
0x61254 : 0xB4A0B4A
# intel_reg_read 0x61250
0x61250 : 0xE0000000

With nomodeset:
# intel_reg_read 0x61254
0x61254 : 0xB4A0B4A
# intel_reg_read 0x61250
0x61250 : 0xE0000000

Thus, (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965) is never set. But 
bit 29 would work, so we could add another conditional such as:
#define BLM_POLARITY_PCH_I965 (1 << 29) /* PCH only */
if (PCH)
   try (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_PCH_I965)

	-Carsten.

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Daniel Vetter]

On Mon, Apr 23, 2012 at 03:15:02PM +0200, Carsten Emde wrote:
> On 04/23/2012 02:36 PM, Daniel Vetter wrote:
> >On Mon, Apr 23, 2012 at 02:32:57PM +0200, Daniel Vetter wrote:
> >>On Mon, Apr 23, 2012 at 01:54:23PM +0200, Carsten Emde wrote:
> >>>On 04/23/2012 11:32 AM, Daniel Vetter wrote:
> >>>>There's a bit in the docs for gen4 only that says whether the
> >>>>backlight control is inverted. And both the quirk we have and
> >>>>all bugs only concern i965gm and gm45 (and mostly Acer) afaics.
> >>>>
> >>>>So lets drop the quirk and use the bit instead.
> >>>>
> >>>>Also clean up the BLC register definitions a bit by correctly
> >>>>grouping the CTL and CTL2 definitions together.
> >>>>[..]
> >>>>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>>>index 4c844c6..5d215f0 100644
> >>>>--- a/drivers/gpu/drm/i915/intel_display.c
> >>>>+++ b/drivers/gpu/drm/i915/intel_display.c
> >>>>@@ -6482,9 +6482,6 @@ static struct intel_quirk intel_quirks[] = {
> >>>>
> >>>>  	/* Sony Vaio Y cannot use SSC on LVDS */
> >>>>  	{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
> >>>>-
> >>>>-	/* Acer Aspire 5734Z must invert backlight brightness */
> >>>>-	{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness },
> >>>Made a first test with these lines removed and the new mechanism
> >>>added.  Unfortunately, the screen is now dark again. Works after
> >>>re-adding the quirk.
> >>>
> >>>>+	/* gen4 has a polarity bit */
> >>>>+	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965))
> >>>>+			return intel_panel_get_max_backlight(dev) - val;
> >>>>+
> >>>Apparently, these conditions are not met on the Acer Aspire 5734Z.
> >>>Will add some debug output and evaluate further. For the time being,
> >>>please leave the quirk in place.
> >>
> >>Hm, I've tried this and when I set this bit, panel brightness is inverted
> >>on my gm45. Can you install intel-gpu-tools and read out the 2 backlight
> >>control registers with
> >>
> >># intel_reg_read 0x61254
> >># intel_reg_read 0x61250
> >
> >I've forgotten to add: Also please grab these register values when booting
> >with nomodeset, so that we can compare the values the bios sets with
> >whatever i915 sets.
> 
> Without nomodeset:
> # intel_reg_read 0x61254
> 0x61254 : 0xB4A0B4A
> # intel_reg_read 0x61250
> 0x61250 : 0xE0000000
> 
> With nomodeset:
> # intel_reg_read 0x61254
> 0x61254 : 0xB4A0B4A
> # intel_reg_read 0x61250
> 0x61250 : 0xE0000000
> 
> Thus, (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965) is never set.
> But bit 29 would work, so we could add another conditional such as:
> #define BLM_POLARITY_PCH_I965 (1 << 29) /* PCH only */
> if (PCH)
>   try (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_PCH_I965)

Ok, let's try something funny. Please boot with kms enabled and try out
what the following values for CTL1 do:

# intel_reg_write 0x61250 0x80000000
# intel_reg_write 0x61250 0xa0000000
# intel_reg_write 0x61250 0x90000000
# intel_reg_write 0x61250 0xb0000000

Thanks, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Daniel Vetter]

On Mon, Apr 23, 2012 at 01:32:31PM +0100, Chris Wilson wrote:
> On Mon, 23 Apr 2012 14:21:20 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> > On my specs bit29 is pipe assignement, we should set it if the panel is on
> > pipe B (well, it just takes the pll to do the modulation from that pipe
> > then).
> 
> On CTL1 rather than CTL2, if that makes a difference. Listed in both the
> IBX and CPT specs as bit 29, backlight polarity. The joy of ever
> changing documentation.

Ah, in the PCH part, not the north display unit. I guess we have a little
mess here.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Carsten Emde]

On 04/23/2012 03:39 PM, Daniel Vetter wrote:
> On Mon, Apr 23, 2012 at 03:15:02PM +0200, Carsten Emde wrote:
>> On 04/23/2012 02:36 PM, Daniel Vetter wrote:
>>> On Mon, Apr 23, 2012 at 02:32:57PM +0200, Daniel Vetter wrote:
>>>> On Mon, Apr 23, 2012 at 01:54:23PM +0200, Carsten Emde wrote:
>>>>> On 04/23/2012 11:32 AM, Daniel Vetter wrote:
>>>>>> There's a bit in the docs for gen4 only that says whether the
>>>>>> backlight control is inverted. And both the quirk we have and
>>>>>> all bugs only concern i965gm and gm45 (and mostly Acer) afaics.
>>>>>>
>>>>>> So lets drop the quirk and use the bit instead.
>>>>>>
>>>>>> Also clean up the BLC register definitions a bit by correctly
>>>>>> grouping the CTL and CTL2 definitions together.
>>>>>> [..]
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>>>>> index 4c844c6..5d215f0 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>>>> @@ -6482,9 +6482,6 @@ static struct intel_quirk intel_quirks[] = {
>>>>>>
>>>>>>   	/* Sony Vaio Y cannot use SSC on LVDS */
>>>>>>   	{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
>>>>>> -
>>>>>> -	/* Acer Aspire 5734Z must invert backlight brightness */
>>>>>> -	{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness },
>>>>> Made a first test with these lines removed and the new mechanism
>>>>> added.  Unfortunately, the screen is now dark again. Works after
>>>>> re-adding the quirk.
>>>>>
>>>>>> +	/* gen4 has a polarity bit */
>>>>>> +	if (IS_GEN4(dev)&&  (I915_READ(BLC_PWM_CTL2)&  BLM_POLARITY_I965))
>>>>>> +			return intel_panel_get_max_backlight(dev) - val;
>>>>>> +
>>>>> Apparently, these conditions are not met on the Acer Aspire 5734Z.
>>>>> Will add some debug output and evaluate further. For the time being,
>>>>> please leave the quirk in place.
>>>>
>>>> Hm, I've tried this and when I set this bit, panel brightness is inverted
>>>> on my gm45. Can you install intel-gpu-tools and read out the 2 backlight
>>>> control registers with
>>>>
>>>> # intel_reg_read 0x61254
>>>> # intel_reg_read 0x61250
>>>
>>> I've forgotten to add: Also please grab these register values when booting
>>> with nomodeset, so that we can compare the values the bios sets with
>>> whatever i915 sets.
>>
>> Without nomodeset:
>> # intel_reg_read 0x61254
>> 0x61254 : 0xB4A0B4A
>> # intel_reg_read 0x61250
>> 0x61250 : 0xE0000000
>>
>> With nomodeset:
>> # intel_reg_read 0x61254
>> 0x61254 : 0xB4A0B4A
>> # intel_reg_read 0x61250
>> 0x61250 : 0xE0000000
>>
>> Thus, (I915_READ(BLC_PWM_CTL2)&  BLM_POLARITY_I965) is never set.
>> But bit 29 would work, so we could add another conditional such as:
>> #define BLM_POLARITY_PCH_I965 (1<<  29) /* PCH only */
>> if (PCH)
>>    try (I915_READ(BLC_PWM_CTL2)&  BLM_POLARITY_PCH_I965)
>
> Ok, let's try something funny. Please boot with kms enabled and try out
> what the following values for CTL1 do:
>
> # intel_reg_write 0x61250 0x80000000
> # intel_reg_write 0x61250 0xa0000000
> # intel_reg_write 0x61250 0x90000000
> # intel_reg_write 0x61250 0xb0000000

# intel_reg_write 0x61250 0x80000000
Value before: 0xE0000000
Value after: 0x80000000
# intel_reg_read 0x61254
0x61254 : 0xB4A0B4A

# intel_reg_write 0x61250 0xa0000000
Value before: 0x80000000
Value after: 0xA0000000
# intel_reg_read 0x61254
0x61254 : 0xB4A0B4A

# intel_reg_write 0x61250 0x90000000
Value before: 0xA0000000
Value after: 0x90000000
# intel_reg_read 0x61254
0x61254 : 0xB4A0B4A

# intel_reg_write 0x61250 0xb0000000
Value before: 0x90000000
Value after: 0xB0000000
# intel_reg_read 0x61254
0x61254 : 0xB4A0B4A

Not much.

	-Carsten.

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Daniel Vetter]

On Mon, Apr 23, 2012 at 04:00:23PM +0200, Carsten Emde wrote:
> # intel_reg_write 0x61250 0x80000000
> Value before: 0xE0000000
> Value after: 0x80000000
> # intel_reg_read 0x61254
> 0x61254 : 0xB4A0B4A
> 
> # intel_reg_write 0x61250 0xa0000000
> Value before: 0x80000000
> Value after: 0xA0000000
> # intel_reg_read 0x61254
> 0x61254 : 0xB4A0B4A
> 
> # intel_reg_write 0x61250 0x90000000
> Value before: 0xA0000000
> Value after: 0x90000000
> # intel_reg_read 0x61254
> 0x61254 : 0xB4A0B4A
> 
> # intel_reg_write 0x61250 0xb0000000
> Value before: 0x90000000
> Value after: 0xB0000000
> # intel_reg_read 0x61254
> 0x61254 : 0xB4A0B4A
> 
> Not much.

The idea was to boot with kms and see whether any of these values would
restore the backlight. Writing to CTL1 should change anything in CTL2.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Carsten Emde]

On 04/23/2012 04:22 PM, Daniel Vetter wrote:
> On Mon, Apr 23, 2012 at 04:00:23PM +0200, Carsten Emde wrote:
>> # intel_reg_write 0x61250 0x80000000
>> Value before: 0xE0000000
>> Value after: 0x80000000
>> # intel_reg_read 0x61254
>> 0x61254 : 0xB4A0B4A
>>
>> # intel_reg_write 0x61250 0xa0000000
>> Value before: 0x80000000
>> Value after: 0xA0000000
>> # intel_reg_read 0x61254
>> 0x61254 : 0xB4A0B4A
>>
>> # intel_reg_write 0x61250 0x90000000
>> Value before: 0xA0000000
>> Value after: 0x90000000
>> # intel_reg_read 0x61254
>> 0x61254 : 0xB4A0B4A
>>
>> # intel_reg_write 0x61250 0xb0000000
>> Value before: 0x90000000
>> Value after: 0xB0000000
>> # intel_reg_read 0x61254
>> 0x61254 : 0xB4A0B4A
>>
>> Not much.
>
> The idea was to boot with kms and see whether any of these values would
> restore the backlight. Writing to CTL1 should change anything in CTL2.
Ah, sorry, ok. Removed the quirk again and tested the various settings:

-> Initial screen: dark

# intel_reg_write 0x61250 0x80000000
Value before: 0xE0000000
Value after: 0x80000000
-> Still dark

# intel_reg_write 0x61250 0xa0000000
Value before: 0x80000000
Value after: 0xA0000000
-> Still dark

# intel_reg_write 0x61250 0x90000000
Value before: 0xA0000000
Value after: 0x90000000
-> BACKLIGHT ON!

# intel_reg_write 0x61250 0xb0000000
Value before: 0x90000000
Value after: 0xB0000000
-> Still ON.

	-Carsten.

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Daniel Vetter]

On Mon, Apr 23, 2012 at 05:06:53PM +0200, Carsten Emde wrote:
> On 04/23/2012 04:22 PM, Daniel Vetter wrote:
> >On Mon, Apr 23, 2012 at 04:00:23PM +0200, Carsten Emde wrote:
> >># intel_reg_write 0x61250 0x80000000
> >>Value before: 0xE0000000
> >>Value after: 0x80000000
> >># intel_reg_read 0x61254
> >>0x61254 : 0xB4A0B4A
> >>
> >># intel_reg_write 0x61250 0xa0000000
> >>Value before: 0x80000000
> >>Value after: 0xA0000000
> >># intel_reg_read 0x61254
> >>0x61254 : 0xB4A0B4A
> >>
> >># intel_reg_write 0x61250 0x90000000
> >>Value before: 0xA0000000
> >>Value after: 0x90000000
> >># intel_reg_read 0x61254
> >>0x61254 : 0xB4A0B4A
> >>
> >># intel_reg_write 0x61250 0xb0000000
> >>Value before: 0x90000000
> >>Value after: 0xB0000000
> >># intel_reg_read 0x61254
> >>0x61254 : 0xB4A0B4A
> >>
> >>Not much.
> >
> >The idea was to boot with kms and see whether any of these values would
> >restore the backlight. Writing to CTL1 should change anything in CTL2.
> Ah, sorry, ok. Removed the quirk again and tested the various settings:
> 
> -> Initial screen: dark
> 
> # intel_reg_write 0x61250 0x80000000
> Value before: 0xE0000000
> Value after: 0x80000000
> -> Still dark
> 
> # intel_reg_write 0x61250 0xa0000000
> Value before: 0x80000000
> Value after: 0xA0000000
> -> Still dark
> 
> # intel_reg_write 0x61250 0x90000000
> Value before: 0xA0000000
> Value after: 0x90000000
> -> BACKLIGHT ON!
> 
> # intel_reg_write 0x61250 0xb0000000
> Value before: 0x90000000
> Value after: 0xB0000000
> -> Still ON.

Neat. Let's test two more:

# intel_reg_write 0x61250 0xd0000000
# intel_reg_write 0x61250 0xc0000000

-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Carsten Emde]

On 04/23/2012 05:22 PM, Daniel Vetter wrote:
> On Mon, Apr 23, 2012 at 05:06:53PM +0200, Carsten Emde wrote:
>> On 04/23/2012 04:22 PM, Daniel Vetter wrote:
>>> On Mon, Apr 23, 2012 at 04:00:23PM +0200, Carsten Emde wrote:
>>>  [..]
>>> The idea was to boot with kms and see whether any of these values would
>>> restore the backlight. Writing to CTL1 should change anything in CTL2.
>> Ah, sorry, ok. Removed the quirk again and tested the various settings:
>>
>> ->  Initial screen: dark
>>
>> # intel_reg_write 0x61250 0x80000000
>> Value before: 0xE0000000
>> Value after: 0x80000000
>> ->  Still dark
>>
>> # intel_reg_write 0x61250 0xa0000000
>> Value before: 0x80000000
>> Value after: 0xA0000000
>> ->  Still dark
>>
>> # intel_reg_write 0x61250 0x90000000
>> Value before: 0xA0000000
>> Value after: 0x90000000
>> ->  BACKLIGHT ON!
>>
>> # intel_reg_write 0x61250 0xb0000000
>> Value before: 0x90000000
>> Value after: 0xB0000000
>> ->  Still ON.
>
> Neat. Let's test two more:
>
> # intel_reg_write 0x61250 0xd0000000
> # intel_reg_write 0x61250 0xc0000000
Here we go.

->  Initial screen: dark

# intel_reg_write 0x61250 0xd0000000
Value before: 0xE0000000
Value after: 0xD0000000
->  BACKLIGHT ON!

# intel_reg_write 0x61250 0xc0000000
Value before: 0xD0000000
Value after: 0xC0000000
->  Dark again.

	-Carsten.

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Daniel Vetter]

On Mon, Apr 23, 2012 at 05:38:27PM +0200, Carsten Emde wrote:
> On 04/23/2012 05:22 PM, Daniel Vetter wrote:
> >On Mon, Apr 23, 2012 at 05:06:53PM +0200, Carsten Emde wrote:
> >>On 04/23/2012 04:22 PM, Daniel Vetter wrote:
> >>>On Mon, Apr 23, 2012 at 04:00:23PM +0200, Carsten Emde wrote:
> >>> [..]
> >>>The idea was to boot with kms and see whether any of these values would
> >>>restore the backlight. Writing to CTL1 should change anything in CTL2.
> >>Ah, sorry, ok. Removed the quirk again and tested the various settings:
> >>
> >>->  Initial screen: dark
> >>
> >># intel_reg_write 0x61250 0x80000000
> >>Value before: 0xE0000000
> >>Value after: 0x80000000
> >>->  Still dark
> >>
> >># intel_reg_write 0x61250 0xa0000000
> >>Value before: 0x80000000
> >>Value after: 0xA0000000
> >>->  Still dark
> >>
> >># intel_reg_write 0x61250 0x90000000
> >>Value before: 0xA0000000
> >>Value after: 0x90000000
> >>->  BACKLIGHT ON!
> >>
> >># intel_reg_write 0x61250 0xb0000000
> >>Value before: 0x90000000
> >>Value after: 0xB0000000
> >>->  Still ON.
> >
> >Neat. Let's test two more:
> >
> ># intel_reg_write 0x61250 0xd0000000
> ># intel_reg_write 0x61250 0xc0000000
> Here we go.
> 
> ->  Initial screen: dark
> 
> # intel_reg_write 0x61250 0xd0000000
> Value before: 0xE0000000
> Value after: 0xD0000000
> ->  BACKLIGHT ON!
> 
> # intel_reg_write 0x61250 0xc0000000
> Value before: 0xD0000000
> Value after: 0xC0000000
> ->  Dark again.

Ok, so the polarity bit does work as advertised. But I still don't
understand how your machine works, so assuming your machine has backlight
control keys, please boot with kms disabled and grab the both registers
values for maximal brightness and minimal brightness. Then reconfigure the
backlight with

# intel_reg_write 0x61250 0x90000000

and check how the behaviour changes. Somehow we lose something when
setting up drm/i915, but atm I can't figure out what yet.

Thanks, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 1/2] drm/i915: clear up backlight inversion confusion on gen4

[Carsten Emde]

On 04/23/2012 05:56 PM, Daniel Vetter wrote:
> On Mon, Apr 23, 2012 at 05:38:27PM +0200, Carsten Emde wrote:
>> On 04/23/2012 05:22 PM, Daniel Vetter wrote:
>>> On Mon, Apr 23, 2012 at 05:06:53PM +0200, Carsten Emde wrote:
>>>> On 04/23/2012 04:22 PM, Daniel Vetter wrote:
>>>>> On Mon, Apr 23, 2012 at 04:00:23PM +0200, Carsten Emde wrote:
>>>>> [..]
>>>>> The idea was to boot with kms and see whether any of these values would
>>>>> restore the backlight. Writing to CTL1 should change anything in CTL2.
>>>> Ah, sorry, ok. Removed the quirk again and tested the various settings:
>>>>
>>>> ->   Initial screen: dark
>>>>
>>>> # intel_reg_write 0x61250 0x80000000
>>>> Value before: 0xE0000000
>>>> Value after: 0x80000000
>>>> ->   Still dark
>>>>
>>>> # intel_reg_write 0x61250 0xa0000000
>>>> Value before: 0x80000000
>>>> Value after: 0xA0000000
>>>> ->   Still dark
>>>>
>>>> # intel_reg_write 0x61250 0x90000000
>>>> Value before: 0xA0000000
>>>> Value after: 0x90000000
>>>> ->   BACKLIGHT ON!
>>>>
>>>> # intel_reg_write 0x61250 0xb0000000
>>>> Value before: 0x90000000
>>>> Value after: 0xB0000000
>>>> ->   Still ON.
>>>
>>> Neat. Let's test two more:
>>>
>>> # intel_reg_write 0x61250 0xd0000000
>>> # intel_reg_write 0x61250 0xc0000000
>> Here we go.
>>
>> ->   Initial screen: dark
>>
>> # intel_reg_write 0x61250 0xd0000000
>> Value before: 0xE0000000
>> Value after: 0xD0000000
>> ->   BACKLIGHT ON!
>>
>> # intel_reg_write 0x61250 0xc0000000
>> Value before: 0xD0000000
>> Value after: 0xC0000000
>> ->   Dark again.
>
> Ok, so the polarity bit does work as advertised. But I still don't
> understand how your machine works, so assuming your machine has backlight
> control keys,
Yes it has, but they don't have any effect.

Unfortunately, I have to leave now and will be away from the lab for two 
days. Will continue to work on the problem when I'll be back. For the 
time being, I let the two conditions, your new patch and the old quirk, 
coexist in the code.

Thanks,
	-Carsten.

[PATCH 0/4] drm/i915: Re: clear up backlight inversion confusion on gen4

[Carsten Emde]

[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]

On 04/23/2012 06:55 PM, Carsten Emde wrote:
> On 04/23/2012 05:56 PM, Daniel Vetter wrote:
>> On Mon, Apr 23, 2012 at 05:38:27PM +0200, Carsten Emde wrote:
>>> On 04/23/2012 05:22 PM, Daniel Vetter wrote:
>> Ok, so the polarity bit does work as advertised. But I still don't
>> understand how your machine works,
Let's go ahead and summarize what we have up to now.

1. With KMS enabled, the backlight panel of the Acer Aspire 5734Z remains dark.

2. Further evaluation showed that the brightness is inverted on this machine,
i.e. while setting the Legacy Backlight Brightness (LBB) register to 0x0
normally causes the backlight to be turned off, and 0xFF causes the backlight
to be set to 100% intensity, the Acer Aspire 5734Z turns the backlight off at
0xFF and sets it to maximum intensity at 0.

3. In a first step, a quirk was introduced to cope with this particular oddity.

4. Daniel Vetter found out there is a bit in the 2nd backlight control register
(BLC_PWM_CTL2) that indicates panel backlight brightness is inverted.

5. On the Acer Aspire 5734Z, however, this bit is not set.

6. Chris Wilson found out that Daniel's bit #28 sometimes could be bit #29.

7. On the Aspire 5734Z, bit #29 is set.

As a conclusion, I have prepared a patch series that
- uses Danieĺ's patch to invert brightness, if required,
- reverts the quirk to invert backlight brightness,
- introduces a new quirk to indicate bit #29 instead of #28 is used,
- marks the Acer Aspire 5734Z to use the quirked bit.

With these patches applied, the Acer Aspire 5734Z works.

        -Carsten.


[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

[PATCH 1/4] drm/i915: clear up backlight inversion confusion on gen4

[Carsten Emde]

[-- Attachment #1: drivers-gpu-drm-i915-invert-brightness-if-required.patch --]
[-- Type: text/plain, Size: 3144 bytes --]

There's a bit in the docs for gen4 only that says whether the
backlight control is inverted. And both the quirk we have and
all bugs only concern i965gm and gm45 (and mostly Acer) afaics.

So lets drop the quirk and use the bit instead.

Also clean up the BLC register definitions a bit by correctly
grouping the CTL and CTL2 definitions together.

This quirk was originally added in

commit 5a15ab5b93e4a3ebcd4fa6c76cf646a45e9cf806
Author: Carsten Emde <C.Emde@osadl.org>
Date:   Thu Mar 15 15:56:27 2012 +0100

    drm/i915: panel: invert brightness acer aspire 5734z

References: https://bugzilla.kernel.org/show_bug.cgi?id=31522
References: https://bugs.freedesktop.org/show_bug.cgi?id=37986
References: https://bugs.freedesktop.org/show_bug.cgi?id=40455

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Carsten Emde <C.Emde@osadl.org>

---
 drivers/gpu/drm/i915/i915_reg.h      |    7 ++++---
 drivers/gpu/drm/i915/intel_display.c |    3 ---
 drivers/gpu/drm/i915/intel_panel.c   |    4 ++++
 3 files changed, 8 insertions(+), 6 deletions(-)

Index: linux-tip/drivers/gpu/drm/i915/i915_reg.h
===================================================================
--- linux-tip.orig/drivers/gpu/drm/i915/i915_reg.h
+++ linux-tip/drivers/gpu/drm/i915/i915_reg.h
@@ -1683,16 +1683,17 @@
 #define PFIT_AUTO_RATIOS 0x61238
 
 /* Backlight control */
-#define BLC_PWM_CTL		0x61254
-#define   BACKLIGHT_MODULATION_FREQ_SHIFT		(17)
 #define BLC_PWM_CTL2		0x61250 /* 965+ only */
-#define   BLM_COMBINATION_MODE (1 << 30)
+#define   BLM_COMBINATION_MODE	(1 << 30)
+#define   BLM_POLARITY_I965	(1 << 28) /* gen4 only */
+#define BLC_PWM_CTL		0x61254
 /*
  * This is the most significant 15 bits of the number of backlight cycles in a
  * complete cycle of the modulated backlight control.
  *
  * The actual value is this field multiplied by two.
  */
+#define   BACKLIGHT_MODULATION_FREQ_SHIFT		(17)
 #define   BACKLIGHT_MODULATION_FREQ_MASK		(0x7fff << 17)
 #define   BLM_LEGACY_MODE				(1 << 16)
 /*
Index: linux-tip/drivers/gpu/drm/i915/intel_display.c
===================================================================
--- linux-tip.orig/drivers/gpu/drm/i915/intel_display.c
+++ linux-tip/drivers/gpu/drm/i915/intel_display.c
@@ -9143,9 +9143,6 @@ struct intel_quirk intel_quirks[] = {
 
 	/* Sony Vaio Y cannot use SSC on LVDS */
 	{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
-
-	/* Acer Aspire 5734Z must invert backlight brightness */
-	{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness },
 };
 
 static void intel_init_quirks(struct drm_device *dev)
Index: linux-tip/drivers/gpu/drm/i915/intel_panel.c
===================================================================
--- linux-tip.orig/drivers/gpu/drm/i915/intel_panel.c
+++ linux-tip/drivers/gpu/drm/i915/intel_panel.c
@@ -208,6 +208,10 @@ static u32 intel_panel_compute_brightnes
 	    dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS)
 		return intel_panel_get_max_backlight(dev) - val;
 
+	/* gen4 has a polarity bit */
+	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965))
+			return intel_panel_get_max_backlight(dev) - val;
+
 	return val;
 }

[PATCH 2/4] drm/i915: completely revert the invert brightness quirk

[Carsten Emde]

[-- Attachment #1: drivers-gpu-drm-i915-revert-invert-brightness-quirk.patch --]
[-- Type: text/plain, Size: 1391 bytes --]

The quirk to indicate that the panel backlight brightness is
inverted is not longer needed. Remove it.

Signed-off-by: Carsten Emde <C.Emde@osadl.org>

---
 drivers/gpu/drm/i915/i915_drv.h      |    1 -
 drivers/gpu/drm/i915/intel_display.c |   10 ----------
 2 files changed, 11 deletions(-)

Index: linux-tip/drivers/gpu/drm/i915/i915_drv.h
===================================================================
--- linux-tip.orig/drivers/gpu/drm/i915/i915_drv.h
+++ linux-tip/drivers/gpu/drm/i915/i915_drv.h
@@ -295,7 +295,6 @@ enum intel_pch {
 
 #define QUIRK_PIPEA_FORCE (1<<0)
 #define QUIRK_LVDS_SSC_DISABLE (1<<1)
-#define QUIRK_INVERT_BRIGHTNESS (1<<2)
 
 struct intel_fbdev;
 struct intel_fbc_work;
Index: linux-tip/drivers/gpu/drm/i915/intel_display.c
===================================================================
--- linux-tip.orig/drivers/gpu/drm/i915/intel_display.c
+++ linux-tip/drivers/gpu/drm/i915/intel_display.c
@@ -9101,16 +9101,6 @@ static void quirk_ssc_force_disable(stru
 	dev_priv->quirks |= QUIRK_LVDS_SSC_DISABLE;
 }
 
-/*
- * A machine (e.g. Acer Aspire 5734Z) may need to invert the panel backlight
- * brightness value
- */
-static void quirk_invert_brightness(struct drm_device *dev)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	dev_priv->quirks |= QUIRK_INVERT_BRIGHTNESS;
-}
-
 struct intel_quirk {
 	int device;
 	int subsystem_vendor;

[PATCH 3/4] drm/i915: add quirk to indicate that an alt bit is used for brightness inversion

[Carsten Emde]

[-- Attachment #1: drivers-gpu-drm-i915-add-quirk-to-check-alt-bit-for-backlight-brightness-inversion.patch --]
[-- Type: text/plain, Size: 3010 bytes --]

Bit #28 of the backlight control BLC_PWM_CTL2 is normally used
to indicate whether the brightness of the panel backlight is
inverted. Sometimes, however, bit #29 is used for this purpose.
Add a quirk to mark machines that do so.

Signed-off-by: Carsten Emde <C.Emde@osadl.org>

---
 drivers/gpu/drm/i915/i915_drv.h      |    1 +
 drivers/gpu/drm/i915/i915_reg.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |   10 ++++++++++
 drivers/gpu/drm/i915/intel_panel.c   |   12 ++++++++++--
 4 files changed, 22 insertions(+), 2 deletions(-)

Index: linux-tip/drivers/gpu/drm/i915/i915_drv.h
===================================================================
--- linux-tip.orig/drivers/gpu/drm/i915/i915_drv.h
+++ linux-tip/drivers/gpu/drm/i915/i915_drv.h
@@ -295,6 +295,7 @@ enum intel_pch {
 
 #define QUIRK_PIPEA_FORCE (1<<0)
 #define QUIRK_LVDS_SSC_DISABLE (1<<1)
+#define QUIRK_ALT_BIT_FOR_BRIGHTNESS_INVERSION (2<<1)
 
 struct intel_fbdev;
 struct intel_fbc_work;
Index: linux-tip/drivers/gpu/drm/i915/i915_reg.h
===================================================================
--- linux-tip.orig/drivers/gpu/drm/i915/i915_reg.h
+++ linux-tip/drivers/gpu/drm/i915/i915_reg.h
@@ -1685,6 +1685,7 @@
 /* Backlight control */
 #define BLC_PWM_CTL2		0x61250 /* 965+ only */
 #define   BLM_COMBINATION_MODE	(1 << 30)
+#define   BLM_ALT_POLARITY_I965	(1 << 29) /* quirk only */
 #define   BLM_POLARITY_I965	(1 << 28) /* gen4 only */
 #define BLC_PWM_CTL		0x61254
 /*
Index: linux-tip/drivers/gpu/drm/i915/intel_display.c
===================================================================
--- linux-tip.orig/drivers/gpu/drm/i915/intel_display.c
+++ linux-tip/drivers/gpu/drm/i915/intel_display.c
@@ -9101,6 +9101,16 @@ static void quirk_ssc_force_disable(stru
 	dev_priv->quirks |= QUIRK_LVDS_SSC_DISABLE;
 }
 
+/*
+ * A machine may use an alternate bit to indicate panel backlight brightness
+ * inversion
+ */
+static void quirk_alt_bit_for_brightness_inversion(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	dev_priv->quirks |= QUIRK_ALT_BIT_FOR_BRIGHTNESS_INVERSION;
+}
+
 struct intel_quirk {
 	int device;
 	int subsystem_vendor;
Index: linux-tip/drivers/gpu/drm/i915/intel_panel.c
===================================================================
--- linux-tip.orig/drivers/gpu/drm/i915/intel_panel.c
+++ linux-tip/drivers/gpu/drm/i915/intel_panel.c
@@ -208,9 +208,17 @@ static u32 intel_panel_compute_brightnes
 	    dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS)
 		return intel_panel_get_max_backlight(dev) - val;
 
-	/* gen4 has a polarity bit */
-	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965))
+	if (IS_GEN4(dev)) {
+		/* gen4 has a - possibly quirked - polarity bit */
+		int mask;
+
+		if (dev_priv->quirks & QUIRK_ALT_BIT_FOR_BRIGHTNESS_INVERSION)
+			mask = BLM_ALT_POLARITY_I965;
+		else
+			mask = BLM_POLARITY_I965;
+		if (I915_READ(BLC_PWM_CTL2) & mask)
 			return intel_panel_get_max_backlight(dev) - val;
+	}
 
 	return val;
 }

[PATCH 4/4] drm/i915: assign the brightness inversion quirk to Acer Aspire 5734Z

[Carsten Emde]

[-- Attachment #1: drivers-gpu-drm-i915-panel-quirk-alt-bit-brightness-inversion-acer-aspire-5734z.patch --]
[-- Type: text/plain, Size: 919 bytes --]

The Acer Aspire 5734Z uses bit #29 instead of bit #28 to indicate that
the panel backlight brightness is inverted. Assign it to the related
quirk.

Signed-off-by: Carsten Emde <C.Emde@osadl.org>

---
 drivers/gpu/drm/i915/intel_display.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-tip/drivers/gpu/drm/i915/intel_display.c
===================================================================
--- linux-tip.orig/drivers/gpu/drm/i915/intel_display.c
+++ linux-tip/drivers/gpu/drm/i915/intel_display.c
@@ -9143,6 +9143,12 @@ struct intel_quirk intel_quirks[] = {
 
 	/* Sony Vaio Y cannot use SSC on LVDS */
 	{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },
+
+	/*
+	 * Acer Aspire 5734Z uses an alternate bit to indicate
+	 * panel backlight brightness inversion
+	 */
+	{ 0x2a42, 0x1025, 0x0459, quirk_alt_bit_for_brightness_inversion },
 };
 
 static void intel_init_quirks(struct drm_device *dev)

Re: [PATCH 3/4] drm/i915: add quirk to indicate that an alt bit is used for brightness inversion

[Daniel Vetter]

On Thu, Apr 26, 2012 at 06:48:36PM +0200, Carsten Emde wrote:
> Bit #28 of the backlight control BLC_PWM_CTL2 is normally used
> to indicate whether the brightness of the panel backlight is
> inverted. Sometimes, however, bit #29 is used for this purpose.
> Add a quirk to mark machines that do so.
> 
> Signed-off-by: Carsten Emde <C.Emde@osadl.org>
> 
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    1 +
>  drivers/gpu/drm/i915/i915_reg.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |   10 ++++++++++
>  drivers/gpu/drm/i915/intel_panel.c   |   12 ++++++++++--
>  4 files changed, 22 insertions(+), 2 deletions(-)
> 
> Index: linux-tip/drivers/gpu/drm/i915/i915_drv.h
> ===================================================================
> --- linux-tip.orig/drivers/gpu/drm/i915/i915_drv.h
> +++ linux-tip/drivers/gpu/drm/i915/i915_drv.h
> @@ -295,6 +295,7 @@ enum intel_pch {
>  
>  #define QUIRK_PIPEA_FORCE (1<<0)
>  #define QUIRK_LVDS_SSC_DISABLE (1<<1)
> +#define QUIRK_ALT_BIT_FOR_BRIGHTNESS_INVERSION (2<<1)
>  
>  struct intel_fbdev;
>  struct intel_fbc_work;
> Index: linux-tip/drivers/gpu/drm/i915/i915_reg.h
> ===================================================================
> --- linux-tip.orig/drivers/gpu/drm/i915/i915_reg.h
> +++ linux-tip/drivers/gpu/drm/i915/i915_reg.h
> @@ -1685,6 +1685,7 @@
>  /* Backlight control */
>  #define BLC_PWM_CTL2		0x61250 /* 965+ only */
>  #define   BLM_COMBINATION_MODE	(1 << 30)
> +#define   BLM_ALT_POLARITY_I965	(1 << 29) /* quirk only */

bit29 is the pipe select bit, i.e. the backlight controller uses the pixel
clock from this pipe as the signal to module the backlight.

I need to look some more into this.
-Daniel


>  #define   BLM_POLARITY_I965	(1 << 28) /* gen4 only */
>  #define BLC_PWM_CTL		0x61254
>  /*
> Index: linux-tip/drivers/gpu/drm/i915/intel_display.c
> ===================================================================
> --- linux-tip.orig/drivers/gpu/drm/i915/intel_display.c
> +++ linux-tip/drivers/gpu/drm/i915/intel_display.c
> @@ -9101,6 +9101,16 @@ static void quirk_ssc_force_disable(stru
>  	dev_priv->quirks |= QUIRK_LVDS_SSC_DISABLE;
>  }
>  
> +/*
> + * A machine may use an alternate bit to indicate panel backlight brightness
> + * inversion
> + */
> +static void quirk_alt_bit_for_brightness_inversion(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	dev_priv->quirks |= QUIRK_ALT_BIT_FOR_BRIGHTNESS_INVERSION;
> +}
> +
>  struct intel_quirk {
>  	int device;
>  	int subsystem_vendor;
> Index: linux-tip/drivers/gpu/drm/i915/intel_panel.c
> ===================================================================
> --- linux-tip.orig/drivers/gpu/drm/i915/intel_panel.c
> +++ linux-tip/drivers/gpu/drm/i915/intel_panel.c
> @@ -208,9 +208,17 @@ static u32 intel_panel_compute_brightnes
>  	    dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS)
>  		return intel_panel_get_max_backlight(dev) - val;
>  
> -	/* gen4 has a polarity bit */
> -	if (IS_GEN4(dev) && (I915_READ(BLC_PWM_CTL2) & BLM_POLARITY_I965))
> +	if (IS_GEN4(dev)) {
> +		/* gen4 has a - possibly quirked - polarity bit */
> +		int mask;
> +
> +		if (dev_priv->quirks & QUIRK_ALT_BIT_FOR_BRIGHTNESS_INVERSION)
> +			mask = BLM_ALT_POLARITY_I965;
> +		else
> +			mask = BLM_POLARITY_I965;
> +		if (I915_READ(BLC_PWM_CTL2) & mask)
>  			return intel_panel_get_max_backlight(dev) - val;
> +	}
>  
>  	return val;
>  }
> 

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

[PATCH] properly enable the blc controller on the right pipe

[Daniel Vetter]

Depending upon how things are set up, this might help. If not, please
install intel-gpu-tools and grab the output of intel_reg_dumper both
when booting with nomodeset and when booting with kms.

Thanks, Daniel
---
 drivers/gpu/drm/i915/intel_drv.h   |    3 ++-
 drivers/gpu/drm/i915/intel_lvds.c  |    3 ++-
 drivers/gpu/drm/i915/intel_panel.c |   15 ++++++++++++++-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 715afa1..5a021ce 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -323,7 +323,8 @@ extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
 extern u32 intel_panel_get_backlight(struct drm_device *dev);
 extern void intel_panel_set_backlight(struct drm_device *dev, u32 level);
 extern int intel_panel_setup_backlight(struct drm_device *dev);
-extern void intel_panel_enable_backlight(struct drm_device *dev);
+extern void intel_panel_enable_backlight(struct drm_device *dev,
+					 enum pipe pipe);
 extern void intel_panel_disable_backlight(struct drm_device *dev);
 extern void intel_panel_destroy_backlight(struct drm_device *dev);
 extern enum drm_connector_status intel_panel_detect(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 30e2c82..3e71b86 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -71,6 +71,7 @@ static struct intel_lvds *intel_attached_lvds(struct drm_connector *connector)
 static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 {
 	struct drm_device *dev = intel_lvds->base.base.dev;
+	struct intel_crtc *intel_crtc = to_intel_crtc(intel_lvds->base.base.crtc);
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 ctl_reg, lvds_reg, stat_reg;
 
@@ -107,7 +108,7 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 	if (wait_for((I915_READ(stat_reg) & PP_ON) != 0, 1000))
 		DRM_ERROR("timed out waiting for panel to power on\n");
 
-	intel_panel_enable_backlight(dev);
+	intel_panel_enable_backlight(dev, intel_crtc->pipe);
 }
 
 static void intel_lvds_disable(struct intel_lvds *intel_lvds)
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 48177ec..486cf92 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -261,17 +261,30 @@ void intel_panel_disable_backlight(struct drm_device *dev)
 
 	dev_priv->backlight_enabled = false;
 	intel_panel_actually_set_backlight(dev, 0);
+
+	/*disable the blc*/
+	I915_WRITE(BLC_PWM_CTL2,
+		   I915_READ(BLC_PWM_CTL2) & ~(1<<31));
 }
 
-void intel_panel_enable_backlight(struct drm_device *dev)
+void intel_panel_enable_backlight(struct drm_device *dev,
+				  enum pipe pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 tmp;
 
 	if (dev_priv->backlight_level == 0)
 		dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
 
 	dev_priv->backlight_enabled = true;
 	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
+
+	/*enable the blc on the right pipe*/
+	tmp = I915_READ(BLC_PWM_CTL2);
+	tmp &= ~(1<<29); /* pipe select */
+	tmp |= pipe == PIPE_A ? 0 : 1;
+	I915_WRITE(BLC_PWM_CTL2,
+		   I915_READ(BLC_PWM_CTL2) & ~(1<<31));
 }
 
 static void intel_panel_init_backlight(struct drm_device *dev)
-- 
1.7.10

Re: [PATCH] properly enable the blc controller on the right pipe

[Daniel Vetter]

On Thu, Apr 26, 2012 at 07:25:32PM +0200, Daniel Vetter wrote:
> Depending upon how things are set up, this might help. If not, please
> install intel-gpu-tools and grab the output of intel_reg_dumper both
> when booting with nomodeset and when booting with kms.
> 
> Thanks, Daniel

btw, all these stuff is in the public documentation at

http://intellinuxgraphics.org/documentation.html

The backlight stuff is in volume three: display registers.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_drv.h   |    3 ++-
>  drivers/gpu/drm/i915/intel_lvds.c  |    3 ++-
>  drivers/gpu/drm/i915/intel_panel.c |   15 ++++++++++++++-
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 715afa1..5a021ce 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -323,7 +323,8 @@ extern u32 intel_panel_get_max_backlight(struct drm_device *dev);
>  extern u32 intel_panel_get_backlight(struct drm_device *dev);
>  extern void intel_panel_set_backlight(struct drm_device *dev, u32 level);
>  extern int intel_panel_setup_backlight(struct drm_device *dev);
> -extern void intel_panel_enable_backlight(struct drm_device *dev);
> +extern void intel_panel_enable_backlight(struct drm_device *dev,
> +					 enum pipe pipe);
>  extern void intel_panel_disable_backlight(struct drm_device *dev);
>  extern void intel_panel_destroy_backlight(struct drm_device *dev);
>  extern enum drm_connector_status intel_panel_detect(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 30e2c82..3e71b86 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -71,6 +71,7 @@ static struct intel_lvds *intel_attached_lvds(struct drm_connector *connector)
>  static void intel_lvds_enable(struct intel_lvds *intel_lvds)
>  {
>  	struct drm_device *dev = intel_lvds->base.base.dev;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(intel_lvds->base.base.crtc);
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 ctl_reg, lvds_reg, stat_reg;
>  
> @@ -107,7 +108,7 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
>  	if (wait_for((I915_READ(stat_reg) & PP_ON) != 0, 1000))
>  		DRM_ERROR("timed out waiting for panel to power on\n");
>  
> -	intel_panel_enable_backlight(dev);
> +	intel_panel_enable_backlight(dev, intel_crtc->pipe);
>  }
>  
>  static void intel_lvds_disable(struct intel_lvds *intel_lvds)
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 48177ec..486cf92 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -261,17 +261,30 @@ void intel_panel_disable_backlight(struct drm_device *dev)
>  
>  	dev_priv->backlight_enabled = false;
>  	intel_panel_actually_set_backlight(dev, 0);
> +
> +	/*disable the blc*/
> +	I915_WRITE(BLC_PWM_CTL2,
> +		   I915_READ(BLC_PWM_CTL2) & ~(1<<31));
>  }
>  
> -void intel_panel_enable_backlight(struct drm_device *dev)
> +void intel_panel_enable_backlight(struct drm_device *dev,
> +				  enum pipe pipe)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 tmp;
>  
>  	if (dev_priv->backlight_level == 0)
>  		dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
>  
>  	dev_priv->backlight_enabled = true;
>  	intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
> +
> +	/*enable the blc on the right pipe*/
> +	tmp = I915_READ(BLC_PWM_CTL2);
> +	tmp &= ~(1<<29); /* pipe select */
> +	tmp |= pipe == PIPE_A ? 0 : 1;
> +	I915_WRITE(BLC_PWM_CTL2,
> +		   I915_READ(BLC_PWM_CTL2) & ~(1<<31));
>  }
>  
>  static void intel_panel_init_backlight(struct drm_device *dev)
> -- 
> 1.7.10
> 

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] properly enable the blc controller on the right pipe

[Carsten Emde]

Daniel,

> Depending upon how things are set up, this might help.
Yeah, thanks a lot, works great now!

I removed patch 3/4 and 4/4 and pushed this one; so currently applied are:
[PATCH 1/4] drm/i915: clear up backlight inversion confusion on gen4
[PATCH 2/4] drm/i915: completely revert the invert brightness quirk
[PATCH] properly enable the blc controller on the right pipe

Can you pick these three patches?

Thanks,
	-Carsten.

Re: [PATCH] properly enable the blc controller on the right pipe

[Daniel Vetter]

On Thu, Apr 26, 2012 at 09:12:16PM +0200, Carsten Emde wrote:
> Daniel,
> 
> >Depending upon how things are set up, this might help.
> Yeah, thanks a lot, works great now!
> 
> I removed patch 3/4 and 4/4 and pushed this one; so currently applied are:
> [PATCH 1/4] drm/i915: clear up backlight inversion confusion on gen4
> [PATCH 2/4] drm/i915: completely revert the invert brightness quirk
> [PATCH] properly enable the blc controller on the right pipe
> 
> Can you pick these three patches?

Hey, that was just a quick hack to check things ;-) Can you please also
check what happens when you drop the first patch "clear up backlight
inversion confusion"? Hopefully that still works ...
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 0/4] drm/i915: Re: clear up backlight inversion confusion on gen4

[Monark Gondaliya]

[-- Attachment #1.1: Type: text/plain, Size: 327 bytes --]

Hello All,
   I have given one hardware that is D2plug of marvell .it's running
completely .but now i have to write Test application in C launguge.
   (1) UART test in C
   (2) NANDflash test in C
   (3) microSD test in C
   (4) SATA test in C
   (5) Ethernet test in C


so plz anybody can help me .......'
'


Regards
Monark

[-- Attachment #1.2: Type: text/html, Size: 814 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [PATCH] properly enable the blc controller on the right pipe

[Carsten Emde]

Daniel,

>>> Depending upon how things are set up, this might help.
>> Yeah, thanks a lot, works great now!
>> I removed patch 3/4 and 4/4 and pushed this one; so currently applied are:
>> [PATCH 1/4] drm/i915: clear up backlight inversion confusion on gen4
>> [PATCH 2/4] drm/i915: completely revert the invert brightness quirk
>> [PATCH] properly enable the blc controller on the right pipe
> Hey, that was just a quick hack to check things ;-)
Sorry, too many things in parallel.

Spent some time reading the docs you mentioned - will continue to work 
on the backlight brightness problem and prepare another patch. I just 
need some more time.

Thanks,
	-Carsten.

Re: [PATCH] properly enable the blc controller on the right pipe

[Daniel Vetter]

On Fri, Apr 27, 2012 at 10:18:08PM +0200, Carsten Emde wrote:
> Daniel,
> 
> >>>Depending upon how things are set up, this might help.
> >>Yeah, thanks a lot, works great now!
> >>I removed patch 3/4 and 4/4 and pushed this one; so currently applied are:
> >>[PATCH 1/4] drm/i915: clear up backlight inversion confusion on gen4
> >>[PATCH 2/4] drm/i915: completely revert the invert brightness quirk
> >>[PATCH] properly enable the blc controller on the right pipe
> >Hey, that was just a quick hack to check things ;-)
> Sorry, too many things in parallel.
> 
> Spent some time reading the docs you mentioned - will continue to
> work on the backlight brightness problem and prepare another patch.
> I just need some more time.

Ok, I've stitched together some real patches and actually tested them.
While doing so I've noticed that the patch you've tested essentially
disables the i915 backlight control and leaves everything to the legacy or
platform backlight control.

New patches are at

http://cgit.freedesktop.org/~danvet/drm/log/?h=backlight-confusion

Note that these do not include the revert for the backlight inversion
quirk. You either have to revert that yourself for testing or disable the
backlight inversion on the kernel bootline.

Testing feedback highly welcome.

Yours, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] properly enable the blc controller on the right pipe

[Daniel Vetter]

On Sun, Jun 3, 2012 at 1:08 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Apr 27, 2012 at 10:18:08PM +0200, Carsten Emde wrote:
>> Daniel,
>>
>> >>>Depending upon how things are set up, this might help.
>> >>Yeah, thanks a lot, works great now!
>> >>I removed patch 3/4 and 4/4 and pushed this one; so currently applied are:
>> >>[PATCH 1/4] drm/i915: clear up backlight inversion confusion on gen4
>> >>[PATCH 2/4] drm/i915: completely revert the invert brightness quirk
>> >>[PATCH] properly enable the blc controller on the right pipe
>> >Hey, that was just a quick hack to check things ;-)
>> Sorry, too many things in parallel.
>>
>> Spent some time reading the docs you mentioned - will continue to
>> work on the backlight brightness problem and prepare another patch.
>> I just need some more time.
>
> Ok, I've stitched together some real patches and actually tested them.
> While doing so I've noticed that the patch you've tested essentially
> disables the i915 backlight control and leaves everything to the legacy or
> platform backlight control.
>
> New patches are at
>
> http://cgit.freedesktop.org/~danvet/drm/log/?h=backlight-confusion
>
> Note that these do not include the revert for the backlight inversion
> quirk. You either have to revert that yourself for testing or disable the
> backlight inversion on the kernel bootline.
>
> Testing feedback highly welcome.

I've updated my backlight-confusion branch with new patches for gen4
(it doesn't seem to work for these machines yet). Can you please test
the updated branch?

Thanks, Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] properly enable the blc controller on the right pipe

[Carsten Emde]

On 06/11/2012 10:51 AM, Daniel Vetter wrote:
> On Sun, Jun 3, 2012 at 1:08 AM, Daniel Vetter<daniel@ffwll.ch>  wrote:
>> On Fri, Apr 27, 2012 at 10:18:08PM +0200, Carsten Emde wrote:
>>>>>> Depending upon how things are set up, this might help.
>>>>> Yeah, thanks a lot, works great now!
>>>>> I removed patch 3/4 and 4/4 and pushed this one; so currently applied are:
>>>>> [PATCH 1/4] drm/i915: clear up backlight inversion confusion on gen4
>>>>> [PATCH 2/4] drm/i915: completely revert the invert brightness quirk
>>>>> [PATCH] properly enable the blc controller on the right pipe
>>>> Hey, that was just a quick hack to check things ;-)
>>> Sorry, too many things in parallel.
>>>
>>> Spent some time reading the docs you mentioned - will continue to
>>> work on the backlight brightness problem and prepare another patch.
>>> I just need some more time.
>>
>> Ok, I've stitched together some real patches and actually tested them.
>> While doing so I've noticed that the patch you've tested essentially
>> disables the i915 backlight control and leaves everything to the legacy or
>> platform backlight control.
>>
>> New patches are at
>>
>> http://cgit.freedesktop.org/~danvet/drm/log/?h=backlight-confusion
>>
>> Note that these do not include the revert for the backlight inversion
>> quirk. You either have to revert that yourself for testing or disable the
>> backlight inversion on the kernel bootline.
>>
>> Testing feedback highly welcome.
>
> I've updated my backlight-confusion branch with new patches for gen4
> (it doesn't seem to work for these machines yet). Can you please test
> the updated branch?
I have added an additional test rack (#6) to our QA farm (osadl.org/QA) 
and populated this rack solely with notebooks that are equipped with 
Intel graphics board:

Slot #1: Lenovo T61/7661W4G, Intel GMA X3100
Slot #2: IBM T601951-A47, Intel GMA 950
Slot #4: Acer Homa/Extensa 5230E-901G16N, Intel GMA 4500MHD
Slot #5: Acer Morar/TravelMate 2410, Intel GMA 900
Slot #6: Dell Inspirion 1300/0RJ272, Intel GMA 900
Slot #7: Acer Aspire 5734Z, Intel GMA 4500M

With the exception of slot #4 that runs a 3.2-based kernel, all other 
notebooks run a 3.4-based kernel. The notebook in Slot #7 is the one 
with the backlight headache. Unfortunately :-; no other notebook had any 
backlight problem when running a vanilla kernel, even the two new Acers 
are behaving well.

In the meantime, however, I have received a report that the Acer 5732Z 
has the same problem. I am sure that the reporter will be willing to 
help with testing. Looks like the backlight problem is very specific to 
Acer Aspire 573xZ. But I agree that a generic solution always is better 
than a quirk.

I've checked out the backlight-confusion branch of your git tree and 
tested it with and without the Acer quirk. I can report that the 
backlight works correctly with either version. I will now contact the 
owner of the Acer Aspire 5732Z and ask him to test the 
backlight-confusion branch without the Acer quirk on his notebook as well.

Do you wish me to do any other test?

	-Carsten.

Re: [PATCH] properly enable the blc controller on the right pipe

[Daniel Vetter]

On Thu, Jul 19, 2012 at 4:00 PM, Carsten Emde <C.Emde@osadl.org> wrote:
> On 06/11/2012 10:51 AM, Daniel Vetter wrote:
>> I've updated my backlight-confusion branch with new patches for gen4
>> (it doesn't seem to work for these machines yet). Can you please test
>> the updated branch?
>
> I have added an additional test rack (#6) to our QA farm (osadl.org/QA) and
> populated this rack solely with notebooks that are equipped with Intel
> graphics board:
>
> Slot #1: Lenovo T61/7661W4G, Intel GMA X3100
> Slot #2: IBM T601951-A47, Intel GMA 950
> Slot #4: Acer Homa/Extensa 5230E-901G16N, Intel GMA 4500MHD
> Slot #5: Acer Morar/TravelMate 2410, Intel GMA 900
> Slot #6: Dell Inspirion 1300/0RJ272, Intel GMA 900
> Slot #7: Acer Aspire 5734Z, Intel GMA 4500M
>
> With the exception of slot #4 that runs a 3.2-based kernel, all other
> notebooks run a 3.4-based kernel. The notebook in Slot #7 is the one with
> the backlight headache. Unfortunately :-; no other notebook had any
> backlight problem when running a vanilla kernel, even the two new Acers are
> behaving well.
>
> In the meantime, however, I have received a report that the Acer 5732Z has
> the same problem. I am sure that the reporter will be willing to help with
> testing. Looks like the backlight problem is very specific to Acer Aspire
> 573xZ. But I agree that a generic solution always is better than a quirk.
>
> I've checked out the backlight-confusion branch of your git tree and tested
> it with and without the Acer quirk. I can report that the backlight works
> correctly with either version. I will now contact the owner of the Acer
> Aspire 5732Z and ask him to test the backlight-confusion branch without the
> Acer quirk on his notebook as well.
>
> Do you wish me to do any other test?

Well, the backlight-confusion branch had a bug and outright disabled
the intel backlight :( drm-intel-next-queued should have fixes for all
the backlight goofus I've accidentally created, so testing feedback on
that branch would be highly appreciated.

Thanks, Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] properly enable the blc controller on the right pipe

[Carsten Emde]

On 07/19/2012 04:40 PM, Daniel Vetter wrote:
> On Thu, Jul 19, 2012 at 4:00 PM, Carsten Emde<C.Emde@osadl.org>  wrote:
>> On 06/11/2012 10:51 AM, Daniel Vetter wrote:
>>> I've updated my backlight-confusion branch with new patches for gen4
>>> (it doesn't seem to work for these machines yet). Can you please test
>>> the updated branch?
>>
>> I have added an additional test rack (#6) to our QA farm (osadl.org/QA) and
>> populated this rack solely with notebooks that are equipped with Intel
>> graphics board:
>>
>> Slot #1: Lenovo T61/7661W4G, Intel GMA X3100
>> Slot #2: IBM T601951-A47, Intel GMA 950
>> Slot #4: Acer Homa/Extensa 5230E-901G16N, Intel GMA 4500MHD
>> Slot #5: Acer Morar/TravelMate 2410, Intel GMA 900
>> Slot #6: Dell Inspirion 1300/0RJ272, Intel GMA 900
>> Slot #7: Acer Aspire 5734Z, Intel GMA 4500M
>>
>> With the exception of slot #4 that runs a 3.2-based kernel, all other
>> notebooks run a 3.4-based kernel. The notebook in Slot #7 is the one with
>> the backlight headache. Unfortunately :-; no other notebook had any
>> backlight problem when running a vanilla kernel, even the two new Acers are
>> behaving well.
>>
>> In the meantime, however, I have received a report that the Acer 5732Z has
>> the same problem. I am sure that the reporter will be willing to help with
>> testing. Looks like the backlight problem is very specific to Acer Aspire
>> 573xZ. But I agree that a generic solution always is better than a quirk.
>>
>> I've checked out the backlight-confusion branch of your git tree and tested
>> it with and without the Acer quirk. I can report that the backlight works
>> correctly with either version. I will now contact the owner of the Acer
>> Aspire 5732Z and ask him to test the backlight-confusion branch without the
>> Acer quirk on his notebook as well.
>>
>> Do you wish me to do any other test?
>
> Well, the backlight-confusion branch had a bug and outright disabled
> the intel backlight :( drm-intel-next-queued should have fixes for all
> the backlight goofus I've accidentally created, so testing feedback on
> that branch would be highly appreciated.
This branch is now broken again. The original drm-intel-next-queued 
branch works well (it still has the quirk), but after I reverted the 
Acer quirk, the screen remains dark.

I double-checked it and used exactly the same patch to revert the quirk 
in the two trees: The backlight-confusion branch works, and the 
drm-intel-next-queued branch does not.

I then used drivers/gpu/drm/i915/intel_panel.c from backlight-confusion 
in drm-intel-next-queued and - it worked again. When testing the two 
differences separately it turned out this patch did the magic:

Index: drm-intel/drivers/gpu/drm/i915/intel_panel.c
===================================================================
--- drm-intel.orig/drivers/gpu/drm/i915/intel_panel.c
+++ drm-intel/drivers/gpu/drm/i915/intel_panel.c
@@ -121,6 +121,8 @@ static int is_backlight_combination_mode
  {
  	struct drm_i915_private *dev_priv = dev->dev_private;

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

Finally, only the first condition is obviously harmful - this patch also 
works well:

Index: drm-intel/drivers/gpu/drm/i915/intel_panel.c
===================================================================
--- drm-intel.orig/drivers/gpu/drm/i915/intel_panel.c
+++ drm-intel/drivers/gpu/drm/i915/intel_panel.c
@@ -121,8 +121,10 @@ static int is_backlight_combination_mode
  {
  	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;
+#endif

  	if (IS_GEN2(dev))
  		return I915_READ(BLC_PWM_CTL) & BLM_LEGACY_MODE;

Hope this helps,

	-Carsten.

Re: [PATCH] properly enable the blc controller on the right pipe

[Daniel Vetter]

On Fri, Jul 20, 2012 at 12:51:41AM +0200, Carsten Emde wrote:
> On 07/19/2012 04:40 PM, Daniel Vetter wrote:
> >Well, the backlight-confusion branch had a bug and outright disabled
> >the intel backlight :( drm-intel-next-queued should have fixes for all
> >the backlight goofus I've accidentally created, so testing feedback on
> >that branch would be highly appreciated.
> This branch is now broken again. The original drm-intel-next-queued
> branch works well (it still has the quirk), but after I reverted the
> Acer quirk, the screen remains dark.
> 
> I double-checked it and used exactly the same patch to revert the
> quirk in the two trees: The backlight-confusion branch works, and
> the drm-intel-next-queued branch does not.
> 
> I then used drivers/gpu/drm/i915/intel_panel.c from
> backlight-confusion in drm-intel-next-queued and - it worked again.
> When testing the two differences separately it turned out this patch
> did the magic:

Yeah, I guess your system only worked whith the old code because it
accidentally disabled the entire backlight stuff. Now that that doesn't
happen any more, it's as broken as ever.

I'll look into creating another patch, afaict from digging through git
history, this backlight combination mode is totally bogus. But if we kill
it, a few machines will resume with dimmed backlight, because we don't
restore the lbpc register properly.

Thanks for testing, Daniel

> 
> Index: drm-intel/drivers/gpu/drm/i915/intel_panel.c
> ===================================================================
> --- drm-intel.orig/drivers/gpu/drm/i915/intel_panel.c
> +++ drm-intel/drivers/gpu/drm/i915/intel_panel.c
> @@ -121,6 +121,8 @@ static int is_backlight_combination_mode
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> 
> +	return 0;
> +
>  	if (INTEL_INFO(dev)->gen >= 4)
>  		return I915_READ(BLC_PWM_CTL2) & BLM_COMBINATION_MODE;
> 
> Finally, only the first condition is obviously harmful - this patch
> also works well:
> 
> Index: drm-intel/drivers/gpu/drm/i915/intel_panel.c
> ===================================================================
> --- drm-intel.orig/drivers/gpu/drm/i915/intel_panel.c
> +++ drm-intel/drivers/gpu/drm/i915/intel_panel.c
> @@ -121,8 +121,10 @@ static int is_backlight_combination_mode
>  {
>  	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;
> +#endif
> 
>  	if (IS_GEN2(dev))
>  		return I915_READ(BLC_PWM_CTL) & BLM_LEGACY_MODE;
> 
> Hope this helps,
> 
> 	-Carsten.

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] properly enable the blc controller on the right pipe

[Carsten Emde]

On 07/20/2012 10:10 AM, Daniel Vetter wrote:
> On Fri, Jul 20, 2012 at 12:51:41AM +0200, Carsten Emde wrote:
>> On 07/19/2012 04:40 PM, Daniel Vetter wrote:
>>> Well, the backlight-confusion branch had a bug and outright disabled
>>> the intel backlight :( drm-intel-next-queued should have fixes for all
>>> the backlight goofus I've accidentally created, so testing feedback on
>>> that branch would be highly appreciated.
>> This branch is now broken again. The original drm-intel-next-queued
>> branch works well (it still has the quirk), but after I reverted the
>> Acer quirk, the screen remains dark.
>>
>> I double-checked it and used exactly the same patch to revert the
>> quirk in the two trees: The backlight-confusion branch works, and
>> the drm-intel-next-queued branch does not.
>>
>> I then used drivers/gpu/drm/i915/intel_panel.c from
>> backlight-confusion in drm-intel-next-queued and - it worked again.
>> When testing the two differences separately it turned out this patch
>> did the magic:
>
> Yeah, I guess your system only worked whith the old code because it
> accidentally disabled the entire backlight stuff. Now that that doesn't
> happen any more, it's as broken as ever.
>
> I'll look into creating another patch, afaict from digging through git
> history, this backlight combination mode is totally bogus. But if we kill
> it, a few machines will resume with dimmed backlight, because we don't
> restore the lbpc register properly.
After all that hassle and bearing in mind that we found only two 
notebooks so far, Acer Aspire 5732Z and 5734Z, that suffer from this 
backlight inversion problem, shouldn't we simply return to the quirk 
solution? I would quickly add the data of the other Acer to the 
backlight quirks, send a rebased patch, and we are all set.

	-Carsten.

Re: [PATCH] properly enable the blc controller on the right pipe

[Carsten Emde]

On 07/24/2012 09:30 AM, Carsten Emde wrote:
> On 07/20/2012 10:10 AM, Daniel Vetter wrote:
>> On Fri, Jul 20, 2012 at 12:51:41AM +0200, Carsten Emde wrote:
>>> On 07/19/2012 04:40 PM, Daniel Vetter wrote:
>>>> Well, the backlight-confusion branch had a bug and outright disabled
>>>> the intel backlight :( drm-intel-next-queued should have fixes for all
>>>> the backlight goofus I've accidentally created, so testing feedback on
>>>> that branch would be highly appreciated.
>>> This branch is now broken again. The original drm-intel-next-queued
>>> branch works well (it still has the quirk), but after I reverted the
>>> Acer quirk, the screen remains dark.
>>>
>>> I double-checked it and used exactly the same patch to revert the
>>> quirk in the two trees: The backlight-confusion branch works, and
>>> the drm-intel-next-queued branch does not.
>>>
>>> I then used drivers/gpu/drm/i915/intel_panel.c from
>>> backlight-confusion in drm-intel-next-queued and - it worked again.
>>> When testing the two differences separately it turned out this patch
>>> did the magic:
>>
>> Yeah, I guess your system only worked whith the old code because it
>> accidentally disabled the entire backlight stuff. Now that that doesn't
>> happen any more, it's as broken as ever.
>>
>> I'll look into creating another patch, afaict from digging through git
>> history, this backlight combination mode is totally bogus. But if we kill
>> it, a few machines will resume with dimmed backlight, because we don't
>> restore the lbpc register properly.
> After all that hassle and bearing in mind that we found only two
> notebooks so far, Acer Aspire 5732Z and 5734Z, that suffer from this
> backlight inversion problem, shouldn't we simply return to the quirk
> solution? I would quickly add the data of the other Acer to the
> backlight quirks, send a rebased patch, and we are all set.
These are the data of the other Acer Aspire that needs backlight inversion.

  Index: linux-git/drivers/gpu/drm/i915/intel_display.c
===================================================================
--- linux-git.orig/drivers/gpu/drm/i915/intel_display.c
+++ linux-git/drivers/gpu/drm/i915/intel_display.c
@@ -6877,7 +6877,8 @@ static struct intel_quirk intel_quirks[]
  	/* Sony Vaio Y cannot use SSC on LVDS */
  	{ 0x0046, 0x104d, 0x9076, quirk_ssc_force_disable },

-	/* Acer Aspire 5734Z must invert backlight brightness */
+	/* Acer Aspire 5732Z and 5734Z must invert backlight brightness */
+	{ 0x2a42, 0x1025, 0x0212, quirk_invert_brightness },
  	{ 0x2a42, 0x1025, 0x0459, quirk_invert_brightness },
  };

Do you wish me to submit this as a regular patch?

	-Carsten.

[PATCH] drm/i915 disable combination mode

[Daniel Vetter]

... but this time around don't forget to save/restore the lbpc reg.

--

Hi Carsten,

Please test this quick hack, afaict that should be more towards the
ultimate truth of gen4 backlight heaven than adding random invert
brightness quirks.

Yours, Daniel
---
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/i915_reg.h     |    3 +++
 drivers/gpu/drm/i915/i915_suspend.c |   10 ++++++++++
 drivers/gpu/drm/i915/intel_panel.c  |    4 ++--
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0b2eb17..f483ef4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -507,6 +507,7 @@ typedef struct drm_i915_private {
 	/* Register state */
 	bool modeset_on_lid;
 	u8 saveLBB;
+	u8 saveLBPC;
 	u32 saveDSPACNTR;
 	u32 saveDSPBCNTR;
 	u32 saveDSPARB;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1310caa..29ccd22 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1882,6 +1882,9 @@
 
 #define PFIT_AUTO_RATIOS 0x61238
 
+/* legacy/combination backlight modes in the pci config space */
+#define PCI_LBPC 0xf4
+
 /* Backlight control */
 #define BLC_PWM_CTL2		0x61250 /* 965+ only */
 #define   BLM_PWM_ENABLE		(1 << 31)
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 740c076..63f9c09 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -641,6 +641,11 @@ static void i915_save_display(struct drm_device *dev)
 			dev_priv->saveLVDS = I915_READ(LVDS);
 	}
 
+	if (IS_GEN2(dev) || IS_GEN4(dev)) {
+		pci_read_config_byte(dev->pdev, PCI_LBPC,
+				     &dev_priv->saveLBPC);
+	}
+
 	if (!IS_I830(dev) && !IS_845G(dev) && !HAS_PCH_SPLIT(dev))
 		dev_priv->savePFIT_CONTROL = I915_READ(PFIT_CONTROL);
 
@@ -758,6 +763,11 @@ static void i915_restore_display(struct drm_device *dev)
 		I915_WRITE(PP_CONTROL, dev_priv->savePP_CONTROL);
 	}
 
+	if (IS_GEN2(dev) || IS_GEN4(dev)) {
+		pci_read_config_byte(dev->pdev, PCI_LBPC,
+				     &dev_priv->saveLBPC);
+	}
+
 	/* Display Port state */
 	if (SUPPORTS_INTEGRATED_DP(dev)) {
 		I915_WRITE(DP_B, dev_priv->saveDP_B);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 10c7d39..c8b6bc5 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -33,8 +33,6 @@
 #include <linux/moduleparam.h>
 #include "intel_drv.h"
 
-#define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
-
 void
 intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
 		       struct drm_display_mode *adjusted_mode)
@@ -121,11 +119,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;
 }
-- 
1.7.10.4

[PATCH] drm/i915 disable combination mode

[Daniel Vetter]

... but this time around don't forget to save/restore the lbpc reg.

v2: Actually try to restroe LBPC on resume.
--

Hi Carsten,

Please test this quick hack, afaict that should be more towards the
ultimate truth of gen4 backlight heaven than adding random invert
brightness quirks.

Yours, Daniel
---
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/i915_reg.h     |    3 +++
 drivers/gpu/drm/i915/i915_suspend.c |   10 ++++++++++
 drivers/gpu/drm/i915/intel_panel.c  |    4 ++--
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0b2eb17..f483ef4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -507,6 +507,7 @@ typedef struct drm_i915_private {
 	/* Register state */
 	bool modeset_on_lid;
 	u8 saveLBB;
+	u8 saveLBPC;
 	u32 saveDSPACNTR;
 	u32 saveDSPBCNTR;
 	u32 saveDSPARB;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1310caa..29ccd22 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1882,6 +1882,9 @@
 
 #define PFIT_AUTO_RATIOS 0x61238
 
+/* legacy/combination backlight modes in the pci config space */
+#define PCI_LBPC 0xf4
+
 /* Backlight control */
 #define BLC_PWM_CTL2		0x61250 /* 965+ only */
 #define   BLM_PWM_ENABLE		(1 << 31)
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 740c076..63f9c09 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -641,6 +641,11 @@ static void i915_save_display(struct drm_device *dev)
 			dev_priv->saveLVDS = I915_READ(LVDS);
 	}
 
+	if (IS_GEN2(dev) || IS_GEN4(dev)) {
+		pci_read_config_byte(dev->pdev, PCI_LBPC,
+				     &dev_priv->saveLBPC);
+	}
+
 	if (!IS_I830(dev) && !IS_845G(dev) && !HAS_PCH_SPLIT(dev))
 		dev_priv->savePFIT_CONTROL = I915_READ(PFIT_CONTROL);
 
@@ -758,6 +763,11 @@ static void i915_restore_display(struct drm_device *dev)
 		I915_WRITE(PP_CONTROL, dev_priv->savePP_CONTROL);
 	}
 
+	if (IS_GEN2(dev) || IS_GEN4(dev)) {
+		pci_read_config_byte(dev->pdev, PCI_LBPC,
+				     &dev_priv->saveLBPC);
+	}
+
 	/* Display Port state */
 	if (SUPPORTS_INTEGRATED_DP(dev)) {
 		I915_WRITE(DP_B, dev_priv->saveDP_B);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 10c7d39..c8b6bc5 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -33,8 +33,6 @@
 #include <linux/moduleparam.h>
 #include "intel_drv.h"
 
-#define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
-
 void
 intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
 		       struct drm_display_mode *adjusted_mode)
@@ -121,11 +119,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;
 }
-- 
1.7.10.4

[PATCH] drm/i915 disable combination mode

[Daniel Vetter]

... but this time around don't forget to save/restore the lbpc reg.

v2: Actually try to restroe LBPC on resume.
--

Hi Carsten,

Please test this quick hack, afaict that should be more towards the
ultimate truth of gen4 backlight heaven than adding random invert
brightness quirks.

Yours, Daniel
---
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/i915_reg.h     |    3 +++
 drivers/gpu/drm/i915/i915_suspend.c |   10 ++++++++++
 drivers/gpu/drm/i915/intel_panel.c  |    4 ++--
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0b2eb17..f483ef4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -507,6 +507,7 @@ typedef struct drm_i915_private {
 	/* Register state */
 	bool modeset_on_lid;
 	u8 saveLBB;
+	u8 saveLBPC;
 	u32 saveDSPACNTR;
 	u32 saveDSPBCNTR;
 	u32 saveDSPARB;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1310caa..29ccd22 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1882,6 +1882,9 @@
 
 #define PFIT_AUTO_RATIOS 0x61238
 
+/* legacy/combination backlight modes in the pci config space */
+#define PCI_LBPC 0xf4
+
 /* Backlight control */
 #define BLC_PWM_CTL2		0x61250 /* 965+ only */
 #define   BLM_PWM_ENABLE		(1 << 31)
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 740c076..63f9c09 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -641,6 +641,11 @@ static void i915_save_display(struct drm_device *dev)
 			dev_priv->saveLVDS = I915_READ(LVDS);
 	}
 
+	if (IS_GEN2(dev) || IS_GEN4(dev)) {
+		pci_read_config_byte(dev->pdev, PCI_LBPC,
+				     &dev_priv->saveLBPC);
+	}
+
 	if (!IS_I830(dev) && !IS_845G(dev) && !HAS_PCH_SPLIT(dev))
 		dev_priv->savePFIT_CONTROL = I915_READ(PFIT_CONTROL);
 
@@ -758,6 +763,11 @@ static void i915_restore_display(struct drm_device *dev)
 		I915_WRITE(PP_CONTROL, dev_priv->savePP_CONTROL);
 	}
 
+	if (IS_GEN2(dev) || IS_GEN4(dev)) {
+		pci_read_config_byte(dev->pdev, PCI_LBPC,
+				     &dev_priv->saveLBPC);
+	}
+
 	/* Display Port state */
 	if (SUPPORTS_INTEGRATED_DP(dev)) {
 		I915_WRITE(DP_B, dev_priv->saveDP_B);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 10c7d39..c8b6bc5 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -33,8 +33,6 @@
 #include <linux/moduleparam.h>
 #include "intel_drv.h"
 
-#define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
-
 void
 intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
 		       struct drm_display_mode *adjusted_mode)
@@ -121,11 +119,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;
 }
-- 
1.7.10.4

[PATCH] drm/i915 disable combination mode

[Daniel Vetter]

... but this time around don't forget to save/restore the lbpc reg.

v2: Actually try to restroe LBPC on resume.
v3: _Really_ try to git add.
--

Hi Carsten,

Please test this quick hack, afaict that should be more towards the
ultimate truth of gen4 backlight heaven than adding random invert
brightness quirks.

Yours, Daniel
---
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/i915_reg.h     |    3 +++
 drivers/gpu/drm/i915/i915_suspend.c |   10 ++++++++++
 drivers/gpu/drm/i915/intel_panel.c  |    4 ++--
 4 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0b2eb17..f483ef4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -507,6 +507,7 @@ typedef struct drm_i915_private {
 	/* Register state */
 	bool modeset_on_lid;
 	u8 saveLBB;
+	u8 saveLBPC;
 	u32 saveDSPACNTR;
 	u32 saveDSPBCNTR;
 	u32 saveDSPARB;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1310caa..29ccd22 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1882,6 +1882,9 @@
 
 #define PFIT_AUTO_RATIOS 0x61238
 
+/* legacy/combination backlight modes in the pci config space */
+#define PCI_LBPC 0xf4
+
 /* Backlight control */
 #define BLC_PWM_CTL2		0x61250 /* 965+ only */
 #define   BLM_PWM_ENABLE		(1 << 31)
diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index 740c076..7f44079 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -641,6 +641,11 @@ static void i915_save_display(struct drm_device *dev)
 			dev_priv->saveLVDS = I915_READ(LVDS);
 	}
 
+	if (IS_GEN2(dev) || IS_GEN4(dev)) {
+		pci_read_config_byte(dev->pdev, PCI_LBPC,
+				     &dev_priv->saveLBPC);
+	}
+
 	if (!IS_I830(dev) && !IS_845G(dev) && !HAS_PCH_SPLIT(dev))
 		dev_priv->savePFIT_CONTROL = I915_READ(PFIT_CONTROL);
 
@@ -758,6 +763,11 @@ static void i915_restore_display(struct drm_device *dev)
 		I915_WRITE(PP_CONTROL, dev_priv->savePP_CONTROL);
 	}
 
+	if (IS_GEN2(dev) || IS_GEN4(dev)) {
+		pci_write_config_byte(dev->pdev, PCI_LBPC,
+				      dev_priv->saveLBPC);
+	}
+
 	/* Display Port state */
 	if (SUPPORTS_INTEGRATED_DP(dev)) {
 		I915_WRITE(DP_B, dev_priv->saveDP_B);
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 10c7d39..c8b6bc5 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -33,8 +33,6 @@
 #include <linux/moduleparam.h>
 #include "intel_drv.h"
 
-#define PCI_LBPC 0xf4 /* legacy/combination backlight modes */
-
 void
 intel_fixed_panel_mode(struct drm_display_mode *fixed_mode,
 		       struct drm_display_mode *adjusted_mode)
@@ -121,11 +119,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;
 }
-- 
1.7.10.4