Hello Lukas,
Yes, your patch solves the issue for me.
Thanks,
Gaele
On 14-02-16 14:24, Lukas Wunner wrote:
> Hi Gaele,
>
> On Sun, Feb 14, 2016 at 12:39:12AM +0100, Lukas Wunner wrote:
>> On dual GPU MacBook Pros, brightness is controlled by gmux.
>> It is changed by writing to gmux' I/O port range, 0x700 - 0x7FF.
>> gmux is located on the LPC bus, i.e. behind the southbridge.
>>
>> For communication with gmux to work, the above-mentioned I/O port
>> range needs to be routed to bus 00, where the LPC bus bridge is
>> located. Incidentally the integrated GPU is also located on that
>> same bus, whereas the discrete GPU is on a different bus. (It is
>> directly connected to the PCI Root Complex in the CPU.)
> FWIW, I've cobbled together an experimental patch (included below)
> which locks I/O to the LPC bridge instead of the integrated GPU.
>
> This requires a modification to vgaarb.c to deal with not just
> PCI_CLASS_DISPLAY_VGA, but also PCI_CLASS_BRIDGE_ISA devices.
> As to the justification, well it *could* be argued that back in
> the 1990s we had mainboards with PCI + ISA slots and for backward
> compatibility we need to support VGA devices in ISA slots located
> behind a PCI/ISA bridge. ;-)
>
> Seriously though, maybe you want to give this a try and see if
> it solves the issue for you (apply with git am --scissors).
>
> Best regards,
>
> Lukas
>
> -- >8 --
> Subject: [PATCH] apple-gmux: Fix IO locking
>
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> ---
> drivers/gpu/vga/vgaarb.c | 3 ++-
> drivers/platform/x86/apple-gmux.c | 51 ++++++++++++++-------------------------
> 2 files changed, 20 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index f17cb04..b23b471 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -529,7 +529,8 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev)
> u16 cmd;
>
> /* Only deal with VGA class devices */
> - if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
> + if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA &&
> + (pdev->class >> 8) != PCI_CLASS_BRIDGE_ISA)
> return false;
>
> /* Allocate structure */
> diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
> index f236250..999daad 100644
> --- a/drivers/platform/x86/apple-gmux.c
> +++ b/drivers/platform/x86/apple-gmux.c
> @@ -48,10 +48,10 @@
> struct apple_gmux_data {
> unsigned long iostart;
> unsigned long iolen;
> + struct pci_dev *lpc_bridge;
> bool indexed;
> struct mutex index_lock;
>
> - struct pci_dev *pdev;
> struct backlight_device *bdev;
>
> /* switcheroo data */
> @@ -530,34 +530,17 @@ static int gmux_resume(struct device *dev)
> return 0;
> }
>
> -static struct pci_dev *gmux_get_io_pdev(void)
> -{
> - struct pci_dev *pdev = NULL;
> -
> - while ((pdev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, pdev))) {
> - u16 cmd;
> -
> - pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> - if (!(cmd & PCI_COMMAND_IO))
> - continue;
> -
> - return pdev;
> - }
> -
> - return NULL;
> -}
> -
> static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
> {
> struct apple_gmux_data *gmux_data;
> struct resource *res;
> + struct pci_dev *lpc_bridge;
> struct backlight_properties props;
> struct backlight_device *bdev;
> u8 ver_major, ver_minor, ver_release;
> int ret = -ENXIO;
> acpi_status status;
> unsigned long long gpe;
> - struct pci_dev *pdev = NULL;
>
> if (apple_gmux_data)
> return -EBUSY;
> @@ -622,16 +605,17 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id)
> * disables IO/MEM used for backlight control on some systems.
> * Lock IO+MEM to GPU with active IO to prevent switch.
> */
> - pdev = gmux_get_io_pdev();
> - if (pdev && vga_tryget(pdev,
> - VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM)) {
> - pr_err("IO+MEM vgaarb-locking for PCI:%s failed\n",
> - pci_name(pdev));
> + lpc_bridge = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
> + if (lpc_bridge &&
> + !vga_tryget(lpc_bridge, VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM)) {
> + pr_debug("Locked IO to %s\n", pci_name(lpc_bridge));
> + gmux_data->lpc_bridge = lpc_bridge;
> + } else {
> + pr_err("Failed to lock IO to %s\n", pci_name(lpc_bridge));
> + pci_dev_put(lpc_bridge);
> ret = -EBUSY;
> goto err_release;
> - } else if (pdev)
> - pr_info("locked IO for PCI:%s\n", pci_name(pdev));
> - gmux_data->pdev = pdev;
> + }
>
> memset(&props, 0, sizeof(props));
> props.type = BACKLIGHT_PLATFORM;
> @@ -725,10 +709,11 @@ err_enable_gpe:
> err_notify:
> backlight_device_unregister(bdev);
> err_release:
> - if (gmux_data->pdev)
> - vga_put(gmux_data->pdev,
> + if (gmux_data->lpc_bridge) {
> + vga_put(gmux_data->lpc_bridge,
> VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM);
> - pci_dev_put(pdev);
> + pci_dev_put(gmux_data->lpc_bridge);
> + }
> release_region(gmux_data->iostart, gmux_data->iolen);
> err_free:
> kfree(gmux_data);
> @@ -748,10 +733,10 @@ static void gmux_remove(struct pnp_dev *pnp)
> &gmux_notify_handler);
> }
>
> - if (gmux_data->pdev) {
> - vga_put(gmux_data->pdev,
> + if (gmux_data->lpc_bridge) {
> + vga_put(gmux_data->lpc_bridge,
> VGA_RSRC_NORMAL_IO | VGA_RSRC_NORMAL_MEM);
> - pci_dev_put(gmux_data->pdev);
> + pci_dev_put(gmux_data->lpc_bridge);
> }
> backlight_device_unregister(gmux_data->bdev);
>
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
