Hi Am 28.02.20 um 09:40 schrieb Daniel Vetter: > On Fri, Feb 28, 2020 at 8:44 AM Thomas Zimmermann wrote: >> >> Hi Daniel >> >> Am 27.02.20 um 19:15 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. >> >> The disconnect hook calls drm_kms_helper_poll_disable() instead if >> _fini(). They are the same, except that _disable() does not clear >> dev->mode_config.poll_enabled to false. Is this OK? > > oops, I overlooked that. But yeah for driver shutdown it's the same > really, we're not going to re-enable. _disable is meant for suspend so > youc an re-enable again on resume. > > I'll augment the commit message on the next round to clarify that. Well, we have a managed API. It could support drmm_kms_helper_poll_init(). :) Best regards Thomas > -Daniel > > >> Best regards >> Thomas >> >>> >>> 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. >>> >>> Signed-off-by: Daniel Vetter >>> Cc: Dave Airlie >>> Cc: Sean Paul >>> Cc: Daniel Vetter >>> Cc: Thomas Zimmermann >>> Cc: Emil Velikov >>> Cc: Gerd Hoffmann >>> Cc: "Noralf Trønnes" >>> Cc: Sam Ravnborg >>> Cc: Thomas Gleixner >>> Cc: Alex Deucher >>> --- >>> drivers/gpu/drm/udl/udl_drv.c | 6 ------ >>> drivers/gpu/drm/udl/udl_drv.h | 1 - >>> drivers/gpu/drm/udl/udl_main.c | 10 ---------- >>> 3 files changed, 17 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c >>> index b447fb053e78..7f140898df3e 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, >>> 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); >>> -} >>> >> >> -- >> Thomas Zimmermann >> Graphics Driver Developer >> SUSE Software Solutions Germany GmbH >> Maxfeldstr. 5, 90409 Nürnberg, Germany >> (HRB 36809, AG Nürnberg) >> Geschäftsführer: Felix Imendörffer >> > > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer