Re: [PATCH 1/2] [media] v4l2-mc: add a generic function to create the media graph

[Shuah Khan <shuahkh@osg.samsung.com>]

On 02/05/2016 02:51 AM, Mauro Carvalho Chehab wrote:
> The em28xx_v4l2_create_media_graph() is almost generic enough to
> be at the core, as an ancillary function. Make it even more generic,
> by getting rid of em28xx-specific code, relying only at the
> media_device, in order to discover all entities found on PC-customer's
> hardware and add it at the V4L2 core.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> ---
>  drivers/media/v4l2-core/Makefile  |   1 +
>  drivers/media/v4l2-core/v4l2-mc.c | 181 ++++++++++++++++++++++++++++++++++++++
>  include/media/v4l2-mc.h           |  24 +++++
>  3 files changed, 206 insertions(+)
>  create mode 100644 drivers/media/v4l2-core/v4l2-mc.c


Hi Mauro,

The following patch I sent a couple of days ago creates
this file rivers/media/v4l2-core/v4l2-mc.c

You had some minor comments on that patch and I am working
on addressing them.

[PATCH v2 07/22] media: v4l-core add enable/disable source common interfaces
https://lkml.org/lkml/2016/2/3/1500

Looks like you added it in your patch. This would lead a conflict
between our patches. How do you propose to resolve this?

thanks,
-- Shuah


