LinuxLists.cc - Linux 2.6.33-rc4

[permalink] [raw]

Subject: Re: Linux 2.6.33-rc4

On Wed, Jan 13, 2010 at 7:44 AM, Linus Torvalds <[email protected]>
wrote:
> Hmm. Odd release. Something like 40% of the patches are in DRM (mostly
> nouveau and radeon, both staging, so it's a bit less scary than it sounds.
> But there's a noticeable i915 component too). That's all pretty unusual,
> afaik.

2.6.33-rc4 still suffers from an annoying screen flicker with i915 and
KMS on my machine. I *think* it started with 2.6.33-rc1 but I haven't
had the time to dig deeper.

Pekka

2010-01-13 21:34:01

[permalink] [raw]

Subject: Re: Linux 2.6.33-rc4

On Wed, 13 Jan 2010 23:03:00 +0200
Pekka Enberg <[email protected]> wrote:

> On Wed, Jan 13, 2010 at 7:44 AM, Linus Torvalds
> <[email protected]> wrote:
> > Hmm. Odd release. Something like 40% of the patches are in DRM
> > (mostly nouveau and radeon, both staging, so it's a bit less scary
> > than it sounds. But there's a noticeable i915 component too).
> > That's all pretty unusual, afaik.
>
> 2.6.33-rc4 still suffers from an annoying screen flicker with i915 and
> KMS on my machine. I *think* it started with 2.6.33-rc1 but I haven't
> had the time to dig deeper.

Does this patch fix it? If so I'll queue something like this up for
-rc5.

--
Jesse Barnes, Intel Open Source Technology Center

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9187a17..0ab1bef 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3806,7 +3806,7 @@ static void intel_increase_pllclock(struct drm_crtc *crtc, bool schedule)
if (IS_IRONLAKE(dev))
return;

- if (!dev_priv->lvds_downclock_avail)
+ if (!dev_priv->lvds_downclock_avail || 1)
return;

if (!HAS_PIPE_CXSR(dev) && (dpll & DISPLAY_RATE_SELECT_FPA1)) {
@@ -3845,7 +3845,7 @@ static void intel_decrease_pllclock(struct drm_crtc *crtc)
if (IS_IRONLAKE(dev))
return;

- if (!dev_priv->lvds_downclock_avail)
+ if (!dev_priv->lvds_downclock_avail || 1)
return;

/*

2010-01-13 21:51:13

[permalink] [raw]

Subject: Re: Linux 2.6.33-rc4

Jesse Barnes wrote:
> On Wed, 13 Jan 2010 23:03:00 +0200
> Pekka Enberg <[email protected]> wrote:
>
>> On Wed, Jan 13, 2010 at 7:44 AM, Linus Torvalds
>> <[email protected]> wrote:
>>> Hmm. Odd release. Something like 40% of the patches are in DRM
>>> (mostly nouveau and radeon, both staging, so it's a bit less scary
>>> than it sounds. But there's a noticeable i915 component too).
>>> That's all pretty unusual, afaik.
>> 2.6.33-rc4 still suffers from an annoying screen flicker with i915 and
>> KMS on my machine. I *think* it started with 2.6.33-rc1 but I haven't
>> had the time to dig deeper.
>
> Does this patch fix it? If so I'll queue something like this up for
> -rc5.

Yes, it does. Thanks!

Pekka

2010-01-14 00:55:35

[permalink] [raw]

Subject: Re: Linux 2.6.33-rc4

On Wed, 13 Jan 2010 23:50:52 +0200
Pekka Enberg <[email protected]> wrote:

> Jesse Barnes wrote:
> > On Wed, 13 Jan 2010 23:03:00 +0200
> > Pekka Enberg <[email protected]> wrote:
> >
> >> On Wed, Jan 13, 2010 at 7:44 AM, Linus Torvalds
> >> <[email protected]> wrote:
> >>> Hmm. Odd release. Something like 40% of the patches are in DRM
> >>> (mostly nouveau and radeon, both staging, so it's a bit less scary
> >>> than it sounds. But there's a noticeable i915 component too).
> >>> That's all pretty unusual, afaik.
> >> 2.6.33-rc4 still suffers from an annoying screen flicker with i915
> >> and KMS on my machine. I *think* it started with 2.6.33-rc1 but I
> >> haven't had the time to dig deeper.
> >
> > Does this patch fix it? If so I'll queue something like this up for
> > -rc5.
>
> Yes, it does. Thanks!

Great, thanks for testing. I'll send Eric a patch to disable this
feature more properly.

Thanks,
--
Jesse Barnes, Intel Open Source Technology Center

2010-01-14 19:16:06

[permalink] [raw]

Subject: Re: Linux 2.6.33-rc4

Jesse Barnes wrote:
> On Wed, 13 Jan 2010 23:50:52 +0200
> Pekka Enberg <[email protected]> wrote:
>
>> Jesse Barnes wrote:
>>> On Wed, 13 Jan 2010 23:03:00 +0200
>>> Pekka Enberg <[email protected]> wrote:
>>>
>>>> On Wed, Jan 13, 2010 at 7:44 AM, Linus Torvalds
>>>> <[email protected]> wrote:
>>>>> Hmm. Odd release. Something like 40% of the patches are in DRM
>>>>> (mostly nouveau and radeon, both staging, so it's a bit less scary
>>>>> than it sounds. But there's a noticeable i915 component too).
>>>>> That's all pretty unusual, afaik.
>>>> 2.6.33-rc4 still suffers from an annoying screen flicker with i915
>>>> and KMS on my machine. I *think* it started with 2.6.33-rc1 but I
>>>> haven't had the time to dig deeper.
>>> Does this patch fix it? If so I'll queue something like this up for
>>> -rc5.
>> Yes, it does. Thanks!
>
> Great, thanks for testing. I'll send Eric a patch to disable this
> feature more properly.

OK, I think I am seeing a different kind of flicker now. It doesn't
happen anywhere as often as before (which is probably why I missed it
last night). The flicker is more like a flash of some other window
whereas the flicker before used to "shake" the screen.

Does this ring a bell?

Pekka

2010-01-14 19:26:42

[permalink] [raw]

Subject: Re: Linux 2.6.33-rc4

On Thu, 14 Jan 2010 21:15:43 +0200
Pekka Enberg <[email protected]> wrote:

> Jesse Barnes wrote:
> > On Wed, 13 Jan 2010 23:50:52 +0200
> > Pekka Enberg <[email protected]> wrote:
> >
> >> Jesse Barnes wrote:
> >>> On Wed, 13 Jan 2010 23:03:00 +0200
> >>> Pekka Enberg <[email protected]> wrote:
> >>>
> >>>> On Wed, Jan 13, 2010 at 7:44 AM, Linus Torvalds
> >>>> <[email protected]> wrote:
> >>>>> Hmm. Odd release. Something like 40% of the patches are in DRM
> >>>>> (mostly nouveau and radeon, both staging, so it's a bit less
> >>>>> scary than it sounds. But there's a noticeable i915 component
> >>>>> too). That's all pretty unusual, afaik.
> >>>> 2.6.33-rc4 still suffers from an annoying screen flicker with
> >>>> i915 and KMS on my machine. I *think* it started with 2.6.33-rc1
> >>>> but I haven't had the time to dig deeper.
> >>> Does this patch fix it? If so I'll queue something like this up
> >>> for -rc5.
> >> Yes, it does. Thanks!
> >
> > Great, thanks for testing. I'll send Eric a patch to disable this
> > feature more properly.
>
> OK, I think I am seeing a different kind of flicker now. It doesn't
> happen anywhere as often as before (which is probably why I missed it
> last night). The flicker is more like a flash of some other window
> whereas the flicker before used to "shake" the screen.
>
> Does this ring a bell?

Sounds separate from the other issue. Would it be possible to capture
a video of the flicker? If it's a FIFO underrun, you may be able to
trigger it by running apps in the background that use a lot of memory
bandwidth.

If it's FBC related, it should occur in 15s intervals or so, depending
on whether you have other stuff going on (again would be memory
bandwidth related).

Please file a bug at bugs.freedesktop.org against drm/intel with what
you find.

Thanks,
--
Jesse Barnes, Intel Open Source Technology Center

2010-01-14 19:31:29

[permalink] [raw]

Subject: Re: Linux 2.6.33-rc4

On 21:15 Thu 14 Jan , Pekka Enberg wrote:
> OK, I think I am seeing a different kind of flicker now. It doesn't
> happen anywhere as often as before (which is probably why I missed it
> last night). The flicker is more like a flash of some other window
> whereas the flicker before used to "shake" the screen.
>
> Does this ring a bell?

Your symptoms sound identical to mine on my ThinkPad T500 laptop, as
described in http://bugs.freedesktop.org/show_bug.cgi?id=25809.

This is the patch I am currently applying locally which solves all my
problems.

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index f4b4aa2..4fb5911 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -914,7 +914,7 @@ static void intel_find_lvds_downclock(struct drm_device *dev,
mutex_unlock(&dev->mode_config.mutex);
if (temp_downclock < panel_fixed_mode->clock) {
/* We found the downclock for LVDS. */
- dev_priv->lvds_downclock_avail = 1;
+ dev_priv->lvds_downclock_avail = 0;
dev_priv->lvds_downclock = temp_downclock;
DRM_DEBUG_KMS("LVDS downclock is found in EDID. "
"Normal clock %dKhz, downclock %dKhz\n",

--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

2010-01-14 20:18:06

[permalink] [raw]

Subject: Re: Linux 2.6.33-rc4

On Thu, Jan 14, 2010 at 9:31 PM, Nick Bowler <[email protected]> wrote:
> On 21:15 Thu 14 Jan ? ? , Pekka Enberg wrote:
>> OK, I think I am seeing a different kind of flicker now. It doesn't
>> happen anywhere as often as before (which is probably why I missed it
>> last night). The flicker is more like a flash of some other window
>> whereas the flicker before used to "shake" the screen.
>>
>> Does this ring a bell?
>
> Your symptoms sound identical to mine on my ThinkPad T500 laptop, as
> described in http://bugs.freedesktop.org/show_bug.cgi?id=25809.
>
> This is the patch I am currently applying locally which solves all my
> problems.
>
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index f4b4aa2..4fb5911 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -914,7 +914,7 @@ static void intel_find_lvds_downclock(struct drm_device *dev,
> ? ? ? ?mutex_unlock(&dev->mode_config.mutex);
> ? ? ? ?if (temp_downclock < panel_fixed_mode->clock) {
> ? ? ? ? ? ? ? ?/* We found the downclock for LVDS. */
> - ? ? ? ? ? ? ? dev_priv->lvds_downclock_avail = 1;
> + ? ? ? ? ? ? ? dev_priv->lvds_downclock_avail = 0;
> ? ? ? ? ? ? ? ?dev_priv->lvds_downclock = temp_downclock;
> ? ? ? ? ? ? ? ?DRM_DEBUG_KMS("LVDS downclock is found in EDID. "
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Normal clock %dKhz, downclock %dKhz\n",

Should I try just this or combine it with Jesse's patch?

2010-01-14 20:28:11

[permalink] [raw]

Subject: Re: Linux 2.6.33-rc4

On 22:18 Thu 14 Jan , Pekka Enberg wrote:
> On Thu, Jan 14, 2010 at 9:31 PM, Nick Bowler <[email protected]> wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index f4b4aa2..4fb5911 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -914,7 +914,7 @@ static void intel_find_lvds_downclock(struct drm_device *dev,
> > ? ? ? ?mutex_unlock(&dev->mode_config.mutex);
> > ? ? ? ?if (temp_downclock < panel_fixed_mode->clock) {
> > ? ? ? ? ? ? ? ?/* We found the downclock for LVDS. */
> > - ? ? ? ? ? ? ? dev_priv->lvds_downclock_avail = 1;
> > + ? ? ? ? ? ? ? dev_priv->lvds_downclock_avail = 0;
> > ? ? ? ? ? ? ? ?dev_priv->lvds_downclock = temp_downclock;
> > ? ? ? ? ? ? ? ?DRM_DEBUG_KMS("LVDS downclock is found in EDID. "
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Normal clock %dKhz, downclock %dKhz\n",
>
> Should I try just this or combine it with Jesse's patch?

That's the only patch on top of mainline that I use -- it's been fixing
my problems since -rc1.

--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

2010-01-14 20:48:09

[permalink] [raw]

Subject: [PATCH] drm/i915: disable LVDS downclock by default

Many platform support this feature, and it can provide significant
power savings when the reduced refresh rate is low. However, on some
platforms a secondary (reduced) timing is provided but not actually
supported by the hardware. This results in undesirable flicker at
runtime.

So disable the feature by default, but allow users to opt-in to the
reduced clock behavior with a new module parameter, lvds_downclock,
that can be set to 1 to enable the feature.

Signed-off-by: Jesse Barnes <[email protected]>

--

Nick and Pekka, please try this out and reply with your Tested-by
(assuming it works, if not I'll fix it up).

Thanks,
Jesse

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 66f7bac..46d8896 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -45,6 +45,9 @@ module_param_named(fbpercrtc, i915_fbpercrtc, int, 0400);
unsigned int i915_powersave = 1;
module_param_named(powersave, i915_powersave, int, 0400);

+unsigned int i915_lvds_downclock = 0;
+module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400);
+
static struct drm_driver driver;

#define INTEL_VGA_DEVICE(id, info) { \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 445c49c..5f781a7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -722,6 +722,7 @@ extern struct drm_ioctl_desc i915_ioctls[];
extern int i915_max_ioctl;
extern unsigned int i915_fbpercrtc;
extern unsigned int i915_powersave;
+extern unsigned int i915_lvds_downclock;

extern void i915_save_display(struct drm_device *dev);
extern void i915_restore_display(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
index f275677..b53c46f 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -197,7 +197,8 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
memset(temp_mode, 0, sizeof(*temp_mode));
}
kfree(temp_mode);
- if (temp_downclock < panel_fixed_mode->clock) {
+ if (temp_downclock < panel_fixed_mode->clock &&
+ i915_lvds_downclock) {
dev_priv->lvds_downclock_avail = 1;
dev_priv->lvds_downclock = temp_downclock;
DRM_DEBUG_KMS("LVDS downclock is found in VBT. ",
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 5041590..aa74e59 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -926,7 +926,8 @@ static void intel_find_lvds_downclock(struct drm_device *dev,
}
}
mutex_unlock(&dev->mode_config.mutex);
- if (temp_downclock < panel_fixed_mode->clock) {
+ if (temp_downclock < panel_fixed_mode->clock &&
+ i915_lvds_downclock) {
/* We found the downclock for LVDS. */
dev_priv->lvds_downclock_avail = 1;
dev_priv->lvds_downclock = temp_downclock;

2010-01-14 20:58:40

by Peter Clifton

[permalink] [raw]

Subject: Re: [Intel-gfx] [PATCH] drm/i915: disable LVDS downclock by default

On Thu, 2010-01-14 at 12:48 -0800, Jesse Barnes wrote:
> Many platform support this feature, and it can provide significant
> power savings when the reduced refresh rate is low. However, on some
> platforms a secondary (reduced) timing is provided but not actually
> supported by the hardware. This results in undesirable flicker at
> runtime.
>
> So disable the feature by default, but allow users to opt-in to the
> reduced clock behavior with a new module parameter, lvds_downclock,
> that can be set to 1 to enable the feature.

Would it not be a better idea to turn this feature on by default, then
use quirks to disable it on the afflicted borken machines?

Requiring special module parameters to enable the feature, almost
guarantees that no normal end-users will end up benefiting from the
feature. Many of whom will have bought machines which don't have screwey
BIOS implementations.

I think (on a general note) that vendors supplying defective BIOSen or
config should be "named and shamed" in quirk tables - so eventually they
will get something done about the problems for future models.

2010-01-14 21:05:49

by Linus Torvalds

[permalink] [raw]

Subject: Re: [Intel-gfx] [PATCH] drm/i915: disable LVDS downclock by default

On Thu, 14 Jan 2010, Peter Clifton wrote:
>
> Would it not be a better idea to turn this feature on by default, then
> use quirks to disable it on the afflicted borken machines?

No.

"Working" is good. "Broken" is bad.

We don't default to clearly unsafe models.

Maybe a few years from now, when people have learnt to do power management
correctly and BIOSes fill in the fields right, we can reconsider. But
right now, it's _way_ more important that things work reliably.

That said, I think a module parameter is the wrong thing. If this can be
done dynamically with a sysfs value, do it that way instead (yes, I
realize that module parameters end up being also visible in /sys, but I
think Jesse's patch doesn't allow a person to set the value - and make it
change the behavior - _while_ the display is all up and running).

Linus

2010-01-14 21:16:20

[permalink] [raw]

Subject: Re: [Intel-gfx] [PATCH] drm/i915: disable LVDS downclock by default

On 20:58 Thu 14 Jan , Peter Clifton wrote:
> On Thu, 2010-01-14 at 12:48 -0800, Jesse Barnes wrote:
> > Many platform support this feature, and it can provide significant
> > power savings when the reduced refresh rate is low. However, on some
> > platforms a secondary (reduced) timing is provided but not actually
> > supported by the hardware. This results in undesirable flicker at
> > runtime.
> >
> > So disable the feature by default, but allow users to opt-in to the
> > reduced clock behavior with a new module parameter, lvds_downclock,
> > that can be set to 1 to enable the feature.
>
> Would it not be a better idea to turn this feature on by default, then
> use quirks to disable it on the afflicted borken machines?

If there is a high degree of confidence that correct quirks are in place
for all "afflicted borken machines", then this is probably OK.

The difference between 2.6.32 and 2.6.33-rc1 on the T500 is phenominal:
the LVDS display is so erratic in the latter as to be almost completely
useless. There is a patch on fdo bugzilla which makes the display less
broken, but there is still distracting flicker.

> Requiring special module parameters to enable the feature, almost
> guarantees that no normal end-users will end up benefiting from the
> feature. Many of whom will have bought machines which don't have screwey
> BIOS implementations.

On the other hand, it completely guarantees that no normal end-users
will end up with useless displays as a result of the feature. Many of
whom have bough machines which have screwey BIOS implementations (or
whatever the problem actually is).

> I think (on a general note) that vendors supplying defective BIOSen or
> config should be "named and shamed" in quirk tables - so eventually they
> will get something done about the problems for future models.

Is it really a defective BIOS? I don't have my laptop handy right now,
but the lower refresh mode is reported in the EDID and can be set
successfully (no idea if the change actually does anything). However,
there is a visible artifact whenever a transition occurs.

--
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)

2010-01-14 21:21:40

[permalink] [raw]

Subject: Re: [Intel-gfx] [PATCH] drm/i915: disable LVDS downclock by default

On Thu, 14 Jan 2010 13:05:17 -0800 (PST)
Linus Torvalds <[email protected]> wrote:

>
>
> On Thu, 14 Jan 2010, Peter Clifton wrote:
> >
> > Would it not be a better idea to turn this feature on by default,
> > then use quirks to disable it on the afflicted borken machines?
>
> No.
>
> "Working" is good. "Broken" is bad.
>
> We don't default to clearly unsafe models.
>
> Maybe a few years from now, when people have learnt to do power
> management correctly and BIOSes fill in the fields right, we can
> reconsider. But right now, it's _way_ more important that things work
> reliably.
>
> That said, I think a module parameter is the wrong thing. If this can
> be done dynamically with a sysfs value, do it that way instead (yes,
> I realize that module parameters end up being also visible in /sys,
> but I think Jesse's patch doesn't allow a person to set the value -
> and make it change the behavior - _while_ the display is all up and
> running).

Having a runtime option would be good, but we'd have to do some extra
code for that since the feature is either enabled or disabled at LVDS
and VBIOS parse time only.

With a sysfs interface we could allow the user to select the timing for
their reduced mode though, which would give them more power saving
potentially (if their panel worked with a very low refresh).

I can do that, but it seems more appropriate for 2.6.34? I'd just
remove the module param at that time I guess.

--
Jesse Barnes, Intel Open Source Technology Center

2010-01-14 21:26:00

[permalink] [raw]

Subject: Re: [PATCH] drm/i915: disable LVDS downclock by default

Hi Jesse,

On Thu, Jan 14, 2010 at 10:48 PM, Jesse Barnes <[email protected]> wrote:
> Many platform support this feature, and it can provide significant
> power savings when the reduced refresh rate is low. ?However, on some
> platforms a secondary (reduced) timing is provided but not actually
> supported by the hardware. ?This results in undesirable flicker at
> runtime.
>
> So disable the feature by default, but allow users to opt-in to the
> reduced clock behavior with a new module parameter, lvds_downclock,
> that can be set to 1 to enable the feature.
>
> Signed-off-by: Jesse Barnes <[email protected]>
>
> --
>
> Nick and Pekka, please try this out and reply with your Tested-by
> (assuming it works, if not I'll fix it up).

This gives me pretty much identical results as your original patch.
That is, the "shakes" are gone but I still see the occasional "flash".
Should I give Nick's patch a try to see if we're looking at the same
issue or wait for your new patch?

Pekka

2010-01-14 21:27:44

[permalink] [raw]

Subject: Re: [Intel-gfx] [PATCH] drm/i915: disable LVDS downclock by default

On Thu, 14 Jan 2010 16:16:10 -0500
Nick Bowler <[email protected]> wrote:

> Is it really a defective BIOS? I don't have my laptop handy right
> now, but the lower refresh mode is reported in the EDID and can be set
> successfully (no idea if the change actually does anything). However,
> there is a visible artifact whenever a transition occurs.

No, it's possible the BIOS has it right but we're failing to transition
gracefully.

Your platform should support automatic hardware frequency switching.
You could try disabling that (clear the has_pipe_cxsr field of your
driver struct in i915_drv.c) and see if the manual method works for you.

--
Jesse Barnes, Intel Open Source Technology Center

2010-01-14 22:06:10

[permalink] [raw]

Subject: Re: [PATCH] drm/i915: disable LVDS downclock by default

On Thu, 14 Jan 2010 23:25:57 +0200
Pekka Enberg <[email protected]> wrote:

> Hi Jesse,
>
> On Thu, Jan 14, 2010 at 10:48 PM, Jesse Barnes
> <[email protected]> wrote:
> > Many platform support this feature, and it can provide significant
> > power savings when the reduced refresh rate is low. However, on
> > some platforms a secondary (reduced) timing is provided but not
> > actually supported by the hardware. This results in undesirable
> > flicker at runtime.
> >
> > So disable the feature by default, but allow users to opt-in to the
> > reduced clock behavior with a new module parameter, lvds_downclock,
> > that can be set to 1 to enable the feature.
> >
> > Signed-off-by: Jesse Barnes <[email protected]>
> >
> > --
> >
> > Nick and Pekka, please try this out and reply with your Tested-by
> > (assuming it works, if not I'll fix it up).
>
> This gives me pretty much identical results as your original patch.
> That is, the "shakes" are gone but I still see the occasional "flash".
> Should I give Nick's patch a try to see if we're looking at the same
> issue or wait for your new patch?

Nick's patch should be the same, thanks for testing.

--
Jesse Barnes, Intel Open Source Technology Center

2010-01-15 01:16:28

[permalink] [raw]

Subject: Re: [PATCH] drm/i915: disable LVDS downclock by default

On Thu, 14 Jan 2010 12:48:02 -0800, Jesse Barnes wrote:
> Nick and Pekka, please try this out and reply with your Tested-by
> (assuming it works, if not I'll fix it up).

Not surprisingly, your patch solves the issues on my T500, and allows me
to re-enable the broken behaviour by adding the i915.lvds_downclock=1
boot option.

Tested-by: Nick Bowler <[email protected]>

2010-01-15 08:54:12

[permalink] [raw]

Subject: Re: [PATCH] drm/i915: disable LVDS downclock by default

Hi,

(Fixing CC.)

On Fri, Jan 15, 2010 at 3:15 AM, Nick Bowler <[email protected]> wrote:
> On Thu, 14 Jan 2010 12:48:02 -0800, Jesse Barnes wrote:
>> Nick and Pekka, please try this out and reply with your Tested-by
>> (assuming it works, if not I'll fix it up).
>
> Not surprisingly, your patch solves the issues on my T500, and allows me
> to re-enable the broken behaviour by adding the i915.lvds_downclock=1
> boot option.
>
> Tested-by: Nick Bowler <[email protected]>

So, any suggestions how I should proceed with figuring out the
"flashing" problem I still see with Jesse's patch? I can do a full git
bisect if it comes to that but it's going to be bit difficult to
determine if a commit is good or bad as there seem to be _two_
different problems both of which cause screen flickering.

Heelp!

Pekka

2010-01-15 16:15:34

by Thomas Meyer

[permalink] [raw]

Subject: Re: [PATCH] drm/i915: disable LVDS downclock by default

Am Freitag, den 15.01.2010, 10:54 +0200 schrieb Pekka Enberg:

> So, any suggestions how I should proceed with figuring out the
> "flashing" problem I still see with Jesse's patch? I can do a full git
> bisect if it comes to that but it's going to be bit difficult to
> determine if a commit is good or bad as there seem to be _two_
> different problems both of which cause screen flickering.
>
> Heelp!

Does it look like the video in this bug, see attached video in the bug
report:

http://bugzilla.kernel.org/show_bug.cgi?id=14670

greets
thomas

2010-01-15 16:21:43

[permalink] [raw]

Subject: Re: [PATCH] drm/i915: disable LVDS downclock by default

Hi Thomas,

On Fri, 2010-01-15 at 17:14 +0100, Thomas Meyer wrote:
> Am Freitag, den 15.01.2010, 10:54 +0200 schrieb Pekka Enberg:
>
> > So, any suggestions how I should proceed with figuring out the
> > "flashing" problem I still see with Jesse's patch? I can do a full git
> > bisect if it comes to that but it's going to be bit difficult to
> > determine if a commit is good or bad as there seem to be _two_
> > different problems both of which cause screen flickering.
> >
> > Heelp!
>
> Does it look like the video in this bug, see attached video in the bug
> report:
>
> http://bugzilla.kernel.org/show_bug.cgi?id=14670

No, that looks more like the "shaking" problem that's fixed by Nick's
and Jesse's patches. The "flashing" bug I'm talking about looks like I'm
seeing an old frame or something that's partially garbled with normal
output. The bug also appears much more infrequently than what your video
shows.

Pekka

2010-01-15 16:32:59