On 08/25/2015 05:12 PM, Mauro Carvalho Chehab wrote: > Em Tue, 25 Aug 2015 15:54:09 +0200 > Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > >> On 08/25/15 13:32, Mauro Carvalho Chehab wrote: >>> Em Tue, 25 Aug 2015 11:23:24 +0200 >>> Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: >>> >>>> On 08/23/15 22:17, Mauro Carvalho Chehab wrote: >>>>> Now that interfaces and entities are distinct, it makes no sense >>>>> of keeping something named as MEDIA_ENT_T_DEVNODE. >>>>> >>>>> This change was done with this script: >>>>> >>>>> for i in $(git grep -l MEDIA_ENT_T|grep -v uapi/linux/media.h); do sed s,MEDIA_ENT_T_DEVNODE_V4L,MEDIA_ENT_T_V4L2_VIDEO, <$i >a && mv a $i; done >>>>> >>>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx> >>>>> >>>>> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml >>>>> index 5872f8bbf774..910243d4edb8 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_V4L2_VIDEO</constant></entry> >>>>> <entry>V4L video, radio or vbi device node</entry> >>>>> </row> >>>> >>>> OK, this makes no sense and that ties in with my confusion of the previous patch. >>>> >>>> These are not device nodes, in the new scheme these are DMA entities (I know, >>>> naming TDB) that have an associated interface. >>> >>> Yes. Well, DMA is a bad name. It won't cover USB devices, where the DMA >>> engine is outside the V4L2 drivers, nor it would work for RDS radio data, >>> with may not need any DMA at all on no-USB devices, as the data flows via >>> the I2C bus. >>> >>>> I think a much better approach would be to add entity type(s) for such DMA >>>> engines in patch 24, then use that new name in existing drivers and split >>>> up the existing DEVNODE_V4L media_entity into a media_entity and a >>>> media_intf_devnode: >>> >>> Sorry, but I didn't get. That's precisely what I did ;) >>> >>>> The current media_entity defined in struct video_device has to be replaced >>>> by media_intf_devnode, and the DMA entity has to be added as a new entity >>>> to these drivers. >>> >>> If I do this way, it would break bisectability. I need first to replace >>> the names, but keep them as entities, and then add the interfaces. >>> >>>> >>>> This reflects these two action items from our meeting: >>>> >>>> Migration: add v4l-subdev media_interface: Laurent >>>> Migration: add explicit DMA Engine entity: Laurent >>>> >>>> Unless Laurent says differently I think this is something you'll have to >>>> do given Laurent's workload. >>> >>> Yes. The above action items are covered on this series. >>> >>> What patch 24 does is to define the new namespace, moving the legacy >>> symbols kept due to backward compatibility on a separate part of the >>> header. >>> >>> Then, patches 25-38 replace the occurrences of the deprecated names >>> by the new ones. >>> >>> Nothing is touched at the interfaces yet, to avoid breaking bisectability. >> >> I don't follow why that would break bisect. > > It won't break compilation, but it will break runtime. > > I mean: if we replace the current occurrences of the > "video output data entities" [1] any userspace app that would be used to test > somethingwill stop working. > > Ok, that means that it would break bisectability for us ;) > Still, better to avoid. > > [1] I don't like the "DMA" entities term, as it is too broken. > I prefer to refer to them with some other name, like I/O entities. > However, even this name is not perfect. Those are, in reality, a > "data interface", while what we call interface is actually a > "control interface", but calling like that would be confusing, I think. > So, I'll simply call it as "video/vbi/... output data entities". > >> >>> Then, the next patches add interfaces support at the V4L side. >> >> So this is not yet included in this patch series? That would explain >> my confusion. If it is, then I need to take another look on Friday. > > It is on the 3 patches I sent yesterday, after this patch series: > https://patchwork.linuxtv.org/patch/31081/ > https://patchwork.linuxtv.org/patch/31082/ > https://patchwork.linuxtv.org/patch/31083/ > Ah, OK. These are not part of this patch series, so that explains it. I hadn't gotten around to reviewing these 3. I'll plan reviewing these on Friday and I'll revisit the patches I skipped in the 44 part-series with this in mind. Regards, Hans > Please notice that the above patch series is not complete, as > there's something non-trivial to be addressed on non-subdev > V4L2 interfaces: how to create the indirect links. > > By indirect links, I meant to refer to the interface links that > don't control an entity directly, but via the internal hardware > control/I2C bus(es). > > So, a video interface, on a PC customer's hardware controls not only > the video output data entity, but it also indirectly controls the > tuner and the analog demod. Or, on a webcam hardware, it will also > controls the sensor. > > However, on platform drivers, it controls just the > "video output data entity" that is directly associated with it via > its device node. > > We need to add some support to automatically create those links, > once available, but only if the device is a PC customer's hardware. > > Btw, that's another reason to postpone it: creating the interfaces > offer this additional challenge, while creating the entities are > easy, as nothing changes there. > > Regards, > Mauro > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
