linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Prepare for statless decoder for fwht
@ 2019-01-26 13:47 Dafna Hirschfeld
  2019-01-26 13:47 ` [PATCH 1/3] media: vicodec: add struct for encoder/decoder instance Dafna Hirschfeld
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Dafna Hirschfeld @ 2019-01-26 13:47 UTC (permalink / raw)
  To: linux-media; +Cc: hverkuil, helen.koike, Dafna Hirschfeld

Patches to prepare for the implementation of the statless
decoder api in vicodec

Dafna Hirschfeld (3):
  media: vicodec: add struct for encoder/decoder instance
  media: vicodec: Introducing stateless fwht defs and structs
  media: vicodec: Register another node for stateless decoder

 drivers/media/platform/vicodec/vicodec-core.c | 231 +++++++++++-------
 drivers/media/v4l2-core/v4l2-ctrls.c          |   6 +
 include/uapi/linux/v4l2-controls.h            |  10 +
 include/uapi/linux/videodev2.h                |   1 +
 4 files changed, 163 insertions(+), 85 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] media: vicodec: add struct for encoder/decoder instance
  2019-01-26 13:47 [PATCH 0/3] Prepare for statless decoder for fwht Dafna Hirschfeld
@ 2019-01-26 13:47 ` Dafna Hirschfeld
  2019-01-28  9:22   ` Hans Verkuil
  2019-01-26 13:47 ` [PATCH 2/3] media: vicodec: Introducing stateless fwht defs and structs Dafna Hirschfeld
  2019-01-26 13:47 ` [PATCH 3/3] media: vicodec: Register another node for stateless decoder Dafna Hirschfeld
  2 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2019-01-26 13:47 UTC (permalink / raw)
  To: linux-media; +Cc: hverkuil, helen.koike, Dafna Hirschfeld

Add struct 'vicodec_dev_instance' for the fields in vicodec_dev
that have have both decoder and encoder versions.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 175 +++++++++---------
 1 file changed, 89 insertions(+), 86 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 953b9c4816a5..370517707324 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -89,21 +89,21 @@ enum {
 	V4L2_M2M_DST = 1,
 };
 
+struct vicodec_dev_instance {
+	struct video_device     vfd;
+	struct mutex            mutex;
+	spinlock_t              lock;
+	struct v4l2_m2m_dev     *m2m_dev;
+};
+
 struct vicodec_dev {
 	struct v4l2_device	v4l2_dev;
-	struct video_device	enc_vfd;
-	struct video_device	dec_vfd;
+	struct vicodec_dev_instance enc_instance;
+	struct vicodec_dev_instance dec_instance;
 #ifdef CONFIG_MEDIA_CONTROLLER
 	struct media_device	mdev;
 #endif
 
-	struct mutex		enc_mutex;
-	struct mutex		dec_mutex;
-	spinlock_t		enc_lock;
-	spinlock_t		dec_lock;
-
-	struct v4l2_m2m_dev	*enc_dev;
-	struct v4l2_m2m_dev	*dec_dev;
 };
 
 struct vicodec_ctx {
@@ -309,9 +309,9 @@ static void device_run(void *priv)
 	spin_unlock(ctx->lock);
 
 	if (ctx->is_enc)
-		v4l2_m2m_job_finish(dev->enc_dev, ctx->fh.m2m_ctx);
+		v4l2_m2m_job_finish(dev->enc_instance.m2m_dev, ctx->fh.m2m_ctx);
 	else
-		v4l2_m2m_job_finish(dev->dec_dev, ctx->fh.m2m_ctx);
+		v4l2_m2m_job_finish(dev->dec_instance.m2m_dev, ctx->fh.m2m_ctx);
 }
 
 static void job_remove_src_buf(struct vicodec_ctx *ctx, u32 state)
@@ -1440,9 +1440,8 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->ops = &vicodec_qops;
 	src_vq->mem_ops = &vb2_vmalloc_memops;
 	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
-	src_vq->lock = ctx->is_enc ? &ctx->dev->enc_mutex :
-		&ctx->dev->dec_mutex;
-
+	src_vq->lock = ctx->is_enc ? &ctx->dev->enc_instance.mutex :
+		&ctx->dev->dec_instance.mutex;
 	ret = vb2_queue_init(src_vq);
 	if (ret)
 		return ret;
@@ -1530,7 +1529,7 @@ static int vicodec_open(struct file *file)
 		goto open_unlock;
 	}
 
-	if (vfd == &dev->enc_vfd)
+	if (vfd == &dev->enc_instance.vfd)
 		ctx->is_enc = true;
 
 	v4l2_fh_init(&ctx->fh, video_devdata(file));
@@ -1578,13 +1577,13 @@ static int vicodec_open(struct file *file)
 	ctx->state.colorspace = V4L2_COLORSPACE_REC709;
 
 	if (ctx->is_enc) {
-		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->enc_dev, ctx,
-						    &queue_init);
-		ctx->lock = &dev->enc_lock;
+		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->enc_instance.m2m_dev,
+						    ctx, &queue_init);
+		ctx->lock = &dev->enc_instance.lock;
 	} else {
-		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->dec_dev, ctx,
-						    &queue_init);
-		ctx->lock = &dev->dec_lock;
+		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->dec_instance.m2m_dev,
+						    ctx, &queue_init);
+		ctx->lock = &dev->dec_instance.lock;
 	}
 
 	if (IS_ERR(ctx->fh.m2m_ctx)) {
@@ -1642,18 +1641,50 @@ static const struct v4l2_m2m_ops m2m_ops = {
 	.job_ready	= job_ready,
 };
 
+static int register_instance(struct vicodec_dev *dev,
+			     struct vicodec_dev_instance *dev_instance,
+			     const char *name, bool is_enc)
+{
+	struct video_device *vfd;
+	int ret;
+
+	dev_instance->vfd = vicodec_videodev;
+	vfd = &dev_instance->vfd;
+	vfd->lock = &dev_instance->mutex;
+	vfd->v4l2_dev = &dev->v4l2_dev;
+	strscpy(vfd->name, name, sizeof(vfd->name));
+	vfd->device_caps = V4L2_CAP_STREAMING |
+		(multiplanar ? V4L2_CAP_VIDEO_M2M_MPLANE : V4L2_CAP_VIDEO_M2M);
+	if (is_enc) {
+		v4l2_disable_ioctl(vfd, VIDIOC_DECODER_CMD);
+		v4l2_disable_ioctl(vfd, VIDIOC_TRY_DECODER_CMD);
+	} else {
+		v4l2_disable_ioctl(vfd, VIDIOC_ENCODER_CMD);
+		v4l2_disable_ioctl(vfd, VIDIOC_TRY_ENCODER_CMD);
+	}
+	video_set_drvdata(vfd, dev);
+
+	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
+	if (ret) {
+		v4l2_err(&dev->v4l2_dev, "Failed to register video device '%s'\n", name);
+		return ret;
+	}
+	v4l2_info(&dev->v4l2_dev, "Device '%s' registered as /dev/video%d\n",
+		  name, vfd->num);
+	return ret;
+}
+
 static int vicodec_probe(struct platform_device *pdev)
 {
 	struct vicodec_dev *dev;
-	struct video_device *vfd;
 	int ret;
 
 	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
 
-	spin_lock_init(&dev->enc_lock);
-	spin_lock_init(&dev->dec_lock);
+	spin_lock_init(&dev->enc_instance.lock);
+	spin_lock_init(&dev->dec_instance.lock);
 
 	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
 	if (ret)
@@ -1666,75 +1697,47 @@ static int vicodec_probe(struct platform_device *pdev)
 	dev->v4l2_dev.mdev = &dev->mdev;
 #endif
 
-	mutex_init(&dev->enc_mutex);
-	mutex_init(&dev->dec_mutex);
+	mutex_init(&dev->enc_instance.mutex);
+	mutex_init(&dev->dec_instance.mutex);
 
 	platform_set_drvdata(pdev, dev);
 
-	dev->enc_dev = v4l2_m2m_init(&m2m_ops);
-	if (IS_ERR(dev->enc_dev)) {
-		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec device\n");
-		ret = PTR_ERR(dev->enc_dev);
+	dev->enc_instance.m2m_dev = v4l2_m2m_init(&m2m_ops);
+	if (IS_ERR(dev->enc_instance.m2m_dev)) {
+		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec enc device\n");
+		ret = PTR_ERR(dev->enc_instance.m2m_dev);
 		goto unreg_dev;
 	}
 
-	dev->dec_dev = v4l2_m2m_init(&m2m_ops);
-	if (IS_ERR(dev->dec_dev)) {
-		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec device\n");
-		ret = PTR_ERR(dev->dec_dev);
+	dev->dec_instance.m2m_dev = v4l2_m2m_init(&m2m_ops);
+	if (IS_ERR(dev->dec_instance.m2m_dev)) {
+		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec dec device\n");
+		ret = PTR_ERR(dev->dec_instance.m2m_dev);
 		goto err_enc_m2m;
 	}
 
-	dev->enc_vfd = vicodec_videodev;
-	vfd = &dev->enc_vfd;
-	vfd->lock = &dev->enc_mutex;
-	vfd->v4l2_dev = &dev->v4l2_dev;
-	strscpy(vfd->name, "vicodec-enc", sizeof(vfd->name));
-	vfd->device_caps = V4L2_CAP_STREAMING |
-		(multiplanar ? V4L2_CAP_VIDEO_M2M_MPLANE : V4L2_CAP_VIDEO_M2M);
-	v4l2_disable_ioctl(vfd, VIDIOC_DECODER_CMD);
-	v4l2_disable_ioctl(vfd, VIDIOC_TRY_DECODER_CMD);
-	video_set_drvdata(vfd, dev);
-
-	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
-	if (ret) {
-		v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
+	if (register_instance(dev, &dev->enc_instance,
+			      "videdev-enc", true))
 		goto err_dec_m2m;
-	}
-	v4l2_info(&dev->v4l2_dev,
-			"Device registered as /dev/video%d\n", vfd->num);
 
-	dev->dec_vfd = vicodec_videodev;
-	vfd = &dev->dec_vfd;
-	vfd->lock = &dev->dec_mutex;
-	vfd->v4l2_dev = &dev->v4l2_dev;
-	vfd->device_caps = V4L2_CAP_STREAMING |
-		(multiplanar ? V4L2_CAP_VIDEO_M2M_MPLANE : V4L2_CAP_VIDEO_M2M);
-	strscpy(vfd->name, "vicodec-dec", sizeof(vfd->name));
-	v4l2_disable_ioctl(vfd, VIDIOC_ENCODER_CMD);
-	v4l2_disable_ioctl(vfd, VIDIOC_TRY_ENCODER_CMD);
-	video_set_drvdata(vfd, dev);
-
-	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
-	if (ret) {
-		v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
+	if (register_instance(dev, &dev->dec_instance,
+			      "videdev-statefull-dec", false))
 		goto unreg_enc;
-	}
-	v4l2_info(&dev->v4l2_dev,
-			"Device registered as /dev/video%d\n", vfd->num);
 
 #ifdef CONFIG_MEDIA_CONTROLLER
-	ret = v4l2_m2m_register_media_controller(dev->enc_dev,
-			&dev->enc_vfd, MEDIA_ENT_F_PROC_VIDEO_ENCODER);
+	ret = v4l2_m2m_register_media_controller(dev->enc_instance.m2m_dev,
+						 &dev->enc_instance.vfd,
+						 MEDIA_ENT_F_PROC_VIDEO_ENCODER);
 	if (ret) {
-		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller\n");
+		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for enc\n");
 		goto unreg_m2m;
 	}
 
-	ret = v4l2_m2m_register_media_controller(dev->dec_dev,
-			&dev->dec_vfd, MEDIA_ENT_F_PROC_VIDEO_DECODER);
+	ret = v4l2_m2m_register_media_controller(dev->dec_instance.m2m_dev,
+						 &dev->dec_instance.vfd,
+						 MEDIA_ENT_F_PROC_VIDEO_DECODER);
 	if (ret) {
-		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller\n");
+		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for dec\n");
 		goto unreg_m2m_enc_mc;
 	}
 
@@ -1748,18 +1751,18 @@ static int vicodec_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_MEDIA_CONTROLLER
 unreg_m2m_dec_mc:
-	v4l2_m2m_unregister_media_controller(dev->dec_dev);
+	v4l2_m2m_unregister_media_controller(dev->dec_instance.m2m_dev);
 unreg_m2m_enc_mc:
-	v4l2_m2m_unregister_media_controller(dev->enc_dev);
+	v4l2_m2m_unregister_media_controller(dev->enc_instance.m2m_dev);
 unreg_m2m:
-	video_unregister_device(&dev->dec_vfd);
+	video_unregister_device(&dev->dec_instance.vfd);
 #endif
 unreg_enc:
-	video_unregister_device(&dev->enc_vfd);
+	video_unregister_device(&dev->enc_instance.vfd);
 err_dec_m2m:
-	v4l2_m2m_release(dev->dec_dev);
+	v4l2_m2m_release(dev->dec_instance.m2m_dev);
 err_enc_m2m:
-	v4l2_m2m_release(dev->enc_dev);
+	v4l2_m2m_release(dev->enc_instance.m2m_dev);
 unreg_dev:
 	v4l2_device_unregister(&dev->v4l2_dev);
 
@@ -1774,15 +1777,15 @@ static int vicodec_remove(struct platform_device *pdev)
 
 #ifdef CONFIG_MEDIA_CONTROLLER
 	media_device_unregister(&dev->mdev);
-	v4l2_m2m_unregister_media_controller(dev->enc_dev);
-	v4l2_m2m_unregister_media_controller(dev->dec_dev);
+	v4l2_m2m_unregister_media_controller(dev->enc_instance.m2m_dev);
+	v4l2_m2m_unregister_media_controller(dev->dec_instance.m2m_dev);
 	media_device_cleanup(&dev->mdev);
 #endif
 
-	v4l2_m2m_release(dev->enc_dev);
-	v4l2_m2m_release(dev->dec_dev);
-	video_unregister_device(&dev->enc_vfd);
-	video_unregister_device(&dev->dec_vfd);
+	v4l2_m2m_release(dev->enc_instance.m2m_dev);
+	v4l2_m2m_release(dev->dec_instance.m2m_dev);
+	video_unregister_device(&dev->enc_instance.vfd);
+	video_unregister_device(&dev->dec_instance.vfd);
 	v4l2_device_unregister(&dev->v4l2_dev);
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] media: vicodec: Introducing stateless fwht defs and structs
  2019-01-26 13:47 [PATCH 0/3] Prepare for statless decoder for fwht Dafna Hirschfeld
  2019-01-26 13:47 ` [PATCH 1/3] media: vicodec: add struct for encoder/decoder instance Dafna Hirschfeld
