Up to now, once a bridge has been attached to a DRM device, it cannot be undone.
In particular you couldn't rmmod/insmod a DRM driver that uses a bridge, because the bridge would remain bound to the first (dead) driver instance.
This patch fixes this by introducing drm_encoder_detach() and a ->detach callback in drm_bridge_funcs for the bridge to be notified about detaches.
It's DRM/KMS driver responsibility to call drm_encoder_detach().
While adding the bridge detach callback, with its kerneldoc, I also added kerneldoc for attach callback.
Suggested-by: Daniel Vetter daniel@ffwll.ch Suggested-by: Lucas Stach l.stach@pengutronix.de Signed-off-by: Andrea Merello andrea.merello@gmail.com Cc: Archit Taneja architt@codeaurora.org Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: Lucas Stach l.stach@pengutronix.de --- drivers/gpu/drm/drm_bridge.c | 26 ++++++++++++++++++++++++++ include/drm/drm_crtc.h | 17 +++++++++++++++++ 2 files changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 2555430..a77b3e0 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -125,6 +125,32 @@ int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge) EXPORT_SYMBOL(drm_bridge_attach);
/** + * drm_bridge_detach - deassociate given bridge from its DRM device + * + * @bridge: bridge control structure + * + * called by a kms driver to unlink the given bridge from its DRM device + * + * Note that tearing down links between the bridge and our encoder/bridge + * objects needs to be handled by the kms driver itself + * + */ +void drm_bridge_detach(struct drm_bridge *bridge) +{ + if (WARN_ON(!bridge)) + return; + + if (WARN_ON(!bridge->dev)) + return; + + if (bridge->funcs->detach) + bridge->funcs->detach(bridge); + + bridge->dev = NULL; +} +EXPORT_SYMBOL(drm_bridge_detach); + +/** * DOC: bridge callbacks * * The &drm_bridge_funcs ops are populated by the bridge driver. The DRM diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index b618b50..5e25e23 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1765,9 +1765,25 @@ struct drm_plane { * @attach: Called during drm_bridge_attach */ struct drm_bridge_funcs { + /** + * @attach: + * + * This callback is invoked whenever our bridge is being attached. + * + * RETURNS: + * + * Zero on success, error code on failure + */ int (*attach)(struct drm_bridge *bridge);
/** + * @detach: + * + * This callback is invoked whenever our bridge is being detached. + */ + void (*detach)(struct drm_bridge *bridge); + + /** * @mode_fixup: * * This callback is used to validate and adjust a mode. The paramater @@ -3196,6 +3212,7 @@ extern int drm_bridge_add(struct drm_bridge *bridge); extern void drm_bridge_remove(struct drm_bridge *bridge); extern struct drm_bridge *of_drm_find_bridge(struct device_node *np); extern int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge); +extern void drm_bridge_detach(struct drm_bridge *bridge);
bool drm_bridge_mode_fixup(struct drm_bridge *bridge, const struct drm_display_mode *mode,
drm_simple_display_pipe_init() pretends to attach a connector to the display pipe.
In case a drm bridge has to be used, then it's the bridge that takes care of connectors.
This patch makes the connector parameter optional for drm_simple_display_pipe_init(), so that a drm bridge could handle connector by itself later.
Signed-off-by: Andrea Merello andrea.merello@gmail.com Cc: David Airlie airlied@linux.ie Cc: Noralf Trønnes noralf@tronnes.org Cc: Daniel Vetter daniel@ffwll.ch --- drivers/gpu/drm/drm_simple_kms_helper.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index aeb7a75..d07cdb8 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -146,10 +146,13 @@ static const struct drm_plane_funcs drm_simple_kms_plane_funcs = { * @funcs: callbacks for the display pipe (optional) * @formats: array of supported formats (%DRM_FORMAT_*) * @format_count: number of elements in @formats - * @connector: connector to attach and register + * @connector: connector to attach and register (optional) * * Sets up a display pipeline which consist of a really simple - * plane-crtc-encoder pipe coupled with the provided connector. + * plane-crtc-encoder pipe. + * If a connector is supplied, the pipe will be coupled with the provided + * connector. You may supply a NULL connector when using drm bridges, that + * handle connectors themselves (see drm_simple_display_pipe_bridge_attach()). * Teardown of a simple display pipe is all handled automatically by the drm * core through calling drm_mode_config_cleanup(). Drivers afterwards need to * release the memory for the structure themselves. @@ -188,7 +191,7 @@ int drm_simple_display_pipe_init(struct drm_device *dev, encoder->possible_crtcs = 1 << drm_crtc_index(crtc); ret = drm_encoder_init(dev, encoder, &drm_simple_kms_encoder_funcs, DRM_MODE_ENCODER_NONE, NULL); - if (ret) + if (ret || !connector) return ret;
return drm_mode_connector_attach_encoder(connector, encoder);
On Wed, Aug 24, 2016 at 02:06:00PM +0200, Andrea Merello wrote:
Nit: Empty comment line if you want to start a new paragraph. Otherwise pls reflow the comment to make it use the full width.
With that Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
I guess I'll let Archit apply the entire series once it's all ready. -Daniel
Introduce drm_simple_display_pipe_attach_bridge() and drm_simple_display_pipe_detach_bridge() in order to make it possible to use drm encoders with the simple display pipes managed by simple_kms_helpers
Suggested-by: Daniel Vetter daniel@ffwll.ch Signed-off-by: Andrea Merello andrea.merello@gmail.com Cc: Noralf Trønnes noralf@tronnes.org Cc: Daniel Vetter daniel@ffwll.ch Cc: David Airlie airlied@linux.ie --- drivers/gpu/drm/drm_simple_kms_helper.c | 42 ++++++++++++++++++++++++++++++++- include/drm/drm_simple_kms_helper.h | 5 ++++ 2 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index d07cdb8..dfa3f1b 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -140,6 +140,46 @@ static const struct drm_plane_funcs drm_simple_kms_plane_funcs = { };
/** + * drm_simple_display_pipe_attach_bridge - Attach a bridge to the display pipe + * @pipe: simple display pipe object + * @bridge: bridge to attach + * + * Makes it possible to still use the drm_simple_display_pipe helpers when + * a DRM bridge has to be used. + * Note that you probably want to initialize the pipe by passing a NULL + * connector to drm_simple_display_pipe_init() + * + * Returns: + * Zero on success, negative error code on failure. + */ +int drm_simple_display_pipe_attach_bridge(struct drm_simple_display_pipe *pipe, + struct drm_bridge *bridge) +{ + bridge->encoder = &pipe->encoder; + pipe->encoder.bridge = bridge; + return drm_bridge_attach(pipe->encoder.dev, bridge); +} +EXPORT_SYMBOL(drm_simple_display_pipe_attach_bridge); + +/** + * drm_simple_display_pipe_detach_bridge - Detach the bridge from the display pipe + * @pipe: simple display pipe object + * + * Detaches the drm bridge previously attached with + * drm_simple_display_pipe_attach_bridge() + * + */ +void drm_simple_display_pipe_detach_bridge(struct drm_simple_display_pipe *pipe) +{ + if (WARN_ON(!pipe->encoder.bridge)) + return; + + drm_bridge_detach(pipe->encoder.bridge); + pipe->encoder.bridge = NULL; +} +EXPORT_SYMBOL(drm_simple_display_pipe_detach_bridge); + +/** * drm_simple_display_pipe_init - Initialize a simple display pipeline * @dev: DRM device * @pipe: simple display pipe object to initialize @@ -152,7 +192,7 @@ static const struct drm_plane_funcs drm_simple_kms_plane_funcs = { * plane-crtc-encoder pipe. * If a connector is supplied, the pipe will be coupled with the provided * connector. You may supply a NULL connector when using drm bridges, that - * handle connectors themselves (see drm_simple_display_pipe_bridge_attach()). + * handle connectors themselves (see drm_simple_display_pipe_attach_bridge()). * Teardown of a simple display pipe is all handled automatically by the drm * core through calling drm_mode_config_cleanup(). Drivers afterwards need to * release the memory for the structure themselves. diff --git a/include/drm/drm_simple_kms_helper.h b/include/drm/drm_simple_kms_helper.h index 2690397..5245d1f 100644 --- a/include/drm/drm_simple_kms_helper.h +++ b/include/drm/drm_simple_kms_helper.h @@ -85,6 +85,11 @@ struct drm_simple_display_pipe { const struct drm_simple_display_pipe_funcs *funcs; };
+int drm_simple_display_pipe_attach_bridge(struct drm_simple_display_pipe *pipe, + struct drm_bridge *bridge); + +void drm_simple_display_pipe_detach_bridge(struct drm_simple_display_pipe *pipe); + int drm_simple_display_pipe_init(struct drm_device *dev, struct drm_simple_display_pipe *pipe, const struct drm_simple_display_pipe_funcs *funcs,
On Wed, Aug 24, 2016 at 02:06:01PM +0200, Andrea Merello wrote:
Same nit: New paragraphs need and empty line, or reflow to make it one that uses the full width.
Empty line.
With the nits addressed: Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
On Wed, Aug 24, 2016 at 02:05:59PM +0200, Andrea Merello wrote:
Empty line, and we do full sentences (including capitalization and punctuation) in the full kernel-doc paragraphs.
... attached to a &drm_encoder. Similar for @detach.
Also I think we should mentation that these callbacks are optional, like with the others. Just noticed that that remark is missing for @mode_fixup, can you pls add it too?
lgtm otherwise. -Daniel
dri-devel@lists.freedesktop.org
-
Andrea Merello -
Daniel Vetter