All of lore.kernel.org
 help / color / mirror / Atom feed
* Linux 2.6.37
@ 2011-01-05  1:15 Linus Torvalds
  2011-01-06 10:48 ` Michal Hocko
  0 siblings, 1 reply; 24+ messages in thread
From: Linus Torvalds @ 2011-01-05  1:15 UTC (permalink / raw)
  To: Linux Kernel Mailing List

It's early January, and bleary-eyed people everywhere are getting over
their hangovers and wondering where they should send their merge
requests to.

And now they can. Because 2.6.37 is out, and the merge window for the
next release is thus open. Of course, as usual, I'll probably let
2.6.37 cool for a few days to try to encourage people to look at the
release rather than go all crazy with newly merged features in the
next tree.

All in all, not a lot happened the last week. The diffstat is
dominated by some VFS locking documentation and a few updates to fix
up some ASoC codex register cache changes. The rest is really pretty
small and boring. Full shortlog (from -rc8, naturally - the shortlog
for the whole 2.6.37 release is much too big) appended.

We did have another revert to fix hopefullythe last "blank screen"
regression on intel graphics.

As to the big picture, on the whole I think 2.6.37 has been fairly
calm. That will likely change with the merge window, as I'm looking
forward to getting the new RCU-based pathname lookup and similar
exciting features. So enjoy the calm sanity while it lasts.

                    Linus

---
Andrew Morton (1):
      arch/mn10300/kernel/irq.c: fix build

Andy Walls (1):
      [media] cx25840: Prevent device probe failure due to volume
control ERANGE error

Aric D. Blumer (1):
      ARM: pxa: fix page table corruption on resume

Avi Kivity (2):
      KVM: MMU: Fix incorrect direct gfn for unpaged mode shadow
      KVM: i8259: initialize isr_ack

Axel Lin (1):
      ARM: 6605/1: Add missing include "asm/memory.h"

Ben Hutchings (2):
      starfire: Fix dma_addr_t size test for MIPS
      watchdog: Improve initialisation error message and documentation

Breno Leitao (1):
      ehea: Avoid changing vlan flags

Chris Wilson (4):
      drm/i915/sdvo: Add hdmi connector properties after initing the connector
      drm/i915: Verify Ironlake eDP presence on DP_A using the capability fuse
      Revert "drm/i915/bios: Reverse order of 100/120 Mhz SSC clocks"
      drm/i915/dvo: Report LVDS attached to ch701x as connected

Christoph Hellwig (2):
      update Documentation/filesystems/Locking
      remove trim_fs method from Documentation/filesystems/Locking

Dan Carpenter (1):
      skfp: testing the wrong variable in skfp_driver_init()

Dan Rosenberg (2):
      sound: Prevent buffer overflow in OSS load_mixer_volumes
      CAN: Use inode instead of kernel address for /proc file

Dan Williams (1):
      ueagle-atm: fix PHY signal initialization race

Daniel T Chen (1):
      ALSA: hda: Use LPIB quirk for Dell Inspiron m101z/1120

David Sterba (1):
      tg3: fix return value check in tg3_read_vpd()

Eric Anholt (2):
      drm/i915: Set the required VFMUNIT clock gating disable on Ironlake.
      drm/i915, intel_ips: When i915 loads after IPS, make IPS relink to i915.

Florian Westphal (1):
      bridge: stp: ensure mac header is set

Frederic Weisbecker (1):
      perf: Fix callchain hit bad cast on ascii display

Gregory CLEMENT (1):
      spi/omap2_mcspi.c: Force CS to be in inactive state after
off-mode transition

Guennadi Liakhovetski (1):
      dmaengine: provide dummy functions for DMA_ENGINE=n

Hans Verkuil (1):
      [media] em28xx: radio_fops should also use unlocked_ioctl

Hillf Danton (1):
      fix freeing user_struct in user cache

J. K. Cliburn (1):
      atl1: fix oops when changing tx/rx ring params

Jan Beulich (2):
      kconfig: fix undesirable side effect of adding "visible" menu attribute
      name_to_dev_t() must not call __init code

Jate Sujjavanich (1):
      spi/m68knommu: Coldfire QSPI platform support

Jesper Juhl (2):
      ISDN, Gigaset: Fix memory leak in do_disconnect_req()
      Broadcom CNIC core network driver: fix mem leak on allocation
failures in cnic_alloc_uio_rings()

Joel Sing (1):
      ipv4/route.c: respect prefsrc for local routes

Julia Lawall (1):
      drivers/atm/atmtcp.c: add missing atm_dev_put

KAMEZAWA Hiroyuki (1):
      memcg: fix wrong VM_BUG_ON() in try_charge()'s mm->owner check

Lars-Peter Clausen (9):
      ASoC: codecs: Add missing control_type initialization
      ASoC: codecs: max98088: Fix register cache incoherency
      ASoC: codecs: wm8523: Fix register cache incoherency
      ASoC: codecs: wm8741: Fix register cache incoherency
      ASoC: codecs: wm8904: Fix register cache incoherency
      ASoC: codecs: wm8955: Fix register cache incoherency
      ASoC: codecs: wm8962: Fix register cache incoherency
      ASoC: codecs: wm9090: Fix register cache incoherency
      ASoC: codecs: wm8753: Fix register cache incoherency

Lennert Buytenhek (2):
      ARM: pxa: PXA_ESERIES depends on FB_W100.
      ARM: pxa: PXA_ESERIES depends on FB_W100.

Linus Torvalds (1):
      Linux 2.6.37

Linus Walleij (1):
      ARM: 6537/1: update Nomadik, U300 and Ux500 maintainers

Mauro Carvalho Chehab (1):
      [media] wm8775: Revert changeset fcb9757333 to avoid a regression

Maurus Cuelenaere (1):
      hwmon: (s3c-hwmon) Fix compilation

Mike Rapoport (1):
      ARM: it8152: add IT8152_LAST_IRQ definition to fix build error

Mimi Zohar (1):
      ima: fix add LSM rule bug

Nicolas Pitre (3):
      ARM: get rid of kmap_high_l1_vipt()
      ARM: fix cache-xsc3l2 after stack based kmap_atomic()
      ARM: fix cache-feroceon-l2 after stack based kmap_atomic()

Nitin Gupta (1):
      Revert "Staging: zram: work around oops due to startup ordering snafu"

Robert Richter (1):
      arch/x86/oprofile/op_model_amd.c: Perform initialisation on a single CPU

Russell King (1):
      ARM: smp: avoid incrementing mm_users on CPU startup

Saeed Bishara (1):
      mv_xor: fix race in tasklet function

Stephen Warren (1):
      ARM: 6536/1: Add missing SZ_{32,64,128}

Todd Android Poynor (1):
      ARM: 6540/1: Stop irqsoff trace on return to user

Tomas Winkler (1):
      bridge: fix br_multicast_ipv6_rcv for paged skbs

stephen hemminger (1):
      ppp: allow disabling multilink protocol ID compression

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-05  1:15 Linux 2.6.37 Linus Torvalds
@ 2011-01-06 10:48 ` Michal Hocko
  2011-01-06 16:29   ` Linus Torvalds
                     ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Michal Hocko @ 2011-01-06 10:48 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Linux Kernel Mailing List, Chris Wilson, dri-devel

Hi,

On Tue 04-01-11 17:15:45, Linus Torvalds wrote:
[...]
> We did have another revert to fix hopefullythe last "blank screen"
> regression on intel graphics.

It seems that there is still a regression for intel graphic cards
backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
I can reproduce the problem easily by:
xset dpms force standby; sleep 3s; xset dpms force on

backlight doesn't get up (there is really dark picture though which
doesn't get brighter by function keys which work normally) after dpms on
until I close and open lid. 

The problem wasn't present in 2.6.36

$ lspci -vv
[...]
00:02.0 VGA compatible controller: Intel Corporation Mobile 945GM/GMS, 943/940GML Express Integrated Graphics Controller (rev 03) (prog-if 00 [VGA controller])
        Subsystem: Fujitsu Limited. Device 137a
        Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
        Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
        Latency: 0
        Interrupt: pin A routed to IRQ 16
        Region 0: Memory at f0300000 (32-bit, non-prefetchable) [size=512K]
        Region 1: I/O ports at 1800 [size=8]
        Region 2: Memory at e0000000 (32-bit, prefetchable) [size=256M]
        Region 3: Memory at f0400000 (32-bit, non-prefetchable) [size=256K]
        Expansion ROM at <unassigned> [disabled]
        Capabilities: <access denied>
        Kernel driver in use: i915
[...]
-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-06 10:48 ` Michal Hocko
@ 2011-01-06 16:29   ` Linus Torvalds
  2011-01-06 17:03       ` Michal Hocko
  2011-01-06 17:49     ` Chris Wilson
  2011-01-11 13:33   ` Pavel Machek
  2011-01-11 17:19   ` Michal Hocko
  2 siblings, 2 replies; 24+ messages in thread
From: Linus Torvalds @ 2011-01-06 16:29 UTC (permalink / raw)
  To: Michal Hocko; +Cc: Linux Kernel Mailing List, Chris Wilson, dri-devel

On Thu, Jan 6, 2011 at 2:48 AM, Michal Hocko <mhocko@suse.cz> wrote:
>
> It seems that there is still a regression for intel graphic cards
> backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> I can reproduce the problem easily by:
> xset dpms force standby; sleep 3s; xset dpms force on
>
> backlight doesn't get up (there is really dark picture though which
> doesn't get brighter by function keys which work normally) after dpms on
> until I close and open lid.

Hmm. That commit no longer reverts cleanly, so it's not trivial to
test whether all those things are exactly the same issue. It's been
bisected in the bugzilla entry, but it would be good to verify that
yes, reverting it really does fix the issue, and your issue is the
exact same one.

Chris, any ideas?

                       Linus

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-06 16:29   ` Linus Torvalds
@ 2011-01-06 17:03       ` Michal Hocko
  2011-01-06 17:49     ` Chris Wilson
  1 sibling, 0 replies; 24+ messages in thread
