diff mbox

Fixup - Re: [stable] drm/i915 patches fix Latitude E6410 video issues.

Message ID 1280443867.23856.77.camel@xps-1
State Accepted
Delegated to: Steve Conklin
Headers show

Commit Message

Steve Conklin July 29, 2010, 10:51 p.m. UTC 
On Wed, 2010-07-28 at 13:45 -0500, Manoj Iyer wrote:
> Please consider the following upstream commits to stable 2.6.33.y
> 
> 1. drm/i915: add PANEL_UNLOCK_REGS definition
>     SHAID: 4a655f043160eeae447efd3be297b6b4c397a640
> 2. drm/i915: make sure eDP panel is turned on
>     SHAID: 9934c132989d5c488d2e15188220ce240960ce96
> 
> Depends on
> 
> 3. drm/i915: Rename intel_output to intel_encoder.
>     SHAID: 21d40d37eca86872f2bf0af995809ebdef25c9d9
> 
> 
> These patches fix video issues on Dell Latitude E6410. Reported in bugs
> 
> http://launchpad.net/bugs/578673
> http://launchpad.net/bugs/561802
> 
> The patches apply cleanly to 2.6.33.y (see attachments), and they were 
> tested against the latest Lucid kernel and reported to fix the 2 issues. 
> Kernel was tested by user community, as well as, on hardware available at 
> Canonical.
> 
> Regarding the issue with dim back-light on resume, Jesse wrote to me saying 
> he suspects some other agent like firmware might be a suspect in zeroing 
> it out before the driver saves it, ie driver seems to do the right thing.
> 
> Regards
> Manoj Iyer

(as Brad pointed out)
1. I think that we're better off backporting this without the
driver-wide variable rename, until/unless that lands in stable, or it's
going to cause us pain for the rest of Lucid's lifetime when we take
stable patches. It's a lot of change compared with the six lines in 
the dependent patch.

2. There was a bug in the upstream patch:
drm/i915: make sure eDP panel is turned on
which was fixed in a subsequent commit. backlight_off()was being called
twice and panel_off() was not being called.

This is fixed by including the upstream patch
drm/i915: make sure we shut off the panel in eDP configs
(also backported because of conflicts caused by the global rename)

3. The drm-i915-add-PANEL_UNLOCK_REGS-definition patch is fine, but
needs to be applied before the others, the original patches were out of
order.

Attached are the three required patches in order, having dropped the
rename patch, and adding the fix for panel_off().

These replace the patches proposed in the original email.

They apply cleanly to lucid and to linux-2.6.33.y.

Steve Conklin

Comments

Greg KH July 30, 2010, 1:05 a.m. UTC | #1 
On Thu, Jul 29, 2010 at 05:51:07PM -0500, Steve Conklin wrote:
> On Wed, 2010-07-28 at 13:45 -0500, Manoj Iyer wrote:
> > Please consider the following upstream commits to stable 2.6.33.y
> > 
> > 1. drm/i915: add PANEL_UNLOCK_REGS definition
> >     SHAID: 4a655f043160eeae447efd3be297b6b4c397a640
> > 2. drm/i915: make sure eDP panel is turned on
> >     SHAID: 9934c132989d5c488d2e15188220ce240960ce96

You do realize that these two aren't in the .34 tree either, right?
Shouldn't they go to the .34-stable tree?

Remember, .33 is only getting one more release, and I'm really not
wanting to do much work on it anymore.

> > Depends on
> > 
> > 3. drm/i915: Rename intel_output to intel_encoder.
> >     SHAID: 21d40d37eca86872f2bf0af995809ebdef25c9d9

This one is in .34, so the above two should be fine for .34-stable.

