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

[Steve Longerbeam <slongerbeam@gmail.com>]

Hi Hans,

On 12/6/18 4:32 AM, Hans Verkuil wrote:
> On 12/06/18 00:13, Steve Longerbeam wrote:
>>
>> On 12/5/18 10:50 AM, 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 basically a stand-alone m2m driver, but it makes use of some
>> imx-media utility functions like imx_media_enum_format(). Also making it
>> a true stand-alone driver would require creating a second /dev/mediaN
>> device.
> If it is standalone, is it reused in newer iMX versions? (7 or 8)

No, this driver makes use of the Image Converter in IPUv3, so it will 
only run on iMX 5/6. The IPU has been dropped in iMX 7 and 8.

> And if it is just a regular m2m device, then it doesn't need to create a
> media device either (doesn't hurt, but it is not required).

Ok, I'll leave that up to Philipp. I don't mind either way whether it is 
folded into imx-media device, or whether it is made stand-alone with or 
without a new media device.

Steve