[PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[Lennart Poettering]

On the Yoga 13 the backlight control doesn't work via ACPI. (And doesn't
work either with the low-level platform driver ideapad_laptop; but
works correctly via the intel video driver).  This patch hence adds the
Yoga 13 to the ACPI video detect blacklist, to make sure the broken ACPI
backlight device is never exposed to userspace.

Note that this appears unrelated to the Windows 8 backlight issues tracked
here:

https://bugzilla.kernel.org/show_bug.cgi?id=51231
https://bugzilla.kernel.org/show_bug.cgi?id=60682

The Yoga's ACPI backlight controls work neither with nor without
acpi_osi="!Windows 2012" on the kernel command line. It appears that
backlight control via the EC simply is not available at all, regardless
whether done via ACPI or via the vendor driver.

Signed-off-by: Lennart Poettering <lennart@poettering.net>
---
 drivers/acpi/video_detect.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
index 940edbf..a88e8f7 100644
--- a/drivers/acpi/video_detect.c
+++ b/drivers/acpi/video_detect.c
@@ -168,6 +168,14 @@ static struct dmi_system_id video_detect_dmi_table[] = {
 		DMI_MATCH(DMI_PRODUCT_NAME, "UL30A"),
 		},
 	},
+	{
+	.callback = video_detect_force_vendor,
+	.ident = "Lenovo Yoga 13",
+	.matches = {
+		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+		DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),
+		},
+	},
 	{ },
 };
 
-- 
1.8.3.1

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

Re: [PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[Aaron Lu]

On 10/14/2013 08:55 AM, Lennart Poettering wrote:
> On the Yoga 13 the backlight control doesn't work via ACPI. (And doesn't
> work either with the low-level platform driver ideapad_laptop; but
> works correctly via the intel video driver).  This patch hence adds the
> Yoga 13 to the ACPI video detect blacklist, to make sure the broken ACPI
> backlight device is never exposed to userspace.
> 
> Note that this appears unrelated to the Windows 8 backlight issues tracked
> here:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=51231
> https://bugzilla.kernel.org/show_bug.cgi?id=60682
> 
> The Yoga's ACPI backlight controls work neither with nor without
> acpi_osi="!Windows 2012" on the kernel command line. It appears that
> backlight control via the EC simply is not available at all, regardless
> whether done via ACPI or via the vendor driver.

Just a side note, if the firmware of Yoga 13 has a _OSI("Windows 2012")
query, then it should be solved with the patch proposed here:
https://lkml.org/lkml/2013/10/11/409, Fix Win8 backlight issue.

We are still discussing a proper default behaviour in that patchset.

Thanks,
Aaron

> 
> Signed-off-by: Lennart Poettering <lennart@poettering.net>
> ---
>  drivers/acpi/video_detect.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> index 940edbf..a88e8f7 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -168,6 +168,14 @@ static struct dmi_system_id video_detect_dmi_table[] = {
>  		DMI_MATCH(DMI_PRODUCT_NAME, "UL30A"),
>  		},
>  	},
> +	{
> +	.callback = video_detect_force_vendor,
> +	.ident = "Lenovo Yoga 13",
> +	.matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +		DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),
> +		},
> +	},
>  	{ },
>  };
>  
> 

