Re: [PATCH 03/13] v4l2-mc: switch it to use the new approach to setup pipelines

[Mauro Carvalho Chehab]

Em Wed, 26 Sep 2018 17:44:53 +0300
Laurent Pinchart  escreveu:

> Hi Mauro,
> 
> Thank you for the patch.
> 
> On Wednesday, 1 August 2018 18:55:05 EEST Mauro Carvalho Chehab wrote:
> > Instead of relying on a static map for pids, use the new sig_type
> > "taint" type to setup the pipelines with the same tipe between  
> 
> s/tipe/type/
> 
> > different entities.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > ---
> >  drivers/media/media-entity.c  | 26 +++
> >  drivers/media/v4l2-core/v4l2-mc.c | 73 ---
> >  include/media/media-entity.h  | 19 
> >  3 files changed, 101 insertions(+), 17 deletions(-)
> > 
> > diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> > index 3498551e618e..0b1cb3559140 100644
> > --- a/drivers/media/media-entity.c
> > +++ b/drivers/media/media-entity.c
> > @@ -662,6 +662,32 @@ static void __media_entity_remove_link(struct
> > media_entity *entity, kfree(link);
> >  }
> > 
> > +int media_get_pad_index(struct media_entity *entity, bool is_sink,
> > +   enum media_pad_signal_type sig_type)
> > +{
> > +   int i;  
> 
> is is never negative, please use an unsigned int.
> 
> > +   bool pad_is_sink;
> > +
> > +   if (!entity)
> > +   return -EINVAL;
> > +
> > +   for (i = 0; i < entity->num_pads; i++) {
> > +   if (entity->pads[i].flags == MEDIA_PAD_FL_SINK)
> > +   pad_is_sink = true;
> > +   else if (entity->pads[i].flags == MEDIA_PAD_FL_SOURCE)
> > +   pad_is_sink = false;
> > +   else
> > +   continue;   /* This is an error! */
> > +
> > +   if (pad_is_sink != is_sink)
> > +   continue;
> > +   if (entity->pads[i].sig_type == sig_type)
> > +   return i;
> > +   }
> > +   return -EINVAL;
> > +}
> > +EXPORT_SYMBOL_GPL(media_get_pad_index);
> > +
> >  int
> >  media_create_pad_link(struct media_entity *source, u16 source_pad,
> >  struct media_entity *sink, u16 sink_pad, u32 flags)
> > diff --git a/drivers/media/v4l2-core/v4l2-mc.c
> > b/drivers/media/v4l2-core/v4l2-mc.c index 982bab3530f6..1925e1a3b861 100644
> > --- a/drivers/media/v4l2-core/v4l2-mc.c
> > +++ b/drivers/media/v4l2-core/v4l2-mc.c
> > @@ -28,7 +28,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
> > struct media_entity *io_v4l = NULL, *io_vbi = NULL, *io_swradio = NULL;
> > bool is_webcam = false;
> > u32 flags;
> > -   int ret;
> > +   int ret, pad_sink, pad_source;
> > 
> > if (!mdev)
> > return 0;
> > @@ -97,29 +97,52 @@ int v4l2_mc_create_media_graph(struct media_device
> > *mdev) /* 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,
> > +   pad_source = media_get_pad_index(tuner, false,
> > +PAD_SIGNAL_ANALOG);
> > +   pad_sink = media_get_pad_index(if_vid, true,
> > +  PAD_SIGNAL_ANALOG);
> > +   if (pad_source < 0 || pad_sink < 0)
> > +   return -EINVAL;
> > +   ret = media_create_pad_link(tuner, pad_source,
> > +   if_vid, pad_sink,
> > 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,
> > +
> > +   pad_source = media_get_pad_index(if_vid, false,
> > +PAD_SIGNAL_DV);
> > +   pad_sink = media_get_pad_index(decoder, true,
> > +  PAD_SIGNAL_DV);
> > +   if (pad_source < 0 || pad_sink < 0)
> > +   return -EINVAL;
> > +   ret = media_create_pad_link(if_vid, pad_source,
> > +   decoder, pad_sink,
> > MEDIA_LNK_FL_ENABLED);
> > if (ret)
> > return ret;
> > } else {
> > -   ret = media_create_pad_link(tuner, TUNER_PAD_OUTPUT,
> > -   decoder, DEMOD_PAD_IF_INPUT,
> > +   pad_source = media_get_pad_index(tuner, false,
> > +PAD_SIGNAL_ANALOG);
> > +   pad_sink = media_get_pad_index(decoder, true,
> > +

Re: [PATCH 03/13] v4l2-mc: switch it to use the new approach to setup pipelines

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Wednesday, 1 August 2018 18:55:05 EEST Mauro Carvalho Chehab wrote:
> Instead of relying on a static map for pids, use the new sig_type
> "taint" type to setup the pipelines with the same tipe between

s/tipe/type/

> different entities.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> ---
>  drivers/media/media-entity.c  | 26 +++
>  drivers/media/v4l2-core/v4l2-mc.c | 73 ---
>  include/media/media-entity.h  | 19 
>  3 files changed, 101 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 3498551e618e..0b1cb3559140 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -662,6 +662,32 @@ static void __media_entity_remove_link(struct
> media_entity *entity, kfree(link);
>  }
> 
> +int media_get_pad_index(struct media_entity *entity, bool is_sink,
> + enum media_pad_signal_type sig_type)
> +{
> + int i;

is is never negative, please use an unsigned int.

> + bool pad_is_sink;
> +
> + if (!entity)
> + return -EINVAL;
> +
> + for (i = 0; i < entity->num_pads; i++) {
> + if (entity->pads[i].flags == MEDIA_PAD_FL_SINK)
> + pad_is_sink = true;
> + else if (entity->pads[i].flags == MEDIA_PAD_FL_SOURCE)
> + pad_is_sink = false;
> + else
> + continue;   /* This is an error! */
> +
> + if (pad_is_sink != is_sink)
> + continue;
> + if (entity->pads[i].sig_type == sig_type)
> + return i;
> + }
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(media_get_pad_index);
> +
>  int
>  media_create_pad_link(struct media_entity *source, u16 source_pad,
>struct media_entity *sink, u16 sink_pad, u32 flags)
> diff --git a/drivers/media/v4l2-core/v4l2-mc.c
> b/drivers/media/v4l2-core/v4l2-mc.c index 982bab3530f6..1925e1a3b861 100644
> --- a/drivers/media/v4l2-core/v4l2-mc.c
> +++ b/drivers/media/v4l2-core/v4l2-mc.c
> @@ -28,7 +28,7 @@ int v4l2_mc_create_media_graph(struct media_device *mdev)
>   struct media_entity *io_v4l = NULL, *io_vbi = NULL, *io_swradio = NULL;
>   bool is_webcam = false;
>   u32 flags;
> - int ret;
> + int ret, pad_sink, pad_source;
> 
>   if (!mdev)
>   return 0;
> @@ -97,29 +97,52 @@ int v4l2_mc_create_media_graph(struct media_device
> *mdev) /* 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,
> + pad_source = media_get_pad_index(tuner, false,
> +  PAD_SIGNAL_ANALOG);
> + pad_sink = media_get_pad_index(if_vid, true,
> +PAD_SIGNAL_ANALOG);
> + if (pad_source < 0 || pad_sink < 0)
> + return -EINVAL;
> + ret = media_create_pad_link(tuner, pad_source,
> + if_vid, pad_sink,
>   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,
> +
> + pad_source = media_get_pad_index(if_vid, false,
> +  PAD_SIGNAL_DV);
> + pad_sink = media_get_pad_index(decoder, true,
> +PAD_SIGNAL_DV);
> + if (pad_source < 0 || pad_sink < 0)
> + return -EINVAL;
> + ret = media_create_pad_link(if_vid, pad_source,
> + decoder, pad_sink,
>   MEDIA_LNK_FL_ENABLED);
>   if (ret)
>   return ret;
>   } else {
> - ret = media_create_pad_link(tuner, TUNER_PAD_OUTPUT,
> - decoder, DEMOD_PAD_IF_INPUT,
> + pad_source = media_get_pad_index(tuner, false,
> +  PAD_SIGNAL_ANALOG);
> + pad_sink = media_get_pad_index(decoder, true,
> +PAD_SIGNAL_ANALOG);
> + if (pad_source < 0 || pad_sink < 0)
> + return -EINVAL;
> + ret =