> > These patches fix video issues on Dell Latitude E6410. Reported in bugs
> > 
> > http://launchpad.net/bugs/578673
> > http://launchpad.net/bugs/561802
> > 
> > The patches apply cleanly to 2.6.33.y (see attachments), and they were 
> > tested against the latest Lucid kernel and reported to fix the 2 issues. 
> > Kernel was tested by user community, as well as, on hardware available at 
> > Canonical.
> > 
> > Regarding the issue with dim back-light on resume, Jesse wrote to me saying 
> > he suspects some other agent like firmware might be a suspect in zeroing 
> > it out before the driver saves it, ie driver seems to do the right thing.
> > 
> > Regards
> > Manoj Iyer
> 
> (as Brad pointed out)
> 1. I think that we're better off backporting this without the
> driver-wide variable rename, until/unless that lands in stable, or it's
> going to cause us pain for the rest of Lucid's lifetime when we take
> stable patches. It's a lot of change compared with the six lines in 
> the dependent patch.
> 
> 2. There was a bug in the upstream patch:
> drm/i915: make sure eDP panel is turned on
> which was fixed in a subsequent commit. backlight_off()was being called
> twice and panel_off() was not being called.
> 
> This is fixed by including the upstream patch
> drm/i915: make sure we shut off the panel in eDP configs
> (also backported because of conflicts caused by the global rename)
> 
> 3. The drm-i915-add-PANEL_UNLOCK_REGS-definition patch is fine, but
> needs to be applied before the others, the original patches were out of
> order.
> 
> Attached are the three required patches in order, having dropped the
> rename patch, and adding the fix for panel_off().
> 
> These replace the patches proposed in the original email.
> 
> They apply cleanly to lucid and to linux-2.6.33.y.

Mind if I just apply them to .34-stable instead?

thanks,

greg k-h
Steve Conklin July 30, 2010, 3:12 a.m. UTC | #2 
On Thu, 2010-07-29 at 18:05 -0700, Greg KH wrote:
> On Thu, Jul 29, 2010 at 05:51:07PM -0500, Steve Conklin wrote:
> > On Wed, 2010-07-28 at 13:45 -0500, Manoj Iyer wrote:
> > > Please consider the following upstream commits to stable 2.6.33.y
> > > 
> > > 1. drm/i915: add PANEL_UNLOCK_REGS definition
> > >     SHAID: 4a655f043160eeae447efd3be297b6b4c397a640
> > > 2. drm/i915: make sure eDP panel is turned on
> > >     SHAID: 9934c132989d5c488d2e15188220ce240960ce96
> 
> You do realize that these two aren't in the .34 tree either, right?
> Shouldn't they go to the .34-stable tree?
> 
> Remember, .33 is only getting one more release, and I'm really not
> wanting to do much work on it anymore.
> 
> > > Depends on
> > > 
> > > 3. drm/i915: Rename intel_output to intel_encoder.
> > >     SHAID: 21d40d37eca86872f2bf0af995809ebdef25c9d9
> 
> This one is in .34, so the above two should be fine for .34-stable.
> 
> > > These patches fix video issues on Dell Latitude E6410. Reported in bugs
> > > 
> > > http://launchpad.net/bugs/578673
> > > http://launchpad.net/bugs/561802
> > > 
> > > The patches apply cleanly to 2.6.33.y (see attachments), and they were 
> > > tested against the latest Lucid kernel and reported to fix the 2 issues. 
> > > Kernel was tested by user community, as well as, on hardware available at 
> > > Canonical.
> > > 
> > > Regarding the issue with dim back-light on resume, Jesse wrote to me saying 
> > > he suspects some other agent like firmware might be a suspect in zeroing 
> > > it out before the driver saves it, ie driver seems to do the right thing.
> > > 
> > > Regards
> > > Manoj Iyer
> > 
> > (as Brad pointed out)
> > 1. I think that we're better off backporting this without the
> > driver-wide variable rename, until/unless that lands in stable, or it's
> > going to cause us pain for the rest of Lucid's lifetime when we take
> > stable patches. It's a lot of change compared with the six lines in 
> > the dependent patch.
> > 
> > 2. There was a bug in the upstream patch:
> > drm/i915: make sure eDP panel is turned on
> > which was fixed in a subsequent commit. backlight_off()was being called
> > twice and panel_off() was not being called.
> > 
> > This is fixed by including the upstream patch
> > drm/i915: make sure we shut off the panel in eDP configs
> > (also backported because of conflicts caused by the global rename)
> > 
> > 3. The drm-i915-add-PANEL_UNLOCK_REGS-definition patch is fine, but
> > needs to be applied before the others, the original patches were out of
> > order.
> > 
> > Attached are the three required patches in order, having dropped the
> > rename patch, and adding the fix for panel_off().
> > 
> > These replace the patches proposed in the original email.
> > 
> > They apply cleanly to lucid and to linux-2.6.33.y.
> 
> Mind if I just apply them to .34-stable instead?
> 
> thanks,
> 
> greg k-h

