Re: [RFC 2/7] media: v4l: subdev: Support INTERNAL_SOURCE pads in routing IOCTLs

[Sakari Ailus <sakari.ailus@linux.intel.com>]

Moi,

On Mon, May 08, 2023 at 01:14:07PM +0300, Tomi Valkeinen wrote:
> On 06/05/2023 00:52, Sakari Ailus wrote:
> > Take the new INTERNAL_SOURCE pad flag into account in validating routing
> > IOCTL argument. Effectively this is a SINK pad in this respect. Due to the
> > union there's no need to check for the particular name.
> 
> What union? The one you add in the next patch?

Oops. I think we can reorder the patches.

> 
> As a concept the internal source pads sound good, and they work in practice
> in my tests. But this union is what grates me a bit. We have a flag,
> MEDIA_PAD_FL_INTERNAL_SOURCE, which tells which field of the union to use,
> and then we go and do not use the new union field. Well, and also the
> naming, as we normally have source and sink pads, but now we also have
> internal source pads, which are actually identical to sink pads...

The union still should be used by the user space. We're testing flags here
and those flags are the same independently of the INTERNAL_SOURCE flag.

I'm fine by not adding that union though, but for the user space I think
it's better we have it: explaining that the sink_pad has a different
meaning if that flag is set is harder than making it a union member.

> 
> I understand the idea and reasoning, but the two points above do confuse me
> and I'm sure would confuse others also.
> 
> I wonder if it would be less or more confusing to simplify this by just
> adding a MEDIA_PAD_FL_INTERNAL, which could be applied to either a source or
> a sink pad, and would prevent linking to it. The flag would indicate that
> the stream from/to that pad is generated/consumed internally. (I don't know
> when you would need an internal pad to consume data, but... who knows, the
> need might pop up =).

This is another option. But I envision there will be more compatibility
issues. Although... generally using such hardware requires knowledge of the
device, and we haven't obviously had any support for devices needing this
functionality in the tree. So in the end it might not matter much.

> 
> That would mean that an "internal sink pad" would generate a data stream,
> i.e. it's a "source", but I think a normal sink pad is almost the same
> anyway: when considering entity's internal routing, the normal sink pad is a
> "source", and the same logic would apply with the internal pads too.
> 
> An internal sink pad that generates a stream is a bit more confusing than an
> internal source pad, but I think that confusion is less than the rest of the
> confusion in the code-side that comes with the internal source pads.

I prefer having the possible sources of the confusion in the framework than
in the drivers. Therefore I think INTERNAL_SOURCE flag is a (slightly)
better option.

I wonder what Llaurent thinks.

-- 
Kind regards,

Sakari Ailus