@ 2019-01-26 13:47 ` Dafna Hirschfeld
  2019-01-28  9:28   ` Hans Verkuil
  2019-01-26 13:47 ` [PATCH 3/3] media: vicodec: Register another node for stateless decoder Dafna Hirschfeld
  2 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2019-01-26 13:47 UTC (permalink / raw)
  To: linux-media; +Cc: hverkuil, helen.koike, Dafna Hirschfeld

Add structs and definitions needed to implement stateless
decoder for fwht.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 12 ++++++++++++
 drivers/media/v4l2-core/v4l2-ctrls.c          |  6 ++++++
 include/uapi/linux/v4l2-controls.h            | 10 ++++++++++
 include/uapi/linux/videodev2.h                |  1 +
 4 files changed, 29 insertions(+)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 370517707324..25831d992681 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -64,6 +64,10 @@ static const struct v4l2_fwht_pixfmt_info pixfmt_fwht = {
 	V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1, 0, 1
 };
 
+static const struct v4l2_fwht_pixfmt_info pixfmt_stateless_fwht = {
+	V4L2_PIX_FMT_FWHT_STATELESS, 0, 3, 1, 1, 1, 1, 1, 0, 1
+};
+
 static void vicodec_dev_release(struct device *dev)
 {
 }
@@ -1463,6 +1467,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 #define VICODEC_CID_CUSTOM_BASE		(V4L2_CID_MPEG_BASE | 0xf000)
 #define VICODEC_CID_I_FRAME_QP		(VICODEC_CID_CUSTOM_BASE + 0)
 #define VICODEC_CID_P_FRAME_QP		(VICODEC_CID_CUSTOM_BASE + 1)
+#define VICODEC_CID_STATELESS_FWHT	(VICODEC_CID_CUSTOM_BASE + 2)
 
 static int vicodec_s_ctrl(struct v4l2_ctrl *ctrl)
 {
@@ -1509,6 +1514,13 @@ static const struct v4l2_ctrl_config vicodec_ctrl_p_frame = {
 	.step = 1,
 };
 
+static const struct v4l2_ctrl_config vicodec_ctrl_stateless_state = {
+	.id		= VICODEC_CID_STATELESS_FWHT,
+	.elem_size	= sizeof(struct v4l2_ctrl_fwht_params),
+	.name		= "FWHT-Stateless State Params",
+	.type		= V4L2_CTRL_TYPE_FWHT_PARAMS,
+};
+
 /*
  * File operations
  */
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
index 99308dac2daa..b05e51312430 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls.c
@@ -1669,6 +1669,9 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
 	case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
 		return 0;
 
+	case V4L2_CTRL_TYPE_FWHT_PARAMS:
+		return 0;
+
 	default:
 		return -EINVAL;
 	}
@@ -2249,6 +2252,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
 	case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
 		elem_size = sizeof(struct v4l2_ctrl_mpeg2_quantization);
 		break;
+	case V4L2_CTRL_TYPE_FWHT_PARAMS:
+		elem_size = sizeof(struct v4l2_ctrl_fwht_params);
+		break;
 	default:
 		if (type < V4L2_CTRL_COMPOUND_TYPES)
 			elem_size = sizeof(s32);
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index 06479f2fb3ae..2d6e91cb615a 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -52,6 +52,7 @@
 
 #include <linux/types.h>
 
+#define V4L2_CTRL_TYPE_FWHT_PARAMS 0x0105
 /* Control classes */
 #define V4L2_CTRL_CLASS_USER		0x00980000	/* Old-style 'user' controls */
 #define V4L2_CTRL_CLASS_MPEG		0x00990000	/* MPEG-compression controls */
@@ -1096,4 +1097,13 @@ enum v4l2_detect_md_mode {
 #define V4L2_CID_DETECT_MD_THRESHOLD_GRID	(V4L2_CID_DETECT_CLASS_BASE + 3)
 #define V4L2_CID_DETECT_MD_REGION_GRID		(V4L2_CID_DETECT_CLASS_BASE + 4)
 
+struct v4l2_ctrl_fwht_params {
+	__u32 flags;
+	__u32 colorspace;
+	__u32 xfer_func;
+	__u32 ycbcr_enc;
+	__u32 quantization;
+	__u64 backward_ref_ts;
+};
+
 #endif
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index 9a920f071ff9..37ac240eba01 100644
--- a/include/uapi/linux/videodev2.h
+++ b/include/uapi/linux/videodev2.h
@@ -665,6 +665,7 @@ struct v4l2_pix_format {
 #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
 #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
 #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
+#define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
 
 /*  Vendor-specific formats   */
 #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] media: vicodec: Register another node for stateless decoder
  2019-01-26 13:47 [PATCH 0/3] Prepare for statless decoder for fwht Dafna Hirschfeld
  2019-01-26 13:47 ` [PATCH 1/3] media: vicodec: add struct for encoder/decoder instance Dafna Hirschfeld
  2019-01-26 13:47 ` [PATCH 2/3] media: vicodec: Introducing stateless fwht defs and structs Dafna Hirschfeld
@ 2019-01-26 13:47 ` Dafna Hirschfeld
  2019-01-28  9:32   ` Hans Verkuil
  2 siblings, 1 reply; 7+ messages in thread
From: Dafna Hirschfeld @ 2019-01-26 13:47 UTC (permalink / raw)
  To: linux-media; +Cc: hverkuil, helen.koike, Dafna Hirschfeld

Add stateless decoder instance field to the dev struct and
register another node for the statelsess decoder.
The stateless API for the node will be implemented in further patches.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/vicodec-core.c | 56 +++++++++++++++++--
 1 file changed, 51 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 25831d992681..7c2ad7d5f356 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -104,6 +104,7 @@ struct vicodec_dev {
 	struct v4l2_device	v4l2_dev;
 	struct vicodec_dev_instance enc_instance;
 	struct vicodec_dev_instance dec_instance;
+	struct vicodec_dev_instance stateless_dec_instance;
 #ifdef CONFIG_MEDIA_CONTROLLER
 	struct media_device	mdev;
 #endif
@@ -114,6 +115,7 @@ struct vicodec_ctx {
 	struct v4l2_fh		fh;
 	struct vicodec_dev	*dev;
 	bool			is_enc;
+	bool			is_stateless_dec;
 	spinlock_t		*lock;
 
 	struct v4l2_ctrl_handler hdl;
@@ -314,6 +316,9 @@ static void device_run(void *priv)
 
 	if (ctx->is_enc)
 		v4l2_m2m_job_finish(dev->enc_instance.m2m_dev, ctx->fh.m2m_ctx);
+	else if (ctx->is_stateless_dec)
+		v4l2_m2m_job_finish(dev->stateless_dec_instance.m2m_dev,
+				    ctx->fh.m2m_ctx);
 	else
 		v4l2_m2m_job_finish(dev->dec_instance.m2m_dev, ctx->fh.m2m_ctx);
 }
@@ -1444,8 +1449,13 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
 	src_vq->ops = &vicodec_qops;
 	src_vq->mem_ops = &vb2_vmalloc_memops;
 	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
-	src_vq->lock = ctx->is_enc ? &ctx->dev->enc_instance.mutex :
-		&ctx->dev->dec_instance.mutex;
+	if (ctx->is_enc)
+		src_vq->lock = &ctx->dev->enc_instance.mutex;
+	else if (ctx->is_stateless_dec)
+		src_vq->lock = &ctx->dev->stateless_dec_instance.mutex;
+	else
+		src_vq->lock = &ctx->dev->dec_instance.mutex;
+	src_vq->supports_requests = ctx->is_stateless_dec ? true : false;
 	ret = vb2_queue_init(src_vq);
 	if (ret)
 		return ret;
@@ -1543,6 +1553,8 @@ static int vicodec_open(struct file *file)
 
 	if (vfd == &dev->enc_instance.vfd)
 		ctx->is_enc = true;
+	else if (vfd == &dev->stateless_dec_instance.vfd)
+		ctx->is_stateless_dec = true;
 
 	v4l2_fh_init(&ctx->fh, video_devdata(file));
 	file->private_data = &ctx->fh;
@@ -1553,6 +1565,7 @@ static int vicodec_open(struct file *file)
 			  1, 16, 1, 10);
 	v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_i_frame, NULL);
 	v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_p_frame, NULL);
+	v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_stateless_state, NULL);
 	if (hdl->error) {
 		rc = hdl->error;
 		v4l2_ctrl_handler_free(hdl);
@@ -1592,6 +1605,10 @@ static int vicodec_open(struct file *file)
 		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->enc_instance.m2m_dev,
 						    ctx, &queue_init);
 		ctx->lock = &dev->enc_instance.lock;
+	} else if (ctx->is_stateless_dec) {
+		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateless_dec_instance.m2m_dev,
+						    ctx, &queue_init);
+		ctx->lock = &dev->stateless_dec_instance.lock;
 	} else {
 		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->dec_instance.m2m_dev,
 						    ctx, &queue_init);
@@ -1697,6 +1714,7 @@ static int vicodec_probe(struct platform_device *pdev)
 
 	spin_lock_init(&dev->enc_instance.lock);
 	spin_lock_init(&dev->dec_instance.lock);
+	spin_lock_init(&dev->stateless_dec_instance.lock);
 
 	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
 	if (ret)
@@ -1711,6 +1729,7 @@ static int vicodec_probe(struct platform_device *pdev)
 
 	mutex_init(&dev->enc_instance.mutex);
 	mutex_init(&dev->dec_instance.mutex);
+	mutex_init(&dev->stateless_dec_instance.mutex);
 
 	platform_set_drvdata(pdev, dev);
 
@@ -1728,14 +1747,25 @@ static int vicodec_probe(struct platform_device *pdev)
 		goto err_enc_m2m;
 	}
 
+	dev->stateless_dec_instance.m2m_dev = v4l2_m2m_init(&m2m_ops);
+	if (IS_ERR(dev->stateless_dec_instance.m2m_dev)) {
+		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec stateless dec device\n");
+		ret = PTR_ERR(dev->stateless_dec_instance.m2m_dev);
+		goto err_dec_m2m;
+	}
+
 	if (register_instance(dev, &dev->enc_instance,
 			      "videdev-enc", true))
-		goto err_dec_m2m;
+		goto err_sdec_m2m;
 
 	if (register_instance(dev, &dev->dec_instance,
 			      "videdev-statefull-dec", false))
 		goto unreg_enc;
 
+	if (register_instance(dev, &dev->stateless_dec_instance,
+			      "videdev-stateless-dec", false))
+		goto unreg_dec;
+
 #ifdef CONFIG_MEDIA_CONTROLLER
 	ret = v4l2_m2m_register_media_controller(dev->enc_instance.m2m_dev,
 						 &dev->enc_instance.vfd,
@@ -1753,24 +1783,38 @@ static int vicodec_probe(struct platform_device *pdev)
 		goto unreg_m2m_enc_mc;
 	}
 
+	ret = v4l2_m2m_register_media_controller(dev->stateless_dec_instance.m2m_dev,
+						 &dev->stateless_dec_instance.vfd,
+						 MEDIA_ENT_F_PROC_VIDEO_DECODER);
+	if (ret) {
+		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for stateless dec\n");
+		goto unreg_m2m_dec_mc;
+	}
+
 	ret = media_device_register(&dev->mdev);
 	if (ret) {
 		v4l2_err(&dev->v4l2_dev, "Failed to register mem2mem media device\n");
-		goto unreg_m2m_dec_mc;
+		goto unreg_m2m_sdec_mc;
 	}
 #endif
 	return 0;
 
 #ifdef CONFIG_MEDIA_CONTROLLER
+unreg_m2m_sdec_mc:
+	v4l2_m2m_unregister_media_controller(dev->stateless_dec_instance.m2m_dev);
 unreg_m2m_dec_mc:
 	v4l2_m2m_unregister_media_controller(dev->dec_instance.m2m_dev);
 unreg_m2m_enc_mc:
 	v4l2_m2m_unregister_media_controller(dev->enc_instance.m2m_dev);
 unreg_m2m:
-	video_unregister_device(&dev->dec_instance.vfd);
+	video_unregister_device(&dev->stateless_dec_instance.vfd);
 #endif
+unreg_dec:
+	video_unregister_device(&dev->dec_instance.vfd);
 unreg_enc:
 	video_unregister_device(&dev->enc_instance.vfd);
+err_sdec_m2m:
+	v4l2_m2m_release(dev->stateless_dec_instance.m2m_dev);
 err_dec_m2m:
 	v4l2_m2m_release(dev->dec_instance.m2m_dev);
 err_enc_m2m:
@@ -1791,6 +1835,7 @@ static int vicodec_remove(struct platform_device *pdev)
 	media_device_unregister(&dev->mdev);
 	v4l2_m2m_unregister_media_controller(dev->enc_instance.m2m_dev);
 	v4l2_m2m_unregister_media_controller(dev->dec_instance.m2m_dev);
+	v4l2_m2m_unregister_media_controller(dev->stateless_dec_instance.m2m_dev);
 	media_device_cleanup(&dev->mdev);
 #endif
 
@@ -1798,6 +1843,7 @@ static int vicodec_remove(struct platform_device *pdev)
 	v4l2_m2m_release(dev->dec_instance.m2m_dev);
 	video_unregister_device(&dev->enc_instance.vfd);
 	video_unregister_device(&dev->dec_instance.vfd);
+	video_unregister_device(&dev->stateless_dec_instance.vfd);
 	v4l2_device_unregister(&dev->v4l2_dev);
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/3] media: vicodec: add struct for encoder/decoder instance
  2019-01-26 13:47 ` [PATCH 1/3] media: vicodec: add struct for encoder/decoder instance Dafna Hirschfeld