Sure.34 is a good place for these. I don't pay as much attention to .34
as the others, as we have no release based on it. We'll be supporting
.33 drm for a while yet in 10.04, a long-term release.

If you want to apply them to .34 you'll need the ones that I didn't bash
around the renaming. That's mainline commits:

4a655f043160eeae447efd3be297b6b4c397a640 drm/i915: add PANEL_UNLOCK_REGS
definition

9934c132989d5c488d2e15188220ce240960ce96 drm/i915: make sure eDP panel is turned on

5620ae29f1eabe655f44335231b580a78c8364ea drm/i915: make sure we shut off the panel in eDP configs

Steve
Stefan Bader July 30, 2010, 7:55 a.m. UTC | #3 
On 07/30/2010 05:12 AM, Steve Conklin wrote:
> On Thu, 2010-07-29 at 18:05 -0700, Greg KH wrote:
>> On Thu, Jul 29, 2010 at 05:51:07PM -0500, Steve Conklin wrote:
>>> On Wed, 2010-07-28 at 13:45 -0500, Manoj Iyer wrote:
>>>> Please consider the following upstream commits to stable 2.6.33.y
>>>>
>>>> 1. drm/i915: add PANEL_UNLOCK_REGS definition
>>>>     SHAID: 4a655f043160eeae447efd3be297b6b4c397a640
>>>> 2. drm/i915: make sure eDP panel is turned on
>>>>     SHAID: 9934c132989d5c488d2e15188220ce240960ce96
>>
>> You do realize that these two aren't in the .34 tree either, right?
>> Shouldn't they go to the .34-stable tree?
>>
>> Remember, .33 is only getting one more release, and I'm really not
>> wanting to do much work on it anymore.
>>
>>>> Depends on
>>>>
>>>> 3. drm/i915: Rename intel_output to intel_encoder.
>>>>     SHAID: 21d40d37eca86872f2bf0af995809ebdef25c9d9
>>
>> This one is in .34, so the above two should be fine for .34-stable.
>>
>>>> These patches fix video issues on Dell Latitude E6410. Reported in bugs
>>>>
>>>> http://launchpad.net/bugs/578673
>>>> http://launchpad.net/bugs/561802
>>>>
>>>> The patches apply cleanly to 2.6.33.y (see attachments), and they were 
>>>> tested against the latest Lucid kernel and reported to fix the 2 issues. 
>>>> Kernel was tested by user community, as well as, on hardware available at 
>>>> Canonical.
>>>>
>>>> Regarding the issue with dim back-light on resume, Jesse wrote to me saying 
>>>> he suspects some other agent like firmware might be a suspect in zeroing 
>>>> it out before the driver saves it, ie driver seems to do the right thing.
>>>>
>>>> Regards
>>>> Manoj Iyer
>>>
>>> (as Brad pointed out)
>>> 1. I think that we're better off backporting this without the
>>> driver-wide variable rename, until/unless that lands in stable, or it's
>>> going to cause us pain for the rest of Lucid's lifetime when we take
>>> stable patches. It's a lot of change compared with the six lines in 
>>> the dependent patch.
>>>
>>> 2. There was a bug in the upstream patch:
>>> drm/i915: make sure eDP panel is turned on
>>> which was fixed in a subsequent commit. backlight_off()was being called
>>> twice and panel_off() was not being called.
>>>
>>> This is fixed by including the upstream patch
>>> drm/i915: make sure we shut off the panel in eDP configs
>>> (also backported because of conflicts caused by the global rename)
>>>
>>> 3. The drm-i915-add-PANEL_UNLOCK_REGS-definition patch is fine, but
>>> needs to be applied before the others, the original patches were out of
>>> order.
>>>
>>> Attached are the three required patches in order, having dropped the
>>> rename patch, and adding the fix for panel_off().
>>>
>>> These replace the patches proposed in the original email.
>>>
>>> They apply cleanly to lucid and to linux-2.6.33.y.
>>
>> Mind if I just apply them to .34-stable instead?
>>
>> thanks,
>>
>> greg k-h
> 
> Sure.34 is a good place for these. I don't pay as much attention to .34
> as the others, as we have no release based on it. We'll be supporting
> .33 drm for a while yet in 10.04, a long-term release.

