Andrew Zabolotny wrote:
> On Fri, 28 May 2004 14:59:53 -0700
> Todd Poynor <[email protected]> wrote:
>
>
>>Hi, you're adding new interfaces for power management of LCD and
>>backlight devices. Since there's already LDM/sysfs interfaces for
>>reading and writing power state of generic devices, is it necessary to
>>add ones particular to these devices or device classes? In other words,
>>is /sys/devices/<bus>/<device>/power/state not suitable for these purposes?
>
> Well, first liquid crystal displays and backlight are not connected to any
> specific bus. They could, on some platforms, and could not on other. For
> example, on some PDAs backlight is controlled via the programmed I/O CPU pins
> (GPIO), on other they are connected to some weird companion chips, on third
> they could be controlled via the PCI bus (on large PCs, I mean) and so on. So
> there will be no easy way for an application to find the backlight devices
> and control them. In our case that's easy: opendir ("/sys/class/backlight")
> and you found all of them.
I'm not questioning the usefulness of the other aspects of the patch,
such as adding an LCD/backlight class for framebuffer access and adding
attributes for the unique features of LCD/backlight devices. My
questions are specific to the power management interfaces, since there's
already interfaces for this, with different semantics than the new class
interfaces, and there's some value in sticking with a single consistent
interface for it.
If I understand correctly, the LCD device is registered on a bus (either
a platform-specific bus or the generic "platform" bus), and the device
therefore already has a power/state attribute; the class entry could
refer back to that device if needed. So I'm interested in discussing
whether the existing PM interface suffices for LCD/backlight devices, or
if not, should the existing interfaces be improved (rather than working
around deficiencies via device-specific interfaces)?
> [...]
> On the third hand (:-) the lcd device class, for example, actually has *two*
> power switches: the 'power' and the 'enable' attribute. The first powers off
> the liquid crystal display, and seconds powers off the lcd controller. These
> are different things and it would be wise to leave them apart, as having finer
> grained control is always better. [...]
But it also sounds like the single LDM registration for an LCD device
could be better handled by registering the LCD display, LCD controller,
and backlight as separate devices (which they probably are), allowing
individual control through the standard interfaces.
>>And if a PM interface for device classes is needed that ties into the
>>device driver suspend/resume callbacks, perhaps it can be modeled more
>>closely on the existing interfaces? These new interfaces seem to be
>>intended to define: 0 == power off, 1 == power on. The existing
>>ACPI-inspired interfaces use: 0 == power on/full-power, 1/2/3/4 ==
>>low-power/off state.
>
> Um, well, this could be implemented. Although I don't see much reason for a
> backlight to be in a "semi-enabled state"; besides if it will remain a
> separate class device, it doesn't matter much.
The main problem is that existing interfaces write zero for on and
non-zero for off, while the new interface reverses things. I realize
that multiple power states are not implemented by all devices; if
there's interest in tailoring device states to more closely match the
hardware capabilities of non-ACPI systems then I'd be happy to talk more
about that.
So none of my objections are terribly crucial, and if Greg et al don't
have a problem with device-class-specific PM interfaces that have
different semantics and/or capabilities than those of the device
power/state attributes then I don't have much of a problem with it
either. Just seems worthwhile to check whether there's improvements
needed in the existing PM interfaces instead.
--
Todd
On Tue, 01 Jun 2004 13:09:46 -0700
Todd Poynor <[email protected]> wrote:
> I'm not questioning the usefulness of the other aspects of the patch,
> such as adding an LCD/backlight class for framebuffer access and adding
> attributes for the unique features of LCD/backlight devices. My
> questions are specific to the power management interfaces, since there's
> already interfaces for this, with different semantics than the new class
> interfaces, and there's some value in sticking with a single consistent
> interface for it.
Well, after thinking awhile, I have changed it so that 0 means power on,
1..3 intermediate values (although for now they are interpreted as poweroff by
existing drivers since there are no intermediate states) and 4 is 'off for
real'. Indeed, there is no much reason to have them use different semantics.
> If I understand correctly, the LCD device is registered on a bus (either
> a platform-specific bus or the generic "platform" bus)
No, they are registered just as a class device. There is no corresponding
device on any other bus, this would mean a lot of unneeded overhead.
> therefore already has a power/state attribute; the class entry could
> refer back to that device if needed. So I'm interested in discussing
> whether the existing PM interface suffices for LCD/backlight devices, or
> if not, should the existing interfaces be improved (rather than working
> around deficiencies via device-specific interfaces)?
Um, well, the LCD device actually has two power controls, like I said
before: one toggles the power to the LCD itself, another one (the 'enable'
attribute) controls power to the LCD controller. Not that this explicit
separate control is required very much, but it would be nice to have a degree
of freedom close to that allowed by hardware.
In theory, if we would use the standard power interface, it could use the
different levels of power saving, e.g. 0 - controller and LCD on, 1,2 - LCD
off, controller on, 3,4 - both off.
> But it also sounds like the single LDM registration for an LCD device
> could be better handled by registering the LCD display, LCD controller,
> and backlight as separate devices (which they probably are), allowing
> individual control through the standard interfaces.
Well, the LCD panel is controllable only through the LCD controller, so for
the most part they are the same. The only thing is that the LCD controller has
a one-bit switch to disable the power to the panel. I don't think it makes
sense to separate that bit into a separate device.
> So none of my objections are terribly crucial, and if Greg et al don't
> have a problem with device-class-specific PM interfaces that have
> different semantics and/or capabilities than those of the device
> power/state attributes then I don't have much of a problem with it
> either. Just seems worthwhile to check whether there's improvements
> needed in the existing PM interfaces instead.
Well, the power interface under drivers/ is available for framebuffer.
If it would handle it properly (the framebuffer drivers I've tried
don't, alas), they would toggle the attached (to the framebuffer) LCD
and backlight power state according to its own state (which is not so fine
grained, but is logical). In any case, one of reasons this backlight/lcd patch
has been written was to avoid that mess with callbacks that lately begun to
appear in ARM-specific framebuffer devices (and I shudder at the thought that
MIPS people should be doing something similar).
--
Greetings,
Andrew
On Wed, Jun 02, 2004 at 01:00:36AM +0400, Andrew Zabolotny wrote:
>
> In theory, if we would use the standard power interface, it could use the
> different levels of power saving, e.g. 0 - controller and LCD on, 1,2 - LCD
> off, controller on, 3,4 - both off.
Please use the standard power interface, and use the standard levels of
power state. That's why we _have_ this driver model in the first
place...
> > So none of my objections are terribly crucial, and if Greg et al don't
> > have a problem with device-class-specific PM interfaces that have
> > different semantics and/or capabilities than those of the device
> > power/state attributes then I don't have much of a problem with it
> > either. Just seems worthwhile to check whether there's improvements
> > needed in the existing PM interfaces instead.
I do have a problem with device-class-specific PM interfaces that have
different semantics from the whole rest of the system.
> Well, the power interface under drivers/ is available for framebuffer.
> If it would handle it properly (the framebuffer drivers I've tried
> don't, alas)
Then they need to be fixed to do so.
thanks,
greg k-h
On Wed, 2 Jun 2004 10:15:42 -0700
Greg KH <[email protected]> wrote:
> > In theory, if we would use the standard power interface, it could use the
> > different levels of power saving, e.g. 0 - controller and LCD on, 1,2 -
> > LCD off, controller on, 3,4 - both off.
> Please use the standard power interface, and use the standard levels of
> power state. That's why we _have_ this driver model in the first
> place...
The problem is that I cannot use the standard power interface because I don't
know how. The patch in question implements class devices, not regular devices.
Class devices don't use the standard power management scheme. Here:
/sys # ls devices/platform/
detach_state power pxa2xx_udc0
mq11xx0 pxa2xx-pcmcia0 pxafb0
mq11xx_udc0 pxa2xx_serial0 pxamci0
None of the above devices implement the backlight and LCD; they use bits from
different devices; in my case several GPIO pins from the mq11xx0 device, a
couple of CPU' GPIO pins (which aren't present in sysfs at all).
Now the mq11xx0 device (which is a System-On-Chip, e.g. a single chip
encomprising a lot of somehow-independent-functions) has a lot of subdevices:
/sys # ls devices/platform/mq11xx0/
detach_state mq11xx_i2s0 mq11xx_spi0 mq11xx_uhc0
mq11xx_fb0 mq11xx_lcd0 mq11xx_udc0 power
The backlight is controlled by a few spare bits from the mq11xx_spi0 device
(which is a SPI bus controller). I don't think it makes sense to create the
backlight device node under /sys/devices/platform/mq11xx0/mq11xx_spi0/;
besides that's on my PDA; on other PDAs they could be located anywhere else,
thus the software will have a major headache to find the backlight/lcd power
controls.
Right now it works this way:
/sys # ls class/backlight/mq11xx_fb0/
brightness max_brightness power
/sys # ls class/lcd/mq11xx_fb0/
contrast enable max_contrast power
I could unify lcd's 'power' and 'enable' attributes into one (they already use
the 'standard' 0-4 power levels), like I stated in one of my previous
messages, but it anyways would be quite different from the current power
management structure.
> > Well, the power interface under drivers/ is available for framebuffer.
> > If it would handle it properly (the framebuffer drivers I've tried
> > don't, alas)
> Then they need to be fixed to do so.
Doh, you don't want me to fix the whole kernel in one patch, don't you? :-)
--
Greetings,
Andrew
On Wed, Jun 02, 2004 at 10:15:42AM -0700, Greg KH wrote:
> On Wed, Jun 02, 2004 at 01:00:36AM +0400, Andrew Zabolotny wrote:
> >
> > In theory, if we would use the standard power interface, it could use the
> > different levels of power saving, e.g. 0 - controller and LCD on, 1,2 - LCD
> > off, controller on, 3,4 - both off.
>
> Please use the standard power interface, and use the standard levels of
> power state. That's why we _have_ this driver model in the first
> place...
It /doesn't/ make any sense to in this case. We're talking effectively
about the LCD panel attributes, not a device as such.
IOW:
- LCD backlight brightness
- LCD backlight on/off
- LCD panel power on/off
Typically, the last item needs to be closely controlled in relation to
the LCD controller itself, and it just does not make sense to separate
the LCD panel power control from the controller itself in the device
model. In fact, exposing the LCD panel power control independent of
the LCD controller state can lead to cases where you end up destroying
your LCD panel - there is a very defined sequence to power up/down
these things to avoid degredation.
However, the issue is that all of the three things can be handled
god-knows-which-way on any platform, even when they're using the
same LCD controllers. So we need a way for platform/machine specific
code to provide the information so that the LCD controller can
correctly handle these settings on blank, etc.
Hope this provides some extra reasoning why using the device model
for these attributes is wrong.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
On Wed, Jun 02, 2004 at 10:32:19PM +0100, Russell King wrote:
> On Wed, Jun 02, 2004 at 10:15:42AM -0700, Greg KH wrote:
> > On Wed, Jun 02, 2004 at 01:00:36AM +0400, Andrew Zabolotny wrote:
> > >
> > > In theory, if we would use the standard power interface, it could use the
> > > different levels of power saving, e.g. 0 - controller and LCD on, 1,2 - LCD
> > > off, controller on, 3,4 - both off.
> >
> > Please use the standard power interface, and use the standard levels of
> > power state. That's why we _have_ this driver model in the first
> > place...
>
> It /doesn't/ make any sense to in this case. We're talking effectively
> about the LCD panel attributes, not a device as such.
<snip>
> Hope this provides some extra reasoning why using the device model
> for these attributes is wrong.
Ok, this makes more sense now, thanks for explaining it.
greg k-h