| Message ID | 20200302222631.3861340-51-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm_device managed resources, v4 | expand |
Hi Am 02.03.20 um 23:26 schrieb Daniel Vetter: > There's only two functions called from that: > drm_kms_helper_poll_fini() and udl_free_urb_list(). Both of these are > also called from the ubs_driver->disconnect hook, so entirely > pointless to do the same again in the ->release hook. > > Furthermore by the time we clean up the drm_driver we really shouldn't > be touching hardware anymore, so stopping the poll worker and freeing > the urb allocations in ->disconnect is the right thing to do. > > Now disconnect still cleans things up before unregistering the driver, > but that's a different issue. > > v2: Use _fini, not _disable in unplug, motivated by discussions with > Thomas. _disable/_enable are for suspend/resume. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Dave Airlie <airlied@redhat.com> > Cc: Sean Paul <sean@poorly.run> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: Emil Velikov <emil.l.velikov@gmail.com> > Cc: Gerd Hoffmann <kraxel@redhat.com> > Cc: "Noralf Trønnes" <noralf@tronnes.org> > Cc: Sam Ravnborg <sam@ravnborg.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/udl/udl_drv.c | 8 +------- > drivers/gpu/drm/udl/udl_drv.h | 1 - > drivers/gpu/drm/udl/udl_main.c | 10 ---------- > 3 files changed, 1 insertion(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c > index b447fb053e78..1ce2d865c36d 100644 > --- a/drivers/gpu/drm/udl/udl_drv.c > +++ b/drivers/gpu/drm/udl/udl_drv.c > @@ -34,14 +34,8 @@ static int udl_usb_resume(struct usb_interface *interface) > > DEFINE_DRM_GEM_FOPS(udl_driver_fops); > > -static void udl_driver_release(struct drm_device *dev) > -{ > - udl_fini(dev); > -} > - > static struct drm_driver driver = { > .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET, > - .release = udl_driver_release, > > /* gem hooks */ > .gem_create_object = udl_driver_gem_create_object, > @@ -120,7 +114,7 @@ static void udl_usb_disconnect(struct usb_interface *interface) > { > struct drm_device *dev = usb_get_intfdata(interface); > > - drm_kms_helper_poll_disable(dev); > + drm_kms_helper_poll_fini(dev); > udl_drop_usb(dev); > drm_dev_unplug(dev); > drm_dev_put(dev); > diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h > index 1de7eb1b6aac..2642f94a63fc 100644 > --- a/drivers/gpu/drm/udl/udl_drv.h > +++ b/drivers/gpu/drm/udl/udl_drv.h > @@ -76,7 +76,6 @@ int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len); > void udl_urb_completion(struct urb *urb); > > int udl_init(struct udl_device *udl); > -void udl_fini(struct drm_device *dev); > > int udl_render_hline(struct drm_device *dev, int log_bpp, struct urb **urb_ptr, > const char *front, char **urb_buf_ptr, > diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c > index 538718919916..f5d27f2a5654 100644 > --- a/drivers/gpu/drm/udl/udl_main.c > +++ b/drivers/gpu/drm/udl/udl_main.c > @@ -351,13 +351,3 @@ int udl_drop_usb(struct drm_device *dev) > udl_free_urb_list(dev); > return 0; > } > - > -void udl_fini(struct drm_device *dev) > -{ > - struct udl_device *udl = to_udl(dev); > - > - drm_kms_helper_poll_fini(dev); > - > - if (udl->urbs.count) > - udl_free_urb_list(dev); > -} >
diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c index b447fb053e78..1ce2d865c36d 100644 --- a/drivers/gpu/drm/udl/udl_drv.c +++ b/drivers/gpu/drm/udl/udl_drv.c @@ -34,14 +34,8 @@ static int udl_usb_resume(struct usb_interface *interface) DEFINE_DRM_GEM_FOPS(udl_driver_fops); -static void udl_driver_release(struct drm_device *dev) -{ - udl_fini(dev); -} - static struct drm_driver driver = { .driver_features = DRIVER_ATOMIC | DRIVER_GEM | DRIVER_MODESET, - .release = udl_driver_release, /* gem hooks */ .gem_create_object = udl_driver_gem_create_object, @@ -120,7 +114,7 @@ static void udl_usb_disconnect(struct usb_interface *interface) { struct drm_device *dev = usb_get_intfdata(interface); - drm_kms_helper_poll_disable(dev); + drm_kms_helper_poll_fini(dev); udl_drop_usb(dev); drm_dev_unplug(dev); drm_dev_put(dev); diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h index 1de7eb1b6aac..2642f94a63fc 100644 --- a/drivers/gpu/drm/udl/udl_drv.h +++ b/drivers/gpu/drm/udl/udl_drv.h @@ -76,7 +76,6 @@ int udl_submit_urb(struct drm_device *dev, struct urb *urb, size_t len); void udl_urb_completion(struct urb *urb); int udl_init(struct udl_device *udl); -void udl_fini(struct drm_device *dev); int udl_render_hline(struct drm_device *dev, int log_bpp, struct urb **urb_ptr, const char *front, char **urb_buf_ptr, diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index 538718919916..f5d27f2a5654 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -351,13 +351,3 @@ int udl_drop_usb(struct drm_device *dev) udl_free_urb_list(dev); return 0; } - -void udl_fini(struct drm_device *dev) -{ - struct udl_device *udl = to_udl(dev); - - drm_kms_helper_poll_fini(dev); - - if (udl->urbs.count) - udl_free_urb_list(dev); -}
There's only two functions called from that: drm_kms_helper_poll_fini() and udl_free_urb_list(). Both of these are also called from the ubs_driver->disconnect hook, so entirely pointless to do the same again in the ->release hook. Furthermore by the time we clean up the drm_driver we really shouldn't be touching hardware anymore, so stopping the poll worker and freeing the urb allocations in ->disconnect is the right thing to do. Now disconnect still cleans things up before unregistering the driver, but that's a different issue. v2: Use _fini, not _disable in unplug, motivated by discussions with Thomas. _disable/_enable are for suspend/resume. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Cc: Dave Airlie <airlied@redhat.com> Cc: Sean Paul <sean@poorly.run> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: Emil Velikov <emil.l.velikov@gmail.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: "Noralf Trønnes" <noralf@tronnes.org> Cc: Sam Ravnborg <sam@ravnborg.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/udl/udl_drv.c | 8 +------- drivers/gpu/drm/udl/udl_drv.h | 1 - drivers/gpu/drm/udl/udl_main.c | 10 ---------- 3 files changed, 1 insertion(+), 18 deletions(-)