> 
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index 1dc8bba2b198..795a5352761d 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -16,6 +16,7 @@ endif
>  ifeq ($(CONFIG_TRACEPOINTS),y)
>    videodev-objs += vb2-trace.o v4l2-trace.o
>  endif
> +videodev-$(CONFIG_MEDIA_CONTROLLER) += v4l2-mc.o
>  
>  obj-$(CONFIG_VIDEO_V4L2) += videodev.o
>  obj-$(CONFIG_VIDEO_V4L2) += v4l2-common.o
> diff --git a/drivers/media/v4l2-core/v4l2-mc.c b/drivers/media/v4l2-core/v4l2-mc.c
> new file mode 100644
> index 000000000000..7276dbbbe830
> --- /dev/null
> +++ b/drivers/media/v4l2-core/v4l2-mc.c
> @@ -0,0 +1,181 @@
> +/*
> + * Media Controller ancillary functions
> + *
> + * (c) 2016 Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <media/media-entity.h>
> +#include <media/v4l2-mc.h>
> +
> +int v4l2_mc_create_media_graph(struct media_device *mdev)
> +
> +{
> +	struct media_entity *entity;
> +	struct media_entity *if_vid = NULL, *if_aud = NULL, *sensor = NULL;
> +	struct media_entity *tuner = NULL, *decoder = NULL;
> +	struct media_entity *io_v4l = NULL, *io_vbi = NULL, *io_swradio = NULL;
> +	bool is_webcam = false;
> +	int ret;
> +
> +	if (!mdev)
> +		return 0;
> +
> +	media_device_for_each_entity(entity, mdev) {
> +		switch (entity->function) {
> +		case MEDIA_ENT_F_IF_VID_DECODER:
> +			if_vid = entity;
> +			break;
> +		case MEDIA_ENT_F_IF_AUD_DECODER:
> +			if_aud = entity;
> +			break;
> +		case MEDIA_ENT_F_TUNER:
> +			tuner = entity;
> +			break;
> +		case MEDIA_ENT_F_ATV_DECODER:
> +			decoder = entity;
> +			break;
> +		case MEDIA_ENT_F_IO_V4L:
> +			io_v4l = entity;
> +			break;
> +		case MEDIA_ENT_F_IO_VBI:
> +			io_vbi = entity;
> +			break;
> +		case MEDIA_ENT_F_IO_SWRADIO:
> +			io_swradio = entity;
> +			break;
> +		case MEDIA_ENT_F_CAM_SENSOR:
> +			sensor = entity;
> +			is_webcam = true;
> +			break;
> +		}
> +	}
> +
> +	/* It should have at least one I/O entity */
> +	if (!io_v4l && !io_vbi && !io_swradio)
> +		return -EINVAL;
> +
> +	/*
> +	 * Here, webcams are modeled on a very simple way: the sensor is
> +	 * connected directly to the I/O entity. All dirty details, like
> +	 * scaler and crop HW are hidden. While such mapping is not enough
> +	 * for mc-centric hardware, it is enough for v4l2 interface centric
> +	 * PC-consumer's hardware.
> +	 */
> +	if (is_webcam) {
> +		if (!io_v4l)
> +			return -EINVAL;
> +
> +		media_device_for_each_entity(entity, mdev) {
> +			if (entity->function != MEDIA_ENT_F_CAM_SENSOR)
> +				continue;
> +			ret = media_create_pad_link(entity, 0,
> +						    io_v4l, 0,
> +						    MEDIA_LNK_FL_ENABLED);
> +			if (ret)
> +				return ret;
> +		}
> +		if (!decoder)
> +			return 0;
> +	}
> +
> +	/* The device isn't a webcam. So, it should have a decoder */
> +	if (!decoder)
> +		return -EINVAL;
> +
> +	/* Link the tuner and IF video output pads */
> +	if (tuner) {
> +		if (if_vid) {
> +			ret = media_create_pad_link(tuner, TUNER_PAD_OUTPUT,
> +						    if_vid,
> +						    IF_VID_DEC_PAD_IF_INPUT,
> +						    MEDIA_LNK_FL_ENABLED);
> +			if (ret)
> +				return ret;
> +			ret = media_create_pad_link(if_vid, IF_VID_DEC_PAD_OUT,
> +						decoder, DEMOD_PAD_IF_INPUT,
> +						MEDIA_LNK_FL_ENABLED);
> +			if (ret)
> +				return ret;
> +		} else {
> +			ret = media_create_pad_link(tuner, TUNER_PAD_OUTPUT,
> +						decoder, DEMOD_PAD_IF_INPUT,
> +						MEDIA_LNK_FL_ENABLED);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		if (if_aud) {
> +			ret = media_create_pad_link(tuner, TUNER_PAD_AUD_OUT,
> +						    if_aud,
> +						    IF_AUD_DEC_PAD_IF_INPUT,
> +						    MEDIA_LNK_FL_ENABLED);
> +			if (ret)
> +				return ret;
> +		} else {
> +			if_aud = tuner;
> +		}
> +
> +	}
> +
> +	/* Create demod to V4L, VBI and SDR radio links */
> +	if (io_v4l) {
> +		ret = media_create_pad_link(decoder, DEMOD_PAD_VID_OUT,
> +					io_v4l, 0,
> +					MEDIA_LNK_FL_ENABLED);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (io_swradio) {
> +		ret = media_create_pad_link(decoder, DEMOD_PAD_VID_OUT,
> +					io_swradio, 0,
> +					MEDIA_LNK_FL_ENABLED);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (io_vbi) {
> +		ret = media_create_pad_link(decoder, DEMOD_PAD_VBI_OUT,
> +					    io_vbi, 0,
> +					    MEDIA_LNK_FL_ENABLED);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Create links for the media connectors */
> +	media_device_for_each_entity(entity, mdev) {
> +		switch (entity->function) {
> +		case MEDIA_ENT_F_CONN_RF:
> +			if (!tuner)
> +				continue;
> +
> +			ret = media_create_pad_link(entity, 0, tuner,
> +						    TUNER_PAD_RF_INPUT,
> +						    MEDIA_LNK_FL_ENABLED);
> +			break;
> +		case MEDIA_ENT_F_CONN_SVIDEO:
> +		case MEDIA_ENT_F_CONN_COMPOSITE:
> +		case MEDIA_ENT_F_CONN_TEST:
> +			ret = media_create_pad_link(entity, 0, decoder,
> +						    DEMOD_PAD_IF_INPUT, 0);
> +			break;
> +		default:
> +			continue;
> +		}
> +		if (ret)
> +			return ret;
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(v4l2_mc_create_media_graph);
> diff --git a/include/media/v4l2-mc.h b/include/media/v4l2-mc.h
> index df115195690e..3097493e6cf1 100644
> --- a/include/media/v4l2-mc.h
> +++ b/include/media/v4l2-mc.h
> @@ -14,6 +14,8 @@
>   * GNU General Public License for more details.
>   */
>  
> +#include <media/media-device.h>
> +
>  /**
>   * enum tuner_pad_index - tuner pad index for MEDIA_ENT_F_TUNER
>   *
> @@ -89,3 +91,25 @@ enum demod_pad_index {
>  	DEMOD_PAD_VBI_OUT,
>  	DEMOD_NUM_PADS
>  };
> +
> +/**
> + * v4l2_mc_create_media_graph() - create Media Controller links at the graph.
> + *
> + * @mdev:	pointer to the &media_device struct.
> + *
> + * Add links between the entities commonly found on PC customer's hardware at
> + * the V4L2 side: camera sensors, audio and video PLL-IF decoders, tuners,
> + * analog TV decoder and I/O entities (video, VBI and Software Defined Radio).
> + * NOTE: webcams are modeled on a very simple way: the sensor is
> + * connected directly to the I/O entity. All dirty details, like
> + * scaler and crop HW are hidden. While such mapping is enough for v4l2
> + * interface centric PC-consumer's hardware, V4L2 subdev centric camera
> + * hardware should not use this routine, as it will not build the right graph.
> + */
> +#ifdef CONFIG_MEDIA_CONTROLLER
> +int v4l2_mc_create_media_graph(struct media_device *mdev);
> +#else
> +static inline int v4l2_mc_create_media_graph(struct media_device *mdev) {
> +	return 0;
> +}
> +#endif
> 


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978