On Thu, 2012-08-30 at 13:54 -0400, Joseph Salisbury wrote:
> Hi,
> 
> Please consider reverting commit 
> 3be324a94df0c3f032178d04549dbfbf6cccb09a in the next v3.2.y release.  It
> was included upstream as of v3.4-rc1.  This commit introduced a 
> regression, described in the following bug:
> http://bugs.launchpad.net/bugs/1028151
> 
> The original request to include this commit in v3.2.y can be found at:
> http://www.mail-archive.com/[email protected]/msg10854.html
> 
> The issue with commit 3be324a9, is that samsung-laptop started to load 
> on far more machine types, including some where brightness was already 
> being handled by ACPI video module. samsung-laptop and ACPI video are 
> now conflicting.
> 
> The following commit can fix the regression: f34cd9ca ("samsung-laptop: 
> don't handle backlight if handled by acpi/video") instead of doing the 
> revert.

Right, I clearly should have applied that first.

> The was confirmed in the previously mentioned bug.  However, to 
> cherry-pick commit f34cd9ca, it would need to be backported, and would 
> also require the two commits before to apply successfuly.  The feeling 
> is all these changes may be too much for a stable release.
>
> Commits required as a pre-req to f34cd9ca:
> a6df48943a408b493d1aa141791d614a529d484e
> 5dea7a2094d5e60fe8f8ec4277d22d7ad6fa8c26

Yes, those are rather large!  But I think it can be backported alone
without enormous changes; see attached
'samsung-laptop-don-t-handle-backlight-if-handled-by-acpi-video.patch'.
Then the dropped hunks of 3be324a9 should also be applied; see attached
'samsung-laptop-make-the-dmi-check-less-strict-part-2.patch'.  Do these
work?

Ben.

-- 
Ben Hutchings
Time is nature's way of making sure that everything doesn't happen at once.
From: Corentin Chary <[email protected]>
Date: Sat, 26 Nov 2011 11:00:00 +0100
Subject: samsung-laptop: don't handle backlight if handled by acpi/video

commit f34cd9ca9320876e9c12764f052004628a03ba2d upstream.

samsung-laptop is not at all related to ACPI, but since this interface
is not documented at all, and the driver has to use it at load to
understand how it works on the laptop, I think it's a good idea to
disable it if a better solution is available.

Signed-off-by: Corentin Chary <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
[bwh: Backported to 3.2:
 - Adjust context, and change return to goto, since we do not have commit
   5dea7a2 ('samsung-laptop: move code into init/exit functions')
 - Use static variable since we do not have commit a6df489
   ('samsung-laptop: put all local variables in a single structure')]
Signed-off-by: Ben Hutchings <[email protected]>
---
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -21,6 +21,7 @@
 #include <linux/dmi.h>
 #include <linux/platform_device.h>
 #include <linux/rfkill.h>
+#include <linux/acpi.h>
 
 /*
  * This driver is needed because a number of Samsung laptops do not hook
@@ -226,6 +227,7 @@ static struct backlight_device *backligh
 static struct mutex sabi_mutex;
 static struct platform_device *sdev;
 static struct rfkill *rfk;
+static bool handle_backlight;
 static bool has_stepping_quirk;
 
 static int force;
@@ -602,6 +604,15 @@ static int __init samsung_init(void)
 	int retval;
 
 	mutex_init(&sabi_mutex);
+	handle_backlight = true;
+
+#ifdef CONFIG_ACPI
+	/* Don't handle backlight here if the acpi video already handle it */
+	if (acpi_video_backlight_support()) {
+		pr_info("Backlight controlled by ACPI video driver\n");
+		handle_backlight = false;
+	}
+#endif
 
 	if (!force && !dmi_check_system(samsung_dmi_table))
 		return -ENODEV;
@@ -661,7 +672,8 @@ static int __init samsung_init(void)
 		printk(KERN_DEBUG "ifaceP = 0x%08x\n", ifaceP);
 		printk(KERN_DEBUG "sabi_iface = %p\n", sabi_iface);
 
-		test_backlight();
+		if (handle_backlight)
+			test_backlight();
 		test_wireless();
 
 		retval = sabi_get_command(sabi_config->commands.get_brightness,
@@ -680,13 +692,17 @@ static int __init samsung_init(void)
 	}
 
 	/* Check for stepping quirk */
-	check_for_stepping_quirk();
+	if (handle_backlight)
+		check_for_stepping_quirk();
 
 	/* knock up a platform device to hang stuff off of */
 	sdev = platform_device_register_simple("samsung", -1, NULL, 0);
 	if (IS_ERR(sdev))
 		goto error_no_platform;
 
+	if (!handle_backlight)
+		goto skip_backlight;
+
 	/* create a backlight device to talk to this one */
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_PLATFORM;
@@ -702,6 +718,7 @@ static int __init samsung_init(void)
 	backlight_device->props.power = FB_BLANK_UNBLANK;
 	backlight_update_status(backlight_device);
 
+skip_backlight:
 	retval = init_wireless(sdev);
 	if (retval)
 		goto error_no_rfk;
From: Ben Hutchings <[email protected]>
Date: Sun, 09 Sep 2012 18:49:14 +0100
Subject: samsung-laptop: make the dmi check less strict (part 2)

commit 3be324a94df0c3f032178d04549dbfbf6cccb09a upstream.

These are the hunks that I dropped when backporting for 3.2.24,
which are applicable now that we also have commit f34cd9ca
('samsung-laptop: don't handle backlight if handled by acpi/video').

Signed-off-by: Ben Hutchings <[email protected]>
---
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -608,10 +608,8 @@ static int __init samsung_init(void)
 
 #ifdef CONFIG_ACPI
 	/* Don't handle backlight here if the acpi video already handle it */
-	if (acpi_video_backlight_support()) {
-		pr_info("Backlight controlled by ACPI video driver\n");
+	if (acpi_video_backlight_support())
 		handle_backlight = false;
-	}
 #endif
 
 	if (!force && !dmi_check_system(samsung_dmi_table))
@@ -695,6 +693,12 @@ static int __init samsung_init(void)
 	if (handle_backlight)
 		check_for_stepping_quirk();
 
+#ifdef CONFIG_ACPI
+	/* Only log that if we are really on a sabi platform */
+	if (acpi_video_backlight_support())
+		pr_info("Backlight controlled by ACPI video driver\n");
+#endif
+
 	/* knock up a platform device to hang stuff off of */
 	sdev = platform_device_register_simple("samsung", -1, NULL, 0);
 	if (IS_ERR(sdev))

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to