[Intel-gfx] BACKLIGHT property for KMS case

[Greg KH]

On Mon, Aug 17, 2009 at 06:52:37PM +0200, Matthias Hopf wrote:
> On Aug 17, 09 09:17:02 -0700, Greg KH wrote:
> > On Mon, Aug 17, 2009 at 04:07:45PM +0200, Matthias Hopf wrote:
> > > +#define BACKLIGHT_CLASS "/sys/class/backlight"
> > 
> > Why can't we use libudev to get this kind of device instead of
> > hard-coding it?
> 
> This is just copy and paste from the original code. Typically libudev
> is frowned upon a bit in X due to drivers being compilable on *BSD and
> other OSes, and I'm unsure about the state there (is udev used there? is
> the kernel interface different? do they support KMS already at all?).

We still care about other OSes?  :)

I really doubt that any other operating system has /sys/class/backlight/
so I don't see how this is any different.

And as HAL is depreciated for libudev, I thought that was the way
forward for xorg.

> From a (very) quick glance at the reference manual I don't see how I can
> get the device name from libudev - I guess I need some help from you
> there.

You iterate over the class device names for a specific class, I can
knock up some example code if you want it.

> > > +/*
> > > + * List of available kernel interfaces in priority order
> > > + */
> > > +static char *backlight_interfaces[] = {
> > > +    "asus-laptop",
> > > +    "eeepc",
> > > +    "thinkpad_screen",
> > > +    "acpi_video1",
> > > +    "acpi_video0",
> > > +    "fujitsu-laptop",
> > > +    "sony",
> > > +    NULL,
> > > +};
> > 
> > You forgot to add "samsung" :)
> 
> This is the old list. Expansion should be done as a separate commit.
> 
> Also, as written multiple times I think your driver shouldn't be called
> samsung but i915 or i830 or the like (according to the X11 driver the
> method should work for i830 and up, but native access method should be
> implemented as well).

So you want 2 different drivers controlling the same hardware?  That's a
recipe for disaster :)

I don't care what to call this driver, but I think calling it i915 might
cause big issues as you can't have 2 of the same driver name in Linux...

"intel-backlight" perhaps?

> > Yeah, I know you want to do this instead of using the samsung-backlight
> 
> No, I don't. I'd love to have a kernel driver, this patch is *only*
> about the RandR property. In fact, this patch will *not* enable
> backlight control on the Samsung NC130 without your driver (and as the
> driver name isn't included yet, also not yet with your driver ;-)
> 
> I tested on a different machine that has a working ACPI backlight
> interface.
> 
> > driver, but what about for instances that people are not running this
> > intel driver and they want control over the backlight of their hardware?
> > That would mean that the samsung-backlight driver is needed for them.
> 
> The control method you implemented is AFAICS intel chipset specific. It
> is definitely not Samsung specific, and I very much assume that there
> are other laptops in the wild with broken ACPI interface that would
> benefit from the driver.

I have not heard of any yet, but would not rule it out.

thanks,

greg k-h