search for: nouveau_drm_unload

In preparation for runtime pm on muxed dual GPU laptops, I've fixed all runtime pm ref leaks I could find in nouveau, radeon and amdgpu. To ease reviewing, I've pushed this series to GitHub: https://github.com/l1k/linux/commits/drm_runpm_fixes_v1 @Alex Deucher: I do not have an AMD GPU so couldn't test this beyond verifying that it compiles. Please double-check the patches and test

...; fail_dispinit: @@ -351,12 +370,20 @@ fail_ttm: nouveau_vga_fini(drm); fail_device: nouveau_cli_destroy(&drm->client); +fail_cli: + pci_write_config_byte(pdev, NV_PCI_VGAMEM_ENABLE, 1); + + console_lock(); + do_unblank_screen(1); + console_unlock(); + return ret; } static int nouveau_drm_unload(struct drm_device *dev) { + struct pci_dev *pdev = dev->pdev; struct nouveau_drm *drm = nouveau_drm(dev); nouveau_fbcon_fini(dev); @@ -375,6 +402,8 @@ nouveau_drm_unload(struct drm_device *dev) nouveau_vga_fini(drm); nouveau_cli_destroy(&drm->client); + + pci_write_config_byt...

...tvnv17.c:(.text.nouveau_do_suspend+0x10): undefined reference to `nouveau_led_suspend' drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_resume': tvnv17.c:(.text.nouveau_do_resume+0xf0): undefined reference to `nouveau_led_resume' drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_unload': tvnv17.c:(.text.nouveau_drm_unload+0x34): undefined reference to `nouveau_led_fini' drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_load': tvnv17.c:(.text.nouveau_drm_load+0x7d0): undefined reference to `nouveau_led_init' This adds a separate Kconfig symbol for the LE...

On Tue, May 24, 2016 at 06:03:27PM +0200, Lukas Wunner wrote: > nouveau_drm_load() calls pm_runtime_put() if nouveau_runtime_pm != 0, > but nouveau_drm_unload() calls pm_runtime_get_sync() unconditionally. > We therefore leak a runtime pm ref whenever nouveau is loaded with > runpm=0 and then unloaded. The GPU will subsequently never runtime > suspend even if nouveau is loaded again with runpm=1. > > Fix by taking the runtime pm ref under...

...); +} +static void +nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv) +{ + struct nouveau_cli *cli = nouveau_cli(fpriv); nouveau_cli_fini(cli); kfree(cli); pm_runtime_mark_last_busy(dev->dev); @@ -964,6 +969,7 @@ driver_stub = { .load = nouveau_drm_load, .unload = nouveau_drm_unload, .open = nouveau_drm_open, + .preclose = nouveau_drm_preclose, .postclose = nouveau_drm_postclose, .lastclose = nouveau_vga_lastclose, -- 2.13.2

...eturn ret; - } - - return 0; -} - static int -nouveau_drm_load(struct drm_device *dev, unsigned long flags) +nouveau_drm_device_init(struct drm_device *dev) { struct nouveau_drm *drm; int ret; @@ -613,7 +546,7 @@ nouveau_drm_load(struct drm_device *dev, unsigned long flags) } static void -nouveau_drm_unload(struct drm_device *dev) +nouveau_drm_device_fini(struct drm_device *dev) { struct nouveau_drm *drm = nouveau_drm(dev); @@ -642,18 +575,116 @@ nouveau_drm_unload(struct drm_device *dev) kfree(drm); } +static int nouveau_drm_probe(struct pci_dev *pdev, + const struct pci_device_id *p...

nouveau_drm_load() calls pm_runtime_put() if nouveau_runtime_pm != 0, but nouveau_drm_unload() calls pm_runtime_get_sync() unconditionally. We therefore leak a runtime pm ref whenever nouveau is loaded with runpm=0 and then unloaded. The GPU will subsequently never runtime suspend even if nouveau is loaded again with runpm=1. Fix by taking the runtime pm ref under the same condition that...

...vice is put into sleep state. --- drm/nouveau/nouveau_drm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drm/nouveau/nouveau_drm.c b/drm/nouveau/nouveau_drm.c index 5b1746b..2aaf9fe 100644 --- a/drm/nouveau/nouveau_drm.c +++ b/drm/nouveau/nouveau_drm.c @@ -481,7 +481,8 @@ nouveau_drm_unload(struct drm_device *dev) { struct nouveau_drm *drm = nouveau_drm(dev); - pm_runtime_get_sync(dev->dev); + if (atomic_read(&dev->dev->power.usage_count) == 0) + pm_runtime_get_sync(dev->dev); nouveau_fbcon_fini(dev); nouveau_accel_fini(drm); nouveau_hwmon_fini(dev); -- 2....

....nouveau_do_suspend+0x10): undefined reference to `nouveau_led_suspend' > drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_resume': > tvnv17.c:(.text.nouveau_do_resume+0xf0): undefined reference to `nouveau_led_resume' > drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_unload': > tvnv17.c:(.text.nouveau_drm_unload+0x34): undefined reference to `nouveau_led_fini' > drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_load': > tvnv17.c:(.text.nouveau_drm_load+0x7d0): undefined reference to `nouveau_led_init' > > This adds a separate K...

...-} - -static void -nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv) -{ - struct nouveau_cli *cli = nouveau_cli(fpriv); nouveau_cli_fini(cli); kfree(cli); pm_runtime_mark_last_busy(dev->dev); @@ -974,7 +968,6 @@ driver_stub = { .load = nouveau_drm_load, .unload = nouveau_drm_unload, .open = nouveau_drm_open, - .preclose = nouveau_drm_preclose, .postclose = nouveau_drm_postclose, .lastclose = nouveau_vga_lastclose, -- 2.11.0

Hi Peter, On Fri, May 27, 2016 at 03:07:33AM +0200, Peter Wu wrote: > On Tue, May 24, 2016 at 06:03:27PM +0200, Lukas Wunner wrote: > > nouveau_drm_load() calls pm_runtime_put() if nouveau_runtime_pm != 0, > > but nouveau_drm_unload() calls pm_runtime_get_sync() unconditionally. > > We therefore leak a runtime pm ref whenever nouveau is loaded with > > runpm=0 and then unloaded. The GPU will subsequently never runtime > > suspend even if nouveau is loaded again with runpm=1. > > > > Fix by taking...

...-} - -static void -nouveau_drm_postclose(struct drm_device *dev, struct drm_file *fpriv) -{ - struct nouveau_cli *cli = nouveau_cli(fpriv); nouveau_cli_fini(cli); kfree(cli); pm_runtime_mark_last_busy(dev->dev); @@ -974,7 +968,6 @@ driver_stub = { .load = nouveau_drm_load, .unload = nouveau_drm_unload, .open = nouveau_drm_open, - .preclose = nouveau_drm_preclose, .postclose = nouveau_drm_postclose, .lastclose = nouveau_vga_lastclose, -- 2.11.0

...veau_drm_load(struct drm_device *dev, unsigned long flags) > +nouveau_drm_device_init(struct drm_device *dev) > { > struct nouveau_drm *drm; > int ret; > @@ -613,7 +546,7 @@ nouveau_drm_load(struct drm_device *dev, unsigned long > flags) > } > > static void > -nouveau_drm_unload(struct drm_device *dev) > +nouveau_drm_device_fini(struct drm_device *dev) > { > struct nouveau_drm *drm = nouveau_drm(dev); > > @@ -642,18 +575,116 @@ nouveau_drm_unload(struct drm_device *dev) > kfree(drm); > } > > +static int nouveau_drm_probe(struct pci_dev...

...NING: CPU: 2 PID: 1434 at fs/sysfs/group.c:235 sysfs_remove_group+0x76/0x80 RIP: 0010:sysfs_remove_group+0x76/0x80 Call Trace: device_del+0x56/0x350 ? down_write+0xe/0x40 device_unregister+0x16/0x60 nouveau_backlight_exit+0x4a/0x60 [nouveau] nouveau_display_destroy+0x29/0x80 [nouveau] nouveau_drm_unload+0x61/0xd0 [nouveau] drm_dev_unregister+0x3f/0xe0 [drm] drm_put_dev+0x27/0x50 [drm] nouveau_drm_device_remove+0x47/0x70 [nouveau] pci_device_remove+0x3b/0xb0 device_release_driver_internal+0x180/0x250 driver_detach+0x32/0x5f bus_remove_driver+0x74/0xc6 pci_unregister_driver+0x22/0xa0...

...sibly) execute vbios init tables (see nouveau_agp.h) */ @@ -400,6 +427,9 @@ fail_ttm: nouveau_agp_fini(drm); nouveau_vga_fini(drm); fail_device: + if (drm->hdmi_device) + pci_dev_put(drm->hdmi_device); + nouveau_cli_destroy(&drm->client); return ret; } @@ -424,6 +454,8 @@ nouveau_drm_unload(struct drm_device *dev) nouveau_agp_fini(drm); nouveau_vga_fini(drm); + if (drm->hdmi_device) + pci_dev_put(drm->hdmi_device); nouveau_cli_destroy(&drm->client); return 0; } diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.h b/drivers/gpu/drm/nouveau/nouveau_drm.h index 41f...

...x80 >> RIP: 0010:sysfs_remove_group+0x76/0x80 >> Call Trace: >> device_del+0x56/0x350 >> ? down_write+0xe/0x40 >> device_unregister+0x16/0x60 >> nouveau_backlight_exit+0x4a/0x60 [nouveau] >> nouveau_display_destroy+0x29/0x80 [nouveau] >> nouveau_drm_unload+0x61/0xd0 [nouveau] >> drm_dev_unregister+0x3f/0xe0 [drm] >> drm_put_dev+0x27/0x50 [drm] >> nouveau_drm_device_remove+0x47/0x70 [nouveau] >> pci_device_remove+0x3b/0xb0 >> device_release_driver_internal+0x180/0x250 >> driver_detach+0x32/0x5f >>...

...signed long flags) >> +nouveau_drm_device_init(struct drm_device *dev) >> { >> struct nouveau_drm *drm; >> int ret; >> @@ -613,7 +546,7 @@ nouveau_drm_load(struct drm_device *dev, unsigned long >> flags) >> } >> >> static void >> -nouveau_drm_unload(struct drm_device *dev) >> +nouveau_drm_device_fini(struct drm_device *dev) >> { >> struct nouveau_drm *drm = nouveau_drm(dev); >> >> @@ -642,18 +575,116 @@ nouveau_drm_unload(struct drm_device *dev) >> kfree(drm); >> } >> >> +static i...

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

Second iteration of my endeavour to rid nouveau, radeon and amdgpu of runtime pm ref leaks. Patches 1 to 8 are identical to v1. Patch 9 of v1 modified the DRM core to turn off all CRTCs on driver unload. Based on feedback by Daniel Vetter, I've replaced this with a helper to turn off all CRTCs, which is called by nouveau, radeon and amdgpu on unload. In other words, this is now opt-in. So

https://bugs.freedesktop.org/show_bug.cgi?id=74195 Priority: medium Bug ID: 74195 Assignee: nouveau at lists.freedesktop.org Summary: Responsivity to input with vsync on is slower after update QA Contact: xorg-team at lists.x.org Severity: normal Classification: Unclassified OS: Linux (All)