Re: [PATCH v4] platform/x86: Add new vga-switcheroo gmux driver for ACPI-driven muxes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Daniel,

Quick self intro: I have take over drivers/platform/x86
maintainership from Andy; and I'm working my way through
the backlog of old patches in patchwork:
https://patchwork.kernel.org/project/platform-driver-x86/list/

On 9/2/20 7:38 PM, Daniel Dadap wrote:
> Some upcoming notebook designs utilize display muxes driven by a
> pair of ACPI methods, MXDM to query and configure the operational
> mode of the mux (integrated only, discrete only, hybrid non-muxed,
> hybrid with dynamic mux switching), and MXDS to query and set the
> mux state when running in dynamic switch mode.
> 
> Add a vga-switcheroo driver to support switching the mux on systems
> with the ACPI MXDM/MXDS interface. The mux mode cannot be changed
> dynamically (calling MXDM to change the mode won't have effect until
> the next boot, and calling MXDM to read the mux mode returns the
> active mode, not the mode that will be enabled on next boot), and
> MXDS only works when the mux mode is set to dynamic switch, so this
> driver will fail to load when MXDM reports any non-dynamic mode.
> 
> This driver currently only supports systems with Intel integrated
> graphics and NVIDIA discrete graphics. It will need to be updated if
> designs are developed using the same interfaces which utilize GPUs
> from other vendors.
> 
> v2,v3: misc. fixes suggested by Barnabás Pőcze <pobrn@xxxxxxxxxxxxxx>
> v4: misc. changes suggested by Lukas Wunner <lukas@xxxxxxxxx>

According to the discussion archived here:
https://patchwork.kernel.org/project/platform-driver-x86/patch/20200902173851.224368-1-ddadap@xxxxxxxxxx/

Andy did a review and you promised a new version, but I don't see
a newer version in patchwork. Can you please submit a new version
addressing Andy's remarks ? Then I will review and merge the new
version.

In that discussion you also promised a new version of this patch:
https://patchwork.kernel.org/project/platform-driver-x86/patch/20200731202154.11382-1-ddadap@xxxxxxxxxx/

So please also submit a new version of that series.

Thanks & Regards,

Hans





