[trusty] ACPI / video: Change the default for video.use_native_backlight to 1

[Seth Forshee]

On Thu, Aug 14, 2014 at 05:16:57PM +0800, Yidi Lin wrote:
> I would like to propose this patch for trusty kernel again.
> 
> According to upstream's discussion[1], following patches are necessary.
> 
> commit fbc9fe1b4f222a7c575e3bd8e9defe59c6190a04
> Author: Aaron Lu <aaron.lu at intel.com>
> Date:   Fri Oct 11 21:27:45 2013 +0800
> 
>     ACPI / video: Do not register backlight if win8 and native interface exists
> 
> commit 0e9f81d3b7cd0649a3bc437391b6a0650f98f844
> Author: Aaron Lu <aaron.lu at intel.com>
> Date:   Tue Feb 18 13:54:20 2014 +0800
> 
>     ACPI / video: Add systems that should favour native backlight interface
> 
> commit 751109aad5834375ca9ed0dcfcd85a00cbf872b5
> Author: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
> Date:   Thu Jun 5 22:47:35 2014 +0200
> 
>     ACPI / video: Change the default for video.use_native_backlight to 1
> 
> First two patches have already committed in trusty kernel.
> And "video.use_native_backlight=1" fixes most cases of backlight issues[2].
> 
> So I consider this patch could be backported to trusty kernel.

In some ways I agree. Taking the big picture view, this is better than
piecemeal fixing of systems by adding more and more quirks to the table.

On the other hand I expect there to be regressions on some systems. 3.16
hasn't been out long enough to make its way to most users, and once it
does I wouldn't be surprised to find out that the GPU backlight drivers
are broken for some of these Win 8 systems. If I'm right, any such
machines will be left without working backlight control by default when
this change hits. In fact, a quick google search turned up LP#1341108
which complains about backlight behavior regressing when we moved utopic
to 3.16, and also https://bugzilla.redhat.com/show_bug.cgi?id=1123661.
And then there's
http://comments.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/40409,
for which we may or may not have the fix.

There's also the fact that this was implemented by defaulting the module
parameter to 1, which has the effect of making the default value
override the related dmi quirks:

    static bool acpi_video_use_native_backlight(void)
    {
            if (use_native_backlight_param != -1)
                    return use_native_backlight_param;
            else
                    return use_native_backlight_dmi;
    }

Even if we wanted to we couldn't quirk around the new default behavior
without changing the implementation.

Also if you look at the rest of the commit message for 751109aa you'll
see, "Now that we're hoping to have resolved all of the problems with
video.use_native_backlight=1, make that the default at last." Iirc those
"problems" were primarily bugs in the GPU backlight drivers which left
users without functional backlight control by default. Have you verified
that we've picked up all those fixes?

So given our policy against regressions in SRU kernels, I'm still not
favorably disposed towards picking this up right now.

> 
> Edward
> 
> [1]: http://marc.info/?l=linux-acpi&m=140429106012604&w=2
> [2]: bug 1284030 / bug 1284030 / bug 1332437 / bug 1250401 / bug 1332379 /
>       bug 1354253 / bug 1341988 / bug 1354313
> 
> On Thu, Jun 19, 2014 at 10:27 AM, Yidi Lin <yidi.lin at canonical.com> wrote:
> > After checking upstream commit logs, several patches are needed for
> > use_native_backlight=1.
> > But it does not guarantee that the patches work for every model.
> > It seems that adding a quirk for Dell Inspiron 5737 is much safer.
> > I will take Seth's suggestion.
> > Thanks.
> >
> > On Wed, Jun 18, 2014 at 8:37 PM, Seth Forshee
> > <seth.forshee at canonical.com> wrote:
> >> On Wed, Jun 18, 2014 at 10:21:31AM +0800, Yidi Lin wrote:
> >>> Thanks for reminding.
> >>>
> >>> I will check what patches are necessary when use_native_backlight=1 is in use.
> >>
> >> Your other option is a sauce patch to add this machine to the dmi list
> >> in acpi_video.c. I think this is the safer option to avoid regressions
> >> with other hardware in 3.13. They've been working on these backlight
> >> issues for a long time because the solutions kept causing regressions on
> >> various machines, so I'm going to remain skeptical that they've really
> >> eliminated all the regressions until the changes have been upstream for
> >> quite a bit longer.
> >>
> >> Seth
> >>
> >>>
> >>> On Tue, Jun 17, 2014 at 7:40 PM, Tim Gardner <tim.gardner at canonical.com> wrote:
> >>> > On 06/16/2014 10:55 PM, Yidi Lin wrote:
> >>> >> BugLink: https://bugs.launchpad.net/bugs/1250401
> >>> >>
> >>> >> This patch is committed in v3.16-rc1 which fixes win8 backlight issue.
> >>> >> I had verified this patch for bug #1250401.
> >>> >>
> >>> >> This patch also fixes bug #1284030.
> >>> >>
> >>> >> commit 751109aad5834375ca9ed0dcfcd85a00cbf872b5
> >>> >> Author: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
> >>> >> Date:   Thu Jun 5 22:47:35 2014 +0200
> >>> >>
> >>> >>     ACPI / video: Change the default for video.use_native_backlight to 1
> >>> >>
> >>> >>     Now that we're hoping to have resolved all of the problems with
> >>> >>     video.use_native_backlight=1, make that the default at last.
> >>> >>
> >>> >>     Link: http://marc.info/?l=linux-acpi&m=139716088401106&w=2
> >>> >>     Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
> >>> >>
> >>> >
> >>> > While this patch may fix back light issues for a Dell Inspiron 5737, how
> >>> > do you know changing the default won't break other platforms ? I imagine
> >>> > there was a bit of work that went into back light support between 3.13
> >>> > and 3.16-rc1, so I'm not sure flipping the default in 3.13 is sufficient.
> >>> >
> >>> > rtg
> >>> > --
> >>> > Tim Gardner tim.gardner at canonical.com
> >>>
> >>> --
> >>> kernel-team mailing list
> >>> kernel-team at lists.ubuntu.com
> >>> https://lists.ubuntu.com/mailman/listinfo/kernel-team