From: Michal Hocko @ 2011-01-06 17:03 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Linux Kernel Mailing List, Chris Wilson, dri-devel

Just for reference, my initial report was:
https://lkml.org/lkml/2010/11/23/146

On Thu 06-01-11 08:29:22, Linus Torvalds wrote:
> On Thu, Jan 6, 2011 at 2:48 AM, Michal Hocko <mhocko@suse.cz> wrote:
> >
> > It seems that there is still a regression for intel graphic cards
> > backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> > I can reproduce the problem easily by:
> > xset dpms force standby; sleep 3s; xset dpms force on
> >
> > backlight doesn't get up (there is really dark picture though which
> > doesn't get brighter by function keys which work normally) after dpms on
> > until I close and open lid.
> 
> Hmm. That commit no longer reverts cleanly, so it's not trivial to
> test whether all those things are exactly the same issue. It's been
> bisected in the bugzilla entry, but it would be good to verify that
> yes, reverting it really does fix the issue, and your issue is the
> exact same one.
> 
> Chris, any ideas?
> 
>                        Linus

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
@ 2011-01-06 17:03       ` Michal Hocko
  0 siblings, 0 replies; 24+ messages in thread
From: Michal Hocko @ 2011-01-06 17:03 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Linux Kernel Mailing List, dri-devel

Just for reference, my initial report was:
https://lkml.org/lkml/2010/11/23/146

On Thu 06-01-11 08:29:22, Linus Torvalds wrote:
> On Thu, Jan 6, 2011 at 2:48 AM, Michal Hocko <mhocko@suse.cz> wrote:
> >
> > It seems that there is still a regression for intel graphic cards
> > backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> > I can reproduce the problem easily by:
> > xset dpms force standby; sleep 3s; xset dpms force on
> >
> > backlight doesn't get up (there is really dark picture though which
> > doesn't get brighter by function keys which work normally) after dpms on
> > until I close and open lid.
> 
> Hmm. That commit no longer reverts cleanly, so it's not trivial to
> test whether all those things are exactly the same issue. It's been
> bisected in the bugzilla entry, but it would be good to verify that
> yes, reverting it really does fix the issue, and your issue is the
> exact same one.
> 
> Chris, any ideas?
> 
>                        Linus

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-06 16:29   ` Linus Torvalds
  2011-01-06 17:03       ` Michal Hocko
@ 2011-01-06 17:49     ` Chris Wilson
  2011-01-06 20:55       ` Alex Riesen
  1 sibling, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2011-01-06 17:49 UTC (permalink / raw)
  To: Linus Torvalds, Michal Hocko; +Cc: Linux Kernel Mailing List, dri-devel

On Thu, 6 Jan 2011 08:29:22 -0800, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Thu, Jan 6, 2011 at 2:48 AM, Michal Hocko <mhocko@suse.cz> wrote:
> >
> > It seems that there is still a regression for intel graphic cards
> > backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> > I can reproduce the problem easily by:
> > xset dpms force standby; sleep 3s; xset dpms force on
> >
> > backlight doesn't get up (there is really dark picture though which
> > doesn't get brighter by function keys which work normally) after dpms on
> > until I close and open lid.
> 
> Hmm. That commit no longer reverts cleanly, so it's not trivial to
> test whether all those things are exactly the same issue. It's been
> bisected in the bugzilla entry, but it would be good to verify that
> yes, reverting it really does fix the issue, and your issue is the
> exact same one.
> 
> Chris, any ideas?

My fear is that some machines have a dependency between the backlight
and panel power status. The patch in question changed the timing between
turning on the panel and adjusting the backlight which would be restore
with:

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index aa23070..0b40b4f 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -106,6 +106,12 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
 	I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
 	POSTING_READ(lvds_reg);
 
+	{
+		u32 reg = HAS_PCH_SPLIT(dev) ? PCH_PP_STATUS : PPS_STATUS;
+		if (wait_for(I915_READ(reg) & PP_ON, 1000))
+			DRM_ERROR("timed out waiting for panel to power up\n");
+	}
+
 	intel_panel_set_backlight(dev, dev_priv->backlight_level);
 }

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-06 17:49     ` Chris Wilson
@ 2011-01-06 20:55       ` Alex Riesen
  2011-01-06 21:03           ` Chris Wilson
  2011-01-06 21:08         ` Alex Riesen
  0 siblings, 2 replies; 24+ messages in thread
From: Alex Riesen @ 2011-01-06 20:55 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Linus Torvalds, Michal Hocko, Linux Kernel Mailing List, dri-devel

On Thu, Jan 6, 2011 at 18:49, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>
> My fear is that some machines have a dependency between the backlight
> and panel power status. The patch in question changed the timing between
> turning on the panel and adjusting the backlight which would be restore
> with:
>
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index aa23070..0b40b4f 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -106,6 +106,12 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
>        I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
>        POSTING_READ(lvds_reg);
>
> +       {
> +               u32 reg = HAS_PCH_SPLIT(dev) ? PCH_PP_STATUS : PPS_STATUS;
> +               if (wait_for(I915_READ(reg) & PP_ON, 1000))
> +                       DRM_ERROR("timed out waiting for panel to power up\n");
> +       }
> +
>        intel_panel_set_backlight(dev, dev_priv->backlight_level);
>  }

FWIW it does not compile:
  CC      drivers/gpu/drm/i915/intel_lvds.o
drivers/gpu/drm/i915/intel_lvds.c: In function ‘intel_lvds_enable’:
drivers/gpu/drm/i915/intel_lvds.c:110: error: ‘PPS_STATUS’ undeclared
(first use in this function)
drivers/gpu/drm/i915/intel_lvds.c:110: error: (Each undeclared
identifier is reported only once
drivers/gpu/drm/i915/intel_lvds.c:110: error: for each function it appears in.)
make[4]: *** [drivers/gpu/drm/i915/intel_lvds.o] Error 1

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-06 20:55       ` Alex Riesen
@ 2011-01-06 21:03           ` Chris Wilson
  2011-01-06 21:08         ` Alex Riesen
  1 sibling, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2011-01-06 21:03 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Linus Torvalds, Michal Hocko, Linux Kernel Mailing List, dri-devel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1819 bytes --]