Just a reminder that currently we use .33 updates only for the combined
2.6.32+drm33 tree as well as Lucid. So if things are only applied to .34 there
is a chance to miss them. I don't want to add drm patches other than .33 to my
combined branch as long as a real .33 is maintained by Greg.

Greg, do you have an estimate how many .33 releases you will keep doing? The
currently queued one likely the last or more to come?

-Stefan

> If you want to apply them to .34 you'll need the ones that I didn't bash
> around the renaming. That's mainline commits:
> 
> 4a655f043160eeae447efd3be297b6b4c397a640 drm/i915: add PANEL_UNLOCK_REGS
> definition
> 
> 9934c132989d5c488d2e15188220ce240960ce96 drm/i915: make sure eDP panel is turned on
> 
> 5620ae29f1eabe655f44335231b580a78c8364ea drm/i915: make sure we shut off the panel in eDP configs
> 
> Steve
> 
>
Stefan Bader July 30, 2010, 8:23 a.m. UTC | #4 
On 07/30/2010 09:55 AM, Stefan Bader wrote:
> On 07/30/2010 05:12 AM, Steve Conklin wrote:
>> On Thu, 2010-07-29 at 18:05 -0700, Greg KH wrote:
>>> On Thu, Jul 29, 2010 at 05:51:07PM -0500, Steve Conklin wrote:
>>>> On Wed, 2010-07-28 at 13:45 -0500, Manoj Iyer wrote:
>>> Remember, .33 is only getting one more release, and I'm really not
>>> wanting to do much work on it anymore.
>>>
>>
>> Sure.34 is a good place for these. I don't pay as much attention to .34
>> as the others, as we have no release based on it. We'll be supporting
>> .33 drm for a while yet in 10.04, a long-term release.
> 
> Just a reminder that currently we use .33 updates only for the combined
> 2.6.32+drm33 tree as well as Lucid. So if things are only applied to .34 there
> is a chance to miss them. I don't want to add drm patches other than .33 to my
> combined branch as long as a real .33 is maintained by Greg.
> 
> Greg, do you have an estimate how many .33 releases you will keep doing? The
> currently queued one likely the last or more to come?

Ok, forget my last question. It seems there was too many trees for me to see the
answer right in there. So I need to be careful with the next stable releases to
get things together from various sources.

