| Message ID | 20241024032201.3867638-1-hongju.wang@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | media: intel/ipu6: Set the VC of the stream as the SOF event id | expand |
Hi Hongju, On Thu, Oct 24, 2024 at 11:22:01AM +0800, Hongju Wang wrote: > In the virtual channel case, they should use v4l2_event.id to distinguish > SOF events of different streams. Therefore, we set the virtual channel > number of the stream as the SOF event ID. This number is unique. > > Signed-off-by: Hongju Wang <hongju.wang@intel.com> > --- > drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > index 051898ce53f4..5ad426afa0f0 100644 > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > @@ -578,6 +578,7 @@ void ipu6_isys_csi2_sof_event_by_stream(struct ipu6_isys_stream *stream) > .type = V4L2_EVENT_FRAME_SYNC, > }; > > + ev.id = stream->vc; > ev.u.frame_sync.frame_sequence = atomic_fetch_inc(&stream->sequence); > v4l2_event_queue(vdev, &ev); > The id field in struct v4l2_event isn't used for the FRAME_SYNC event and also the virtual channel isn't communicated to the user space currently. Doesn't the video device itself (and the routing configuration) associate this to a given sensor already?
Hi Sakari > -----Original Message----- > From: Sakari Ailus <sakari.ailus@linux.intel.com> > Sent: Thursday, October 24, 2024 6:31 PM > To: Wang, Hongju <hongju.wang@intel.com> > Cc: linux-media@vger.kernel.org; bingbu.cao@linux.intel.com > Subject: Re: [PATCH] media: intel/ipu6: Set the VC of the stream as the > SOF event id > > Hi Hongju, > > On Thu, Oct 24, 2024 at 11:22:01AM +0800, Hongju Wang wrote: > > In the virtual channel case, they should use v4l2_event.id to > > distinguish SOF events of different streams. Therefore, we set the > > virtual channel number of the stream as the SOF event ID. This number is > unique. > > > > Signed-off-by: Hongju Wang <hongju.wang@intel.com> > > --- > > drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > index 051898ce53f4..5ad426afa0f0 100644 > > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > @@ -578,6 +578,7 @@ void ipu6_isys_csi2_sof_event_by_stream(struct > ipu6_isys_stream *stream) > > .type = V4L2_EVENT_FRAME_SYNC, > > }; > > > > + ev.id = stream->vc; > > ev.u.frame_sync.frame_sequence = atomic_fetch_inc(&stream->sequence); > > v4l2_event_queue(vdev, &ev); > > > > The id field in struct v4l2_event isn't used for the FRAME_SYNC event and > also the virtual channel isn't communicated to the user space currently. How to use the id field? User space should use SOF to do FRAME_SYNC, but the v4l2_event doesn't distinguish different streams. Do you have any good suggestions? > Doesn't the video device itself (and the routing configuration) associate > this to a given sensor already? Yes, they are already connected. But we can't use the video device to do FRAME_SYNC, The CSI2 hardware interrupt has processing for FRAME_SYNC, we cannot pass it to the video device. > > -- > Kind regards, > > Sakari Ailus
Hi Hongju, On Mon, Nov 04, 2024 at 03:07:58AM +0000, Wang, Hongju wrote: > Hi Sakari > > > -----Original Message----- > > From: Sakari Ailus <sakari.ailus@linux.intel.com> > > Sent: Thursday, October 24, 2024 6:31 PM > > To: Wang, Hongju <hongju.wang@intel.com> > > Cc: linux-media@vger.kernel.org; bingbu.cao@linux.intel.com > > Subject: Re: [PATCH] media: intel/ipu6: Set the VC of the stream as the > > SOF event id > > > > Hi Hongju, > > > > On Thu, Oct 24, 2024 at 11:22:01AM +0800, Hongju Wang wrote: > > > In the virtual channel case, they should use v4l2_event.id to > > > distinguish SOF events of different streams. Therefore, we set the > > > virtual channel number of the stream as the SOF event ID. This number is > > unique. > > > > > > Signed-off-by: Hongju Wang <hongju.wang@intel.com> > > > --- > > > drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > index 051898ce53f4..5ad426afa0f0 100644 > > > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > @@ -578,6 +578,7 @@ void ipu6_isys_csi2_sof_event_by_stream(struct > > ipu6_isys_stream *stream) > > > .type = V4L2_EVENT_FRAME_SYNC, > > > }; > > > > > > + ev.id = stream->vc; > > > ev.u.frame_sync.frame_sequence = atomic_fetch_inc(&stream->sequence); > > > v4l2_event_queue(vdev, &ev); > > > > > > > The id field in struct v4l2_event isn't used for the FRAME_SYNC event and > > also the virtual channel isn't communicated to the user space currently. > > How to use the id field? User space should use SOF to do FRAME_SYNC, > but the v4l2_event doesn't distinguish different streams. Do you have > any good suggestions? Ah, now I think I do understand the problem. The events are produced by the CSI-2 receiver sub-device but there are multiple streams and the event isn't stream-aware? It's nice that the id field of the FRAME_SYNC event isn't in use for any other purpose. This should be taken into account in event subscription, too. I'll add a patch documenting this in the metadata series. Any thoughts? Cc Laurent and Hans.
On Mon, Nov 04, 2024 at 07:56:51AM +0000, Sakari Ailus wrote: > On Mon, Nov 04, 2024 at 03:07:58AM +0000, Wang, Hongju wrote: > > On Thursday, October 24, 2024 6:31 PM, Sakari Ailus wrote: > > > On Thu, Oct 24, 2024 at 11:22:01AM +0800, Hongju Wang wrote: > > > > In the virtual channel case, they should use v4l2_event.id to > > > > distinguish SOF events of different streams. Therefore, we set the > > > > virtual channel number of the stream as the SOF event ID. This number is unique. > > > > > > > > Signed-off-by: Hongju Wang <hongju.wang@intel.com> > > > > --- > > > > drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > index 051898ce53f4..5ad426afa0f0 100644 > > > > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > @@ -578,6 +578,7 @@ void ipu6_isys_csi2_sof_event_by_stream(struct ipu6_isys_stream *stream) > > > > .type = V4L2_EVENT_FRAME_SYNC, > > > > }; > > > > > > > > + ev.id = stream->vc; > > > > ev.u.frame_sync.frame_sequence = atomic_fetch_inc(&stream->sequence); > > > > v4l2_event_queue(vdev, &ev); > > > > > > > > > > The id field in struct v4l2_event isn't used for the FRAME_SYNC event and > > > also the virtual channel isn't communicated to the user space currently. > > > > How to use the id field? User space should use SOF to do FRAME_SYNC, > > but the v4l2_event doesn't distinguish different streams. Do you have > > any good suggestions? > > Ah, now I think I do understand the problem. The events are produced by the > CSI-2 receiver sub-device but there are multiple streams and the event > isn't stream-aware? > > It's nice that the id field of the FRAME_SYNC event isn't in use for any > other purpose. This should be taken into account in event subscription, > too. > > I'll add a patch documenting this in the metadata series. Any thoughts? I think we could use the id field for this purpose, but maybe not to report the VC. Userspace isn't VC-aware, we should instead convert the VC to a stream. > Cc Laurent and Hans.
Hi Laurent, > -----Original Message----- > From: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Sent: Monday, November 4, 2024 4:25 PM > To: Sakari Ailus <sakari.ailus@linux.intel.com> > Cc: Wang, Hongju <hongju.wang@intel.com>; linux-media@vger.kernel.org; > bingbu.cao@linux.intel.com; hverkuil@xs4all.nl > Subject: Re: [PATCH] media: intel/ipu6: Set the VC of the stream as the > SOF event id > > On Mon, Nov 04, 2024 at 07:56:51AM +0000, Sakari Ailus wrote: > > On Mon, Nov 04, 2024 at 03:07:58AM +0000, Wang, Hongju wrote: > > > On Thursday, October 24, 2024 6:31 PM, Sakari Ailus wrote: > > > > On Thu, Oct 24, 2024 at 11:22:01AM +0800, Hongju Wang wrote: > > > > > In the virtual channel case, they should use v4l2_event.id to > > > > > distinguish SOF events of different streams. Therefore, we set > > > > > the virtual channel number of the stream as the SOF event ID. This > number is unique. > > > > > > > > > > Signed-off-by: Hongju Wang <hongju.wang@intel.com> > > > > > --- > > > > > drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > > b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > > index 051898ce53f4..5ad426afa0f0 100644 > > > > > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > > @@ -578,6 +578,7 @@ void ipu6_isys_csi2_sof_event_by_stream(struct > ipu6_isys_stream *stream) > > > > > .type = V4L2_EVENT_FRAME_SYNC, > > > > > }; > > > > > > > > > > + ev.id = stream->vc; > > > > > ev.u.frame_sync.frame_sequence = atomic_fetch_inc(&stream- > >sequence); > > > > > v4l2_event_queue(vdev, &ev); > > > > > > > > > > > > > The id field in struct v4l2_event isn't used for the FRAME_SYNC > > > > event and also the virtual channel isn't communicated to the user > space currently. > > > > > > How to use the id field? User space should use SOF to do FRAME_SYNC, > > > but the v4l2_event doesn't distinguish different streams. Do you > have > > > any good suggestions? > > > > Ah, now I think I do understand the problem. The events are produced > > by the > > CSI-2 receiver sub-device but there are multiple streams and the event > > isn't stream-aware? > > > > It's nice that the id field of the FRAME_SYNC event isn't in use for > > any other purpose. This should be taken into account in event > > subscription, too. > > > > I'll add a patch documenting this in the metadata series. Any thoughts? > > I think we could use the id field for this purpose, but maybe not to > report the VC. Userspace isn't VC-aware, we should instead convert the VC > to a stream. > Do you mean that we use a stream id? But which stream is used isn't fixed. Only when video streams on, we select a stream for it. Which sensor uses Which VC is fixed by sensor driver. > > Cc Laurent and Hans. > > -- > Regards, > > Laurent Pinchart
On Tue, Nov 05, 2024 at 03:23:50AM +0000, Wang, Hongju wrote: > On Monday, November 4, 2024 4:25 PM, Laurent Pinchart wrote: > > On Mon, Nov 04, 2024 at 07:56:51AM +0000, Sakari Ailus wrote: > > > On Mon, Nov 04, 2024 at 03:07:58AM +0000, Wang, Hongju wrote: > > > > On Thursday, October 24, 2024 6:31 PM, Sakari Ailus wrote: > > > > > On Thu, Oct 24, 2024 at 11:22:01AM +0800, Hongju Wang wrote: > > > > > > In the virtual channel case, they should use v4l2_event.id to > > > > > > distinguish SOF events of different streams. Therefore, we set > > > > > > the virtual channel number of the stream as the SOF event ID. This number is unique. > > > > > > > > > > > > Signed-off-by: Hongju Wang <hongju.wang@intel.com> > > > > > > --- > > > > > > drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 1 + > > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > > > diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > > > b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > > > index 051898ce53f4..5ad426afa0f0 100644 > > > > > > --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > > > +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c > > > > > > @@ -578,6 +578,7 @@ void ipu6_isys_csi2_sof_event_by_stream(struct ipu6_isys_stream *stream) > > > > > > .type = V4L2_EVENT_FRAME_SYNC, > > > > > > }; > > > > > > > > > > > > + ev.id = stream->vc; > > > > > > ev.u.frame_sync.frame_sequence = atomic_fetch_inc(&stream->sequence); > > > > > > v4l2_event_queue(vdev, &ev); > > > > > > > > > > > > > > > > The id field in struct v4l2_event isn't used for the FRAME_SYNC > > > > > event and also the virtual channel isn't communicated to the user space currently. > > > > > > > > How to use the id field? User space should use SOF to do FRAME_SYNC, > > > > but the v4l2_event doesn't distinguish different streams. Do you have > > > > any good suggestions? > > > > > > Ah, now I think I do understand the problem. The events are produced by the > > > CSI-2 receiver sub-device but there are multiple streams and the event > > > isn't stream-aware? > > > > > > It's nice that the id field of the FRAME_SYNC event isn't in use for > > > any other purpose. This should be taken into account in event > > > subscription, too. > > > > > > I'll add a patch documenting this in the metadata series. Any thoughts? > > > > I think we could use the id field for this purpose, but maybe not to > > report the VC. Userspace isn't VC-aware, we should instead convert the VC > > to a stream. > > Do you mean that we use a stream id? But which stream is used isn't fixed. > Only when video streams on, we select a stream for it. Which sensor uses > Which VC is fixed by sensor driver. The event isn't generated until you start streaming, so by that time you should be able to convert the VC to a stream ID. The VC can't easily be used by userspace, as there's no way for an application to map VCs to streams. To make this API really usable, we need something that applications can meaningfully use. I would like to see a userspace implementation (ideally in libcamera) to showcase that the API works as expected. > > > Cc Laurent and Hans.
diff --git a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c index 051898ce53f4..5ad426afa0f0 100644 --- a/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c +++ b/drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c @@ -578,6 +578,7 @@ void ipu6_isys_csi2_sof_event_by_stream(struct ipu6_isys_stream *stream) .type = V4L2_EVENT_FRAME_SYNC, }; + ev.id = stream->vc; ev.u.frame_sync.frame_sequence = atomic_fetch_inc(&stream->sequence); v4l2_event_queue(vdev, &ev);
In the virtual channel case, they should use v4l2_event.id to distinguish SOF events of different streams. Therefore, we set the virtual channel number of the stream as the SOF event ID. This number is unique. Signed-off-by: Hongju Wang <hongju.wang@intel.com> --- drivers/media/pci/intel/ipu6/ipu6-isys-csi2.c | 1 + 1 file changed, 1 insertion(+)