FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu Kernel Team

 
 
LinkBack Thread Tools
 
Old 12-02-2011, 07:29 PM
Seth Forshee
 
Default Platform: Brightness quirk for samsung laptop driver

BugLink: http://bugs.launchpad.net/bugs/897378

== SRU Justification ==

Impact: On Samsung NF310 and other models, backlight adjustments do not
work properly. The ACPI backlight interface doesn't work at all. The
samsung-laptop driver sort of works, but the backlight can only be set
to brightness levels 0, 1, and 8.

Fix: Upstream cherry pick for samsung-laptop to quirk brightness
adjustments for affected models.

Test Case: I've verified this personally on the NF310. Also verified to
help on the N220P on LP #810093, although an additional patch is
required to fully fix the issues on that machine.

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 12-02-2011, 07:29 PM
Seth Forshee
 
Default Platform: Brightness quirk for samsung laptop driver

From: Jason Stubbs <jasonbstubbs@gmail.com>

On some Samsung laptops the brightness regulation works slightly different.
All SABI commands except for set_brightness work as expected. The behaviour
of set_brightness is as follows:

- Setting a new brightness will only step one level toward the new brightness
level. For example, setting a level of 5 when the current level is 2 will
result in a brightness level of 3.
- A spurious KEY_BRIGHTNESS_UP or KEY_BRIGHTNESS_DOWN event is also generated
along with the change in brightness.
- Neither of the above two issues occur when changing from/to brightness
level 0.

This patch adds detection and a non-intrusive workaround for the above issues.

Signed-off-by: Jason Stubbs <jasonbstubbs@gmail.com>
Tested-by: David Herrmann <dh.herrmann@googlemail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-off-by: Matthew Garrett <mjg@redhat.com>

(cherry picked from commit ac080523141d5bfa5f60ef2436480f645f915e9c)
BugLink: http://bugs.launchpad.net/bugs/897378
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
drivers/platform/x86/samsung-laptop.c | 43 +++++++++++++++++++++++++++++++++
1 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
index ec85987..c32dc39 100644
--- a/drivers/platform/x86/samsung-laptop.c
+++ b/drivers/platform/x86/samsung-laptop.c
@@ -226,6 +226,7 @@ static struct backlight_device *backlight_device;
static struct mutex sabi_mutex;
static struct platform_device *sdev;
static struct rfkill *rfk;
+static bool has_stepping_quirk;

static int force;
module_param(force, bool, 0);
@@ -382,6 +383,17 @@ static void set_brightness(u8 user_brightness)
{
u8 user_level = user_brightness + sabi_config->min_brightness;

+ if (has_stepping_quirk && user_level != 0) {
+ /*
+ * short circuit if the specified level is what's already set
+ * to prevent the screen from flickering needlessly
+ */
+ if (user_brightness == read_brightness())
+ return;
+
+ sabi_set_command(sabi_config->commands.set_brightness, 0);
+ }
+
sabi_set_command(sabi_config->commands.set_brightness, user_level);
}

@@ -390,6 +402,34 @@ static int get_brightness(struct backlight_device *bd)
return (int)read_brightness();
}

+static void check_for_stepping_quirk(void)
+{
+ u8 initial_level = read_brightness();
+ u8 check_level;
+
+ /*
+ * Some laptops exhibit the strange behaviour of stepping toward
+ * (rather than setting) the brightness except when changing to/from
+ * brightness level 0. This behaviour is checked for here and worked
+ * around in set_brightness.
+ */
+
+ if (initial_level <= 2)
+ check_level = initial_level + 2;
+ else
+ check_level = initial_level - 2;
+
+ has_stepping_quirk = false;
+ set_brightness(check_level);
+
+ if (read_brightness() != check_level) {
+ has_stepping_quirk = true;
+ pr_info("enabled workaround for brightness stepping quirk
");
+ }
+
+ set_brightness(initial_level);
+}
+
static int update_status(struct backlight_device *bd)
{
set_brightness(bd->props.brightness);
@@ -813,6 +853,9 @@ static int __init samsung_init(void)
}
}

+ /* Check for stepping quirk */
+ 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))
--
1.7.5.4


--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 12-07-2011, 12:10 PM
Andy Whitcroft
 
Default Platform: Brightness quirk for samsung laptop driver

