| Message ID | 20191022163717.1064-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/doc: Drop misleading comment on drm_mode_config_cleanup | expand |
On Tuesday, 22 October 2019 17:37:17 BST Daniel Vetter wrote: > This is not something we'll fix, because failing to clean up stuff (or > doing it in the wrong order) is a driver bug. The offending FIXME goes > all the way back to the original modeset merge. > > We've added a WARN_ON in > > commit 2b677e8c08eed11e4ebe66a7c334f03e389a19a3 > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > Date: Mon Dec 10 21:16:05 2012 +0100 > > drm: reference framebuffers which are on the idr > > including a comment blaming drivers on this. Right thing to do is most > likely drm_atomic_helper_shutdown plus making sure that > drm_mode_config_cleanup is not called too early (i.e. not in driver > unload, but only in the final drm_device release callback). > > Cc: Mihail Atanassov <Mihail.Atanassov@arm.com> > Reported-by: Mihail Atanassov <Mihail.Atanassov@arm.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/drm_mode_config.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c > index 7bc03c3c154f..3b570a404933 100644 > --- a/drivers/gpu/drm/drm_mode_config.c > +++ b/drivers/gpu/drm/drm_mode_config.c > @@ -428,8 +428,6 @@ EXPORT_SYMBOL(drm_mode_config_init); > * Note that since this /should/ happen single-threaded at driver/device > * teardown time, no locking is required. It's the driver's job to ensure that > * this guarantee actually holds true. > - * > - * FIXME: cleanup any dangling user buffer objects too > */ > void drm_mode_config_cleanup(struct drm_device *dev) > { > Thanks, Reviewed-by: Mihail Atanassov <mihail.atanassov@arm.com>
On Tue, Oct 22, 2019 at 05:02:40PM +0000, Mihail Atanassov wrote: > On Tuesday, 22 October 2019 17:37:17 BST Daniel Vetter wrote: > > This is not something we'll fix, because failing to clean up stuff (or > > doing it in the wrong order) is a driver bug. The offending FIXME goes > > all the way back to the original modeset merge. > > > > We've added a WARN_ON in > > > > commit 2b677e8c08eed11e4ebe66a7c334f03e389a19a3 > > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > > Date: Mon Dec 10 21:16:05 2012 +0100 > > > > drm: reference framebuffers which are on the idr > > > > including a comment blaming drivers on this. Right thing to do is most > > likely drm_atomic_helper_shutdown plus making sure that > > drm_mode_config_cleanup is not called too early (i.e. not in driver > > unload, but only in the final drm_device release callback). > > > > Cc: Mihail Atanassov <Mihail.Atanassov@arm.com> > > Reported-by: Mihail Atanassov <Mihail.Atanassov@arm.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/drm_mode_config.c | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c > > index 7bc03c3c154f..3b570a404933 100644 > > --- a/drivers/gpu/drm/drm_mode_config.c > > +++ b/drivers/gpu/drm/drm_mode_config.c > > @@ -428,8 +428,6 @@ EXPORT_SYMBOL(drm_mode_config_init); > > * Note that since this /should/ happen single-threaded at driver/device > > * teardown time, no locking is required. It's the driver's job to ensure that > > * this guarantee actually holds true. > > - * > > - * FIXME: cleanup any dangling user buffer objects too > > */ > > void drm_mode_config_cleanup(struct drm_device *dev) > > { > > > > Thanks, > > Reviewed-by: Mihail Atanassov <mihail.atanassov@arm.com> Thanks for your review&report, patch applied. -Daniel
diff --git a/drivers/gpu/drm/drm_mode_config.c b/drivers/gpu/drm/drm_mode_config.c index 7bc03c3c154f..3b570a404933 100644 --- a/drivers/gpu/drm/drm_mode_config.c +++ b/drivers/gpu/drm/drm_mode_config.c @@ -428,8 +428,6 @@ EXPORT_SYMBOL(drm_mode_config_init); * Note that since this /should/ happen single-threaded at driver/device * teardown time, no locking is required. It's the driver's job to ensure that * this guarantee actually holds true. - * - * FIXME: cleanup any dangling user buffer objects too */ void drm_mode_config_cleanup(struct drm_device *dev) {
This is not something we'll fix, because failing to clean up stuff (or doing it in the wrong order) is a driver bug. The offending FIXME goes all the way back to the original modeset merge. We've added a WARN_ON in commit 2b677e8c08eed11e4ebe66a7c334f03e389a19a3 Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Mon Dec 10 21:16:05 2012 +0100 drm: reference framebuffers which are on the idr including a comment blaming drivers on this. Right thing to do is most likely drm_atomic_helper_shutdown plus making sure that drm_mode_config_cleanup is not called too early (i.e. not in driver unload, but only in the final drm_device release callback). Cc: Mihail Atanassov <Mihail.Atanassov@arm.com> Reported-by: Mihail Atanassov <Mihail.Atanassov@arm.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/drm_mode_config.c | 2 -- 1 file changed, 2 deletions(-)