Linux Media Controller development
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Prabhakar <prabhakar.csengg@gmail.com>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Hans Verkuil <hverkuil-cisco@xs4all.nl>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-renesas-soc@vger.kernel.org,
	Biju Das <biju.das.jz@bp.renesas.com>,
	Fabrizio Castro <fabrizio.castro.jz@renesas.com>,
	Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: Re: [PATCH v3 00/10] media: ov5645: Add support for streams
Date: Mon, 21 Oct 2024 01:39:23 +0300	[thread overview]
Message-ID: <20241020223923.GB14328@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20241018153230.235647-1-prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi Prabhakar,

Thank you for the patches.

I think patch 01/10 should be dropped in favour of
https://lore.kernel.org/r/20241020164354.GG7770@pendragon.ideasonboard.com
("[PATCH v2] media: v4l2-subdev: Refactor events"). Patches 02/10 to
08/10 seem ready, should we merge them without waiting for 09/10 and
10/10 ?

On Fri, Oct 18, 2024 at 04:32:20PM +0100, 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
> 
> Note, these patches are dependent on below:
> 1] https://patchwork.kernel.org/project/linux-media/patch/20240416193319.778192-27-sakari.ailus@linux.intel.com/
> 2] https://patchwork.kernel.org/project/linux-media/patch/20240416193319.778192-26-sakari.ailus@linux.intel.com/
> 
> v2->v3
> - Fixed review commments from Laurent
> - Included RB tags from Laurent
> - Dropped patch "media: i2c: ov5645: Enable runtime PM after v4l2_async_register_subdev()"
> - Fixed checkpatch issues (ie used --max-line-length=80)
> 
> RFC->v2
> - Dropped setting of VC using routes
> - Defaulted the native format to MEDIA_BUS_FMT_SBGGR8_1X8
> - Fixed ov5645_enum_frame_size and ov5645_enum_mbus_code
>   for internal image pad
> 
> RFC patch,
> Link: https://lore.kernel.org/all/20240904210719.52466-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
> 
> Test logs:
> ====================================
> root@smarc-rzg2l:~# media-ctl -p
> ......
> - 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/0 [ACTIVE]
>     pad0: SOURCE
>         [stream:0 fmt:UYVY8_1X16/1280x960 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range
>         crop:(0,0)/1280x960]
>         -> "csi-10830400.csi2":0 [ENABLED,IMMUTABLE]
>     pad1: SINK,0x8
>         [stream:0 fmt:SBGGR8_1X8/2592x1944 field:none colorspace:srgb xfer:srgb ycbcr:601 quantization:lim-range
>         crop:(0,0)/1280x960]
> ......
> 
> root@smarc-rzg2l:~# v4l2-ctl --device /dev/v4l-subdev1 --list-subdev-mbus-codes pad=0
> ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=0,stream=0)
>     0x200f: MEDIA_BUS_FMT_UYVY8_1X16
> root@smarc-rzg2l:~# v4l2-ctl --device /dev/v4l-subdev1 --list-subdev-mbus-codes pad=1
> ioctl: VIDIOC_SUBDEV_ENUM_MBUS_CODE (pad=1,stream=0)
>     0x3001: MEDIA_BUS_FMT_SBGGR8_1X8
> root@smarc-rzg2l:~# v4l2-ctl --device /dev/v4l-subdev1 --list-subdev-framesizes pad=1,code=0x3001
> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=1,stream=0)
>     Size Range: 2592x1944 - 2592x1944
> root@smarc-rzg2l:~# v4l2-ctl --device /dev/v4l-subdev1 --list-subdev-framesizes pad=0,code=0x200f
> ioctl: VIDIOC_SUBDEV_ENUM_FRAME_SIZE (pad=0,stream=0)
>     Size Range: 1280x960 - 1280x960
>     Size Range: 1920x1080 - 1920x1080
>     Size Range: 2592x1944 - 2592x1944
> 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.12.0
>     Capabilities   : 0x00000002
>         Streams Support
>     Client Capabilities: 0x00000[ 2429.125325] ov5645 0-003c: ================= START STATUS =================
> 00000000003
> streams int[ 2429.134589] ov5645 0-003c: ================== END STATUS ==================
> erval-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
> ------------------------------------------------------------
> 
> Lad Prabhakar (10):
>   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: Replace dev_err with dev_err_probe in probe
>     function
>   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: Report streams using frame descriptors
>   media: i2c: ov5645: Add internal image sink pad
>   media: i2c: ov5645: Report internal routes to userspace
> 
>  drivers/media/i2c/ov5645.c | 435 +++++++++++++++++++++----------------
>  1 file changed, 246 insertions(+), 189 deletions(-)

-- 
Regards,

Laurent Pinchart

  parent reply	other threads:[~2024-10-20 22:39 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-18 15:32 [PATCH v3 00/10] media: ov5645: Add support for streams Prabhakar
2024-10-18 15:32 ` [PATCH v3 01/10] media: i2c: ov5645: Add V4L2_SUBDEV_FL_HAS_EVENTS and subscribe hooks Prabhakar
2024-10-18 15:32 ` [PATCH v3 02/10] media: i2c: ov5645: Use local `dev` pointer for subdev device assignment Prabhakar
2024-10-21  7:16   ` Tommaso Merciai
2024-10-18 15:32 ` [PATCH v3 03/10] media: i2c: ov5645: Replace dev_err with dev_err_probe in probe function Prabhakar
2024-10-20 22:33   ` Laurent Pinchart
2024-10-18 15:32 ` [PATCH v3 04/10] media: i2c: ov5645: Use v4l2_async_register_subdev_sensor() Prabhakar
2024-10-18 15:32 ` [PATCH v3 05/10] media: i2c: ov5645: Drop `power_lock` mutex Prabhakar
2024-10-18 15:32 ` [PATCH v3 06/10] media: i2c: ov5645: Use subdev active state Prabhakar
2024-10-18 15:32 ` [PATCH v3 07/10] media: i2c: ov5645: Switch to {enable,disable}_streams Prabhakar
2024-10-18 15:32 ` [PATCH v3 08/10] media: i2c: ov5645: Report streams using frame descriptors Prabhakar
2024-11-05 12:07   ` Sakari Ailus
2024-10-18 15:32 ` [PATCH v3 09/10] media: i2c: ov5645: Add internal image sink pad Prabhakar
2024-10-18 15:32 ` [PATCH v3 10/10] media: i2c: ov5645: Report internal routes to userspace Prabhakar
2024-10-20 22:39 ` Laurent Pinchart [this message]
2024-10-21  8:06   ` [PATCH v3 00/10] media: ov5645: Add support for streams Lad, Prabhakar
2024-11-04 23:32     ` Lad, Prabhakar
2024-10-21  8:10 ` Tommaso Merciai
2024-10-21  9:22   ` Lad, Prabhakar
2024-11-05 11:18 ` Sakari Ailus
2024-11-05 11:23   ` Lad, Prabhakar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241020223923.GB14328@pendragon.ideasonboard.com \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=biju.das.jz@bp.renesas.com \
    --cc=fabrizio.castro.jz@renesas.com \
    --cc=hverkuil-cisco@xs4all.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=prabhakar.csengg@gmail.com \
    --cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
    --cc=sakari.ailus@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox