Re: [PATCH v5] media: imx: add mem2mem device

[Philipp Zabel <p.zabel@pengutronix.de>]

Hi Hans,

On Wed, 2018-12-05 at 19:50 +0100, Hans Verkuil wrote:
> On 12/05/2018 02:20 AM, Steve Longerbeam wrote:
> > Hi Hans, Philipp,
> > 
> > One comment on my side...
> > 
> > On 12/3/18 7:21 AM, Hans Verkuil wrote:
> > > <snip>
> > > > +void imx_media_mem2mem_device_unregister(struct imx_media_video_dev *vdev)
> > > > +{
> > > > +	struct mem2mem_priv *priv = to_mem2mem_priv(vdev);
> > > > +	struct video_device *vfd = priv->vdev.vfd;
> > > > +
> > > > +	mutex_lock(&priv->mutex);
> > > > +
> > > > +	if (video_is_registered(vfd)) {
> > > > +		video_unregister_device(vfd);
> > > > +		media_entity_cleanup(&vfd->entity);
> > > 
> > > Is this needed?
> > > 
> > > If this is to be part of the media controller, then I expect to see a call
> > > to v4l2_m2m_register_media_controller() somewhere.
> > > 
> > 
> > Yes, I agree there should be a call to 
> > v4l2_m2m_register_media_controller(). This driver does not connect with 
> > any of the imx-media entities, but calling it will at least make the 
> > mem2mem output/capture device entities (and processing entity) visible 
> > in the media graph.
> > 
> > Philipp, can you pick/squash the following from my media-tree github fork?
> > 
> > 6fa05f5170 ("media: imx: mem2mem: Add missing media-device header")
> > d355bf8b15 ("media: imx: Add missing unregister and remove of mem2mem 
> > device")
> > 6787a50cdc ("media: imx: mem2mem: Register with media control")
> > 
> > Steve
> > 
> 
> Why is this driver part of the imx driver? Since it doesn't connect with
> any of the imx-media entities, doesn't that mean that this is really a
> stand-alone driver?

It is part of the same hardware unit. The mem2mem tasks are scheduled to
the same IC that is also used for scaling in the capture path.

Since the mem2mem driver is at a higher abstraction level, it would be
possible to split it out into a separate platform device, but that felt
a bit artificial, and it would have to be registered from imx-media-dev
anyway.

regards
Philipp