Re: [PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[Lennart Poettering]

On Mon, 14.10.13 09:11, Aaron Lu (aaron.lu@intel.com) wrote:

> > Note that this appears unrelated to the Windows 8 backlight issues tracked
> > here:
> > 
> > https://bugzilla.kernel.org/show_bug.cgi?id=51231
> > https://bugzilla.kernel.org/show_bug.cgi?id=60682
> > 
> > The Yoga's ACPI backlight controls work neither with nor without
> > acpi_osi="!Windows 2012" on the kernel command line. It appears that
> > backlight control via the EC simply is not available at all, regardless
> > whether done via ACPI or via the vendor driver.
> 
> Just a side note, if the firmware of Yoga 13 has a _OSI("Windows 2012")
> query, then it should be solved with the patch proposed here:
> https://lkml.org/lkml/2013/10/11/409, Fix Win8 backlight issue.
> 
> We are still discussing a proper default behaviour in that patchset.

No. 

Did you actually read the commit message of the patch? Please do!

The backlight for the Yoga 13 doesn't work, regardless what the _OSI
value is. In fact, it doesn't even work by directly accessing the EC via
the ideapad-laptop driver. 

So, again:

- Backlight control doesn't work via ACPI without acpi_osi="!Windows 2012"
- Backlight control doesn't work via ACPI with acpi_osi="!Windows 2012"
- Backlight control doesn't work via EC commands from ideapad-laptop.c

The only way backlight handling is supported on the Yoga 13 is via the
intel video driver.

Or in other words: the situation for the Yoga 13 is *unrelated* to the
Windows 8 issues, and your patch.

Hence this patch I posted, which blacklists the backlight control
entirely in the ACPI driver, since the Windows 2012 setting is
irrelevant to it.

I'll soon send another patch which also blacklists the thing in the
ideapad driver, so that only the intel backlight driver is enabled on
Yoga 13 systems, at which point everything will work fine.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

Re: [PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[Aaron Lu]

On 10/14/2013 10:23 AM, Lennart Poettering wrote:
> On Mon, 14.10.13 09:11, Aaron Lu (aaron.lu@intel.com) wrote:
> 
>>> Note that this appears unrelated to the Windows 8 backlight issues tracked
>>> here:
>>>
>>> https://bugzilla.kernel.org/show_bug.cgi?id=51231
>>> https://bugzilla.kernel.org/show_bug.cgi?id=60682
>>>
>>> The Yoga's ACPI backlight controls work neither with nor without
>>> acpi_osi="!Windows 2012" on the kernel command line. It appears that
>>> backlight control via the EC simply is not available at all, regardless
>>> whether done via ACPI or via the vendor driver.
>>
>> Just a side note, if the firmware of Yoga 13 has a _OSI("Windows 2012")
>> query, then it should be solved with the patch proposed here:
>> https://lkml.org/lkml/2013/10/11/409, Fix Win8 backlight issue.
>>
>> We are still discussing a proper default behaviour in that patchset.
> 
> No. 
> 
> Did you actually read the commit message of the patch? Please do!

Yes I've read the commit message...

> 
> The backlight for the Yoga 13 doesn't work, regardless what the _OSI
> value is. In fact, it doesn't even work by directly accessing the EC via
> the ideapad-laptop driver. 
> 
> So, again:
> 
> - Backlight control doesn't work via ACPI without acpi_osi="!Windows 2012"
> - Backlight control doesn't work via ACPI with acpi_osi="!Windows 2012"
> - Backlight control doesn't work via EC commands from ideapad-laptop.c
> 
> The only way backlight handling is supported on the Yoga 13 is via the
> intel video driver.

Since I don't have access to the acpidump of this system, my only
question is, does the firmware has a _OSI("Windows 2012") query in DSDT
table?

> 
> Or in other words: the situation for the Yoga 13 is *unrelated* to the
> Windows 8 issues, and your patch.

I think they are related...
If the firmware is compatible to Windows 8, then my patch will disable
ACPI video backlight interface to prefer GPU's interface.

> 
> Hence this patch I posted, which blacklists the backlight control
> entirely in the ACPI driver, since the Windows 2012 setting is
> irrelevant to it.

Yes, the Windows 2012 setting is irrelevant to ACPI's interface(with or
without it, it doesn't work), but it's relevant to my patch as that is
the condition we are using to decide if we should skip registering ACPI
video's backlight interface.

> 
> I'll soon send another patch which also blacklists the thing in the
> ideapad driver, so that only the intel backlight driver is enabled on
> Yoga 13 systems, at which point everything will work fine.

Right, that is needed. And if going with my patch, the ideapad driver
will need to be patched similarly like thinkpad_acpi to add a check of
acpi_video_backlight_support before it decides to register its own
backlight interface.

-Aaron

Re: [PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[Lennart Poettering]

On Mon, 14.10.13 10:36, Aaron Lu (aaron.lu@intel.com) wrote:

> > - Backlight control doesn't work via ACPI without acpi_osi="!Windows 2012"
> > - Backlight control doesn't work via ACPI with acpi_osi="!Windows 2012"
> > - Backlight control doesn't work via EC commands from ideapad-laptop.c
> > 
> > The only way backlight handling is supported on the Yoga 13 is via the
> > intel video driver.
> 
> Since I don't have access to the acpidump of this system, my only
> question is, does the firmware has a _OSI("Windows 2012") query in DSDT
> table?

Yes, it appears to do that as part of _OSC.

The disassembled DSDT table is here:

http://0pointer.de/public/yoga13-dsdt.dsl

> > Or in other words: the situation for the Yoga 13 is *unrelated* to the
> > Windows 8 issues, and your patch.
> 
> I think they are related...
> If the firmware is compatible to Windows 8, then my patch will disable
> ACPI video backlight interface to prefer GPU's interface.

OK, that might indeed work.

> > I'll soon send another patch which also blacklists the thing in the
> > ideapad driver, so that only the intel backlight driver is enabled on
> > Yoga 13 systems, at which point everything will work fine.
> 
> Right, that is needed. And if going with my patch, the ideapad driver
> will need to be patched similarly like thinkpad_acpi to add a check of
> acpi_video_backlight_support before it decides to register its own
> backlight interface.

The ideapad driver currently skips registration of the backlight device
if acpi_video_backlight_support() returns true. is that all you need?

Hmm, regarding your patch series, do you plan to skip the registration
of the acpi backlight device if the "raw" device is supported? I mean,
the intel driver could be compiled as a module (and generally is on the
popular distros), so at the time the ACPI subsystem wants to register
the backlight device and know if a raw backlight device is around it
never will be, so what is the point of that? Or am I missing something?

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

Re: [PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[Aaron Lu]

On 10/14/2013 11:49 AM, Lennart Poettering wrote:
> On Mon, 14.10.13 10:36, Aaron Lu (aaron.lu@intel.com) wrote:
> 
>>> - Backlight control doesn't work via ACPI without acpi_osi="!Windows 2012"
>>> - Backlight control doesn't work via ACPI with acpi_osi="!Windows 2012"
>>> - Backlight control doesn't work via EC commands from ideapad-laptop.c
>>>
>>> The only way backlight handling is supported on the Yoga 13 is via the
>>> intel video driver.
>>
>> Since I don't have access to the acpidump of this system, my only
>> question is, does the firmware has a _OSI("Windows 2012") query in DSDT
>> table?
> 
> Yes, it appears to do that as part of _OSC.
> 
> The disassembled DSDT table is here:
> 
> http://0pointer.de/public/yoga13-dsdt.dsl

Thanks for the info. The firmware has a _OSI("Windows 2012") query so
should be covered by my patch.

> 
>>> Or in other words: the situation for the Yoga 13 is *unrelated* to the
>>> Windows 8 issues, and your patch.
>>
>> I think they are related...
>> If the firmware is compatible to Windows 8, then my patch will disable
>> ACPI video backlight interface to prefer GPU's interface.
> 
> OK, that might indeed work.
> 
>>> I'll soon send another patch which also blacklists the thing in the
>>> ideapad driver, so that only the intel backlight driver is enabled on
>>> Yoga 13 systems, at which point everything will work fine.
>>
>> Right, that is needed. And if going with my patch, the ideapad driver
>> will need to be patched similarly like thinkpad_acpi to add a check of
>> acpi_video_backlight_support before it decides to register its own
>> backlight interface.
> 
> The ideapad driver currently skips registration of the backlight device
> if acpi_video_backlight_support() returns true. is that all you need?

Yes, that's all.

> 
> Hmm, regarding your patch series, do you plan to skip the registration
> of the acpi backlight device if the "raw" device is supported? I mean,

Yes, that's right.

> the intel driver could be compiled as a module (and generally is on the
> popular distros), so at the time the ACPI subsystem wants to register
> the backlight device and know if a raw backlight device is around it
> never will be, so what is the point of that? Or am I missing something?

For systems with Intel i915 GPU, ACPI video will wait for GPU driver to
run first, see drivers/acpi/video.c acpi_video_init, the actual
acpi_video_register function is called by i915 driver in
i915_driver_load due to operation region related stuff. Since all
problematic systems reported so far has an Intel GPU, I'm doing it this
way now. If things change, we can enhance it then.

-Aaron

Re: [PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[Lennart Poettering]

On Mon, 14.10.13 12:17, Aaron Lu (aaron.lu@intel.com) wrote:

> > Hmm, regarding your patch series, do you plan to skip the registration
> > of the acpi backlight device if the "raw" device is supported? I mean,
> 
> Yes, that's right.
> 
> > the intel driver could be compiled as a module (and generally is on the
> > popular distros), so at the time the ACPI subsystem wants to register
> > the backlight device and know if a raw backlight device is around it
> > never will be, so what is the point of that? Or am I missing something?
> 
> For systems with Intel i915 GPU, ACPI video will wait for GPU driver to
> run first, see drivers/acpi/video.c acpi_video_init, the actual
> acpi_video_register function is called by i915 driver in
> i915_driver_load due to operation region related stuff. Since all
> problematic systems reported so far has an Intel GPU, I'm doing it this
> way now. If things change, we can enhance it then.

Sorry, still not getting this. How should this ever work if the intel
video driver is compiled as kmod? That means that it isn't clear at all
when the kmod is going to be loaded or if it is loaded at all, you
cannot delay the registration of the acpi backlight that long, since the
time you'd have to wait is basically unbounded...

So, how could this ever work?

AFAICS all popular distros ship the video drivers as kernel modules,
hence trying to avoid registration of the ACPI backlight if the intel
driver is compiled will be an entirely pointless excercise on all
distros?

What am I missing?

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

Re: [PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[Matthew Garrett]

On Mon, 2013-10-14 at 17:36 +0200, Lennart Poettering wrote:

> Sorry, still not getting this. How should this ever work if the intel
> video driver is compiled as kmod? That means that it isn't clear at all
> when the kmod is going to be loaded or if it is loaded at all, you
> cannot delay the registration of the acpi backlight that long, since the
> time you'd have to wait is basically unbounded...

See the intel_opregion_present() code in drivers/acpi/video.c. The ACPI
driver won't bind to Intel hardware until i915 indicates that it should
do so.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[Lennart Poettering]

On Mon, 14.10.13 15:48, Matthew Garrett (matthew.garrett@nebula.com) wrote:

> On Mon, 2013-10-14 at 17:36 +0200, Lennart Poettering wrote:
> 
> > Sorry, still not getting this. How should this ever work if the intel
> > video driver is compiled as kmod? That means that it isn't clear at all
> > when the kmod is going to be loaded or if it is loaded at all, you
> > cannot delay the registration of the acpi backlight that long, since the
> > time you'd have to wait is basically unbounded...
> 
> See the intel_opregion_present() code in drivers/acpi/video.c. The ACPI
> driver won't bind to Intel hardware until i915 indicates that it should
> do so.

Hmm, OK, so this means that the acpi backlight will check whether i915
hw is around, and not whether the i915 driver is actually loaded? That
would work for me I guess. Thanks.

That means on win8 machines with win8 graphics there'll always be a
single backlight device in userspace only?

Lennart

-- 
Lennart Poettering - Red Hat, Inc.

Re: [PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[Matthew Garrett]

On Mon, 2013-10-14 at 18:24 +0200, Lennart Poettering wrote:

> That means on win8 machines with win8 graphics there'll always be a
> single backlight device in userspace only?

That's the assumption, but we potentially still need to figure out why a
couple of machines are broken to make that happen.

-- 
Matthew Garrett <matthew.garrett@nebula.com>

Re: [PATCH] acpi/video: Add Lenovo IdeaPad Yoga 13 to acpi video detect blacklist

[Rafael J. Wysocki]

On Monday, October 14, 2013 02:55:05 AM Lennart Poettering wrote:
> On the Yoga 13 the backlight control doesn't work via ACPI. (And doesn't
> work either with the low-level platform driver ideapad_laptop; but
> works correctly via the intel video driver).  This patch hence adds the
> Yoga 13 to the ACPI video detect blacklist, to make sure the broken ACPI
> backlight device is never exposed to userspace.
> 
> Note that this appears unrelated to the Windows 8 backlight issues tracked
> here:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=51231
> https://bugzilla.kernel.org/show_bug.cgi?id=60682
> 
> The Yoga's ACPI backlight controls work neither with nor without
> acpi_osi="!Windows 2012" on the kernel command line. It appears that
> backlight control via the EC simply is not available at all, regardless
> whether done via ACPI or via the vendor driver.
> 
> Signed-off-by: Lennart Poettering <lennart@poettering.net>

I've added this to my queue for 3.13.

Thanks!

> ---
>  drivers/acpi/video_detect.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/acpi/video_detect.c b/drivers/acpi/video_detect.c
> index 940edbf..a88e8f7 100644
> --- a/drivers/acpi/video_detect.c
> +++ b/drivers/acpi/video_detect.c
> @@ -168,6 +168,14 @@ static struct dmi_system_id video_detect_dmi_table[] = {
>  		DMI_MATCH(DMI_PRODUCT_NAME, "UL30A"),
>  		},
>  	},
> +	{
> +	.callback = video_detect_force_vendor,
> +	.ident = "Lenovo Yoga 13",
> +	.matches = {
> +		DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> +		DMI_MATCH(DMI_PRODUCT_VERSION, "Lenovo IdeaPad Yoga 13"),
> +		},
> +	},
>  	{ },
>  };
>  
> 
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.