> 
> Signed-off-by: Daniel Dadap <ddadap@xxxxxxxxxx>
> ---
>  MAINTAINERS                      |   6 +
>  drivers/platform/x86/Kconfig     |   9 ++
>  drivers/platform/x86/Makefile    |   2 +
>  drivers/platform/x86/mxds-gmux.c | 265 +++++++++++++++++++++++++++++++
>  4 files changed, 282 insertions(+)
>  create mode 100644 drivers/platform/x86/mxds-gmux.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index eeff55560759..636c9259b345 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11510,6 +11510,12 @@ L:	linux-usb@xxxxxxxxxxxxxxx
>  S:	Maintained
>  F:	drivers/usb/musb/
>  
> +MXDS GMUX DRIVER
> +M:	Daniel Dadap <ddadap@xxxxxxxxxx>
> +L:	platform-driver-x86@xxxxxxxxxxxxxxx
> +S:	Supported
> +F:	drivers/platform/x86/mxds-gmux.c
> +
>  MXL301RF MEDIA DRIVER
>  M:	Akihiro Tsukada <tskd08@xxxxxxxxx>
>  L:	linux-media@xxxxxxxxxxxxxxx
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 0ad7ad8cf8e1..5d00ad1ffc0e 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1368,6 +1368,15 @@ config INTEL_TELEMETRY
>  	  directly via debugfs files. Various tools may use
>  	  this interface for SoC state monitoring.
>  
> +config MXDS_GMUX
> +	tristate "ACPI MXDS Gmux Driver"
> +	depends on ACPI_WMI
> +	depends on ACPI
> +	depends on VGA_SWITCHEROO
> +	help
> +	  This driver provides support for ACPI-driven gmux devices which are
> +	  present on some notebook designs with hybrid graphics.
> +
>  endif # X86_PLATFORM_DEVICES
>  
>  config PMC_ATOM
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 53408d965874..b79000733fae 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -146,3 +146,5 @@ obj-$(CONFIG_INTEL_TELEMETRY)		+= intel_telemetry_core.o \
>  					   intel_telemetry_pltdrv.o \
>  					   intel_telemetry_debugfs.o
>  obj-$(CONFIG_PMC_ATOM)			+= pmc_atom.o
> +
> +obj-$(CONFIG_MXDS_GMUX)			+= mxds-gmux.o
> diff --git a/drivers/platform/x86/mxds-gmux.c b/drivers/platform/x86/mxds-gmux.c
> new file mode 100644
> index 000000000000..dd7f6edaf0f3
> --- /dev/null
> +++ b/drivers/platform/x86/mxds-gmux.c
> @@ -0,0 +1,265 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * mxds-gmux: vga_switcheroo mux handler for ACPI MXDS muxes
> + *
> + * Copyright (C) 2020 NVIDIA Corporation
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses>.
> + *
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/acpi.h>
> +#include <linux/pci.h>
> +#include <linux/vga_switcheroo.h>
> +#include <linux/delay.h>
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("vga_switcheroo mux handler for ACPI MXDS muxes");
> +MODULE_AUTHOR("Daniel Dadap <ddadap@xxxxxxxxxx>");
> +
> +/*
> + * The mux doesn't have its own ACPI HID/CID, or WMI wrapper, so key off of
> + * the WMI wrapper for the related WMAA method for backlight control.
> + */
> +MODULE_ALIAS("wmi:603E9613-EF25-4338-A3D0-C46177516DB7");
> +
> +static struct pci_dev *ig_dev, *dg_dev;
> +static acpi_handle internal_mux_handle;
> +static acpi_handle external_mux_handle;
> +
> +enum acpi_method {
> +	MXDM = 0,
> +	MXDS,
> +	NUM_ACPI_METHODS
> +};
> +
> +static char *acpi_methods[NUM_ACPI_METHODS] = {
> +	[MXDM] = "MXDM",
> +	[MXDS] = "MXDS",
> +};
> +
> +enum mux_mode_command {
> +	MUX_MODE_GET = 0,
> +};
> +
> +enum mux_mode {
> +	MUX_MODE_DGPU_ONLY = 1,
> +	MUX_MODE_IGPU_ONLY = 2,
> +	MUX_MODE_MSHYBRID = 3,	/* Dual GPU, mux switched to iGPU */
> +	MUX_MODE_DYNAMIC = 4,	/* Dual GPU, dynamic mux switching */
> +};
> +
> +/*
> + * Call MXDS with argument value 0 to read the current state.
> + * When reading, a return value of 1 means iGPU and 2 means dGPU.
> + * Call MXDS with bit 0 set to change the current state.
> + * When changing state, clear bit 4 for iGPU and set bit 4 for dGPU.
> + */
> +
> +enum mux_state {
> +	MUX_STATE_IGPU = 1,
> +	MUX_STATE_DGPU = 2,
> +};
> +
> +enum mux_state_command {
> +	MUX_STATE_GET = 0,
> +	MUX_STATE_SET_IGPU = BIT(0),
> +	MUX_STATE_SET_DGPU = BIT(4) | BIT(0),
> +};
> +
> +static acpi_integer acpi_helper(acpi_handle handle, enum acpi_method method,
> +				acpi_integer action)
> +{
> +	union acpi_object arg = {
> +		.integer = { .type = ACPI_TYPE_INTEGER, .value = action }
> +	};
> +	struct acpi_object_list in = {.count = 1, .pointer = &arg};
> +	acpi_integer ret;
> +	acpi_status status;
> +
> +	status = acpi_evaluate_integer(handle, acpi_methods[method], &in, &ret);
> +
> +	if (ACPI_FAILURE(status)) {
> +		pr_err("ACPI %s failed: %s\n", acpi_methods[method],
> +			acpi_format_exception(status));
> +		return 0;
> +	}
> +
> +	return ret;
> +}
> +
> +static acpi_integer get_mux_mode(acpi_handle handle)
> +{
> +	return acpi_helper(handle, MXDM, MUX_MODE_GET);
> +}
> +
> +static acpi_integer get_mux_state(acpi_handle handle)
> +{
> +	return acpi_helper(handle, MXDS, MUX_STATE_GET);
> +}
> +
> +static void set_mux_state(acpi_handle handle, enum mux_state state)
> +{
> +	enum mux_state_command command;
> +
> +	switch (state) {
> +	case MUX_STATE_IGPU:
> +		command = MUX_STATE_SET_IGPU;
> +		break;
> +	case MUX_STATE_DGPU:
> +		command = MUX_STATE_SET_DGPU;
> +		break;
> +	default:
> +		return;
> +	}
> +
> +	acpi_helper(handle, MXDS, command);
> +}
> +
> +static int mxds_gmux_switchto(enum vga_switcheroo_client_id id)
> +{
> +	enum mux_state state;
> +
> +	switch (id) {
> +	case VGA_SWITCHEROO_IGD:
> +		state = MUX_STATE_IGPU;
> +		break;
> +	case VGA_SWITCHEROO_DIS:
> +		state = MUX_STATE_DGPU;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (internal_mux_handle) {
> +		set_mux_state(internal_mux_handle, state);
> +		if (get_mux_state(internal_mux_handle) != state)
> +			return -EAGAIN;
> +	}
> +
> +	if (external_mux_handle) {
> +		set_mux_state(external_mux_handle, state);
> +		if (get_mux_state(external_mux_handle) != state)
> +			return -EAGAIN;
> +	}
> +
> +	/* DP AUX can take up to 100ms to settle after mux switch */
> +	mdelay(100);
> +
> +	return 0;
> +}
> +
> +static enum vga_switcheroo_client_id mxds_gmux_get_client_id(
> +	struct pci_dev *dev)
> +{
> +	if (dev == ig_dev)
> +		return VGA_SWITCHEROO_IGD;
> +	if (dev == dg_dev)
> +		return VGA_SWITCHEROO_DIS;
> +
> +	return VGA_SWITCHEROO_UNKNOWN_ID;
> +}
> +
> +static const struct vga_switcheroo_handler handler = {
> +	.switchto = mxds_gmux_switchto,
> +	.get_client_id = mxds_gmux_get_client_id,
> +};
> +
> +static acpi_status find_acpi_methods(
> +	acpi_handle object, u32 nesting_level, void *context,
> +	void **return_value)
> +{
> +	acpi_handle search;
> +
> +	/* If either MXDM or MXDS is missing, we can't use this object */
> +	if (acpi_get_handle(object, "MXDM", &search))
> +		return AE_OK;
> +	if (acpi_get_handle(object, "MXDS", &search))
> +		return AE_OK;
> +
> +	/* MXDS only works when MXDM indicates dynamic mode */
> +	if (get_mux_mode(object) != MUX_MODE_DYNAMIC)
> +		return AE_OK;
> +
> +	/* Internal display has _BCL; external does not */
> +	if (acpi_get_handle(object, "_BCL", &search))
> +		external_mux_handle = object;
> +	else
> +		internal_mux_handle = object;
> +
> +	return AE_OK;
> +}
> +
> +static int __init mxds_gmux_init(void)
> +{
> +	int ret = 0;
> +	struct pci_dev *dev = NULL;
> +
> +	/* Currently only supports Intel integrated and NVIDIA discrete GPUs */
> +	while ((dev = pci_get_class(PCI_CLASS_DISPLAY_VGA << 8, dev))) {
> +		/* Ignore eGPU */
> +		if (pci_is_thunderbolt_attached(dev))
> +			continue;
> +
> +		switch (dev->vendor) {
> +		case PCI_VENDOR_ID_INTEL:
> +			pci_dev_put(ig_dev);
> +			ig_dev = pci_dev_get(dev);
> +			break;
> +		case PCI_VENDOR_ID_NVIDIA:
> +			pci_dev_put(dg_dev);
> +			dg_dev = pci_dev_get(dev);
> +			break;
> +		default:
> +			break;
> +		}
> +	}
> +
> +	/* Require both integrated and discrete GPUs */
> +	if (!ig_dev || !dg_dev) {
> +		ret = -ENODEV;
> +		goto done;
> +	}
> +
> +	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX,
> +		find_acpi_methods, NULL, NULL, NULL);
> +
> +	/* Require at least one mux */
> +	if (!internal_mux_handle && !external_mux_handle) {
> +		ret = -ENODEV;
> +		goto done;
> +	}
> +
> +	ret = vga_switcheroo_register_handler(&handler, 0);
> +
> +done:
> +
> +	if (ret) {
> +		pci_dev_put(ig_dev);
> +		pci_dev_put(dg_dev);
> +	}
> +
> +	return ret;
> +}
> +module_init(mxds_gmux_init);
> +
> +static void __exit mxds_gmux_exit(void)
> +{
> +	vga_switcheroo_unregister_handler();
> +	pci_dev_put(ig_dev);
> +	pci_dev_put(dg_dev);
> +}
> +module_exit(mxds_gmux_exit);
> 

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux