diff mbox

[1/3] drm: Add callbacks for late registering

Message ID 1466436177-25659-2-git-send-email-benjamin.gaignard@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Benjamin Gaignard June 20, 2016, 3:22 p.m. UTC 
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.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 drivers/gpu/drm/drm_drv.c | 42 ++++++++++++++++++++++++++
 include/drm/drm_crtc.h    | 77 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

Comments

Chris Wilson June 20, 2016, 3:30 p.m. UTC | #1 
On Mon, Jun 20, 2016 at 05:22:55PM +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.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
>  drivers/gpu/drm/drm_drv.c | 42 ++++++++++++++++++++++++++
>  include/drm/drm_crtc.h    | 77 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 119 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index c7101c0..b4f7f62 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -666,6 +666,9 @@ EXPORT_SYMBOL(drm_dev_unref);
>  int drm_dev_register(struct drm_device *dev, unsigned long flags)
>  {
>  	int ret;
> +	struct drm_crtc *crtc;
> +	struct drm_plane *plane;
> +	struct drm_encoder *encoder;
>  
>  	mutex_lock(&drm_global_mutex);
>  
> @@ -690,6 +693,27 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
>  	if (drm_core_check_feature(dev, DRIVER_MODESET))
>  		drm_connector_register_all(dev);

Look above for the ouch.

>  
> +	drm_for_each_crtc(crtc, dev) {
> +		if (crtc->funcs->late_register)
> +			ret = crtc->funcs->late_register(crtc);
> +		if (ret)
> +			goto err_minors;
> +	}

For the sake of conformity, is it worth adding drm_crtc_register_all()
et all?

Then a drm_modeset_register_all ? (i.e.
if (drm_core_check_feature(dev, DRIVER_MODESET))
	drm_modeset_register_all(dev);
	  -> { connector, encoder, plane, crtc )

Also, is the logical order 

drm_connector_register_all();
drm_encoder_register_all();
drm_plane_register_all();
drm_crtc_register_all();

or the other way around (basically working from output back to input, or
input to output)?
-Chris
Ville Syrjälä June 20, 2016, 3:54 p.m. UTC | #2 
On Mon, Jun 20, 2016 at 04:30:36PM +0100, Chris Wilson wrote:
> On Mon, Jun 20, 2016 at 05:22:55PM +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.
> > 
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> > ---
> >  drivers/gpu/drm/drm_drv.c | 42 ++++++++++++++++++++++++++
> >  include/drm/drm_crtc.h    | 77 +++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 119 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index c7101c0..b4f7f62 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -666,6 +666,9 @@ EXPORT_SYMBOL(drm_dev_unref);
> >  int drm_dev_register(struct drm_device *dev, unsigned long flags)
> >  {
> >  	int ret;
> > +	struct drm_crtc *crtc;
> > +	struct drm_plane *plane;
> > +	struct drm_encoder *encoder;
> >  
> >  	mutex_lock(&drm_global_mutex);
> >  
> > @@ -690,6 +693,27 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> >  	if (drm_core_check_feature(dev, DRIVER_MODESET))
> >  		drm_connector_register_all(dev);
> 
> Look above for the ouch.
> 
> >  
> > +	drm_for_each_crtc(crtc, dev) {
> > +		if (crtc->funcs->late_register)
> > +			ret = crtc->funcs->late_register(crtc);
> > +		if (ret)
> > +			goto err_minors;
> > +	}
> 
> For the sake of conformity, is it worth adding drm_crtc_register_all()
> et all?
> 
> Then a drm_modeset_register_all ? (i.e.
> if (drm_core_check_feature(dev, DRIVER_MODESET))
> 	drm_modeset_register_all(dev);
> 	  -> { connector, encoder, plane, crtc )
> 
> Also, is the logical order 
> 
> drm_connector_register_all();
> drm_encoder_register_all();
> drm_plane_register_all();
> drm_crtc_register_all();
> 
> or the other way around (basically working from output back to input, or
> input to output)?

input to output would make sense to me. Once the output appears
everything upstream of it would then be in place already.
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index c7101c0..b4f7f62 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -666,6 +666,9 @@  EXPORT_SYMBOL(drm_dev_unref);
 int drm_dev_register(struct drm_device *dev, unsigned long flags)
 {
 	int ret;
+	struct drm_crtc *crtc;
+	struct drm_plane *plane;
+	struct drm_encoder *encoder;
 
 	mutex_lock(&drm_global_mutex);
 
@@ -690,6 +693,27 @@  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_for_each_crtc(crtc, dev) {
+		if (crtc->funcs->late_register)
+			ret = crtc->funcs->late_register(crtc);
+		if (ret)
+			goto err_minors;
+	}
+
+	drm_for_each_plane(plane, dev) {
+		if (plane->funcs->late_register)
+			ret = plane->funcs->late_register(plane);
+		if (ret)
+			goto err_minors;
+	}
+
+	drm_for_each_encoder(encoder, dev) {
+		if (encoder->funcs->late_register)
+			ret = encoder->funcs->late_register(encoder);
+		if (ret)
+			goto err_minors;
+	}
+
 	ret = 0;
 	goto out_unlock;
 
@@ -717,12 +741,30 @@  EXPORT_SYMBOL(drm_dev_register);
 void drm_dev_unregister(struct drm_device *dev)
 {
 	struct drm_map_list *r_list, *list_temp;
+	struct drm_crtc *crtc;
+	struct drm_plane *plane;
+	struct drm_encoder *encoder;
 
 	drm_lastclose(dev);
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET))
 		drm_connector_unregister_all(dev);
 
+	drm_for_each_crtc(crtc, dev) {
+		if (crtc->funcs->early_unregister)
+			crtc->funcs->early_unregister(crtc);
+	}
+
+	drm_for_each_plane(plane, dev) {
+		if (plane->funcs->early_unregister)
+			plane->funcs->early_unregister(plane);
+	}
+
+	drm_for_each_encoder(encoder, dev) {
+		if (encoder->funcs->early_unregister)
+			encoder->funcs->early_unregister(encoder);
+	}
+
 	if (dev->driver->unload)
 		dev->driver->unload(dev);
 
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c273497..b4ab33f 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 {