| Message ID | 20240904210719.52466-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | media: ov5645: Add support for streams | expand |
On 05/09/2024 00:07, Prabhakar wrote: > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > Hi All, > > This patch series aims to add the below features, > - Support subdev active state > - Support for streams > - Support for virtual channel > - Code cleanup > > Sending this is as an RFC, I've done limited testing and Im seeing issue > when route is updated and wanted some feedback on the implementation. > > When route is updated for the sensor with below command, and when start > streaming I get "Dangling sink streams: mask 0x1" error. Should the > corresponding bridge also need to support routes to fix this? or is it > something missing Ive missed in the current implementation. > > $ media-ctl -R "'ov5645 0-003c' [1/0->0/1[1]]" > > Output after route update: > - entity 4: ov5645 0-003c (2 pads, 1 link, 1 route) > type V4L2 subdev subtype Sensor flags 0 > device node name /dev/v4l-subdev1 > routes: > 1/0 -> 0/1 [ACTIVE] > pad0: SOURCE > [stream:1 fmt:UYVY8_1X16/1920x1080 field:none colorspace:srgb > crop:(0,0)/1920x1080] > -> "csi-10830400.csi2":0 [ENABLED,IMMUTABLE] > pad1: SINK,0x8 > [stream:0 fmt:UYVY8_1X16/2592x1944 field:none colorspace:srgb > crop:(0,0)/1920x1080] I think you actually want 1/0->0/0 routing. The error says that the sink side device has routing which does not have a stream at stream ID 1, or no routing support at all, which implies a single stream at stream ID 0. Tomi > > v4l2-compliance log after this patch series: > -------------------------------------------- > root@smarc-rzg2l:~# v4l2-compliance -u /dev/v4l-subdev1 > v4l2-compliance 1.28.1-5233, 64 bits, 64-bit time_t > v4l2-compliance SHA: fc15e229d9d3 2024-07-23 19:22:15 > > Compliance test for device /dev/v4l-subdev1: > > Driver Info: > Driver version : 6.11.0 > Capabilities : 0x00000002 > Streams Support > Client Capabilities:[ 53.919041] ov5645 0-003c: ================= START STATUS ================= > 0x0000000000000[ 53.927271] ov5645 0-003c: ================== END STATUS ================== > 003 > streams interval-uses-which > Required ioctls: > test VIDIOC_SUDBEV_QUERYCAP: OK > test invalid ioctls: OK > > Allow for multiple opens: > test second /dev/v4l-subdev1 open: OK > test VIDIOC_SUBDEV_QUERYCAP: OK > test for unlimited opens: OK > > Debug ioctls: > test VIDIOC_LOG_STATUS: OK (Not Supported) > > Input ioctls: > test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) > test VIDIOC_ENUMAUDIO: OK (Not Supported) > test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) > test VIDIOC_G/S_AUDIO: OK (Not Supported) > Inputs: 0 Audio Inputs: 0 Tuners: 0 > > Output ioctls: > test VIDIOC_G/S_MODULATOR: OK (Not Supported) > test VIDIOC_G/S_FREQUENCY: OK (Not Supported) > test VIDIOC_ENUMAUDOUT: OK (Not Supported) > test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) > test VIDIOC_G/S_AUDOUT: OK (Not Supported) > Outputs: 0 Audio Outputs: 0 Modulators: 0 > > Input/Output configuration ioctls: > test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) > test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) > test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) > test VIDIOC_G/S_EDID: OK (Not Supported) > > Sub-Device routing ioctls: > test Try VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: OK > test Active VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: OK > > Control ioctls: > test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK > test VIDIOC_QUERYCTRL: OK > test VIDIOC_G/S_CTRL: OK > test VIDIOC_G/S/TRY_EXT_CTRLS: OK > test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK > test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) > Standard Controls: 12 Private Controls: 0 > > Format ioctls: > test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported) > test VIDIOC_G/S_PARM: OK (Not Supported) > test VIDIOC_G_FBUF: OK (Not Supported) > test VIDIOC_G_FMT: OK (Not Supported) > test VIDIOC_TRY_FMT: OK (Not Supported) > test VIDIOC_S_FMT: OK (Not Supported) > test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) > test Cropping: OK (Not Supported) > test Composing: OK (Not Supported) > test Scaling: OK (Not Supported) > > Codec ioctls: > test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) > test VIDIOC_G_ENC_INDEX: OK (Not Supported) > test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) > > Buffer ioctls: > test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported) > test CREATE_BUFS maximum buffers: OK > test VIDIOC_REMOVE_BUFS: OK > test VIDIOC_EXPBUF: OK (Not Supported) > test Requests: OK (Not Supported) > > Total for device /dev/v4l-subdev1: 47, Succeeded: 47, Failed: 0, Warnings: 0 > > Cheers, > Prabhakar > > Lad Prabhakar (12): > media: i2c: ov5645: Add V4L2_SUBDEV_FL_HAS_EVENTS and subscribe hooks > media: i2c: ov5645: Use local `dev` pointer for subdev device > assignment > media: i2c: ov5645: Enable runtime PM after > v4l2_async_register_subdev() > media: i2c: ov5645: Use dev_err_probe instead of dev_err > media: i2c: ov5645: Use v4l2_async_register_subdev_sensor() > media: i2c: ov5645: Drop `power_lock` mutex > media: i2c: ov5645: Use subdev active state > media: i2c: ov5645: Switch to {enable,disable}_streams > media: i2c: ov5645: Add internal image sink pad > media: i2c: ov5645: Report internal routes to userspace > media: i2c: ov5645: Report streams using frame descriptors > media: i2c: ov5645: Add virtual channel support > > drivers/media/i2c/ov5645.c | 507 +++++++++++++++++++++++-------------- > 1 file changed, 316 insertions(+), 191 deletions(-) >
On 05/09/2024 09:48, Tomi Valkeinen wrote: > On 05/09/2024 00:07, Prabhakar wrote: >> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> >> >> Hi All, >> >> This patch series aims to add the below features, >> - Support subdev active state >> - Support for streams >> - Support for virtual channel >> - Code cleanup >> >> Sending this is as an RFC, I've done limited testing and Im seeing issue >> when route is updated and wanted some feedback on the implementation. >> >> When route is updated for the sensor with below command, and when start >> streaming I get "Dangling sink streams: mask 0x1" error. Should the >> corresponding bridge also need to support routes to fix this? or is it >> something missing Ive missed in the current implementation. >> >> $ media-ctl -R "'ov5645 0-003c' [1/0->0/1[1]]" >> >> Output after route update: >> - entity 4: ov5645 0-003c (2 pads, 1 link, 1 route) >> type V4L2 subdev subtype Sensor flags 0 >> device node name /dev/v4l-subdev1 >> routes: >> 1/0 -> 0/1 [ACTIVE] >> pad0: SOURCE >> [stream:1 fmt:UYVY8_1X16/1920x1080 field:none >> colorspace:srgb >> crop:(0,0)/1920x1080] >> -> "csi-10830400.csi2":0 [ENABLED,IMMUTABLE] >> pad1: SINK,0x8 >> [stream:0 fmt:UYVY8_1X16/2592x1944 field:none >> colorspace:srgb >> crop:(0,0)/1920x1080] > > I think you actually want 1/0->0/0 routing. The error says that the sink > side device has routing which does not have a stream at stream ID 1, or > no routing support at all, which implies a single stream at stream ID 0. Looking at patch 12, there's something wrong with the approach here. Are you perhaps trying to define the CSI-2 VC with the streams? If you have a camera with a single image stream coming from an internal pad, you should have one hardcoded route, 1/0->0/0. The .get_frame_desc() should return the CSI-2 VC (most likely always 0) and DT (based on the format) for that stream. If you also have embedded data, then you'd have another internal pad (pad number 2), and the routing would be: 1/0->0/0 2/0->0/1 And here .get_frame_desc() would also handle the second stream, and most likely for that stream the VC would also be 0. The stream ID in the routing table is a software level concept, not related to the VC. Also, the internal pad should use the sensor's native format, not UYVY8_1X16. Tomi
Hi Tomi, Thanks for the feedback. On Thu, Sep 5, 2024 at 7:48 AM Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > On 05/09/2024 00:07, Prabhakar wrote: > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > > Hi All, > > > > This patch series aims to add the below features, > > - Support subdev active state > > - Support for streams > > - Support for virtual channel > > - Code cleanup > > > > Sending this is as an RFC, I've done limited testing and Im seeing issue > > when route is updated and wanted some feedback on the implementation. > > > > When route is updated for the sensor with below command, and when start > > streaming I get "Dangling sink streams: mask 0x1" error. Should the > > corresponding bridge also need to support routes to fix this? or is it > > something missing Ive missed in the current implementation. > > > > $ media-ctl -R "'ov5645 0-003c' [1/0->0/1[1]]" > > > > Output after route update: > > - entity 4: ov5645 0-003c (2 pads, 1 link, 1 route) > > type V4L2 subdev subtype Sensor flags 0 > > device node name /dev/v4l-subdev1 > > routes: > > 1/0 -> 0/1 [ACTIVE] > > pad0: SOURCE > > [stream:1 fmt:UYVY8_1X16/1920x1080 field:none colorspace:srgb > > crop:(0,0)/1920x1080] > > -> "csi-10830400.csi2":0 [ENABLED,IMMUTABLE] > > pad1: SINK,0x8 > > [stream:0 fmt:UYVY8_1X16/2592x1944 field:none colorspace:srgb > > crop:(0,0)/1920x1080] > > I think you actually want 1/0->0/0 routing. PAD1 -> Internal image pad Sink PAD0 -> Source The above combination works were pad1/stream0 ---> pad0/stream0 > The error says that the sink > side device has routing which does not have a stream at stream ID 1, or > no routing support at all, which implies a single stream at stream ID 0. > To support virtual channels I have the below routes, struct v4l2_subdev_route routes[OV5645_ROUTES_MAX] = { { .sink_pad = OV5645_PAD_IMAGE, .sink_stream = 0, .source_pad = OV5645_PAD_SOURCE, .source_stream = 0, .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE, }, { .sink_pad = OV5645_PAD_IMAGE, .sink_stream = 0, .source_pad = OV5645_PAD_SOURCE, .source_stream = 1, }, { .sink_pad = OV5645_PAD_IMAGE, .sink_stream = 0, .source_pad = OV5645_PAD_SOURCE, .source_stream = 2, }, { .sink_pad = OV5645_PAD_IMAGE, .sink_stream = 0, .source_pad = OV5645_PAD_SOURCE, .source_stream = 3, }, }; Where the image pad sink stream0 can connect source stream0/1/2/3 So to switch source stream1 I use the below: $ media-ctl -R "'ov5645 0-003c' [1/0->0/1[1]]" With the above I connect sink pad1 stream0 -> source pad0 stream1, (unless Ive misunderstood the connection graph). Cheers, Prabhakar
Hi Tomi, On Thu, Sep 5, 2024 at 10:11 AM Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > <snip> > > I think you actually want 1/0->0/0 routing. The error says that the sink > > side device has routing which does not have a stream at stream ID 1, or > > no routing support at all, which implies a single stream at stream ID 0. > > Looking at patch 12, there's something wrong with the approach here. Are > you perhaps trying to define the CSI-2 VC with the streams? > Yes, based on the previous feedback received, I am implementing virtual channels as streams. If this isn't the correct approach can you please guide me on what we should be using to support virtual channels? > If you have a camera with a single image stream coming from an internal > pad, you should have one hardcoded route, 1/0->0/0. The > .get_frame_desc() should return the CSI-2 VC (most likely always 0) and > DT (based on the format) for that stream. > Agreed. > If you also have embedded data, then you'd have another internal pad > (pad number 2), and the routing would be: > > 1/0->0/0 > 2/0->0/1 > > And here .get_frame_desc() would also handle the second stream, and most > likely for that stream the VC would also be 0. > Ok agreed. > The stream ID in the routing table is a software level concept, not > related to the VC. > > Also, the internal pad should use the sensor's native format, not > UYVY8_1X16. > Ok, I will update it to MEDIA_BUS_FMT_SBGGR10_1X10. Cheers, Prabhakar
On 05/09/2024 13:27, Lad, Prabhakar wrote: > Hi Tomi, > > On Thu, Sep 5, 2024 at 10:11 AM Tomi Valkeinen > <tomi.valkeinen@ideasonboard.com> wrote: >> > <snip> >>> I think you actually want 1/0->0/0 routing. The error says that the sink >>> side device has routing which does not have a stream at stream ID 1, or >>> no routing support at all, which implies a single stream at stream ID 0. >> >> Looking at patch 12, there's something wrong with the approach here. Are >> you perhaps trying to define the CSI-2 VC with the streams? >> > Yes, based on the previous feedback received, I am implementing > virtual channels as streams. If this isn't the correct approach can > you please guide me on what we should be using to support virtual > channels? Can you describe what kind of hardware you have and how the VCs are supposed to be configured and used there? Tomi
Hi Tomi, On Thu, Sep 5, 2024 at 11:29 AM Tomi Valkeinen <tomi.valkeinen@ideasonboard.com> wrote: > > On 05/09/2024 13:27, Lad, Prabhakar wrote: > > Hi Tomi, > > > > On Thu, Sep 5, 2024 at 10:11 AM Tomi Valkeinen > > <tomi.valkeinen@ideasonboard.com> wrote: > >> > > <snip> > >>> I think you actually want 1/0->0/0 routing. The error says that the sink > >>> side device has routing which does not have a stream at stream ID 1, or > >>> no routing support at all, which implies a single stream at stream ID 0. > >> > >> Looking at patch 12, there's something wrong with the approach here. Are > >> you perhaps trying to define the CSI-2 VC with the streams? > >> > > Yes, based on the previous feedback received, I am implementing > > virtual channels as streams. If this isn't the correct approach can > > you please guide me on what we should be using to support virtual > > channels? > > Can you describe what kind of hardware you have and how the VCs are > supposed to be configured and used there? > __________________ ______________ _____________________ | | | | | | | OV5645 Sensor | | RZ/G2L CSI2 | | RZ/G2L CRU | | 1ch out of 4x VC |---------------> | 4x VC |-------------->| 1Ch capture out 4xVC | |__________________| |_____________| |____________________| Cheers, Prabhakar
On 05/09/2024 13:51, Lad, Prabhakar wrote: > Hi Tomi, > > On Thu, Sep 5, 2024 at 11:29 AM Tomi Valkeinen > <tomi.valkeinen@ideasonboard.com> wrote: >> >> On 05/09/2024 13:27, Lad, Prabhakar wrote: >>> Hi Tomi, >>> >>> On Thu, Sep 5, 2024 at 10:11 AM Tomi Valkeinen >>> <tomi.valkeinen@ideasonboard.com> wrote: >>>> >>> <snip> >>>>> I think you actually want 1/0->0/0 routing. The error says that the sink >>>>> side device has routing which does not have a stream at stream ID 1, or >>>>> no routing support at all, which implies a single stream at stream ID 0. >>>> >>>> Looking at patch 12, there's something wrong with the approach here. Are >>>> you perhaps trying to define the CSI-2 VC with the streams? >>>> >>> Yes, based on the previous feedback received, I am implementing >>> virtual channels as streams. If this isn't the correct approach can >>> you please guide me on what we should be using to support virtual >>> channels? >> >> Can you describe what kind of hardware you have and how the VCs are >> supposed to be configured and used there? >> > > __________________ ______________ > _____________________ > | | | > | | > | > | OV5645 Sensor | | RZ/G2L CSI2 | > | RZ/G2L CRU | > | 1ch out of 4x VC |---------------> | 4x VC > |-------------->| 1Ch capture out 4xVC | > |__________________| |_____________| > |____________________| The formatting there is quite broken, but afaiu you have a standard setup with single stream. Then it should be as I wrote earlier: a single hardcoded 1/0->0/0 route in the sensor, and .get_frame_desc() returns VC 0 and a suitable DT for that stream. When you say "implementing virtual channels as streams", what is the feature/requirement you are looking at? Do you have some specific need to use other VCs than 0? Tomi
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Hi All, This patch series aims to add the below features, - Support subdev active state - Support for streams - Support for virtual channel - Code cleanup Sending this is as an RFC, I've done limited testing and Im seeing issue when route is updated and wanted some feedback on the implementation. When route is updated for the sensor with below command, and when start streaming I get "Dangling sink streams: mask 0x1" error. Should the corresponding bridge also need to support routes to fix this? or is it something missing Ive missed in the current implementation. $ media-ctl -R "'ov5645 0-003c' [1/0->0/1[1]]" Output after route update: - entity 4: ov5645 0-003c (2 pads, 1 link, 1 route) type V4L2 subdev subtype Sensor flags 0 device node name /dev/v4l-subdev1 routes: 1/0 -> 0/1 [ACTIVE] pad0: SOURCE [stream:1 fmt:UYVY8_1X16/1920x1080 field:none colorspace:srgb crop:(0,0)/1920x1080] -> "csi-10830400.csi2":0 [ENABLED,IMMUTABLE] pad1: SINK,0x8 [stream:0 fmt:UYVY8_1X16/2592x1944 field:none colorspace:srgb crop:(0,0)/1920x1080] v4l2-compliance log after this patch series: -------------------------------------------- root@smarc-rzg2l:~# v4l2-compliance -u /dev/v4l-subdev1 v4l2-compliance 1.28.1-5233, 64 bits, 64-bit time_t v4l2-compliance SHA: fc15e229d9d3 2024-07-23 19:22:15 Compliance test for device /dev/v4l-subdev1: Driver Info: Driver version : 6.11.0 Capabilities : 0x00000002 Streams Support Client Capabilities:[ 53.919041] ov5645 0-003c: ================= START STATUS ================= 0x0000000000000[ 53.927271] ov5645 0-003c: ================== END STATUS ================== 003 streams interval-uses-which Required ioctls: test VIDIOC_SUDBEV_QUERYCAP: OK test invalid ioctls: OK Allow for multiple opens: test second /dev/v4l-subdev1 open: OK test VIDIOC_SUBDEV_QUERYCAP: OK test for unlimited opens: OK Debug ioctls: test VIDIOC_LOG_STATUS: OK (Not Supported) Input ioctls: test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported) test VIDIOC_ENUMAUDIO: OK (Not Supported) test VIDIOC_G/S/ENUMINPUT: OK (Not Supported) test VIDIOC_G/S_AUDIO: OK (Not Supported) Inputs: 0 Audio Inputs: 0 Tuners: 0 Output ioctls: test VIDIOC_G/S_MODULATOR: OK (Not Supported) test VIDIOC_G/S_FREQUENCY: OK (Not Supported) test VIDIOC_ENUMAUDOUT: OK (Not Supported) test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported) test VIDIOC_G/S_AUDOUT: OK (Not Supported) Outputs: 0 Audio Outputs: 0 Modulators: 0 Input/Output configuration ioctls: test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported) test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported) test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported) test VIDIOC_G/S_EDID: OK (Not Supported) Sub-Device routing ioctls: test Try VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: OK test Active VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: OK Control ioctls: test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK test VIDIOC_QUERYCTRL: OK test VIDIOC_G/S_CTRL: OK test VIDIOC_G/S/TRY_EXT_CTRLS: OK test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK test VIDIOC_G/S_JPEGCOMP: OK (Not Supported) Standard Controls: 12 Private Controls: 0 Format ioctls: test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported) test VIDIOC_G/S_PARM: OK (Not Supported) test VIDIOC_G_FBUF: OK (Not Supported) test VIDIOC_G_FMT: OK (Not Supported) test VIDIOC_TRY_FMT: OK (Not Supported) test VIDIOC_S_FMT: OK (Not Supported) test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported) test Cropping: OK (Not Supported) test Composing: OK (Not Supported) test Scaling: OK (Not Supported) Codec ioctls: test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported) test VIDIOC_G_ENC_INDEX: OK (Not Supported) test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported) Buffer ioctls: test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported) test CREATE_BUFS maximum buffers: OK test VIDIOC_REMOVE_BUFS: OK test VIDIOC_EXPBUF: OK (Not Supported) test Requests: OK (Not Supported) Total for device /dev/v4l-subdev1: 47, Succeeded: 47, Failed: 0, Warnings: 0 Cheers, Prabhakar Lad Prabhakar (12): media: i2c: ov5645: Add V4L2_SUBDEV_FL_HAS_EVENTS and subscribe hooks media: i2c: ov5645: Use local `dev` pointer for subdev device assignment media: i2c: ov5645: Enable runtime PM after v4l2_async_register_subdev() media: i2c: ov5645: Use dev_err_probe instead of dev_err media: i2c: ov5645: Use v4l2_async_register_subdev_sensor() media: i2c: ov5645: Drop `power_lock` mutex media: i2c: ov5645: Use subdev active state media: i2c: ov5645: Switch to {enable,disable}_streams media: i2c: ov5645: Add internal image sink pad media: i2c: ov5645: Report internal routes to userspace media: i2c: ov5645: Report streams using frame descriptors media: i2c: ov5645: Add virtual channel support drivers/media/i2c/ov5645.c | 507 +++++++++++++++++++++++-------------- 1 file changed, 316 insertions(+), 191 deletions(-)