fbdev - wipe screen (dd) with ioctl ?

[Ran Shalit]

Hello,

I use the following to wipe a screen:
dd if=/dev/zero of=/dev/fb0 code

Is there a way to do the same thing in code (using ioctl I suppose) ?

Regards,
Ran
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/32] VSP: Add R-Car Gen3 support

[Geert Uytterhoeven]

Hi Laurent,

On Sat, Dec 5, 2015 at 3:12 AM, Laurent Pinchart
 wrote:
> This patch set adds support for the Renesas R-Car Gen3 SoC family to the VSP1
> driver. The large number of patches is caused by a change in the display
> controller architecture that makes usage of the VSP mandatory as the display
> controller has lost the ability to read data from memory.
>
> Patch 01/32 to 27/32 prepare for the implementation of an API exported to the
> DRM driver in patch 28/32. Patches 31/32 enables support for the R-Car Gen3
> family, and patch 32/32 finally enhances perfomances by implementing support
> for display lists.
>
> The major change compared to v1 is the usage of the IP version register
> instead of DT properties to configure device parameters such as the number of
> BRU inputs or the availability of the BRU.

Thanks for your series!

As http://git.linuxtv.org/pinchartl/media.git/tag/?id=vsp1-kms-20151112 is
getting old, and has lots of conflicts with recent -next, do you plan to publish
this in a branch, and a separate branch for integration, to ease integration
in renesas-drivers?

Alternatively, I can just import the series you posted, but having the
broken-out integration part would be nice.

Thanks again!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

v4l2 kernel module debugging methods

[Ran Shalit]

Hello,

I would like to ask a general question regarding methods to debug a
v4l2 device driver.
Since I assume that the kernel driver will probably won't work in
first try after coding everything inside the device driver...

1. Do you think qemu/kgdb debugger is a good method for the device
driver debugging , or is it plain printing ?

2. Is there a simple way to display the image of a YUV-like buffer in memory ?

Any other methods, tips, about validation, testing and developing a
v4l2 device is appreciated.

Best Regards,
Ran
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: v4l2 kernel module debugging methods

[Nicolas Dufresne]

Le dimanche 06 décembre 2015 à 00:00 +0200, Ran Shalit a écrit :
> Hello,
> 
> I would like to ask a general question regarding methods to debug a
> v4l2 device driver.
> Since I assume that the kernel driver will probably won't work in
> first try after coding everything inside the device driver...
> 
> 1. Do you think qemu/kgdb debugger is a good method for the device
> driver debugging , or is it plain printing ?
> 
> 2. Is there a simple way to display the image of a YUV-like buffer in
> memory ?

Most Linux distribution ships GStreamer. You can with GStreamer read
and display a raw YUV images (you need to know the specific format)
using videoparse element.

  gst-launch-1.0 filesrc location=my.yuv ! videoparse format=yuy2 width=320 
height=240 ! imagefreeze ! videoconvert ! autovideosink

You could also encode and store to various formats, replacing the
imagefreeze ... section with an encoder and a filesink. Note that
videoparse unfortunatly does not allow passing strides array or
offsets. So it will work only if you set the width/height to padded
width/height.

regards,
Nicolas

signature.asc
Description: This is a digitally signed message part

