| Message ID | 1472115874-6219-1-git-send-email-andrea.merello@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Aug 25, 2016 at 11:04:32AM +0200, Andrea Merello wrote: > 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. > > Few other kerneldocs fixes around there are included. > > 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> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> As discussed on the last round, I'll let Archit apply all 3 to drm-misc, so he can check them out too. -Daniel > --- > drivers/gpu/drm/drm_bridge.c | 29 +++++++++++++++++++++++++++-- > include/drm/drm_crtc.h | 26 +++++++++++++++++++++++++- > 2 files changed, 52 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index 2555430..4840466 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -98,11 +98,11 @@ EXPORT_SYMBOL(drm_bridge_remove); > * @dev: DRM device > * @bridge: bridge control structure > * > - * called by a kms driver to link one of our encoder/bridge to the given > + * Called by a kms driver to link one of our encoder/bridge to the given > * bridge. > * > * Note that setting up links between the bridge and our encoder/bridge > - * objects needs to be handled by the kms driver itself > + * objects needs to be handled by the kms driver itself. > * > * RETURNS: > * Zero on success, error code on failure > @@ -125,6 +125,31 @@ 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 3fa0275..bb214a1 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -1118,12 +1118,33 @@ struct drm_plane { > > /** > * struct drm_bridge_funcs - drm_bridge control functions > - * @attach: Called during drm_bridge_attach > */ > struct drm_bridge_funcs { > + /** > + * @attach: > + * > + * This callback is invoked whenever our bridge is being attached to a > + * &drm_encoder. > + * > + * The attach callback is optional. > + * > + * 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 from a > + * &drm_encoder. > + * > + * The detach callback is optional. > + */ > + void (*detach)(struct drm_bridge *bridge); > + > + /** > * @mode_fixup: > * > * This callback is used to validate and adjust a mode. The paramater > @@ -1137,6 +1158,8 @@ struct drm_bridge_funcs { > * this function passes all other callbacks must succeed for this > * configuration. > * > + * The mode_fixup callback is optional. > + * > * NOTE: > * > * This function is called in the check phase of atomic modesets, which > @@ -2426,6 +2449,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, > -- > 2.7.4 >
On 08/25/2016 05:46 PM, Daniel Vetter wrote: > On Thu, Aug 25, 2016 at 11:04:32AM +0200, Andrea Merello wrote: >> 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. >> >> Few other kerneldocs fixes around there are included. >> >> 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> > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > As discussed on the last round, I'll let Archit apply all 3 to drm-misc, > so he can check them out too. Pushed to drm-misc. Thanks, Archit > -Daniel >> --- >> drivers/gpu/drm/drm_bridge.c | 29 +++++++++++++++++++++++++++-- >> include/drm/drm_crtc.h | 26 +++++++++++++++++++++++++- >> 2 files changed, 52 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c >> index 2555430..4840466 100644 >> --- a/drivers/gpu/drm/drm_bridge.c >> +++ b/drivers/gpu/drm/drm_bridge.c >> @@ -98,11 +98,11 @@ EXPORT_SYMBOL(drm_bridge_remove); >> * @dev: DRM device >> * @bridge: bridge control structure >> * >> - * called by a kms driver to link one of our encoder/bridge to the given >> + * Called by a kms driver to link one of our encoder/bridge to the given >> * bridge. >> * >> * Note that setting up links between the bridge and our encoder/bridge >> - * objects needs to be handled by the kms driver itself >> + * objects needs to be handled by the kms driver itself. >> * >> * RETURNS: >> * Zero on success, error code on failure >> @@ -125,6 +125,31 @@ 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 3fa0275..bb214a1 100644 >> --- a/include/drm/drm_crtc.h >> +++ b/include/drm/drm_crtc.h >> @@ -1118,12 +1118,33 @@ struct drm_plane { >> >> /** >> * struct drm_bridge_funcs - drm_bridge control functions >> - * @attach: Called during drm_bridge_attach >> */ >> struct drm_bridge_funcs { >> + /** >> + * @attach: >> + * >> + * This callback is invoked whenever our bridge is being attached to a >> + * &drm_encoder. >> + * >> + * The attach callback is optional. >> + * >> + * 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 from a >> + * &drm_encoder. >> + * >> + * The detach callback is optional. >> + */ >> + void (*detach)(struct drm_bridge *bridge); >> + >> + /** >> * @mode_fixup: >> * >> * This callback is used to validate and adjust a mode. The paramater >> @@ -1137,6 +1158,8 @@ struct drm_bridge_funcs { >> * this function passes all other callbacks must succeed for this >> * configuration. >> * >> + * The mode_fixup callback is optional. >> + * >> * NOTE: >> * >> * This function is called in the check phase of atomic modesets, which >> @@ -2426,6 +2449,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, >> -- >> 2.7.4 >> >
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c index 2555430..4840466 100644 --- a/drivers/gpu/drm/drm_bridge.c +++ b/drivers/gpu/drm/drm_bridge.c @@ -98,11 +98,11 @@ EXPORT_SYMBOL(drm_bridge_remove); * @dev: DRM device * @bridge: bridge control structure * - * called by a kms driver to link one of our encoder/bridge to the given + * Called by a kms driver to link one of our encoder/bridge to the given * bridge. * * Note that setting up links between the bridge and our encoder/bridge - * objects needs to be handled by the kms driver itself + * objects needs to be handled by the kms driver itself. * * RETURNS: * Zero on success, error code on failure @@ -125,6 +125,31 @@ 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 3fa0275..bb214a1 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1118,12 +1118,33 @@ struct drm_plane { /** * struct drm_bridge_funcs - drm_bridge control functions - * @attach: Called during drm_bridge_attach */ struct drm_bridge_funcs { + /** + * @attach: + * + * This callback is invoked whenever our bridge is being attached to a + * &drm_encoder. + * + * The attach callback is optional. + * + * 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 from a + * &drm_encoder. + * + * The detach callback is optional. + */ + void (*detach)(struct drm_bridge *bridge); + + /** * @mode_fixup: * * This callback is used to validate and adjust a mode. The paramater @@ -1137,6 +1158,8 @@ struct drm_bridge_funcs { * this function passes all other callbacks must succeed for this * configuration. * + * The mode_fixup callback is optional. + * * NOTE: * * This function is called in the check phase of atomic modesets, which @@ -2426,6 +2449,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,
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. Few other kerneldocs fixes around there are included. 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 | 29 +++++++++++++++++++++++++++-- include/drm/drm_crtc.h | 26 +++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-)