On Thu, 6 Jan 2011 21:55:23 +0100, Alex Riesen <raa.lkml@gmail.com> wrote:
> On Thu, Jan 6, 2011 at 18:49, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > My fear is that some machines have a dependency between the backlight
> > and panel power status. The patch in question changed the timing between
> > turning on the panel and adjusting the backlight which would be restore
> > with:
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index aa23070..0b40b4f 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -106,6 +106,12 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
> >        I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
> >        POSTING_READ(lvds_reg);
> >
> > +       {
> > +               u32 reg = HAS_PCH_SPLIT(dev) ? PCH_PP_STATUS : PPS_STATUS;
> > +               if (wait_for(I915_READ(reg) & PP_ON, 1000))
> > +                       DRM_ERROR("timed out waiting for panel to power up\n");
> > +       }
> > +
> >        intel_panel_set_backlight(dev, dev_priv->backlight_level);
> >  }
> 
> FWIW it does not compile:
>   CC      drivers/gpu/drm/i915/intel_lvds.o
> drivers/gpu/drm/i915/intel_lvds.c: In function ‘intel_lvds_enable’:
> drivers/gpu/drm/i915/intel_lvds.c:110: error: ‘PPS_STATUS’ undeclared
> (first use in this function)
> drivers/gpu/drm/i915/intel_lvds.c:110: error: (Each undeclared
> identifier is reported only once
> drivers/gpu/drm/i915/intel_lvds.c:110: error: for each function it appears in.)
> make[4]: *** [drivers/gpu/drm/i915/intel_lvds.o] Error 1

Daniel quickly pointed out my typo: s/PPS_STATUS/PP_STATUS/