On Fri, Dec 02, 2011 at 02:29:28PM -0600, Seth Forshee wrote:
> From: Jason Stubbs <jasonbstubbs@gmail.com>
>
> On some Samsung laptops the brightness regulation works slightly different.
> All SABI commands except for set_brightness work as expected. The behaviour
> of set_brightness is as follows:
>
> - Setting a new brightness will only step one level toward the new brightness
> level. For example, setting a level of 5 when the current level is 2 will
> result in a brightness level of 3.
> - A spurious KEY_BRIGHTNESS_UP or KEY_BRIGHTNESS_DOWN event is also generated
> along with the change in brightness.
> - Neither of the above two issues occur when changing from/to brightness
> level 0.
>
> This patch adds detection and a non-intrusive workaround for the above issues.
>
> Signed-off-by: Jason Stubbs <jasonbstubbs@gmail.com>
> Tested-by: David Herrmann <dh.herrmann@googlemail.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
>
> (cherry picked from commit ac080523141d5bfa5f60ef2436480f645f915e9c)
> BugLink: http://bugs.launchpad.net/bugs/897378
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
> drivers/platform/x86/samsung-laptop.c | 43 +++++++++++++++++++++++++++++++++
> 1 files changed, 43 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
> index ec85987..c32dc39 100644
> --- a/drivers/platform/x86/samsung-laptop.c
> +++ b/drivers/platform/x86/samsung-laptop.c
> @@ -226,6 +226,7 @@ static struct backlight_device *backlight_device;
> static struct mutex sabi_mutex;
> static struct platform_device *sdev;
> static struct rfkill *rfk;
> +static bool has_stepping_quirk;
>
> static int force;
> module_param(force, bool, 0);
> @@ -382,6 +383,17 @@ static void set_brightness(u8 user_brightness)
> {
> u8 user_level = user_brightness + sabi_config->min_brightness;
>
> + if (has_stepping_quirk && user_level != 0) {
> + /*
> + * short circuit if the specified level is what's already set
> + * to prevent the screen from flickering needlessly
> + */
> + if (user_brightness == read_brightness())
> + return;
> +
> + sabi_set_command(sabi_config->commands.set_brightness, 0);
> + }
> +
> sabi_set_command(sabi_config->commands.set_brightness, user_level);

So I take it the quirk here is to flick to 0 and back to the new
brightness. As we can read the brightness could we just do like:

while (user_level != read_brightness())
sabi_set_command(sabi_config->commands.set_brightness,
user_level);

Either way it looks fine.

-apw

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 12-07-2011, 01:29 PM
Seth Forshee
 
Default Platform: Brightness quirk for samsung laptop driver

On Wed, Dec 07, 2011 at 01:10:23PM +0000, Andy Whitcroft wrote:
> On Fri, Dec 02, 2011 at 02:29:28PM -0600, Seth Forshee wrote:
> > From: Jason Stubbs <jasonbstubbs@gmail.com>
> >
> > On some Samsung laptops the brightness regulation works slightly different.
> > All SABI commands except for set_brightness work as expected. The behaviour
> > of set_brightness is as follows:
> >
> > - Setting a new brightness will only step one level toward the new brightness
> > level. For example, setting a level of 5 when the current level is 2 will
> > result in a brightness level of 3.
> > - A spurious KEY_BRIGHTNESS_UP or KEY_BRIGHTNESS_DOWN event is also generated
> > along with the change in brightness.
> > - Neither of the above two issues occur when changing from/to brightness
> > level 0.
> >
> > This patch adds detection and a non-intrusive workaround for the above issues.
> >
> > Signed-off-by: Jason Stubbs <jasonbstubbs@gmail.com>
> > Tested-by: David Herrmann <dh.herrmann@googlemail.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
> > Signed-off-by: Matthew Garrett <mjg@redhat.com>
> >
> > (cherry picked from commit ac080523141d5bfa5f60ef2436480f645f915e9c)
> > BugLink: http://bugs.launchpad.net/bugs/897378
> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> > ---
> > drivers/platform/x86/samsung-laptop.c | 43 +++++++++++++++++++++++++++++++++
> > 1 files changed, 43 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/platform/x86/samsung-laptop.c b/drivers/platform/x86/samsung-laptop.c
> > index ec85987..c32dc39 100644
> > --- a/drivers/platform/x86/samsung-laptop.c
> > +++ b/drivers/platform/x86/samsung-laptop.c
> > @@ -226,6 +226,7 @@ static struct backlight_device *backlight_device;
> > static struct mutex sabi_mutex;
> > static struct platform_device *sdev;
> > static struct rfkill *rfk;
> > +static bool has_stepping_quirk;
> >
> > static int force;
> > module_param(force, bool, 0);
> > @@ -382,6 +383,17 @@ static void set_brightness(u8 user_brightness)
> > {
> > u8 user_level = user_brightness + sabi_config->min_brightness;
> >
> > + if (has_stepping_quirk && user_level != 0) {
> > + /*
> > + * short circuit if the specified level is what's already set
> > + * to prevent the screen from flickering needlessly
> > + */
> > + if (user_brightness == read_brightness())
> > + return;
> > +
> > + sabi_set_command(sabi_config->commands.set_brightness, 0);
> > + }
> > +
> > sabi_set_command(sabi_config->commands.set_brightness, user_level);
>
> So I take it the quirk here is to flick to 0 and back to the new
> brightness. As we can read the brightness could we just do like:
>
> while (user_level != read_brightness())
> sabi_set_command(sabi_config->commands.set_brightness,
> user_level);

I actually tried this as on the NF310 as I occasionally see the first
part (setting the brightness to zero) succeed and the second part
(setting it to the desired value) fail, so I end up with a brightness of
0. These are pretty rare though; I've only seen them when I'm pounding
continuously on the backlight controls.

When using the loop you suggested, for whatever reason the value the
BIOS returns for the current brightness never changes even though the
brightness itself is changing, so the loop never terminates. If you
read the brightness later on the value is correct, but as long as you're
reading it in that loop the value doesn't change. Bouncing through 0
seems to be the most reliable method on these models.

The BIOS on the NF310 really just turns out to be utter garbage. There's
a number of other things I've found with equally broken behaviors.

Seth

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 12-08-2011, 09:36 AM
Andy Whitcroft
 
Default Platform: Brightness quirk for samsung laptop driver

On Wed, Dec 07, 2011 at 08:29:07AM -0600, Seth Forshee wrote:

> I actually tried this as on the NF310 as I occasionally see the first
> part (setting the brightness to zero) succeed and the second part
> (setting it to the desired value) fail, so I end up with a brightness of
> 0. These are pretty rare though; I've only seen them when I'm pounding
> continuously on the backlight controls.
>
> When using the loop you suggested, for whatever reason the value the
> BIOS returns for the current brightness never changes even though the
> brightness itself is changing, so the loop never terminates. If you
> read the brightness later on the value is correct, but as long as you're
> reading it in that loop the value doesn't change. Bouncing through 0
> seems to be the most reliable method on these models.

That almost sounds like gcc is not realising the function is volatile
and optimising it away. You might want to look at the loop and see if
the function is being inlined, and whether the optimiser has neutered
it. Stuffing in a full memory barrier in the loop may get gcc's head in
the game.

-apw

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 
Old 12-12-2011, 02:11 PM
Seth Forshee
 
Default Platform: Brightness quirk for samsung laptop driver

On Thu, Dec 08, 2011 at 10:36:37AM +0000, Andy Whitcroft wrote:
> On Wed, Dec 07, 2011 at 08:29:07AM -0600, Seth Forshee wrote:
>
> > I actually tried this as on the NF310 as I occasionally see the first
> > part (setting the brightness to zero) succeed and the second part
> > (setting it to the desired value) fail, so I end up with a brightness of
> > 0. These are pretty rare though; I've only seen them when I'm pounding
> > continuously on the backlight controls.
> >
> > When using the loop you suggested, for whatever reason the value the
> > BIOS returns for the current brightness never changes even though the
> > brightness itself is changing, so the loop never terminates. If you
> > read the brightness later on the value is correct, but as long as you're
> > reading it in that loop the value doesn't change. Bouncing through 0
> > seems to be the most reliable method on these models.
>
> That almost sounds like gcc is not realising the function is volatile
> and optimising it away. You might want to look at the loop and see if
> the function is being inlined, and whether the optimiser has neutered
> it. Stuffing in a full memory barrier in the loop may get gcc's head in
> the game.

I was pretty sure that wasn't the case, but I went back and
double-checked to be sure. read_brightness is in fact being called for
each loop iteration, but the value it returns doesn't always reflect
the actual brightness (sometimes the loop works, but rarely).

--
kernel-team mailing list
kernel-team@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/kernel-team
 

Thread Tools




All times are GMT. The time now is 08:20 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org