[PATCH V2 1/2] gmux: Add generic write32 function

[PATCH V2 1/2] gmux: Add generic write32 function

[Matthew Garrett]

Move the special-cased backlight update function to a generic gmux_write32
function.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
Cc: Seth Forshee <seth.forshee@canonical.com>
---

Fixed the typo and irrelevant hunk

 drivers/platform/x86/apple-gmux.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index 905fa01..c9db5072 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -75,6 +75,18 @@ static inline u32 gmux_read32(struct apple_gmux_data *gmux_data, int port)
 	return inl(gmux_data->iostart + port);
 }
 
+static inline u32 gmux_write32(struct apple_gmux_data *gmux_data, int port,
+			       u32 val)
+{
+	int i;
+	u8 tmpval;
+
+	for (i = 0; i < 4; i++) {
+		tmpval = (val >> (i * 8)) & 0xff;
+		outb(tmpval, port + i);
+	}
+}
+
 static int gmux_get_brightness(struct backlight_device *bd)
 {
 	struct apple_gmux_data *gmux_data = bl_get_data(bd);
@@ -90,16 +102,7 @@ static int gmux_update_status(struct backlight_device *bd)
 	if (bd->props.state & BL_CORE_SUSPENDED)
 		return 0;
 
-	/*
-	 * Older gmux versions require writing out lower bytes first then
-	 * setting the upper byte to 0 to flush the values. Newer versions
-	 * accept a single u32 write, but the old method also works, so we
-	 * just use the old method for all gmux versions.
-	 */
-	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS, brightness);
-	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 1, brightness >> 8);
-	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 2, brightness >> 16);
-	gmux_write8(gmux_data, GMUX_PORT_BRIGHTNESS + 3, 0);
+	gmux_write32(gmux_data, GMUX_PORT_BRIGHTNESS, brightness);
 
 	return 0;
 }
-- 
1.7.11.2

[PATCH V2 2/2] apple_gmux: Add support for newer hardware

[Matthew Garrett]

New gmux devices have a different method for accessing the registers.
Update the driver to cope. Incorporates feedback from Bernhard Froemel.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
Cc: Bernhard Froemel <froemel@vmars.tuwien.ac.at>
Cc: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/platform/x86/apple-gmux.c | 179 +++++++++++++++++++++++++++++++++++---
 1 file changed, 165 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c
index c9db5072..612b6f6 100644
--- a/drivers/platform/x86/apple-gmux.c
+++ b/drivers/platform/x86/apple-gmux.c
@@ -18,12 +18,15 @@
 #include <linux/pnp.h>
 #include <linux/apple_bl.h>
 #include <linux/slab.h>
+#include <linux/delay.h>
 #include <acpi/video.h>
 #include <asm/io.h>
 
 struct apple_gmux_data {
 	unsigned long iostart;
 	unsigned long iolen;
+	bool indexed;
+	struct mutex index_lock;
 
 	struct backlight_device *bdev;
 };
