Hi Darren,
the following patch is a useful fix for apple-gmux by Matthew Garrett which is well over a year old but unfortunately never got merged.
The commit message makes it sound as if the fix is only needed for reprobing (in case apple-gmux registers after the DRM drivers). I'm not yet sure if we'll use reprobing or deferred initialization, however the patch is needed even if we go with deferred initialization as it fixes a race condition that is triggered by invoking a handler callback between the call to vga_switcheroo_register_handler() and the assignment of apple_gmux_data.
The patch has seen extensive testing and is actively used by myself and others on various MacBook Pro models.
Could you take a look at the patch and (barring any objections) ack it?
Thanks,
Lukas
Matthew Garrett (1): apple-gmux: Assign apple_gmux_data before registering
drivers/platform/x86/apple-gmux.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
From: Matthew Garrett matthew.garrett@nebula.com
Registering the handler after both GPUs will trigger a DDC switch for connector reprobing. This will oops if apple_gmux_data hasn't already been assigned. Reorder the code to do that.
Tested-by: Pierre Moreau pierre.morrow@free.fr [MBP 5,3 2009 nvidia MCP79 + G96 pre-retina 15"] Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina 15"] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina 15"] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina 15"] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina 15"]
Signed-off-by: Matthew Garrett matthew.garrett@nebula.com Reviewed-by: Lukas Wunner lukas@wunner.de Signed-off-by: Lukas Wunner lukas@wunner.de --- drivers/platform/x86/apple-gmux.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c index 976efeb..aa58d41 100644 --- a/drivers/platform/x86/apple-gmux.c +++ b/drivers/platform/x86/apple-gmux.c @@ -588,18 +588,20 @@ static int gmux_probe(struct pnp_dev *pnp, const struct pnp_device_id *id) gmux_data->gpe = -1; }
+ apple_gmux_data = gmux_data; + init_completion(&gmux_data->powerchange_done); + gmux_enable_interrupts(gmux_data); + if (vga_switcheroo_register_handler(&gmux_handler)) { ret = -ENODEV; goto err_register_handler; }
- init_completion(&gmux_data->powerchange_done); - apple_gmux_data = gmux_data; - gmux_enable_interrupts(gmux_data); - return 0;
err_register_handler: + gmux_disable_interrupts(gmux_data); + apple_gmux_data = NULL; if (gmux_data->gpe >= 0) acpi_disable_gpe(NULL, gmux_data->gpe); err_enable_gpe:
On Mon, Nov 16, 2015 at 09:38:40PM +0100, Lukas Wunner wrote:
From: Matthew Garrett matthew.garrett@nebula.com
Registering the handler after both GPUs will trigger a DDC switch for connector reprobing. This will oops if apple_gmux_data hasn't already been assigned. Reorder the code to do that.
[Lukas: More generally, this commit fixes a race condition that is triggered by invoking a handler callback between the call to vga_switcheroo_register_handler() and the assignment of apple_gmux_data.]
Tested-by: Pierre Moreau pierre.morrow@free.fr [MBP 5,3 2009 nvidia MCP79 + G96 pre-retina 15"] Tested-by: Paul Hordiienko pvt.gord@gmail.com [MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina 15"] Tested-by: Lukas Wunner lukas@wunner.de [MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina 15"] Tested-by: William Brown william@blackhats.net.au [MBP 8,2 2011 intel SNB + amd turks pre-retina 15"] Tested-by: Bruno Bierbaumer bruno@bierbaumer.net [MBP 11,3 2013 intel HSW + nvidia GK107 retina 15"]
Signed-off-by: Matthew Garrett matthew.garrett@nebula.com Reviewed-by: Lukas Wunner lukas@wunner.de Signed-off-by: Lukas Wunner lukas@wunner.de
My apologies for the delay. Thank you for the testing data and submitting.
I have queued this to testing. Pending success on 0-day, it will land in linux-next shortly (tomorrow most likely) where I hope it will receive additional testing.
Hi Lukas,
Please send the patch to the list and maintainers reported by get_maintainer.pl (specifically the platform-driver-x86 list and my infradead ID) so we can have the review on list.
Thanks,
On 11/9/15 11:28 AM, Lukas Wunner wrote:
Hi Darren,
the following patch is a useful fix for apple-gmux by Matthew Garrett which is well over a year old but unfortunately never got merged.
The commit message makes it sound as if the fix is only needed for reprobing (in case apple-gmux registers after the DRM drivers). I'm not yet sure if we'll use reprobing or deferred initialization, however the patch is needed even if we go with deferred initialization as it fixes a race condition that is triggered by invoking a handler callback between the call to vga_switcheroo_register_handler() and the assignment of apple_gmux_data.
The patch has seen extensive testing and is actively used by myself and others on various MacBook Pro models.
Could you take a look at the patch and (barring any objections) ack it?
Thanks,
Lukas
Matthew Garrett (1): apple-gmux: Assign apple_gmux_data before registering
drivers/platform/x86/apple-gmux.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
dri-devel@lists.freedesktop.org
-
Darren Hart -
Darren Hart -
Lukas Wunner