Commit d143908f authored by Hans de Goede's avatar Hans de Goede
Browse files

platform/x86: apple-gmux: Add apple_gmux_detect() helper 

Add a new (static inline) apple_gmux_detect() helper to apple-gmux.h
which can be used for gmux detection instead of apple_gmux_present().

The latter is not really reliable since an ACPI device with a HID
of APP000B is present on some devices without a gmux at all, as well
as on devices with a newer (unsupported) MMIO based gmux model.

This causes apple_gmux_present() to return false-positives on
a number of different Apple laptop models.

This new helper uses the same probing as the actual apple-gmux
driver, so that it does not return false positives.

To avoid code duplication the gmux_probe() function of the actual
driver is also moved over to using the new apple_gmux_detect() helper.

This avoids false positives (vs _HID + IO region detection) on:

MacBookPro5,4
https://pastebin.com/8Xjq7RhS

MacBookPro8,1
https://linux-hardware.org/?probe=e513cfbadb&log=dmesg

MacBookPro9,2
https://bugzilla.kernel.org/attachment.cgi?id=278961

MacBookPro10,2
https://lkml.org/lkml/2014/9/22/657

MacBookPro11,2
https://forums.fedora-fr.org/viewtopic.php?id=70142

MacBookPro11,4
https://raw.githubusercontent.com/im-0/investigate-card-reader-suspend-problem-on-mbp11.4/master/test-16/dmesg

Fixes: 21245df3 ("ACPI: video: Add Apple GMUX brightness control detection")
Link: https://lore.kernel.org/platform-driver-x86/20230123113750.462144-1-hdegoede@redhat.com/


Reported-by: default avatarEmmanouil Kouroupakis <kartebi@gmail.com>
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20230124105754.62167-3-hdegoede@redhat.com
parent 39f5a81f

drivers/platform/x86/apple-gmux.c

+18 −52
Original line number Diff line number Diff line
@@ -226,23 +226,6 @@ static void gmux_write32(struct apple_gmux_data *gmux_data, int port, 
		gmux_pio_write32(gmux_data, port, val);

}



static bool gmux_is_indexed(struct apple_gmux_data *gmux_data)

{

	u16 val;



	outb(0xaa, gmux_data->iostart + 0xcc);

	outb(0x55, gmux_data->iostart + 0xcd);

	outb(0x00, gmux_data->iostart + 0xce);



	val = inb(gmux_data->iostart + 0xcc) |

		(inb(gmux_data->iostart + 0xcd) << 8);



	if (val == 0x55aa)

		return true;



	return false;

}



