Re: [PATCH 2/2] ACPI Check for backlight support via ACPI video.ko otherwise use vendor ACPI drivers

[Matthew Garrett <mjg59@srcf.ucam.org>]

On Wed, Jul 09, 2008 at 04:49:49PM +0200, Thomas Renninger wrote:
> On Thursday 03 July 2008 18:21:52 Matthew Garrett wrote:
> > On Thu, Jul 03, 2008 at 06:08:59PM +0200, Thomas Renninger wrote:
> > > +	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "DRDY", &h_dummy))) {
> > > +		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Found IGD device\n"));
> > > +		video_caps |= ACPI_VIDEO_IGD;
> > > +	}
> >
> > In principle, it's possible for a vendor to implement opregion but not
> > actually use it until the tche flags are set. If anyone's done that then
> > you've just broken backlight control on their system.
> 
> Who should set that flags?
> The dri driver is going to set that if he can serve IGD parts, right?

No. The opregion spec requires you to use video.ko. 

> Currently this patch makes the vendor specific e.g. thinkpad_acpi to serve 
> backlight brightness switching if a IGD device is found. Because currently 
> there is no IGD support in the kernel.

Yes, but that's *wrong*. There are many machines that support opregion 
but do not require its use - they'll behave using the old ACPI mechanism 
until you switch them to the new mode. Except right now if you switch 
them to the new mode then they'll stop working, because the DRI code 
isn't merged yet.

> You should export a function intel_opregion_init()? so that built in kernel 
> parts can check for IGD backlight support, instead of the lines above (part 
> of my patch). If this succeeds, my patch will tell vendor specfic drivers to 
> keep fingers off backlight switching.

No. Please read the opregion spec. The way it works is as follows:

1) Firmware exports an ACPI video class
2) OS checks whether the hardware supports the opregion ACPI video 
handling (this is done by looking at tables in a region of memory 
pointed at by a PCI configuration register)
3) OS sets opregion flags
4) OS calls the ACPI backlight set function
5) Firmware writes desired brightness to an opregion mailbox and 
triggers an interrupt
6) OS sets the brightness

For this to work, the ACPI video driver needs to work as it currently 
does. It is not correct for it to disable itself on IGD systems.
-- 
Matthew Garrett | mjg59@srcf.ucam.org