Re: [Intel-gfx] Linux 3.11-rc2 [backlight] [ASUS N56VZ]

[Jani Nikula <jani.nikula@linux.intel.com>]

On Thu, 25 Jul 2013, "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> On Thursday, July 25, 2013 11:09:27 AM Jani Nikula wrote:
>> On Thu, 25 Jul 2013, "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
>> > Well, I wonder what about the appended (untested) patch?
>> 
>> Rafael, before going there, I've been trying to wrap my (poor, rusty
>> after vacation) head around
>> 
>> commit 8c5bd7adb2ce47e6aa39d17b2375f69b0c0aa255
>> Author: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>> Date:   Thu Jul 18 02:08:06 2013 +0200
>> 
>>     ACPI / video / i915: No ACPI backlight if firmware expects Windows 8
>> 
>> and I can't see how it could work.
>
> Well, if it didn't work, people wouldn't see either improvement or breakage
> from it, but they do see that, so it evidently works. :-)

I didn't claim it didn't work, just that *I* didn't see how it could. ;)

>> First, the ACPI_VIDEO_SKIP_BACKLIGHT flag seems to be checked before
>> it's actually set anywhere.
>
> Are you sure about that?
>
> acpi_video_bus_add() is the .add() callback routine for acpi_video_bus which
> in fact is an ACPI driver (the naming sucks, but I didn't invent it).  This
> means that acpi_video_bus_add() can only be called *after* acpi_video_bus
> has been registered with the ACPI subsystem (and the driver core).  That
> is done by acpi_bus_register_driver() and, guess what?, this happens in
> __acpi_video_register().  So clearly, acpi_video_bus_add() *cannot* run before
> __acpi_video_register().

Right. I totally missed the call within the ternary operator. Thanks for
the explanation, and apologies for the noise.

>> Second, with i915 that has opregion support, __acpi_video_register()
>> should only ever get called once. Which means the acpi_walk_namespace()
>> with video_unregister_backlight() should never get called in register.
>> 
>> Please enlighten me.
>
> Actually, that's correct, so we don't need the whole
> video_unregister_backlight() thing, calling acpi_video_backlight_quirks() would
> be sufficient.
>
> Ah, one more reason to do a full revert.  I'm thinking, though, that I'll leave
> acpi_video_backlight_quirks() as is so that it can be used by
> acpi_video_bus_(start)|(stop)_devices(), because that doesn't seem to cause
> problems to happen.

I observe that for the regular non-quirk acpi_video_register() call,
acpi_video_backlight_quirks() won't be called during register, but it
will get called later. This might have subtle effects later on, don't
you think?

As to the original problem, and your patch in this thread, what do you
think about having another value in acpi_backlight kernel parameter for
it? Having an i915 module parameter to tell acpi to use or not use
quirks seems odd, since the i915 is not really taking over
anything. It's just passing the info on to acpi.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Technology Center