-Stefan
Greg KH July 30, 2010, 3:59 p.m. UTC | #5 
On Thu, Jul 29, 2010 at 10:12:35PM -0500, Steve Conklin wrote:
> On Thu, 2010-07-29 at 18:05 -0700, Greg KH wrote:
> > On Thu, Jul 29, 2010 at 05:51:07PM -0500, Steve Conklin wrote:
> > > On Wed, 2010-07-28 at 13:45 -0500, Manoj Iyer wrote:
> > > > Please consider the following upstream commits to stable 2.6.33.y
> > > > 
> > > > 1. drm/i915: add PANEL_UNLOCK_REGS definition
> > > >     SHAID: 4a655f043160eeae447efd3be297b6b4c397a640
> > > > 2. drm/i915: make sure eDP panel is turned on
> > > >     SHAID: 9934c132989d5c488d2e15188220ce240960ce96
> > 
> > You do realize that these two aren't in the .34 tree either, right?
> > Shouldn't they go to the .34-stable tree?
> > 
> > Remember, .33 is only getting one more release, and I'm really not
> > wanting to do much work on it anymore.
> > 
> > > > Depends on
> > > > 
> > > > 3. drm/i915: Rename intel_output to intel_encoder.
> > > >     SHAID: 21d40d37eca86872f2bf0af995809ebdef25c9d9
> > 
> > This one is in .34, so the above two should be fine for .34-stable.
> > 
> > > > These patches fix video issues on Dell Latitude E6410. Reported in bugs
> > > > 
> > > > http://launchpad.net/bugs/578673
> > > > http://launchpad.net/bugs/561802
> > > > 
> > > > The patches apply cleanly to 2.6.33.y (see attachments), and they were 
> > > > tested against the latest Lucid kernel and reported to fix the 2 issues. 
> > > > Kernel was tested by user community, as well as, on hardware available at 
> > > > Canonical.
> > > > 
> > > > Regarding the issue with dim back-light on resume, Jesse wrote to me saying 
> > > > he suspects some other agent like firmware might be a suspect in zeroing 
> > > > it out before the driver saves it, ie driver seems to do the right thing.
> > > > 
> > > > Regards
> > > > Manoj Iyer
> > > 
> > > (as Brad pointed out)
> > > 1. I think that we're better off backporting this without the
> > > driver-wide variable rename, until/unless that lands in stable, or it's
> > > going to cause us pain for the rest of Lucid's lifetime when we take
> > > stable patches. It's a lot of change compared with the six lines in 
> > > the dependent patch.
> > > 
> > > 2. There was a bug in the upstream patch:
> > > drm/i915: make sure eDP panel is turned on
> > > which was fixed in a subsequent commit. backlight_off()was being called
> > > twice and panel_off() was not being called.
> > > 
> > > This is fixed by including the upstream patch
> > > drm/i915: make sure we shut off the panel in eDP configs
> > > (also backported because of conflicts caused by the global rename)
> > > 
> > > 3. The drm-i915-add-PANEL_UNLOCK_REGS-definition patch is fine, but
> > > needs to be applied before the others, the original patches were out of
> > > order.
> > > 
> > > Attached are the three required patches in order, having dropped the
> > > rename patch, and adding the fix for panel_off().
> > > 
> > > These replace the patches proposed in the original email.
> > > 
> > > They apply cleanly to lucid and to linux-2.6.33.y.
> > 
> > Mind if I just apply them to .34-stable instead?
> > 
> > thanks,
> > 
> > greg k-h
> 
> Sure.34 is a good place for these. I don't pay as much attention to .34
> as the others, as we have no release based on it. We'll be supporting
> .33 drm for a while yet in 10.04, a long-term release.

That's nice, but it has nothing to do with the upstream -stable releases
:)

> If you want to apply them to .34 you'll need the ones that I didn't bash
> around the renaming. That's mainline commits:
> 
> 4a655f043160eeae447efd3be297b6b4c397a640 drm/i915: add PANEL_UNLOCK_REGS
> definition
> 
> 9934c132989d5c488d2e15188220ce240960ce96 drm/i915: make sure eDP panel is turned on
> 
> 5620ae29f1eabe655f44335231b580a78c8364ea drm/i915: make sure we shut off the panel in eDP configs

Ok, I'll see about adding those later after this release.

thanks,

greg k-h
Greg KH July 30, 2010, 3:59 p.m. UTC | #6 
On Fri, Jul 30, 2010 at 09:55:43AM +0200, Stefan Bader wrote:
> Greg, do you have an estimate how many .33 releases you will keep doing? The
> currently queued one likely the last or more to come?

This is going to be the last one.

thanks,

