mbox series

[0/4] media: ti-vpe: cal: multiplexed streams support

Message ID 20210427132028.1005757-1-tomi.valkeinen@ideasonboard.com (mailing list archive)
Headers show
Series media: ti-vpe: cal: multiplexed streams support | expand

Message

Tomi Valkeinen April 27, 2021, 1:20 p.m. UTC
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@ideasonboard.com/

https://lore.kernel.org/linux-media/20210420120433.902394-1-tomi.valkeinen@ideasonboard.com/

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.

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(-)

Comments

Laurent Pinchart April 29, 2021, 3:53 a.m. UTC | #1
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@ideasonboard.com/
> 
> https://lore.kernel.org/linux-media/20210420120433.902394-1-tomi.valkeinen@ideasonboard.com/
> 
> 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(-)