Re: [PATCH 5/8] drm/imx: parallel-display: use drm managed resources

[daniel@ffwll.ch]

On Wed, Jul 22, 2020 at 04:01:53PM +0200, Philipp Zabel wrote:
> On Wed, 2020-07-22 at 15:30 +0200, Philipp Zabel wrote:
> [...]
> > and use drmm_add_action_or_reset() to make sure drm_encoder_cleanup() is
> > called before the memory is freed.
> [...]
> > @@ -259,6 +259,13 @@ static const struct drm_bridge_funcs imx_pd_bridge_funcs = {
> >  	.atomic_get_output_bus_fmts = imx_pd_bridge_atomic_get_output_bus_fmts,
> >  };
> >  
> > +static void imx_pd_encoder_cleanup(struct drm_device *drm, void *ptr)
> > +{
> > +	struct drm_encoder *encoder = ptr;
> > +
> > +	drm_encoder_cleanup(encoder);
> > +}
> > +
> >  static int imx_pd_register(struct drm_device *drm,
> >  	struct imx_parallel_display *imxpd)
> >  {
> > @@ -276,7 +283,13 @@ static int imx_pd_register(struct drm_device *drm,
> >  	 */
> >  	imxpd->connector.dpms = DRM_MODE_DPMS_OFF;
> >  
> > -	drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_NONE);
> > +	ret = drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_NONE);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = drmm_add_action_or_reset(drm, imx_pd_encoder_cleanup, encoder);
> > +	if (ret)
> > +		return ret;
> 
> This is only required because this is a component driver: our
> drmm_kzalloc() is called after drmm_mode_config_init(), so we can't rely
> on drm_mode_config_init_release() for cleanup. That is only called after
> drmres already freed our memory.

Yeah I know about the inversion, which is why I haven't yet started typing
the mass conversion for all the drm objects. I think the explicit
drmm_add_action_or_reset is indeed the way to go, except we probably want
some helpers to wrap the allocation, drm_foo_init and adding the reset
action all into one thing (plus you can stuff the reset action into the
allocation instead of the kfree action only, even nicer that way).

But that's maybe for later, and good to have some examples in drivers
already converted over as guidance.

On the series: Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

But way too late for solid review :-)

Cheers, Daniel

> 
> regards
> Philipp
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel