Message ID | 20250212131244.1397722-4-y-abhilashchandra@ti.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | Enable support for error detection in CSI2RX | expand |
Hi Abhilash, On Wed, Feb 12, 2025 at 06:42:44PM +0530, Yemike Abhilash Chandra wrote: > The VIDIOC_LOG_STATUS ioctl outputs the current status of a device to the > kernel log. When this ioctl is called on a video device, the current > implementation queries the log status for all connected subdevices in the > media pipeline. > What is the benefit of doing this for a video node? The user can directly check the status on the cdns-csi2rx subdev for CSI errors. As far as I understand, the video node corresponds to a particular stream, but the cdns-csi2rx source pad is shared for all video nodes, so it will report the total errors seen for all video nodes in multi-camera scenarios. This approach will also give you v4l2 control handler status from a few sensors (like OV5640) that implement the ioctl using v4l2_ctrl_subdev_log_status(), which is probably just noise for the case where a user wants to check for stream errors. > Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com> > --- > drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c > index 6412a00be8ea..946704458fee 100644 > --- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c > +++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c > @@ -377,6 +377,15 @@ static int ti_csi2rx_enum_framesizes(struct file *file, void *fh, > return 0; > } > > +static int ti_csi2rx_log_status(struct file *file, void *fh) > +{ > + struct ti_csi2rx_dev *csi = video_drvdata(file); > + > + v4l2_device_call_all(&csi->v4l2_dev, 0, core, log_status); > + > + return 0; > +} > + > static const struct v4l2_ioctl_ops csi_ioctl_ops = { > .vidioc_querycap = ti_csi2rx_querycap, > .vidioc_enum_fmt_vid_cap = ti_csi2rx_enum_fmt_vid_cap, > @@ -393,6 +402,7 @@ static const struct v4l2_ioctl_ops csi_ioctl_ops = { > .vidioc_expbuf = vb2_ioctl_expbuf, > .vidioc_streamon = vb2_ioctl_streamon, > .vidioc_streamoff = vb2_ioctl_streamoff, > + .vidioc_log_status = ti_csi2rx_log_status, > }; > > static const struct v4l2_file_operations csi_fops = { > -- > 2.34.1 > Thanks, Jai
Hi Jai, Thank you for the review. On 13/02/25 18:56, Jai Luthra wrote: > Hi Abhilash, > > On Wed, Feb 12, 2025 at 06:42:44PM +0530, Yemike Abhilash Chandra wrote: >> The VIDIOC_LOG_STATUS ioctl outputs the current status of a device to the >> kernel log. When this ioctl is called on a video device, the current >> implementation queries the log status for all connected subdevices in the >> media pipeline. >> > > What is the benefit of doing this for a video node? The user can directly > check the status on the cdns-csi2rx subdev for CSI errors. > > As far as I understand, the video node corresponds to a particular stream, but > the cdns-csi2rx source pad is shared for all video nodes, so it will report > the total errors seen for all video nodes in multi-camera scenarios. > > This approach will also give you v4l2 control handler status from a few > sensors (like OV5640) that implement the ioctl using > v4l2_ctrl_subdev_log_status(), which is probably just noise for the case where > a user wants to check for stream errors. I understand that this change does not add any value and may introduce unnecessary noise. Given that the user can directly check the status on the cdns-csi2rx subdevice for stream errors, I will remove this change in v2. Thanks and Regards, Yemike Abhilash Chandra > >> Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com> >> --- >> drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c >> index 6412a00be8ea..946704458fee 100644 >> --- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c >> +++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c >> @@ -377,6 +377,15 @@ static int ti_csi2rx_enum_framesizes(struct file *file, void *fh, >> return 0; >> } >> >> +static int ti_csi2rx_log_status(struct file *file, void *fh) >> +{ >> + struct ti_csi2rx_dev *csi = video_drvdata(file); >> + >> + v4l2_device_call_all(&csi->v4l2_dev, 0, core, log_status); >> + >> + return 0; >> +} >> + >> static const struct v4l2_ioctl_ops csi_ioctl_ops = { >> .vidioc_querycap = ti_csi2rx_querycap, >> .vidioc_enum_fmt_vid_cap = ti_csi2rx_enum_fmt_vid_cap, >> @@ -393,6 +402,7 @@ static const struct v4l2_ioctl_ops csi_ioctl_ops = { >> .vidioc_expbuf = vb2_ioctl_expbuf, >> .vidioc_streamon = vb2_ioctl_streamon, >> .vidioc_streamoff = vb2_ioctl_streamoff, >> + .vidioc_log_status = ti_csi2rx_log_status, >> }; >> >> static const struct v4l2_file_operations csi_fops = { >> -- >> 2.34.1 >> > > Thanks, > Jai
diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c index 6412a00be8ea..946704458fee 100644 --- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c +++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c @@ -377,6 +377,15 @@ static int ti_csi2rx_enum_framesizes(struct file *file, void *fh, return 0; } +static int ti_csi2rx_log_status(struct file *file, void *fh) +{ + struct ti_csi2rx_dev *csi = video_drvdata(file); + + v4l2_device_call_all(&csi->v4l2_dev, 0, core, log_status); + + return 0; +} + static const struct v4l2_ioctl_ops csi_ioctl_ops = { .vidioc_querycap = ti_csi2rx_querycap, .vidioc_enum_fmt_vid_cap = ti_csi2rx_enum_fmt_vid_cap, @@ -393,6 +402,7 @@ static const struct v4l2_ioctl_ops csi_ioctl_ops = { .vidioc_expbuf = vb2_ioctl_expbuf, .vidioc_streamon = vb2_ioctl_streamon, .vidioc_streamoff = vb2_ioctl_streamoff, + .vidioc_log_status = ti_csi2rx_log_status, }; static const struct v4l2_file_operations csi_fops = {
The VIDIOC_LOG_STATUS ioctl outputs the current status of a device to the kernel log. When this ioctl is called on a video device, the current implementation queries the log status for all connected subdevices in the media pipeline. Signed-off-by: Yemike Abhilash Chandra <y-abhilashchandra@ti.com> --- drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c | 10 ++++++++++ 1 file changed, 10 insertions(+)