Re: [PATCH v2 1/2] media: add helpers for memory-to-memory media controller

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/21/2018 10:38 PM, Ezequiel Garcia wrote:
> A memory-to-memory pipeline device consists in three
> entities: two DMA engine and one video processing entities.
> The DMA engine entities are linked to a V4L interface.
> 
> This commit add a new v4l2_m2m_{un}register_media_controller
> API to register this topology.
> 
> For instance, a typical mem2mem device topology would
> look like this:
> 
> Device topology
> - entity 1: source (1 pad, 1 link)
>             type Node subtype V4L flags 0
> 	pad0: Source
> 		-> "proc":1 [ENABLED,IMMUTABLE]
> 
> - entity 3: proc (2 pads, 2 links)
>             type Node subtype Unknown flags 0
> 	pad0: Source
> 		-> "sink":0 [ENABLED,IMMUTABLE]
> 	pad1: Sink
> 		<- "source":0 [ENABLED,IMMUTABLE]
> 
> - entity 6: sink (1 pad, 1 link)
>             type Node subtype V4L flags 0
> 	pad0: Sink
> 		<- "proc":0 [ENABLED,IMMUTABLE]
> 
> Suggested-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> Suggested-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> ---
>  drivers/media/v4l2-core/v4l2-dev.c     |  13 +-
>  drivers/media/v4l2-core/v4l2-mem2mem.c | 174 +++++++++++++++++++++++++
>  include/media/v4l2-mem2mem.h           |  19 +++
>  include/uapi/linux/media.h             |   3 +
>  4 files changed, 204 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 4ffd7d60a901..c1996d73ca74 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -202,7 +202,7 @@ static void v4l2_device_release(struct device *cd)
>  	mutex_unlock(&videodev_lock);
>  
>  #if defined(CONFIG_MEDIA_CONTROLLER)
> -	if (v4l2_dev->mdev) {
> +	if (v4l2_dev->mdev && vdev->vfl_dir != VFL_DIR_M2M) {
>  		/* Remove interfaces and interface links */
>  		media_devnode_remove(vdev->intf_devnode);
>  		if (vdev->entity.function != MEDIA_ENT_F_UNKNOWN)
> @@ -733,19 +733,22 @@ static void determine_valid_ioctls(struct video_device *vdev)
>  			BASE_VIDIOC_PRIVATE);
>  }
>  
> -static int video_register_media_controller(struct video_device *vdev, int type)
> +static int video_register_media_controller(struct video_device *vdev)
>  {
>  #if defined(CONFIG_MEDIA_CONTROLLER)
>  	u32 intf_type;
>  	int ret;
>  
> -	if (!vdev->v4l2_dev->mdev)
> +	/* Memory-to-memory devices are more complex and use
> +	 * their own function to register its mc entities.
> +	 */
> +	if (!vdev->v4l2_dev->mdev || vdev->vfl_dir == VFL_DIR_M2M)
>  		return 0;
>  
>  	vdev->entity.obj_type = MEDIA_ENTITY_TYPE_VIDEO_DEVICE;
>  	vdev->entity.function = MEDIA_ENT_F_UNKNOWN;
>  
> -	switch (type) {
> +	switch (vdev->vfl_type) {
>  	case VFL_TYPE_GRABBER:
>  		intf_type = MEDIA_INTF_T_V4L_VIDEO;
>  		vdev->entity.function = MEDIA_ENT_F_IO_V4L;
> @@ -993,7 +996,7 @@ int __video_register_device(struct video_device *vdev,
>  	v4l2_device_get(vdev->v4l2_dev);
>  
>  	/* Part 5: Register the entity. */
> -	ret = video_register_media_controller(vdev, type);
> +	ret = video_register_media_controller(vdev);
>  
>  	/* Part 6: Activate this minor. The char device can now be used. */
>  	set_bit(V4L2_FL_REGISTERED, &vdev->flags);
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index c4f963d96a79..e0e7262b7e75 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -17,9 +17,11 @@
>  #include <linux/sched.h>
>  #include <linux/slab.h>
>  
> +#include <media/media-device.h>
>  #include <media/videobuf2-v4l2.h>
>  #include <media/v4l2-mem2mem.h>
>  #include <media/v4l2-dev.h>
> +#include <media/v4l2-device.h>
>  #include <media/v4l2-fh.h>
>  #include <media/v4l2-event.h>
>  
> @@ -50,6 +52,19 @@ module_param(debug, bool, 0644);
>   * offsets but for different queues */
>  #define DST_QUEUE_OFF_BASE	(1 << 30)
>  
> +enum v4l2_m2m_entity_type {
> +	MEM2MEM_ENT_TYPE_SOURCE,
> +	MEM2MEM_ENT_TYPE_SINK,
> +	MEM2MEM_ENT_TYPE_PROC,
> +	MEM2MEM_ENT_TYPE_MAX
> +};
> +
> +static const char * const m2m_entity_name[] = {
> +	"source",
> +	"sink",
> +	"proc"
> +};
> +
>  
>  /**
>   * struct v4l2_m2m_dev - per-device context
> @@ -60,6 +75,15 @@ module_param(debug, bool, 0644);
>   */
>  struct v4l2_m2m_dev {
>  	struct v4l2_m2m_ctx	*curr_ctx;
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +	struct media_entity	*source;
> +	struct media_pad	source_pad;
> +	struct media_entity	sink;
> +	struct media_pad	sink_pad;
> +	struct media_entity	proc;
> +	struct media_pad	proc_pads[2];
> +	struct media_intf_devnode *intf_devnode;
> +#endif
>  
>  	struct list_head	job_queue;
>  	spinlock_t		job_spinlock;
> @@ -595,6 +619,156 @@ int v4l2_m2m_mmap(struct file *file, struct v4l2_m2m_ctx *m2m_ctx,
>  }
>  EXPORT_SYMBOL(v4l2_m2m_mmap);
>  
> +#if defined(CONFIG_MEDIA_CONTROLLER)
> +void v4l2_m2m_unregister_media_controller(struct v4l2_m2m_dev *m2m_dev)
> +{
> +	media_remove_intf_links(&m2m_dev->intf_devnode->intf);
> +	media_devnode_remove(m2m_dev->intf_devnode);
> +
> +	media_entity_remove_links(m2m_dev->source);
> +	media_entity_remove_links(&m2m_dev->sink);
> +	media_entity_remove_links(&m2m_dev->proc);
> +	media_device_unregister_entity(m2m_dev->source);
> +	media_device_unregister_entity(&m2m_dev->sink);
> +	media_device_unregister_entity(&m2m_dev->proc);
> +}
> +EXPORT_SYMBOL_GPL(v4l2_m2m_unregister_media_controller);
> +
> +static int v4l2_m2m_register_entity(struct media_device *mdev,
> +	struct v4l2_m2m_dev *m2m_dev, enum v4l2_m2m_entity_type type,
> +	int function)
> +{
> +	struct media_entity *entity;
> +	struct media_pad *pads;
> +	int num_pads;
> +	int ret;
> +
> +	switch (type) {
> +	case MEM2MEM_ENT_TYPE_SOURCE:
> +		entity = m2m_dev->source;
> +		pads = &m2m_dev->source_pad;
> +		entity->name = m2m_entity_name[type];
> +		pads[0].flags = MEDIA_PAD_FL_SOURCE;
> +		num_pads = 1;
> +		break;
> +	case MEM2MEM_ENT_TYPE_SINK:
> +		entity = &m2m_dev->sink;
> +		pads = &m2m_dev->sink_pad;
> +		pads[0].flags = MEDIA_PAD_FL_SINK;
> +		num_pads = 1;
> +		break;
> +	case MEM2MEM_ENT_TYPE_PROC:
> +		entity = &m2m_dev->proc;
> +		pads = m2m_dev->proc_pads;
> +		pads[0].flags = MEDIA_PAD_FL_SOURCE;
> +		pads[1].flags = MEDIA_PAD_FL_SINK;

Can you swap this? The v4l2-compliance test gave a "fail: v4l2-test-media.cpp(333): found_source"
message which is because (I think) it expects sink pads before source pads.

I'm not actually sure that this is a requirement, at least I can't find this in the spec,
but for some reason I have this memory that this actually is the right order. It might just
be a custom rather than a requirement.

Anyway, it doesn't matter for this code, so it is easiest to swap it and run v4l2-compliance -m
again.

I am interested in the v4l2-compliance output of the topology.

Regards,

	Hans



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux