[PATCH 1/2] asus-wmi: Add a no backlight quirk

[PATCH 1/2] asus-wmi: Add a no backlight quirk

[Hans de Goede]

Some Asus motherboards for desktop PC-s export an acpi-video and
an asus-wmi interface advertising backlight support. Add a quirk to allow
to blacklist these so that desktop environments such as gnome don't start
showing nonsense brightness controls.

https://bugzilla.redhat.com/show_bug.cgi?id=1097436

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/asus-wmi.c | 8 ++++++--
 drivers/platform/x86/asus-wmi.h | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index c5e082f..6f73dc5 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -1272,6 +1272,9 @@ static int asus_wmi_backlight_init(struct asus_wmi *asus)
 	int max;
 	int power;
 
+	if (asus->driver->quirks->no_backlight)
+		return -ENODEV;
+
 	max = read_brightness_max(asus);
 
 	if (max == -ENODEV)
@@ -1370,7 +1373,7 @@ static void asus_wmi_notify(u32 value, void *context)
 		code = ASUS_WMI_BRN_DOWN;
 
 	if (code == ASUS_WMI_BRN_DOWN || code == ASUS_WMI_BRN_UP) {
-		if (!acpi_video_backlight_support()) {
+		if (asus->backlight_device) {
 			asus_wmi_backlight_notify(asus, orig_code);
 			goto exit;
 		}
@@ -1773,7 +1776,8 @@ static int asus_wmi_add(struct platform_device *pdev)
 	if (err)
 		goto fail_rfkill;
 
-	if (asus->driver->quirks->wmi_backlight_power)
+	if (asus->driver->quirks->wmi_backlight_power ||
+			asus->driver->quirks->no_backlight)
 		acpi_video_dmi_promote_vendor();
 	if (!acpi_video_backlight_support()) {
 		pr_info("Disabling ACPI video driver\n");
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index 4da4c8b..cc47efe 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -42,6 +42,7 @@ struct quirk_entry {
 	bool scalar_panel_brightness;
 	bool store_backlight_power;
 	bool wmi_backlight_power;
+	bool no_backlight;
 	int wapf;
 	/*
 	 * For machines with AMD graphic chips, it will send out WMI event
-- 
1.9.0

[PATCH 2/2] eeepc-wmi: Add no backlight quirk for Asus H87I-PLUS Motherboard

[Hans de Goede]

https://bugzilla.redhat.com/show_bug.cgi?id=1097436

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/eeepc-wmi.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/platform/x86/eeepc-wmi.c b/drivers/platform/x86/eeepc-wmi.c
index 6112933..a7286bb 100644
--- a/drivers/platform/x86/eeepc-wmi.c
+++ b/drivers/platform/x86/eeepc-wmi.c
@@ -114,6 +114,10 @@ static struct quirk_entry quirk_asus_x101ch = {
 	.wmi_backlight_power = true,
 };
 
+static struct quirk_entry quirk_asus_no_backlight = {
+	.no_backlight = true,
+};
+
 static struct quirk_entry *quirks;
 
 static void et2012_quirks(void)
@@ -182,6 +186,15 @@ static struct dmi_system_id asus_quirks[] = {
 		},
 		.driver_data = &quirk_asus_x101ch,
 	},
+	{
+		.callback = dmi_matched,
+		.ident = "ASUSTeK Computer INC. H87I-PLUS",
+		.matches = {
+			DMI_MATCH(DMI_BOARD_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_BOARD_NAME, "H87I-PLUS"),
+		},
+		.driver_data = &quirk_asus_no_backlight,
+	},
 	{},
 };
 
-- 
1.9.0

Re: [PATCH 2/2] eeepc-wmi: Add no backlight quirk for Asus H87I-PLUS Motherboard

[Matthew Garrett]

On Thu, 2014-05-15 at 11:39 +0200, Hans de Goede wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1097436

I'm not especially keen on this - if this seems like a general problem,
adding boards piecemeal to a DMI table will never solve it for most
people. What does performing the backlight calls actually do?

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

Re: [PATCH 2/2] eeepc-wmi: Add no backlight quirk for Asus H87I-PLUS Motherboard

[Matthew Garrett]

On Tue, 2014-06-10 at 16:16 +0000, Matthew Garrett wrote:
> On Thu, 2014-05-15 at 11:39 +0200, Hans de Goede wrote:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1097436
> 
> I'm not especially keen on this - if this seems like a general problem,
> adding boards piecemeal to a DMI table will never solve it for most
> people. What does performing the backlight calls actually do?

Or, alternatively, check the DMI chassis type and just skip desktop
boards?

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

Re: [PATCH 2/2] eeepc-wmi: Add no backlight quirk for Asus H87I-PLUS Motherboard

[Hans de Goede]

Hi,

On 06/10/2014 06:16 PM, Matthew Garrett wrote:
> On Thu, 2014-05-15 at 11:39 +0200, Hans de Goede wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1097436
> 
> I'm not especially keen on this - if this seems like a general problem,
> adding boards piecemeal to a DMI table will never solve it for most
> people. What does performing the backlight calls actually do?

The user has reported no adverse effects from the backlight control being
present, other then gnome3 showing a brightness control in its system
menu which is undesirable on a desktop machine.

Regards,

Hans

Re: [PATCH 2/2] eeepc-wmi: Add no backlight quirk for Asus H87I-PLUS Motherboard

[Hans de Goede]

Hi,

On 06/10/2014 06:17 PM, Matthew Garrett wrote:
> On Tue, 2014-06-10 at 16:16 +0000, Matthew Garrett wrote:
>> On Thu, 2014-05-15 at 11:39 +0200, Hans de Goede wrote:
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1097436
>>
>> I'm not especially keen on this - if this seems like a general problem,
>> adding boards piecemeal to a DMI table will never solve it for most
>> people. What does performing the backlight calls actually do?
> 
> Or, alternatively, check the DMI chassis type and just skip desktop
> boards?

That might work, note that part of the problem is the BIOS exporting
an acpi-video interface. So we would either need to do this check
in the acpi-video driver, or alternatively do it in the asus-wmi driver
and call acpi_video_dmi_promote_vendor() when the check fails.

I'm not 100% sold on adding this check in general, because it assumes
that the chassis type will be reliable, which seems like a long shot,
ie what if an all in one, with a backlight, uses 3 / Desktop as chassis
type ?

Note that once the acpi-video interface is disabled by using e.g.
acpi_backlight=vendor, then the asus-wmi driver will create a backlight
control with a max_brightness of 0, which seems like a bug in the asus-wmi
driver. I did not do a patch for this because I was afraid that not
registering the asus-wmi brightness control when the max_brightness == 0
might cause regressions (e.g. it will also remove the bl_power function,
what if in some cases max_brightness == 0, but we want / need bl_power ?) .

Regards,

Hans

Re: [PATCH 2/2] eeepc-wmi: Add no backlight quirk for Asus H87I-PLUS Motherboard

[Matthew Garrett]

On Wed, Jun 11, 2014 at 04:13:00PM +0200, Hans de Goede wrote:

> That might work, note that part of the problem is the BIOS exporting
> an acpi-video interface. So we would either need to do this check
> in the acpi-video driver, or alternatively do it in the asus-wmi driver
> and call acpi_video_dmi_promote_vendor() when the check fails.

Mm. Doing it in asus-wmi sounds like a reasonable plan for the moment if 
we don't see this more generally.

> I'm not 100% sold on adding this check in general, because it assumes
> that the chassis type will be reliable, which seems like a long shot,
> ie what if an all in one, with a backlight, uses 3 / Desktop as chassis
> type ?

I'm sure someone will let us know if we break that.

> Note that once the acpi-video interface is disabled by using e.g.
> acpi_backlight=vendor, then the asus-wmi driver will create a backlight
> control with a max_brightness of 0, which seems like a bug in the asus-wmi
> driver. I did not do a patch for this because I was afraid that not
> registering the asus-wmi brightness control when the max_brightness == 0
> might cause regressions (e.g. it will also remove the bl_power function,
> what if in some cases max_brightness == 0, but we want / need bl_power ?) .

I'd be... surprised if anyone's using the interface that way. Let's give 
it a go and see?

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/2] eeepc-wmi: Add no backlight quirk for Asus H87I-PLUS Motherboard

[Matthew Garrett]

On Wed, 2014-06-11 at 15:57 +0200, Hans de Goede wrote:
> Hi,
> 
> On 06/10/2014 06:16 PM, Matthew Garrett wrote:
> > On Thu, 2014-05-15 at 11:39 +0200, Hans de Goede wrote:
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1097436
> > 
> > I'm not especially keen on this - if this seems like a general problem,
> > adding boards piecemeal to a DMI table will never solve it for most
> > people. What does performing the backlight calls actually do?
> 
> The user has reported no adverse effects from the backlight control being
> present, other then gnome3 showing a brightness control in its system
> menu which is undesirable on a desktop machine.

Sorry, yeah, I wasn't clear. What results do we get from the calls? If
they just return 0s then we should alter the driver to skip backlight
registration in that case.

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

Re: [PATCH 2/2] eeepc-wmi: Add no backlight quirk for Asus H87I-PLUS Motherboard

[Hans de Goede]

Hi,

On 06/11/2014 06:03 PM, Matthew Garrett wrote:
> On Wed, 2014-06-11 at 15:57 +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 06/10/2014 06:16 PM, Matthew Garrett wrote:
>>> On Thu, 2014-05-15 at 11:39 +0200, Hans de Goede wrote:
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1097436
>>>
>>> I'm not especially keen on this - if this seems like a general problem,
>>> adding boards piecemeal to a DMI table will never solve it for most
>>> people. What does performing the backlight calls actually do?
>>
>> The user has reported no adverse effects from the backlight control being
>> present, other then gnome3 showing a brightness control in its system
>> menu which is undesirable on a desktop machine.
>
> Sorry, yeah, I wasn't clear. What results do we get from the calls? If
> they just return 0s then we should alter the driver to skip backlight
> registration in that case.

The acpi_video0 backlight interface reports a max_backlight of 100, so it
appears to be functional, unlike the asus-wmi interface which is present but
appears non functional.

Regards,

Hans