Apologies,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
@ 2011-01-06 21:03           ` Chris Wilson
  0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2011-01-06 21:03 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Linus Torvalds, Michal Hocko, Linux Kernel Mailing List, dri-devel

On Thu, 6 Jan 2011 21:55:23 +0100, Alex Riesen <raa.lkml@gmail.com> wrote:
> On Thu, Jan 6, 2011 at 18:49, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >
> > My fear is that some machines have a dependency between the backlight
> > and panel power status. The patch in question changed the timing between
> > turning on the panel and adjusting the backlight which would be restore
> > with:
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index aa23070..0b40b4f 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -106,6 +106,12 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
> >        I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
> >        POSTING_READ(lvds_reg);
> >
> > +       {
> > +               u32 reg = HAS_PCH_SPLIT(dev) ? PCH_PP_STATUS : PPS_STATUS;
> > +               if (wait_for(I915_READ(reg) & PP_ON, 1000))
> > +                       DRM_ERROR("timed out waiting for panel to power up\n");
> > +       }
> > +
> >        intel_panel_set_backlight(dev, dev_priv->backlight_level);
> >  }
> 
> FWIW it does not compile:
>   CC      drivers/gpu/drm/i915/intel_lvds.o
> drivers/gpu/drm/i915/intel_lvds.c: In function ‘intel_lvds_enable’:
> drivers/gpu/drm/i915/intel_lvds.c:110: error: ‘PPS_STATUS’ undeclared
> (first use in this function)
> drivers/gpu/drm/i915/intel_lvds.c:110: error: (Each undeclared
> identifier is reported only once
> drivers/gpu/drm/i915/intel_lvds.c:110: error: for each function it appears in.)
> make[4]: *** [drivers/gpu/drm/i915/intel_lvds.o] Error 1

Daniel quickly pointed out my typo: s/PPS_STATUS/PP_STATUS/

Apologies,
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-06 20:55       ` Alex Riesen
  2011-01-06 21:03           ` Chris Wilson
@ 2011-01-06 21:08         ` Alex Riesen
  2011-01-07 15:52           ` Michal Hocko
  1 sibling, 1 reply; 24+ messages in thread
From: Alex Riesen @ 2011-01-06 21:08 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Linus Torvalds, Michal Hocko, Linux Kernel Mailing List, dri-devel

On Thu, Jan 6, 2011 at 21:55, Alex Riesen <raa.lkml@gmail.com> wrote:
> On Thu, Jan 6, 2011 at 18:49, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>>
>> My fear is that some machines have a dependency between the backlight
>> and panel power status. The patch in question changed the timing between
>> turning on the panel and adjusting the backlight which would be restore
>> with:
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
>> index aa23070..0b40b4f 100644
>> --- a/drivers/gpu/drm/i915/intel_lvds.c
>> +++ b/drivers/gpu/drm/i915/intel_lvds.c
>> @@ -106,6 +106,12 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
>>        I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
>>        POSTING_READ(lvds_reg);
>>
>> +       {
>> +               u32 reg = HAS_PCH_SPLIT(dev) ? PCH_PP_STATUS : PPS_STATUS;
...
> FWIW it does not compile:
>  CC      drivers/gpu/drm/i915/intel_lvds.o
> drivers/gpu/drm/i915/intel_lvds.c: In function ‘intel_lvds_enable’:
> drivers/gpu/drm/i915/intel_lvds.c:110: error: ‘PPS_STATUS’ undeclared

Ah, I see. Should be PP_STATUS. Whatever. It does not help. The backlight
stays off.

P.S. Probably unrelated, but I just noticed that the backlight never goes
off when closing the lid. Am I supposed to hook up on the corresponding
input event and put the panel in standby? It used to work all by itself,
I think...

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-06 21:08         ` Alex Riesen
@ 2011-01-07 15:52           ` Michal Hocko
  0 siblings, 0 replies; 24+ messages in thread
From: Michal Hocko @ 2011-01-07 15:52 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Chris Wilson, Linus Torvalds, Linux Kernel Mailing List, dri-devel

On Thu 06-01-11 22:08:46, Alex Riesen wrote:
> On Thu, Jan 6, 2011 at 21:55, Alex Riesen <raa.lkml@gmail.com> wrote:
> > On Thu, Jan 6, 2011 at 18:49, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> >>
> >> My fear is that some machines have a dependency between the backlight
> >> and panel power status. The patch in question changed the timing between
> >> turning on the panel and adjusting the backlight which would be restore
> >> with:
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> >> index aa23070..0b40b4f 100644
> >> --- a/drivers/gpu/drm/i915/intel_lvds.c
> >> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> >> @@ -106,6 +106,12 @@ static void intel_lvds_enable(struct intel_lvds *intel_lvds)
> >> ?? ?? ?? ??I915_WRITE(ctl_reg, I915_READ(ctl_reg) | POWER_TARGET_ON);
> >> ?? ?? ?? ??POSTING_READ(lvds_reg);
> >>
> >> + ?? ?? ?? {
> >> + ?? ?? ?? ?? ?? ?? ?? u32 reg = HAS_PCH_SPLIT(dev) ? PCH_PP_STATUS : PPS_STATUS;
> ...
> > FWIW it does not compile:
> > ??CC ?? ?? ??drivers/gpu/drm/i915/intel_lvds.o
> > drivers/gpu/drm/i915/intel_lvds.c: In function ???intel_lvds_enable???:
> > drivers/gpu/drm/i915/intel_lvds.c:110: error: ???PPS_STATUS??? undeclared
> 
> Ah, I see. Should be PP_STATUS. Whatever. It does not help. The backlight
> stays off.

It didn't help in my case either

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-06 10:48 ` Michal Hocko
  2011-01-06 16:29   ` Linus Torvalds
@ 2011-01-11 13:33   ` Pavel Machek
  2011-01-11 14:28     ` Michal Hocko
  2011-01-11 14:47     ` Alex Riesen
  2011-01-11 17:19   ` Michal Hocko
  2 siblings, 2 replies; 24+ messages in thread
From: Pavel Machek @ 2011-01-11 13:33 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Linus Torvalds, Linux Kernel Mailing List, Chris Wilson, dri-devel

> Hi,
> 
> On Tue 04-01-11 17:15:45, Linus Torvalds wrote:
> [...]
> > We did have another revert to fix hopefullythe last "blank screen"
> > regression on intel graphics.
> 
> It seems that there is still a regression for intel graphic cards
> backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> I can reproduce the problem easily by:
> xset dpms force standby; sleep 3s; xset dpms force on

(You are using "vesa" or "fbcon" X11 driver, right? I seen same problem
until I switched to "intel" X11 driver).

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-11 13:33   ` Pavel Machek
@ 2011-01-11 14:28     ` Michal Hocko
  2011-01-11 14:47     ` Alex Riesen
  1 sibling, 0 replies; 24+ messages in thread