Re: [PATCH v8 44/55] [media] uapi/media.h: Add MEDIA_IOC_G_TOPOLOGY ioctl

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 06 September 2015 09:03:04 Mauro Carvalho Chehab wrote:
> Add a new ioctl that will report the entire topology on
> one go.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 7320cdc45833..2d5ad40254b7 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -181,6 +181,8 @@ struct media_interface {
>   */
>  struct media_intf_devnode {
>   struct media_interface  intf;
> +
> + /* Should match the fields at media_v2_intf_devnode */
>   u32 major;
>   u32 minor;
>  };
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index a1bd7afba110..b17f6763aff4 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -206,6 +206,10 @@ struct media_pad_desc {
>  #define MEDIA_LNK_FL_IMMUTABLE   (1 << 1)
>  #define MEDIA_LNK_FL_DYNAMIC (1 << 2)
> 
> +#define MEDIA_LNK_FL_LINK_TYPE   (0xf << 28)
> +#  define MEDIA_LNK_FL_DATA_LINK (0 << 28)
> +#  define MEDIA_LNK_FL_INTERFACE_LINK(1 << 28)
> +
>  struct media_link_desc {
>   struct media_pad_desc source;
>   struct media_pad_desc sink;
> @@ -249,11 +253,93 @@ struct media_links_enum {
>  #define MEDIA_INTF_T_ALSA_RAWMIDI   (MEDIA_INTF_T_ALSA_BASE + 4)
>  #define MEDIA_INTF_T_ALSA_HWDEP (MEDIA_INTF_T_ALSA_BASE + 5)
> 
> -/* TBD: declare the structs needed for the new G_TOPOLOGY ioctl */
> +/*
> + * MC next gen API definitions
> + *
> + * NOTE: The declarations below are close to the MC RFC for the Media
> + *Controller, the next generation. Yet, there are a few adjustments
> + *to do, as we want to be able to have a functional API before
> + *the MC properties change. Those will be properly marked below.
> + *Please also notice that I removed "num_pads", "num_links",
> + *from the proposal, as a proper userspace application will likely
> + *use lists for pads/links, just as we intend to do in Kernelspace.
> + *The API definition should be freed from fields that are bound to
> + *some specific data structure.
> + *
> + * FIXME: Currently, I opted to name the new types as "media_v2", as this
> + * won't cause any conflict with the Kernelspace namespace, nor with
> + * the previous kAPI media_*_desc namespace. This can be changed
> + * later, before the adding this API upstream.

Yes, that's a good idea. Or at least we need to remove this comment if we 
decide to keep the v2 names :-)

> + */
> +
> +
> +struct media_v2_entity {
> + __u32 id;
> + char name[64];  /* FIXME: move to a property? (RFC says so) */

I agree with Sakari that we can keep the name here even if we also expose it 
as a property. However, there's one issue we need to address : we need to 
clearly define what the name field should contain and how it should be 
constructed, otherwise we'll end up with the exact same mess as today, and I 
don't want that. We can discuss it in this mail thread or as replies to a 
future documentation patch.

> + __u16 reserved[14];

Sakari and Hans have already commented on using __u32 instead of __u16 for 
reserved fields, as well as on the number of reserved fields. I agree with 
them but have nothing to add.

> +};
> +
> +/* Should match the specific fields at media_intf_devnode */
> +struct media_v2_intf_devnode {
> + __u32 major;
> + __u32 minor;
> +};
> +
> +struct media_v2_interface {
> + __u32 id;
> + __u32 intf_type;
> + __u32 flags;
> + __u32 reserved[9];
> +
> + union {
> + struct media_v2_intf_devnode devnode;
> + __u32 raw[16];
> + };
> +};
> +
> +struct media_v2_pad {
> + __u32 id;
> + __u32 entity_id;
> + __u32 flags;
> + __u16 reserved[9];
> +};
> +
> +struct media_v2_link {
> +__u32 id;
> +__u32 source_id;
> +__u32 sink_id;
> +__u32 flags;
> +__u32 reserved[5];
> +};
> +
> +struct media_v2_topology {
> + __u32 topology_version;
> +
> + __u32 num_entities;
> + struct media_v2_entity *entities;

The kernel seems to be moving to using __u64 instead of pointers in userspace-
facing structures to avoid compat32 code.

> +
> + __u32 num_interfaces;
> + struct media_v2_interface *interfaces;
> +
> + __u32 num_pads;
> + struct media_v2_pad *pads;
> +
> + __u32 num_links;
> + struct media_v2_link *links;
> +
> + struct {
> + __u32 reserved_num;
> + void *reserved_ptr;
> + } reserved_types[16];
> + __u32 reserved[8];

I'd just create __u32 reserved fields without any reserved_types, we can 
always use the reserved fields to add new types later.

> +};
> +
> +/* ioctls */
> 
>  #define MEDIA_IOC_DEVICE_INFO_IOWR('|', 0x00, struct 
media_device_info)
>  #define 

Re: [PATCH v8 43/55] [media] media: report if a pad is sink or source at debug msg

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 06 September 2015 09:03:03 Mauro Carvalho Chehab wrote:
> Sometimes, it is important to see if the created pad is
> sink or source. Add info to track that.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index d8038a53f945..6ed5eef88593 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -121,8 +121,11 @@ static void dev_dbg_obj(const char *event_name,  struct
> media_gobj *gobj) struct media_pad *pad = gobj_to_pad(gobj);
> 
>   dev_dbg(gobj->mdev->dev,
> - "%s: id 0x%08x pad#%d: '%s':%d\n",
> - event_name, gobj->id, media_localid(gobj),
> + "%s: id 0x%08x %s%spad#%d: '%s':%d\n",
> + event_name, gobj->id,
> + pad->flags & MEDIA_PAD_FL_SINK   ? "  sink " : "",
> + pad->flags & MEDIA_PAD_FL_SOURCE ? "source " : "",

I'm wondering if we really need the two leading spaces in "  sink ", as a 
bidirectional pad would print "  sink source pad" and mess up the alignment 
anyway.

> + media_localid(gobj),
>   pad->entity->name, pad->index);
>   break;
>   }

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 39/55] [media] media controller: get rid of entity subtype on Kernel

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 06 September 2015 09:02:59 Mauro Carvalho Chehab wrote:
> Don't use anymore the type/subtype entity data/macros
> inside the Kernel.
> 
> Signed-off-by: Mauro Carvalho Chehab 

Acked-by: Laurent Pinchart 

> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 220864319d21..7320cdc45833 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -185,16 +185,6 @@ struct media_intf_devnode {
>   u32 minor;
>  };
> 
> -static inline u32 media_entity_type(struct media_entity *entity)
> -{
> - return entity->type & MEDIA_ENT_TYPE_MASK;
> -}
> -
> -static inline u32 media_entity_subtype(struct media_entity *entity)
> -{
> - return entity->type & MEDIA_ENT_SUBTYPE_MASK;
> -}
> -
>  static inline u32 media_entity_id(struct media_entity *entity)
>  {
>   return entity->graph_obj.id;
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 3d6210095336..f90147cb9b57 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,8 +42,6 @@ struct media_device_info {
> 
>  #define MEDIA_ENT_ID_FLAG_NEXT   (1 << 31)
> 
> -/* Used values for media_entity_desc::type */
> -
>  /*
>   * Initial value to be used when a new entity is created
>   * Drivers should change it to something useful

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 40/55] [media] media.h: don't use legacy entity macros at Kernel

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 06 September 2015 09:03:00 Mauro Carvalho Chehab wrote:
> Put the legacy MEDIA_ENT_* macros under a #ifndef __KERNEL__,
> in order to be sure that none of those old symbols are used
> inside the Kernel.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> Acked-by: Hans Verkuil 

Acked-by: Laurent Pinchart 

> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index f90147cb9b57..a1bd7afba110 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -105,6 +105,7 @@ struct media_device_info {
>  #define MEDIA_ENT_T_DVB_CA   (MEDIA_ENT_T_DVB_BASE + 4)
>  #define MEDIA_ENT_T_DVB_NET_DECAP(MEDIA_ENT_T_DVB_BASE + 5)
> 
> +#ifndef __KERNEL__
>  /* Legacy symbols used to avoid userspace compilation breakages */
>  #define MEDIA_ENT_TYPE_SHIFT 16
>  #define MEDIA_ENT_TYPE_MASK  0x00ff
> @@ -118,6 +119,7 @@ struct media_device_info {
>  #define MEDIA_ENT_T_DEVNODE_FB   (MEDIA_ENT_T_DEVNODE + 2)
>  #define MEDIA_ENT_T_DEVNODE_ALSA (MEDIA_ENT_T_DEVNODE + 3)
>  #define MEDIA_ENT_T_DEVNODE_DVB  (MEDIA_ENT_T_DEVNODE + 4)
> +#endif
> 
>  /* Entity types */

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 41/55] [media] DocBook: update descriptions for the media controller entities

[Laurent Pinchart]

Hello,

On Friday 11 September 2015 15:13:30 Hans Verkuil wrote:
> On 09/06/2015 02:03 PM, Mauro Carvalho Chehab wrote:
> > Cleanup the media controller entities description:
> > - remove MEDIA_ENT_T_DEVNODE and MEDIA_ENT_T_V4L2_SUBDEV entity
> >   types, as they don't mean anything;
> > - add MEDIA_ENT_T_UNKNOWN with a proper description;
> > - remove ALSA and FB entity types. Those should not be used, as
> >   the types are deprecated. We'll soon be adidng ALSA, but with
> >   a different entity namespace;
> > - improve the description of some entities.
> > 
> > Signed-off-by: Mauro Carvalho Chehab 
> > 
> > diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml index
> > 32a783635649..bc101516e372 100644
> > --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > @@ -179,70 +179,65 @@
> >  
> > 
> >   
> > -   MEDIA_ENT_T_DEVNODE
> > -   Unknown device node
> > +   MEDIA_ENT_T_UNKNOWN and
> > MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN + 
> > Unknown entity. That generally indicates that
> > +   a driver didn't initialize properly the entity, with is a Kernel
> > bug> 
> >   
> 
> I'm wondering: if userspace should never see an unknown entity, wouldn't it
> be better to move these UNKNOWN defines out of the public header to a kernel
> header and drop this from the documentation?

And shouldn't the bug be caught in kernelspace before it reaches the user ?

> >   
> > MEDIA_ENT_T_V4L2_VIDEO
> > -   V4L video, radio or vbi device node
> > +   V4L video streaming input or output entity
> >   
> > - 
> > -   MEDIA_ENT_T_DEVNODE_FB
> > -   Frame buffer device node
> > +   MEDIA_ENT_T_V4L2_VBI
> > +   V4L VBI streaming input or output entity
> >   
> > - 
> > -   MEDIA_ENT_T_DEVNODE_ALSA
> > -   ALSA card
> > +   MEDIA_ENT_T_V4L2_SWRADIO
> > +   V4L Sofware Digital Radio (SDR) streaming input or output
> > entity
>
> s/Sofware/Software/
> 
> >   
> >   
> > MEDIA_ENT_T_DVB_DEMOD
> > -   DVB frontend devnode
> > +   DVB demodulator entity
> >   
> >   
> > MEDIA_ENT_T_DVB_DEMUX
> > -   DVB demux devnode
> > +   DVB demux entity. Could be implemented on hardware or in
> > Kernelspace

s/Could be/Can be/

> s/on/in/
> 
> >   
> >   
> > MEDIA_ENT_T_DVB_TSOUT
> > -   DVB DVR devnode
> > +   DVB Transport Stream output entity
> >   
> >   
> > MEDIA_ENT_T_DVB_CA
> > -   DVB CAM devnode
> > +   DVB Conditional Access module (CAM) entity
> >   
> >   
> > 
MEDIA_ENT_T_DVB_DEMOD_NET_DECAP
> > -   DVB network devnode
> > - 
> > - 
> > -   MEDIA_ENT_T_V4L2_SUBDEV
> > -   Unknown V4L sub-device
> > +   DVB network ULE/MLE desencapsulation entity. Could be
> > implemented on hardware or in Kernelspace

s/Could be/Can be/

> s/on/in/
> 
> Hmm, is desencapsulation correct? Could it be 'de-encapsulation' instead? It
> looks weird.
>
> >   
> >   
> > 
MEDIA_ENT_T_V4L2_SUBDEV_SENSOR
> > -   Video sensor
> > +   Camera video sensor entity

s/video sensor/image sensor/

> >   
> >   
> > MEDIA_ENT_T_V4L2_SUBDEV_FLASH
> > -   Flash controller
> > +   Flash controller entity
> >   
> >   
> > MEDIA_ENT_T_V4L2_SUBDEV_LENS
> > -   Lens controller
> > +   Lens controller entity
> >   
> >   
> > 
MEDIA_ENT_T_V4L2_SUBDEV_DECODER
> > -   Video decoder, the basic function of the video decoder is 
to
> > -   accept analogue video from a wide variety of sources such as
> > +   Analog video decoder, the basic function of the video 
decoder
> > +   is to accept analogue video from a wide variety of sources such 
as
> > broadcast, DVD players, cameras and video cassette recorders, in
> > -   either NTSC, PAL or HD format and still occasionally SECAM, 
separate
> > -   it into its component parts, luminance and chrominance, and 
output
> > +   either NTSC, PAL, SECAM or HD format, separating the stream
> > +   into its component parts, luminance and chrominance, and output
> > it in some digital video standard, with appropriate embedded 
timing
> > signals.

Does timing signals refer to synchronization signals ? They don't have to be 
embedded, do they ?

> >   
> >   
> > MEDIA_ENT_T_V4L2_SUBDEV_TUNER
> > -   TV and/or radio tuner
> > +   Digital TV, analog TV, radio and/or software radio
> > tuner> 
> >   
> > 
> >

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

Re: [PATCH 0/3] Update Brazilian channel tables

[Mauro Carvalho Chehab]

Em Fri, 04 Dec 2015 23:30:03 +0100
Olliver Schinagl  escreveu:

> Hey Mauro,
> 
> On 03-12-15 22:27, Mauro Carvalho Chehab wrote:
> > (re-sent as vger didn't recognize the original post)
> >
> > Em Thu, 03 Dec 2015 14:56:52 +0100
> > Olliver Schinagl  escreveu:
> >
> >> Hey Mauro,
> >>
> >> On 03-12-15 14:55, Mauro Carvalho Chehab wrote:
> >>> Em Mon, 6 Jul 2015 21:28:10 +0200
> >>> Olliver Schinagl  escreveu:
> >>>
>  I tried to manually fix things, by changing the charset to utf-8 in the
>  saved e-mail. That worked but it was only wrong for patch 1/3 and 3/3. I
>  blindly did assume it was utf-8 so i'm not sure if I got for patch 1:
> 
>  warning: squelched 70 whitespace errors
>  warning: 75 lines add whitespace errors.
> 
>  and about the same for patch 3 the same amount.
> 
>  So i have the correct data now in my local tree (sans the whtiespace
>  fixes). Do you want me to push them as is, or do you want to see/fix
>  things first?
> >>> Hi Olliver,
> >>>
> >>> It seems that you're not finding much time lately to apply those
> >>> patches, as there are some patches pending for quite some time at
> >>> patchwork.
> >> Are there? I admit I do not check patchwork, I'll have to re-learn to
> >> use patchwork. I apply everything I get via e-mail though within a few 
> >> days.
> > Applying via email is ok. The big advantage of patchwork is that it
> > prevents us to forget something.
> >
> > So, in thesis, all patches sent to the ML should also be at patchwork,
> > as it stores every valid patch there. The status update of the patch
> > is manual, though.
> >
> > Yet, with the patchwork version we've just upgraded, there's a way to
> > tell patchwork to automatically update a patch when it gets applied
> > upstream via a hook.
> >
> > I installed such hook today. Let's see if it will work. If it works, it
> > should print something like:
> >
> > remote: I: 1 patch(es) updated to state Accepted.
> >
> > after the patch got pushed upstream.
> >
> >>> If you don't mind, I would like to add myself to it, as this way
> >>> I can drop those patches from patchwork, avoiding me to look on
> >>> them every time I review the pending patches ;)
> >> No problem, feel free to add them! As an excersize however, should I do
> >> this round of patchwork files?
> >>
> >> Can you send me a link to a patch so I can figure it out from there on out?
> > I'm seeing those two patches:
> >
> > Patchwork: http://patchwork.linuxtv.org/patch/31537/
> > Patchwork: http://patchwork.linuxtv.org/patch/32006/
> >
> > plus this broken one:
> >
> > Patchwork: http://patchwork.linuxtv.org/patch/31724/
> Ok applied them all and fixed the broken one. Took me more then an hour 
> and a half to fix and verify it!

Well, when the patch is so broken, I usually just ask the author to
re-send again with a better emailer ;)

> 
> Pushing worked, but got the following messages:
> 
> Total 12 (delta 7), reused 0 (delta 0)
> remote: E: failed to find patch for rev 
> 78fa84fbfac86aef7921f5381828dea8ab9af053.
> remote: E: failed to find patch for rev 
> 16270d7088c2bae2ffbfc8a73946db58ff4498fd.
> remote: E: failed to find patch for rev 
> 6da0f5e851dd5e51e43b87136228a88705a7ba69.
> remote: I: 0 patch(es) updated to state Accepted.

Yeah, that what I was afraid... The script doesn't seem to be doing
a good job to detect the patch. When have some time, I'll try to
take a look and see if it could be improved.
> 
> Also under my patchwork account (which I rememberd the log in for!) did 
> not find any reference to the patches? Are they supposed to show up 
> somewhere where I can easily recognize them (and am I supposed to be 
> notified by mail when there are patches there?)

No, the patch would be at the main list of patches. Ideally, we
should create a separate project for dtv-scan-tables, but we'll
need to add some way to split the patches among the different
projects. Perhaps adding a perl script to .forward with some
regex expressions to detect if the patch is for dtv-scan-tables.
> 
> Olliver
> >
> > Regards,
> > Mauro
> >
> >
> >> My appologies for not looking at the right place!
> >>> Regards,
> >>> Mauro
> >>>
>  Olliver
> 
>  On 06-07-15 21:19, Olliver Schinagl wrote:
> > Hey Mauro,
> >
> > I still am having issues with your patches. Maybe my workflow is wrong
> > or my git installation is somehow borked, but i'm getting:
> >
> > git am \[PATCH\ 1_3\]\ Update\ Brazilian\ ISDB-T\ tables\ -\ Mauro\
> > Carvalho\ Chehab\ \\ -\ 2015-07-06\ 1509.eml
> > fatal: cannot convert from true to UTF-8
> >
> > I download the messages from thunderbird as email's and then run git am
> > on them. This tends to work from others just fine. My git is from a
> > pretty recent build, git version 2.3.6. I am one of the crazy people
> > that does use gentoo for things, 

Re: [PATCH v2 17/32] v4l: vsp1: Fix typo in VI6_DISP_IRQ_STA_DST register name

[Laurent Pinchart]

Hi Sergio,

On Sunday 06 December 2015 00:47:21 Sergei Shtylyov wrote:
> On 12/5/2015 5:12 AM, Laurent Pinchart wrote:
> > Rename the VI6_DISP_IRQ_STA_DSE register
> 
> Register bit, perhaps?

Indeed. I know I can count on you to catch even such small issues :-)

I'll fix it for the next version (or the pull request if no issue that require 
a new review round is found).

