| Message ID | 1466514580-15194-2-git-send-email-benjamin.gaignard@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 21, 2016 at 03:09:38PM +0200, Benjamin Gaignard wrote: > Like what has been done for connectors add callbacks on encoder, > crtc and plane to let driver do actions after drm device registration. > > Correspondingly, add callbacks called before unregister drm device. > > version 2: > add drm_modeset_register_all() and drm_modeset_unregister_all() > to centralize all calls > > version 3: > in error case unwind registers in drm_modeset_register_all > fix uninitialed return value > inverse order of unregistration in drm_modeset_unregister_all > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> > --- > drivers/gpu/drm/drm_crtc.c | 132 +++++++++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_drv.c | 4 +- > include/drm/drm_crtc.h | 79 +++++++++++++++++++++++++++ > 3 files changed, 213 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index e7c862b..c078bc4 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -608,6 +608,31 @@ static unsigned int drm_num_crtcs(struct drm_device *dev) > return num; > } > > +static int drm_crtc_register_all(struct drm_device *dev) > +{ > + struct drm_crtc *crtc; > + int ret = 0; > + > + drm_for_each_crtc(crtc, dev) { > + if (crtc->funcs->late_register) > + ret = crtc->funcs->late_register(crtc); > + if (ret) What happened to the unwind here? > + return ret; > + } > + > + return 0; > +} > +/** > + * drm_modeset_register_all - do late registration > + * @dev: drm device > + * > + * This function call late_register callback for all planes, > + * crcts, encoders and connectors > + * > + * Returns: > + * Zero on success, erro code on failure > + */ > +int drm_modeset_register_all(struct drm_device *dev) > +{ > + int ret; > + > + ret = drm_plane_register_all(dev); > + if (ret) > + goto err_plane; > + > + ret = drm_crtc_register_all(dev); > + if (ret) > + goto err_crtc; > + > + ret = drm_encoder_register_all(dev); > + if (ret) > + goto err_encoder; > + > + ret = drm_connector_register_all(dev); > + if (ret) > + goto err_connector; > + > + return 0; > + > +err_connector: > + drm_encoder_unregister_all(dev); The name here should be what we are about to free. That just makes it a bit easier to change the sequence later (if adding new stags). Looks good enough though, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Tue, Jun 21, 2016 at 03:09:38PM +0200, Benjamin Gaignard wrote: > Like what has been done for connectors add callbacks on encoder, > crtc and plane to let driver do actions after drm device registration. > > Correspondingly, add callbacks called before unregister drm device. > > version 2: > add drm_modeset_register_all() and drm_modeset_unregister_all() > to centralize all calls > > version 3: > in error case unwind registers in drm_modeset_register_all > fix uninitialed return value > inverse order of unregistration in drm_modeset_unregister_all > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> > --- > drivers/gpu/drm/drm_crtc.c | 132 +++++++++++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_drv.c | 4 +- > include/drm/drm_crtc.h | 79 +++++++++++++++++++++++++++ > 3 files changed, 213 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index e7c862b..c078bc4 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -608,6 +608,31 @@ static unsigned int drm_num_crtcs(struct drm_device *dev) > return num; > } > > +static int drm_crtc_register_all(struct drm_device *dev) > +{ > + struct drm_crtc *crtc; > + int ret = 0; > + > + drm_for_each_crtc(crtc, dev) { > + if (crtc->funcs->late_register) > + ret = crtc->funcs->late_register(crtc); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static void drm_crtc_unregister_all(struct drm_device *dev) > +{ > + struct drm_crtc *crtc; > + > + drm_for_each_crtc(crtc, dev) { > + if (crtc->funcs->early_unregister) > + crtc->funcs->early_unregister(crtc); > + } > +} > + > /** > * drm_crtc_init_with_planes - Initialise a new CRTC object with > * specified primary and cursor planes. > @@ -1102,6 +1127,31 @@ void drm_connector_unregister_all(struct drm_device *dev) > } > EXPORT_SYMBOL(drm_connector_unregister_all); > > +static int drm_encoder_register_all(struct drm_device *dev) > +{ > + struct drm_encoder *encoder; > + int ret = 0; > + > + drm_for_each_encoder(encoder, dev) { > + if (encoder->funcs->late_register) > + ret = encoder->funcs->late_register(encoder); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static void drm_encoder_unregister_all(struct drm_device *dev) > +{ > + struct drm_encoder *encoder; > + > + drm_for_each_encoder(encoder, dev) { > + if (encoder->funcs->early_unregister) > + encoder->funcs->early_unregister(encoder); > + } > +} > + > /** > * drm_encoder_init - Init a preallocated encoder > * @dev: drm device > @@ -1290,6 +1340,31 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, > } > EXPORT_SYMBOL(drm_universal_plane_init); > > +static int drm_plane_register_all(struct drm_device *dev) > +{ > + struct drm_plane *plane; > + int ret = 0; > + > + drm_for_each_plane(plane, dev) { > + if (plane->funcs->late_register) > + ret = plane->funcs->late_register(plane); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > +static void drm_plane_unregister_all(struct drm_device *dev) > +{ > + struct drm_plane *plane; > + > + drm_for_each_plane(plane, dev) { > + if (plane->funcs->early_unregister) > + plane->funcs->early_unregister(plane); > + } > +} > + > /** > * drm_plane_init - Initialize a legacy plane > * @dev: DRM device > @@ -1412,6 +1487,63 @@ void drm_plane_force_disable(struct drm_plane *plane) > } > EXPORT_SYMBOL(drm_plane_force_disable); > > +/** > + * drm_modeset_register_all - do late registration > + * @dev: drm device > + * > + * This function call late_register callback for all planes, > + * crcts, encoders and connectors > + * > + * Returns: > + * Zero on success, erro code on failure > + */ We don't document drm-internal functions, it just confuses the docs (which are aimed at driver writers). Since the comment doesn't explain much of what the code does, please remove. Same for unregister below. > +int drm_modeset_register_all(struct drm_device *dev) > +{ > + int ret; > + > + ret = drm_plane_register_all(dev); > + if (ret) > + goto err_plane; > + > + ret = drm_crtc_register_all(dev); > + if (ret) > + goto err_crtc; > + > + ret = drm_encoder_register_all(dev); > + if (ret) > + goto err_encoder; > + > + ret = drm_connector_register_all(dev); > + if (ret) > + goto err_connector; > + > + return 0; > + > +err_connector: > + drm_encoder_unregister_all(dev); > +err_encoder: > + drm_crtc_unregister_all(dev); > +err_crtc: > + drm_plane_unregister_all(dev); > +err_plane: > + return ret; > +} > + > +/** > + * drm_modeset_unregister_all - do early unregistration > + * @dev: drm device > + * > + * This function call early_unregister callbacks for all > + * connectors, encoders, crtcs and planes > + */ > +void drm_modeset_unregister_all(struct drm_device *dev) > +{ > + drm_connector_unregister_all(dev); > + drm_encoder_unregister_all(dev); > + drm_crtc_unregister_all(dev); > + drm_plane_unregister_all(dev); > +} > + > static int drm_mode_create_standard_properties(struct drm_device *dev) > { > struct drm_property *prop; > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c > index c7101c0..53eadca 100644 > --- a/drivers/gpu/drm/drm_drv.c > +++ b/drivers/gpu/drm/drm_drv.c > @@ -688,7 +688,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) > } > > if (drm_core_check_feature(dev, DRIVER_MODESET)) > - drm_connector_register_all(dev); > + drm_modeset_register_all(dev); > > ret = 0; > goto out_unlock; > @@ -721,7 +721,7 @@ void drm_dev_unregister(struct drm_device *dev) > drm_lastclose(dev); > > if (drm_core_check_feature(dev, DRIVER_MODESET)) > - drm_connector_unregister_all(dev); > + drm_modeset_unregister_all(dev); > > if (dev->driver->unload) > dev->driver->unload(dev); > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index c273497..d0f2256b 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -704,6 +704,32 @@ struct drm_crtc_funcs { > const struct drm_crtc_state *state, > struct drm_property *property, > uint64_t *val); > + > + /** > + * @late_register: > + * > + * This optional hook can be used to register additional userspace > + * interfaces attached to the crtc like debugfs interfaces. > + * It is called late in the driver load sequence from drm_dev_register(). > + * Everything added from this callback should be unregistered in > + * the early_unregister callback. > + * > + * Returns: > + * > + * 0 on success, or a negative error code on failure. > + */ > + int (*late_register)(struct drm_crtc *crtc); > + > + /** > + * @early_unregister: > + * > + * This optional hook should be used to unregister the additional > + * userspace interfaces attached to the crtc from > + * late_unregister(). It is called from drm_dev_unregister(), > + * early in the driver unload sequence to disable userspace access > + * before data structures are torndown. > + */ > + void (*early_unregister)(struct drm_crtc *crtc); > }; > > /** > @@ -1127,6 +1153,32 @@ struct drm_encoder_funcs { > * hotplugged in DRM. > */ > void (*destroy)(struct drm_encoder *encoder); > + > + /** > + * @late_register: > + * > + * This optional hook can be used to register additional userspace > + * interfaces attached to the encoder like debugfs interfaces. > + * It is called late in the driver load sequence from drm_dev_register(). > + * Everything added from this callback should be unregistered in > + * the early_unregister callback. > + * > + * Returns: > + * > + * 0 on success, or a negative error code on failure. > + */ > + int (*late_register)(struct drm_encoder *encoder); > + > + /** > + * @early_unregister: > + * > + * This optional hook should be used to unregister the additional > + * userspace interfaces attached to the encoder from > + * late_unregister(). It is called from drm_dev_unregister(), > + * early in the driver unload sequence to disable userspace access > + * before data structures are torndown. > + */ > + void (*early_unregister)(struct drm_encoder *encoder); > }; > > #define DRM_CONNECTOR_MAX_ENCODER 3 > @@ -1570,6 +1622,31 @@ struct drm_plane_funcs { > const struct drm_plane_state *state, > struct drm_property *property, > uint64_t *val); > + /** > + * @late_register: > + * > + * This optional hook can be used to register additional userspace > + * interfaces attached to the plane like debugfs interfaces. > + * It is called late in the driver load sequence from drm_dev_register(). > + * Everything added from this callback should be unregistered in > + * the early_unregister callback. > + * > + * Returns: > + * > + * 0 on success, or a negative error code on failure. > + */ > + int (*late_register)(struct drm_plane *plane); > + > + /** > + * @early_unregister: > + * > + * This optional hook should be used to unregister the additional > + * userspace interfaces attached to the plane from > + * late_unregister(). It is called from drm_dev_unregister(), > + * early in the driver unload sequence to disable userspace access > + * before data structures are torndown. > + */ > + void (*early_unregister)(struct drm_plane *plane); > }; > > enum drm_plane_type { > @@ -2514,6 +2591,8 @@ static inline unsigned drm_connector_index(struct drm_connector *connector) > /* helpers to {un}register all connectors from sysfs for device */ > extern int drm_connector_register_all(struct drm_device *dev); > extern void drm_connector_unregister_all(struct drm_device *dev); > +int drm_modeset_register_all(struct drm_device *dev); > +void drm_modeset_unregister_all(struct drm_device *dev); Please move these to drm_crtc_internal.h, it's not part of the driver api. -Daniel > > extern int drm_bridge_add(struct drm_bridge *bridge); > extern void drm_bridge_remove(struct drm_bridge *bridge); > -- > 1.9.1 >
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index e7c862b..c078bc4 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -608,6 +608,31 @@ static unsigned int drm_num_crtcs(struct drm_device *dev) return num; } +static int drm_crtc_register_all(struct drm_device *dev) +{ + struct drm_crtc *crtc; + int ret = 0; + + drm_for_each_crtc(crtc, dev) { + if (crtc->funcs->late_register) + ret = crtc->funcs->late_register(crtc); + if (ret) + return ret; + } + + return 0; +} + +static void drm_crtc_unregister_all(struct drm_device *dev) +{ + struct drm_crtc *crtc; + + drm_for_each_crtc(crtc, dev) { + if (crtc->funcs->early_unregister) + crtc->funcs->early_unregister(crtc); + } +} + /** * drm_crtc_init_with_planes - Initialise a new CRTC object with * specified primary and cursor planes. @@ -1102,6 +1127,31 @@ void drm_connector_unregister_all(struct drm_device *dev) } EXPORT_SYMBOL(drm_connector_unregister_all); +static int drm_encoder_register_all(struct drm_device *dev) +{ + struct drm_encoder *encoder; + int ret = 0; + + drm_for_each_encoder(encoder, dev) { + if (encoder->funcs->late_register) + ret = encoder->funcs->late_register(encoder); + if (ret) + return ret; + } + + return 0; +} + +static void drm_encoder_unregister_all(struct drm_device *dev) +{ + struct drm_encoder *encoder; + + drm_for_each_encoder(encoder, dev) { + if (encoder->funcs->early_unregister) + encoder->funcs->early_unregister(encoder); + } +} + /** * drm_encoder_init - Init a preallocated encoder * @dev: drm device @@ -1290,6 +1340,31 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane, } EXPORT_SYMBOL(drm_universal_plane_init); +static int drm_plane_register_all(struct drm_device *dev) +{ + struct drm_plane *plane; + int ret = 0; + + drm_for_each_plane(plane, dev) { + if (plane->funcs->late_register) + ret = plane->funcs->late_register(plane); + if (ret) + return ret; + } + + return 0; +} + +static void drm_plane_unregister_all(struct drm_device *dev) +{ + struct drm_plane *plane; + + drm_for_each_plane(plane, dev) { + if (plane->funcs->early_unregister) + plane->funcs->early_unregister(plane); + } +} + /** * drm_plane_init - Initialize a legacy plane * @dev: DRM device @@ -1412,6 +1487,63 @@ void drm_plane_force_disable(struct drm_plane *plane) } EXPORT_SYMBOL(drm_plane_force_disable); +/** + * drm_modeset_register_all - do late registration + * @dev: drm device + * + * This function call late_register callback for all planes, + * crcts, encoders and connectors + * + * Returns: + * Zero on success, erro code on failure + */ +int drm_modeset_register_all(struct drm_device *dev) +{ + int ret; + + ret = drm_plane_register_all(dev); + if (ret) + goto err_plane; + + ret = drm_crtc_register_all(dev); + if (ret) + goto err_crtc; + + ret = drm_encoder_register_all(dev); + if (ret) + goto err_encoder; + + ret = drm_connector_register_all(dev); + if (ret) + goto err_connector; + + return 0; + +err_connector: + drm_encoder_unregister_all(dev); +err_encoder: + drm_crtc_unregister_all(dev); +err_crtc: + drm_plane_unregister_all(dev); +err_plane: + return ret; +} + +/** + * drm_modeset_unregister_all - do early unregistration + * @dev: drm device + * + * This function call early_unregister callbacks for all + * connectors, encoders, crtcs and planes + */ +void drm_modeset_unregister_all(struct drm_device *dev) +{ + drm_connector_unregister_all(dev); + drm_encoder_unregister_all(dev); + drm_crtc_unregister_all(dev); + drm_plane_unregister_all(dev); +} + static int drm_mode_create_standard_properties(struct drm_device *dev) { struct drm_property *prop; diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index c7101c0..53eadca 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -688,7 +688,7 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags) } if (drm_core_check_feature(dev, DRIVER_MODESET)) - drm_connector_register_all(dev); + drm_modeset_register_all(dev); ret = 0; goto out_unlock; @@ -721,7 +721,7 @@ void drm_dev_unregister(struct drm_device *dev) drm_lastclose(dev); if (drm_core_check_feature(dev, DRIVER_MODESET)) - drm_connector_unregister_all(dev); + drm_modeset_unregister_all(dev); if (dev->driver->unload) dev->driver->unload(dev); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index c273497..d0f2256b 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -704,6 +704,32 @@ struct drm_crtc_funcs { const struct drm_crtc_state *state, struct drm_property *property, uint64_t *val); + + /** + * @late_register: + * + * This optional hook can be used to register additional userspace + * interfaces attached to the crtc like debugfs interfaces. + * It is called late in the driver load sequence from drm_dev_register(). + * Everything added from this callback should be unregistered in + * the early_unregister callback. + * + * Returns: + * + * 0 on success, or a negative error code on failure. + */ + int (*late_register)(struct drm_crtc *crtc); + + /** + * @early_unregister: + * + * This optional hook should be used to unregister the additional + * userspace interfaces attached to the crtc from + * late_unregister(). It is called from drm_dev_unregister(), + * early in the driver unload sequence to disable userspace access + * before data structures are torndown. + */ + void (*early_unregister)(struct drm_crtc *crtc); }; /** @@ -1127,6 +1153,32 @@ struct drm_encoder_funcs { * hotplugged in DRM. */ void (*destroy)(struct drm_encoder *encoder); + + /** + * @late_register: + * + * This optional hook can be used to register additional userspace + * interfaces attached to the encoder like debugfs interfaces. + * It is called late in the driver load sequence from drm_dev_register(). + * Everything added from this callback should be unregistered in + * the early_unregister callback. + * + * Returns: + * + * 0 on success, or a negative error code on failure. + */ + int (*late_register)(struct drm_encoder *encoder); + + /** + * @early_unregister: + * + * This optional hook should be used to unregister the additional + * userspace interfaces attached to the encoder from + * late_unregister(). It is called from drm_dev_unregister(), + * early in the driver unload sequence to disable userspace access + * before data structures are torndown. + */ + void (*early_unregister)(struct drm_encoder *encoder); }; #define DRM_CONNECTOR_MAX_ENCODER 3 @@ -1570,6 +1622,31 @@ struct drm_plane_funcs { const struct drm_plane_state *state, struct drm_property *property, uint64_t *val); + /** + * @late_register: + * + * This optional hook can be used to register additional userspace + * interfaces attached to the plane like debugfs interfaces. + * It is called late in the driver load sequence from drm_dev_register(). + * Everything added from this callback should be unregistered in + * the early_unregister callback. + * + * Returns: + * + * 0 on success, or a negative error code on failure. + */ + int (*late_register)(struct drm_plane *plane); + + /** + * @early_unregister: + * + * This optional hook should be used to unregister the additional + * userspace interfaces attached to the plane from + * late_unregister(). It is called from drm_dev_unregister(), + * early in the driver unload sequence to disable userspace access + * before data structures are torndown. + */ + void (*early_unregister)(struct drm_plane *plane); }; enum drm_plane_type { @@ -2514,6 +2591,8 @@ static inline unsigned drm_connector_index(struct drm_connector *connector) /* helpers to {un}register all connectors from sysfs for device */ extern int drm_connector_register_all(struct drm_device *dev); extern void drm_connector_unregister_all(struct drm_device *dev); +int drm_modeset_register_all(struct drm_device *dev); +void drm_modeset_unregister_all(struct drm_device *dev); extern int drm_bridge_add(struct drm_bridge *bridge); extern void drm_bridge_remove(struct drm_bridge *bridge);
Like what has been done for connectors add callbacks on encoder, crtc and plane to let driver do actions after drm device registration. Correspondingly, add callbacks called before unregister drm device. version 2: add drm_modeset_register_all() and drm_modeset_unregister_all() to centralize all calls version 3: in error case unwind registers in drm_modeset_register_all fix uninitialed return value inverse order of unregistration in drm_modeset_unregister_all Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org> --- drivers/gpu/drm/drm_crtc.c | 132 +++++++++++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_drv.c | 4 +- include/drm/drm_crtc.h | 79 +++++++++++++++++++++++++++ 3 files changed, 213 insertions(+), 2 deletions(-)