| Message ID | afb84e3d80fc4f6f2465a123012f161b8c29f1c4.1431046915.git.mchehab@osg.samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote: > At the Video4Linux API, the /dev/video?, /dev/vbi? and > /dev/radio? device nodes are used for the chipset that You should add /dev/swradio? for SDR devices. > provides the bridge between video/radio streams and the > USB, PCI or CPU buses. > > Such bridge is also typically used to control the V4L2 device > as a hole. hole -> whole > > For video streaming devices and SDR radio devices, they're > also associated with the DMA engines that transfer the > video stream (or SDR stream) to the CPU's memory. > > It should be noticed, however, this is not true on non-SDR > radio devices, I think you forgot that SDR devices are not using /dev/radio but /dev/swradio. They have different names. Radio devices never stream (OK, I think there are one or two exceptions, but that's really because nobody bothered to make an alsa driver. Those boards are in practice out of spec.) > and may also not be true on embedded devices > that, due to DRM reasons, don't allow writing unencrypted > data on a memory that could be seen by the CPU. This actually might still work by using opaque DMABUF handles. But that's under discussion right now in the Secure Data Path thread. Another reason can also be that the SoC vendor re-invented the wheel and made there own DMA streaming solution. You can still make V4L drivers that control the video receivers/transmitters, but for the actual streaming you are forced to use the vendor's crap code (hello TI!). I've bitter experiences with that :-( > So, we'll eventually need to add another entity for such > bridge chipsets that have a video/vbi/radio device node > associated, but don't have DMA engines on (some) devnodes. > > As, currently, we don't have any such case, ??? Radio devices are exactly that. > let's for now > just rename the device nodes that are associated with a > DMA engine as MEDIA_ENT_T_AV_DMA. > > So, > MEDIA_ENT_T_DEVNODE_V4L -> MEDIA_ENT_T_AV_DMA > > PS.: This is not actually true for USB devices, as the > DMA engine is an internal component, as it is up to the > Kernel to strip the stream payload from the URB packages. How about MEDIA_ENT_T_DATA_STREAMING? Or perhaps DATA_IO? Perhaps even just "IO"? That would cover USB as well, and I dislike the use of "AV", since the data might contain other things besides audio and/or video. Regards, Hans > > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > > diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml > index 5872f8bbf774..5b8147629159 100644 > --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml > +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml > @@ -183,7 +183,7 @@ > <entry>Unknown device node</entry> > </row> > <row> > - <entry><constant>MEDIA_ENT_T_DEVNODE_V4L</constant></entry> > + <entry><constant>MEDIA_ENT_T_AV_DMA</constant></entry> > <entry>V4L video, radio or vbi device node</entry> > </row> > <row> > diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c > index efde88adf624..7fa0cc0f08f0 100644 > --- a/drivers/media/platform/xilinx/xilinx-dma.c > +++ b/drivers/media/platform/xilinx/xilinx-dma.c > @@ -193,7 +193,7 @@ static int xvip_pipeline_validate(struct xvip_pipeline *pipe, > while ((entity = media_entity_graph_walk_next(&graph))) { > struct xvip_dma *dma; > > - if (entity->type != MEDIA_ENT_T_DEVNODE_V4L) > + if (entity->type != MEDIA_ENT_T_AV_DMA) > continue; > > dma = to_xvip_dma(media_entity_to_video_device(entity)); > diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c > index 71a1b93b0790..9ef920221b5a 100644 > --- a/drivers/media/v4l2-core/v4l2-dev.c > +++ b/drivers/media/v4l2-core/v4l2-dev.c > @@ -912,7 +912,7 @@ int __video_register_device(struct video_device *vdev, int type, int nr, > /* Part 5: Register the entity. */ > if (vdev->v4l2_dev->mdev && > vdev->vfl_type != VFL_TYPE_SUBDEV) { > - vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L; > + vdev->entity.type = MEDIA_ENT_T_AV_DMA; > vdev->entity.name = vdev->name; > vdev->entity.info.dev.major = VIDEO_MAJOR; > vdev->entity.info.dev.minor = vdev->minor; > diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c > index 63596063b213..9f8fc8330b3e 100644 > --- a/drivers/media/v4l2-core/v4l2-subdev.c > +++ b/drivers/media/v4l2-core/v4l2-subdev.c > @@ -535,7 +535,7 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad, > return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt); > } > > - WARN(pad->entity->type != MEDIA_ENT_T_DEVNODE_V4L, > + WARN(pad->entity->type != MEDIA_ENT_T_AV_DMA, > "Driver bug! Wrong media entity type 0x%08x, entity %s\n", > pad->entity->type, pad->entity->name); > > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h > index 775c11c6b173..a7aa2aac9c23 100644 > --- a/include/uapi/linux/media.h > +++ b/include/uapi/linux/media.h > @@ -44,12 +44,12 @@ struct media_device_info { > > /* Used values for media_entity_desc::type */ > > -#define MEDIA_ENT_T_DEVNODE_V4L (((1 << 16)) + 1) > -#define MEDIA_ENT_T_DEVNODE_DVB_FE (MEDIA_ENT_T_DEVNODE_V4L + 3) > -#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX (MEDIA_ENT_T_DEVNODE_V4L + 4) > -#define MEDIA_ENT_T_DEVNODE_DVB_DVR (MEDIA_ENT_T_DEVNODE_V4L + 5) > -#define MEDIA_ENT_T_DEVNODE_DVB_CA (MEDIA_ENT_T_DEVNODE_V4L + 6) > -#define MEDIA_ENT_T_DEVNODE_DVB_NET (MEDIA_ENT_T_DEVNODE_V4L + 7) > +#define MEDIA_ENT_T_AV_DMA (((1 << 16)) + 1) > +#define MEDIA_ENT_T_DEVNODE_DVB_FE (MEDIA_ENT_T_AV_DMA + 3) > +#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX (MEDIA_ENT_T_AV_DMA + 4) > +#define MEDIA_ENT_T_DEVNODE_DVB_DVR (MEDIA_ENT_T_AV_DMA + 5) > +#define MEDIA_ENT_T_DEVNODE_DVB_CA (MEDIA_ENT_T_AV_DMA + 6) > +#define MEDIA_ENT_T_DEVNODE_DVB_NET (MEDIA_ENT_T_AV_DMA + 7) > > #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR ((2 << 16) + 1) > #define MEDIA_ENT_T_V4L2_SUBDEV_FLASH (MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 1) >
Em Fri, 08 May 2015 13:54:07 +0200 Hans Verkuil <hverkuil@xs4all.nl> escreveu: > On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote: > > At the Video4Linux API, the /dev/video?, /dev/vbi? and > > /dev/radio? device nodes are used for the chipset that I knew that this patch would cause some discussions ;) > > You should add /dev/swradio? for SDR devices. I will. > > > provides the bridge between video/radio streams and the > > USB, PCI or CPU buses. > > > > Such bridge is also typically used to control the V4L2 device > > as a hole. > > hole -> whole Ok. > > > > For video streaming devices and SDR radio devices, they're > > also associated with the DMA engines that transfer the > > video stream (or SDR stream) to the CPU's memory. > > > > It should be noticed, however, this is not true on non-SDR > > radio devices, > > I think you forgot that SDR devices are not using /dev/radio > but /dev/swradio. Yeah, true. I forgot that, in the end of the day, we used a different naming for SDR radio devnodes. I'll fix the comments. > They have different names. Radio devices never > stream (OK, I think there are one or two exceptions, but that's really > because nobody bothered to make an alsa driver. Those boards are > in practice out of spec.) Yes, I know. > > and may also not be true on embedded devices > > that, due to DRM reasons, don't allow writing unencrypted > > data on a memory that could be seen by the CPU. > > This actually might still work by using opaque DMABUF handles. But that's > under discussion right now in the Secure Data Path thread. Well, a DMABUF opaque handler like that actually refers to either a buffer that is shared only between 2 devices or to a device-to-device DMA transfer. Such dataflow is different than the usual meaning of the video devnode, where the devnode is used to do I/O transfers. So, it may actually be mapped as a different type of entity. We'll need to discuss further when we start mapping this via MC. > Another reason can also be that the SoC vendor re-invented the wheel > and made there own DMA streaming solution. You can still make V4L drivers > that control the video receivers/transmitters, but for the actual streaming > you are forced to use the vendor's crap code (hello TI!). > > I've bitter experiences with that :-( > > > So, we'll eventually need to add another entity for such > > bridge chipsets that have a video/vbi/radio device node > > associated, but don't have DMA engines on (some) devnodes. > > > > As, currently, we don't have any such case, > > ??? Radio devices are exactly that. I actually meant to say: "As, currently, no driver uses Media Controller on such cases," > > let's for now > > just rename the device nodes that are associated with a > > DMA engine as MEDIA_ENT_T_AV_DMA. > > > > So, > > MEDIA_ENT_T_DEVNODE_V4L -> MEDIA_ENT_T_AV_DMA > > > > PS.: This is not actually true for USB devices, as the > > DMA engine is an internal component, as it is up to the > > Kernel to strip the stream payload from the URB packages. > > How about MEDIA_ENT_T_DATA_STREAMING? Or perhaps DATA_IO? Perhaps even just > "IO"? Almost entities do I/O and data streaming (exceptions are flash controller, SEC and similar control subdevices). So, DATA_STREAMING, DATA_IO or IO are a way too generic. DMA is a little bit more restrictive than we wanted. Actually, I originally named those as MEDIA_ENT_T_AV_BRIDGE, because the hardware component that implements the device->CPU I/O is typically a bridge. But then I went into the drivers, and I noticed that several devices with just one bridge have several different entities for I/O. So, I ran this script: $ git filter-branch -f --msg-filter 'cat && sed s,MEDIA_ENT_T_AV_BRIDGE,MEDIA_ENT_T_AV_DMA,g' origin/patchwork.. $ git filter-branch -f --tree-filter 'for i in $(git grep -l MEDIA_ENT_T_AV_BRIDGE); do sed s,MEDIA_ENT_T_AV_BRIDGE,MEDIA_ENT_T_AV_DMA,g $i >a && mv a $i; done' origin/patchwork.. To replace the name everywere. Provided that we decide a better name, this can be easily fixable by doing the above scripting. Perhaps MEDIA_ENT_T_DEV_CPU_AV_IO would be a better name? > That would cover USB as well, and I dislike the use of "AV", since the > data might contain other things besides audio and/or video. True, but how to distinguish such device from an ALSA DEV/CPU IO? Answering myself, I see one alternative for that... we could use MEDIA_ENT_T_DEV_CPU_IO for all device->CPU I/O devices, and use properties to tell what APIs are valid on such entity. The bad thing is that someone could add multiple "incompatible" APIs at the same device (like ALSA, V4L and DVB - all on the dame sevnode). I'm running out of ideas here ;) In the lack of a better name, I would keep MEDIA_ENT_T_AV_DMA, as it is the closest one to what's provided by such entities (or the less wrong one). Regards, Mauro
Hi Mauro, On 05/08/2015 02:32 PM, Mauro Carvalho Chehab wrote: >>> and may also not be true on embedded devices >>> that, due to DRM reasons, don't allow writing unencrypted >>> data on a memory that could be seen by the CPU. >> >> This actually might still work by using opaque DMABUF handles. But that's >> under discussion right now in the Secure Data Path thread. > > Well, a DMABUF opaque handler like that actually refers to either a > buffer that is shared only between 2 devices or to a device-to-device > DMA transfer. > > Such dataflow is different than the usual meaning of the video devnode, > where the devnode is used to do I/O transfers. So, it may actually > be mapped as a different type of entity. > > We'll need to discuss further when we start mapping this via MC. Yes, this is quite theoretical at the moment. >>> So, we'll eventually need to add another entity for such >>> bridge chipsets that have a video/vbi/radio device node >>> associated, but don't have DMA engines on (some) devnodes. >>> >>> As, currently, we don't have any such case, >> >> ??? Radio devices are exactly that. > > I actually meant to say: > > "As, currently, no driver uses Media Controller on such cases," Ah, OK. That makes more sense :-) > >>> let's for now >>> just rename the device nodes that are associated with a >>> DMA engine as MEDIA_ENT_T_AV_DMA. >>> >>> So, >>> MEDIA_ENT_T_DEVNODE_V4L -> MEDIA_ENT_T_AV_DMA >>> >>> PS.: This is not actually true for USB devices, as the >>> DMA engine is an internal component, as it is up to the >>> Kernel to strip the stream payload from the URB packages. >> >> How about MEDIA_ENT_T_DATA_STREAMING? Or perhaps DATA_IO? Perhaps even just >> "IO"? > > Almost entities do I/O and data streaming (exceptions are flash controller, > SEC and similar control subdevices). So, DATA_STREAMING, DATA_IO or IO > are a way too generic. For some of our products we have lots of video nodes that just control the receiver or transmitter (and possibly other related blocks), but leave the streaming to vendor code where we are forced to use that. So this can be a lot more common than you might think, although you won't see that appearing in the kernel. > > DMA is a little bit more restrictive than we wanted. > > Actually, I originally named those as MEDIA_ENT_T_AV_BRIDGE, because > the hardware component that implements the device->CPU I/O is typically > a bridge. But then I went into the drivers, and I noticed that several > devices with just one bridge have several different entities for I/O. > > So, I ran this script: > $ git filter-branch -f --msg-filter 'cat && sed s,MEDIA_ENT_T_AV_BRIDGE,MEDIA_ENT_T_AV_DMA,g' origin/patchwork.. > $ git filter-branch -f --tree-filter 'for i in $(git grep -l MEDIA_ENT_T_AV_BRIDGE); do sed s,MEDIA_ENT_T_AV_BRIDGE,MEDIA_ENT_T_AV_DMA,g $i >a && mv a $i; done' origin/patchwork.. > > To replace the name everywere. Provided that we decide a better name, > this can be easily fixable by doing the above scripting. > > Perhaps MEDIA_ENT_T_DEV_CPU_AV_IO would be a better name? > >> That would cover USB as well, and I dislike the use of "AV", since the >> data might contain other things besides audio and/or video. > > True, but how to distinguish such device from an ALSA DEV/CPU IO? > > Answering myself, I see one alternative for that... we could use > MEDIA_ENT_T_DEV_CPU_IO for all device->CPU I/O devices, and use > properties to tell what APIs are valid on such entity. The bad thing > is that someone could add multiple "incompatible" APIs at the same > device (like ALSA, V4L and DVB - all on the dame sevnode). > > I'm running out of ideas here ;) > > In the lack of a better name, I would keep MEDIA_ENT_T_AV_DMA, as > it is the closest one to what's provided by such entities (or the > less wrong one). See my reply to patch 10/18: I see whether streaming is supported or not as a function (or feature) of the entity, just like a subdev entity can be both a tuner and a video decoder. I.e. these are properties. I think the naming issue here shows the problem with your approach. Whereas having a 'streaming' property simplifies this IMHO. Regards, Hans
diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml index 5872f8bbf774..5b8147629159 100644 --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml @@ -183,7 +183,7 @@ <entry>Unknown device node</entry> </row> <row> - <entry><constant>MEDIA_ENT_T_DEVNODE_V4L</constant></entry> + <entry><constant>MEDIA_ENT_T_AV_DMA</constant></entry> <entry>V4L video, radio or vbi device node</entry> </row> <row> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c index efde88adf624..7fa0cc0f08f0 100644 --- a/drivers/media/platform/xilinx/xilinx-dma.c +++ b/drivers/media/platform/xilinx/xilinx-dma.c @@ -193,7 +193,7 @@ static int xvip_pipeline_validate(struct xvip_pipeline *pipe, while ((entity = media_entity_graph_walk_next(&graph))) { struct xvip_dma *dma; - if (entity->type != MEDIA_ENT_T_DEVNODE_V4L) + if (entity->type != MEDIA_ENT_T_AV_DMA) continue; dma = to_xvip_dma(media_entity_to_video_device(entity)); diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c index 71a1b93b0790..9ef920221b5a 100644 --- a/drivers/media/v4l2-core/v4l2-dev.c +++ b/drivers/media/v4l2-core/v4l2-dev.c @@ -912,7 +912,7 @@ int __video_register_device(struct video_device *vdev, int type, int nr, /* Part 5: Register the entity. */ if (vdev->v4l2_dev->mdev && vdev->vfl_type != VFL_TYPE_SUBDEV) { - vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L; + vdev->entity.type = MEDIA_ENT_T_AV_DMA; vdev->entity.name = vdev->name; vdev->entity.info.dev.major = VIDEO_MAJOR; vdev->entity.info.dev.minor = vdev->minor; diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c index 63596063b213..9f8fc8330b3e 100644 --- a/drivers/media/v4l2-core/v4l2-subdev.c +++ b/drivers/media/v4l2-core/v4l2-subdev.c @@ -535,7 +535,7 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad, return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt); } - WARN(pad->entity->type != MEDIA_ENT_T_DEVNODE_V4L, + WARN(pad->entity->type != MEDIA_ENT_T_AV_DMA, "Driver bug! Wrong media entity type 0x%08x, entity %s\n", pad->entity->type, pad->entity->name); diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h index 775c11c6b173..a7aa2aac9c23 100644 --- a/include/uapi/linux/media.h +++ b/include/uapi/linux/media.h @@ -44,12 +44,12 @@ struct media_device_info { /* Used values for media_entity_desc::type */ -#define MEDIA_ENT_T_DEVNODE_V4L (((1 << 16)) + 1) -#define MEDIA_ENT_T_DEVNODE_DVB_FE (MEDIA_ENT_T_DEVNODE_V4L + 3) -#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX (MEDIA_ENT_T_DEVNODE_V4L + 4) -#define MEDIA_ENT_T_DEVNODE_DVB_DVR (MEDIA_ENT_T_DEVNODE_V4L + 5) -#define MEDIA_ENT_T_DEVNODE_DVB_CA (MEDIA_ENT_T_DEVNODE_V4L + 6) -#define MEDIA_ENT_T_DEVNODE_DVB_NET (MEDIA_ENT_T_DEVNODE_V4L + 7) +#define MEDIA_ENT_T_AV_DMA (((1 << 16)) + 1) +#define MEDIA_ENT_T_DEVNODE_DVB_FE (MEDIA_ENT_T_AV_DMA + 3) +#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX (MEDIA_ENT_T_AV_DMA + 4) +#define MEDIA_ENT_T_DEVNODE_DVB_DVR (MEDIA_ENT_T_AV_DMA + 5) +#define MEDIA_ENT_T_DEVNODE_DVB_CA (MEDIA_ENT_T_AV_DMA + 6) +#define MEDIA_ENT_T_DEVNODE_DVB_NET (MEDIA_ENT_T_AV_DMA + 7) #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR ((2 << 16) + 1) #define MEDIA_ENT_T_V4L2_SUBDEV_FLASH (MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 1)
At the Video4Linux API, the /dev/video?, /dev/vbi? and /dev/radio? device nodes are used for the chipset that provides the bridge between video/radio streams and the USB, PCI or CPU buses. Such bridge is also typically used to control the V4L2 device as a hole. For video streaming devices and SDR radio devices, they're also associated with the DMA engines that transfer the video stream (or SDR stream) to the CPU's memory. It should be noticed, however, this is not true on non-SDR radio devices, and may also not be true on embedded devices that, due to DRM reasons, don't allow writing unencrypted data on a memory that could be seen by the CPU. So, we'll eventually need to add another entity for such bridge chipsets that have a video/vbi/radio device node associated, but don't have DMA engines on (some) devnodes. As, currently, we don't have any such case, let's for now just rename the device nodes that are associated with a DMA engine as MEDIA_ENT_T_AV_DMA. So, MEDIA_ENT_T_DEVNODE_V4L -> MEDIA_ENT_T_AV_DMA PS.: This is not actually true for USB devices, as the DMA engine is an internal component, as it is up to the Kernel to strip the stream payload from the URB packages. Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>