greg k-h
Steve Conklin Aug. 3, 2010, 9:12 p.m. UTC | #7 
On Thu, 2010-07-29 at 17:51 -0500, Steve Conklin wrote:
> On Wed, 2010-07-28 at 13:45 -0500, Manoj Iyer wrote:
> > Please consider the following upstream commits to stable 2.6.33.y
> > 
> > 1. drm/i915: add PANEL_UNLOCK_REGS definition
> >     SHAID: 4a655f043160eeae447efd3be297b6b4c397a640
> > 2. drm/i915: make sure eDP panel is turned on
> >     SHAID: 9934c132989d5c488d2e15188220ce240960ce96
> > 
> > Depends on
> > 
> > 3. drm/i915: Rename intel_output to intel_encoder.
> >     SHAID: 21d40d37eca86872f2bf0af995809ebdef25c9d9
> > 
> > 
> > These patches fix video issues on Dell Latitude E6410. Reported in bugs
> > 
> > http://launchpad.net/bugs/578673
> > http://launchpad.net/bugs/561802
> > 
> > The patches apply cleanly to 2.6.33.y (see attachments), and they were 
> > tested against the latest Lucid kernel and reported to fix the 2 issues. 
> > Kernel was tested by user community, as well as, on hardware available at 
> > Canonical.
> > 
> > Regarding the issue with dim back-light on resume, Jesse wrote to me saying 
> > he suspects some other agent like firmware might be a suspect in zeroing 
> > it out before the driver saves it, ie driver seems to do the right thing.
> > 
> > Regards
> > Manoj Iyer
> 
> (as Brad pointed out)
> 1. I think that we're better off backporting this without the
> driver-wide variable rename, until/unless that lands in stable, or it's
> going to cause us pain for the rest of Lucid's lifetime when we take
> stable patches. It's a lot of change compared with the six lines in 
> the dependent patch.
> 
> 2. There was a bug in the upstream patch:
> drm/i915: make sure eDP panel is turned on
> which was fixed in a subsequent commit. backlight_off()was being called
> twice and panel_off() was not being called.
> 
> This is fixed by including the upstream patch
> drm/i915: make sure we shut off the panel in eDP configs
> (also backported because of conflicts caused by the global rename)
> 
> 3. The drm-i915-add-PANEL_UNLOCK_REGS-definition patch is fine, but
> needs to be applied before the others, the original patches were out of
> order.
> 
> Attached are the three required patches in order, having dropped the
> rename patch, and adding the fix for panel_off().
> 
> These replace the patches proposed in the original email.
> 
> They apply cleanly to lucid and to linux-2.6.33.y.
> 
> Steve Conklin
> 
> 
>
diff mbox

Patch

From 520d78adefad5f91e3616a891f0b2966d47bcfe1 Mon Sep 17 00:00:00 2001
From: Steve Conklin <sconklin@canonical.com>
Date: Thu, 29 Jul 2010 16:58:43 -0500
Subject: [PATCH 3/3] UBUNTU: SAUCE: drm/i915: make sure we shut off the panel in eDP configs

This patch is backported from commit 5620ae29f1eabe655f44335231b580a78c8364ea
It does not apply as a cherry-pick because of a driver-wide variable rename.

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

OriginalAuthor: Jesse Barnes <jbarnes@virtuousgeek.org>
Fix error from the last pull request.  Making sure we shut the panel off
is more correct and saves power.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Steve Conklin <sconklin@canonical.com>
---
 drivers/gpu/drm/i915/intel_dp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 40ca37a..6b62762 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -757,7 +757,7 @@  intel_dp_dpms(struct drm_encoder *encoder, int mode)
 			intel_dp_link_down(intel_output, dp_priv->DP);
 			if (IS_eDP(intel_output)) {
 				ironlake_edp_backlight_off(dev);
-				ironlake_edp_backlight_off(dev);
+				ironlake_edp_panel_off(dev);
 			}
 		}
 	} else {
-- 
1.7.0.4