2012-08-13 22:53:07

by Matthew Garrett

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

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

Signed-off-by: Matthew Garrett <[email protected]>
Cc: Seth Forshee <[email protected]>
---

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


2012-08-13 22:53:09

by Matthew Garrett

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

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 <[email protected]>
Cc: Bernhard Froemel <[email protected]>
Cc: Seth Forshee <[email protected]>
---
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

2012-08-14 00:45:12

by Seth Forshee

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

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 <[email protected]>
> Cc: Seth Forshee <[email protected]>

Acked-by: Seth Forshee <[email protected]>

2012-08-14 00:46:38

by Seth Forshee

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

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 <[email protected]>
> Cc: Bernhard Froemel <[email protected]>
> Cc: Seth Forshee <[email protected]>

Acked-by: Seth Forshee <[email protected]>

2012-08-14 14:53:59

by Bernhard Froemel

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

-----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 <[email protected]> Cc: Bernhard
>>> Froemel <[email protected]> Cc: Seth Forshee
>>> <[email protected]>
>
>> Acked-by: Seth Forshee <[email protected]>
>
>
>

-----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-----

2012-08-14 15:01:31

by Bernhard Froemel

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

-----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 <[email protected]> Cc: Bernhard
>> Froemel <[email protected]> Cc: Seth Forshee
>> <[email protected]>
>
> Acked-by: Seth Forshee <[email protected]>
>

-----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-----

2012-08-14 15:16:03

by Seth Forshee

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

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

2012-08-14 17:11:30

by Bernhard Froemel

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

-----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-----

2012-08-15 07:46:43

by Bernhard Froemel

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

-----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-----