/**

 * DOC: Backlight control

 *
@@ -582,60 +565,43 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id) 
	int ret = -ENXIO;

	acpi_status status;

	unsigned long long gpe;

	bool indexed = false;

	u32 version;



	if (apple_gmux_data)

		return -EBUSY;



	if (!apple_gmux_detect(pnp, &indexed)) {

		pr_info("gmux device not present\n");

		return -ENODEV;

	}



	gmux_data = kzalloc(sizeof(*gmux_data), GFP_KERNEL);

	if (!gmux_data)

		return -ENOMEM;

	pnp_set_drvdata(pnp, gmux_data);



	res = pnp_get_resource(pnp, IORESOURCE_IO, 0);

	if (!res) {

		pr_err("Failed to find gmux I/O resource\n");

		goto err_free;

	}



	gmux_data->iostart = res->start;

	gmux_data->iolen = resource_size(res);



	if (gmux_data->iolen < GMUX_MIN_IO_LEN) {

		pr_err("gmux I/O region too small (%lu < %u)\n",

		       gmux_data->iolen, GMUX_MIN_IO_LEN);

		goto err_free;

	}



	if (!request_region(gmux_data->iostart, gmux_data->iolen,

			    "Apple gmux")) {

		pr_err("gmux I/O already in use\n");

		goto err_free;

	}



	/*

	 * Invalid version information may indicate either that the gmux

	 * device isn't present or that it's a new one that uses indexed

	 * io

	 */



	ver_major = gmux_read8(gmux_data, GMUX_PORT_VERSION_MAJOR);

	ver_minor = gmux_read8(gmux_data, GMUX_PORT_VERSION_MINOR);

	ver_release = gmux_read8(gmux_data, GMUX_PORT_VERSION_RELEASE);

	if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {

		if (gmux_is_indexed(gmux_data)) {

			u32 version;

	if (indexed) {

		mutex_init(&gmux_data->index_lock);

		gmux_data->indexed = true;

			version = gmux_read32(gmux_data,

				GMUX_PORT_VERSION_MAJOR);

		version = gmux_read32(gmux_data, GMUX_PORT_VERSION_MAJOR);

		ver_major = (version >> 24) & 0xff;

		ver_minor = (version >> 16) & 0xff;

		ver_release = (version >> 8) & 0xff;

	} else {

			pr_info("gmux device not present\n");

			ret = -ENODEV;

			goto err_release;

		}

		ver_major = gmux_read8(gmux_data, GMUX_PORT_VERSION_MAJOR);

		ver_minor = gmux_read8(gmux_data, GMUX_PORT_VERSION_MINOR);

		ver_release = gmux_read8(gmux_data, GMUX_PORT_VERSION_RELEASE);

	}

	pr_info("Found gmux version %d.%d.%d [%s]\n", ver_major, ver_minor,

		ver_release, (gmux_data->indexed ? "indexed" : "classic"));

include/linux/apple-gmux.h

+84 −2
Original line number Diff line number Diff line
@@ -8,6 +8,8 @@ 
#define LINUX_APPLE_GMUX_H



#include <linux/acpi.h>

#include <linux/io.h>

#include <linux/pnp.h>



#define GMUX_ACPI_HID "APP000B"
@@ -35,14 +37,89 @@ 
#define GMUX_MIN_IO_LEN			(GMUX_PORT_BRIGHTNESS + 4)



#if IS_ENABLED(CONFIG_APPLE_GMUX)

static inline bool apple_gmux_is_indexed(unsigned long iostart)

{

	u16 val;



	outb(0xaa, iostart + 0xcc);

	outb(0x55, iostart + 0xcd);

	outb(0x00, iostart + 0xce);



	val = inb(iostart + 0xcc) | (inb(iostart + 0xcd) << 8);

	if (val == 0x55aa)

		return true;



	return false;

}



/**

 * apple_gmux_present() - detect if gmux is built into the machine

 * apple_gmux_detect() - detect if gmux is built into the machine

 *

 * @pnp_dev:     Device to probe or NULL to use the first matching device

 * @indexed_ret: Returns (by reference) if the gmux is indexed or not

 *

 * Detect if a supported gmux device is present by actually probing it.

 * This avoids the false positives returned on some models by

 * apple_gmux_present().

 *

 * Return: %true if a supported gmux ACPI device is detected and the kernel

 * was configured with CONFIG_APPLE_GMUX, %false otherwise.

 */

static inline bool apple_gmux_detect(struct pnp_dev *pnp_dev, bool *indexed_ret)

{

	u8 ver_major, ver_minor, ver_release;

	struct device *dev = NULL;

	struct acpi_device *adev;

	struct resource *res;

	bool indexed = false;

	bool ret = false;



	if (!pnp_dev) {

		adev = acpi_dev_get_first_match_dev(GMUX_ACPI_HID, NULL, -1);

		if (!adev)

			return false;



		dev = get_device(acpi_get_first_physical_node(adev));

		acpi_dev_put(adev);

		if (!dev)

			return false;



		pnp_dev = to_pnp_dev(dev);

	}



	res = pnp_get_resource(pnp_dev, IORESOURCE_IO, 0);

	if (!res || resource_size(res) < GMUX_MIN_IO_LEN)

		goto out;



	/*

	 * Invalid version information may indicate either that the gmux

	 * device isn't present or that it's a new one that uses indexed io.

	 */

	ver_major = inb(res->start + GMUX_PORT_VERSION_MAJOR);

	ver_minor = inb(res->start + GMUX_PORT_VERSION_MINOR);

	ver_release = inb(res->start + GMUX_PORT_VERSION_RELEASE);

	if (ver_major == 0xff && ver_minor == 0xff && ver_release == 0xff) {

		indexed = apple_gmux_is_indexed(res->start);

		if (!indexed)

			goto out;

	}



	if (indexed_ret)

		*indexed_ret = indexed;



	ret = true;

out:

	put_device(dev);

	return ret;

}



/**

 * apple_gmux_present() - check if gmux ACPI device is present

 *

 * Drivers may use this to activate quirks specific to dual GPU MacBook Pros

 * and Mac Pros, e.g. for deferred probing, runtime pm and backlight.

 *

 * Return: %true if gmux is present and the kernel was configured

 * Return: %true if gmux ACPI device is present and the kernel was configured

 * with CONFIG_APPLE_GMUX, %false otherwise.

 */

static inline bool apple_gmux_present(void)
@@ -57,6 +134,11 @@ static inline bool apple_gmux_present(void) 
	return false;

}



static inline bool apple_gmux_detect(struct pnp_dev *pnp_dev, bool *indexed_ret)

{

	return false;

}



#endif /* !CONFIG_APPLE_GMUX */



#endif /* LINUX_APPLE_GMUX_H */