Hi Tomi,
On Thu, May 27, 2021 at 07:33:57PM +0300, Tomi Valkeinen wrote:
> On 27/05/2021 19:30, Laurent Pinchart wrote:
> > On Thu, May 27, 2021 at 07:10:42PM +0300, Tomi Valkeinen wrote:
> >> On 27/05/2021 19:06, Pratyush Yadav wrote:
> >>> On 24/05/21 02:09PM, Tomi Valkeinen wrote:
> >>>> Add routing and stream_config support to CAL driver.
> >>>>
> >>>> Add multiplexed streams support. CAL has 8 dma-engines and can capture 8
> >>>> separate streams at the same time.
> >>>>
> >>>> Add 8 video device nodes, each representing a single dma-engine, and set
> >>>> the number of source pads on camerarx to 8. Each video node can be
> >>>> connected to any of the source pads on either of the camerarx instances
> >>>> using media links. Camerarx internal routing is used to route the
> >>>> incoming CSI-2 streams to one of the 8 source pads.
> >>>>
> >>>> CAL doesn't support transcoding, so the driver currently allows changes
> >>>> only on the camerarx sink side, and then copies the sink pad config to
> >>>> the source pad. This becomes slighly more complex with 8 source pads and
> >>>> multiple streams on the sink pad. A helper,
> >>>> cal_camerarx_get_opposite_stream_format(), is added, which uses the
> >>>> routing table to get the format from the "opposite" side.
> >>>>
> >>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@xxxxxxxxxxxxxxxx>
> >>>> ---
> >>>> drivers/media/platform/ti-vpe/cal-camerarx.c | 303 ++++++++++++++++---
> >>>> drivers/media/platform/ti-vpe/cal-video.c | 103 ++++++-
> >>>> drivers/media/platform/ti-vpe/cal.c | 34 ++-
> >>>> drivers/media/platform/ti-vpe/cal.h | 12 +-
> >>>> 4 files changed, 385 insertions(+), 67 deletions(-)
> >>>>
> >>> [...]
> >>>> @@ -1178,18 +1177,33 @@ static int cal_probe(struct platform_device *pdev)
> >>>> }
> >>>>
> >>>> /* Create contexts. */
> >>>> - for (i = 0; i < cal->data->num_csi2_phy; ++i) {
> >>>> - if (!cal->phy[i]->source_node)
> >>>> - continue;
> >>>> + if (!cal_mc_api) {
> >>>> + for (i = 0; i < cal->data->num_csi2_phy; ++i) {
> >>>> + if (!cal->phy[i]->source_node)
> >>>> + continue;
> >>>> +
> >>>> + cal->ctx[i] = cal_ctx_create(cal, i);
> >>>> + if (!cal->ctx[i]) {
> >>>> + cal_err(cal, "Failed to create context %u\n", i);
> >>>> + ret = -ENODEV;
> >>>> + goto error_context;
> >>>> + }
> >>>>
> >>>> - cal->ctx[i] = cal_ctx_create(cal, i);
> >>>> - if (!cal->ctx[i]) {
> >>>> - cal_err(cal, "Failed to create context %u\n", i);
> >>>> - ret = -ENODEV;
> >>>> - goto error_context;
> >>>> + cal->ctx[i]->phy = cal->phy[i];
> >>>> +
> >>>> + cal->num_contexts++;
> >>>> }
> >>>> + } else {
> >>>> + for (i = 0; i < ARRAY_SIZE(cal->ctx); ++i) {
> >>>> + cal->ctx[i] = cal_ctx_create(cal, i);
> >>>> + if (!cal->ctx[i]) {
> >>>> + cal_err(cal, "Failed to create context %u\n", i);
> >>>> + ret = -ENODEV;
> >>>> + goto error_context;
> >>>> + }
> >>>>
> >>>> - cal->num_contexts++;
> >>>> + cal->num_contexts++;
> >>>
> >>> In cal_async_notifier_complete() I see:
> >>>
> >>> for (i = 0; i < cal->num_contexts; i++)
> >>> ret = cal_ctx_v4l2_register();
> >>>
> >>> This means that if the CAL device has 8 DMA contexts it will create 8
> >>> /dev/videoX nodes, even if the hardware setup is only capable of 1
> >>> stream.
> >>>
> >>> Would it make more sense to populate /dev/videoX nodes based on the
> >>> configured routing? So for example, if only one pad is being used to
> >>> output, only create one node corresponding to that pad. If there are 3
> >>> pads being populated then create 3 nodes and so on.
> >>
> >> Routing is a runtime configuration, so it could mean creating or
> >> removing video nodes every time the user changes the routing. I believe
> >> video nodes are supposed to be more permanent than that.
> >>
> >> If we knew that the HW setup can only ever have N routes, we could limit
> >> the number of video nodes, but I don't think we have means to figure
> >> that out.
> >
> > And even if we did, I think that wouldn't help userspace. The media
> > graph is meant to model the hardware topology, it's best to minimize the
> > complexity on the kernel side and let userspace deal with routing
> > configuration.
>
> I think it's a valid question. Maybe a CSI-2 RX uses system DMA, and can
> support, say, 128 contexts. We probably don't want 128 video nodes (of
> which perhaps 1-4 are ever used). But in CAL's case, I think always
> having all the 8 video nodes is acceptable.
Agreed, I wouldn't want to see 128 video nodes. If we had to support a
large number of contexts (in which case those contexts would either not
map to dedicated hardware resources, or map to very cheap hardware
resources), then I'd vote for adding a context ID to the buffer and
streaming ioctls on the video node. VIDIOC_STREAMON and VIDIOC_STREAMOFF
would be problematic as there's no room for extension, but it could be a
good occasion to introduce a VIDIOC_S_STREAM.
--
Regards,
Laurent Pinchart
