All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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: link
Be 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.