Hi Tomi, Thank you for the series. Nice to see the multiplexed streams API in operation. On Tue, Apr 27, 2021 at 04:20:24PM +0300, Tomi Valkeinen wrote: > Hi, > > This series adds embedded data and multiplexed streams support to TI CAL > driver. These patches depend on the subdev routing series and CAL preparation > series: > > https://lore.kernel.org/linux-media/20210427124523.990938-1-tomi.valkeinen@xxxxxxxxxxxxxxxx/ > > https://lore.kernel.org/linux-media/20210420120433.902394-1-tomi.valkeinen@xxxxxxxxxxxxxxxx/ > > One detail I'm not too happy about is adding a new kernel module parameter > 'cal_streams_api'. Setting this parameter enables the multiplexed streams > support for the CAL sink pads. I'd like the driver to be able to figure this > out itself, but there's no simple answer to it. > > A pad in a subdev has to be either in non-multiplexed or multiplexed mode. If > the subdev cannot support multiplexed streams, then the answer is clear. But if > it can, then the question becomes "what's connected to the pad, and what does > it want". Afaik, the idea with subdev configuration is that each subdev can be > configured independently, so the driver trying to figure out the answer to that > question breaks this idea. Also, two connected subdevs that both can support > both modes wouldn't even give the answer, but the answer might be found further > away, probably from the sensor subdev. > > So, maybe it should be the userspace that can set a subdev pad to either of the > modes. There's currently no such uAPI, but I think this option makes the most > sense. We'll need to brainstorm this a bit. I agree that the module parameter isn't a good solution. > Another detail to note is the embedded data support. I don't have hardware with > real embedded data, so I have not added any embedded data formats. Also, the > embedded data is captured with the video capture ioctls, not the metadata > capture ioctls. The reason for this is that in the case of CSI-2 embedded data > (at least for this hardware) the embedded data is very much like pixel data: > the data is sent in lines just before or after the pixel content and each > subdev needs a set_fmt call with the width, height, and mbus-code. > > Using the metadata ioctls is possible, but it only results in much more complex > driver code, and, I think, more confusing userspace. > > Tomi > > Tomi Valkeinen (4): > media: ti-vpe: cal: add embedded data support > media: ti-vpe: cal: use frame desc to get vc and dt > media: ti-vpe: cal: add routing support > media: ti-vpe: cal: add multiplexed streams support > > drivers/media/platform/ti-vpe/cal-camerarx.c | 234 ++++++++++++++++--- > drivers/media/platform/ti-vpe/cal-video.c | 95 +++++++- > drivers/media/platform/ti-vpe/cal.c | 102 ++++++-- > drivers/media/platform/ti-vpe/cal.h | 10 +- > 4 files changed, 383 insertions(+), 58 deletions(-) -- Regards, Laurent Pinchart