[Nouveau] [Bug 86539] [NVAC] Trying to register nv_backlight after NV96 did it

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=86539

--- Comment #7 from Hans de Goede <jwrdegoede at fedoraproject.org> ---
(In reply to Lukas Wunner from comment #6)
> To add to this discussion, radeon has a quirk for the MacBookPro8,1 wherein
> a backlight is *not* registered since this machine has a gmux built in. See
> radeon_atom_backlight_init  in atombios_encoders.c:
> 
> http://cgit.freedesktop.org/~airlied/linux/tree/drivers/gpu/drm/radeon/
> atombios_encoders.c?h=drm-next#n189
> 
> 	/* Mac laptops with multiple GPUs use the gmux driver for backlight
> 	 * so don't register a backlight device
> 	 */
> 	if ((rdev->pdev->subsystem_vendor == PCI_VENDOR_ID_APPLE) &&
> 	    (rdev->pdev->device == 0x6741))
> 		return;
> 
> The code is wrong though in that it tries to recognize gmux by way of the
> machine's PCI ID. The MacBookPro8,1 was available with multiple different
> AMD GPUs and we'd need to hardcode all of their PCI IDs. Also, the
> MacBookPro 11,5 has an AMD GPU and we'd need to add its PCI ID as well.
> 
> A more sensible solution, as Pierre suggests, is to detect the presence of
> gmux and not register a backlight if gmux is found. So basically a quirk
> specific to all dual GPU MacBook Pros.
> 
> This patch adds a handy function to detect the presence of gmux.
> Unfortunately noone has reviewed the series it belongs to so far:
> http://lists.freedesktop.org/archives/dri-devel/2016-January/098403.html

Hi,

I agree that having an apple_gmux_present() helper is present then the current
code in the radeon driver, and we could use that in the nouveau driver too. But
this really should not be necessary at all. Things should work fine with the
radeon / nouveau driver registering their native / BACKLIGHT_RAW backlight
interfaces in sysfs, userspace should prefer the apple_gmux sysfs entry over a
raw type sysfs entry.

This is how things work on regular x86 laptops too, which often have both an
acpi_video backlight interface and a native one, so they have 2 backlight sysfs
interfaces too.

With that said I do agree that not registering a native driver at all in these
cases is better. So I think your apple_gmux patch + matching patches in the
nouveau / ati drivers are a good idea, but I do wonder if
this is really necessary.

Eitherway we should first solve the namespace clash when nouveau tries to
register multiple backlights, that may happen in non apple weird setups too, so
is something which we should fix anyways IMHO,
and fixing that before not registering a native backlight when the apple_gmux
is present allows us to actually test that fix :)

Pierre, can you whip up a patch for the nouveau_bl name clash, and that test
that on your affected machine ? Note I understand if you will delay this till
after Fosdem :)

Regards,

Hans

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/nouveau/attachments/20160126/34770484/attachment.html>