[PATCH 1/1][Oneiric SRU] Platform: Brightness quirk for samsung laptop driver

[Seth Forshee]

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 at 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 at gmail.com>
> > Tested-by: David Herrmann <dh.herrmann at googlemail.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh at suse.de>
> > Signed-off-by: Matthew Garrett <mjg at redhat.com>
> > 
> > (cherry picked from commit ac080523141d5bfa5f60ef2436480f645f915e9c)
> > BugLink: http://bugs.launchpad.net/bugs/897378
> > Signed-off-by: Seth Forshee <seth.forshee at 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