Re: [PATCH] drm/nouveau/bl: Fix oops on driver unbind

[Pierre Moreau <pierre.morrow-GANU6spQydw@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2806 bytes --]

My bad; I did test suspend/resume, but not unbinding. Shouldn’t that
happen on shutdown as well, as the driver gets unloaded? I don’t remember
seeing that issue there though.

On 2018-02-17 — 13:40, Lukas Wunner wrote:
> Unbinding nouveau on a dual GPU MacBook Pro oopses because we iterate
> over the bl_connectors list in nouveau_backlight_exit() but skipped
> initializing it in nouveau_backlight_init().  Stacktrace for posterity:
> 
>     BUG: unable to handle kernel NULL pointer dereference at 0000000000000010
>     IP: nouveau_backlight_exit+0x2b/0x70 [nouveau]
>     nouveau_display_destroy+0x29/0x80 [nouveau]
>     nouveau_drm_unload+0x65/0xe0 [nouveau]
>     drm_dev_unregister+0x3c/0xe0 [drm]
>     drm_put_dev+0x2e/0x60 [drm]
>     nouveau_drm_device_remove+0x47/0x70 [nouveau]
>     pci_device_remove+0x36/0xb0
>     device_release_driver_internal+0x157/0x220
>     driver_detach+0x39/0x70
>     bus_remove_driver+0x51/0xd0
>     pci_unregister_driver+0x2a/0xa0
>     nouveau_drm_exit+0x15/0xfb0 [nouveau]
>     SyS_delete_module+0x18c/0x290
>     system_call_fast_compare_end+0xc/0x6f
> 
> Fixes: b53ac1ee12a3 ("drm/nouveau/bl: Do not register interface if Apple GMUX detected")
> Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org # v4.10+
> Cc: Pierre Moreau <pierre.morrow-GANU6spQydw@public.gmane.org>
> Signed-off-by: Lukas Wunner <lukas-JFq808J9C/izQB+pC5nmwQ@public.gmane.org>
> ---
> I reviewed the patch causing the oops but unfortunately missed this, sorry!
> 
>  drivers/gpu/drm/nouveau/nouveau_backlight.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/nouveau_backlight.c b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> index 380f340204e8..f56f60f695e1 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_backlight.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_backlight.c
> @@ -268,13 +268,13 @@ nouveau_backlight_init(struct drm_device *dev)
>  	struct nvif_device *device = &drm->client.device;
>  	struct drm_connector *connector;
>  
> +	INIT_LIST_HEAD(&drm->bl_connectors);
> +

We could instead have an early return in the exit function if
`apple_gmux_present()` or `drm->bl_connectors` is null, but your current fix
seems better.

Reviewed-by: Pierre Moreau <pierre.morrow-GANU6spQydw@public.gmane.org>

Thank you for the fix!
Pierre

>  	if (apple_gmux_present()) {
>  		NV_INFO(drm, "Apple GMUX detected: not registering Nouveau backlight interface\n");
>  		return 0;
>  	}
>  
> -	INIT_LIST_HEAD(&drm->bl_connectors);
> -
>  	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>  		if (connector->connector_type != DRM_MODE_CONNECTOR_LVDS &&
>  		    connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> -- 
> 2.15.1
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau