| Message ID | 1464357644-16775-3-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, May 27, 2016 at 03:00:35PM +0100, Chris Wilson wrote: > If a driver wants to more precisely control its initialisation and in > particular, defer registering its interfaces with userspace until after > everything is setup, it also needs to defer registering the connectors. > As some devices need more work during registration, add a callback so > that drivers can do additional work if required for a connector. > > Correspondingly, we also require an unregister callback. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: dri-devel@lists.freedesktop.org Smells a bit too much like midlayer too me ;-) Imo perfectly fine to have a driver loop besides the register_all which registers stuff like backlights and similar things. But otoh this is a bit a high-grade bikeshed ... -Daniel > --- > drivers/gpu/drm/drm_crtc.c | 18 ++++++++++++++++-- > include/drm/drm_crtc.h | 21 +++++++++++++++++++++ > 2 files changed, 37 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index d2a6d958ca76..81641544ac3e 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -1036,13 +1036,24 @@ int drm_connector_register(struct drm_connector *connector) > > ret = drm_debugfs_connector_add(connector); > if (ret) { > - drm_sysfs_connector_remove(connector); > - return ret; > + goto err_sysfs; > + } > + > + if (connector->funcs->late_register) { > + ret = connector->funcs->late_register(connector); > + if (ret) > + goto err_debugfs; > } > > drm_mode_object_register(connector->dev, &connector->base); > > return 0; > + > +err_debugfs: > + drm_debugfs_connector_remove(connector); > +err_sysfs: > + drm_sysfs_connector_remove(connector); > + return ret; > } > EXPORT_SYMBOL(drm_connector_register); > > @@ -1054,6 +1065,9 @@ EXPORT_SYMBOL(drm_connector_register); > */ > void drm_connector_unregister(struct drm_connector *connector) > { > + if (connector->funcs->early_unregister) > + connector->funcs->early_unregister(connector); > + > drm_sysfs_connector_remove(connector); > drm_debugfs_connector_remove(connector); > } > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index d1559cd04e3d..0f7425444a44 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -926,6 +926,27 @@ struct drm_connector_funcs { > uint64_t val); > > /** > + * @late_register: > + * > + * Register the connector with userspace, called from > + * drm_connector_register. This should be called after driver > + * load during its registration phase. All actions such as registering > + * with auxiliary devices (such as drm_dp_aux_register) should be done > + * during this callback. > + */ > + int (*late_register)(struct drm_connector *connector); > + > + /** > + * @early_unregister: > + * > + * Unregister the connector with userspace, called from > + * drm_connector_unregister. This is called early in the driver > + * unload sequence to disable userspace access before data > + * structures are torndown. > + */ > + void (*early_unregister)(struct drm_connector *connector); > + > + /** > * @destroy: > * > * Clean up connector resources. This is called at driver unload time > -- > 2.8.1 >
On Mon, May 30, 2016 at 10:49:43AM +0200, Daniel Vetter wrote: > On Fri, May 27, 2016 at 03:00:35PM +0100, Chris Wilson wrote: > > If a driver wants to more precisely control its initialisation and in > > particular, defer registering its interfaces with userspace until after > > everything is setup, it also needs to defer registering the connectors. > > As some devices need more work during registration, add a callback so > > that drivers can do additional work if required for a connector. > > > > Correspondingly, we also require an unregister callback. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: dri-devel@lists.freedesktop.org > > Smells a bit too much like midlayer too me ;-) Imo perfectly fine to have > a driver loop besides the register_all which registers stuff like > backlights and similar things. But otoh this is a bit a high-grade > bikeshed ... We either: intel_connector_register() { drm_connector_register() intel_connector->register(); } intel_connector_unregister() { intel_connector->unregister(); drm_connector_unregister() } or drm_connector_register() { /*blah*/ connector->func->register(); } drm_connector_unregister() { connector->func->unregister(); /*blah*/ } We still get to drive the registration (hence why it doesn't smell too much like midlayer just a vfunc wrapper), just the second is a tad more generic and we aren't the only ones who require the calback. -Chris
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d2a6d958ca76..81641544ac3e 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1036,13 +1036,24 @@ int drm_connector_register(struct drm_connector *connector) ret = drm_debugfs_connector_add(connector); if (ret) { - drm_sysfs_connector_remove(connector); - return ret; + goto err_sysfs; + } + + if (connector->funcs->late_register) { + ret = connector->funcs->late_register(connector); + if (ret) + goto err_debugfs; } drm_mode_object_register(connector->dev, &connector->base); return 0; + +err_debugfs: + drm_debugfs_connector_remove(connector); +err_sysfs: + drm_sysfs_connector_remove(connector); + return ret; } EXPORT_SYMBOL(drm_connector_register); @@ -1054,6 +1065,9 @@ EXPORT_SYMBOL(drm_connector_register); */ void drm_connector_unregister(struct drm_connector *connector) { + if (connector->funcs->early_unregister) + connector->funcs->early_unregister(connector); + drm_sysfs_connector_remove(connector); drm_debugfs_connector_remove(connector); } diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index d1559cd04e3d..0f7425444a44 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -926,6 +926,27 @@ struct drm_connector_funcs { uint64_t val); /** + * @late_register: + * + * Register the connector with userspace, called from + * drm_connector_register. This should be called after driver + * load during its registration phase. All actions such as registering + * with auxiliary devices (such as drm_dp_aux_register) should be done + * during this callback. + */ + int (*late_register)(struct drm_connector *connector); + + /** + * @early_unregister: + * + * Unregister the connector with userspace, called from + * drm_connector_unregister. This is called early in the driver + * unload sequence to disable userspace access before data + * structures are torndown. + */ + void (*early_unregister)(struct drm_connector *connector); + + /** * @destroy: * * Clean up connector resources. This is called at driver unload time
If a driver wants to more precisely control its initialisation and in particular, defer registering its interfaces with userspace until after everything is setup, it also needs to defer registering the connectors. As some devices need more work during registration, add a callback so that drivers can do additional work if required for a connector. Correspondingly, we also require an unregister callback. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: dri-devel@lists.freedesktop.org --- drivers/gpu/drm/drm_crtc.c | 18 ++++++++++++++++-- include/drm/drm_crtc.h | 21 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-)