@ 2019-01-28  9:22   ` Hans Verkuil
  0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2019-01-28  9:22 UTC (permalink / raw)
  To: Dafna Hirschfeld, linux-media; +Cc: helen.koike

On 1/26/19 2:47 PM, Dafna Hirschfeld wrote:
> Add struct 'vicodec_dev_instance' for the fields in vicodec_dev
> that have have both decoder and encoder versions.
> 
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---
>  drivers/media/platform/vicodec/vicodec-core.c | 175 +++++++++---------
>  1 file changed, 89 insertions(+), 86 deletions(-)
> 
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
> index 953b9c4816a5..370517707324 100644
> --- a/drivers/media/platform/vicodec/vicodec-core.c
> +++ b/drivers/media/platform/vicodec/vicodec-core.c
> @@ -89,21 +89,21 @@ enum {
>  	V4L2_M2M_DST = 1,
>  };
>  
> +struct vicodec_dev_instance {
> +	struct video_device     vfd;
> +	struct mutex            mutex;
> +	spinlock_t              lock;
> +	struct v4l2_m2m_dev     *m2m_dev;
> +};
> +
>  struct vicodec_dev {
>  	struct v4l2_device	v4l2_dev;
> -	struct video_device	enc_vfd;
> -	struct video_device	dec_vfd;
> +	struct vicodec_dev_instance enc_instance;
> +	struct vicodec_dev_instance dec_instance;

I'd call this stateful_enc/dec.

And in patch 3 stateless_dec.

It's more descriptive.

Regards,

	Hans

>  #ifdef CONFIG_MEDIA_CONTROLLER
>  	struct media_device	mdev;
>  #endif
>  
> -	struct mutex		enc_mutex;
> -	struct mutex		dec_mutex;
> -	spinlock_t		enc_lock;
> -	spinlock_t		dec_lock;
> -
> -	struct v4l2_m2m_dev	*enc_dev;
> -	struct v4l2_m2m_dev	*dec_dev;
>  };
>  
>  struct vicodec_ctx {
> @@ -309,9 +309,9 @@ static void device_run(void *priv)
>  	spin_unlock(ctx->lock);
>  
>  	if (ctx->is_enc)
> -		v4l2_m2m_job_finish(dev->enc_dev, ctx->fh.m2m_ctx);
> +		v4l2_m2m_job_finish(dev->enc_instance.m2m_dev, ctx->fh.m2m_ctx);
>  	else
> -		v4l2_m2m_job_finish(dev->dec_dev, ctx->fh.m2m_ctx);
> +		v4l2_m2m_job_finish(dev->dec_instance.m2m_dev, ctx->fh.m2m_ctx);
>  }
>  
>  static void job_remove_src_buf(struct vicodec_ctx *ctx, u32 state)
> @@ -1440,9 +1440,8 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	src_vq->ops = &vicodec_qops;
>  	src_vq->mem_ops = &vb2_vmalloc_memops;
>  	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> -	src_vq->lock = ctx->is_enc ? &ctx->dev->enc_mutex :
> -		&ctx->dev->dec_mutex;
> -
> +	src_vq->lock = ctx->is_enc ? &ctx->dev->enc_instance.mutex :
> +		&ctx->dev->dec_instance.mutex;
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
>  		return ret;
> @@ -1530,7 +1529,7 @@ static int vicodec_open(struct file *file)
>  		goto open_unlock;
>  	}
>  
> -	if (vfd == &dev->enc_vfd)
> +	if (vfd == &dev->enc_instance.vfd)
>  		ctx->is_enc = true;
>  
>  	v4l2_fh_init(&ctx->fh, video_devdata(file));
> @@ -1578,13 +1577,13 @@ static int vicodec_open(struct file *file)
>  	ctx->state.colorspace = V4L2_COLORSPACE_REC709;
>  
>  	if (ctx->is_enc) {
> -		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->enc_dev, ctx,
> -						    &queue_init);
> -		ctx->lock = &dev->enc_lock;
> +		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->enc_instance.m2m_dev,
> +						    ctx, &queue_init);
> +		ctx->lock = &dev->enc_instance.lock;
>  	} else {
> -		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->dec_dev, ctx,
> -						    &queue_init);
> -		ctx->lock = &dev->dec_lock;
> +		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->dec_instance.m2m_dev,
> +						    ctx, &queue_init);
> +		ctx->lock = &dev->dec_instance.lock;
>  	}
>  
>  	if (IS_ERR(ctx->fh.m2m_ctx)) {
> @@ -1642,18 +1641,50 @@ static const struct v4l2_m2m_ops m2m_ops = {
>  	.job_ready	= job_ready,
>  };
>  
> +static int register_instance(struct vicodec_dev *dev,
> +			     struct vicodec_dev_instance *dev_instance,
> +			     const char *name, bool is_enc)
> +{
> +	struct video_device *vfd;
> +	int ret;
> +
> +	dev_instance->vfd = vicodec_videodev;
> +	vfd = &dev_instance->vfd;
> +	vfd->lock = &dev_instance->mutex;
> +	vfd->v4l2_dev = &dev->v4l2_dev;
> +	strscpy(vfd->name, name, sizeof(vfd->name));
> +	vfd->device_caps = V4L2_CAP_STREAMING |
> +		(multiplanar ? V4L2_CAP_VIDEO_M2M_MPLANE : V4L2_CAP_VIDEO_M2M);
> +	if (is_enc) {
> +		v4l2_disable_ioctl(vfd, VIDIOC_DECODER_CMD);
> +		v4l2_disable_ioctl(vfd, VIDIOC_TRY_DECODER_CMD);
> +	} else {
> +		v4l2_disable_ioctl(vfd, VIDIOC_ENCODER_CMD);
> +		v4l2_disable_ioctl(vfd, VIDIOC_TRY_ENCODER_CMD);
> +	}
> +	video_set_drvdata(vfd, dev);
> +
> +	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
> +	if (ret) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to register video device '%s'\n", name);
> +		return ret;
> +	}
> +	v4l2_info(&dev->v4l2_dev, "Device '%s' registered as /dev/video%d\n",
> +		  name, vfd->num);
> +	return ret;
> +}
> +
>  static int vicodec_probe(struct platform_device *pdev)
>  {
>  	struct vicodec_dev *dev;
> -	struct video_device *vfd;
>  	int ret;
>  
>  	dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
>  	if (!dev)
>  		return -ENOMEM;
>  
> -	spin_lock_init(&dev->enc_lock);
> -	spin_lock_init(&dev->dec_lock);
> +	spin_lock_init(&dev->enc_instance.lock);
> +	spin_lock_init(&dev->dec_instance.lock);
>  
>  	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
>  	if (ret)
> @@ -1666,75 +1697,47 @@ static int vicodec_probe(struct platform_device *pdev)
>  	dev->v4l2_dev.mdev = &dev->mdev;
>  #endif
>  
> -	mutex_init(&dev->enc_mutex);
> -	mutex_init(&dev->dec_mutex);
> +	mutex_init(&dev->enc_instance.mutex);
> +	mutex_init(&dev->dec_instance.mutex);
>  
>  	platform_set_drvdata(pdev, dev);
>  
> -	dev->enc_dev = v4l2_m2m_init(&m2m_ops);
> -	if (IS_ERR(dev->enc_dev)) {
> -		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec device\n");
> -		ret = PTR_ERR(dev->enc_dev);
> +	dev->enc_instance.m2m_dev = v4l2_m2m_init(&m2m_ops);
> +	if (IS_ERR(dev->enc_instance.m2m_dev)) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec enc device\n");
> +		ret = PTR_ERR(dev->enc_instance.m2m_dev);
>  		goto unreg_dev;
>  	}
>  
> -	dev->dec_dev = v4l2_m2m_init(&m2m_ops);
> -	if (IS_ERR(dev->dec_dev)) {
> -		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec device\n");
> -		ret = PTR_ERR(dev->dec_dev);
> +	dev->dec_instance.m2m_dev = v4l2_m2m_init(&m2m_ops);
> +	if (IS_ERR(dev->dec_instance.m2m_dev)) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec dec device\n");
> +		ret = PTR_ERR(dev->dec_instance.m2m_dev);
>  		goto err_enc_m2m;
>  	}
>  
> -	dev->enc_vfd = vicodec_videodev;
> -	vfd = &dev->enc_vfd;
> -	vfd->lock = &dev->enc_mutex;
> -	vfd->v4l2_dev = &dev->v4l2_dev;
> -	strscpy(vfd->name, "vicodec-enc", sizeof(vfd->name));
> -	vfd->device_caps = V4L2_CAP_STREAMING |
> -		(multiplanar ? V4L2_CAP_VIDEO_M2M_MPLANE : V4L2_CAP_VIDEO_M2M);
> -	v4l2_disable_ioctl(vfd, VIDIOC_DECODER_CMD);
> -	v4l2_disable_ioctl(vfd, VIDIOC_TRY_DECODER_CMD);
> -	video_set_drvdata(vfd, dev);
> -
> -	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
> -	if (ret) {
> -		v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
> +	if (register_instance(dev, &dev->enc_instance,
> +			      "videdev-enc", true))
>  		goto err_dec_m2m;
> -	}
> -	v4l2_info(&dev->v4l2_dev,
> -			"Device registered as /dev/video%d\n", vfd->num);
>  
> -	dev->dec_vfd = vicodec_videodev;
> -	vfd = &dev->dec_vfd;
> -	vfd->lock = &dev->dec_mutex;
> -	vfd->v4l2_dev = &dev->v4l2_dev;
> -	vfd->device_caps = V4L2_CAP_STREAMING |
> -		(multiplanar ? V4L2_CAP_VIDEO_M2M_MPLANE : V4L2_CAP_VIDEO_M2M);
> -	strscpy(vfd->name, "vicodec-dec", sizeof(vfd->name));
> -	v4l2_disable_ioctl(vfd, VIDIOC_ENCODER_CMD);
> -	v4l2_disable_ioctl(vfd, VIDIOC_TRY_ENCODER_CMD);
> -	video_set_drvdata(vfd, dev);
> -
> -	ret = video_register_device(vfd, VFL_TYPE_GRABBER, 0);
> -	if (ret) {
> -		v4l2_err(&dev->v4l2_dev, "Failed to register video device\n");
> +	if (register_instance(dev, &dev->dec_instance,
> +			      "videdev-statefull-dec", false))
>  		goto unreg_enc;
> -	}
> -	v4l2_info(&dev->v4l2_dev,
> -			"Device registered as /dev/video%d\n", vfd->num);
>  
>  #ifdef CONFIG_MEDIA_CONTROLLER
> -	ret = v4l2_m2m_register_media_controller(dev->enc_dev,
> -			&dev->enc_vfd, MEDIA_ENT_F_PROC_VIDEO_ENCODER);
> +	ret = v4l2_m2m_register_media_controller(dev->enc_instance.m2m_dev,
> +						 &dev->enc_instance.vfd,
> +						 MEDIA_ENT_F_PROC_VIDEO_ENCODER);
>  	if (ret) {
> -		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller\n");
> +		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for enc\n");
>  		goto unreg_m2m;
>  	}
>  
> -	ret = v4l2_m2m_register_media_controller(dev->dec_dev,
> -			&dev->dec_vfd, MEDIA_ENT_F_PROC_VIDEO_DECODER);
> +	ret = v4l2_m2m_register_media_controller(dev->dec_instance.m2m_dev,
> +						 &dev->dec_instance.vfd,
> +						 MEDIA_ENT_F_PROC_VIDEO_DECODER);
>  	if (ret) {
> -		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller\n");
> +		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for dec\n");
>  		goto unreg_m2m_enc_mc;
>  	}
>  
> @@ -1748,18 +1751,18 @@ static int vicodec_probe(struct platform_device *pdev)
>  
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  unreg_m2m_dec_mc:
> -	v4l2_m2m_unregister_media_controller(dev->dec_dev);
> +	v4l2_m2m_unregister_media_controller(dev->dec_instance.m2m_dev);
>  unreg_m2m_enc_mc:
> -	v4l2_m2m_unregister_media_controller(dev->enc_dev);
> +	v4l2_m2m_unregister_media_controller(dev->enc_instance.m2m_dev);
>  unreg_m2m:
> -	video_unregister_device(&dev->dec_vfd);
> +	video_unregister_device(&dev->dec_instance.vfd);
>  #endif
>  unreg_enc:
> -	video_unregister_device(&dev->enc_vfd);
> +	video_unregister_device(&dev->enc_instance.vfd);
>  err_dec_m2m:
> -	v4l2_m2m_release(dev->dec_dev);
> +	v4l2_m2m_release(dev->dec_instance.m2m_dev);
>  err_enc_m2m:
> -	v4l2_m2m_release(dev->enc_dev);
> +	v4l2_m2m_release(dev->enc_instance.m2m_dev);
>  unreg_dev:
>  	v4l2_device_unregister(&dev->v4l2_dev);
>  
> @@ -1774,15 +1777,15 @@ static int vicodec_remove(struct platform_device *pdev)
>  
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  	media_device_unregister(&dev->mdev);
> -	v4l2_m2m_unregister_media_controller(dev->enc_dev);
> -	v4l2_m2m_unregister_media_controller(dev->dec_dev);
> +	v4l2_m2m_unregister_media_controller(dev->enc_instance.m2m_dev);
> +	v4l2_m2m_unregister_media_controller(dev->dec_instance.m2m_dev);
>  	media_device_cleanup(&dev->mdev);
>  #endif
>  
> -	v4l2_m2m_release(dev->enc_dev);
> -	v4l2_m2m_release(dev->dec_dev);
> -	video_unregister_device(&dev->enc_vfd);
> -	video_unregister_device(&dev->dec_vfd);
> +	v4l2_m2m_release(dev->enc_instance.m2m_dev);
> +	v4l2_m2m_release(dev->dec_instance.m2m_dev);
> +	video_unregister_device(&dev->enc_instance.vfd);
> +	video_unregister_device(&dev->dec_instance.vfd);
>  	v4l2_device_unregister(&dev->v4l2_dev);
>  
>  	return 0;
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] media: vicodec: Introducing stateless fwht defs and structs
  2019-01-26 13:47 ` [PATCH 2/3] media: vicodec: Introducing stateless fwht defs and structs Dafna Hirschfeld
