| Message ID | 20180702135433.7960-6-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Jul 02, 2018 at 03:54:29PM +0200, Noralf Trønnes wrote: > Add client callbacks and hook them up. > Add a list of clients per drm_device. > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> btw for reviewing it'd be simpler if you merge the 2 patches that add the client library, avoids me having to jump back&forth .. Bunch of comments below still. -Daniel > --- > drivers/gpu/drm/drm_client.c | 92 ++++++++++++++++++++++++++++++++++++- > drivers/gpu/drm/drm_drv.c | 7 +++ > drivers/gpu/drm/drm_fb_cma_helper.c | 2 +- > drivers/gpu/drm/drm_fb_helper.c | 11 ++++- > drivers/gpu/drm/drm_file.c | 3 ++ > drivers/gpu/drm/drm_probe_helper.c | 3 ++ > include/drm/drm_client.h | 75 +++++++++++++++++++++++++++++- > include/drm/drm_device.h | 14 ++++++ > 8 files changed, 201 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c > index 9c9b8ac7aea3..f05ee98bd10c 100644 > --- a/drivers/gpu/drm/drm_client.c > +++ b/drivers/gpu/drm/drm_client.c > @@ -4,6 +4,7 @@ > */ > > #include <linux/list.h> > +#include <linux/module.h> > #include <linux/mutex.h> > #include <linux/seq_file.h> > #include <linux/slab.h> > @@ -66,6 +67,7 @@ EXPORT_SYMBOL(drm_client_close); > * @dev: DRM device > * @client: DRM client > * @name: Client name > + * @funcs: DRM client functions (optional) > * > * Use drm_client_put() to free the client. > * > @@ -73,24 +75,47 @@ EXPORT_SYMBOL(drm_client_close); > * Zero on success or negative error code on failure. > */ > int drm_client_new(struct drm_device *dev, struct drm_client_dev *client, > - const char *name) > + const char *name, const struct drm_client_funcs *funcs) > { > + bool registered; > int ret; > > if (!drm_core_check_feature(dev, DRIVER_MODESET) || > !dev->driver->dumb_create || !dev->driver->gem_prime_vmap) > return -ENOTSUPP; > > + if (funcs && !try_module_get(funcs->owner)) > + return -ENODEV; > + > client->dev = dev; > client->name = name; > + client->funcs = funcs; > > ret = drm_client_open(client); > if (ret) > - return ret; > + goto err_put_module; > + > + mutex_lock(&dev->clientlist_mutex); > + registered = dev->registered; > + if (registered) > + list_add(&client->list, &dev->clientlist); > + mutex_unlock(&dev->clientlist_mutex); > + if (!registered) { > + ret = -ENODEV; > + goto err_close; > + } > > drm_dev_get(dev); This only works if the caller holds a reference for us on dev already, or has some other guarantee that it won't disappear. Would be good to mention this in the kerneldoc. > return 0; > + > +err_close: > + drm_client_close(client); > +err_put_module: > + if (funcs) > + module_put(funcs->owner); > + > + return ret; > } > EXPORT_SYMBOL(drm_client_new); > > @@ -116,9 +141,72 @@ void drm_client_release(struct drm_client_dev *client) > > drm_client_close(client); > drm_dev_put(dev); > + if (client->funcs) > + module_put(client->funcs->owner); > } > EXPORT_SYMBOL(drm_client_release); > > +void drm_client_dev_unregister(struct drm_device *dev) > +{ > + struct drm_client_dev *client, *tmp; > + > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > + return; > + > + mutex_lock(&dev->clientlist_mutex); > + list_for_each_entry_safe(client, tmp, &dev->clientlist, list) { > + list_del(&client->list); > + if (client->funcs && client->funcs->unregister) { > + client->funcs->unregister(client); Hm, not ->unregister is called while holding the lock. I thought that blows up for you? > + } else { > + drm_client_release(client); > + kfree(client); > + } > + } > + mutex_unlock(&dev->clientlist_mutex); > +} > + Needs a bit of kerneldoc here since exported function. Drivers might also want to call this from their own hotplug handling. > +void drm_client_dev_hotplug(struct drm_device *dev) > +{ > + struct drm_client_dev *client; > + int ret; > + > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > + return; > + > + mutex_lock(&dev->clientlist_mutex); > + list_for_each_entry(client, &dev->clientlist, list) { > + if (!client->funcs || !client->funcs->hotplug) > + continue; > + > + ret = client->funcs->hotplug(client); > + DRM_DEV_DEBUG_KMS(dev->dev, "%s: ret=%d\n", client->name, ret); > + } > + mutex_unlock(&dev->clientlist_mutex); > +} > +EXPORT_SYMBOL(drm_client_dev_hotplug); > + > +void drm_client_dev_restore(struct drm_device *dev) > +{ > + struct drm_client_dev *client; > + int ret; > + > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > + return; > + > + mutex_lock(&dev->clientlist_mutex); > + list_for_each_entry(client, &dev->clientlist, list) { > + if (!client->funcs || !client->funcs->restore) > + continue; > + > + ret = client->funcs->restore(client); > + DRM_DEV_DEBUG_KMS(dev->dev, "%s: ret=%d\n", client->name, ret); > + if (!ret) /* The first one to return zero gets the privilege to restore */ > + break; > + } > + mutex_unlock(&dev->clientlist_mutex); > +} > + > static void drm_client_buffer_delete(struct drm_client_buffer *buffer) > { > struct drm_device *dev = buffer->client->dev; > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index e7ff0b03328b..6eb935bb2f92 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -34,6 +34,7 @@ > #include <linux/slab.h> > #include <linux/srcu.h> > > +#include <drm/drm_client.h> > #include <drm/drm_drv.h> > #include <drm/drmP.h> > > @@ -506,6 +507,7 @@ int drm_dev_init(struct drm_device *dev, > > INIT_LIST_HEAD(&dev->filelist); > INIT_LIST_HEAD(&dev->filelist_internal); > + INIT_LIST_HEAD(&dev->clientlist); > INIT_LIST_HEAD(&dev->ctxlist); > INIT_LIST_HEAD(&dev->vmalist); > INIT_LIST_HEAD(&dev->maplist); > @@ -515,6 +517,7 @@ int drm_dev_init(struct drm_device *dev, > spin_lock_init(&dev->event_lock); > mutex_init(&dev->struct_mutex); > mutex_init(&dev->filelist_mutex); > + mutex_init(&dev->clientlist_mutex); > mutex_init(&dev->ctxlist_mutex); > mutex_init(&dev->master_mutex); > > @@ -570,6 +573,7 @@ int drm_dev_init(struct drm_device *dev, > err_free: > mutex_destroy(&dev->master_mutex); > mutex_destroy(&dev->ctxlist_mutex); > + mutex_destroy(&dev->clientlist_mutex); > mutex_destroy(&dev->filelist_mutex); > mutex_destroy(&dev->struct_mutex); > return ret; > @@ -604,6 +608,7 @@ void drm_dev_fini(struct drm_device *dev) > > mutex_destroy(&dev->master_mutex); > mutex_destroy(&dev->ctxlist_mutex); > + mutex_destroy(&dev->clientlist_mutex); > mutex_destroy(&dev->filelist_mutex); > mutex_destroy(&dev->struct_mutex); > kfree(dev->unique); > @@ -859,6 +864,8 @@ void drm_dev_unregister(struct drm_device *dev) > > dev->registered = false; > > + drm_client_dev_unregister(dev); > + > if (drm_core_check_feature(dev, DRIVER_MODESET)) > drm_modeset_unregister_all(dev); > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c > index 5762a7c441e9..718c7f961d8a 100644 > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > @@ -181,7 +181,7 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev, > > fb_helper = &fbdev_cma->fb_helper; > > - ret = drm_client_new(dev, &fb_helper->client, "fbdev"); > + ret = drm_client_new(dev, &fb_helper->client, "fbdev", NULL); > if (ret) > goto err_free; > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 0a0a577ebc3c..bea3a0cb324a 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -2989,8 +2989,15 @@ static void drm_fbdev_fb_destroy(struct fb_info *info) > } > > drm_client_framebuffer_delete(fb_helper->buffer); > - drm_client_release(&fb_helper->client); > - kfree(fb_helper); > + /* > + * FIXME: > + * Remove conditional when all CMA drivers have been moved over to using > + * drm_fbdev_generic_setup(). > + */ > + if (fb_helper->client.funcs) { > + drm_client_release(&fb_helper->client); > + kfree(fb_helper); > + } > } > > static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > index 66bb403dc8ab..ffa8dc35515f 100644 > --- a/drivers/gpu/drm/drm_file.c > +++ b/drivers/gpu/drm/drm_file.c > @@ -35,6 +35,7 @@ > #include <linux/slab.h> > #include <linux/module.h> > > +#include <drm/drm_client.h> > #include <drm/drm_file.h> > #include <drm/drmP.h> > > @@ -444,6 +445,8 @@ void drm_lastclose(struct drm_device * dev) > > if (drm_core_check_feature(dev, DRIVER_LEGACY)) > drm_legacy_dev_reinit(dev); > + > + drm_client_dev_restore(dev); > } > > /** > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index 527743394150..26be57e28a9d 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -33,6 +33,7 @@ > #include <linux/moduleparam.h> > > #include <drm/drmP.h> > +#include <drm/drm_client.h> > #include <drm/drm_crtc.h> > #include <drm/drm_fourcc.h> > #include <drm/drm_crtc_helper.h> > @@ -563,6 +564,8 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev) > drm_sysfs_hotplug_event(dev); > if (dev->mode_config.funcs->output_poll_changed) > dev->mode_config.funcs->output_poll_changed(dev); > + > + drm_client_dev_hotplug(dev); > } > EXPORT_SYMBOL(drm_kms_helper_hotplug_event); > > diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h > index e366f95d4414..02cbb02714d8 100644 > --- a/include/drm/drm_client.h > +++ b/include/drm/drm_client.h > @@ -5,10 +5,66 @@ > > #include <linux/types.h> > > +struct drm_client_dev; > struct drm_device; > struct drm_file; > struct drm_framebuffer; > struct drm_gem_object; > +struct module; > + > +/** > + * struct drm_client_funcs - DRM client callbacks > + */ > +struct drm_client_funcs { > + /** > + * @owner: The module owner > + */ > + struct module *owner; > + > + /** > + * @release: > + * > + * Called when the reference count is dropped to zero. Users of this > + * callback is responsible for calling drm_client_close() and freeing > + * the client structure. > + * > + * This callback is optional. > + */ > + void (*release)(struct drm_client_dev *client); Hm, is this no longer in use? > + > + /** > + * @unregister: > + * > + * Called when &drm_device is unregistered. The client should respond by > + * releasing it's resources using drm_client_put(). If it can't do so > + * due to resoruces being tied up, like fbdev with open file handles, > + * it should release it's resources as soon as possible. This still talks about refcounting and _put ... needs a refresher. > + * > + * This callback is optional. > + */ > + void (*unregister)(struct drm_client_dev *client); > + > + /** > + * @restore: > + * > + * Called on drm_lastclose(). The first client instance in the list that > + * returns zero gets the privilege to restore and no more clients are > + * called. This callback is not called after @unregister has been called. > + * > + * This callback is optional. > + */ > + int (*restore)(struct drm_client_dev *client); > + > + /** > + * @hotplug: > + * > + * Called on drm_kms_helper_hotplug_event(). > + * This callback is not called after @unregister has been called. > + * > + * This callback is optional. > + */ > + int (*hotplug)(struct drm_client_dev *client); > +}; > > /** > * struct drm_client_dev - DRM client instance > @@ -24,6 +80,19 @@ struct drm_client_dev { > */ > const char *name; > > + /** > + * @list: > + * > + * List of all clients of a DRM device, linked into > + * &drm_device.clientlist. Protected by &drm_device.clientlist_mutex. > + */ > + struct list_head list; > + > + /** > + * @funcs: DRM client functions (optional) > + */ > + const struct drm_client_funcs *funcs; > + > /** > * @file: DRM file > */ > @@ -31,9 +100,13 @@ struct drm_client_dev { > }; > > int drm_client_new(struct drm_device *dev, struct drm_client_dev *client, > - const char *name); > + const char *name, const struct drm_client_funcs *funcs); > void drm_client_release(struct drm_client_dev *client); > > +void drm_client_dev_unregister(struct drm_device *dev); > +void drm_client_dev_hotplug(struct drm_device *dev); > +void drm_client_dev_restore(struct drm_device *dev); > + > /** > * struct drm_client_buffer - DRM client buffer > */ > diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h > index 9e29976d4e98..f9c6e0e3aec7 100644 > --- a/include/drm/drm_device.h > +++ b/include/drm/drm_device.h > @@ -81,6 +81,20 @@ struct drm_device { > */ > struct list_head filelist_internal; > > + /** > + * @clientlist_mutex: > + * > + * Protects @clientlist access. > + */ > + struct mutex clientlist_mutex; > + > + /** > + * @clientlist: > + * > + * List of in-kernel clients. Protected by @clientlist_mutex. > + */ > + struct list_head clientlist; > + > /** \name Memory management */ > /*@{ */ > struct list_head maplist; /**< Linked list of regions */ > -- > 2.15.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Den 03.07.2018 09.46, skrev Daniel Vetter: > On Mon, Jul 02, 2018 at 03:54:29PM +0200, Noralf Trønnes wrote: >> Add client callbacks and hook them up. >> Add a list of clients per drm_device. >> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > btw for reviewing it'd be simpler if you merge the 2 patches that add the > client library, avoids me having to jump back&forth .. > > Bunch of comments below still. > -Daniel > >> --- >> drivers/gpu/drm/drm_client.c | 92 ++++++++++++++++++++++++++++++++++++- >> drivers/gpu/drm/drm_drv.c | 7 +++ >> drivers/gpu/drm/drm_fb_cma_helper.c | 2 +- >> drivers/gpu/drm/drm_fb_helper.c | 11 ++++- >> drivers/gpu/drm/drm_file.c | 3 ++ >> drivers/gpu/drm/drm_probe_helper.c | 3 ++ >> include/drm/drm_client.h | 75 +++++++++++++++++++++++++++++- >> include/drm/drm_device.h | 14 ++++++ >> 8 files changed, 201 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c >> index 9c9b8ac7aea3..f05ee98bd10c 100644 >> --- a/drivers/gpu/drm/drm_client.c >> +++ b/drivers/gpu/drm/drm_client.c >> @@ -4,6 +4,7 @@ >> */ >> >> #include <linux/list.h> >> +#include <linux/module.h> >> #include <linux/mutex.h> >> #include <linux/seq_file.h> >> #include <linux/slab.h> >> @@ -66,6 +67,7 @@ EXPORT_SYMBOL(drm_client_close); >> * @dev: DRM device >> * @client: DRM client >> * @name: Client name >> + * @funcs: DRM client functions (optional) >> * >> * Use drm_client_put() to free the client. >> * >> @@ -73,24 +75,47 @@ EXPORT_SYMBOL(drm_client_close); >> * Zero on success or negative error code on failure. >> */ >> int drm_client_new(struct drm_device *dev, struct drm_client_dev *client, >> - const char *name) >> + const char *name, const struct drm_client_funcs *funcs) >> { >> + bool registered; >> int ret; >> >> if (!drm_core_check_feature(dev, DRIVER_MODESET) || >> !dev->driver->dumb_create || !dev->driver->gem_prime_vmap) >> return -ENOTSUPP; >> >> + if (funcs && !try_module_get(funcs->owner)) >> + return -ENODEV; >> + >> client->dev = dev; >> client->name = name; >> + client->funcs = funcs; >> >> ret = drm_client_open(client); >> if (ret) >> - return ret; >> + goto err_put_module; >> + >> + mutex_lock(&dev->clientlist_mutex); >> + registered = dev->registered; >> + if (registered) >> + list_add(&client->list, &dev->clientlist); >> + mutex_unlock(&dev->clientlist_mutex); >> + if (!registered) { >> + ret = -ENODEV; >> + goto err_close; >> + } >> >> drm_dev_get(dev); > This only works if the caller holds a reference for us on dev already, or > has some other guarantee that it won't disappear. Would be good to mention > this in the kerneldoc. > >> return 0; >> + >> +err_close: >> + drm_client_close(client); >> +err_put_module: >> + if (funcs) >> + module_put(funcs->owner); >> + >> + return ret; >> } >> EXPORT_SYMBOL(drm_client_new); >> >> @@ -116,9 +141,72 @@ void drm_client_release(struct drm_client_dev *client) >> >> drm_client_close(client); >> drm_dev_put(dev); >> + if (client->funcs) >> + module_put(client->funcs->owner); >> } >> EXPORT_SYMBOL(drm_client_release); >> >> +void drm_client_dev_unregister(struct drm_device *dev) >> +{ >> + struct drm_client_dev *client, *tmp; >> + >> + if (!drm_core_check_feature(dev, DRIVER_MODESET)) >> + return; >> + >> + mutex_lock(&dev->clientlist_mutex); >> + list_for_each_entry_safe(client, tmp, &dev->clientlist, list) { >> + list_del(&client->list); >> + if (client->funcs && client->funcs->unregister) { >> + client->funcs->unregister(client); > Hm, not ->unregister is called while holding the lock. I thought that > blows up for you? It is fine now that we decided that the client can't remove itself. Only the driver can do it on drm_dev_unregister(). >> + } else { >> + drm_client_release(client); >> + kfree(client); >> + } >> + } >> + mutex_unlock(&dev->clientlist_mutex); >> +} >> + > Needs a bit of kerneldoc here since exported function. Drivers might also > want to call this from their own hotplug handling. drm_client_dev_hotplug() is only called by drm_kms_helper_hotplug_event(). The reason it's exported is because the helper can be built as a module. Noralf. >> +void drm_client_dev_hotplug(struct drm_device *dev) >> +{ >> + struct drm_client_dev *client; >> + int ret; >> + >> + if (!drm_core_check_feature(dev, DRIVER_MODESET)) >> + return; >> + >> + mutex_lock(&dev->clientlist_mutex); >> + list_for_each_entry(client, &dev->clientlist, list) { >> + if (!client->funcs || !client->funcs->hotplug) >> + continue; >> + >> + ret = client->funcs->hotplug(client); >> + DRM_DEV_DEBUG_KMS(dev->dev, "%s: ret=%d\n", client->name, ret); >> + } >> + mutex_unlock(&dev->clientlist_mutex); >> +} >> +EXPORT_SYMBOL(drm_client_dev_hotplug); >> + >> +void drm_client_dev_restore(struct drm_device *dev) >> +{ >> + struct drm_client_dev *client; >> + int ret; >> + >> + if (!drm_core_check_feature(dev, DRIVER_MODESET)) >> + return; >> + >> + mutex_lock(&dev->clientlist_mutex); >> + list_for_each_entry(client, &dev->clientlist, list) { >> + if (!client->funcs || !client->funcs->restore) >> + continue; >> + >> + ret = client->funcs->restore(client); >> + DRM_DEV_DEBUG_KMS(dev->dev, "%s: ret=%d\n", client->name, ret); >> + if (!ret) /* The first one to return zero gets the privilege to restore */ >> + break; >> + } >> + mutex_unlock(&dev->clientlist_mutex); >> +} >> + >> static void drm_client_buffer_delete(struct drm_client_buffer *buffer) >> { >> struct drm_device *dev = buffer->client->dev; >> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c >> index e7ff0b03328b..6eb935bb2f92 100644 >> --- a/drivers/gpu/drm/drm_drv.c >> +++ b/drivers/gpu/drm/drm_drv.c >> @@ -34,6 +34,7 @@ >> #include <linux/slab.h> >> #include <linux/srcu.h> >> >> +#include <drm/drm_client.h> >> #include <drm/drm_drv.h> >> #include <drm/drmP.h> >> >> @@ -506,6 +507,7 @@ int drm_dev_init(struct drm_device *dev, >> >> INIT_LIST_HEAD(&dev->filelist); >> INIT_LIST_HEAD(&dev->filelist_internal); >> + INIT_LIST_HEAD(&dev->clientlist); >> INIT_LIST_HEAD(&dev->ctxlist); >> INIT_LIST_HEAD(&dev->vmalist); >> INIT_LIST_HEAD(&dev->maplist); >> @@ -515,6 +517,7 @@ int drm_dev_init(struct drm_device *dev, >> spin_lock_init(&dev->event_lock); >> mutex_init(&dev->struct_mutex); >> mutex_init(&dev->filelist_mutex); >> + mutex_init(&dev->clientlist_mutex); >> mutex_init(&dev->ctxlist_mutex); >> mutex_init(&dev->master_mutex); >> >> @@ -570,6 +573,7 @@ int drm_dev_init(struct drm_device *dev, >> err_free: >> mutex_destroy(&dev->master_mutex); >> mutex_destroy(&dev->ctxlist_mutex); >> + mutex_destroy(&dev->clientlist_mutex); >> mutex_destroy(&dev->filelist_mutex); >> mutex_destroy(&dev->struct_mutex); >> return ret; >> @@ -604,6 +608,7 @@ void drm_dev_fini(struct drm_device *dev) >> >> mutex_destroy(&dev->master_mutex); >> mutex_destroy(&dev->ctxlist_mutex); >> + mutex_destroy(&dev->clientlist_mutex); >> mutex_destroy(&dev->filelist_mutex); >> mutex_destroy(&dev->struct_mutex); >> kfree(dev->unique); >> @@ -859,6 +864,8 @@ void drm_dev_unregister(struct drm_device *dev) >> >> dev->registered = false; >> >> + drm_client_dev_unregister(dev); >> + >> if (drm_core_check_feature(dev, DRIVER_MODESET)) >> drm_modeset_unregister_all(dev); >> >> diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c >> index 5762a7c441e9..718c7f961d8a 100644 >> --- a/drivers/gpu/drm/drm_fb_cma_helper.c >> +++ b/drivers/gpu/drm/drm_fb_cma_helper.c >> @@ -181,7 +181,7 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev, >> >> fb_helper = &fbdev_cma->fb_helper; >> >> - ret = drm_client_new(dev, &fb_helper->client, "fbdev"); >> + ret = drm_client_new(dev, &fb_helper->client, "fbdev", NULL); >> if (ret) >> goto err_free; >> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >> index 0a0a577ebc3c..bea3a0cb324a 100644 >> --- a/drivers/gpu/drm/drm_fb_helper.c >> +++ b/drivers/gpu/drm/drm_fb_helper.c >> @@ -2989,8 +2989,15 @@ static void drm_fbdev_fb_destroy(struct fb_info *info) >> } >> >> drm_client_framebuffer_delete(fb_helper->buffer); >> - drm_client_release(&fb_helper->client); >> - kfree(fb_helper); >> + /* >> + * FIXME: >> + * Remove conditional when all CMA drivers have been moved over to using >> + * drm_fbdev_generic_setup(). >> + */ >> + if (fb_helper->client.funcs) { >> + drm_client_release(&fb_helper->client); >> + kfree(fb_helper); >> + } >> } >> >> static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) >> diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c >> index 66bb403dc8ab..ffa8dc35515f 100644 >> --- a/drivers/gpu/drm/drm_file.c >> +++ b/drivers/gpu/drm/drm_file.c >> @@ -35,6 +35,7 @@ >> #include <linux/slab.h> >> #include <linux/module.h> >> >> +#include <drm/drm_client.h> >> #include <drm/drm_file.h> >> #include <drm/drmP.h> >> >> @@ -444,6 +445,8 @@ void drm_lastclose(struct drm_device * dev) >> >> if (drm_core_check_feature(dev, DRIVER_LEGACY)) >> drm_legacy_dev_reinit(dev); >> + >> + drm_client_dev_restore(dev); >> } >> >> /** >> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c >> index 527743394150..26be57e28a9d 100644 >> --- a/drivers/gpu/drm/drm_probe_helper.c >> +++ b/drivers/gpu/drm/drm_probe_helper.c >> @@ -33,6 +33,7 @@ >> #include <linux/moduleparam.h> >> >> #include <drm/drmP.h> >> +#include <drm/drm_client.h> >> #include <drm/drm_crtc.h> >> #include <drm/drm_fourcc.h> >> #include <drm/drm_crtc_helper.h> >> @@ -563,6 +564,8 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev) >> drm_sysfs_hotplug_event(dev); >> if (dev->mode_config.funcs->output_poll_changed) >> dev->mode_config.funcs->output_poll_changed(dev); >> + >> + drm_client_dev_hotplug(dev); >> } >> EXPORT_SYMBOL(drm_kms_helper_hotplug_event); >> >> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h >> index e366f95d4414..02cbb02714d8 100644 >> --- a/include/drm/drm_client.h >> +++ b/include/drm/drm_client.h >> @@ -5,10 +5,66 @@ >> >> #include <linux/types.h> >> >> +struct drm_client_dev; >> struct drm_device; >> struct drm_file; >> struct drm_framebuffer; >> struct drm_gem_object; >> +struct module; >> + >> +/** >> + * struct drm_client_funcs - DRM client callbacks >> + */ >> +struct drm_client_funcs { >> + /** >> + * @owner: The module owner >> + */ >> + struct module *owner; >> + >> + /** >> + * @release: >> + * >> + * Called when the reference count is dropped to zero. Users of this >> + * callback is responsible for calling drm_client_close() and freeing >> + * the client structure. >> + * >> + * This callback is optional. >> + */ >> + void (*release)(struct drm_client_dev *client); > Hm, is this no longer in use? > >> + >> + /** >> + * @unregister: >> + * >> + * Called when &drm_device is unregistered. The client should respond by >> + * releasing it's resources using drm_client_put(). If it can't do so >> + * due to resoruces being tied up, like fbdev with open file handles, >> + * it should release it's resources as soon as possible. > This still talks about refcounting and _put ... needs a refresher. > >> + * >> + * This callback is optional. >> + */ >> + void (*unregister)(struct drm_client_dev *client); >> + >> + /** >> + * @restore: >> + * >> + * Called on drm_lastclose(). The first client instance in the list that >> + * returns zero gets the privilege to restore and no more clients are >> + * called. This callback is not called after @unregister has been called. >> + * >> + * This callback is optional. >> + */ >> + int (*restore)(struct drm_client_dev *client); >> + >> + /** >> + * @hotplug: >> + * >> + * Called on drm_kms_helper_hotplug_event(). >> + * This callback is not called after @unregister has been called. >> + * >> + * This callback is optional. >> + */ >> + int (*hotplug)(struct drm_client_dev *client); >> +}; >> >> /** >> * struct drm_client_dev - DRM client instance >> @@ -24,6 +80,19 @@ struct drm_client_dev { >> */ >> const char *name; >> >> + /** >> + * @list: >> + * >> + * List of all clients of a DRM device, linked into >> + * &drm_device.clientlist. Protected by &drm_device.clientlist_mutex. >> + */ >> + struct list_head list; >> + >> + /** >> + * @funcs: DRM client functions (optional) >> + */ >> + const struct drm_client_funcs *funcs; >> + >> /** >> * @file: DRM file >> */ >> @@ -31,9 +100,13 @@ struct drm_client_dev { >> }; >> >> int drm_client_new(struct drm_device *dev, struct drm_client_dev *client, >> - const char *name); >> + const char *name, const struct drm_client_funcs *funcs); >> void drm_client_release(struct drm_client_dev *client); >> >> +void drm_client_dev_unregister(struct drm_device *dev); >> +void drm_client_dev_hotplug(struct drm_device *dev); >> +void drm_client_dev_restore(struct drm_device *dev); >> + >> /** >> * struct drm_client_buffer - DRM client buffer >> */ >> diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h >> index 9e29976d4e98..f9c6e0e3aec7 100644 >> --- a/include/drm/drm_device.h >> +++ b/include/drm/drm_device.h >> @@ -81,6 +81,20 @@ struct drm_device { >> */ >> struct list_head filelist_internal; >> >> + /** >> + * @clientlist_mutex: >> + * >> + * Protects @clientlist access. >> + */ >> + struct mutex clientlist_mutex; >> + >> + /** >> + * @clientlist: >> + * >> + * List of in-kernel clients. Protected by @clientlist_mutex. >> + */ >> + struct list_head clientlist; >> + >> /** \name Memory management */ >> /*@{ */ >> struct list_head maplist; /**< Linked list of regions */ >> -- >> 2.15.1 >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Jul 03, 2018 at 03:07:50PM +0200, Noralf Trønnes wrote: > > Den 03.07.2018 09.46, skrev Daniel Vetter: > > On Mon, Jul 02, 2018 at 03:54:29PM +0200, Noralf Trønnes wrote: > > > Add client callbacks and hook them up. > > > Add a list of clients per drm_device. > > > > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > btw for reviewing it'd be simpler if you merge the 2 patches that add the > > client library, avoids me having to jump back&forth .. > > > > Bunch of comments below still. > > -Daniel > > > > > --- > > > drivers/gpu/drm/drm_client.c | 92 ++++++++++++++++++++++++++++++++++++- > > > drivers/gpu/drm/drm_drv.c | 7 +++ > > > drivers/gpu/drm/drm_fb_cma_helper.c | 2 +- > > > drivers/gpu/drm/drm_fb_helper.c | 11 ++++- > > > drivers/gpu/drm/drm_file.c | 3 ++ > > > drivers/gpu/drm/drm_probe_helper.c | 3 ++ > > > include/drm/drm_client.h | 75 +++++++++++++++++++++++++++++- > > > include/drm/drm_device.h | 14 ++++++ > > > 8 files changed, 201 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c > > > index 9c9b8ac7aea3..f05ee98bd10c 100644 > > > --- a/drivers/gpu/drm/drm_client.c > > > +++ b/drivers/gpu/drm/drm_client.c > > > @@ -4,6 +4,7 @@ > > > */ > > > #include <linux/list.h> > > > +#include <linux/module.h> > > > #include <linux/mutex.h> > > > #include <linux/seq_file.h> > > > #include <linux/slab.h> > > > @@ -66,6 +67,7 @@ EXPORT_SYMBOL(drm_client_close); > > > * @dev: DRM device > > > * @client: DRM client > > > * @name: Client name > > > + * @funcs: DRM client functions (optional) > > > * > > > * Use drm_client_put() to free the client. > > > * > > > @@ -73,24 +75,47 @@ EXPORT_SYMBOL(drm_client_close); > > > * Zero on success or negative error code on failure. > > > */ > > > int drm_client_new(struct drm_device *dev, struct drm_client_dev *client, > > > - const char *name) > > > + const char *name, const struct drm_client_funcs *funcs) > > > { > > > + bool registered; > > > int ret; > > > if (!drm_core_check_feature(dev, DRIVER_MODESET) || > > > !dev->driver->dumb_create || !dev->driver->gem_prime_vmap) > > > return -ENOTSUPP; > > > + if (funcs && !try_module_get(funcs->owner)) > > > + return -ENODEV; > > > + > > > client->dev = dev; > > > client->name = name; > > > + client->funcs = funcs; > > > ret = drm_client_open(client); > > > if (ret) > > > - return ret; > > > + goto err_put_module; > > > + > > > + mutex_lock(&dev->clientlist_mutex); > > > + registered = dev->registered; > > > + if (registered) > > > + list_add(&client->list, &dev->clientlist); > > > + mutex_unlock(&dev->clientlist_mutex); > > > + if (!registered) { > > > + ret = -ENODEV; > > > + goto err_close; > > > + } > > > drm_dev_get(dev); > > This only works if the caller holds a reference for us on dev already, or > > has some other guarantee that it won't disappear. Would be good to mention > > this in the kerneldoc. > > > > > return 0; > > > + > > > +err_close: > > > + drm_client_close(client); > > > +err_put_module: > > > + if (funcs) > > > + module_put(funcs->owner); > > > + > > > + return ret; > > > } > > > EXPORT_SYMBOL(drm_client_new); > > > @@ -116,9 +141,72 @@ void drm_client_release(struct drm_client_dev *client) > > > drm_client_close(client); > > > drm_dev_put(dev); > > > + if (client->funcs) > > > + module_put(client->funcs->owner); > > > } > > > EXPORT_SYMBOL(drm_client_release); > > > +void drm_client_dev_unregister(struct drm_device *dev) > > > +{ > > > + struct drm_client_dev *client, *tmp; > > > + > > > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > > > + return; > > > + > > > + mutex_lock(&dev->clientlist_mutex); > > > + list_for_each_entry_safe(client, tmp, &dev->clientlist, list) { > > > + list_del(&client->list); > > > + if (client->funcs && client->funcs->unregister) { > > > + client->funcs->unregister(client); > > Hm, not ->unregister is called while holding the lock. I thought that > > blows up for you? > > It is fine now that we decided that the client can't remove itself. > Only the driver can do it on drm_dev_unregister(). I was more wondering about creating an unecessary locking hierarchy complication. But through the ->hotplug and ->restore callbacks we already require that all client locks (which will also include anything related to fbdev and fbcon, hence also console_lock) must nest within dev->clientlist_mutex. That's the part I was worried about, but that's not a good concern really. So all fine for me on 2nd thought. > > > > + } else { > > > + drm_client_release(client); > > > + kfree(client); > > > + } > > > + } > > > + mutex_unlock(&dev->clientlist_mutex); > > > +} > > > + > > Needs a bit of kerneldoc here since exported function. Drivers might also > > want to call this from their own hotplug handling. > > drm_client_dev_hotplug() is only called by drm_kms_helper_hotplug_event(). > The reason it's exported is because the helper can be built as a module. Anything helpers do drivers should be able to override. Anything drivers should be able to use should have docs. And e.g. drm_sysfs_hotplug_event is actually called by a driver namely vmwgfx, and I think these two functions are pretty much equivalent - one informs userspace clients about a hotplug, the other kernel clients about a hotplug. Anyway, since my only major concern (the locking question) is cleared up after a bit more think I think this is good for an r-b, once the kerneldoc is all fixed up and the ->release callback gone: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Cheers, Daniel > > Noralf. > > > > +void drm_client_dev_hotplug(struct drm_device *dev) > > > +{ > > > + struct drm_client_dev *client; > > > + int ret; > > > + > > > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > > > + return; > > > + > > > + mutex_lock(&dev->clientlist_mutex); > > > + list_for_each_entry(client, &dev->clientlist, list) { > > > + if (!client->funcs || !client->funcs->hotplug) > > > + continue; > > > + > > > + ret = client->funcs->hotplug(client); > > > + DRM_DEV_DEBUG_KMS(dev->dev, "%s: ret=%d\n", client->name, ret); > > > + } > > > + mutex_unlock(&dev->clientlist_mutex); > > > +} > > > +EXPORT_SYMBOL(drm_client_dev_hotplug); > > > + > > > +void drm_client_dev_restore(struct drm_device *dev) > > > +{ > > > + struct drm_client_dev *client; > > > + int ret; > > > + > > > + if (!drm_core_check_feature(dev, DRIVER_MODESET)) > > > + return; > > > + > > > + mutex_lock(&dev->clientlist_mutex); > > > + list_for_each_entry(client, &dev->clientlist, list) { > > > + if (!client->funcs || !client->funcs->restore) > > > + continue; > > > + > > > + ret = client->funcs->restore(client); > > > + DRM_DEV_DEBUG_KMS(dev->dev, "%s: ret=%d\n", client->name, ret); > > > + if (!ret) /* The first one to return zero gets the privilege to restore */ > > > + break; > > > + } > > > + mutex_unlock(&dev->clientlist_mutex); > > > +} > > > + > > > static void drm_client_buffer_delete(struct drm_client_buffer *buffer) > > > { > > > struct drm_device *dev = buffer->client->dev; > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > > > index e7ff0b03328b..6eb935bb2f92 100644 > > > --- a/drivers/gpu/drm/drm_drv.c > > > +++ b/drivers/gpu/drm/drm_drv.c > > > @@ -34,6 +34,7 @@ > > > #include <linux/slab.h> > > > #include <linux/srcu.h> > > > +#include <drm/drm_client.h> > > > #include <drm/drm_drv.h> > > > #include <drm/drmP.h> > > > @@ -506,6 +507,7 @@ int drm_dev_init(struct drm_device *dev, > > > INIT_LIST_HEAD(&dev->filelist); > > > INIT_LIST_HEAD(&dev->filelist_internal); > > > + INIT_LIST_HEAD(&dev->clientlist); > > > INIT_LIST_HEAD(&dev->ctxlist); > > > INIT_LIST_HEAD(&dev->vmalist); > > > INIT_LIST_HEAD(&dev->maplist); > > > @@ -515,6 +517,7 @@ int drm_dev_init(struct drm_device *dev, > > > spin_lock_init(&dev->event_lock); > > > mutex_init(&dev->struct_mutex); > > > mutex_init(&dev->filelist_mutex); > > > + mutex_init(&dev->clientlist_mutex); > > > mutex_init(&dev->ctxlist_mutex); > > > mutex_init(&dev->master_mutex); > > > @@ -570,6 +573,7 @@ int drm_dev_init(struct drm_device *dev, > > > err_free: > > > mutex_destroy(&dev->master_mutex); > > > mutex_destroy(&dev->ctxlist_mutex); > > > + mutex_destroy(&dev->clientlist_mutex); > > > mutex_destroy(&dev->filelist_mutex); > > > mutex_destroy(&dev->struct_mutex); > > > return ret; > > > @@ -604,6 +608,7 @@ void drm_dev_fini(struct drm_device *dev) > > > mutex_destroy(&dev->master_mutex); > > > mutex_destroy(&dev->ctxlist_mutex); > > > + mutex_destroy(&dev->clientlist_mutex); > > > mutex_destroy(&dev->filelist_mutex); > > > mutex_destroy(&dev->struct_mutex); > > > kfree(dev->unique); > > > @@ -859,6 +864,8 @@ void drm_dev_unregister(struct drm_device *dev) > > > dev->registered = false; > > > + drm_client_dev_unregister(dev); > > > + > > > if (drm_core_check_feature(dev, DRIVER_MODESET)) > > > drm_modeset_unregister_all(dev); > > > diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c > > > index 5762a7c441e9..718c7f961d8a 100644 > > > --- a/drivers/gpu/drm/drm_fb_cma_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_cma_helper.c > > > @@ -181,7 +181,7 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev, > > > fb_helper = &fbdev_cma->fb_helper; > > > - ret = drm_client_new(dev, &fb_helper->client, "fbdev"); > > > + ret = drm_client_new(dev, &fb_helper->client, "fbdev", NULL); > > > if (ret) > > > goto err_free; > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > > > index 0a0a577ebc3c..bea3a0cb324a 100644 > > > --- a/drivers/gpu/drm/drm_fb_helper.c > > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > > @@ -2989,8 +2989,15 @@ static void drm_fbdev_fb_destroy(struct fb_info *info) > > > } > > > drm_client_framebuffer_delete(fb_helper->buffer); > > > - drm_client_release(&fb_helper->client); > > > - kfree(fb_helper); > > > + /* > > > + * FIXME: > > > + * Remove conditional when all CMA drivers have been moved over to using > > > + * drm_fbdev_generic_setup(). > > > + */ > > > + if (fb_helper->client.funcs) { > > > + drm_client_release(&fb_helper->client); > > > + kfree(fb_helper); > > > + } > > > } > > > static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) > > > diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c > > > index 66bb403dc8ab..ffa8dc35515f 100644 > > > --- a/drivers/gpu/drm/drm_file.c > > > +++ b/drivers/gpu/drm/drm_file.c > > > @@ -35,6 +35,7 @@ > > > #include <linux/slab.h> > > > #include <linux/module.h> > > > +#include <drm/drm_client.h> > > > #include <drm/drm_file.h> > > > #include <drm/drmP.h> > > > @@ -444,6 +445,8 @@ void drm_lastclose(struct drm_device * dev) > > > if (drm_core_check_feature(dev, DRIVER_LEGACY)) > > > drm_legacy_dev_reinit(dev); > > > + > > > + drm_client_dev_restore(dev); > > > } > > > /** > > > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > > > index 527743394150..26be57e28a9d 100644 > > > --- a/drivers/gpu/drm/drm_probe_helper.c > > > +++ b/drivers/gpu/drm/drm_probe_helper.c > > > @@ -33,6 +33,7 @@ > > > #include <linux/moduleparam.h> > > > #include <drm/drmP.h> > > > +#include <drm/drm_client.h> > > > #include <drm/drm_crtc.h> > > > #include <drm/drm_fourcc.h> > > > #include <drm/drm_crtc_helper.h> > > > @@ -563,6 +564,8 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev) > > > drm_sysfs_hotplug_event(dev); > > > if (dev->mode_config.funcs->output_poll_changed) > > > dev->mode_config.funcs->output_poll_changed(dev); > > > + > > > + drm_client_dev_hotplug(dev); > > > } > > > EXPORT_SYMBOL(drm_kms_helper_hotplug_event); > > > diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h > > > index e366f95d4414..02cbb02714d8 100644 > > > --- a/include/drm/drm_client.h > > > +++ b/include/drm/drm_client.h > > > @@ -5,10 +5,66 @@ > > > #include <linux/types.h> > > > +struct drm_client_dev; > > > struct drm_device; > > > struct drm_file; > > > struct drm_framebuffer; > > > struct drm_gem_object; > > > +struct module; > > > + > > > +/** > > > + * struct drm_client_funcs - DRM client callbacks > > > + */ > > > +struct drm_client_funcs { > > > + /** > > > + * @owner: The module owner > > > + */ > > > + struct module *owner; > > > + > > > + /** > > > + * @release: > > > + * > > > + * Called when the reference count is dropped to zero. Users of this > > > + * callback is responsible for calling drm_client_close() and freeing > > > + * the client structure. > > > + * > > > + * This callback is optional. > > > + */ > > > + void (*release)(struct drm_client_dev *client); > > Hm, is this no longer in use? > > > > > + > > > + /** > > > + * @unregister: > > > + * > > > + * Called when &drm_device is unregistered. The client should respond by > > > + * releasing it's resources using drm_client_put(). If it can't do so > > > + * due to resoruces being tied up, like fbdev with open file handles, > > > + * it should release it's resources as soon as possible. > > This still talks about refcounting and _put ... needs a refresher. > > > > > + * > > > + * This callback is optional. > > > + */ > > > + void (*unregister)(struct drm_client_dev *client); > > > + > > > + /** > > > + * @restore: > > > + * > > > + * Called on drm_lastclose(). The first client instance in the list that > > > + * returns zero gets the privilege to restore and no more clients are > > > + * called. This callback is not called after @unregister has been called. > > > + * > > > + * This callback is optional. > > > + */ > > > + int (*restore)(struct drm_client_dev *client); > > > + > > > + /** > > > + * @hotplug: > > > + * > > > + * Called on drm_kms_helper_hotplug_event(). > > > + * This callback is not called after @unregister has been called. > > > + * > > > + * This callback is optional. > > > + */ > > > + int (*hotplug)(struct drm_client_dev *client); > > > +}; > > > /** > > > * struct drm_client_dev - DRM client instance > > > @@ -24,6 +80,19 @@ struct drm_client_dev { > > > */ > > > const char *name; > > > + /** > > > + * @list: > > > + * > > > + * List of all clients of a DRM device, linked into > > > + * &drm_device.clientlist. Protected by &drm_device.clientlist_mutex. > > > + */ > > > + struct list_head list; > > > + > > > + /** > > > + * @funcs: DRM client functions (optional) > > > + */ > > > + const struct drm_client_funcs *funcs; > > > + > > > /** > > > * @file: DRM file > > > */ > > > @@ -31,9 +100,13 @@ struct drm_client_dev { > > > }; > > > int drm_client_new(struct drm_device *dev, struct drm_client_dev *client, > > > - const char *name); > > > + const char *name, const struct drm_client_funcs *funcs); > > > void drm_client_release(struct drm_client_dev *client); > > > +void drm_client_dev_unregister(struct drm_device *dev); > > > +void drm_client_dev_hotplug(struct drm_device *dev); > > > +void drm_client_dev_restore(struct drm_device *dev); > > > + > > > /** > > > * struct drm_client_buffer - DRM client buffer > > > */ > > > diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h > > > index 9e29976d4e98..f9c6e0e3aec7 100644 > > > --- a/include/drm/drm_device.h > > > +++ b/include/drm/drm_device.h > > > @@ -81,6 +81,20 @@ struct drm_device { > > > */ > > > struct list_head filelist_internal; > > > + /** > > > + * @clientlist_mutex: > > > + * > > > + * Protects @clientlist access. > > > + */ > > > + struct mutex clientlist_mutex; > > > + > > > + /** > > > + * @clientlist: > > > + * > > > + * List of in-kernel clients. Protected by @clientlist_mutex. > > > + */ > > > + struct list_head clientlist; > > > + > > > /** \name Memory management */ > > > /*@{ */ > > > struct list_head maplist; /**< Linked list of regions */ > > > -- > > > 2.15.1 > > > > > > _______________________________________________ > > > dri-devel mailing list > > > dri-devel@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c index 9c9b8ac7aea3..f05ee98bd10c 100644 --- a/drivers/gpu/drm/drm_client.c +++ b/drivers/gpu/drm/drm_client.c @@ -4,6 +4,7 @@ */ #include <linux/list.h> +#include <linux/module.h> #include <linux/mutex.h> #include <linux/seq_file.h> #include <linux/slab.h> @@ -66,6 +67,7 @@ EXPORT_SYMBOL(drm_client_close); * @dev: DRM device * @client: DRM client * @name: Client name + * @funcs: DRM client functions (optional) * * Use drm_client_put() to free the client. * @@ -73,24 +75,47 @@ EXPORT_SYMBOL(drm_client_close); * Zero on success or negative error code on failure. */ int drm_client_new(struct drm_device *dev, struct drm_client_dev *client, - const char *name) + const char *name, const struct drm_client_funcs *funcs) { + bool registered; int ret; if (!drm_core_check_feature(dev, DRIVER_MODESET) || !dev->driver->dumb_create || !dev->driver->gem_prime_vmap) return -ENOTSUPP; + if (funcs && !try_module_get(funcs->owner)) + return -ENODEV; + client->dev = dev; client->name = name; + client->funcs = funcs; ret = drm_client_open(client); if (ret) - return ret; + goto err_put_module; + + mutex_lock(&dev->clientlist_mutex); + registered = dev->registered; + if (registered) + list_add(&client->list, &dev->clientlist); + mutex_unlock(&dev->clientlist_mutex); + if (!registered) { + ret = -ENODEV; + goto err_close; + } drm_dev_get(dev); return 0; + +err_close: + drm_client_close(client); +err_put_module: + if (funcs) + module_put(funcs->owner); + + return ret; } EXPORT_SYMBOL(drm_client_new); @@ -116,9 +141,72 @@ void drm_client_release(struct drm_client_dev *client) drm_client_close(client); drm_dev_put(dev); + if (client->funcs) + module_put(client->funcs->owner); } EXPORT_SYMBOL(drm_client_release); +void drm_client_dev_unregister(struct drm_device *dev) +{ + struct drm_client_dev *client, *tmp; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return; + + mutex_lock(&dev->clientlist_mutex); + list_for_each_entry_safe(client, tmp, &dev->clientlist, list) { + list_del(&client->list); + if (client->funcs && client->funcs->unregister) { + client->funcs->unregister(client); + } else { + drm_client_release(client); + kfree(client); + } + } + mutex_unlock(&dev->clientlist_mutex); +} + +void drm_client_dev_hotplug(struct drm_device *dev) +{ + struct drm_client_dev *client; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return; + + mutex_lock(&dev->clientlist_mutex); + list_for_each_entry(client, &dev->clientlist, list) { + if (!client->funcs || !client->funcs->hotplug) + continue; + + ret = client->funcs->hotplug(client); + DRM_DEV_DEBUG_KMS(dev->dev, "%s: ret=%d\n", client->name, ret); + } + mutex_unlock(&dev->clientlist_mutex); +} +EXPORT_SYMBOL(drm_client_dev_hotplug); + +void drm_client_dev_restore(struct drm_device *dev) +{ + struct drm_client_dev *client; + int ret; + + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + return; + + mutex_lock(&dev->clientlist_mutex); + list_for_each_entry(client, &dev->clientlist, list) { + if (!client->funcs || !client->funcs->restore) + continue; + + ret = client->funcs->restore(client); + DRM_DEV_DEBUG_KMS(dev->dev, "%s: ret=%d\n", client->name, ret); + if (!ret) /* The first one to return zero gets the privilege to restore */ + break; + } + mutex_unlock(&dev->clientlist_mutex); +} + static void drm_client_buffer_delete(struct drm_client_buffer *buffer) { struct drm_device *dev = buffer->client->dev; diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index e7ff0b03328b..6eb935bb2f92 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -34,6 +34,7 @@ #include <linux/slab.h> #include <linux/srcu.h> +#include <drm/drm_client.h> #include <drm/drm_drv.h> #include <drm/drmP.h> @@ -506,6 +507,7 @@ int drm_dev_init(struct drm_device *dev, INIT_LIST_HEAD(&dev->filelist); INIT_LIST_HEAD(&dev->filelist_internal); + INIT_LIST_HEAD(&dev->clientlist); INIT_LIST_HEAD(&dev->ctxlist); INIT_LIST_HEAD(&dev->vmalist); INIT_LIST_HEAD(&dev->maplist); @@ -515,6 +517,7 @@ int drm_dev_init(struct drm_device *dev, spin_lock_init(&dev->event_lock); mutex_init(&dev->struct_mutex); mutex_init(&dev->filelist_mutex); + mutex_init(&dev->clientlist_mutex); mutex_init(&dev->ctxlist_mutex); mutex_init(&dev->master_mutex); @@ -570,6 +573,7 @@ int drm_dev_init(struct drm_device *dev, err_free: mutex_destroy(&dev->master_mutex); mutex_destroy(&dev->ctxlist_mutex); + mutex_destroy(&dev->clientlist_mutex); mutex_destroy(&dev->filelist_mutex); mutex_destroy(&dev->struct_mutex); return ret; @@ -604,6 +608,7 @@ void drm_dev_fini(struct drm_device *dev) mutex_destroy(&dev->master_mutex); mutex_destroy(&dev->ctxlist_mutex); + mutex_destroy(&dev->clientlist_mutex); mutex_destroy(&dev->filelist_mutex); mutex_destroy(&dev->struct_mutex); kfree(dev->unique); @@ -859,6 +864,8 @@ void drm_dev_unregister(struct drm_device *dev) dev->registered = false; + drm_client_dev_unregister(dev); + if (drm_core_check_feature(dev, DRIVER_MODESET)) drm_modeset_unregister_all(dev); diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 5762a7c441e9..718c7f961d8a 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -181,7 +181,7 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev, fb_helper = &fbdev_cma->fb_helper; - ret = drm_client_new(dev, &fb_helper->client, "fbdev"); + ret = drm_client_new(dev, &fb_helper->client, "fbdev", NULL); if (ret) goto err_free; diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 0a0a577ebc3c..bea3a0cb324a 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2989,8 +2989,15 @@ static void drm_fbdev_fb_destroy(struct fb_info *info) } drm_client_framebuffer_delete(fb_helper->buffer); - drm_client_release(&fb_helper->client); - kfree(fb_helper); + /* + * FIXME: + * Remove conditional when all CMA drivers have been moved over to using + * drm_fbdev_generic_setup(). + */ + if (fb_helper->client.funcs) { + drm_client_release(&fb_helper->client); + kfree(fb_helper); + } } static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma) diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index 66bb403dc8ab..ffa8dc35515f 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -35,6 +35,7 @@ #include <linux/slab.h> #include <linux/module.h> +#include <drm/drm_client.h> #include <drm/drm_file.h> #include <drm/drmP.h> @@ -444,6 +445,8 @@ void drm_lastclose(struct drm_device * dev) if (drm_core_check_feature(dev, DRIVER_LEGACY)) drm_legacy_dev_reinit(dev); + + drm_client_dev_restore(dev); } /** diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 527743394150..26be57e28a9d 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -33,6 +33,7 @@ #include <linux/moduleparam.h> #include <drm/drmP.h> +#include <drm/drm_client.h> #include <drm/drm_crtc.h> #include <drm/drm_fourcc.h> #include <drm/drm_crtc_helper.h> @@ -563,6 +564,8 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev) drm_sysfs_hotplug_event(dev); if (dev->mode_config.funcs->output_poll_changed) dev->mode_config.funcs->output_poll_changed(dev); + + drm_client_dev_hotplug(dev); } EXPORT_SYMBOL(drm_kms_helper_hotplug_event); diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h index e366f95d4414..02cbb02714d8 100644 --- a/include/drm/drm_client.h +++ b/include/drm/drm_client.h @@ -5,10 +5,66 @@ #include <linux/types.h> +struct drm_client_dev; struct drm_device; struct drm_file; struct drm_framebuffer; struct drm_gem_object; +struct module; + +/** + * struct drm_client_funcs - DRM client callbacks + */ +struct drm_client_funcs { + /** + * @owner: The module owner + */ + struct module *owner; + + /** + * @release: + * + * Called when the reference count is dropped to zero. Users of this + * callback is responsible for calling drm_client_close() and freeing + * the client structure. + * + * This callback is optional. + */ + void (*release)(struct drm_client_dev *client); + + /** + * @unregister: + * + * Called when &drm_device is unregistered. The client should respond by + * releasing it's resources using drm_client_put(). If it can't do so + * due to resoruces being tied up, like fbdev with open file handles, + * it should release it's resources as soon as possible. + * + * This callback is optional. + */ + void (*unregister)(struct drm_client_dev *client); + + /** + * @restore: + * + * Called on drm_lastclose(). The first client instance in the list that + * returns zero gets the privilege to restore and no more clients are + * called. This callback is not called after @unregister has been called. + * + * This callback is optional. + */ + int (*restore)(struct drm_client_dev *client); + + /** + * @hotplug: + * + * Called on drm_kms_helper_hotplug_event(). + * This callback is not called after @unregister has been called. + * + * This callback is optional. + */ + int (*hotplug)(struct drm_client_dev *client); +}; /** * struct drm_client_dev - DRM client instance @@ -24,6 +80,19 @@ struct drm_client_dev { */ const char *name; + /** + * @list: + * + * List of all clients of a DRM device, linked into + * &drm_device.clientlist. Protected by &drm_device.clientlist_mutex. + */ + struct list_head list; + + /** + * @funcs: DRM client functions (optional) + */ + const struct drm_client_funcs *funcs; + /** * @file: DRM file */ @@ -31,9 +100,13 @@ struct drm_client_dev { }; int drm_client_new(struct drm_device *dev, struct drm_client_dev *client, - const char *name); + const char *name, const struct drm_client_funcs *funcs); void drm_client_release(struct drm_client_dev *client); +void drm_client_dev_unregister(struct drm_device *dev); +void drm_client_dev_hotplug(struct drm_device *dev); +void drm_client_dev_restore(struct drm_device *dev); + /** * struct drm_client_buffer - DRM client buffer */ diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h index 9e29976d4e98..f9c6e0e3aec7 100644 --- a/include/drm/drm_device.h +++ b/include/drm/drm_device.h @@ -81,6 +81,20 @@ struct drm_device { */ struct list_head filelist_internal; + /** + * @clientlist_mutex: + * + * Protects @clientlist access. + */ + struct mutex clientlist_mutex; + + /** + * @clientlist: + * + * List of in-kernel clients. Protected by @clientlist_mutex. + */ + struct list_head clientlist; + /** \name Memory management */ /*@{ */ struct list_head maplist; /**< Linked list of regions */
Add client callbacks and hook them up. Add a list of clients per drm_device. Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- drivers/gpu/drm/drm_client.c | 92 ++++++++++++++++++++++++++++++++++++- drivers/gpu/drm/drm_drv.c | 7 +++ drivers/gpu/drm/drm_fb_cma_helper.c | 2 +- drivers/gpu/drm/drm_fb_helper.c | 11 ++++- drivers/gpu/drm/drm_file.c | 3 ++ drivers/gpu/drm/drm_probe_helper.c | 3 ++ include/drm/drm_client.h | 75 +++++++++++++++++++++++++++++- include/drm/drm_device.h | 14 ++++++ 8 files changed, 201 insertions(+), 6 deletions(-)