On 02/05/2018 12:15 PM, Mauro Carvalho Chehab wrote:
> Hi Hans,
>
> Em Sun, 4 Feb 2018 14:06:42 +0100
> Hans Verkuil <hverk...@xs4all.nl> escreveu:
>
>> Hi Mauro,
>>
>> I'm working on adding proper compliance tests for the MC but I think
>> something
>> is missing in the G_TOPOLOGY ioctl w.r.t. pads.
>>
>> In several v4l-subdev ioctls you need to pass the pad. There the pad is an
>> index
>> for the corresponding entity. I.e. an entity has 3 pads, so the pad argument
>> is
>> [0-2].
>>
>> The G_TOPOLOGY ioctl returns a pad ID, which is > 0x01000000. I can't use
>> that
>> in the v4l-subdev ioctls, so how do I translate that to a pad index in my
>> application?
>>
>> It seems to be a missing feature in the API. I assume this information is
>> available
>> in the core, so then I would add a field to struct media_v2_pad with the pad
>> index
>> for the entity.
>
> No, this was designed this way by purpose, back in 2015, when this was
> discussed at the first MC workshop. It was a consensus on that time that
> parameters like PAD index, PAD name, etc should be implemented via the
> properties API, as proposed by Sakari [1]. We also had a few discussions
> about that on both IRC and ML.
>
> [1] See: https://linuxtv.org/news.php?entry=2015-08-17.mchehab
>
> 3.3 Action items
> ...
> media_properties: RFC userspace API: Sakari
>
> Unfortunately, Sakari never found the time to send us a patch series
> implementing a properties API, even as RFC.
>
> One of the other missing features is to add a new version for setting
> links (I guess we called it as S_LINKS_V2 at the meetings there).
> The hole idea is to provide a way to setup the entire pipeline using
> a single ioctl, on an atomic way.
>
> The big problem with pad indexes happen on entities that have PADs with
> different types of signal input or output, like for example a TV tuner.
>
> On almost all PC consumers hardware supported by the Kernel, the same
> PCI/USB ID may come with different types of hardware. This may also
> happen with embedded TV sets, as, depending on the market is is
> sold, the same product may come with a different tuner.
>
> A "classic" TV tuner usually has those types of output signals:
>
> - analog TV luminance IF;
> - analog TV chrominance IF [1];
> - digital TV OFDM IF;
> - audio IF;
>
> [1] As both luminance and chrominance should go to the same TV
> demod, in practice, we can group both signals together on a
> single pad.
>
> More modern tuners also have an audio demod integrated, meaning that
> they also have another PAD:
> - digital mono or stereo audio.
>
> At the simplest possible scenario, let's say that we have a TV device
> has those entities (among others), each with a single PAD input:
>
> - entity #0: a TV tuner;
> - entity #1: an audio demod;
> - entity #2: an analog TV demod;
> - entity #3: a digital TV demod.
>
> And the TV tuner has 4 output pads:
>
> - pad #0 -> analog TV luminance/chrominance;
> - pad #1 -> digital TV IF;
> - pad #2 -> audio IF;
> - pad #3 -> digital audio.
>
> There, pad #0 can only be connected to entity #2. You cannot
> connect any other pad to entity #2. The same apply to every other
> TV tuner output PAD.
>
> In this specific scenario, entity #1 can only be connected to pad #2,
> and pad #3 can't be connected anywhere, as, on this model opted to
> have a separate audio demod, and didn't wired the digital audio output.
> Yet, the subdev has it.
>
> Another TV set may have different pad numbers - placing them even on a
> different order, and opting to use the audio demod tuner, wiring the
> digital audio output.
>
> Currently, there's no way for an userspace application to know what pads
> can be linked to each entity, as there's no way to tell userspace what
> kind of signal each pad supports.
>
> In any case, the Kernel should either detect the tuner model or know
> it (via a different DT entry), but userspace need such information,
> in order to be able to properly set the pipelines.
>
> So, what we really need here is passing an optional set of properties
> to userspace in order to allow it to do the right thing.
I fail to see the problem. Entities have pads. Pads have both a unique
ID and an index within the entity pad list. I.e. the pad ID and the
(entity ID + pad index) tuple both uniquely identify the pad.
Whether a pad is connected to anything or what type it is is unrelated
to this. Passing the pad index of an unconnected pad to e.g. SUBDEV_S_FMT
will just result in an error. All I need is to be able to obtain the
pad index so I can call SUBDEV_S_FMT at all!
I actually like G_TOPOLOGY, it's nice. But it just does not give all the
information needed.
> Yet, I agree with you: we should not wait anymore for a properties API,
> as it seems unlikely that we'll add support for it anytime soon.
>
> So, let's add two fields there:
> - pad index number;
So should I also add a flag to signal that the pad index is set? Since
current and past kernels never set this. Obviously none of the current
applications using G_TOPOLOGY would use a pad index because there isn't
one. It would only be a problem for new applications that switch to
G_TOPOLOGY, use subdevs and assume that the kernel that is used supports
the pad index. I'm not sure if this warrants a new pad flag though.
> - pad type.
>
> In order to make things easy, I guess the best would be to use a string
> for the pad type, and fill it only for entities where it is relevant
> (like TV tuners and demods).
struct media_v2_pad {
__u32 id;
__u32 entity_id;
__u32 flags;
__u32 reserved[5];
} __attribute__ ((packed));
This does not easily lend itself to a string. A pad type u16 would be easy
to add though. That said, adding a pad type is a completely separate issue
and needs an RFC or something to define this. If there such an RFC was posted
in the past, can you provide a link to refresh my memory?
Regards,
Hans