@ 2019-01-28  9:28   ` Hans Verkuil
  0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2019-01-28  9:28 UTC (permalink / raw)
  To: Dafna Hirschfeld, linux-media; +Cc: helen.koike

On 1/26/19 2:47 PM, Dafna Hirschfeld wrote:
> Add structs and definitions needed to implement stateless
> decoder for fwht.
> 
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---
>  drivers/media/platform/vicodec/vicodec-core.c | 12 ++++++++++++
>  drivers/media/v4l2-core/v4l2-ctrls.c          |  6 ++++++
>  include/uapi/linux/v4l2-controls.h            | 10 ++++++++++
>  include/uapi/linux/videodev2.h                |  1 +
>  4 files changed, 29 insertions(+)
> 
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
> index 370517707324..25831d992681 100644
> --- a/drivers/media/platform/vicodec/vicodec-core.c
> +++ b/drivers/media/platform/vicodec/vicodec-core.c
> @@ -64,6 +64,10 @@ static const struct v4l2_fwht_pixfmt_info pixfmt_fwht = {
>  	V4L2_PIX_FMT_FWHT, 0, 3, 1, 1, 1, 1, 1, 0, 1
>  };
>  
> +static const struct v4l2_fwht_pixfmt_info pixfmt_stateless_fwht = {
> +	V4L2_PIX_FMT_FWHT_STATELESS, 0, 3, 1, 1, 1, 1, 1, 0, 1
> +};
> +
>  static void vicodec_dev_release(struct device *dev)
>  {
>  }
> @@ -1463,6 +1467,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  #define VICODEC_CID_CUSTOM_BASE		(V4L2_CID_MPEG_BASE | 0xf000)
>  #define VICODEC_CID_I_FRAME_QP		(VICODEC_CID_CUSTOM_BASE + 0)
>  #define VICODEC_CID_P_FRAME_QP		(VICODEC_CID_CUSTOM_BASE + 1)
> +#define VICODEC_CID_STATELESS_FWHT	(VICODEC_CID_CUSTOM_BASE + 2)
>  
>  static int vicodec_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
> @@ -1509,6 +1514,13 @@ static const struct v4l2_ctrl_config vicodec_ctrl_p_frame = {
>  	.step = 1,
>  };
>  
> +static const struct v4l2_ctrl_config vicodec_ctrl_stateless_state = {
> +	.id		= VICODEC_CID_STATELESS_FWHT,
> +	.elem_size	= sizeof(struct v4l2_ctrl_fwht_params),
> +	.name		= "FWHT-Stateless State Params",
> +	.type		= V4L2_CTRL_TYPE_FWHT_PARAMS,
> +};
> +
>  /*
>   * File operations
>   */
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index 99308dac2daa..b05e51312430 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1669,6 +1669,9 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx,
>  	case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
>  		return 0;
>  
> +	case V4L2_CTRL_TYPE_FWHT_PARAMS:
> +		return 0;
> +
>  	default:
>  		return -EINVAL;
>  	}
> @@ -2249,6 +2252,9 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct v4l2_ctrl_handler *hdl,
>  	case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION:
>  		elem_size = sizeof(struct v4l2_ctrl_mpeg2_quantization);
>  		break;
> +	case V4L2_CTRL_TYPE_FWHT_PARAMS:
> +		elem_size = sizeof(struct v4l2_ctrl_fwht_params);
> +		break;
>  	default:
>  		if (type < V4L2_CTRL_COMPOUND_TYPES)
>  			elem_size = sizeof(s32);
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 06479f2fb3ae..2d6e91cb615a 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -52,6 +52,7 @@
>  
>  #include <linux/types.h>
>  
> +#define V4L2_CTRL_TYPE_FWHT_PARAMS 0x0105
>  /* Control classes */
>  #define V4L2_CTRL_CLASS_USER		0x00980000	/* Old-style 'user' controls */
>  #define V4L2_CTRL_CLASS_MPEG		0x00990000	/* MPEG-compression controls */
> @@ -1096,4 +1097,13 @@ enum v4l2_detect_md_mode {
>  #define V4L2_CID_DETECT_MD_THRESHOLD_GRID	(V4L2_CID_DETECT_CLASS_BASE + 3)
>  #define V4L2_CID_DETECT_MD_REGION_GRID		(V4L2_CID_DETECT_CLASS_BASE + 4)
>  
> +struct v4l2_ctrl_fwht_params {
> +	__u32 flags;
> +	__u32 colorspace;
> +	__u32 xfer_func;
> +	__u32 ycbcr_enc;
> +	__u32 quantization;
> +	__u64 backward_ref_ts;

It is best if backward_ref_ts is moved to the beginning of the struct.

The problem here is that 64 bit fields are aligned to an 8 byte
boundary and so a 4 byte hole is inserted before backward_ref_ts. Also,
the i686 architecture is different from most other 32 bit architectures
in that it aligns 64 bit values to a 4 byte boundary. As a result using
this structure will require a 32 bit to 64 bit conversion since the
layout is different.

To avoid all this mess it is best to always place 64 bit values at a
proper 8 byte alignment.

> +};
> +
>  #endif
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 9a920f071ff9..37ac240eba01 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -665,6 +665,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_VP9      v4l2_fourcc('V', 'P', '9', '0') /* VP9 */
>  #define V4L2_PIX_FMT_HEVC     v4l2_fourcc('H', 'E', 'V', 'C') /* HEVC aka H.265 */
>  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
> +#define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
>  
>  /*  Vendor-specific formats   */
>  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */
> 

Regards,

	Hans

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] media: vicodec: Register another node for stateless decoder
  2019-01-26 13:47 ` [PATCH 3/3] media: vicodec: Register another node for stateless decoder Dafna Hirschfeld