From: Michal Hocko @ 2011-01-11 14:28 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Linus Torvalds, Linux Kernel Mailing List, Chris Wilson, dri-devel

On Tue 11-01-11 14:33:20, Pavel Machek wrote:
> > Hi,
> > 
> > On Tue 04-01-11 17:15:45, Linus Torvalds wrote:
> > [...]
> > > We did have another revert to fix hopefullythe last "blank screen"
> > > regression on intel graphics.
> > 
> > It seems that there is still a regression for intel graphic cards
> > backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> > I can reproduce the problem easily by:
> > xset dpms force standby; sleep 3s; xset dpms force on
> 
> (You are using "vesa" or "fbcon" X11 driver, right? I seen same problem
> until I switched to "intel" X11 driver).

I am using intel X11 driver:
[...]
(II) Loading extension DRI2
(II) LoadModule: "intel"
(II) Loading /usr/lib/xorg/modules/drivers/intel_drv.so
[...]

So this doesn't look like the case.

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-11 13:33   ` Pavel Machek
  2011-01-11 14:28     ` Michal Hocko
@ 2011-01-11 14:47     ` Alex Riesen
  1 sibling, 0 replies; 24+ messages in thread
From: Alex Riesen @ 2011-01-11 14:47 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Michal Hocko, Linus Torvalds, Linux Kernel Mailing List,
	Chris Wilson, dri-devel

On Tue, Jan 11, 2011 at 14:33, Pavel Machek <pavel@ucw.cz> wrote:
>> I can reproduce the problem easily by:
>> xset dpms force standby; sleep 3s; xset dpms force on
>
> (You are using "vesa" or "fbcon" X11 driver, right? I seen same problem
> until I switched to "intel" X11 driver).

No, the "intel".

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-06 10:48 ` Michal Hocko
  2011-01-06 16:29   ` Linus Torvalds
  2011-01-11 13:33   ` Pavel Machek
@ 2011-01-11 17:19   ` Michal Hocko
  2011-01-11 17:37     ` Michal Hocko
  2011-01-11 17:39     ` Chris Wilson
  2 siblings, 2 replies; 24+ messages in thread
From: Michal Hocko @ 2011-01-11 17:19 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Linux Kernel Mailing List, Chris Wilson, dri-devel, indan

[Let's CC Indan - author of the bugzilla patches]

On Thu 06-01-11 11:48:16, Michal Hocko wrote:
> Hi,
> 
> On Tue 04-01-11 17:15:45, Linus Torvalds wrote:
> [...]
> > We did have another revert to fix hopefullythe last "blank screen"
> > regression on intel graphics.
> 
> It seems that there is still a regression for intel graphic cards
> backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> I can reproduce the problem easily by:
> xset dpms force standby; sleep 3s; xset dpms force on
> 
> backlight doesn't get up (there is really dark picture though which
> doesn't get brighter by function keys which work normally) after dpms on
> until I close and open lid. 
> 
> The problem wasn't present in 2.6.36

I can confirm that this problem is not present if both patches from
bko#22672 are applied on top of 2.6.37 kernel.
I haven't tried both patches separately, but I can surely try it if it
makes any sense.

> 
> $ lspci -vv
> [...]
> 00:02.0 VGA compatible controller: Intel Corporation Mobile 945GM/GMS, 943/940GML Express Integrated Graphics Controller (rev 03) (prog-if 00 [VGA controller])
>         Subsystem: Fujitsu Limited. Device 137a
>         Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
>         Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
>         Latency: 0
>         Interrupt: pin A routed to IRQ 16
>         Region 0: Memory at f0300000 (32-bit, non-prefetchable) [size=512K]
>         Region 1: I/O ports at 1800 [size=8]
>         Region 2: Memory at e0000000 (32-bit, prefetchable) [size=256M]
>         Region 3: Memory at f0400000 (32-bit, non-prefetchable) [size=256K]
>         Expansion ROM at <unassigned> [disabled]
>         Capabilities: <access denied>
>         Kernel driver in use: i915
[...]

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-11 17:19   ` Michal Hocko
@ 2011-01-11 17:37     ` Michal Hocko
  2011-01-11 18:37       ` Michal Hocko
  2011-01-11 17:39     ` Chris Wilson
  1 sibling, 1 reply; 24+ messages in thread
From: Michal Hocko @ 2011-01-11 17:37 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Linux Kernel Mailing List, Chris Wilson, dri-devel, indan

On Tue 11-01-11 18:17:44, Michal Hocko wrote:
> [Let's CC Indan - author of the bugzilla patches]
> 
> On Thu 06-01-11 11:48:16, Michal Hocko wrote:
> > Hi,
> > 
> > On Tue 04-01-11 17:15:45, Linus Torvalds wrote:
> > [...]
> > > We did have another revert to fix hopefullythe last "blank screen"
> > > regression on intel graphics.
> > 
> > It seems that there is still a regression for intel graphic cards
> > backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> > I can reproduce the problem easily by:
> > xset dpms force standby; sleep 3s; xset dpms force on
> > 
> > backlight doesn't get up (there is really dark picture though which
> > doesn't get brighter by function keys which work normally) after dpms on
> > until I close and open lid. 
> > 
> > The problem wasn't present in 2.6.36
> 
> I can confirm that this problem is not present if both patches from
> bko#22672 are applied on top of 2.6.37 kernel.
> I haven't tried both patches separately, but I can surely try it if it
> makes any sense.

I have just tried that and they are really both necessary.


-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-11 17:19   ` Michal Hocko
  2011-01-11 17:37     ` Michal Hocko
@ 2011-01-11 17:39     ` Chris Wilson
  2011-01-11 17:44       ` Michal Hocko
  2011-01-12  0:35       ` Indan Zupancic
  1 sibling, 2 replies; 24+ messages in thread
From: Chris Wilson @ 2011-01-11 17:39 UTC (permalink / raw)
  To: Michal Hocko, Linus Torvalds; +Cc: Linux Kernel Mailing List, dri-devel, indan

