apple-gmux: Assign apple_gmux_data before registering
cover apple-gmux: Assign apple_gmux_data before registering
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(-)
Signature · 6 lines
1.8.5.2 (Apple Git-48) _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 1/1 apple-gmux: Assign apple_gmux_data before registering
From: Matthew Garrett <[email protected]>
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.
[MBP 5,3 2009 nvidia MCP79 + G96 pre-retina 15"]
[MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina 15"]
[MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina 15"]
[MBP 11,3 2013 intel HSW + nvidia GK107 retina 15"]
--- drivers/platform/x86/apple-gmux.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/platform/x86/apple-gmux.c +6 -4
@@ -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:
Signature · 6 lines
1.8.5.2 (Apple Git-48) _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
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,
Quoted 29 lines · Lukas Wunner
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 ofapple_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: Assignapple_gmux_databefore registeringdrivers/platform/x86/apple-gmux.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
Signature · 6 lines
Darren Hart Intel Open Source Technology Center _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 1/1 apple-gmux: Assign apple_gmux_data before registering
In reply to Lukas WunnerFrom: Matthew Garrett <[email protected]>
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.]
[MBP 5,3 2009 nvidia MCP79 + G96 pre-retina 15"]
[MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina 15"]
[MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina 15"]
[MBP 11,3 2013 intel HSW + nvidia GK107 retina 15"]
--- drivers/platform/x86/apple-gmux.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/platform/x86/apple-gmux.c +6 -4
@@ -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:
Signature · 1 line
1.8.5.2 (Apple Git-48)
Quoted 17 lines · Lukas Wunner
From: Matthew Garrett <[email protected]>
Registering the handler after both GPUs will trigger a DDC switch for connector reprobing. This will oops if
apple_gmux_datahasn'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 ofapple_gmux_data.]Tested-by Pierre Moreau <[email protected]>[MBP 5,3 2009 nvidia MCP79 + G96 pre-retina 15"]Tested-by Paul Hordiienko <[email protected]>[MBP 6,2 2010 intel ILK + nvidia GT216 pre-retina 15"]Tested-by Lukas Wunner <[email protected]>[MBP 9,1 2012 intel IVB + nvidia GK107 pre-retina 15"]
Quoted 8 lines
Tested-by William Brown <[email protected]>[MBP 8,2 2011 intel SNB + amd turks pre-retina 15"]Tested-by Bruno Bierbaumer <[email protected]>[MBP 11,3 2013 intel HSW + nvidia GK107 retina 15"]Signed-off-by Matthew Garrett <[email protected]>Reviewed-by Lukas Wunner <[email protected]>Signed-off-by Lukas Wunner <[email protected]>
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.
Signature · 6 lines
Darren Hart Intel Open Source Technology Center _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel