Re: [PATCH v2] platform/x86: Add driver for Apple gmux device

[Seth Forshee <seth.forshee@canonical.com>]

On Thu, Mar 01, 2012 at 10:19:58AM +0100, Corentin Chary wrote:
> On Wed, Feb 29, 2012 at 11:56 PM, Seth Forshee
> <seth.forshee@canonical.com> wrote:
> > On Wed, Feb 29, 2012 at 04:32:28PM -0600, Grant Likely wrote:
> >> On Wed, Feb 29, 2012 at 4:08 PM, Seth Forshee
> >> <seth.forshee@canonical.com> wrote:
> >> > On Wed, Feb 29, 2012 at 03:23:20PM -0600, Grant Likely wrote:
> >> >> On Wed, Feb 29, 2012 at 1:50 PM, Seth Forshee
> >> >> <seth.forshee@canonical.com> wrote:
> >> >> > On Wed, Feb 29, 2012 at 12:43:23PM -0600, Grant Likely wrote:
> >> >> >> On Wed, Feb 29, 2012 at 11:53 AM, Seth Forshee
> >> >> >> <seth.forshee@canonical.com> wrote:
> >> >> >> > On Wed, Feb 29, 2012 at 11:46:39AM -0600, Grant Likely wrote:
> >> >> >> >> On Wed, Feb 22, 2012 at 8:37 AM, Seth Forshee
> >> >> >> >> <seth.forshee@canonical.com> wrote:
> >> >> >> >> > Apple laptops with hybrid graphics have a device named gmux that
> >> >> >> >> > controls the muxing of the LVDS panel between the GPUs as well as screen
> >> >> >> >> > brightness. This driver adds support for the gmux device. Only backlight
> >> >> >> >> > control is supported initially.
> >> >> >> >> >
> >> >> >> >> > Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> >> >> >> >>
> >> >> >> >> Works for me.
> >> >> >> >>
> >> >> >> >> Tested-by: Grant Likely <grant.likely@secretlab.ca>
> >> >> >> >>
> >> >> >> >> Now I just need to figure out how to get the desktop backlight widget
> >> >> >> >> to use gmux_backlight instead of acpi_video0...
> >> >> >> >
> >> >> >> > The easy way is to pass acpi_backlight=vendor to the kernel, then you
> >> >> >> > won't have acpi_vidoe0.
> >> >> >>
> >> >> >> That did it, thanks.  I'm assume something is in the works to set it
> >> >> >> up automatically?
> >> >> >
> >> >> > Not that I'm aware of. A number machines have this problem, that the
> >> >> > standard ACPI backlight interfaces are implemented but don't work. This
> >> >> > generally isn't detectable in software; with the Apples at least
> >> >> > everything looks like it's working except that the brightness doesn't
> >> >> > change (but not all Apple laptops are affected, so qurking based on
> >> >> > manufacturer wouldn't work). All we're left with is DMI quirking, which
> >> >> > isn't practical. Maybe we could add something so a platform driver can
> >> >> > tell acpi_video that it knows the ACPI backlight doesn't work, but I
> >> >> > think on some platforms that still is going to be based off of DMI
> >> >> > information.
> >> >>
> >> >> blacklisting based on specific product name (ie. MacBookPro8,*) or
> >> >> machine model is probably the best.  It wouldn't be the first
> >> >> blacklist in the linux kernel.
> >> >
> >> > I think the blacklist would have to be against specific product names.
> >> > For example, the MacBook Pro 8,1 has a working acpi_video backlight and
> >> > no gmux_backlight, the 8,2 has both but only gmux_backlight works, and I
> >> > suspect the 8,3 is the same as the 8,2.
> >>
> >> I have the 8,3, and my testing confirms that.
> >>
> >> > We'd probably end up with an
> >> > entry in the blacklist for every single model whose acpi_video backlight
> >> > doesn't work, adding entries for each new generation of MacBooks.
> >> >
> >> > And if we start blacklisting Macs we'd have start doing it for other
> >> > machines too, I guess. From what I've seen, open-ended blacklists like
> >> > this get nacked pretty consistently nowadays.
> >>
> >> An alternative would be to blacklist or disable acpi0_backlight when
> >> the apple-gmux driver loads.  I don't know how acceptable that is, but
> >> I also don't have much sympathy for nacking blacklists if there isn't
> >> a viable alternative.
> >
> > Yes, that's one idea I was thinking about. For all the machines I've
> > been able to get tested, if the gmux is present then it can control the
> > backlight and acpi_video cannot, so that approach is reasonable for
> > Macs.
> >
> > There are quite a few machines in this situation though, and whatever
> > solution is arrived at should be flexible enough to work beyond just
> > Macs. I'll try to find some time soon to explore this further and see if
> > I can come up with something.
> 
> Old Samsung laptops have the same issue, and I ended up patching
> drivers/acpi/video_detect.c (check "[PATCH] ACPI / Video: blacklist
> some samsung laptops").
> Doing it in the vendor module is complicated, since, it will be loaded
> after the acpi video module most of the time, and that means addding
> acpi_backlight, then removing it, which will probably confuse
> usespace.
> Patching drivers/acpi/video_detect.c seems safer.

I certainly agree that it's the easiest solution, so maybe it is worth
trying first and seeing what the response is. But last I saw the patch
you're referring to hadn't been merged or even commented on yet. And I
can't help but note that you included as justification for the patch
that the acpi_video backlight works on newer Samsungs so the list
wouldn't grow ;)

Beyond Samsung and Apple I know that there are also Toshibas that suffer
from this, and judging by the list from older versions of g-s-d [1] I'd
say there may well be others.

Seth

[1] http://git.gnome.org/browse/gnome-settings-daemon/tree/plugins/power/gsd-backlight-helper.c?id=GNOME_SETTINGS_DAEMON_3_2_2#n50