On Tue, 11 Jan 2011 18:19:17 +0100, Michal Hocko <mhocko@suse.cz> wrote:
> [Let's CC Indan - author of the bugzilla patches]
> 
> On Thu 06-01-11 11:48:16, Michal Hocko wrote:
> > Hi,
> > 
> > On Tue 04-01-11 17:15:45, Linus Torvalds wrote:
> > [...]
> > > We did have another revert to fix hopefullythe last "blank screen"
> > > regression on intel graphics.
> > 
> > It seems that there is still a regression for intel graphic cards
> > backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> > I can reproduce the problem easily by:
> > xset dpms force standby; sleep 3s; xset dpms force on
> > 
> > backlight doesn't get up (there is really dark picture though which
> > doesn't get brighter by function keys which work normally) after dpms on
> > until I close and open lid. 
> > 
> > The problem wasn't present in 2.6.36
> 
> I can confirm that this problem is not present if both patches from
> bko#22672 are applied on top of 2.6.37 kernel.
> I haven't tried both patches separately, but I can surely try it if it
> makes any sense.

The second patch is wrong in that it will prevent changing resolutions
using the panel fitter. The first patch looks along the right lines, just
misses the possibility that the backlight can be modified by other means.

So hopefully, you just need the first patch.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-11 17:39     ` Chris Wilson
@ 2011-01-11 17:44       ` Michal Hocko
  2011-01-12  0:35       ` Indan Zupancic
  1 sibling, 0 replies; 24+ messages in thread
From: Michal Hocko @ 2011-01-11 17:44 UTC (permalink / raw)
  To: Chris Wilson; +Cc: Linus Torvalds, Linux Kernel Mailing List, dri-devel, indan

On Tue 11-01-11 17:39:42, Chris Wilson wrote:
> On Tue, 11 Jan 2011 18:19:17 +0100, Michal Hocko <mhocko@suse.cz> wrote:
> > [Let's CC Indan - author of the bugzilla patches]
> > 
> > On Thu 06-01-11 11:48:16, Michal Hocko wrote:
> > > Hi,
> > > 
> > > On Tue 04-01-11 17:15:45, Linus Torvalds wrote:
> > > [...]
> > > > We did have another revert to fix hopefullythe last "blank screen"
> > > > regression on intel graphics.
> > > 
> > > It seems that there is still a regression for intel graphic cards
> > > backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> > > I can reproduce the problem easily by:
> > > xset dpms force standby; sleep 3s; xset dpms force on
> > > 
> > > backlight doesn't get up (there is really dark picture though which
> > > doesn't get brighter by function keys which work normally) after dpms on
> > > until I close and open lid. 
> > > 
> > > The problem wasn't present in 2.6.36
> > 
> > I can confirm that this problem is not present if both patches from
> > bko#22672 are applied on top of 2.6.37 kernel.
> > I haven't tried both patches separately, but I can surely try it if it
> > makes any sense.
> 
> The second patch is wrong in that it will prevent changing resolutions
> using the panel fitter. The first patch looks along the right lines, just
> misses the possibility that the backlight can be modified by other means.
> 
> So hopefully, you just need the first patch.

I would have bet I have tested the 1st patch but let me double check.
The 2nd patch alone doesn't fix the problem.

> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-11 17:37     ` Michal Hocko
@ 2011-01-11 18:37       ` Michal Hocko
  0 siblings, 0 replies; 24+ messages in thread
From: Michal Hocko @ 2011-01-11 18:37 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Linux Kernel Mailing List, Chris Wilson, dri-devel, indan

On Tue 11-01-11 18:37:41, Michal Hocko wrote:
> On Tue 11-01-11 18:17:44, Michal Hocko wrote:
> > [Let's CC Indan - author of the bugzilla patches]
> > 
> > On Thu 06-01-11 11:48:16, Michal Hocko wrote:
> > > Hi,
> > > 
> > > On Tue 04-01-11 17:15:45, Linus Torvalds wrote:
> > > [...]
> > > > We did have another revert to fix hopefullythe last "blank screen"
> > > > regression on intel graphics.
> > > 
> > > It seems that there is still a regression for intel graphic cards
> > > backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> > > I can reproduce the problem easily by:
> > > xset dpms force standby; sleep 3s; xset dpms force on
> > > 
> > > backlight doesn't get up (there is really dark picture though which
> > > doesn't get brighter by function keys which work normally) after dpms on
> > > until I close and open lid. 
> > > 
> > > The problem wasn't present in 2.6.36
> > 
> > I can confirm that this problem is not present if both patches from
> > bko#22672 are applied on top of 2.6.37 kernel.
> > I haven't tried both patches separately, but I can surely try it if it
> > makes any sense.
> 
> I have just tried that and they are really both necessary.

I must have mixed something. After retesting with the first patch
(https://bugzilla.kernel.org/show_bug.cgi?id=22672#c33) the usecase
works just fine (backlight is on without need to close&open the lid)

Sorry for confusion

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-11 17:39     ` Chris Wilson
  2011-01-11 17:44       ` Michal Hocko
@ 2011-01-12  0:35       ` Indan Zupancic
  2011-01-12 12:07         ` Chris Wilson
  1 sibling, 1 reply; 24+ messages in thread
From: Indan Zupancic @ 2011-01-12  0:35 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Michal Hocko, Linus Torvalds, Linux Kernel Mailing List, dri-devel

Hello,

On Tue, January 11, 2011 18:39, Chris Wilson wrote:
> On Tue, 11 Jan 2011 18:19:17 +0100, Michal Hocko <mhocko@suse.cz> wrote:
>> [Let's CC Indan - author of the bugzilla patches]
>>
>> On Thu 06-01-11 11:48:16, Michal Hocko wrote:
>> > Hi,
>> >
>> > On Tue 04-01-11 17:15:45, Linus Torvalds wrote:
>> > [...]
>> > > We did have another revert to fix hopefullythe last "blank screen"
>> > > regression on intel graphics.
>> >
>> > It seems that there is still a regression for intel graphic cards
>> > backlight. One report is https://bugzilla.kernel.org/show_bug.cgi?id=22672.
>> > I can reproduce the problem easily by:
>> > xset dpms force standby; sleep 3s; xset dpms force on
>> >
>> > backlight doesn't get up (there is really dark picture though which
>> > doesn't get brighter by function keys which work normally) after dpms on
>> > until I close and open lid.
>> >
>> > The problem wasn't present in 2.6.36
>>
>> I can confirm that this problem is not present if both patches from
>> bko#22672 are applied on top of 2.6.37 kernel.
>> I haven't tried both patches separately, but I can surely try it if it
>> makes any sense.
>
> The second patch is wrong in that it will prevent changing resolutions
> using the panel fitter.

I can confirm that with the second patch changing resolutions doesn't always go right.

$ xrandr
Screen 0: minimum 320 x 200, current 1024 x 768, maximum 2048 x 2048
LVDS1 connected 1024x768+0+0 (normal left inverted right x axis y axis) 0mm x 0mm
   1024x768       50.0*+   85.0     75.0     70.1     60.0
   832x624        74.6
   800x600        85.1     72.2     75.0     60.3     56.2
   640x480        85.0     72.8     75.0     59.9
   720x400        85.0
   640x400        85.1
   640x350        85.1
VGA1 disconnected (normal left inverted right x axis y axis)

Going from xrandr -s 1, 2 or 3 back to 0 works, but not from 4, 5 or 6 to 0.

The second patch was really a stab in the dark, I'm happy it was in the right
direction at least.

> The first patch looks along the right lines, just
> misses the possibility that the backlight can be modified by other means.

I'm not sure about that. All it does is avoiding setting backlight_level to
0. If it's modified some other way and intel_panel_get_backlight() returns 0,
backlight_level is just never set and will stay zero. If there is a way to
switch between ways to modify the backlight then I can see how this may not
always work, because then backlight_level is stuck on the last non-zero value
before it was switched to intel_panel_set_backlight(0) and the different method.

Alex reported a slightly dimmer display after resume, so I wonder what I missed
in my patch. Larry's problem was probably fixed with just the first patch, but
then I wonder why he reported that it didn't work first. Maybe he meant the
setpci thing, or made a mistake.

> So hopefully, you just need the first patch.
> -Chris

Yeah, the second patch is a bit of a desperate attempt because Larry reported that
it didn't fix his problem.

About your patch, you still do:

+void intel_panel_setup_backlight(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
+	dev_priv->backlight_enabled = dev_priv->backlight_level != 0;
+}

While my patch changes that to:

+	u32 level;

-	if (dev_priv->backlight_level == 0)
-		dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
+	if (dev_priv->backlight_level == 0) {
+		level = intel_panel_get_backlight(dev);
+		if (level == 0)
+			level = intel_panel_get_max_backlight(dev);
+		dev_priv->backlight_level = level;
+	}

Your patch uses intel_panel_get_max_backlight() to check if the backlight is
enabled. Is this always correct, or may it cause bugs in the future?

Anyway, my main concern with unconditionally setting the backlight level to
the maximum is that any stored brightness level (by the BIOS or whatever) may
be lost, and that the screen is set to maximum brightness at each boot. This
is certainly the behaviour I've seen with an unpatched kernel. So I propose to
do what my patch does and set it to intel_panel_get_backlight(dev) if that
returns non-zero. Something like this:

+void intel_panel_setup_backlight(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 max, level;
+
+	max = intel_panel_get_max_backlight(dev);
+	if (max) {
+		dev_priv->backlight_enabled = 1;
+		level = intel_panel_get_backlight(dev);
+		if (!level)
+			level = max;
+		dev_priv->backlight_level = level;
+	}
+}

While I'm glad this problem is being fixed upstream, it would be nice to get
some credit for finding the source of the problem.

Greetings,

Indan



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-12  0:35       ` Indan Zupancic
@ 2011-01-12 12:07         ` Chris Wilson
  2011-01-12 12:14           ` Chris Wilson
  2011-01-13  0:35           ` Indan Zupancic
  0 siblings, 2 replies; 24+ messages in thread
From: Chris Wilson @ 2011-01-12 12:07 UTC (permalink / raw)
  To: Indan Zupancic
  Cc: Michal Hocko, Linus Torvalds, Linux Kernel Mailing List, dri-devel

On Wed, 12 Jan 2011 01:35:49 +0100 (CET), "Indan Zupancic" <indan@nul.nu> wrote:
> Yeah, the second patch is a bit of a desperate attempt because Larry reported that
> it didn't fix his problem.
> 
> About your patch, you still do:
> 
> +void intel_panel_setup_backlight(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
> +	dev_priv->backlight_enabled = dev_priv->backlight_level != 0;
> +}
> 
> While my patch changes that to:
> 
> +	u32 level;
> 
> -	if (dev_priv->backlight_level == 0)
> -		dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
> +	if (dev_priv->backlight_level == 0) {
> +		level = intel_panel_get_backlight(dev);
> +		if (level == 0)
> +			level = intel_panel_get_max_backlight(dev);
> +		dev_priv->backlight_level = level;
> +	}
> 
> Your patch uses intel_panel_get_max_backlight() to check if the backlight is
> enabled. Is this always correct, or may it cause bugs in the future?

That was a typo, cut'n'pasting the line from above.

> Anyway, my main concern with unconditionally setting the backlight level to
> the maximum is that any stored brightness level (by the BIOS or whatever) may
> be lost, and that the screen is set to maximum brightness at each boot. This
> is certainly the behaviour I've seen with an unpatched kernel. So I propose to
> do what my patch does and set it to intel_panel_get_backlight(dev) if that
> returns non-zero. Something like this:

Sure, s/intel_panel_get_max_backlight/intel_panel_get_backlight/ and we
get the behaviour we both want - preserving the current backlight unless
none is set.

> While I'm glad this problem is being fixed upstream, it would be nice to get
> some credit for finding the source of the problem.

Sorry. You found the bug but I felt your rationale was off. However, I was
amiss in not giving you the credit you fully deserved.
-Chris

commit 9c1c388a53e5df8819e898106a64e34d0994a5d4
Author: Indan Zupancic <indan@nul.nu>
Date:   Wed Jan 12 11:59:19 2011 +0000

    drm/i915/panel: The backlight is enabled if the current value is non-zero
    
    ... and not if the maximum is non-zero. This fixes the typo introduced
    in 47356eb6728501452 and preserves the backlight value from boot.
    
    [ickle: My thanks also to Indan Zupancic for diagnosing the original
            regression and suggesting the appropriate fix.]
    Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
    Cc: stable@kernel.org # after 47356eb6728501452

diff --git a/drivers/gpu/drm/i915/intel_panel.c
b/drivers/gpu/drm/i915/intel_pan
index e00d200..27c79c7 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -278,6 +278,6 @@ void intel_panel_setup_backlight(struct drm_device
*dev)
 {
        struct drm_i915_private *dev_priv = dev->dev_private;
 
-       dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
+       dev_priv->backlight_level = intel_panel_max_backlight(dev);
        dev_priv->backlight_enabled = dev_priv->backlight_level != 0;
 }

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-12 12:07         ` Chris Wilson
@ 2011-01-12 12:14           ` Chris Wilson
  2011-01-13  0:35           ` Indan Zupancic
  1 sibling, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2011-01-12 12:14 UTC (permalink / raw)
  To: Indan Zupancic
  Cc: Michal Hocko, Linus Torvalds, Linux Kernel Mailing List, dri-devel

On Wed, 12 Jan 2011 12:07:23 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> diff --git a/drivers/gpu/drm/i915/intel_panel.c
> b/drivers/gpu/drm/i915/intel_pan
> index e00d200..27c79c7 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -278,6 +278,6 @@ void intel_panel_setup_backlight(struct drm_device
> *dev)
>  {
>         struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -       dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
> +       dev_priv->backlight_level = intel_panel_max_backlight(dev);
>         dev_priv->backlight_enabled = dev_priv->backlight_level != 0;

          dev_priv->backlight_level = intel_panel_get_backlight(dev);
-ETIRED

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
  2011-01-12 12:07         ` Chris Wilson
  2011-01-12 12:14           ` Chris Wilson
@ 2011-01-13  0:35           ` Indan Zupancic
  1 sibling, 0 replies; 24+ messages in thread
From: Indan Zupancic @ 2011-01-13  0:35 UTC (permalink / raw)
  To: Chris Wilson
  Cc: Michal Hocko, Linus Torvalds, Linux Kernel Mailing List, dri-devel

On Wed, January 12, 2011 13:07, Chris Wilson wrote:
>
> Sure, s/intel_panel_get_max_backlight/intel_panel_get_backlight/ and we
> get the behaviour we both want - preserving the current backlight unless
> none is set.

Indeed, I hadn't noticed that shortcut. That's a lot nicer than my ifdefery.

>
>> While I'm glad this problem is being fixed upstream, it would be nice to get
>> some credit for finding the source of the problem.
>
> Sorry. You found the bug but I felt your rationale was off. However, I was
> amiss in not giving you the credit you fully deserved.

Thank you very much!

The rationale was that intel_panel_set_backlight(0) was somehow called twice,
and that the current code unconditionally stored the old backlight, and thus
losing the original brightness level. This is exactly what happened. My fix
was to prevent backlight_level from being overwritten by zero. Your fix was
more structural and properly fixed backlight enabled/disabled state. In the
end both have the same effect and solve the bug. Perhaps I was unclear in
the bug description.

Anyhow, it's a pleasure working with you. I'll try to not bother you too much,
you got enough on your plate as it is. I'll leave you alone for a while after
you looked into my fix for bug 23472, after that all my Intel graphics are
pretty much solved. :-)

Take care,

Indan



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Linux 2.6.37
@ 2011-01-07 19:20 Zephaniah E. Loss-Cutler-Hull
  0 siblings, 0 replies; 24+ messages in thread
From: Zephaniah E. Loss-Cutler-Hull @ 2011-01-07 19:20 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, Chris Wilson, dri-devel

On Thu, 06 Jan 2011 11:50:02 +0100, Michal Hocko wrote:
> Hi,
> 
> On Tue 04-01-11 17:15:45, Linus Torvalds wrote:
> [...]
> > We did have another revert to fix hopefullythe last "blank screen"
> > regression on intel graphics.
> 
> It seems that there is still a regression for intel graphic cards
> backlight. One report is
> https://bugzilla.kernel.org/show_bug.cgi?id=22672.
> I can reproduce the problem easily by:
> xset dpms force standby; sleep 3s; xset dpms force on
> 
> backlight doesn't get up (there is really dark picture though which
> doesn't get brighter by function keys which work normally) after dpms on
> until I close and open lid.
> 
> The problem wasn't present in 2.6.36

And I have a different regression.

On 2.6.36.2 everything is fine, on 2.6.37 my laptop's display is more or
less blank, backlight on, with a white pattern slowly moving across the
screen like it was a (bad) moving spot light effect.

dmesg output and .configs at
https://bugs.freedesktop.org/show_bug.cgi?id=32903

Zephaniah E. Loss-Cutler-Hull.

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2011-01-13  0:35 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-05  1:15 Linux 2.6.37 Linus Torvalds
2011-01-06 10:48 ` Michal Hocko
2011-01-06 16:29   ` Linus Torvalds
2011-01-06 17:03     ` Michal Hocko
2011-01-06 17:03       ` Michal Hocko
2011-01-06 17:49     ` Chris Wilson
2011-01-06 20:55       ` Alex Riesen
2011-01-06 21:03         ` Chris Wilson
2011-01-06 21:03           ` Chris Wilson
2011-01-06 21:08         ` Alex Riesen
2011-01-07 15:52           ` Michal Hocko
2011-01-11 13:33   ` Pavel Machek
2011-01-11 14:28     ` Michal Hocko
2011-01-11 14:47     ` Alex Riesen
2011-01-11 17:19   ` Michal Hocko
2011-01-11 17:37     ` Michal Hocko
2011-01-11 18:37       ` Michal Hocko
2011-01-11 17:39     ` Chris Wilson
2011-01-11 17:44       ` Michal Hocko
2011-01-12  0:35       ` Indan Zupancic
2011-01-12 12:07         ` Chris Wilson
2011-01-12 12:14           ` Chris Wilson
2011-01-13  0:35           ` Indan Zupancic
2011-01-07 19:20 Zephaniah E. Loss-Cutler-Hull

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.