@@ -45,6 +48,9 @@ struct apple_gmux_data {
 #define GMUX_PORT_DISCRETE_POWER	0x50
 #define GMUX_PORT_MAX_BRIGHTNESS	0x70
 #define GMUX_PORT_BRIGHTNESS		0x74
+#define GMUX_PORT_VALUE			0xc2
+#define GMUX_PORT_READ			0xd0
+#define GMUX_PORT_WRITE			0xd4
 
 #define GMUX_MIN_IO_LEN			(GMUX_PORT_BRIGHTNESS + 4)
 
@@ -59,24 +65,24 @@ struct apple_gmux_data {
 #define GMUX_BRIGHTNESS_MASK		0x00ffffff
 #define GMUX_MAX_BRIGHTNESS		GMUX_BRIGHTNESS_MASK
 
-static inline u8 gmux_read8(struct apple_gmux_data *gmux_data, int port)
+static u8 gmux_pio_read8(struct apple_gmux_data *gmux_data, int port)
 {
 	return inb(gmux_data->iostart + port);
 }
 
-static inline void gmux_write8(struct apple_gmux_data *gmux_data, int port,
+static void gmux_pio_write8(struct apple_gmux_data *gmux_data, int port,
 			       u8 val)
 {
 	outb(val, gmux_data->iostart + port);
 }
 
-static inline u32 gmux_read32(struct apple_gmux_data *gmux_data, int port)
+static u32 gmux_pio_read32(struct apple_gmux_data *gmux_data, int port)
 {
 	return inl(gmux_data->iostart + port);
 }
 
-static inline u32 gmux_write32(struct apple_gmux_data *gmux_data, int port,
-			       u32 val)
+static void gmux_pio_write32(struct apple_gmux_data *gmux_data, int port,
+			     u32 val)
 {
 	int i;
 	u8 tmpval;
@@ -87,6 +93,144 @@ static inline u32 gmux_write32(struct apple_gmux_data *gmux_data, int port,
 	}
 }
 
+static int gmux_index_wait_ready(struct apple_gmux_data *gmux_data)
+{
+	int i = 200;
+	u8 gwr = inb(gmux_data->iostart + GMUX_PORT_WRITE);
+
+	while (i && (gwr & 0x01)) {
+		inb(gmux_data->iostart + GMUX_PORT_READ);
+		gwr = inb(gmux_data->iostart + GMUX_PORT_WRITE);
+		msleep(100);
+		i--;
+	}
+
+	return !!i;
+}
+
+static int gmux_index_wait_complete(struct apple_gmux_data *gmux_data)
+{
+	int i = 200;
+	u8 gwr = inb(gmux_data->iostart + GMUX_PORT_WRITE);
+
+	while (i && (gwr & 0x01)) {
+		gwr = inb(gmux_data->iostart + GMUX_PORT_WRITE);
+		msleep(100);
+		i--;
+	}
+
+	if (gwr & 0x01)
+		inb(gmux_data->iostart + GMUX_PORT_READ);
+
+	return !!i;
+}
+
+static u8 gmux_index_read8(struct apple_gmux_data *gmux_data, int port)
+{
+	u8 val;
+
+	mutex_lock(&gmux_data->index_lock);
+	outb((port & 0xff), gmux_data->iostart + GMUX_PORT_READ);
+	gmux_index_wait_ready(gmux_data);
+	val = inb(gmux_data->iostart + GMUX_PORT_VALUE);
+	mutex_unlock(&gmux_data->index_lock);
+
+	return val;
+}
+
+static void gmux_index_write8(struct apple_gmux_data *gmux_data, int port,
+			      u8 val)
+{
+	mutex_lock(&gmux_data->index_lock);
+	outb(val, gmux_data->iostart + GMUX_PORT_VALUE);
+	gmux_index_wait_ready(gmux_data);
+	outb(port & 0xff, gmux_data->iostart + GMUX_PORT_WRITE);
+	gmux_index_wait_complete(gmux_data);
+	mutex_unlock(&gmux_data->index_lock);
+}
+
+static u32 gmux_index_read32(struct apple_gmux_data *gmux_data, int port)
+{
+	u32 val;
+
+	mutex_lock(&gmux_data->index_lock);
+	outb((port & 0xff), gmux_data->iostart + GMUX_PORT_READ);
+	gmux_index_wait_ready(gmux_data);
+	val = inl(gmux_data->iostart + GMUX_PORT_VALUE);
+	mutex_unlock(&gmux_data->index_lock);
+
+	return val;
+}
+
+static void gmux_index_write32(struct apple_gmux_data *gmux_data, int port,
+			       u32 val)
+{
+	int i;
+	u8 tmpval;
+
+	mutex_lock(&gmux_data->index_lock);
+
+	for (i = 0; i < 4; i++) {
+		tmpval = (val >> (i * 8)) & 0xff;
+		outb(tmpval, gmux_data->iostart + GMUX_PORT_VALUE + i);
+	}
+
+	gmux_index_wait_ready(gmux_data);
+	outb(port & 0xff, gmux_data->iostart + GMUX_PORT_WRITE);
+	gmux_index_wait_complete(gmux_data);
+	mutex_unlock(&gmux_data->index_lock);
+}
+
+static u8 gmux_read8(struct apple_gmux_data *gmux_data, int port)
+{
+	if (gmux_data->indexed)
+		return gmux_index_read8(gmux_data, port);
+	else
+		return gmux_pio_read8(gmux_data, port);
+}
+
+static void gmux_write8(struct apple_gmux_data *gmux_data, int port, u8 val)
+{
+	if (gmux_data->indexed)
+		gmux_index_write8(gmux_data, port, val);
+	else
+		gmux_pio_write8(gmux_data, port, val);
+}
+
+static u32 gmux_read32(struct apple_gmux_data *gmux_data, int port)
+{
+	if (gmux_data->indexed)
+		return gmux_index_read32(gmux_data, port);
+	else
+		return gmux_pio_read32(gmux_data, port);
+}
+
+static void gmux_write32(struct apple_gmux_data *gmux_data, int port,
+			     u32 val)
+{
+	if (gmux_data->indexed)
+		gmux_index_write32(gmux_data, port, val);
+	else
+		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;
+}
+
 static int gmux_get_brightness(struct backlight_device *bd)
 {
 	struct apple_gmux_data *gmux_data = bl_get_data(bd);
@@ -150,22 +294,29 @@ static int __devinit gmux_probe(struct pnp_dev *pnp,
 	}
 
 	/*
-	 * On some machines the gmux is in ACPI even thought the machine
-	 * doesn't really have a gmux. Check for invalid version information
-	 * to detect this.
+	 * 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) {
-		pr_info("gmux device not present\n");
-		ret = -ENODEV;
-		goto err_release;
+		if (gmux_is_indexed(gmux_data)) {
+			mutex_init(&gmux_data->index_lock);
+			gmux_data->indexed = true;
+		} else {
+			pr_info("gmux device not present\n");
+			ret = -ENODEV;
+			goto err_release;
+		}
+		pr_info("Found indexed gmux\n");
+	} else {
+		pr_info("Found gmux version %d.%d.%d\n", ver_major, ver_minor,
+			ver_release);
 	}
 
-	pr_info("Found gmux version %d.%d.%d\n", ver_major, ver_minor,
-		ver_release);
-
 	memset(&props, 0, sizeof(props));
 	props.type = BACKLIGHT_PLATFORM;
 	props.max_brightness = gmux_read32(gmux_data, GMUX_PORT_MAX_BRIGHTNESS);
-- 
1.7.11.2

Re: [PATCH V2 1/2] gmux: Add generic write32 function

[Seth Forshee]

On Mon, Aug 13, 2012 at 06:52:48PM -0400, Matthew Garrett wrote:
> Move the special-cased backlight update function to a generic gmux_write32
> function.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> Cc: Seth Forshee <seth.forshee@canonical.com>

Acked-by: Seth Forshee <seth.forshee@canonical.com>

Re: [PATCH V2 2/2] apple_gmux: Add support for newer hardware

[Seth Forshee]

On Mon, Aug 13, 2012 at 06:52:49PM -0400, Matthew Garrett wrote:
> New gmux devices have a different method for accessing the registers.
> Update the driver to cope. Incorporates feedback from Bernhard Froemel.
> 
> Signed-off-by: Matthew Garrett <mjg@redhat.com>
> Cc: Bernhard Froemel <froemel@vmars.tuwien.ac.at>
> Cc: Seth Forshee <seth.forshee@canonical.com>

Acked-by: Seth Forshee <seth.forshee@canonical.com>

Re: [PATCH V2 2/2] apple_gmux: Add support for newer hardware

[Bernhard Froemel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Please reduce the delay in the gmux_index_wait_[ready|complete]
functions: 100ms is way too long. 1 ms is more than enough. I never
experienced any problems with 100 us.

The version information can also be extracted in the new gmux device
(see my initial patch proposal).

Bernhard

On 08/14/2012 02:46 AM, Seth Forshee wrote:
> On Mon, Aug 13, 2012 at 06:52:49PM -0400, Matthew Garrett wrote:
>> New gmux devices have a different method for accessing the
>> registers. Update the driver to cope. Incorporates feedback from
>> Bernhard Froemel.
>> 
>> Signed-off-by: Matthew Garrett <mjg@redhat.com> Cc: Bernhard
>> Froemel <froemel@vmars.tuwien.ac.at> Cc: Seth Forshee
>> <seth.forshee@canonical.com>
> 
> Acked-by: Seth Forshee <seth.forshee@canonical.com>
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlAqWz8ACgkQ6iVUjPs37Jny4QCgvIj24lw/hpSF35P3k7vKcxtt
xj4AoNdN4u1QE4YErj771aP2ArZMDOS1
=TJHi
-----END PGP SIGNATURE-----

Re: [PATCH V2 2/2] apple_gmux: Add support for newer hardware

[Bernhard Froemel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Only in relation to Seth's future apple-gmux patch series about
vgaswitcheroo and restoring the gmux device configuration across
suspend/resume [1]:
On resume the gmux registers 0x28 (DDC) and 0x10 (SWITCH_DISPLAY)
(i.e., the first two writes) are not restored (result: the screen
remains black after suspend/resume).

Of course, this does not happen with my patch proposal ;)
Unfortunately, I have not the time to sort this out in the next couple
of days -- on the other hand this is probably not a problem w.r.t.
this patch, but needs to be taken care of in Seth's patch series.

Bernhard

[1] http://people.canonical.com/~sforshee/apple-gmux-patches/


On 08/14/2012 04:05 PM, Bernhard Froemel wrote:
> Please reduce the delay in the gmux_index_wait_[ready|complete] 
> functions: 100ms is way too long. 1 ms is more than enough. I
> never experienced any problems with 100 us.
> 
> The version information can also be extracted in the new gmux
> device (see my initial patch proposal).
> 
> Bernhard
> 
> On 08/14/2012 02:46 AM, Seth Forshee wrote:
>> On Mon, Aug 13, 2012 at 06:52:49PM -0400, Matthew Garrett wrote:
>>> New gmux devices have a different method for accessing the 
>>> registers. Update the driver to cope. Incorporates feedback
>>> from Bernhard Froemel.
>>> 
>>> Signed-off-by: Matthew Garrett <mjg@redhat.com> Cc: Bernhard 
>>> Froemel <froemel@vmars.tuwien.ac.at> Cc: Seth Forshee 
>>> <seth.forshee@canonical.com>
> 
>> Acked-by: Seth Forshee <seth.forshee@canonical.com>
> 
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlAqZnsACgkQ6iVUjPs37JlYfwCcCH17qDbQO2xqhlOsqjliVWA/
bm0AnAyeh7pprGvS8qdTAvB0c3RYJsFc
=pQxQ
-----END PGP SIGNATURE-----

Re: [PATCH V2 2/2] apple_gmux: Add support for newer hardware

[Seth Forshee]

On Tue, Aug 14, 2012 at 04:53:47PM +0200, Bernhard Froemel wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Only in relation to Seth's future apple-gmux patch series about
> vgaswitcheroo and restoring the gmux device configuration across
> suspend/resume [1]:
> On resume the gmux registers 0x28 (DDC) and 0x10 (SWITCH_DISPLAY)
> (i.e., the first two writes) are not restored (result: the screen
> remains black after suspend/resume).
> 
> Of course, this does not happen with my patch proposal ;)
> Unfortunately, I have not the time to sort this out in the next couple
> of days -- on the other hand this is probably not a problem w.r.t.
> this patch, but needs to be taken care of in Seth's patch series.

The patch adding support for switcheroo *does* restore those registers
on resume. Are you saying that isn't happening when you use it?

Seth

Re: [PATCH V2 2/2] apple_gmux: Add support for newer hardware

[Bernhard Froemel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/14/2012 05:15 PM, Seth Forshee wrote:
> On Tue, Aug 14, 2012 at 04:53:47PM +0200, Bernhard Froemel wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>> 
>> Only in relation to Seth's future apple-gmux patch series about 
>> vgaswitcheroo and restoring the gmux device configuration across 
>> suspend/resume [1]: On resume the gmux registers 0x28 (DDC) and
>> 0x10 (SWITCH_DISPLAY) (i.e., the first two writes) are not
>> restored (result: the screen remains black after
>> suspend/resume).
>> 
>> Of course, this does not happen with my patch proposal ;) 
>> Unfortunately, I have not the time to sort this out in the next
>> couple of days -- on the other hand this is probably not a
>> problem w.r.t. this patch, but needs to be taken care of in
>> Seth's patch series.
> 
> The patch adding support for switcheroo *does* restore those
> registers on resume. Are you saying that isn't happening when you
> use it?
Yes, the 8bit writes don't seem to get all through with Matthew's
patch, but they do with mine.

Just tested manual switching (/sys/kernel/debug/vgaswitcheroo/switch,
from IGD to DIS) -> doesn't work either; even locks up my system:

> [  123.680062] hda-intel: spurious response 0x407381:0x0, last
> cmd=0x770883 [  123.680070] hda-intel: spurious response
> 0x9000094:0x0, last cmd=0x770883 [  123.680073] hda-intel: spurious
> response 0x185600f0:0x0, last cmd=0x770883 [  123.680076]
> hda-intel: spurious response 0x4:0x0, last cmd=0x770883 [
> 123.680079] hda-intel: spurious response 0xb0a0908:0x0, last
> cmd=0x770883 [  123.680082] hda-intel: spurious response
> 0x407381:0x0, last cmd=0x770883 [  123.680084] hda-intel: spurious
> response 0x9000094:0x0, last cmd=0x770883 [  123.680087] hda-intel:
> spurious response 0x185600f0:0x0, last cmd=0x770883

While comparing our patches, I discovered that mine isnt' a good
reference here, because in my ready and complete function variants I
didn't write/read the correct ioports (forgot about the offset; so
they were only slightly delaying execution). After I fixed that (and a
status-bit semantic mixup), the results appear similar :/
Need to play around further..

Bernhard
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlAqhrcACgkQ6iVUjPs37JnhtwCePUp7sLQW+GhBFOkFbkjr6Yrh
VAsAoJ3TjtJu2gQpayFc6iP5nBQfdAPG
=ziXd
-----END PGP SIGNATURE-----

Re: [PATCH V2 2/2] apple_gmux: Add support for newer hardware

[Bernhard Froemel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 08/14/2012 07:11 PM, Bernhard Froemel wrote:
> Need to play around further..
I think I solved the communication problems concerning byte writes to
the gmux device.

This:
http://luna.vmars.tuwien.ac.at/~froemel/rmbp/patch-apple-gmux_v2.txt
works reliable for me (50+ suspend/resume cycles, many brightness
changes) *without* delays.

I looked once more through Apple's original driver and noticed that
their cmd_done waits until bit 1 is set (not cleared!) and also does
the final read from GMUX_PORT_DPM_RADDR only if bit 1 is not set.
Also it seems that in case of byte writes the old interface should be
followed as well (Apple, why?!).

Bernhard


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlArU9cACgkQ6iVUjPs37Jk6iACfWuZ7zpbc1vFLgJR29UroJeL2
HvMAnja8D7/o+aqywr/qRtNrB/o217Ci
=gcjo
-----END PGP SIGNATURE-----