> > to VI6_DISP_IRQ_STA_DST to fix
> > a typo and match the datasheet.
> > 
> > Signed-off-by: Laurent Pinchart
> > 
> > ---
> > 
> >   drivers/media/platform/vsp1/vsp1_regs.h | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/platform/vsp1/vsp1_regs.h
> > b/drivers/media/platform/vsp1/vsp1_regs.h index
> > 25b48738b147..8173ceaab9f9 100644
> > --- a/drivers/media/platform/vsp1/vsp1_regs.h
> > +++ b/drivers/media/platform/vsp1/vsp1_regs.h
> > @@ -46,7 +46,7 @@
> > 
> >   #define VI6_DISP_IRQ_ENB_LNEE(n)  (1 << (n))
> >   
> >   #define VI6_DISP_IRQ_STA  0x007c
> > 
> > -#define VI6_DISP_IRQ_STA_DSE   (1 << 8)
> > +#define VI6_DISP_IRQ_STA_DST   (1 << 8)
> > 
> >   #define VI6_DISP_IRQ_STA_MAE  (1 << 5)
> >   #define VI6_DISP_IRQ_STA_LNE(n)   (1 << (n))
> 
> MBR, Sergei

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 17/32] v4l: vsp1: Fix typo in VI6_DISP_IRQ_STA_DST register name

[Sergei Shtylyov]

Hello.

On 12/5/2015 5:12 AM, Laurent Pinchart wrote:


Rename the VI6_DISP_IRQ_STA_DSE register


   Register bit, perhaps?


to VI6_DISP_IRQ_STA_DST to fix
a typo and match the datasheet.

Signed-off-by: Laurent Pinchart 
---
  drivers/media/platform/vsp1/vsp1_regs.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vsp1/vsp1_regs.h 
b/drivers/media/platform/vsp1/vsp1_regs.h
index 25b48738b147..8173ceaab9f9 100644
--- a/drivers/media/platform/vsp1/vsp1_regs.h
+++ b/drivers/media/platform/vsp1/vsp1_regs.h
@@ -46,7 +46,7 @@
  #define VI6_DISP_IRQ_ENB_LNEE(n)  (1 << (n))

  #define VI6_DISP_IRQ_STA  0x007c
-#define VI6_DISP_IRQ_STA_DSE   (1 << 8)
+#define VI6_DISP_IRQ_STA_DST   (1 << 8)
  #define VI6_DISP_IRQ_STA_MAE  (1 << 5)
  #define VI6_DISP_IRQ_STA_LNE(n)   (1 << (n))



MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 00/32] VSP: Add R-Car Gen3 support

[Laurent Pinchart]

Hi Geert,

On Saturday 05 December 2015 11:57:49 Geert Uytterhoeven wrote:
> On Sat, Dec 5, 2015 at 3:12 AM, Laurent Pinchart wrote:
> > This patch set adds support for the Renesas R-Car Gen3 SoC family to the
> > VSP1 driver. The large number of patches is caused by a change in the
> > display controller architecture that makes usage of the VSP mandatory as
> > the display controller has lost the ability to read data from memory.
> > 
> > Patch 01/32 to 27/32 prepare for the implementation of an API exported to
> > the DRM driver in patch 28/32. Patches 31/32 enables support for the
> > R-Car Gen3 family, and patch 32/32 finally enhances perfomances by
> > implementing support for display lists.
> > 
> > The major change compared to v1 is the usage of the IP version register
> > instead of DT properties to configure device parameters such as the number
> > of BRU inputs or the availability of the BRU.
> 
> Thanks for your series!
> 
> As http://git.linuxtv.org/pinchartl/media.git/tag/?id=vsp1-kms-20151112 is
> getting old, and has lots of conflicts with recent -next, do you plan to
> publish this in a branch, and a separate branch for integration, to ease
> integration in renesas-drivers?
> 
> Alternatively, I can just import the series you posted, but having the
> broken-out integration part would be nice.

The issue I'm facing is that there's more than just two series. Beside the 
base VSP patches from this series, I have a series of DRM patches that depend 
on this one, a series of V4L2 core patches, another series of VSP patches that 
I still need to finish and a bunch of integration patches. As some of these 
have dependencies on H3 CCF support that hasn't landed in Simon's tree yet, I 
have merged your topic/cpg-mssr-v6 and topic/r8a7795-drivers-sh-v1 branches 
into my tree for development.

I could keep all series in separate branches and merge the two topic branches 
last, but that's not very handy during development when I have to continuously 
rebase my patches. Is there a way I could handle this that would make your 
life easier while not making mine more difficult ?

In the meantime I've pushed vsp1-kms-20151206 to 
git://linuxtv.org/pinchartl/media.git.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [media-workshop] [PATCH v8.4 43/83] [media] v4l2-subdev: use MEDIA_ENT_T_UNKNOWN for new subdevs

[Laurent Pinchart]

Hello,

CC'ing the linux-media mailing list, for real this time.

On Sunday 08 November 2015 00:02:55 Sakari Ailus wrote:
> On Wed, Oct 14, 2015 at 06:35:48PM -0300, Mauro Carvalho Chehab wrote:
> > Em Wed, 14 Oct 2015 13:15:40 +0300 Sakari Ailus escreveu:
> >> On Mon, Oct 12, 2015 at 09:26:04PM -0300, Mauro Carvalho Chehab wrote:
> >>> Em Tue, 13 Oct 2015 01:25:35 +0300 Sakari Ailus escreveu:
>  On Mon, Oct 12, 2015 at 01:43:32PM -0300, Mauro Carvalho Chehab wrote:
> > Instead of abusing MEDIA_ENT_T_V4L2_SUBDEV, initialize
> > new subdev entities as MEDIA_ENT_T_UNKNOWN.
> > 
> > Change-Id: I294ee20f49b6c40dd95339d6730d90fa85b0dea9
> > Signed-off-by: Mauro Carvalho Chehab 
> > Acked-by: Hans Verkuil 
> > ---
> > 
> >  drivers/media/media-device.c  |  6 ++
> >  drivers/media/v4l2-core/v4l2-subdev.c |  2 +-
> >  include/uapi/linux/media.h| 17 +
> >  3 files changed, 24 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/media-device.c
> > b/drivers/media/media-device.c
> > index 659507bce63f..134fe7510195 100644
> > --- a/drivers/media/media-device.c
> > +++ b/drivers/media/media-device.c
> > @@ -435,6 +435,12 @@ int __must_check
> > media_device_register_entity(struct media_device *mdev,
> >  {
> > int i;
> > 
> > +   if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN ||
> > +   entity->type == MEDIA_ENT_T_UNKNOWN)
> > +   dev_warn(mdev->dev,
> > +"Entity type for entity %s was not 
> > initialized!\n",
> > +entity->name);

First of all the subject of the patch is very misleading as you initialize the 
entity type for new subdevs to MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN, not 
MEDIA_ENT_T_UNKNOWN.

>  I don't think I'd warn about MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN ---
>  there are entities that do not fall into any category of existing
>  functions. For instance image signal processors have such; they are
>  hardware specific and often their functionality is somewhat so as
>  well. Some of them perform a variety of functions (image processing
>  algorithms) but I doubt it'd make sense to start e.g. listing those
>  until we have any standardised interface for them.

Most of the subdevs we have today are of the "unknown" type, which isn't very 
user-friendly. Part of the reason is that there has never been a big incentive 
for driver writes to add proper subdev types, as the type is ignored in 
userspace in most cases. This should hopefully change with functions, and 
we'll need to push for new subdevs to have at least one function defined, even 
if it's a generic function such as image processing for lack of a better 
alternative. Warning on MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN has the advantage that 
it will push driver authors to think about the issue instead of just ignoring 
it and setting the type/function to unknown. That might be a wrong solution 
though, as if we introduce a generic image processing function (which we'll 
need for lack of a better or more precise alternative in some cases) driver 
authors might just use that one instead of MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN, 
and it would become the equivalent of the unknown type for all practical 
purpose. We would only have pushed the problem one step further without 
solving it. I wonder how we could improve that.

>  The two entities in smiapp don't have a specific function either.
>  Adding a new one (scaler) might make sense for the two, but I think
>  I'd leave that out from this set.
> >>> 
> >>> IMHO, if the entity function is really unknown, it shouldn't even be
> >>> at the graph in the first place, as an unknown entity can't be
> >>> controlled.
> >> 
> >> These used to be just "sub-devices" without a type that 1) did not fall
> >> into any existing category
> > 
> > No problem. You can create a new function
> > 
> >> and 2) was not generic enough to warrant adding a
> >> specific type for them. I don't think that has really changed with
> >> functions.
> >
> > Well, you could add a device-specific function name. We have already
> > other device specific things (like device-only FOURCCs, device-specific
> > controls). I don't see why not having device-specific functions when
> > we want/need to map such entities.
> 
> What would be the benefit of having device specific functions?
> 
> The user who would need to access such a device would probably use the name
> instead, probably combined with the bus information in the future (or serial
> number etc.).
> 
> >>> So, I think we should either add a new function for those entities,
> >>> for them to be used on userspace, or simply remove them, if they won't
> >>> be used on userspace, because they aren't documented.
> >> 
> >> What kind of "function" could you use for 

Re: [PATCH v8 38/55] [media] v4l2-subdev: use MEDIA_ENT_T_UNKNOWN for new subdevs

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

In addition to my reply to Sakari's e-mail, please see below for a few small 
comments.

On Sunday 06 September 2015 09:02:58 Mauro Carvalho Chehab wrote:
> Instead of abusing MEDIA_ENT_T_V4L2_SUBDEV, initialize
> new subdev entities as MEDIA_ENT_T_UNKNOWN.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> Acked-by: Hans Verkuil 
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 659507bce63f..134fe7510195 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -435,6 +435,12 @@ int __must_check media_device_register_entity(struct
> media_device *mdev, {
>   int i;
> 
> + if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN ||
> + entity->type == MEDIA_ENT_T_UNKNOWN)
> + dev_warn(mdev->dev,
> +  "Entity type for entity %s was not initialized!\n",
> +  entity->name);
> +
>   /* Warn if we apparently re-register an entity */
>   WARN_ON(entity->graph_obj.mdev != NULL);
>   entity->graph_obj.mdev = mdev;
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
> b/drivers/media/v4l2-core/v4l2-subdev.c index 60da43772de9..b3bcc8253182
> 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -584,7 +584,7 @@ void v4l2_subdev_init(struct v4l2_subdev *sd, const
> struct v4l2_subdev_ops *ops) sd->host_priv = NULL;
>  #if defined(CONFIG_MEDIA_CONTROLLER)
>   sd->entity.name = sd->name;
> - sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
> + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN;
>  #endif
>  }
>  EXPORT_SYMBOL(v4l2_subdev_init);
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index f8725b881a1d..3d6210095336 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,6 +42,14 @@ struct media_device_info {
> 
>  #define MEDIA_ENT_ID_FLAG_NEXT   (1 << 31)
> 
> +/* Used values for media_entity_desc::type */
> +

You remove this a couple of patches later, is it worth adding it in the first 
place ?

> +/*
> + * Initial value to be used when a new entity is created
> + * Drivers should change it to something useful

As you warn when the value isn't change I'd say "Drivers must change...".

> + */
> +#define MEDIA_ENT_T_UNKNOWN  0x
> +
>  /*
>   * Base numbers for entity types
>   *
> @@ -75,6 +83,15 @@ struct media_device_info {
>  #define MEDIA_ENT_T_V4L2_SWRADIO (MEDIA_ENT_T_V4L2_BASE + 6)
> 
>  /* V4L2 Sub-device entities */
> +
> + /*
> +  * Subdevs are initialized with MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN,
> +  * in order to preserve backward compatibility.
> +  * Drivers should change to the proper subdev type before
> +  * registering the entity.
> +  */

Leading tabs look weird here.

> +#define MEDIA_ENT_T_V4L2_SUBDEV_UNKNOWN  MEDIA_ENT_T_V4L2_SUBDEV_BASE
> +
>  #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR   (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 
> 1)
>  #define MEDIA_ENT_T_V4L2_SUBDEV_FLASH(MEDIA_ENT_T_V4L2_SUBDEV_BASE + 
> 2)
>  #define MEDIA_ENT_T_V4L2_SUBDEV_LENS (MEDIA_ENT_T_V4L2_SUBDEV_BASE + 3)

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 34/55] [media] s5c73m3: fix subdev type

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 30 August 2015 00:06:45 Mauro Carvalho Chehab wrote:
> This sensor driver is abusing MEDIA_ENT_T_V4L2_SUBDEV, creating
> some subdevs with a non-existing type.
> 
> As this is a sensor driver, the proper type is likely
> MEDIA_ENT_T_CAM_SENSOR.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> b/drivers/media/i2c/s5c73m3/s5c73m3-core.c index c81bfbfea32f..abae37321c0c
> 100644
> --- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> +++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
> @@ -1688,7 +1688,7 @@ static int s5c73m3_probe(struct i2c_client *client,
> 
>   state->sensor_pads[S5C73M3_JPEG_PAD].flags = MEDIA_PAD_FL_SOURCE;
>   state->sensor_pads[S5C73M3_ISP_PAD].flags = MEDIA_PAD_FL_SOURCE;
> - sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
> + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;

As explained in my review of "s5k5baf: fix subdev type", this is correct...

>   ret = media_entity_init(>entity, S5C73M3_NUM_PADS,
>   state->sensor_pads);
> @@ -1704,7 +1704,7 @@ static int s5c73m3_probe(struct i2c_client *client,
>   state->oif_pads[OIF_ISP_PAD].flags = MEDIA_PAD_FL_SINK;
>   state->oif_pads[OIF_JPEG_PAD].flags = MEDIA_PAD_FL_SINK;
>   state->oif_pads[OIF_SOURCE_PAD].flags = MEDIA_PAD_FL_SOURCE;
> - oif_sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
> + oif_sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;

... but this isn't.

>   ret = media_entity_init(_sd->entity, OIF_NUM_PADS,
>   state->oif_pads);

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 33/55] [media] omap3/omap4/davinci: get rid of MEDIA_ENT_T_V4L2_SUBDEV abuse

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 30 August 2015 00:06:44 Mauro Carvalho Chehab wrote:
> On omap3/omap4/davinci drivers, MEDIA_ENT_T_V4L2_SUBDEV macro is
> abused in order to "simplify" the pad checks.

As explained in a couple of other replies to similar patches, it's not a hack 
:-)

