Re: [PATCH v3 04/13] drm: bridge: Detach bridge from encoder at encoder cleanup time

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Daniel,

On Tuesday 29 Nov 2016 10:48:21 Daniel Vetter wrote:
> On Tue, Nov 29, 2016 at 11:04:34AM +0200, Laurent Pinchart wrote:
> > Most drivers that use bridges forgot to detach them at cleanup time.
> > Instead of fixing them one by one, detach the bridge in the core
> > drm_encoder_cleanup() function.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/drm_encoder.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> > index 5c067719164d..9c1f99646e0d 100644
> > --- a/drivers/gpu/drm/drm_encoder.c
> > +++ b/drivers/gpu/drm/drm_encoder.c
> > @@ -164,6 +164,9 @@ void drm_encoder_cleanup(struct drm_encoder *encoder)
> > 
> >  	 * the indices on the drm_encoder after us in the encoder_list.
> >  	 */
> > 
> > +	if (encoder->bridge)
> > +		drm_bridge_detach(encoder->bridge);
> 
> Means we bake in drm_bridge much more as a core thing, but I guess that's
> ok.
> 
> But there's 3 callers of drm_bridge_detach outside of the drm core, can't
> we remove them and drop the EXPORT_SYMBOL for drm_bridge_detach? What's
> it still needed for?

Agreed, I'll fix that.

> I think that cleanup should done in this patch here - drm_bridge_detach
> WARN_ONs if the bridge is already detached ...
>
> > +
> >  	drm_modeset_lock_all(dev);
> >  	drm_mode_object_unregister(dev, &encoder->base);
> >  	kfree(encoder->name);

-- 
Regards,

Laurent Pinchart