From: Richard Purdie <rpurdie@rpsys.net> To: Henrique de Moraes Holschuh <hmh@hmh.eng.br> Cc: Alex Romosan <romosan@sycorax.lbl.gov>, Yaroslav Halchenko <kernel@onerussian.com>, Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org, linux-fbdev-devel@lists.sourceforge.net, James Simmons <jsimmons@infradead.org> Subject: Re: no backlight on radeon after recent kernel "upgrade"s Date: Thu, 22 Feb 2007 15:19:52 +0000 [thread overview] Message-ID: <1172157592.5837.89.camel@localhost.localdomain> (raw) In-Reply-To: <20070222145613.GD25887@khazad-dum.debian.net> On Thu, 2007-02-22 at 12:56 -0200, Henrique de Moraes Holschuh wrote: > On Thu, 22 Feb 2007, Richard Purdie wrote: > > > it to Len Brown for merging into 2.6.21, or NACK it if you'd rather do it in > > > the backlight class core. > > > > We can't change the backlight class code since some hardware can't read > > from the device, only write to it. Initialisation in that case is a bit > > different. > > Initializing stuff after registering is also racy as the device is not > locked but we are going to clobber data in its properties struct. I don't > particularly care about that race, but... If you really care, add a a call to backlight_update_status() after you set the brightness attribute like some of the other drivers have. The only data you're changing are single numbers and as long as update_status is called afterwards, a consistent state is pushed to the hardware so there is no race problem. > Anyway, you have the 2.6.21-rc patch now, to ACK or NACK. I still think the > class should be handling this. If a device is write-only, it should have no > _get ops handler, which means that the class can easily differentiate the > two cases and do the right thing for both. There's less code duplication > that way. Have a look at what corgi_bl does. It can know what state it set the hardware too as it keeps track itself, it just can't read that state from the hardware. Note how there is extra code in it to handle a power limit on the backlight under certain conditions and how this is fed back through the class through the get_brightness method. Adding one line of code (admittedly slight more due to error handling), is hardly that much code duplication. > Howerver, I *do* strongly wish for a way to combine various drivers into a > single backlight device, where radeon/intelfb takes care of some stuff, > ibm-acpi/asus-laptop/sony-laptop takes care of other stuff, etc. Also, a > standard naming for the builtin screen(s) would help, calling it "ibm", > "asus", "sony" is not good IMHO. I wasn't aware of this problem. If some devices need bits from both raedon/whatever and acpi, the current implementations are just plain wrong. Its not really a backlight class problem and more of an implementation and interaction problem between acpi and the framebuffer drivers. They should be presenting and registering *one* backlight class device, not two. Without knowing more about the circumstances and how/when to combine which drivers its hard for me to help further... Richard
WARNING: multiple messages have this Message-ID (diff)
From: Richard Purdie <rpurdie@rpsys.net> To: Henrique de Moraes Holschuh <hmh@hmh.eng.br> Cc: James Simmons <jsimmons@infradead.org>, Alex Romosan <romosan@sycorax.lbl.gov>, linux-kernel@vger.kernel.org, linux-fbdev-devel@lists.sourceforge.net, Yaroslav Halchenko <kernel@onerussian.com>, Andrew Morton <akpm@linux-foundation.org> Subject: Re: no backlight on radeon after recent kernel "upgrade"s Date: Thu, 22 Feb 2007 15:19:52 +0000 [thread overview] Message-ID: <1172157592.5837.89.camel@localhost.localdomain> (raw) In-Reply-To: <20070222145613.GD25887@khazad-dum.debian.net> On Thu, 2007-02-22 at 12:56 -0200, Henrique de Moraes Holschuh wrote: > On Thu, 22 Feb 2007, Richard Purdie wrote: > > > it to Len Brown for merging into 2.6.21, or NACK it if you'd rather do it in > > > the backlight class core. > > > > We can't change the backlight class code since some hardware can't read > > from the device, only write to it. Initialisation in that case is a bit > > different. > > Initializing stuff after registering is also racy as the device is not > locked but we are going to clobber data in its properties struct. I don't > particularly care about that race, but... If you really care, add a a call to backlight_update_status() after you set the brightness attribute like some of the other drivers have. The only data you're changing are single numbers and as long as update_status is called afterwards, a consistent state is pushed to the hardware so there is no race problem. > Anyway, you have the 2.6.21-rc patch now, to ACK or NACK. I still think the > class should be handling this. If a device is write-only, it should have no > _get ops handler, which means that the class can easily differentiate the > two cases and do the right thing for both. There's less code duplication > that way. Have a look at what corgi_bl does. It can know what state it set the hardware too as it keeps track itself, it just can't read that state from the hardware. Note how there is extra code in it to handle a power limit on the backlight under certain conditions and how this is fed back through the class through the get_brightness method. Adding one line of code (admittedly slight more due to error handling), is hardly that much code duplication. > Howerver, I *do* strongly wish for a way to combine various drivers into a > single backlight device, where radeon/intelfb takes care of some stuff, > ibm-acpi/asus-laptop/sony-laptop takes care of other stuff, etc. Also, a > standard naming for the builtin screen(s) would help, calling it "ibm", > "asus", "sony" is not good IMHO. I wasn't aware of this problem. If some devices need bits from both raedon/whatever and acpi, the current implementations are just plain wrong. Its not really a backlight class problem and more of an implementation and interaction problem between acpi and the framebuffer drivers. They should be presenting and registering *one* backlight class device, not two. Without knowing more about the circumstances and how/when to combine which drivers its hard for me to help further... Richard ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
next prev parent reply other threads:[~2007-02-22 15:21 UTC|newest] Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top 2007-02-19 4:46 no backlight on radeon after recent kernel "upgrade"s Yaroslav Halchenko 2007-02-19 8:04 ` Andrew Morton 2007-02-19 8:04 ` Andrew Morton 2007-02-19 9:19 ` Richard Purdie 2007-02-19 9:19 ` Richard Purdie 2007-02-21 5:56 ` Yaroslav Halchenko 2007-02-22 0:34 ` Richard Purdie 2007-02-22 0:34 ` Richard Purdie 2007-02-22 1:07 ` James Simmons 2007-02-22 1:07 ` James Simmons 2007-02-22 9:46 ` Richard Purdie 2007-02-22 9:46 ` Richard Purdie 2007-02-22 15:18 ` James Simmons 2007-02-22 1:11 ` [Linux-fbdev-devel] " James Simmons 2007-02-22 2:09 ` Joel Becker 2007-02-22 15:55 ` James Simmons 2007-02-22 15:55 ` James Simmons 2007-02-22 17:28 ` [Linux-fbdev-devel] " David Miller 2007-02-22 17:28 ` David Miller 2007-02-28 16:55 ` [Linux-fbdev-devel] " James Simmons 2007-03-01 10:57 ` Richard Purdie 2007-03-01 10:57 ` Richard Purdie 2007-03-01 21:08 ` [Linux-fbdev-devel] " James Simmons 2007-03-01 21:08 ` James Simmons 2007-02-21 22:18 ` Alex Romosan 2007-02-21 22:41 ` Richard Purdie 2007-02-21 22:41 ` Richard Purdie 2007-02-21 23:17 ` Henrique de Moraes Holschuh 2007-02-22 0:12 ` Richard Purdie 2007-02-22 0:12 ` Richard Purdie 2007-02-22 0:51 ` Henrique de Moraes Holschuh 2007-02-22 1:10 ` Richard Purdie 2007-02-22 1:10 ` Richard Purdie 2007-02-22 2:13 ` Henrique de Moraes Holschuh 2007-02-22 1:10 ` Henrique de Moraes Holschuh 2007-02-22 1:16 ` ACPI: ibm-acpi: fix initial status of backlight device Henrique de Moraes Holschuh 2007-02-22 10:03 ` Richard Purdie 2007-02-22 14:45 ` Henrique de Moraes Holschuh 2007-02-22 18:19 ` Henrique de Moraes Holschuh 2007-02-22 10:00 ` no backlight on radeon after recent kernel "upgrade"s Richard Purdie 2007-02-22 10:00 ` Richard Purdie 2007-02-22 14:56 ` Henrique de Moraes Holschuh 2007-02-22 15:19 ` Richard Purdie [this message] 2007-02-22 15:19 ` Richard Purdie 2007-02-22 16:00 ` James Simmons 2007-02-22 16:00 ` James Simmons 2007-02-22 16:34 ` Henrique de Moraes Holschuh 2007-02-22 17:08 ` Richard Purdie 2007-02-22 17:08 ` Richard Purdie 2007-02-21 23:51 ` Alex Romosan 2007-02-22 1:13 ` James Simmons 2007-02-22 1:13 ` James Simmons 2007-02-22 9:56 ` Richard Purdie 2007-02-22 9:56 ` Richard Purdie 2007-02-22 14:38 ` Henrique de Moraes Holschuh 2007-02-22 4:03 ` Alex Romosan 2007-02-22 4:58 ` Alex Romosan
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1172157592.5837.89.camel@localhost.localdomain \ --to=rpurdie@rpsys.net \ --cc=akpm@linux-foundation.org \ --cc=hmh@hmh.eng.br \ --cc=jsimmons@infradead.org \ --cc=kernel@onerussian.com \ --cc=linux-fbdev-devel@lists.sourceforge.net \ --cc=linux-kernel@vger.kernel.org \ --cc=romosan@sycorax.lbl.gov \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.