> Basically, it does a logical or of this macro, in order to check
> for a local index and if the entity is either a subdev or not.
> 
> As we'll get rid of MEDIA_ENT_T_V4L2_SUBDEV macro,

This is the reason for this patch, and I agree with it.

> replace it by 2 << 16 where it occurs, and add a note saying that the code
> there is actually a hack.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/platform/omap3isp/ispccdc.c
> b/drivers/media/platform/omap3isp/ispccdc.c index
> 9a811f5741fa..f0e530c98188 100644
> --- a/drivers/media/platform/omap3isp/ispccdc.c
> +++ b/drivers/media/platform/omap3isp/ispccdc.c
> @@ -2513,9 +2513,14 @@ static int ccdc_link_setup(struct media_entity
> *entity, struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
>   struct isp_ccdc_device *ccdc = v4l2_get_subdevdata(sd);
>   struct isp_device *isp = to_isp_device(ccdc);
> + int index = local->index;

The index can never be negative, you can use unsigned int.

> 
> - switch (local->index | media_entity_type(remote->entity)) {
> - case CCDC_PAD_SINK | MEDIA_ENT_T_V4L2_SUBDEV:
> + /* FIXME: this is actually a hack! */

Please, let's not introduce a hack to replace valid code. I'm certainly fine 
with removing usage of MEDIA_ENT_T_V4L2_SUBDEV, but drivers should be modified 
cleanly.

If you rename the index variable to link and use a macro (I would call it 
LINK_TO_SUBDEV for instance) instead of 2 << 16 the implementation wouldn't be 
that bad, as what the switch operates on is the link, not the pad. I would 
also create a LINK_TO_DEVNODE macro, even if it evaluates to 0, to clearly 
label each case.

> + if (is_media_entity_v4l2_subdev(remote->entity))
> + index |= 2 << 16;

Why 2 << 16 and not 1 << 16 ?

Same comments for all the other files below (but please see the very end for 
one last comment).

> + switch (index) {
> + case CCDC_PAD_SINK | 2 << 16:
>   /* Read from the sensor (parallel interface), CCP2, CSI2a or
>* CSI2c.
>*/
> @@ -2543,7 +2548,7 @@ static int ccdc_link_setup(struct media_entity
> *entity, * Revisit this when it will be implemented, and return -EBUSY for
> now. */
> 
> - case CCDC_PAD_SOURCE_VP | MEDIA_ENT_T_V4L2_SUBDEV:
> + case CCDC_PAD_SOURCE_VP | 2 << 16:
>   /* Write to preview engine, histogram and H3A. When none of
>* those links are active, the video port can be disabled.
>*/
> @@ -2556,7 +2561,7 @@ static int ccdc_link_setup(struct media_entity
> *entity, }
>   break;
> 
> - case CCDC_PAD_SOURCE_OF | MEDIA_ENT_T_DEVNODE:
> + case CCDC_PAD_SOURCE_OF:
>   /* Write to memory */
>   if (flags & MEDIA_LNK_FL_ENABLED) {
>   if (ccdc->output & ~CCDC_OUTPUT_MEMORY)
> @@ -2567,7 +2572,7 @@ static int ccdc_link_setup(struct media_entity
> *entity, }
>   break;
> 
> - case CCDC_PAD_SOURCE_OF | MEDIA_ENT_T_V4L2_SUBDEV:
> + case CCDC_PAD_SOURCE_OF | 2 << 16:
>   /* Write to resizer */
>   if (flags & MEDIA_LNK_FL_ENABLED) {
>   if (ccdc->output & ~CCDC_OUTPUT_RESIZER)
> diff --git a/drivers/media/platform/omap3isp/ispccp2.c
> b/drivers/media/platform/omap3isp/ispccp2.c index
> 6ec7d104ab75..ae3038e643cc 100644
> --- a/drivers/media/platform/omap3isp/ispccp2.c
> +++ b/drivers/media/platform/omap3isp/ispccp2.c
> @@ -956,9 +956,14 @@ static int ccp2_link_setup(struct media_entity *entity,
> {
>   struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
>   struct isp_ccp2_device *ccp2 = v4l2_get_subdevdata(sd);
> + int index = local->index;
> 
> - switch (local->index | media_entity_type(remote->entity)) {
> - case CCP2_PAD_SINK | MEDIA_ENT_T_DEVNODE:
> + /* FIXME: this is actually a hack! */
> + if (is_media_entity_v4l2_subdev(remote->entity))
> + index |= 2 << 16;
> +
> + switch (index) {
> + case CCP2_PAD_SINK:
>   /* read from memory */
>   if (flags & MEDIA_LNK_FL_ENABLED) {
>   if (ccp2->input == CCP2_INPUT_SENSOR)
> @@ -970,7 +975,7 @@ static int ccp2_link_setup(struct media_entity *entity,
>   }
>   break;
> 
> - case CCP2_PAD_SINK | MEDIA_ENT_T_V4L2_SUBDEV:
> + case CCP2_PAD_SINK | 2 << 16:
>   /* read from sensor/phy */
>   if (flags & MEDIA_LNK_FL_ENABLED) {
>   if (ccp2->input == CCP2_INPUT_MEMORY)
> @@ -981,7 +986,7 @@ static int ccp2_link_setup(struct media_entity *entity,
>   

Re: [PATCH v8 32/55] [media] media: use macros to check for V4L2 subdev entities

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 30 August 2015 00:06:43 Mauro Carvalho Chehab wrote:
> Instead of relying on media subtype, use the new macros to detect
> if an entity is a subdev or an A/V DMA entity.
> 
> Please note that most drivers assume that there's just AV_DMA or
> V4L2 subdevs. This is not true anymore, as we've added MC support
> for DVB, and there are plans to add support for ALSA and FB/DRM
> too.
> 
> Ok, on the current pipelines supported by those drivers, just V4L
> stuff are there, but, assuming that some day a pipeline that also
> works with other subsystems will ever added, it is better to add
> explicit checks for the AV_DMA stuff.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/platform/exynos4-is/common.c
> b/drivers/media/platform/exynos4-is/common.c index
> 0eb34ecb8ee4..8c9a29e0e294 100644
> --- a/drivers/media/platform/exynos4-is/common.c
> +++ b/drivers/media/platform/exynos4-is/common.c
> @@ -22,8 +22,7 @@ struct v4l2_subdev *fimc_find_remote_sensor(struct
> media_entity *entity) while (pad->flags & MEDIA_PAD_FL_SINK) {
>   /* source pad */
>   pad = media_entity_remote_pad(pad);
> - if (pad == NULL ||
> - media_entity_type(pad->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
> + if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
>   break;
> 
>   sd = media_entity_to_v4l2_subdev(pad->entity);
> diff --git a/drivers/media/platform/exynos4-is/fimc-capture.c
> b/drivers/media/platform/exynos4-is/fimc-capture.c index
> 0627a93b2f3b..e9810fee4c30 100644
> --- a/drivers/media/platform/exynos4-is/fimc-capture.c
> +++ b/drivers/media/platform/exynos4-is/fimc-capture.c
> @@ -1141,8 +1141,7 @@ static int fimc_pipeline_validate(struct fimc_dev
> *fimc) }
>   }
> 
> - if (src_pad == NULL ||
> - media_entity_type(src_pad->entity) != 
> MEDIA_ENT_T_V4L2_SUBDEV)
> + if (!src_pad || !is_media_entity_v4l2_subdev(src_pad->entity))
>   break;
> 
>   /* Don't call FIMC subdev operation to avoid nested locking */
> @@ -1397,7 +1396,7 @@ static int fimc_link_setup(struct media_entity
> *entity, struct fimc_vid_cap *vc = >vid_cap;
>   struct v4l2_subdev *sensor;
> 
> - if (media_entity_type(remote->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
> + if (!is_media_entity_v4l2_subdev(remote->entity))
>   return -EINVAL;
> 
>   if (WARN_ON(fimc == NULL))
> diff --git a/drivers/media/platform/exynos4-is/fimc-isp-video.c
> b/drivers/media/platform/exynos4-is/fimc-isp-video.c index
> 3d9ccbf5f10f..5fbaf5e39903 100644
> --- a/drivers/media/platform/exynos4-is/fimc-isp-video.c
> +++ b/drivers/media/platform/exynos4-is/fimc-isp-video.c
> @@ -467,8 +467,7 @@ static int isp_video_pipeline_validate(struct fimc_isp
> *isp)
> 
>   /* Retrieve format at the source pad */
>   pad = media_entity_remote_pad(pad);
> - if (pad == NULL ||
> - media_entity_type(pad->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
> + if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
>   break;
> 
>   sd = media_entity_to_v4l2_subdev(pad->entity);
> diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c
> b/drivers/media/platform/exynos4-is/fimc-lite.c index
> b2607da4ad14..c2327147b360 100644
> --- a/drivers/media/platform/exynos4-is/fimc-lite.c
> +++ b/drivers/media/platform/exynos4-is/fimc-lite.c
> @@ -814,8 +814,7 @@ static int fimc_pipeline_validate(struct fimc_lite
> *fimc) }
>   /* Retrieve format at the source pad */
>   pad = media_entity_remote_pad(pad);
> - if (pad == NULL ||
> - media_entity_type(pad->entity) != MEDIA_ENT_T_V4L2_SUBDEV)
> + if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
>   break;
> 
>   sd = media_entity_to_v4l2_subdev(pad->entity);
> @@ -988,7 +987,6 @@ static int fimc_lite_link_setup(struct media_entity
> *entity, {
>   struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
>   struct fimc_lite *fimc = v4l2_get_subdevdata(sd);
> - unsigned int remote_ent_type = media_entity_type(remote->entity);
>   int ret = 0;
> 
>   if (WARN_ON(fimc == NULL))
> @@ -1000,7 +998,7 @@ static int fimc_lite_link_setup(struct media_entity
> *entity,
> 
>   switch (local->index) {
>   case FLITE_SD_PAD_SINK:
> - if (remote_ent_type != MEDIA_ENT_T_V4L2_SUBDEV) {
> + if (!is_media_entity_v4l2_subdev(remote->entity)) {
>   ret = -EINVAL;
>   break;
>   }
> @@ -1018,7 +1016,7 @@ static int fimc_lite_link_setup(struct media_entity
> *entity, case FLITE_SD_PAD_SOURCE_DMA:
>   if (!(flags & MEDIA_LNK_FL_ENABLED))
>   atomic_set(>out_path, 

Re: [PATCH 2/5] [media] v4l: vsp1: create pad links after subdev registration

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Thursday 03 September 2015 18:00:33 Javier Martinez Canillas wrote:
> The vsp1 driver creates the pads links before the media entities are
> registered with the media device. This doesn't work now that object
> IDs are used to create links so the media_device has to be set.
> 
> Move entities registration logic before pads links creation.
> 
> Signed-off-by: Javier Martinez Canillas 

Acked-by: Laurent Pinchart 

> ---
> 
>  drivers/media/platform/vsp1/vsp1_drv.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
> b/drivers/media/platform/vsp1/vsp1_drv.c index 9cd94a76a9ed..2aa427d3ff39
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -250,6 +250,14 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1) list_add_tail(>entity.list_dev, >entities);
>   }
> 
> + /* Register all subdevs. */
> + list_for_each_entry(entity, >entities, list_dev) {
> + ret = v4l2_device_register_subdev(>v4l2_dev,
> +   >subdev);
> + if (ret < 0)
> + goto done;
> + }
> +
>   /* Create links. */
>   list_for_each_entry(entity, >entities, list_dev) {
>   if (entity->type == VSP1_ENTITY_LIF ||
> @@ -269,14 +277,6 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1) return ret;
>   }
> 
> - /* Register all subdevs. */
> - list_for_each_entry(entity, >entities, list_dev) {
> - ret = v4l2_device_register_subdev(>v4l2_dev,
> -   >subdev);
> - if (ret < 0)
> - goto done;
> - }
> -
>   ret = v4l2_device_register_subdev_nodes(>v4l2_dev);
> 
>  done:

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 18/55] [media] omap3isp: create links after all subdevs have been bound

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Wednesday 09 September 2015 10:48:29 Javier Martinez Canillas wrote:
> On 09/09/2015 10:03 AM, Sakari Ailus wrote:
> > On Sun, Aug 30, 2015 at 12:06:29AM -0300, Mauro Carvalho Chehab wrote:
> >> From: Javier Martinez Canillas 
> >> 
> >> The omap3isp driver parses the graph endpoints to know how many
> >> subdevices needs to be registered async and register notifiers callbacks
> >> for to know when these are bound and when the async registrations are
> >> completed.
> >> 
> >> Currently the entities pad are linked with the correct ISP input
> >> interface when the subdevs are bound but it happens before entitities are
> >> registered with the media device so that won't work now that the entity
> >> links list is initialized on device registration.
> >> 
> >> So instead creating the pad links when the subdevice is bound, create
> >> them on the complete callback once all the subdevices have been bound but
> >> only try to create for the ones that have a bus configuration set during
> >> bound.
> >> 
> >> Signed-off-by: Javier Martinez Canillas 
> >> Signed-off-by: Mauro Carvalho Chehab 
> >> 
> >> diff --git a/drivers/media/platform/omap3isp/isp.c
> >> b/drivers/media/platform/omap3isp/isp.c index b8f6f81d2db2..69e7733d36cd
> >> 100644
> >> --- a/drivers/media/platform/omap3isp/isp.c
> >> +++ b/drivers/media/platform/omap3isp/isp.c
> >> @@ -2321,26 +2321,33 @@ static int isp_subdev_notifier_bound(struct
> >> v4l2_async_notifier *async,
> >> struct v4l2_subdev *subdev,
> >> struct v4l2_async_subdev *asd)
> >>  {
> >> -  struct isp_device *isp = container_of(async, struct isp_device,
> >> -notifier);
> >>struct isp_async_subdev *isd =
> >>container_of(asd, struct isp_async_subdev, asd);
> >> -  int ret;
> >> -
> >> -  ret = isp_link_entity(isp, >entity, isd->bus.interface);
> >> -  if (ret < 0)
> >> -  return ret;
> >> 
> >>isd->sd = subdev;
> >>isd->sd->host_priv = >bus;
> >> 
> >> -  return ret;
> >> +  return 0;
> >>  }
> >>  
> >>  static int isp_subdev_notifier_complete(struct v4l2_async_notifier
> >>  *async)
> >>  {
> >>struct isp_device *isp = container_of(async, struct isp_device,
> >>  notifier);
> >> +  struct v4l2_device *v4l2_dev = >v4l2_dev;
> >> +  struct v4l2_subdev *sd;
> >> +  struct isp_bus_cfg *bus;
> >> +  int ret;
> >> +
> >> +  list_for_each_entry(sd, _dev->subdevs, list) {
> >> +  /* Only try to link entities whose interface was set on bound */
> >> +  if (sd->host_priv) {
> >> +  bus = (struct isp_bus_cfg *)sd->host_priv;
> >> +  ret = isp_link_entity(isp, >entity, bus->interface);
> >> +  if (ret < 0)
> >> +  return ret;
> >> +  }
> >> +  }
> >>return v4l2_device_register_subdev_nodes(>v4l2_dev);
> >>  }
> > 
> > I think you're working around a problem here, not really fixing it.
> > 
> > This change will create the links only after the media device is
> > registered, which means the user may obtain a partial enumeration of
> > links if the enumeration is performed too early.
> > 
> > Before this set, the problem also was that the media device was registered
> > before the async entities were bound, again making it possible to obtain a
> > partial enumeration of entities.
> 
> You are absolutely correct but I think these are separate issues. The
> problem here is that v4l2_async_test_notify() [0] first invokes the bound
> notifier callback and then calls v4l2_device_register_subdev() that
> register the media entity with the media device.
> 
> Since now is a requirement that the entities must be registered prior
> creating pads links (because to init a MEDIA_GRAPH_LINK object a mdev has
> to be set), $SUBJECT is needed regardless of the race between subdev
> registration and the media dev node being available to user-space before
> everything is registered.
> > What I'd suggest instead is that we split media device initialisation and
> > registration to the system; that way the media device can be prepared
> > (entities registered and links created) before it becomes visible to the
> > user space. I can write a patch for that if you like.
> 
> Agreed, looking at the implementation it seems that
> __media_device_register() has to be split (possibly being renamed to
> __media_device_init) so it only contains the initialization logic and all
> the media device node registration logic moved to another function (that
> would become media_device_register).
> 
> I think the media dev node registration has to be made in the complete
> callback to make sure that happens when all the subdevs have been already
> registered.
> 
> Is that what you had in mind? I can also write such a patch if you want.

I 

Re: [PATCH v8 03/55] [media] omap3isp: get entity ID using media_entity_id()

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Sunday 30 August 2015 00:06:14 Mauro Carvalho Chehab wrote:
> From: Javier Martinez Canillas 
> 
> Assessing media_entity ID should now use media_entity_id() macro to

Did you mean "accessing" ?

> obtain the entity ID, as a next patch will remove the .id field from
> struct media_entity .
> 
> So, get rid of it, otherwise the omap3isp driver will fail to build.
> 
> Signed-off-by: Javier Martinez Canillas 
> Signed-off-by: Mauro Carvalho Chehab 

With the typo fixed,

Acked-by: Laurent Pinchart 

> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 56e683b19a73..e08183f9d0f7
> 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -975,6 +975,7 @@ static int isp_pipeline_disable(struct isp_pipeline
> *pipe) struct v4l2_subdev *subdev;
>   int failure = 0;
>   int ret;
> + u32 id;
> 
>   /*
>* We need to stop all the modules after CCDC first or they'll
> @@ -1027,8 +1028,10 @@ static int isp_pipeline_disable(struct isp_pipeline
> *pipe) if (ret) {
>   dev_info(isp->dev, "Unable to stop %s\n", subdev->name);
>   isp->stop_failure = true;
> - if (subdev == >isp_prev.subdev)
> - isp->crashed |= 1U << subdev->entity.id;
> + if (subdev == >isp_prev.subdev) {
> + id = media_entity_id(>entity);
> + isp->crashed |= 1U << id;
> + }
>   failure = -ETIMEDOUT;
>   }
>   }
> diff --git a/drivers/media/platform/omap3isp/ispccdc.c
> b/drivers/media/platform/omap3isp/ispccdc.c index
> 3b10304b580b..d96e3be5e252 100644
> --- a/drivers/media/platform/omap3isp/ispccdc.c
> +++ b/drivers/media/platform/omap3isp/ispccdc.c
> @@ -1608,7 +1608,7 @@ static int ccdc_isr_buffer(struct isp_ccdc_device
> *ccdc) /* Wait for the CCDC to become idle. */
>   if (ccdc_sbl_wait_idle(ccdc, 1000)) {
>   dev_info(isp->dev, "CCDC won't become idle!\n");
> - isp->crashed |= 1U << ccdc->subdev.entity.id;
> + isp->crashed |= 1U << media_entity_id(>subdev.entity);
>   omap3isp_pipeline_cancel_stream(pipe);
>   return 0;
>   }
> diff --git a/drivers/media/platform/omap3isp/ispvideo.c
> b/drivers/media/platform/omap3isp/ispvideo.c index
> 3094572f8897..6c89dc40df85 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -235,7 +235,7 @@ static int isp_video_get_graph_data(struct isp_video
> *video, while ((entity = media_entity_graph_walk_next())) {
>   struct isp_video *__video;
> 
> - pipe->entities |= 1 << entity->id;
> + pipe->entities |= 1 << media_entity_id(entity);
> 
>   if (far_end != NULL)
>   continue;
> @@ -891,6 +891,7 @@ static int isp_video_check_external_subdevs(struct
> isp_video *video, struct v4l2_ext_control ctrl;
>   unsigned int i;
>   int ret;
> + u32 id;
> 
>   /* Memory-to-memory pipelines have no external subdev. */
>   if (pipe->input != NULL)
> @@ -898,7 +899,7 @@ static int isp_video_check_external_subdevs(struct
> isp_video *video,
> 
>   for (i = 0; i < ARRAY_SIZE(ents); i++) {
>   /* Is the entity part of the pipeline? */
> - if (!(pipe->entities & (1 << ents[i]->id)))
> + if (!(pipe->entities & (1 << media_entity_id(ents[i]
>   continue;
> 
>   /* ISP entities have always sink pad == 0. Find source. */
> @@ -950,7 +951,8 @@ static int isp_video_check_external_subdevs(struct
> isp_video *video,
> 
>   pipe->external_rate = ctrl.value64;
> 
> - if (pipe->entities & (1 << isp->isp_ccdc.subdev.entity.id)) {
> + id = media_entity_id(>isp_ccdc.subdev.entity);
> + if (pipe->entities & (1 << id)) {
>   unsigned int rate = UINT_MAX;
>   /*
>* Check that maximum allowed CCDC pixel rate isn't

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 16/55] [media] media: Don't accept early-created links

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 06 September 2015 09:02:47 Mauro Carvalho Chehab wrote:
> Links are graph objects that represent the links of two already
> existing objects in the graph.
> 
> While with the current implementation, it is possible to create
> the links earlier, It doesn't make any sense to allow linking
> two objects when they are not both created.
> 
> So, remove the code that would be handling those early-created
> links and add a BUG_ON() to ensure that.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> Acked-by: Hans Verkuil 
> 
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 138b18416460..0d85c6c28004 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -443,13 +443,6 @@ int __must_check media_device_register_entity(struct
> media_device *mdev, media_gobj_init(mdev, MEDIA_GRAPH_ENTITY,
> >graph_obj);
>   list_add_tail(>list, >entities);
> 
> - /*
> -  * Initialize objects at the links
> -  * in the case where links got created before entity register
> -  */
> - for (i = 0; i < entity->num_links; i++)
> - media_gobj_init(mdev, MEDIA_GRAPH_LINK,
> - >links[i].graph_obj);
>   /* Initialize objects at the pads */
>   for (i = 0; i < entity->num_pads; i++)
>   media_gobj_init(mdev, MEDIA_GRAPH_PAD,
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 27fce6224972..0926f08be981 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -161,6 +161,8 @@ void media_gobj_init(struct media_device *mdev,
>  enum media_gobj_type type,
>  struct media_gobj *gobj)
>  {
> + BUG_ON(!mdev);
> +

Please use a WARN_ON() and return (and possibly make the function return an 
int error code), we don't want to panic completely for this.

>   gobj->mdev = mdev;
> 
>   /* Create a per-type unique object ID */

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 01/55] [media] media: create a macro to get entity ID

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 30 August 2015 00:06:12 Mauro Carvalho Chehab wrote:
> Instead of accessing directly entity.id, let's create a macro,
> as this field will be moved into a common struct later on.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> Acked-by: Hans Verkuil 

Acked-by: Laurent Pinchart 

> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index c55ab5029323..e429605ca2c3 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -77,8 +77,8 @@ static struct media_entity *find_entity(struct
> media_device *mdev, u32 id) spin_lock(>lock);
> 
>   media_device_for_each_entity(entity, mdev) {
> - if ((entity->id == id && !next) ||
> - (entity->id > id && next)) {
> + if (((media_entity_id(entity) == id) && !next) ||
> + ((media_entity_id(entity) > id) && next)) {
>   spin_unlock(>lock);
>   return entity;
>   }
> @@ -104,7 +104,7 @@ static long media_device_enum_entities(struct
> media_device *mdev, if (ent == NULL)
>   return -EINVAL;
> 
> - u_ent.id = ent->id;
> + u_ent.id = media_entity_id(ent);
>   if (ent->name)
>   strlcpy(u_ent.name, ent->name, sizeof(u_ent.name));
>   u_ent.type = ent->type;
> @@ -122,7 +122,7 @@ static long media_device_enum_entities(struct
> media_device *mdev, static void media_device_kpad_to_upad(const struct
> media_pad *kpad, struct media_pad_desc *upad)
>  {
> - upad->entity = kpad->entity->id;
> + upad->entity = media_entity_id(kpad->entity);
>   upad->index = kpad->index;
>   upad->flags = kpad->flags;
>  }
> diff --git a/drivers/media/media-entity.c b/drivers/media/media-entity.c
> index 949e5f92cbdc..cb0ac4e0dfa5 100644
> --- a/drivers/media/media-entity.c
> +++ b/drivers/media/media-entity.c
> @@ -140,10 +140,10 @@ void media_entity_graph_walk_start(struct
> media_entity_graph *graph, graph->stack[graph->top].entity = NULL;
>   bitmap_zero(graph->entities, MEDIA_ENTITY_ENUM_MAX_ID);
> 
> - if (WARN_ON(entity->id >= MEDIA_ENTITY_ENUM_MAX_ID))
> + if (WARN_ON(media_entity_id(entity) >= MEDIA_ENTITY_ENUM_MAX_ID))
>   return;
> 
> - __set_bit(entity->id, graph->entities);
> + __set_bit(media_entity_id(entity), graph->entities);
>   stack_push(graph, entity);
>  }
>  EXPORT_SYMBOL_GPL(media_entity_graph_walk_start);
> @@ -184,11 +184,11 @@ media_entity_graph_walk_next(struct media_entity_graph
> *graph)
> 
>   /* Get the entity in the other end of the link . */
>   next = media_entity_other(entity, link);
> - if (WARN_ON(next->id >= MEDIA_ENTITY_ENUM_MAX_ID))
> + if (WARN_ON(media_entity_id(next) >= MEDIA_ENTITY_ENUM_MAX_ID))
>   return NULL;
> 
>   /* Has the entity already been visited? */
> - if (__test_and_set_bit(next->id, graph->entities)) {
> + if (__test_and_set_bit(media_entity_id(next), graph->entities)) 
> {
>   link_top(graph)++;
>   continue;
>   }
> diff --git a/drivers/media/platform/vsp1/vsp1_video.c
> b/drivers/media/platform/vsp1/vsp1_video.c index 17f08973f835..debe4e539df6
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_video.c
> +++ b/drivers/media/platform/vsp1/vsp1_video.c
> @@ -352,10 +352,10 @@ static int vsp1_pipeline_validate_branch(struct
> vsp1_pipeline *pipe, break;
> 
>   /* Ensure the branch has no loop. */
> - if (entities & (1 << entity->subdev.entity.id))
> + if (entities & (1 << media_entity_id(>subdev.entity)))
>   return -EPIPE;
> 
> - entities |= 1 << entity->subdev.entity.id;
> + entities |= 1 << media_entity_id(>subdev.entity);
> 
>   /* UDS can't be chained. */
>   if (entity->type == VSP1_ENTITY_UDS) {
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 8b21a4d920d9..0a66fc225559 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -113,6 +113,11 @@ static inline u32 media_entity_subtype(struct
> media_entity *entity) return entity->type & MEDIA_ENT_SUBTYPE_MASK;
>  }
> 
> +static inline u32 media_entity_id(struct media_entity *entity)
> +{
> + return entity->id;
> +}
> +
>  #define MEDIA_ENTITY_ENUM_MAX_DEPTH  16
>  #define MEDIA_ENTITY_ENUM_MAX_ID 64

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

cron job: media_tree daily build: ERRORS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Sun Dec  6 04:00:16 CET 2015
git branch: test
git hash:   21312f6ddb1710750761c4b140b7367208b4f89e
gcc version:i686-linux-gcc (GCC) 5.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3202-g618e15b
host hardware:  x86_64
host os:4.2.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-exynos: OK
linux-git-arm-mx: OK
linux-git-arm-omap: OK
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.32.27-i686: ERRORS
linux-2.6.33.7-i686: ERRORS
linux-2.6.34.7-i686: ERRORS
linux-2.6.35.9-i686: ERRORS
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: OK
linux-3.0.60-i686: OK
linux-3.1.10-i686: OK
linux-3.2.37-i686: OK
linux-3.3.8-i686: OK
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: OK
linux-3.7.4-i686: OK
linux-3.8-i686: OK
linux-3.9.2-i686: OK
linux-3.10.1-i686: OK
linux-3.11.1-i686: OK
linux-3.12.23-i686: OK
linux-3.13.11-i686: OK
linux-3.14.9-i686: OK
linux-3.15.2-i686: OK
linux-3.16.7-i686: OK
linux-3.17.8-i686: OK
linux-3.18.7-i686: OK
linux-3.19-i686: OK
linux-4.0-i686: OK
linux-4.1.1-i686: OK
linux-4.2-i686: OK
linux-4.3-i686: OK
linux-4.4-rc1-i686: OK
linux-2.6.32.27-x86_64: ERRORS
linux-2.6.33.7-x86_64: ERRORS
linux-2.6.34.7-x86_64: ERRORS
linux-2.6.35.9-x86_64: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: OK
linux-3.0.60-x86_64: OK
linux-3.1.10-x86_64: OK
linux-3.2.37-x86_64: OK
linux-3.3.8-x86_64: OK
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: OK
linux-3.7.4-x86_64: OK
linux-3.8-x86_64: OK
linux-3.9.2-x86_64: OK
linux-3.10.1-x86_64: OK
linux-3.11.1-x86_64: OK
linux-3.12.23-x86_64: OK
linux-3.13.11-x86_64: OK
linux-3.14.9-x86_64: OK
linux-3.15.2-x86_64: OK
linux-3.16.7-x86_64: OK
linux-3.17.8-x86_64: OK
linux-3.18.7-x86_64: OK
linux-3.19-x86_64: OK
linux-4.0-x86_64: OK
linux-4.1.1-x86_64: OK
linux-4.2-x86_64: OK
linux-4.3-x86_64: OK
linux-4.4-rc1-x86_64: OK
apps: WARNINGS
spec-git: WARNINGS
sparse: ERRORS
smatch: ERRORS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Sunday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Sunday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 37/55] [media] omap4iss: stop MEDIA_ENT_T_V4L2_SUBDEV abuse

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 30 August 2015 00:06:48 Mauro Carvalho Chehab wrote:
> This driver is abusing MEDIA_ENT_T_V4L2_SUBDEV, as it uses a
> hack to check if the remote entity is a subdev. Get rid of it.

While I agree with the idea of the patch I don't think this is a hack, it was 
a totally valid implementation with the existing API.

> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/staging/media/omap4iss/iss_ipipe.c
> b/drivers/staging/media/omap4iss/iss_ipipe.c index
> e1a7b7ba7362..eb91ec48a21e 100644
> --- a/drivers/staging/media/omap4iss/iss_ipipe.c
> +++ b/drivers/staging/media/omap4iss/iss_ipipe.c
> @@ -447,8 +447,11 @@ static int ipipe_link_setup(struct media_entity
> *entity, struct iss_ipipe_device *ipipe = v4l2_get_subdevdata(sd);
>   struct iss_device *iss = to_iss_device(ipipe);
> 
> - switch (local->index | media_entity_type(remote->entity)) {
> - case IPIPE_PAD_SINK | MEDIA_ENT_T_V4L2_SUBDEV:
> + if (!is_media_entity_v4l2_subdev(remote->entity))
> + return -EINVAL;
> +

Furthermore the ipipe entity is never connected to anything else than a 
subdev, so you can remove this check completely.

I'd rewrite the subject line as "omap4iss: ipipe: Don't check entity type 
needlessly during link setup" and update the commit message accordingly.

> + switch (local->index) {
> + case IPIPE_PAD_SINK:
>   /* Read from IPIPEIF. */
>   if (!(flags & MEDIA_LNK_FL_ENABLED)) {
>   ipipe->input = IPIPE_INPUT_NONE;
> @@ -463,7 +466,7 @@ static int ipipe_link_setup(struct media_entity *entity,
> 
>   break;
> 
> - case IPIPE_PAD_SOURCE_VP | MEDIA_ENT_T_V4L2_SUBDEV:
> + case IPIPE_PAD_SOURCE_VP:
>   /* Send to RESIZER */
>   if (flags & MEDIA_LNK_FL_ENABLED) {
>   if (ipipe->output & ~IPIPE_OUTPUT_VP)

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 36/55] [media] davinci_vbpe: stop MEDIA_ENT_T_V4L2_SUBDEV abuse

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 30 August 2015 00:06:47 Mauro Carvalho Chehab wrote:
> This driver is abusing MEDIA_ENT_T_V4L2_SUBDEV:
> 
> - it uses a hack to check if the remote entity is a subdev;

Same comment as for "omap4iss: stop MEDIA_ENT_T_V4L2_SUBDEV abuse", this isn't 
a hack.

> - it still uses the legacy entity subtype check macro, that
>   will be removed soon.
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c index
> b89a057b8b7e..7fd78329e3e1 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> @@ -1711,8 +1711,11 @@ ipipe_link_setup(struct media_entity *entity, const
> struct media_pad *local, struct vpfe_device *vpfe_dev =
> to_vpfe_device(ipipe);
>   u16 ipipeif_sink = vpfe_dev->vpfe_ipipeif.input;
> 
> - switch (local->index | media_entity_type(remote->entity)) {
> - case IPIPE_PAD_SINK | MEDIA_ENT_T_V4L2_SUBDEV:
> + if (!is_media_entity_v4l2_subdev(remote->entity))
> + return -EINVAL;

You can drop the check (even though the implementation in the switch looks 
dubious to me, but that's not your fault).

> + switch (local->index) {
> + case IPIPE_PAD_SINK:
>   if (!(flags & MEDIA_LNK_FL_ENABLED)) {
>   ipipe->input = IPIPE_INPUT_NONE;
>   break;
> @@ -1725,7 +1728,7 @@ ipipe_link_setup(struct media_entity *entity, const
> struct media_pad *local, ipipe->input = IPIPE_INPUT_CCDC;
>   break;
> 
> - case IPIPE_PAD_SOURCE | MEDIA_ENT_T_V4L2_SUBDEV:
> + case IPIPE_PAD_SOURCE:
>   /* out to RESIZER */
>   if (flags & MEDIA_LNK_FL_ENABLED)
>   ipipe->output = IPIPE_OUTPUT_RESIZER;
> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c
> b/drivers/staging/media/davinci_vpfe/vpfe_video.c index
> 9eef64e0f0ab..2dbf14b9bb5f 100644
> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
> @@ -88,7 +88,7 @@ vpfe_video_remote_subdev(struct vpfe_video_device *video,
> u32 *pad) {
>   struct media_pad *remote = media_entity_remote_pad(>pad);
> 
> - if (remote == NULL || remote->entity->type != MEDIA_ENT_T_V4L2_SUBDEV)
> + if (!remote || !is_media_entity_v4l2_subdev(remote->entity))
>   return NULL;
>   if (pad)
>   *pad = remote->index;
> @@ -243,8 +243,7 @@ static int vpfe_video_validate_pipeline(struct
> vpfe_pipeline *pipe)
> 
>   /* Retrieve the source format */
>   pad = media_entity_remote_pad(pad);
> - if (pad == NULL ||
> - pad->entity->type != MEDIA_ENT_T_V4L2_SUBDEV)
> + if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
>   break;
> 
>   subdev = media_entity_to_v4l2_subdev(pad->entity);

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 35/55] [media] s5k5baf: fix subdev type

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 30 August 2015 00:06:46 Mauro Carvalho Chehab wrote:
> X-Patchwork-Delegate: m.che...@samsung.com
> This sensor driver is abusing MEDIA_ENT_T_V4L2_SUBDEV, creating
> some subdevs with a non-existing type.
> 
> As this is a sensor driver, the proper type is likely
> MEDIA_ENT_T_V4L2_SUBDEV_SENSOR.

That's actually not correct. The driver creates two subdevs, one for the image 
sensor pixel array (and the related readout logic) and one for an ISP. The 
first subdev already uses the MEDIA_ENT_T_V4L2_SUBDEV_SENSOR type, but the 
second subdev isn't a sensor pixel array.

> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
> index d3bff30bcb6f..0513196bd48c 100644
> --- a/drivers/media/i2c/s5k5baf.c
> +++ b/drivers/media/i2c/s5k5baf.c
> @@ -1919,7 +1919,7 @@ static int s5k5baf_configure_subdevs(struct s5k5baf
> *state,
> 
>   state->pads[PAD_CIS].flags = MEDIA_PAD_FL_SINK;
>   state->pads[PAD_OUT].flags = MEDIA_PAD_FL_SOURCE;
> - sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV;
> + sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
>   ret = media_entity_init(>entity, NUM_ISP_PADS, state->pads);
> 
>   if (!ret)

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 31/55] [media] media: add macros to check if subdev or V4L2 DMA

[Laurent Pinchart]

Hi Mauro,

Thank you for the patch.

On Sunday 06 September 2015 09:02:57 Mauro Carvalho Chehab wrote:
> As we'll be removing entity subtypes from the Kernel, we need
> to provide a way for drivers and core to check if a given
> entity is represented by a V4L2 subdev or if it is an V4L2
> I/O entity (typically with DMA).
> 
> Signed-off-by: Mauro Carvalho Chehab 
> 
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 4e36b1f2b2d7..220864319d21 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -220,6 +220,39 @@ static inline u32 media_gobj_gen_id(enum
> media_gobj_type type, u32 local_id) return id;
>  }
> 
> +static inline bool is_media_entity_v4l2_io(struct media_entity *entity)
> +{
> + if (!entity)
> + return false;
> +
> + switch (entity->type) {
> + case MEDIA_ENT_T_V4L2_VIDEO:
> + case MEDIA_ENT_T_V4L2_VBI:
> + case MEDIA_ENT_T_V4L2_SWRADIO:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static inline bool is_media_entity_v4l2_subdev(struct media_entity *entity)
> +{
> + if (!entity)
> + return false;
> +
> + switch (entity->type) {
> + case MEDIA_ENT_T_V4L2_SUBDEV_SENSOR:
> + case MEDIA_ENT_T_V4L2_SUBDEV_FLASH:
> + case MEDIA_ENT_T_V4L2_SUBDEV_LENS:
> + case MEDIA_ENT_T_V4L2_SUBDEV_DECODER:
> + case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:

I'm sorry but this simply won't scale. We need a better way to determine the 
entity type, and this could be a valid use case to actually retain an entity 
type field separate from the function, at least inside the kernel.

> + return true;
> +
> + default:
> + return false;
> + }
> +}
> +
>  #define MEDIA_ENTITY_ENUM_MAX_DEPTH  16
>  #define MEDIA_ENTITY_ENUM_MAX_ID 64

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/5] [media] smiapp: create pad links after subdev registration

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Thursday 03 September 2015 18:00:36 Javier Martinez Canillas wrote:
> The smiapp driver creates the pads links before the media entity is
> registered with the media device. This doesn't work now that object
> IDs are used to create links so the media_device has to be set.
> 
> Move entity registration logic before pads links creation.
> 
> Signed-off-by: Javier Martinez Canillas 

Acked-by: Laurent Pinchart 

> ---
> 
>  drivers/media/i2c/smiapp/smiapp-core.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c
> b/drivers/media/i2c/smiapp/smiapp-core.c index 5aa49eb393a9..938201789ebc
> 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -2495,23 +2495,23 @@ static int smiapp_register_subdevs(struct
> smiapp_sensor *sensor) return rval;
>   }
> 
> - rval = media_create_pad_link(>sd.entity,
> - this->source_pad,
> - >sd.entity,
> - last->sink_pad,
> - MEDIA_LNK_FL_ENABLED |
> - MEDIA_LNK_FL_IMMUTABLE);
> + rval = v4l2_device_register_subdev(sensor->src->sd.v4l2_dev,
> +>sd);
>   if (rval) {
>   dev_err(>dev,
> - "media_create_pad_link failed\n");
> + "v4l2_device_register_subdev failed\n");
>   return rval;
>   }
> 
> - rval = v4l2_device_register_subdev(sensor->src->sd.v4l2_dev,
> ->sd);
> + rval = media_create_pad_link(>sd.entity,
> +  this->source_pad,
> +  >sd.entity,
> +  last->sink_pad,
> +  MEDIA_LNK_FL_ENABLED |
> +  MEDIA_LNK_FL_IMMUTABLE);
>   if (rval) {
>   dev_err(>dev,
> - "v4l2_device_register_subdev failed\n");
> + "media_create_pad_link failed\n");
>   return rval;
>   }
>   }

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/5] [media] uvcvideo: create pad links after subdev registration

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Monday 07 September 2015 16:10:25 Javier Martinez Canillas wrote:

[snip]

> From 8be356e77eeefdc5c0738dd429205f3398c5b76c Mon Sep 17 00:00:00 2001
> From: Javier Martinez Canillas 
> Date: Thu, 3 Sep 2015 13:46:06 +0200
> Subject: [PATCH v2 4/5] [media] uvcvideo: create pad links after subdev
>  registration
> 
> The uvc driver creates the pads links before the media entity is
> registered with the media device. This doesn't work now that obj
> IDs are used to create links so the media_device has to be set.
> 
> Move entities registration logic before pads links creation.
> 
> Signed-off-by: Javier Martinez Canillas 
> ---
> 
> Changes since v1:
>  - Don't try to register a UVC entity subdev for type UVC_TT_STREAMING.
> 
>  drivers/media/usb/uvc/uvc_entity.c | 23 +++
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_entity.c
> b/drivers/media/usb/uvc/uvc_entity.c index 429e428ccd93..7f82b65b238e
> 100644
> --- a/drivers/media/usb/uvc/uvc_entity.c
> +++ b/drivers/media/usb/uvc/uvc_entity.c
> @@ -26,6 +26,15 @@
>  static int uvc_mc_register_entity(struct uvc_video_chain *chain,
> struct uvc_entity *entity)
>  {
> +   if (UVC_ENTITY_TYPE(entity) == UVC_TT_STREAMING)
> +   return 0;
> +
> +   return v4l2_device_register_subdev(>dev->vdev,
> >subdev);
> +}
> +
> +static int uvc_mc_create_pads_links(struct uvc_video_chain *chain,
> +   struct uvc_entity *entity)
> +{
> const u32 flags = MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE;
> struct media_entity *sink;
> unsigned int i;
> @@ -62,10 +71,7 @@ static int uvc_mc_register_entity(struct uvc_video_chain
> *chain, return ret;
> }
>  
> -   if (UVC_ENTITY_TYPE(entity) == UVC_TT_STREAMING)
> -   return 0;
> -
> -   return v4l2_device_register_subdev(>dev->vdev,
> >subdev);
> +   return 0;
>  }
>  
>  static struct v4l2_subdev_ops uvc_subdev_ops = {
> @@ -124,5 +130,14 @@ int uvc_mc_register_entities(struct uvc_video_chain
> *chain) }
> }
>  
> +   list_for_each_entry(entity, >entities, chain) {
> +   ret = uvc_mc_create_pads_links(chain, entity);

You can call this uvc_mc_create_links(), there's no other type of links in the 
driver.

> +   if (ret < 0) {
> +   uvc_printk(KERN_INFO, "Failed to create pads links
> for "

Same here, I'd s/pad links/links/.

> +  "entity %u\n", entity->id);
> +   return ret;
> +   }
> +   }

This creates three loops, and I think that's one too much. The reason why init 
and register are separate is that the latter creates links, which requires all 
entities to be initialized. If you move link create after registration I 
believe you can init and register in a single loop (just move the 
v4l2_device_register_subdev() call in the appropriate location in 
uvc_mc_init_entity()) and then create links in a second loop.

> return 0;
>  }

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/5] [media] v4l: vsp1: separate links creation from entities init

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Thursday 03 September 2015 18:00:34 Javier Martinez Canillas wrote:
> The vsp1 driver initializes the entities and creates the pads links
> before the entities are registered with the media device. This doesn't
> work now that object IDs are used to create links so the media_device
> has to be set.
> 
> Split out the pads links creation from the entity initialization so are
> made after the entities registration.
> 
> Signed-off-by: Javier Martinez Canillas 
> ---
> 
>  drivers/media/platform/vsp1/vsp1_drv.c  | 14 ++--
>  drivers/media/platform/vsp1/vsp1_rpf.c  | 29 
>  drivers/media/platform/vsp1/vsp1_rwpf.h |  5 +
>  drivers/media/platform/vsp1/vsp1_wpf.c  | 40 +-
>  4 files changed, 62 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
> b/drivers/media/platform/vsp1/vsp1_drv.c index 2aa427d3ff39..8f995d267646
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -260,9 +260,19 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1)
> 
>   /* Create links. */
>   list_for_each_entry(entity, >entities, list_dev) {
> - if (entity->type == VSP1_ENTITY_LIF ||
> - entity->type == VSP1_ENTITY_RPF)
> + if (entity->type == VSP1_ENTITY_LIF) {
> + ret = vsp1_wpf_create_pads_links(vsp1, entity);

Could you please s/pads_links/links/ ? There's no other type of links handled 
by the driver.

> + if (ret < 0)
> + goto done;
> + continue;

I would use

} else if (...) {

instead of a continue.

> + }
> +
> + if (entity->type == VSP1_ENTITY_RPF) {
> + ret = vsp1_rpf_create_pads_links(vsp1, entity);
> + if (ret < 0)
> + goto done;
>   continue;

Same here.

Apart from that,

Acked-by: Laurent Pinchart 

> + }
> 
>   ret = vsp1_create_links(vsp1, entity);
>   if (ret < 0)
> diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c
> b/drivers/media/platform/vsp1/vsp1_rpf.c index b60a528a8fe8..38aebdf691b5
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rpf.c
> @@ -277,18 +277,29 @@ struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device
> *vsp1, unsigned int index)
> 
>   rpf->entity.video = video;
> 
> - /* Connect the video device to the RPF. */
> - ret = media_create_pad_link(>video.video.entity, 0,
> ->entity.subdev.entity,
> -RWPF_PAD_SINK,
> -MEDIA_LNK_FL_ENABLED |
> -MEDIA_LNK_FL_IMMUTABLE);
> - if (ret < 0)
> - goto error;
> -
>   return rpf;
> 
>  error:
>   vsp1_entity_destroy(>entity);
>   return ERR_PTR(ret);
>  }
> +
> +/*
> + * vsp1_rpf_create_pads_links_create_pads_links() - RPF pads links creation
> + * @vsp1: Pointer to VSP1 device
> + * @entity: Pointer to VSP1 entity
> + *
> + * return negative error code or zero on success
> + */
> +int vsp1_rpf_create_pads_links(struct vsp1_device *vsp1,
> +struct vsp1_entity *entity)
> +{
> + struct vsp1_rwpf *rpf = to_rwpf(>subdev);
> +
> + /* Connect the video device to the RPF. */
> + return media_create_pad_link(>video.video.entity, 0,
> +  >entity.subdev.entity,
> +  RWPF_PAD_SINK,
> +  MEDIA_LNK_FL_ENABLED |
> +  MEDIA_LNK_FL_IMMUTABLE);
> +}
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.h
> b/drivers/media/platform/vsp1/vsp1_rwpf.h index f452dce1a931..6638b3587369
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.h
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.h
> @@ -50,6 +50,11 @@ static inline struct vsp1_rwpf *to_rwpf(struct
> v4l2_subdev *subdev) struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device
> *vsp1, unsigned int index); struct vsp1_rwpf *vsp1_wpf_create(struct
> vsp1_device *vsp1, unsigned int index);
> 
> +int vsp1_rpf_create_pads_links(struct vsp1_device *vsp1,
> +struct vsp1_entity *entity);
> +int vsp1_wpf_create_pads_links(struct vsp1_device *vsp1,
> +struct vsp1_entity *entity);
> +
>  int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev,
>struct v4l2_subdev_pad_config *cfg,
>struct v4l2_subdev_mbus_code_enum *code);
> diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c
> b/drivers/media/platform/vsp1/vsp1_wpf.c index d39aa4b8aea1..1be363e4f741
> 100644
> --- 

Re: [PATCH 1/5] [media] staging: omap4iss: separate links creation from entities init

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Thursday 03 September 2015 18:00:32 Javier Martinez Canillas wrote:
> The omap4iss driver initializes the entities and creates the pads links
> before the entities are registered with the media device. This does not
> work now that object IDs are used to create links so the media_device
> has to be set.
> 
> Split out the pads links creation from the entity initialization so are
> made after the entities registration.
> 
> Signed-off-by: Javier Martinez Canillas 
> ---
> 
>  drivers/staging/media/omap4iss/iss.c | 101 +++-
>  drivers/staging/media/omap4iss/iss_csi2.c|  35 +++---
>  drivers/staging/media/omap4iss/iss_csi2.h|   1 +
>  drivers/staging/media/omap4iss/iss_ipipeif.c |  29 
>  drivers/staging/media/omap4iss/iss_ipipeif.h |   1 +
>  drivers/staging/media/omap4iss/iss_resizer.c |  29 
>  drivers/staging/media/omap4iss/iss_resizer.h |   1 +
>  7 files changed, 132 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/staging/media/omap4iss/iss.c
> b/drivers/staging/media/omap4iss/iss.c index 44b88ff3ba83..076ddd412201
> 100644
> --- a/drivers/staging/media/omap4iss/iss.c
> +++ b/drivers/staging/media/omap4iss/iss.c
> @@ -1272,6 +1272,68 @@ done:
>   return ret;
>  }
> 
> +/*
> + * iss_create_pads_links() - Pads links creation for the subdevices

Could you please s/pads_links/links/ and s/pads links/links/ ?

Apart from that,

Acked-by: Laurent Pinchart 

> + * @iss : Pointer to ISS device
> + *
> + * return negative error code or zero on success
> + */
> +static int iss_create_pads_links(struct iss_device *iss)
> +{
> + int ret;
> +
> + ret = omap4iss_csi2_create_pads_links(iss);
> + if (ret < 0) {
> + dev_err(iss->dev, "CSI2 pads links creation failed\n");
> + return ret;
> + }
> +
> + ret = omap4iss_ipipeif_create_pads_links(iss);
> + if (ret < 0) {
> + dev_err(iss->dev, "ISP IPIPEIF pads links creation failed\n");
> + return ret;
> + }
> +
> + ret = omap4iss_resizer_create_pads_links(iss);
> + if (ret < 0) {
> + dev_err(iss->dev, "ISP RESIZER pads links creation failed\n");
> + return ret;
> + }
> +
> + /* Connect the submodules. */
> + ret = media_create_pad_link(
> + >csi2a.subdev.entity, CSI2_PAD_SOURCE,
> + >ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
> + if (ret < 0)
> + return ret;
> +
> + ret = media_create_pad_link(
> + >csi2b.subdev.entity, CSI2_PAD_SOURCE,
> + >ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
> + if (ret < 0)
> + return ret;
> +
> + ret = media_create_pad_link(
> + >ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
> + >resizer.subdev.entity, RESIZER_PAD_SINK, 0);
> + if (ret < 0)
> + return ret;
> +
> + ret = media_create_pad_link(
> + >ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
> + >ipipe.subdev.entity, IPIPE_PAD_SINK, 0);
> + if (ret < 0)
> + return ret;
> +
> + ret = media_create_pad_link(
> + >ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP,
> + >resizer.subdev.entity, RESIZER_PAD_SINK, 0);
> + if (ret < 0)
> + return ret;
> +
> + return 0;
> +};
> +
>  static void iss_cleanup_modules(struct iss_device *iss)
>  {
>   omap4iss_csi2_cleanup(iss);
> @@ -1314,41 +1376,8 @@ static int iss_initialize_modules(struct iss_device
> *iss) goto error_resizer;
>   }
> 
> - /* Connect the submodules. */
> - ret = media_create_pad_link(
> - >csi2a.subdev.entity, CSI2_PAD_SOURCE,
> - >ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
> - if (ret < 0)
> - goto error_link;
> -
> - ret = media_create_pad_link(
> - >csi2b.subdev.entity, CSI2_PAD_SOURCE,
> - >ipipeif.subdev.entity, IPIPEIF_PAD_SINK, 0);
> - if (ret < 0)
> - goto error_link;
> -
> - ret = media_create_pad_link(
> - >ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
> - >resizer.subdev.entity, RESIZER_PAD_SINK, 0);
> - if (ret < 0)
> - goto error_link;
> -
> - ret = media_create_pad_link(
> - >ipipeif.subdev.entity, IPIPEIF_PAD_SOURCE_VP,
> - >ipipe.subdev.entity, IPIPE_PAD_SINK, 0);
> - if (ret < 0)
> - goto error_link;
> -
> - ret = media_create_pad_link(
> - >ipipe.subdev.entity, IPIPE_PAD_SOURCE_VP,
> - >resizer.subdev.entity, RESIZER_PAD_SINK, 0);
> - if (ret < 0)
> - goto error_link;
> -
>   return 0;
> 
> -error_link:
> - 

Re: [PATCH v8 02/55] [media] staging: omap4iss: get entity ID using media_entity_id()

[Laurent Pinchart]

Hi Javier,

Thank you for the patch.

On Sunday 30 August 2015 00:06:13 Mauro Carvalho Chehab wrote:
> From: Javier Martinez Canillas 
> 
> Assessing media_entity ID should now use media_entity_id() macro to

Did you mean "accessing" ?

> obtain the entity ID, as a next patch will remove the .id field from
> struct media_entity .
> 
> So, get rid of it, otherwise the omap4iss driver will fail to build.
> 
> Signed-off-by: Javier Martinez Canillas 
> Acked-by: Hans Verkuil 
> Signed-off-by: Mauro Carvalho Chehab 

With the typo fixed,

Acked-by: Laurent Pinchart 

> diff --git a/drivers/staging/media/omap4iss/iss.c
> b/drivers/staging/media/omap4iss/iss.c index 9bfb725b9986..e54a7afd31de
> 100644
> --- a/drivers/staging/media/omap4iss/iss.c
> +++ b/drivers/staging/media/omap4iss/iss.c
> @@ -607,7 +607,7 @@ static int iss_pipeline_disable(struct iss_pipeline
> *pipe, * crashed. Mark it as such, the ISS will be reset when
>* applications will release it.
>*/
> - iss->crashed |= 1U << subdev->entity.id;
> + iss->crashed |= 1U << media_entity_id(>entity);
>   failure = -ETIMEDOUT;
>   }
>   }
> diff --git a/drivers/staging/media/omap4iss/iss_video.c
> b/drivers/staging/media/omap4iss/iss_video.c index
> bae67742706f..25e9e7a6b99d 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -784,7 +784,7 @@ iss_video_streamon(struct file *file, void *fh, enum
> v4l2_buf_type type) entity = >video.entity;
>   media_entity_graph_walk_start(, entity);
>   while ((entity = media_entity_graph_walk_next()))
> - pipe->entities |= 1 << entity->id;
> + pipe->entities |= 1 << media_entity_id(entity);
> 
>   /* Verify that the currently configured format matches the output of
>* the connected subdev.

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html