Re: [PATCH v8 5/7] media: platform: amd: isp4 video node and buffers handling added

["Philip Müller" <philm@manjaro.org>]

Hi Bin Du,

for kernel 7.0-rc1 you may need to drop wait_prepare/finish callbacks. 
Instead require that the vb2_queue lock field is always set and use that 
lock when waiting for buffers to arrive.
See also: 
https://github.com/torvalds/linux/commit/b70886ff5833cf499e77af77d2324ce8f68b60ce

On 2/12/26 09:34, Bin Du wrote:
> Isp video implements v4l2 video interface and supports NV12 and YUYV. It
> manages buffers, pipeline power and state. Cherry-picked Sultan's DMA
> buffer related fix from branch v6.16-drm-tip-isp4-for-amd on
> https://github.com/kerneltoast/kernel_x86_laptop.git
>
> Co-developed-by: Sultan Alsawaf <sultan@kerneltoast.com>
> Signed-off-by: Sultan Alsawaf <sultan@kerneltoast.com>
> Co-developed-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com>
> Signed-off-by: Svetoslav Stoilov <Svetoslav.Stoilov@amd.com>
> Signed-off-by: Bin Du <Bin.Du@amd.com>
> Reviewed-by: Sultan Alsawaf <sultan@kerneltoast.com>
> Tested-by: Alexey Zagorodnikov <xglooom@gmail.com>
> Tested-by: Kate Hsuan <hpa@redhat.com>
> ---
>   MAINTAINERS                                   |   2 +
>   drivers/media/platform/amd/isp4/Makefile      |   3 +-
>   drivers/media/platform/amd/isp4/isp4.c        |  11 +
>   drivers/media/platform/amd/isp4/isp4_subdev.c |  13 +-
>   drivers/media/platform/amd/isp4/isp4_subdev.h |   2 +
>   drivers/media/platform/amd/isp4/isp4_video.c  | 799 ++++++++++++++++++
>   drivers/media/platform/amd/isp4/isp4_video.h  |  57 ++
>   7 files changed, 883 insertions(+), 4 deletions(-)
>   create mode 100644 drivers/media/platform/amd/isp4/isp4_video.c
>   create mode 100644 drivers/media/platform/amd/isp4/isp4_video.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 48ffc8bbdcee..80c966fde0b4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1151,6 +1151,8 @@ F:	drivers/media/platform/amd/isp4/isp4_interface.c
>   F:	drivers/media/platform/amd/isp4/isp4_interface.h
>   F:	drivers/media/platform/amd/isp4/isp4_subdev.c
>   F:	drivers/media/platform/amd/isp4/isp4_subdev.h
> +F:	drivers/media/platform/amd/isp4/isp4_video.c
> +F:	drivers/media/platform/amd/isp4/isp4_video.h
>   
>   AMD KFD
>   M:	Felix Kuehling <Felix.Kuehling@amd.com>
> diff --git a/drivers/media/platform/amd/isp4/Makefile b/drivers/media/platform/amd/isp4/Makefile
> index 7e4854b6a4cd..3fa0ee6d8a96 100644
> --- a/drivers/media/platform/amd/isp4/Makefile
> +++ b/drivers/media/platform/amd/isp4/Makefile
> @@ -5,4 +5,5 @@
>   obj-$(CONFIG_VIDEO_AMD_ISP4_CAPTURE) += amd_isp4_capture.o
>   amd_isp4_capture-objs := isp4.o \
>                            isp4_interface.o \
> -                         isp4_subdev.o
> +                         isp4_subdev.o \
> +                         isp4_video.o
> \ No newline at end of file
> diff --git a/drivers/media/platform/amd/isp4/isp4.c b/drivers/media/platform/amd/isp4/isp4.c
> index e62f5d652d81..9480dfffbcb2 100644
> --- a/drivers/media/platform/amd/isp4/isp4.c
> +++ b/drivers/media/platform/amd/isp4/isp4.c
> @@ -173,6 +173,17 @@ static int isp4_capture_probe(struct platform_device *pdev)
>   		goto err_pm_disable;
>   	}
>   
> +	ret = media_create_pad_link(&isp_dev->isp_subdev.sdev.entity,
> +				    0,
> +				    &isp_dev->isp_subdev.isp_vdev.vdev.entity,
> +				    0,
> +				    MEDIA_LNK_FL_ENABLED |
> +				    MEDIA_LNK_FL_IMMUTABLE);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "fail to create pad link\n");
> +		goto err_isp4_deinit;
> +	}
> +
>   	ret = media_device_register(&isp_dev->mdev);
>   	if (ret) {
>   		dev_err_probe(dev, ret, "fail to register media device\n");
> diff --git a/drivers/media/platform/amd/isp4/isp4_subdev.c b/drivers/media/platform/amd/isp4/isp4_subdev.c
> index 6d571b7f8840..5202232d50c5 100644
> --- a/drivers/media/platform/amd/isp4/isp4_subdev.c
> +++ b/drivers/media/platform/amd/isp4/isp4_subdev.c
> @@ -467,11 +467,13 @@ static void isp4sd_fw_resp_frame_done(struct isp4_subdev *isp_subdev,
>   	     meta->preview.status == ISP4FW_BUFFER_STATUS_DONE ||
>   	     meta->preview.status == ISP4FW_BUFFER_STATUS_DIRTY)) {
>   		prev = isp4if_dequeue_buffer(ispif);
> -		if (prev)
> +		if (prev) {
> +			isp4vid_handle_frame_done(&isp_subdev->isp_vdev,
> +						  &prev->buf_info);
>   			isp4if_dealloc_buffer_node(prev);
> -		else
> +		} else {
>   			dev_err(dev, "fail null prev buf\n");
> -
> +		}
>   	} else if (meta->preview.enabled) {
>   		dev_err(dev, "fail bad preview status %u\n",
>   			meta->preview.status);
> @@ -1006,6 +1008,10 @@ int isp4sd_init(struct isp4_subdev *isp_subdev, struct v4l2_device *v4l2_dev,
>   	isp_subdev->host2fw_seq_num = 1;
>   	ispif->status = ISP4IF_STATUS_PWR_OFF;
>   
> +	ret = isp4vid_dev_init(&isp_subdev->isp_vdev, &isp_subdev->sdev);
> +	if (ret)
> +		goto err_subdev_unreg;
> +
>   	return 0;
>   
>   err_subdev_unreg:
> @@ -1020,6 +1026,7 @@ void isp4sd_deinit(struct isp4_subdev *isp_subdev)
>   {
>   	struct isp4_interface *ispif = &isp_subdev->ispif;
>   
> +	isp4vid_dev_deinit(&isp_subdev->isp_vdev);
>   	v4l2_device_unregister_subdev(&isp_subdev->sdev);
>   	media_entity_cleanup(&isp_subdev->sdev.entity);
>   	isp4if_deinit(ispif);
> diff --git a/drivers/media/platform/amd/isp4/isp4_subdev.h b/drivers/media/platform/amd/isp4/isp4_subdev.h
> index ceade9fec5d9..ddf6bdf4a62a 100644
> --- a/drivers/media/platform/amd/isp4/isp4_subdev.h
> +++ b/drivers/media/platform/amd/isp4/isp4_subdev.h
> @@ -17,6 +17,7 @@
>   #include "isp4_fw_cmd_resp.h"
>   #include "isp4_hw_reg.h"
>   #include "isp4_interface.h"
> +#include "isp4_video.h"
>   
>   /*
>    * One is for none sensor specific response which is not used now.
> @@ -83,6 +84,7 @@ struct isp4_subdev_thread_param {
>   struct isp4_subdev {
>   	struct v4l2_subdev sdev;
>   	struct isp4_interface ispif;
> +	struct isp4vid_dev isp_vdev;
>   
>   	struct media_pad sdev_pad;
>   
> diff --git a/drivers/media/platform/amd/isp4/isp4_video.c b/drivers/media/platform/amd/isp4/isp4_video.c
> new file mode 100644
> index 000000000000..15a7d7c1e4c6
> --- /dev/null
> +++ b/drivers/media/platform/amd/isp4/isp4_video.c
> @@ -0,0 +1,799 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#include <media/v4l2-ioctl.h>
> +#include <media/v4l2-mc.h>
> +#include <media/videobuf2-vmalloc.h>
> +
> +#include "isp4_interface.h"
> +#include "isp4_subdev.h"
> +#include "isp4_video.h"
> +
> +#define ISP4VID_ISP_DRV_NAME "amd_isp_capture"
> +#define ISP4VID_MAX_PREVIEW_FPS 30
> +#define ISP4VID_DEFAULT_FMT isp4vid_formats[0]
> +
> +#define ISP4VID_PAD_VIDEO_OUTPUT 0
> +
> +/* time perframe default */
> +#define ISP4VID_ISP_TPF_DEFAULT isp4vid_tpfs[0]
> +
> +static const char *const isp4vid_video_dev_name = "Preview";
> +
> +/* Sizes must be in increasing order */
> +static const struct v4l2_frmsize_discrete isp4vid_frmsize[] = {
> +	{640, 360},
> +	{640, 480},
> +	{1280, 720},
> +	{1280, 960},
> +	{1920, 1080},
> +	{1920, 1440},
> +	{2560, 1440},
> +	{2880, 1620},
> +	{2880, 1624},
> +	{2888, 1808},
> +};
> +
> +static const u32 isp4vid_formats[] = {
> +	V4L2_PIX_FMT_NV12,
> +	V4L2_PIX_FMT_YUYV
> +};
> +
> +/* time perframe list */
> +static const struct v4l2_fract isp4vid_tpfs[] = {
> +	{ 1, ISP4VID_MAX_PREVIEW_FPS }
> +};
> +
> +void isp4vid_handle_frame_done(struct isp4vid_dev *isp_vdev,
> +			       const struct isp4if_img_buf_info *img_buf)
> +{
> +	struct isp4vid_capture_buffer *isp4vid_buf;
> +	void *vbuf;
> +
> +	scoped_guard(mutex, &isp_vdev->buf_list_lock) {
> +		isp4vid_buf = list_first_entry_or_null(&isp_vdev->buf_list,
> +						       typeof(*isp4vid_buf),
> +						       list);
> +		if (!isp4vid_buf)
> +			return;
> +
> +		vbuf = vb2_plane_vaddr(&isp4vid_buf->vb2.vb2_buf, 0);
> +
> +		if (vbuf != img_buf->planes[0].sys_addr) {
> +			dev_err(isp_vdev->dev, "Invalid vbuf\n");
> +			return;
> +		}
> +
> +		list_del(&isp4vid_buf->list);
> +	}
> +
> +	/* Fill the buffer */
> +	isp4vid_buf->vb2.vb2_buf.timestamp = ktime_get_ns();
> +	isp4vid_buf->vb2.sequence = isp_vdev->sequence++;
> +	isp4vid_buf->vb2.field = V4L2_FIELD_ANY;
> +
> +	vb2_set_plane_payload(&isp4vid_buf->vb2.vb2_buf,
> +			      0, isp_vdev->format.sizeimage);
> +
> +	vb2_buffer_done(&isp4vid_buf->vb2.vb2_buf, VB2_BUF_STATE_DONE);
> +
> +	dev_dbg(isp_vdev->dev, "call vb2_buffer_done(size=%u)\n",
> +		isp_vdev->format.sizeimage);
> +}
> +
> +static const struct v4l2_pix_format isp4vid_fmt_default = {
> +	.width = 1920,
> +	.height = 1080,
> +	.pixelformat = ISP4VID_DEFAULT_FMT,
> +	.field = V4L2_FIELD_NONE,
> +	.colorspace = V4L2_COLORSPACE_SRGB,
> +};
> +
> +static void isp4vid_capture_return_all_buffers(struct isp4vid_dev *isp_vdev,
> +					       enum vb2_buffer_state state)
> +{
> +	struct isp4vid_capture_buffer *vbuf, *node;
> +
> +	scoped_guard(mutex, &isp_vdev->buf_list_lock) {
> +		list_for_each_entry_safe(vbuf, node, &isp_vdev->buf_list, list)
> +			vb2_buffer_done(&vbuf->vb2.vb2_buf, state);
> +		INIT_LIST_HEAD(&isp_vdev->buf_list);
> +	}
> +
> +	dev_dbg(isp_vdev->dev, "call vb2_buffer_done(%d)\n", state);
> +}
> +
> +static int isp4vid_vdev_link_validate(struct media_link *link)
> +{
> +	return 0;
> +}
> +
> +static const struct media_entity_operations isp4vid_vdev_ent_ops = {
> +	.link_validate = isp4vid_vdev_link_validate,
> +};
> +
> +static const struct v4l2_file_operations isp4vid_vdev_fops = {
> +	.owner = THIS_MODULE,
> +	.open = v4l2_fh_open,
> +	.release = vb2_fop_release,
> +	.read = vb2_fop_read,
> +	.poll = vb2_fop_poll,
> +	.unlocked_ioctl = video_ioctl2,
> +	.mmap = vb2_fop_mmap,
> +};
> +
> +static int isp4vid_ioctl_querycap(struct file *file, void *fh,
> +				  struct v4l2_capability *cap)
> +{
> +	struct isp4vid_dev *isp_vdev = video_drvdata(file);
> +
> +	strscpy(cap->driver, ISP4VID_ISP_DRV_NAME, sizeof(cap->driver));
> +	snprintf(cap->card, sizeof(cap->card), "%s", ISP4VID_ISP_DRV_NAME);
> +	cap->capabilities |= V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_CAPTURE;
> +
> +	dev_dbg(isp_vdev->dev, "%s|capabilities=0x%X\n", isp_vdev->vdev.name,
> +		cap->capabilities);
> +
> +	return 0;
> +}
> +
> +static int isp4vid_g_fmt_vid_cap(struct file *file, void *priv,
> +				 struct v4l2_format *f)
> +{
> +	struct isp4vid_dev *isp_vdev = video_drvdata(file);
> +
> +	f->fmt.pix = isp_vdev->format;
> +
> +	return 0;
> +}
> +
> +static int isp4vid_fill_buffer_size(struct v4l2_pix_format *fmt)
> +{
> +	int ret = 0;
> +
> +	switch (fmt->pixelformat) {
> +	case V4L2_PIX_FMT_NV12:
> +		fmt->bytesperline = fmt->width;
> +		fmt->sizeimage = fmt->bytesperline * fmt->height * 3 / 2;
> +		break;
> +	case V4L2_PIX_FMT_YUYV:
> +		fmt->bytesperline = fmt->width * 2;
> +		fmt->sizeimage = fmt->bytesperline * fmt->height;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int isp4vid_try_fmt_vid_cap(struct file *file, void *priv,
> +				   struct v4l2_format *f)
> +{
> +	struct isp4vid_dev *isp_vdev = video_drvdata(file);
> +	struct v4l2_pix_format *format = &f->fmt.pix;
> +	const struct v4l2_frmsize_discrete *fsz;
> +	size_t i;
> +
> +	/*
> +	 * Check if the hardware supports the requested format, use the default
> +	 * format otherwise.
> +	 */
> +	for (i = 0; i < ARRAY_SIZE(isp4vid_formats); i++)
> +		if (isp4vid_formats[i] == format->pixelformat)
> +			break;
> +
> +	if (i == ARRAY_SIZE(isp4vid_formats))
> +		format->pixelformat = ISP4VID_DEFAULT_FMT;
> +
> +	switch (format->pixelformat) {
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_YUYV:
> +		fsz = v4l2_find_nearest_size(isp4vid_frmsize,
> +					     ARRAY_SIZE(isp4vid_frmsize),
> +					     width, height, format->width,
> +					     format->height);
> +		format->width = fsz->width;
> +		format->height = fsz->height;
> +		break;
> +	default:
> +		dev_err(isp_vdev->dev, "%s|unsupported fmt=%u\n",
> +			isp_vdev->vdev.name,
> +			format->pixelformat);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * There is no need to check the return value, as failure will never
> +	 * happen here
> +	 */
> +	isp4vid_fill_buffer_size(format);
> +
> +	if (format->field == V4L2_FIELD_ANY)
> +		format->field = isp4vid_fmt_default.field;
> +
> +	if (format->colorspace == V4L2_COLORSPACE_DEFAULT)
> +		format->colorspace = isp4vid_fmt_default.colorspace;
> +
> +	return 0;
> +}
> +
> +static int isp4vid_set_fmt_2_isp(struct v4l2_subdev *sdev,
> +				 struct v4l2_pix_format *pix_fmt)
> +{
> +	struct v4l2_subdev_format fmt = {};
> +
> +	switch (pix_fmt->pixelformat) {
> +	case V4L2_PIX_FMT_NV12:
> +		fmt.format.code = MEDIA_BUS_FMT_YUYV8_1_5X8;
> +		break;
> +	case V4L2_PIX_FMT_YUYV:
> +		fmt.format.code = MEDIA_BUS_FMT_YUYV8_1X16;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	fmt.which = V4L2_SUBDEV_FORMAT_ACTIVE;
> +	fmt.pad = ISP4VID_PAD_VIDEO_OUTPUT;
> +	fmt.format.width = pix_fmt->width;
> +	fmt.format.height = pix_fmt->height;
> +	return v4l2_subdev_call(sdev, pad, set_fmt, NULL, &fmt);
> +}
> +
> +static int isp4vid_s_fmt_vid_cap(struct file *file, void *priv,
> +				 struct v4l2_format *f)
> +{
> +	struct isp4vid_dev *isp_vdev = video_drvdata(file);
> +	int ret;
> +
> +	/* Do not change the format while stream is on */
> +	if (vb2_is_busy(&isp_vdev->vbq))
> +		return -EBUSY;
> +
> +	ret = isp4vid_try_fmt_vid_cap(file, priv, f);
> +	if (ret)
> +		return ret;
> +
> +	dev_dbg(isp_vdev->dev, "%s|width height:%ux%u->%ux%u\n",
> +		isp_vdev->vdev.name,
> +		isp_vdev->format.width, isp_vdev->format.height,
> +		f->fmt.pix.width, f->fmt.pix.height);
> +	dev_dbg(isp_vdev->dev, "%s|pixelformat:0x%x-0x%x\n",
> +		isp_vdev->vdev.name, isp_vdev->format.pixelformat,
> +		f->fmt.pix.pixelformat);
> +	dev_dbg(isp_vdev->dev, "%s|bytesperline:%u->%u\n",
> +		isp_vdev->vdev.name, isp_vdev->format.bytesperline,
> +		f->fmt.pix.bytesperline);
> +	dev_dbg(isp_vdev->dev, "%s|sizeimage:%u->%u\n",
> +		isp_vdev->vdev.name, isp_vdev->format.sizeimage,
> +		f->fmt.pix.sizeimage);
> +
> +	isp_vdev->format = f->fmt.pix;
> +	ret = isp4vid_set_fmt_2_isp(isp_vdev->isp_sdev, &isp_vdev->format);
> +
> +	return ret;
> +}
> +
> +static int isp4vid_enum_fmt_vid_cap(struct file *file, void *priv,
> +				    struct v4l2_fmtdesc *f)
> +{
> +	struct isp4vid_dev *isp_vdev = video_drvdata(file);
> +
> +	switch (f->index) {
> +	case 0:
> +		f->pixelformat = V4L2_PIX_FMT_NV12;
> +		break;
> +	case 1:
> +		f->pixelformat = V4L2_PIX_FMT_YUYV;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(isp_vdev->dev, "%s|index=%d, pixelformat=0x%X\n",
> +		isp_vdev->vdev.name, f->index, f->pixelformat);
> +
> +	return 0;
> +}
> +
> +static int isp4vid_enum_framesizes(struct file *file, void *fh,
> +				   struct v4l2_frmsizeenum *fsize)
> +{
> +	struct isp4vid_dev *isp_vdev = video_drvdata(file);
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(isp4vid_formats); i++) {
> +		if (isp4vid_formats[i] == fsize->pixel_format)
> +			break;
> +	}
> +
> +	if (i == ARRAY_SIZE(isp4vid_formats))
> +		return -EINVAL;
> +
> +	if (fsize->index < ARRAY_SIZE(isp4vid_frmsize)) {
> +		fsize->type = V4L2_FRMSIZE_TYPE_DISCRETE;
> +		fsize->discrete = isp4vid_frmsize[fsize->index];
> +		dev_dbg(isp_vdev->dev, "%s|size[%d]=%dx%d\n",
> +			isp_vdev->vdev.name, fsize->index,
> +			fsize->discrete.width, fsize->discrete.height);
> +	} else {
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int isp4vid_ioctl_enum_frameintervals(struct file *file, void *priv,
> +					     struct v4l2_frmivalenum *fival)
> +{
> +	struct isp4vid_dev *isp_vdev = video_drvdata(file);
> +	size_t i;
> +
> +	if (fival->index >= ARRAY_SIZE(isp4vid_tpfs))
> +		return -EINVAL;
> +
> +	for (i = 0; i < ARRAY_SIZE(isp4vid_formats); i++)
> +		if (isp4vid_formats[i] == fival->pixel_format)
> +			break;
> +
> +	if (i == ARRAY_SIZE(isp4vid_formats))
> +		return -EINVAL;
> +
> +	for (i = 0; i < ARRAY_SIZE(isp4vid_frmsize); i++)
> +		if (isp4vid_frmsize[i].width == fival->width &&
> +		    isp4vid_frmsize[i].height == fival->height)
> +			break;
> +
> +	if (i == ARRAY_SIZE(isp4vid_frmsize))
> +		return -EINVAL;
> +
> +	fival->type = V4L2_FRMIVAL_TYPE_DISCRETE;
> +	fival->discrete = isp4vid_tpfs[fival->index];
> +	v4l2_simplify_fraction(&fival->discrete.numerator,
> +			       &fival->discrete.denominator, 8, 333);
> +
> +	dev_dbg(isp_vdev->dev, "%s|interval[%d]=%d/%d\n",
> +		isp_vdev->vdev.name, fival->index,
> +		fival->discrete.numerator,
> +		fival->discrete.denominator);
> +
> +	return 0;
> +}
> +
> +static int isp4vid_ioctl_g_param(struct file *file, void *priv,
> +				 struct v4l2_streamparm *param)
> +{
> +	struct v4l2_captureparm *capture = &param->parm.capture;
> +	struct isp4vid_dev *isp_vdev = video_drvdata(file);
> +
> +	if (param->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +		return -EINVAL;
> +
> +	capture->capability   = V4L2_CAP_TIMEPERFRAME;
> +	capture->timeperframe = isp_vdev->timeperframe;
> +	capture->readbuffers  = 0;
> +
> +	dev_dbg(isp_vdev->dev, "%s|timeperframe=%d/%d\n", isp_vdev->vdev.name,
> +		capture->timeperframe.numerator,
> +		capture->timeperframe.denominator);
> +
> +	return 0;
> +}
> +
> +static const struct v4l2_ioctl_ops isp4vid_vdev_ioctl_ops = {
> +	.vidioc_querycap            = isp4vid_ioctl_querycap,
> +	.vidioc_enum_fmt_vid_cap    = isp4vid_enum_fmt_vid_cap,
> +	.vidioc_g_fmt_vid_cap       = isp4vid_g_fmt_vid_cap,
> +	.vidioc_s_fmt_vid_cap       = isp4vid_s_fmt_vid_cap,
> +	.vidioc_try_fmt_vid_cap     = isp4vid_try_fmt_vid_cap,
> +	.vidioc_reqbufs             = vb2_ioctl_reqbufs,
> +	.vidioc_querybuf            = vb2_ioctl_querybuf,
> +	.vidioc_qbuf                = vb2_ioctl_qbuf,
> +	.vidioc_expbuf              = vb2_ioctl_expbuf,
> +	.vidioc_dqbuf               = vb2_ioctl_dqbuf,
> +	.vidioc_create_bufs         = vb2_ioctl_create_bufs,
> +	.vidioc_prepare_buf         = vb2_ioctl_prepare_buf,
> +	.vidioc_streamon            = vb2_ioctl_streamon,
> +	.vidioc_streamoff           = vb2_ioctl_streamoff,
> +	.vidioc_g_parm              = isp4vid_ioctl_g_param,
> +	.vidioc_s_parm              = isp4vid_ioctl_g_param,
> +	.vidioc_enum_framesizes     = isp4vid_enum_framesizes,
> +	.vidioc_enum_frameintervals = isp4vid_ioctl_enum_frameintervals,
> +};
> +
> +static unsigned int isp4vid_get_image_size(struct v4l2_pix_format *fmt)
> +{
> +	switch (fmt->pixelformat) {
> +	case V4L2_PIX_FMT_NV12:
> +		return fmt->width * fmt->height * 3 / 2;
> +	case V4L2_PIX_FMT_YUYV:
> +		return fmt->width * fmt->height * 2;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static int isp4vid_qops_queue_setup(struct vb2_queue *vq,
> +				    unsigned int *nbuffers,
> +				    unsigned int *nplanes, unsigned int sizes[],
> +				    struct device *alloc_devs[])
> +{
> +	struct isp4vid_dev *isp_vdev = vb2_get_drv_priv(vq);
> +	unsigned int q_num_bufs = vb2_get_num_buffers(vq);
> +
> +	if (*nplanes > 1) {
> +		dev_err(isp_vdev->dev,
> +			"fail to setup queue, no mplane supported %u\n",
> +			*nplanes);
> +		return -EINVAL;
> +	}
> +
> +	if (*nplanes == 1) {
> +		unsigned int size;
> +
> +		size = isp4vid_get_image_size(&isp_vdev->format);
> +		if (sizes[0] < size) {
> +			dev_err(isp_vdev->dev,
> +				"fail for small plane size %u, %u expected\n",
> +				sizes[0], size);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (q_num_bufs + *nbuffers < ISP4IF_MAX_STREAM_BUF_COUNT)
> +		*nbuffers = ISP4IF_MAX_STREAM_BUF_COUNT - q_num_bufs;
> +
> +	switch (isp_vdev->format.pixelformat) {
> +	case V4L2_PIX_FMT_NV12:
> +	case V4L2_PIX_FMT_YUYV: {
> +		*nplanes = 1;
> +		sizes[0] = max(sizes[0], isp_vdev->format.sizeimage);
> +		isp_vdev->format.sizeimage = sizes[0];
> +	}
> +	break;
> +	default:
> +		dev_err(isp_vdev->dev, "%s|unsupported fmt=%u\n",
> +			isp_vdev->vdev.name, isp_vdev->format.pixelformat);
> +		return -EINVAL;
> +	}
> +
> +	dev_dbg(isp_vdev->dev, "%s|*nbuffers=%u *nplanes=%u sizes[0]=%u\n",
> +		isp_vdev->vdev.name,
> +		*nbuffers, *nplanes, sizes[0]);
> +
> +	return 0;
> +}
> +
> +static void isp4vid_qops_buffer_queue(struct vb2_buffer *vb)
> +{
> +	struct isp4vid_capture_buffer *buf =
> +		container_of(vb, struct isp4vid_capture_buffer, vb2.vb2_buf);
> +	struct isp4vid_dev *isp_vdev = vb2_get_drv_priv(vb->vb2_queue);
> +	struct isp4if_img_buf_info *img_buf = &buf->img_buf;
> +	void *vaddr = vb2_plane_vaddr(vb, 0);
> +
> +	dev_dbg(isp_vdev->dev, "queue buf, vaddr %p, gpuva 0x%llx, size %u\n",
> +		vaddr, buf->gpu_addr, vb->planes[0].length);
> +
> +	switch (isp_vdev->format.pixelformat) {
> +	case V4L2_PIX_FMT_NV12: {
> +		u32 y_size = isp_vdev->format.sizeimage / 3 * 2;
> +		u32 uv_size = isp_vdev->format.sizeimage / 3;
> +
> +		img_buf->planes[0].len = y_size;
> +		img_buf->planes[0].sys_addr = vaddr;
> +		img_buf->planes[0].mc_addr = buf->gpu_addr;
> +
> +		dev_dbg(isp_vdev->dev, "img_buf[0]: mc=0x%llx size=%u\n",
> +			img_buf->planes[0].mc_addr,
> +			img_buf->planes[0].len);
> +
> +		img_buf->planes[1].len = uv_size;
> +		img_buf->planes[1].sys_addr = vaddr + y_size;
> +		img_buf->planes[1].mc_addr = buf->gpu_addr + y_size;
> +
> +		dev_dbg(isp_vdev->dev, "img_buf[1]: mc=0x%llx size=%u\n",
> +			img_buf->planes[1].mc_addr,
> +			img_buf->planes[1].len);
> +
> +		img_buf->planes[2].len = 0;
> +	}
> +	break;
> +	case V4L2_PIX_FMT_YUYV: {
> +		img_buf->planes[0].len = isp_vdev->format.sizeimage;
> +		img_buf->planes[0].sys_addr = vaddr;
> +		img_buf->planes[0].mc_addr = buf->gpu_addr;
> +
> +		dev_dbg(isp_vdev->dev, "img_buf[0]: mc=0x%llx size=%u\n",
> +			img_buf->planes[0].mc_addr,
> +			img_buf->planes[0].len);
> +
> +		img_buf->planes[1].len = 0;
> +		img_buf->planes[2].len = 0;
> +	}
> +	break;
> +	default:
> +		dev_err(isp_vdev->dev, "%s|unsupported fmt=%u\n",
> +			isp_vdev->vdev.name, isp_vdev->format.pixelformat);
> +		return;
> +	}
> +
> +	if (isp_vdev->stream_started)
> +		isp4sd_ioc_send_img_buf(isp_vdev->isp_sdev, img_buf);
> +
> +	scoped_guard(mutex, &isp_vdev->buf_list_lock)
> +		list_add_tail(&buf->list, &isp_vdev->buf_list);
> +}
> +
> +static int isp4vid_qops_start_streaming(struct vb2_queue *vq,
> +					unsigned int count)
> +{
> +	struct isp4vid_dev *isp_vdev = vb2_get_drv_priv(vq);
> +	struct isp4vid_capture_buffer *isp4vid_buf;
> +	struct media_entity *entity;
> +	struct v4l2_subdev *subdev;
> +	struct media_pad *pad;
> +	int ret = 0;
> +
> +	isp_vdev->sequence = 0;
> +
> +	ret = isp4sd_pwron_and_init(isp_vdev->isp_sdev);
> +	if (ret) {
> +		dev_err(isp_vdev->dev, "power up isp fail %d\n", ret);
> +		goto release_buffers;
> +	}
> +
> +	entity = &isp_vdev->vdev.entity;
> +	while (1) {
> +		pad = &entity->pads[0];
> +		if (!(pad->flags & MEDIA_PAD_FL_SINK))
> +			break;
> +
> +		pad = media_pad_remote_pad_first(pad);
> +		if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
> +			break;
> +
> +		entity = pad->entity;
> +		subdev = media_entity_to_v4l2_subdev(entity);
> +
> +		ret = v4l2_subdev_call(subdev, video, s_stream, 1);
> +		if (ret < 0 && ret != -ENOIOCTLCMD) {
> +			dev_dbg(isp_vdev->dev, "fail start streaming: %s %d\n",
> +				subdev->name, ret);
> +			goto release_buffers;
> +		}
> +	}
> +
> +	list_for_each_entry(isp4vid_buf, &isp_vdev->buf_list, list)
> +		isp4sd_ioc_send_img_buf(isp_vdev->isp_sdev,
> +					&isp4vid_buf->img_buf);
> +
> +	isp_vdev->stream_started = true;
> +
> +	return 0;
> +
> +release_buffers:
> +	isp4vid_capture_return_all_buffers(isp_vdev, VB2_BUF_STATE_QUEUED);
> +	return ret;
> +}
> +
> +static void isp4vid_qops_stop_streaming(struct vb2_queue *vq)
> +{
> +	struct isp4vid_dev *isp_vdev = vb2_get_drv_priv(vq);
> +	struct media_entity *entity;
> +	struct v4l2_subdev *subdev;
> +	struct media_pad *pad;
> +	int ret;
> +
> +	entity = &isp_vdev->vdev.entity;
> +	while (1) {
> +		pad = &entity->pads[0];
> +		if (!(pad->flags & MEDIA_PAD_FL_SINK))
> +			break;
> +
> +		pad = media_pad_remote_pad_first(pad);
> +		if (!pad || !is_media_entity_v4l2_subdev(pad->entity))
> +			break;
> +
> +		entity = pad->entity;
> +		subdev = media_entity_to_v4l2_subdev(entity);
> +
> +		ret = v4l2_subdev_call(subdev, video, s_stream, 0);
> +
> +		if (ret < 0 && ret != -ENOIOCTLCMD)
> +			dev_dbg(isp_vdev->dev, "fail stop streaming: %s %d\n",
> +				subdev->name, ret);
> +	}
> +
> +	isp_vdev->stream_started = false;
> +	isp4sd_pwroff_and_deinit(isp_vdev->isp_sdev);
> +
> +	/* Release all active buffers */
> +	isp4vid_capture_return_all_buffers(isp_vdev, VB2_BUF_STATE_ERROR);
> +}
> +
> +static int isp4vid_qops_buf_init(struct vb2_buffer *vb)
> +{
> +	struct isp4vid_capture_buffer *buf =
> +		container_of(vb, struct isp4vid_capture_buffer, vb2.vb2_buf);
> +	struct isp4vid_dev *isp_vdev = vb2_get_drv_priv(vb->vb2_queue);
> +	void *mem_priv = vb->planes[0].mem_priv;
> +	struct device *dev = isp_vdev->dev;
> +	u64 gpu_addr;
> +	void *bo;
> +	int ret;
> +
> +	if (vb->planes[0].dbuf) {
> +		buf->dbuf = vb->planes[0].dbuf;
> +	} else {
> +		/*
> +		 * HAS_DMA is a Kconfig dependency so CONFIG_HAS_DMA is always
> +		 * defined when this driver is compiled. The #else branch is
> +		 * kept as a safeguard in case the dependency is ever removed.
> +		 */
> +#ifdef CONFIG_HAS_DMA
> +		buf->dbuf = vb2_vmalloc_memops.get_dmabuf(vb, mem_priv, 0);
> +		if (IS_ERR_OR_NULL(buf->dbuf)) {
> +			dev_err(dev, "fail to get dma buf\n");
> +			return -EINVAL;
> +		}
> +#else
> +		dev_err(dev, "get dmabuf fail -- CONFIG_HAS_DMA not defined\n");
> +		buf->dbuf = NULL;
> +		return -EINVAL;
> +#endif
> +	}
> +
> +	/* create isp user BO and obtain gpu_addr */
> +	ret = isp_user_buffer_alloc(dev, buf->dbuf, &bo, &gpu_addr);
> +	if (ret) {
> +		dev_err(dev, "fail to create isp user BO\n");
> +		if (!vb->planes[0].dbuf) {
> +			dma_buf_put(buf->dbuf);
> +			buf->dbuf = NULL;
> +		}
> +
> +		return ret;
> +	}
> +
> +	buf->bo = bo;
> +	buf->gpu_addr = gpu_addr;
> +	return 0;
> +}
> +
> +static void isp4vid_qops_buf_cleanup(struct vb2_buffer *vb)
> +{
> +	struct isp4vid_capture_buffer *buf =
> +		container_of(vb, struct isp4vid_capture_buffer, vb2.vb2_buf);
> +
> +	if (buf->bo) {
> +		isp_user_buffer_free(buf->bo);
> +		buf->bo = NULL;
> +	}
> +
> +	/*
> +	 * Only put dmabufs we obtained ourselves via get_dmabuf, not ones
> +	 * provided by the framework for DMABUF import
> +	 */
> +	if (buf->dbuf && buf->dbuf != vb->planes[0].dbuf)
> +		dma_buf_put(buf->dbuf);
> +
> +	buf->dbuf = NULL;
> +}
> +
> +static const struct vb2_ops isp4vid_qops = {
> +	.queue_setup = isp4vid_qops_queue_setup,
> +	.wait_prepare = vb2_ops_wait_prepare,
> +	.wait_finish = vb2_ops_wait_finish,
> +	.buf_init = isp4vid_qops_buf_init,
> +	.buf_cleanup = isp4vid_qops_buf_cleanup,
> +	.start_streaming = isp4vid_qops_start_streaming,
> +	.stop_streaming = isp4vid_qops_stop_streaming,
> +	.buf_queue = isp4vid_qops_buffer_queue,
> +};
> +
> +int isp4vid_dev_init(struct isp4vid_dev *isp_vdev, struct v4l2_subdev *isp_sd)
> +{
> +	const char *vdev_name = isp4vid_video_dev_name;
> +	struct v4l2_device *v4l2_dev;
> +	struct video_device *vdev;
> +	struct vb2_queue *q;
> +	int ret;
> +
> +	if (!isp_vdev || !isp_sd || !isp_sd->v4l2_dev)
> +		return -EINVAL;
> +
> +	v4l2_dev = isp_sd->v4l2_dev;
> +	vdev = &isp_vdev->vdev;
> +
> +	isp_vdev->isp_sdev = isp_sd;
> +	isp_vdev->dev = v4l2_dev->dev;
> +
> +	/* Initialize the vb2_queue struct */
> +	mutex_init(&isp_vdev->vbq_lock);
> +	q = &isp_vdev->vbq;
> +	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +	q->io_modes = VB2_MMAP | VB2_DMABUF;
> +	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +	q->buf_struct_size = sizeof(struct isp4vid_capture_buffer);
> +	q->min_queued_buffers = 2;
> +	q->ops = &isp4vid_qops;
> +	q->drv_priv = isp_vdev;
> +	q->mem_ops = &vb2_vmalloc_memops;
> +	q->lock = &isp_vdev->vbq_lock;
> +	q->dev = v4l2_dev->dev;
> +	ret = vb2_queue_init(q);
> +	if (ret) {
> +		dev_err(v4l2_dev->dev, "vb2_queue_init error:%d\n", ret);
> +		return ret;
> +	}
> +
> +	/* Initialize buffer list and its lock */
> +	mutex_init(&isp_vdev->buf_list_lock);
> +	INIT_LIST_HEAD(&isp_vdev->buf_list);
> +
> +	/* Set default frame format */
> +	isp_vdev->format = isp4vid_fmt_default;
> +	isp_vdev->timeperframe = ISP4VID_ISP_TPF_DEFAULT;
> +	v4l2_simplify_fraction(&isp_vdev->timeperframe.numerator,
> +			       &isp_vdev->timeperframe.denominator, 8, 333);
> +
> +	ret = isp4vid_fill_buffer_size(&isp_vdev->format);
> +	if (ret) {
> +		dev_err(v4l2_dev->dev, "fail to fill buffer size: %d\n", ret);
> +		goto err_release_vb2_queue;
> +	}
> +
> +	ret = isp4vid_set_fmt_2_isp(isp_sd, &isp_vdev->format);
> +	if (ret) {
> +		dev_err(v4l2_dev->dev, "fail init format :%d\n", ret);
> +		goto err_release_vb2_queue;
> +	}
> +
> +	/* Initialize the video_device struct */
> +	isp_vdev->vdev.entity.name = vdev_name;
> +	isp_vdev->vdev.entity.function = MEDIA_ENT_F_IO_V4L;
> +	isp_vdev->vdev_pad.flags = MEDIA_PAD_FL_SINK;
> +	ret = media_entity_pads_init(&isp_vdev->vdev.entity, 1,
> +				     &isp_vdev->vdev_pad);
> +
> +	if (ret) {
> +		dev_err(v4l2_dev->dev, "init media entity pad fail:%d\n", ret);
> +		goto err_release_vb2_queue;
> +	}
> +
> +	vdev->device_caps = V4L2_CAP_VIDEO_CAPTURE |
> +			    V4L2_CAP_STREAMING | V4L2_CAP_IO_MC;
> +	vdev->entity.ops = &isp4vid_vdev_ent_ops;
> +	vdev->release = video_device_release_empty;
> +	vdev->fops = &isp4vid_vdev_fops;
> +	vdev->ioctl_ops = &isp4vid_vdev_ioctl_ops;
> +	vdev->lock = NULL;
> +	vdev->queue = q;
> +	vdev->v4l2_dev = v4l2_dev;
> +	vdev->vfl_dir = VFL_DIR_RX;
> +	strscpy(vdev->name, vdev_name, sizeof(vdev->name));
> +	video_set_drvdata(vdev, isp_vdev);
> +
> +	ret = video_register_device(vdev, VFL_TYPE_VIDEO, -1);
> +	if (ret) {
> +		dev_err(v4l2_dev->dev, "register video device fail:%d\n", ret);
> +		goto err_entity_cleanup;
> +	}
> +
> +	return 0;
> +
> +err_entity_cleanup:
> +	media_entity_cleanup(&isp_vdev->vdev.entity);
> +err_release_vb2_queue:
> +	vb2_queue_release(q);
> +	return ret;
> +}
> +
> +void isp4vid_dev_deinit(struct isp4vid_dev *isp_vdev)
> +{
> +	vb2_video_unregister_device(&isp_vdev->vdev);
> +}
> diff --git a/drivers/media/platform/amd/isp4/isp4_video.h b/drivers/media/platform/amd/isp4/isp4_video.h
> new file mode 100644
> index 000000000000..c66451e26166
> --- /dev/null
> +++ b/drivers/media/platform/amd/isp4/isp4_video.h
> @@ -0,0 +1,57 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#ifndef _ISP4_VIDEO_H_
> +#define _ISP4_VIDEO_H_
> +
> +#include <media/v4l2-dev.h>
> +#include <media/videobuf2-memops.h>
> +
> +#include "isp4_interface.h"
> +
> +struct isp4vid_capture_buffer {
> +	/*
> +	 * struct vb2_v4l2_buffer must be the first element
> +	 * the videobuf2 framework will allocate this struct based on
> +	 * buf_struct_size and use the first sizeof(struct vb2_buffer) bytes of
> +	 * memory as a vb2_buffer
> +	 */
> +	struct vb2_v4l2_buffer vb2;
> +	struct isp4if_img_buf_info img_buf;
> +	struct list_head list;
> +	struct dma_buf *dbuf;
> +	void *bo;
> +	u64 gpu_addr;
> +};
> +
> +struct isp4vid_dev {
> +	struct video_device vdev;
> +	struct media_pad vdev_pad;
> +	struct v4l2_pix_format format;
> +
> +	/* mutex that protects vbq */
> +	struct mutex vbq_lock;
> +	struct vb2_queue vbq;
> +
> +	/* mutex that protects buf_list */
> +	struct mutex buf_list_lock;
> +	struct list_head buf_list;
> +
> +	u32 sequence;
> +	bool stream_started;
> +
> +	struct device *dev;
> +	struct v4l2_subdev *isp_sdev;
> +	struct v4l2_fract timeperframe;
> +};
> +
> +int isp4vid_dev_init(struct isp4vid_dev *isp_vdev, struct v4l2_subdev *isp_sd);
> +
> +void isp4vid_dev_deinit(struct isp4vid_dev *isp_vdev);
> +
> +void isp4vid_handle_frame_done(struct isp4vid_dev *isp_vdev,
> +			       const struct isp4if_img_buf_info *img_buf);
> +
> +#endif /* _ISP4_VIDEO_H_ */


-- 
Best, Philip