@ 2019-01-28  9:32   ` Hans Verkuil
  0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2019-01-28  9:32 UTC (permalink / raw)
  To: Dafna Hirschfeld, linux-media; +Cc: helen.koike

On 1/26/19 2:47 PM, Dafna Hirschfeld wrote:
> Add stateless decoder instance field to the dev struct and
> register another node for the statelsess decoder.
> The stateless API for the node will be implemented in further patches.
> 
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---
>  drivers/media/platform/vicodec/vicodec-core.c | 56 +++++++++++++++++--
>  1 file changed, 51 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
> index 25831d992681..7c2ad7d5f356 100644
> --- a/drivers/media/platform/vicodec/vicodec-core.c
> +++ b/drivers/media/platform/vicodec/vicodec-core.c
> @@ -104,6 +104,7 @@ struct vicodec_dev {
>  	struct v4l2_device	v4l2_dev;
>  	struct vicodec_dev_instance enc_instance;
>  	struct vicodec_dev_instance dec_instance;
> +	struct vicodec_dev_instance stateless_dec_instance;
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  	struct media_device	mdev;
>  #endif
> @@ -114,6 +115,7 @@ struct vicodec_ctx {
>  	struct v4l2_fh		fh;
>  	struct vicodec_dev	*dev;
>  	bool			is_enc;
> +	bool			is_stateless_dec;

I'd just call this 'is_stateless', since the same will be needed for the
future stateless encoder.

>  	spinlock_t		*lock;
>  
>  	struct v4l2_ctrl_handler hdl;
> @@ -314,6 +316,9 @@ static void device_run(void *priv)
>  
>  	if (ctx->is_enc)
>  		v4l2_m2m_job_finish(dev->enc_instance.m2m_dev, ctx->fh.m2m_ctx);
> +	else if (ctx->is_stateless_dec)
> +		v4l2_m2m_job_finish(dev->stateless_dec_instance.m2m_dev,
> +				    ctx->fh.m2m_ctx);
>  	else
>  		v4l2_m2m_job_finish(dev->dec_instance.m2m_dev, ctx->fh.m2m_ctx);
>  }
> @@ -1444,8 +1449,13 @@ static int queue_init(void *priv, struct vb2_queue *src_vq,
>  	src_vq->ops = &vicodec_qops;
>  	src_vq->mem_ops = &vb2_vmalloc_memops;
>  	src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;
> -	src_vq->lock = ctx->is_enc ? &ctx->dev->enc_instance.mutex :
> -		&ctx->dev->dec_instance.mutex;
> +	if (ctx->is_enc)
> +		src_vq->lock = &ctx->dev->enc_instance.mutex;
> +	else if (ctx->is_stateless_dec)
> +		src_vq->lock = &ctx->dev->stateless_dec_instance.mutex;
> +	else
> +		src_vq->lock = &ctx->dev->dec_instance.mutex;
> +	src_vq->supports_requests = ctx->is_stateless_dec ? true : false;
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
>  		return ret;
> @@ -1543,6 +1553,8 @@ static int vicodec_open(struct file *file)
>  
>  	if (vfd == &dev->enc_instance.vfd)
>  		ctx->is_enc = true;
> +	else if (vfd == &dev->stateless_dec_instance.vfd)
> +		ctx->is_stateless_dec = true;
>  
>  	v4l2_fh_init(&ctx->fh, video_devdata(file));
>  	file->private_data = &ctx->fh;
> @@ -1553,6 +1565,7 @@ static int vicodec_open(struct file *file)
>  			  1, 16, 1, 10);
>  	v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_i_frame, NULL);
>  	v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_p_frame, NULL);
> +	v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_stateless_state, NULL);

This should only be added if this is the stateless decoder.

>  	if (hdl->error) {
>  		rc = hdl->error;
>  		v4l2_ctrl_handler_free(hdl);
> @@ -1592,6 +1605,10 @@ static int vicodec_open(struct file *file)
>  		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->enc_instance.m2m_dev,
>  						    ctx, &queue_init);
>  		ctx->lock = &dev->enc_instance.lock;
> +	} else if (ctx->is_stateless_dec) {
> +		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateless_dec_instance.m2m_dev,
> +						    ctx, &queue_init);
> +		ctx->lock = &dev->stateless_dec_instance.lock;
>  	} else {
>  		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->dec_instance.m2m_dev,
>  						    ctx, &queue_init);
> @@ -1697,6 +1714,7 @@ static int vicodec_probe(struct platform_device *pdev)
>  
>  	spin_lock_init(&dev->enc_instance.lock);
>  	spin_lock_init(&dev->dec_instance.lock);
> +	spin_lock_init(&dev->stateless_dec_instance.lock);
>  
>  	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
>  	if (ret)
> @@ -1711,6 +1729,7 @@ static int vicodec_probe(struct platform_device *pdev)
>  
>  	mutex_init(&dev->enc_instance.mutex);
>  	mutex_init(&dev->dec_instance.mutex);
> +	mutex_init(&dev->stateless_dec_instance.mutex);

Why not init the mutex and the spinlock in register_instance()?

The same for v4l2_m2m_ctx_init().

Regards,

	Hans

>  
>  	platform_set_drvdata(pdev, dev);
>  
> @@ -1728,14 +1747,25 @@ static int vicodec_probe(struct platform_device *pdev)
>  		goto err_enc_m2m;
>  	}
>  
> +	dev->stateless_dec_instance.m2m_dev = v4l2_m2m_init(&m2m_ops);
> +	if (IS_ERR(dev->stateless_dec_instance.m2m_dev)) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec stateless dec device\n");
> +		ret = PTR_ERR(dev->stateless_dec_instance.m2m_dev);
> +		goto err_dec_m2m;
> +	}
> +
>  	if (register_instance(dev, &dev->enc_instance,
>  			      "videdev-enc", true))
> -		goto err_dec_m2m;
> +		goto err_sdec_m2m;
>  
>  	if (register_instance(dev, &dev->dec_instance,
>  			      "videdev-statefull-dec", false))
>  		goto unreg_enc;
>  
> +	if (register_instance(dev, &dev->stateless_dec_instance,
> +			      "videdev-stateless-dec", false))
> +		goto unreg_dec;
> +
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  	ret = v4l2_m2m_register_media_controller(dev->enc_instance.m2m_dev,
>  						 &dev->enc_instance.vfd,
> @@ -1753,24 +1783,38 @@ static int vicodec_probe(struct platform_device *pdev)
>  		goto unreg_m2m_enc_mc;
>  	}
>  
> +	ret = v4l2_m2m_register_media_controller(dev->stateless_dec_instance.m2m_dev,
> +						 &dev->stateless_dec_instance.vfd,
> +						 MEDIA_ENT_F_PROC_VIDEO_DECODER);
> +	if (ret) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for stateless dec\n");
> +		goto unreg_m2m_dec_mc;
> +	}
> +
>  	ret = media_device_register(&dev->mdev);
>  	if (ret) {
>  		v4l2_err(&dev->v4l2_dev, "Failed to register mem2mem media device\n");
> -		goto unreg_m2m_dec_mc;
> +		goto unreg_m2m_sdec_mc;
>  	}
>  #endif
>  	return 0;
>  
>  #ifdef CONFIG_MEDIA_CONTROLLER
> +unreg_m2m_sdec_mc:
> +	v4l2_m2m_unregister_media_controller(dev->stateless_dec_instance.m2m_dev);
>  unreg_m2m_dec_mc:
>  	v4l2_m2m_unregister_media_controller(dev->dec_instance.m2m_dev);
>  unreg_m2m_enc_mc:
>  	v4l2_m2m_unregister_media_controller(dev->enc_instance.m2m_dev);
>  unreg_m2m:
> -	video_unregister_device(&dev->dec_instance.vfd);
> +	video_unregister_device(&dev->stateless_dec_instance.vfd);
>  #endif
> +unreg_dec:
> +	video_unregister_device(&dev->dec_instance.vfd);
>  unreg_enc:
>  	video_unregister_device(&dev->enc_instance.vfd);
> +err_sdec_m2m:
> +	v4l2_m2m_release(dev->stateless_dec_instance.m2m_dev);
>  err_dec_m2m:
>  	v4l2_m2m_release(dev->dec_instance.m2m_dev);
>  err_enc_m2m:
> @@ -1791,6 +1835,7 @@ static int vicodec_remove(struct platform_device *pdev)
>  	media_device_unregister(&dev->mdev);
>  	v4l2_m2m_unregister_media_controller(dev->enc_instance.m2m_dev);
>  	v4l2_m2m_unregister_media_controller(dev->dec_instance.m2m_dev);
> +	v4l2_m2m_unregister_media_controller(dev->stateless_dec_instance.m2m_dev);
>  	media_device_cleanup(&dev->mdev);
>  #endif
>  
> @@ -1798,6 +1843,7 @@ static int vicodec_remove(struct platform_device *pdev)
>  	v4l2_m2m_release(dev->dec_instance.m2m_dev);
>  	video_unregister_device(&dev->enc_instance.vfd);
>  	video_unregister_device(&dev->dec_instance.vfd);
> +	video_unregister_device(&dev->stateless_dec_instance.vfd);
>  	v4l2_device_unregister(&dev->v4l2_dev);
>  
>  	return 0;
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-01-28  9:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-26 13:47 [PATCH 0/3] Prepare for statless decoder for fwht Dafna Hirschfeld
2019-01-26 13:47 ` [PATCH 1/3] media: vicodec: add struct for encoder/decoder instance Dafna Hirschfeld
2019-01-28  9:22   ` Hans Verkuil
2019-01-26 13:47 ` [PATCH 2/3] media: vicodec: Introducing stateless fwht defs and structs Dafna Hirschfeld
2019-01-28  9:28   ` Hans Verkuil
2019-01-26 13:47 ` [PATCH 3/3] media: vicodec: Register another node for stateless decoder Dafna Hirschfeld
2019-01-28  9:32   ` Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).