Linux-Media Archive on lore.kernel.org
 help / Atom feed
* [PATCH 0/9] media: vicodec: add support to stateless decoder
@ 2019-02-09 13:54 Dafna Hirschfeld
  2019-02-09 13:54 ` [PATCH 1/9] media: vicodec: Move raw frame preparation code to a function Dafna Hirschfeld
                   ` (8 more replies)
  0 siblings, 9 replies; 14+ messages in thread
From: Dafna Hirschfeld @ 2019-02-09 13:54 UTC (permalink / raw)
  To: linux-media; +Cc: hverkuil, helen.koike, Dafna Hirschfeld

Changes from previous sets:
---------------------------

Patch 3 - (keep the ref frame according to the format in decoder):
-----------------------------------------------------------------
Move the code that copies the capture buffer to the reference buffer
to a separate function 'copy_cap_to_ref'

Patches 4-6 - fixes according to the code review.

Patch 5 (Introducing stateless fwht defs and struct):
-----------------------------------------------------
add width,height fields to v4l2_ctrl_fwht_params

Patches 7-9 are new

Dafna Hirschfeld (9):
  media: vicodec: Move raw frame preparation code to a function
  media: vicodec: add field 'buf' to fwht_raw_frame
  media: vicodec: keep the ref frame according to the format in decoder
  media: vicodec: add struct for encoder/decoder instance
  media: vicodec: Introducing stateless fwht defs and structs
  media: vicodec: Register another node for stateless decoder
  media: vb2: Add func that return buffer by timestamp
  media: vicodec: call v4l2_m2m_buf_copy_metadata also upon error
  media: vicodec: Add support for stateless decoder.

 .../media/common/videobuf2/videobuf2-v4l2.c   |  14 +
 drivers/media/platform/vicodec/codec-fwht.c   |  68 ++-
 drivers/media/platform/vicodec/codec-fwht.h   |  11 +-
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 416 +++++-----------
 .../media/platform/vicodec/codec-v4l2-fwht.h  |   3 +
 drivers/media/platform/vicodec/vicodec-core.c | 465 +++++++++++++-----
 drivers/media/v4l2-core/v4l2-ctrls.c          |   6 +
 include/media/videobuf2-v4l2.h                |   3 +
 include/uapi/linux/v4l2-controls.h            |  12 +
 include/uapi/linux/videodev2.h                |   1 +
 10 files changed, 555 insertions(+), 444 deletions(-)

-- 
2.17.1


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

* [PATCH 1/9] media: vicodec: Move raw frame preparation code to a function
  2019-02-09 13:54 [PATCH 0/9] media: vicodec: add support to stateless decoder Dafna Hirschfeld
@ 2019-02-09 13:54 ` Dafna Hirschfeld
  2019-02-09 13:54 ` [PATCH 2/9] media: vicodec: add field 'buf' to fwht_raw_frame Dafna Hirschfeld
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Dafna Hirschfeld @ 2019-02-09 13:54 UTC (permalink / raw)
  To: linux-media; +Cc: hverkuil, helen.koike, Dafna Hirschfeld

Introduce 'prepare_raw_frame' function that fills the values
of a raw frame struct according to the format.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 140 ++++++++++--------
 1 file changed, 75 insertions(+), 65 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index c15034849133..728ed5012aed 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -75,117 +75,127 @@ const struct v4l2_fwht_pixfmt_info *v4l2_fwht_get_pixfmt(u32 idx)
 	return v4l2_fwht_pixfmts + idx;
 }
 
-int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
+static int prepare_raw_frame(struct fwht_raw_frame *rf,
+			 const struct v4l2_fwht_pixfmt_info *info, u8 *buf,
+			 unsigned int size)
 {
-	unsigned int size = state->stride * state->coded_height;
-	unsigned int chroma_stride = state->stride;
-	const struct v4l2_fwht_pixfmt_info *info = state->info;
-	struct fwht_cframe_hdr *p_hdr;
-	struct fwht_cframe cf;
-	struct fwht_raw_frame rf;
-	u32 encoding;
-	u32 flags = 0;
-
-	if (!info)
-		return -EINVAL;
-
-	rf.luma = p_in;
-	rf.width_div = info->width_div;
-	rf.height_div = info->height_div;
-	rf.luma_alpha_step = info->luma_alpha_step;
-	rf.chroma_step = info->chroma_step;
-	rf.alpha = NULL;
-	rf.components_num = info->components_num;
+	rf->luma = buf;
+	rf->width_div = info->width_div;
+	rf->height_div = info->height_div;
+	rf->luma_alpha_step = info->luma_alpha_step;
+	rf->chroma_step = info->chroma_step;
+	rf->alpha = NULL;
+	rf->components_num = info->components_num;
 
 	switch (info->id) {
 	case V4L2_PIX_FMT_GREY:
-		rf.cb = NULL;
-		rf.cr = NULL;
+		rf->cb = NULL;
+		rf->cr = NULL;
 		break;
 	case V4L2_PIX_FMT_YUV420:
-		rf.cb = rf.luma + size;
-		rf.cr = rf.cb + size / 4;
-		chroma_stride /= 2;
+		rf->cb = rf->luma + size;
+		rf->cr = rf->cb + size / 4;
 		break;
 	case V4L2_PIX_FMT_YVU420:
-		rf.cr = rf.luma + size;
-		rf.cb = rf.cr + size / 4;
-		chroma_stride /= 2;
+		rf->cr = rf->luma + size;
+		rf->cb = rf->cr + size / 4;
 		break;
 	case V4L2_PIX_FMT_YUV422P:
-		rf.cb = rf.luma + size;
-		rf.cr = rf.cb + size / 2;
-		chroma_stride /= 2;
+		rf->cb = rf->luma + size;
+		rf->cr = rf->cb + size / 2;
 		break;
 	case V4L2_PIX_FMT_NV12:
 	case V4L2_PIX_FMT_NV16:
 	case V4L2_PIX_FMT_NV24:
-		rf.cb = rf.luma + size;
-		rf.cr = rf.cb + 1;
+		rf->cb = rf->luma + size;
+		rf->cr = rf->cb + 1;
 		break;
 	case V4L2_PIX_FMT_NV21:
 	case V4L2_PIX_FMT_NV61:
 	case V4L2_PIX_FMT_NV42:
-		rf.cr = rf.luma + size;
-		rf.cb = rf.cr + 1;
+		rf->cr = rf->luma + size;
+		rf->cb = rf->cr + 1;
 		break;
 	case V4L2_PIX_FMT_YUYV:
-		rf.cb = rf.luma + 1;
-		rf.cr = rf.cb + 2;
+		rf->cb = rf->luma + 1;
+		rf->cr = rf->cb + 2;
 		break;
 	case V4L2_PIX_FMT_YVYU:
-		rf.cr = rf.luma + 1;
-		rf.cb = rf.cr + 2;
+		rf->cr = rf->luma + 1;
+		rf->cb = rf->cr + 2;
 		break;
 	case V4L2_PIX_FMT_UYVY:
-		rf.cb = rf.luma;
-		rf.cr = rf.cb + 2;
-		rf.luma++;
+		rf->cb = rf->luma;
+		rf->cr = rf->cb + 2;
+		rf->luma++;
 		break;
 	case V4L2_PIX_FMT_VYUY:
-		rf.cr = rf.luma;
-		rf.cb = rf.cr + 2;
-		rf.luma++;
+		rf->cr = rf->luma;
+		rf->cb = rf->cr + 2;
+		rf->luma++;
 		break;
 	case V4L2_PIX_FMT_RGB24:
 	case V4L2_PIX_FMT_HSV24:
-		rf.cr = rf.luma;
-		rf.cb = rf.cr + 2;
-		rf.luma++;
+		rf->cr = rf->luma;
+		rf->cb = rf->cr + 2;
+		rf->luma++;
 		break;
 	case V4L2_PIX_FMT_BGR24:
-		rf.cb = rf.luma;
-		rf.cr = rf.cb + 2;
-		rf.luma++;
+		rf->cb = rf->luma;
+		rf->cr = rf->cb + 2;
+		rf->luma++;
 		break;
 	case V4L2_PIX_FMT_RGB32:
 	case V4L2_PIX_FMT_XRGB32:
 	case V4L2_PIX_FMT_HSV32:
-		rf.cr = rf.luma + 1;
-		rf.cb = rf.cr + 2;
-		rf.luma += 2;
+		rf->cr = rf->luma + 1;
+		rf->cb = rf->cr + 2;
+		rf->luma += 2;
 		break;
 	case V4L2_PIX_FMT_BGR32:
 	case V4L2_PIX_FMT_XBGR32:
-		rf.cb = rf.luma;
-		rf.cr = rf.cb + 2;
-		rf.luma++;
+		rf->cb = rf->luma;
+		rf->cr = rf->cb + 2;
+		rf->luma++;
 		break;
 	case V4L2_PIX_FMT_ARGB32:
-		rf.alpha = rf.luma;
-		rf.cr = rf.luma + 1;
-		rf.cb = rf.cr + 2;
-		rf.luma += 2;
+		rf->alpha = rf->luma;
+		rf->cr = rf->luma + 1;
+		rf->cb = rf->cr + 2;
+		rf->luma += 2;
 		break;
 	case V4L2_PIX_FMT_ABGR32:
-		rf.cb = rf.luma;
-		rf.cr = rf.cb + 2;
-		rf.luma++;
-		rf.alpha = rf.cr + 1;
+		rf->cb = rf->luma;
+		rf->cr = rf->cb + 2;
+		rf->luma++;
+		rf->alpha = rf->cr + 1;
 		break;
 	default:
 		return -EINVAL;
 	}
+	return 0;
+}
+
+int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
+{
+	unsigned int size = state->stride * state->coded_height;
+	unsigned int chroma_stride = state->stride;
+	const struct v4l2_fwht_pixfmt_info *info = state->info;
+	struct fwht_cframe_hdr *p_hdr;
+	struct fwht_cframe cf;
+	struct fwht_raw_frame rf;
+	u32 encoding;
+	u32 flags = 0;
+
+	if (!info)
+		return -EINVAL;
+
+	if (prepare_raw_frame(&rf, info, p_in, size))
+		return -EINVAL;
+	if (info->id == V4L2_PIX_FMT_YUV420 ||
+	    info->id == V4L2_PIX_FMT_YVU420 ||
+	    info->id == V4L2_PIX_FMT_YUV422P)
+		chroma_stride /= 2;
 
 	cf.i_frame_qp = state->i_frame_qp;
 	cf.p_frame_qp = state->p_frame_qp;
-- 
2.17.1


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

* [PATCH 2/9] media: vicodec: add field 'buf' to fwht_raw_frame
  2019-02-09 13:54 [PATCH 0/9] media: vicodec: add support to stateless decoder Dafna Hirschfeld
  2019-02-09 13:54 ` [PATCH 1/9] media: vicodec: Move raw frame preparation code to a function Dafna Hirschfeld
@ 2019-02-09 13:54 ` Dafna Hirschfeld
  2019-02-09 13:54 ` [PATCH 3/9] media: vicodec: keep the ref frame according to the format in decoder Dafna Hirschfeld
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Dafna Hirschfeld @ 2019-02-09 13:54 UTC (permalink / raw)
  To: linux-media; +Cc: hverkuil, helen.koike, Dafna Hirschfeld

Add the field 'buf' to fwht_raw_frame to indicate
the start of the raw frame buffer.
This field will be used to copy the capture buffer
to the reference buffer in the next patch.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/codec-fwht.h   | 1 +
 drivers/media/platform/vicodec/vicodec-core.c | 7 +++++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-fwht.h b/drivers/media/platform/vicodec/codec-fwht.h
index 60d71d9dacb3..8a4f07d466cb 100644
--- a/drivers/media/platform/vicodec/codec-fwht.h
+++ b/drivers/media/platform/vicodec/codec-fwht.h
@@ -123,6 +123,7 @@ struct fwht_raw_frame {
 	unsigned int luma_alpha_step;
 	unsigned int chroma_step;
 	unsigned int components_num;
+	u8 *buf;
 	u8 *luma, *cb, *cr, *alpha;
 };
 
diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 9d739ea5542d..8d38bc1ef079 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -1364,7 +1364,8 @@ static int vicodec_start_streaming(struct vb2_queue *q,
 	state->stride = q_data->coded_width *
 				info->bytesperline_mult;
 
-	state->ref_frame.luma = kvmalloc(total_planes_size, GFP_KERNEL);
+	state->ref_frame.buf = kvmalloc(total_planes_size, GFP_KERNEL);
+	state->ref_frame.luma = state->ref_frame.buf;
 	ctx->comp_max_size = total_planes_size;
 	new_comp_frame = kvmalloc(ctx->comp_max_size, GFP_KERNEL);
 
@@ -1413,7 +1414,9 @@ static void vicodec_stop_streaming(struct vb2_queue *q)
 
 	if ((!V4L2_TYPE_IS_OUTPUT(q->type) && !ctx->is_enc) ||
 	    (V4L2_TYPE_IS_OUTPUT(q->type) && ctx->is_enc)) {
-		kvfree(ctx->state.ref_frame.luma);
+		kvfree(ctx->state.ref_frame.buf);
+		ctx->state.ref_frame.buf = NULL;
+		ctx->state.ref_frame.luma = NULL;
 		ctx->comp_max_size = 0;
 		ctx->source_changed = false;
 	}
-- 
2.17.1


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

* [PATCH 3/9] media: vicodec: keep the ref frame according to the format in decoder
  2019-02-09 13:54 [PATCH 0/9] media: vicodec: add support to stateless decoder Dafna Hirschfeld
  2019-02-09 13:54 ` [PATCH 1/9] media: vicodec: Move raw frame preparation code to a function Dafna Hirschfeld
  2019-02-09 13:54 ` [PATCH 2/9] media: vicodec: add field 'buf' to fwht_raw_frame Dafna Hirschfeld
@ 2019-02-09 13:54 ` Dafna Hirschfeld
  2019-02-09 13:54 ` [PATCH 4/9] media: vicodec: add struct for encoder/decoder instance Dafna Hirschfeld
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Dafna Hirschfeld @ 2019-02-09 13:54 UTC (permalink / raw)
  To: linux-media; +Cc: hverkuil, helen.koike, Dafna Hirschfeld

In the decoder, save the inner reference frame in the same
format as the capture buffer.
The decoder writes directly to the capture buffer and then
the capture buffer is copied to the reference buffer.
This will simplify the stateless decoder.

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/platform/vicodec/codec-fwht.c   |  68 +++--
 drivers/media/platform/vicodec/codec-fwht.h   |  10 +-
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 280 +++---------------
 .../media/platform/vicodec/codec-v4l2-fwht.h  |   3 +
 drivers/media/platform/vicodec/vicodec-core.c |   2 +
 5 files changed, 103 insertions(+), 260 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-fwht.c b/drivers/media/platform/vicodec/codec-fwht.c
index d1d6085da9f1..42849476069b 100644
--- a/drivers/media/platform/vicodec/codec-fwht.c
+++ b/drivers/media/platform/vicodec/codec-fwht.c
@@ -632,12 +632,13 @@ static int decide_blocktype(const u8 *cur, const u8 *reference,
 	return vari <= vard ? IBLOCK : PBLOCK;
 }
 
-static void fill_decoder_block(u8 *dst, const s16 *input, int stride)
+static void fill_decoder_block(u8 *dst, const s16 *input, int stride,
+			       unsigned int dst_step)
 {
 	int i, j;
 
 	for (i = 0; i < 8; i++) {
-		for (j = 0; j < 8; j++, input++, dst++) {
+		for (j = 0; j < 8; j++, input++, dst += dst_step) {
 			if (*input < 0)
 				*dst = 0;
 			else if (*input > 255)
@@ -645,17 +646,19 @@ static void fill_decoder_block(u8 *dst, const s16 *input, int stride)
 			else
 				*dst = *input;
 		}
-		dst += stride - 8;
+		dst += stride - (8 * dst_step);
 	}
 }
 
-static void add_deltas(s16 *deltas, const u8 *ref, int stride)
+static void add_deltas(s16 *deltas, const u8 *ref, int stride,
+		       unsigned int ref_step)
 {
 	int k, l;
 
 	for (k = 0; k < 8; k++) {
 		for (l = 0; l < 8; l++) {
-			*deltas += *ref++;
+			*deltas += *ref;
+			ref += ref_step;
 			/*
 			 * Due to quantizing, it might possible that the
 			 * decoded coefficients are slightly out of range
@@ -666,7 +669,7 @@ static void add_deltas(s16 *deltas, const u8 *ref, int stride)
 				*deltas = 255;
 			deltas++;
 		}
-		ref += stride - 8;
+		ref += stride - (8 * ref_step);
 	}
 }
 
@@ -711,8 +714,8 @@ static u32 encode_plane(u8 *input, u8 *refp, __be16 **rlco, __be16 *rlco_max,
 				ifwht(cf->de_coeffs, cf->de_fwht, blocktype);
 
 				if (blocktype == PBLOCK)
-					add_deltas(cf->de_fwht, refp, 8);
-				fill_decoder_block(refp, cf->de_fwht, 8);
+					add_deltas(cf->de_fwht, refp, 8, 1);
+				fill_decoder_block(refp, cf->de_fwht, 8, 1);
 			}
 
 			input += 8 * input_step;
@@ -821,8 +824,10 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
 	return encoding;
 }
 
-static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,
-			 u32 height, u32 width, u32 coded_width,
+static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco,
+			 const u8 *ref, u32 height, u32 width, u32 coded_width,
+			 u8 *dst, unsigned int dst_stride,
+			 unsigned int dst_step, unsigned int ref_step,
 			 bool uncompressed, const __be16 *end_of_rlco_buf)
 {
 	unsigned int copies = 0;
@@ -834,10 +839,15 @@ static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,
 	height = round_up(height, 8);
 
 	if (uncompressed) {
+		int i;
+
 		if (end_of_rlco_buf + 1 < *rlco + width * height / 2)
 			return false;
-		memcpy(ref, *rlco, width * height);
-		*rlco += width * height / 2;
+		for (i = 0; i < height; i++) {
+			memcpy(dst, *rlco, width);
+			dst += dst_stride;
+			*rlco += width / 2;
+		}
 		return true;
 	}
 
@@ -849,15 +859,18 @@ static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,
 	 */
 	for (j = 0; j < height / 8; j++) {
 		for (i = 0; i < width / 8; i++) {
-			u8 *refp = ref + j * 8 * coded_width + i * 8;
+			const u8 *refp = ref + j * 8 * ref_step * coded_width +
+				i * 8 * ref_step;
+			u8 *dstp = dst + j * 8 * dst_stride + i * 8 * dst_step;
 
 			if (copies) {
 				memcpy(cf->de_fwht, copy, sizeof(copy));
 				if (stat & PFRAME_BIT)
 					add_deltas(cf->de_fwht, refp,
-						   coded_width);
-				fill_decoder_block(refp, cf->de_fwht,
-						   coded_width);
+						   coded_width * ref_step,
+						   ref_step);
+				fill_decoder_block(dstp, cf->de_fwht,
+						   dst_stride, dst_step);
 				copies--;
 				continue;
 			}
@@ -877,23 +890,28 @@ static bool decode_plane(struct fwht_cframe *cf, const __be16 **rlco, u8 *ref,
 			if (copies)
 				memcpy(copy, cf->de_fwht, sizeof(copy));
 			if (stat & PFRAME_BIT)
-				add_deltas(cf->de_fwht, refp, coded_width);
-			fill_decoder_block(refp, cf->de_fwht, coded_width);
+				add_deltas(cf->de_fwht, refp,
+					   coded_width * ref_step, ref_step);
+			fill_decoder_block(dstp, cf->de_fwht, dst_stride,
+					   dst_step);
 		}
 	}
 	return true;
 }
 
-bool fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
+bool fwht_decode_frame(struct fwht_cframe *cf, const struct fwht_raw_frame *ref,
 		       u32 hdr_flags, unsigned int components_num,
 		       unsigned int width, unsigned int height,
-		       unsigned int coded_width)
+		       unsigned int coded_width, struct fwht_raw_frame *dst,
+		       unsigned int dst_stride, unsigned int dst_chroma_stride)
 {
 	const __be16 *rlco = cf->rlc_data;
 	const __be16 *end_of_rlco_buf = cf->rlc_data +
 			(cf->size / sizeof(*rlco)) - 1;
 
 	if (!decode_plane(cf, &rlco, ref->luma, height, width, coded_width,
+			  dst->luma, dst_stride, dst->luma_alpha_step,
+			  ref->luma_alpha_step,
 			  hdr_flags & FWHT_FL_LUMA_IS_UNCOMPRESSED,
 			  end_of_rlco_buf))
 		return false;
@@ -909,11 +927,15 @@ bool fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
 			w /= 2;
 			c /= 2;
 		}
-		if (!decode_plane(cf, &rlco, ref->cb, h, w, c,
+		if (!decode_plane(cf, &rlco, ref->cb, h, w, c, dst->cb,
+				  dst_chroma_stride, dst->chroma_step,
+				  ref->chroma_step,
 				  hdr_flags & FWHT_FL_CB_IS_UNCOMPRESSED,
 				  end_of_rlco_buf))
 			return false;
-		if (!decode_plane(cf, &rlco, ref->cr, h, w, c,
+		if (!decode_plane(cf, &rlco, ref->cr, h, w, c, dst->cr,
+				  dst_chroma_stride, dst->chroma_step,
+				  ref->chroma_step,
 				  hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED,
 				  end_of_rlco_buf))
 			return false;
@@ -922,6 +944,8 @@ bool fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
 	if (components_num == 4)
 		if (!decode_plane(cf, &rlco, ref->alpha, height, width,
 				  coded_width,
+				  dst->alpha, dst_stride, dst->luma_alpha_step,
+				  ref->luma_alpha_step,
 				  hdr_flags & FWHT_FL_ALPHA_IS_UNCOMPRESSED,
 				  end_of_rlco_buf))
 			return false;
diff --git a/drivers/media/platform/vicodec/codec-fwht.h b/drivers/media/platform/vicodec/codec-fwht.h
index 8a4f07d466cb..eab4a97aa132 100644
--- a/drivers/media/platform/vicodec/codec-fwht.h
+++ b/drivers/media/platform/vicodec/codec-fwht.h
@@ -140,9 +140,9 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
 		      bool is_intra, bool next_is_intra,
 		      unsigned int width, unsigned int height,
 		      unsigned int stride, unsigned int chroma_stride);
-bool fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
-		       u32 hdr_flags, unsigned int components_num,
-		       unsigned int width, unsigned int height,
-		       unsigned int coded_width);
-
+bool fwht_decode_frame(struct fwht_cframe *cf, const struct fwht_raw_frame *ref,
+		u32 hdr_flags, unsigned int components_num,
+		unsigned int width, unsigned int height,
+		unsigned int coded_width, struct fwht_raw_frame *dst,
+		unsigned int dst_stride, unsigned int dst_chroma_stride);
 #endif
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index 728ed5012aed..40b1f4901fd3 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -75,6 +75,35 @@ const struct v4l2_fwht_pixfmt_info *v4l2_fwht_get_pixfmt(u32 idx)
 	return v4l2_fwht_pixfmts + idx;
 }
 
+void copy_cap_to_ref(u8 *cap, const struct v4l2_fwht_pixfmt_info *info,
+		     struct v4l2_fwht_state *state)
+{
+	int plane_idx;
+	u8 *p_ref = state->ref_frame.buf;
+
+	for (plane_idx = 0; plane_idx < info->planes_num; plane_idx++) {
+		int i;
+		bool is_chroma_plane = plane_idx == 1 || plane_idx == 2;
+		unsigned int h_div = is_chroma_plane ? info->height_div : 1;
+		unsigned int w_div = is_chroma_plane ? info->width_div : 1;
+		unsigned int step = is_chroma_plane ? info->chroma_step :
+			info->luma_alpha_step;
+		unsigned int stride_div =
+			(info->planes_num == 3 && plane_idx > 0) ? 2 : 1;
+
+		u8 *row_dst = cap;
+		u8 *row_ref = p_ref;
+
+		for (i = 0; i < state->visible_height / h_div; i++) {
+			memcpy(row_ref, row_dst, step * state->visible_width / w_div);
+			row_ref += step * state->coded_width / w_div;
+			row_dst += state->stride / stride_div;
+		}
+		cap += (state->stride / stride_div) * (state->coded_height / h_div);
+		p_ref += (step * state->coded_width / w_div) * (state->coded_height / h_div);
+	}
+}
+
 static int prepare_raw_frame(struct fwht_raw_frame *rf,
 			 const struct v4l2_fwht_pixfmt_info *info, u8 *buf,
 			 unsigned int size)
@@ -243,14 +272,16 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
 
 int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
 {
-	unsigned int i, j, k;
 	u32 flags;
 	struct fwht_cframe cf;
-	u8 *p, *ref_p;
 	unsigned int components_num = 3;
 	unsigned int version;
 	const struct v4l2_fwht_pixfmt_info *info;
 	unsigned int hdr_width_div, hdr_height_div;
+	struct fwht_raw_frame dst_rf;
+	unsigned int dst_chroma_stride = state->stride;
+	unsigned int dst_size = state->stride * state->coded_height;
+	unsigned int ref_size;
 
 	if (!state->info)
 		return -EINVAL;
@@ -298,241 +329,24 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out)
 	    hdr_height_div != info->height_div)
 		return -EINVAL;
 
-	if (!fwht_decode_frame(&cf, &state->ref_frame, flags, components_num,
-			       state->visible_width, state->visible_height,
-			       state->coded_width))
+	if (prepare_raw_frame(&dst_rf, info, p_out, dst_size))
 		return -EINVAL;
+	if (info->id == V4L2_PIX_FMT_YUV420 ||
+	    info->id == V4L2_PIX_FMT_YVU420 ||
+	    info->id == V4L2_PIX_FMT_YUV422P)
+		dst_chroma_stride /= 2;
 
-	/*
-	 * TODO - handle the case where the compressed stream encodes a
-	 * different format than the requested decoded format.
-	 */
-	switch (state->info->id) {
-	case V4L2_PIX_FMT_GREY:
-		ref_p = state->ref_frame.luma;
-		for (i = 0; i < state->coded_height; i++)  {
-			memcpy(p_out, ref_p, state->visible_width);
-			p_out += state->stride;
-			ref_p += state->coded_width;
-		}
-		break;
-	case V4L2_PIX_FMT_YUV420:
-	case V4L2_PIX_FMT_YUV422P:
-		ref_p = state->ref_frame.luma;
-		for (i = 0; i < state->coded_height; i++)  {
-			memcpy(p_out, ref_p, state->visible_width);
-			p_out += state->stride;
-			ref_p += state->coded_width;
-		}
-
-		ref_p = state->ref_frame.cb;
-		for (i = 0; i < state->coded_height / 2; i++)  {
-			memcpy(p_out, ref_p, state->visible_width / 2);
-			p_out += state->stride / 2;
-			ref_p += state->coded_width / 2;
-		}
-		ref_p = state->ref_frame.cr;
-		for (i = 0; i < state->coded_height / 2; i++)  {
-			memcpy(p_out, ref_p, state->visible_width / 2);
-			p_out += state->stride / 2;
-			ref_p += state->coded_width / 2;
-		}
-		break;
-	case V4L2_PIX_FMT_YVU420:
-		ref_p = state->ref_frame.luma;
-		for (i = 0; i < state->coded_height; i++)  {
-			memcpy(p_out, ref_p, state->visible_width);
-			p_out += state->stride;
-			ref_p += state->coded_width;
-		}
-
-		ref_p = state->ref_frame.cr;
-		for (i = 0; i < state->coded_height / 2; i++)  {
-			memcpy(p_out, ref_p, state->visible_width / 2);
-			p_out += state->stride / 2;
-			ref_p += state->coded_width / 2;
-		}
-		ref_p = state->ref_frame.cb;
-		for (i = 0; i < state->coded_height / 2; i++)  {
-			memcpy(p_out, ref_p, state->visible_width / 2);
-			p_out += state->stride / 2;
-			ref_p += state->coded_width / 2;
-		}
-		break;
-	case V4L2_PIX_FMT_NV12:
-	case V4L2_PIX_FMT_NV16:
-	case V4L2_PIX_FMT_NV24:
-		ref_p = state->ref_frame.luma;
-		for (i = 0; i < state->coded_height; i++)  {
-			memcpy(p_out, ref_p, state->visible_width);
-			p_out += state->stride;
-			ref_p += state->coded_width;
-		}
+	ref_size = state->coded_width * state->coded_height *
+		info->luma_alpha_step;
 
-		k = 0;
-		for (i = 0; i < state->coded_height / 2; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.cb[k];
-				*p++ = state->ref_frame.cr[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_NV21:
-	case V4L2_PIX_FMT_NV61:
-	case V4L2_PIX_FMT_NV42:
-		ref_p = state->ref_frame.luma;
-		for (i = 0; i < state->coded_height; i++)  {
-			memcpy(p_out, ref_p, state->visible_width);
-			p_out += state->stride;
-			ref_p += state->coded_width;
-		}
+	if (prepare_raw_frame(&state->ref_frame, info, state->ref_frame.buf,
+			      ref_size))
+		return -EINVAL;
 
-		k = 0;
-		for (i = 0; i < state->coded_height / 2; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.cr[k];
-				*p++ = state->ref_frame.cb[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_YUYV:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cb[k / 2];
-				*p++ = state->ref_frame.luma[k + 1];
-				*p++ = state->ref_frame.cr[k / 2];
-				k += 2;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_YVYU:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cr[k / 2];
-				*p++ = state->ref_frame.luma[k + 1];
-				*p++ = state->ref_frame.cb[k / 2];
-				k += 2;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_UYVY:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.cb[k / 2];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cr[k / 2];
-				*p++ = state->ref_frame.luma[k + 1];
-				k += 2;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_VYUY:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width / 2; j++) {
-				*p++ = state->ref_frame.cr[k / 2];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cb[k / 2];
-				*p++ = state->ref_frame.luma[k + 1];
-				k += 2;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_RGB24:
-	case V4L2_PIX_FMT_HSV24:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = state->ref_frame.cr[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cb[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_BGR24:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = state->ref_frame.cb[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cr[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_RGB32:
-	case V4L2_PIX_FMT_XRGB32:
-	case V4L2_PIX_FMT_HSV32:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = 0;
-				*p++ = state->ref_frame.cr[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cb[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_BGR32:
-	case V4L2_PIX_FMT_XBGR32:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = state->ref_frame.cb[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cr[k];
-				*p++ = 0;
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_ARGB32:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = state->ref_frame.alpha[k];
-				*p++ = state->ref_frame.cr[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cb[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	case V4L2_PIX_FMT_ABGR32:
-		k = 0;
-		for (i = 0; i < state->coded_height; i++) {
-			for (j = 0, p = p_out; j < state->coded_width; j++) {
-				*p++ = state->ref_frame.cb[k];
-				*p++ = state->ref_frame.luma[k];
-				*p++ = state->ref_frame.cr[k];
-				*p++ = state->ref_frame.alpha[k];
-				k++;
-			}
-			p_out += state->stride;
-		}
-		break;
-	default:
+	if (!fwht_decode_frame(&cf, &state->ref_frame, flags, components_num,
+			       state->visible_width, state->visible_height,
+			       state->coded_width, &dst_rf, state->stride,
+			       dst_chroma_stride))
 		return -EINVAL;
-	}
 	return 0;
 }
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.h b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
index aa6fa90a48be..75343cdf45e2 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.h
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.h
@@ -53,6 +53,9 @@ const struct v4l2_fwht_pixfmt_info *v4l2_fwht_default_fmt(u32 width_div,
 							  u32 pixenc,
 							  unsigned int start_idx);
 
+void copy_cap_to_ref(u8 *cap, const struct v4l2_fwht_pixfmt_info *info,
+		struct v4l2_fwht_state *state);
+
 int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out);
 int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 *p_in, u8 *p_out);
 
diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 8d38bc1ef079..335a931fdf02 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -194,6 +194,8 @@ static int device_process(struct vicodec_ctx *ctx,
 		ret = v4l2_fwht_decode(state, p_src, p_dst);
 		if (ret < 0)
 			return ret;
+		copy_cap_to_ref(p_dst, ctx->state.info, &ctx->state);
+
 		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, q_dst->sizeimage);
 	}
 
-- 
2.17.1


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

* [PATCH 4/9] media: vicodec: add struct for encoder/decoder instance
  2019-02-09 13:54 [PATCH 0/9] media: vicodec: add support to stateless decoder Dafna Hirschfeld
                   ` (2 preceding siblings ...)
  2019-02-09 13:54 ` [PATCH 3/9] media: vicodec: keep the ref frame according to the format in decoder Dafna Hirschfeld
@ 2019-02-09 13:54 ` Dafna Hirschfeld
  2019-02-11  8:31   ` Hans Verkuil
  2019-02-09 13:54 ` [PATCH 5/9] media: vicodec: Introducing stateless fwht defs and structs Dafna Hirschfeld
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Dafna Hirschfeld @ 2019-02-09 13:54 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 | 194 +++++++++---------
 1 file changed, 92 insertions(+), 102 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 335a931fdf02..e5987229c5a3 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 stateful_enc;
+	struct vicodec_dev_instance stateful_dec;
 #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 {
@@ -312,9 +312,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->stateful_enc.m2m_dev, ctx->fh.m2m_ctx);
 	else
-		v4l2_m2m_job_finish(dev->dec_dev, ctx->fh.m2m_ctx);
+		v4l2_m2m_job_finish(dev->stateful_dec.m2m_dev, ctx->fh.m2m_ctx);
 }
 
 static void job_remove_src_buf(struct vicodec_ctx *ctx, u32 state)
@@ -1457,9 +1457,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->stateful_enc.mutex :
+		&ctx->dev->stateful_dec.mutex;
 	ret = vb2_queue_init(src_vq);
 	if (ret)
 		return ret;
@@ -1547,7 +1546,7 @@ static int vicodec_open(struct file *file)
 		goto open_unlock;
 	}
 
-	if (vfd == &dev->enc_vfd)
+	if (vfd == &dev->stateful_enc.vfd)
 		ctx->is_enc = true;
 
 	v4l2_fh_init(&ctx->fh, video_devdata(file));
@@ -1595,13 +1594,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->stateful_enc.m2m_dev,
+						    ctx, &queue_init);
+		ctx->lock = &dev->stateful_enc.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->stateful_dec.m2m_dev,
+						    ctx, &queue_init);
+		ctx->lock = &dev->stateful_dec.lock;
 	}
 
 	if (IS_ERR(ctx->fh.m2m_ctx)) {
@@ -1659,19 +1658,57 @@ 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;
+
+	spin_lock_init(&dev_instance->lock);
+	mutex_init(&dev_instance->mutex);
+	dev_instance->m2m_dev = v4l2_m2m_init(&m2m_ops);
+	if (IS_ERR(dev_instance->m2m_dev)) {
+		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec enc device\n");
+		return PTR_ERR(dev_instance->m2m_dev);
+	}
+
+	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);
+		v4l2_m2m_release(dev_instance->m2m_dev);
+		return ret;
+	}
+	v4l2_info(&dev->v4l2_dev, "Device '%s' registered as /dev/video%d\n",
+		  name, vfd->num);
+	return 0;
+}
+
 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);
-
 	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
 	if (ret)
 		return ret;
@@ -1685,100 +1722,53 @@ 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);
-
 	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);
+	if (register_instance(dev, &dev->stateful_enc,
+			      "videdev-enc", true))
 		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);
-		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");
-		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");
-		goto unreg_enc;
-	}
-	v4l2_info(&dev->v4l2_dev,
-			"Device registered as /dev/video%d\n", vfd->num);
+	if (register_instance(dev, &dev->stateful_dec,
+			      "videdev-statefull-dec", false))
+		goto unreg_sf_enc;
 
 #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->stateful_enc.m2m_dev,
+						 &dev->stateful_enc.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->stateful_dec.m2m_dev,
+						 &dev->stateful_dec.vfd,
+						 MEDIA_ENT_F_PROC_VIDEO_DECODER);
 	if (ret) {
-		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller\n");
-		goto unreg_m2m_enc_mc;
+		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for dec\n");
+		goto unreg_m2m_sf_enc_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_sf_dec_mc;
 	}
 #endif
 	return 0;
 
 #ifdef CONFIG_MEDIA_CONTROLLER
-unreg_m2m_dec_mc:
-	v4l2_m2m_unregister_media_controller(dev->dec_dev);
-unreg_m2m_enc_mc:
-	v4l2_m2m_unregister_media_controller(dev->enc_dev);
+unreg_m2m_sf_dec_mc:
+	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
+unreg_m2m_sf_enc_mc:
+	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
 unreg_m2m:
-	video_unregister_device(&dev->dec_vfd);
+	video_unregister_device(&dev->stateful_dec.vfd);
+	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
 #endif
-unreg_enc:
-	video_unregister_device(&dev->enc_vfd);
-err_dec_m2m:
-	v4l2_m2m_release(dev->dec_dev);
-err_enc_m2m:
-	v4l2_m2m_release(dev->enc_dev);
+unreg_sf_enc:
+	video_unregister_device(&dev->stateful_enc.vfd);
+	v4l2_m2m_release(dev->stateful_enc.m2m_dev);
 unreg_dev:
 	v4l2_device_unregister(&dev->v4l2_dev);
 
@@ -1793,15 +1783,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->stateful_enc.m2m_dev);
+	v4l2_m2m_unregister_media_controller(dev->stateful_dec.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->stateful_enc.m2m_dev);
+	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
+	video_unregister_device(&dev->stateful_enc.vfd);
+	video_unregister_device(&dev->stateful_dec.vfd);
 	v4l2_device_unregister(&dev->v4l2_dev);
 
 	return 0;
-- 
2.17.1


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

* [PATCH 5/9] media: vicodec: Introducing stateless fwht defs and structs
  2019-02-09 13:54 [PATCH 0/9] media: vicodec: add support to stateless decoder Dafna Hirschfeld
                   ` (3 preceding siblings ...)
  2019-02-09 13:54 ` [PATCH 4/9] media: vicodec: add struct for encoder/decoder instance Dafna Hirschfeld
@ 2019-02-09 13:54 ` Dafna Hirschfeld
  2019-02-09 13:54 ` [PATCH 6/9] media: vicodec: Register another node for stateless decoder Dafna Hirschfeld
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 14+ messages in thread
From: Dafna Hirschfeld @ 2019-02-09 13:54 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            | 12 ++++++++++++
 include/uapi/linux/videodev2.h                |  1 +
 4 files changed, 31 insertions(+)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index e5987229c5a3..95276c09cb9a 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)
 {
 }
@@ -1480,6 +1484,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)
 {
@@ -1526,6 +1531,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..aa2d48a2722f 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,15 @@ 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 {
+	__u64 backward_ref_ts;
+	__u32 width;
+	__u32 height;
+	__u32 flags;
+	__u32 colorspace;
+	__u32 xfer_func;
+	__u32 ycbcr_enc;
+	__u32 quantization;
+};
+
 #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	[flat|nested] 14+ messages in thread

* [PATCH 6/9] media: vicodec: Register another node for stateless decoder
  2019-02-09 13:54 [PATCH 0/9] media: vicodec: add support to stateless decoder Dafna Hirschfeld
                   ` (4 preceding siblings ...)
  2019-02-09 13:54 ` [PATCH 5/9] media: vicodec: Introducing stateless fwht defs and structs Dafna Hirschfeld
@ 2019-02-09 13:54 ` Dafna Hirschfeld
  2019-02-11  8:31   ` Hans Verkuil
  2019-02-09 13:54 ` [PATCH 7/9] media: vb2: Add func that return buffer by timestamp Dafna Hirschfeld
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 14+ messages in thread
From: Dafna Hirschfeld @ 2019-02-09 13:54 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 | 45 +++++++++++++++++--
 1 file changed, 41 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 95276c09cb9a..324ce566478e 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 stateful_enc;
 	struct vicodec_dev_instance stateful_dec;
+	struct vicodec_dev_instance stateless_dec;
 #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;
 	spinlock_t		*lock;
 
 	struct v4l2_ctrl_handler hdl;
@@ -317,6 +319,9 @@ static void device_run(void *priv)
 
 	if (ctx->is_enc)
 		v4l2_m2m_job_finish(dev->stateful_enc.m2m_dev, ctx->fh.m2m_ctx);
+	else if (ctx->is_stateless)
+		v4l2_m2m_job_finish(dev->stateless_dec.m2m_dev,
+				    ctx->fh.m2m_ctx);
 	else
 		v4l2_m2m_job_finish(dev->stateful_dec.m2m_dev, ctx->fh.m2m_ctx);
 }
@@ -1461,8 +1466,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->stateful_enc.mutex :
-		&ctx->dev->stateful_dec.mutex;
+	if (ctx->is_enc)
+		src_vq->lock = &ctx->dev->stateful_enc.mutex;
+	else if (ctx->is_stateless)
+		src_vq->lock = &ctx->dev->stateless_dec.mutex;
+	else
+		src_vq->lock = &ctx->dev->stateful_dec.mutex;
+	src_vq->supports_requests = ctx->is_stateless ? true : false;
 	ret = vb2_queue_init(src_vq);
 	if (ret)
 		return ret;
@@ -1560,6 +1570,8 @@ static int vicodec_open(struct file *file)
 
 	if (vfd == &dev->stateful_enc.vfd)
 		ctx->is_enc = true;
+	else if (vfd == &dev->stateless_dec.vfd)
+		ctx->is_stateless = true;
 
 	v4l2_fh_init(&ctx->fh, video_devdata(file));
 	file->private_data = &ctx->fh;
@@ -1570,6 +1582,8 @@ 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);
+	if (ctx->is_stateless)
+		v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_stateless_state, NULL);
 	if (hdl->error) {
 		rc = hdl->error;
 		v4l2_ctrl_handler_free(hdl);
@@ -1609,6 +1623,10 @@ static int vicodec_open(struct file *file)
 		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateful_enc.m2m_dev,
 						    ctx, &queue_init);
 		ctx->lock = &dev->stateful_enc.lock;
+	} else if (ctx->is_stateless) {
+		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateless_dec.m2m_dev,
+						    ctx, &queue_init);
+		ctx->lock = &dev->stateless_dec.lock;
 	} else {
 		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateful_dec.m2m_dev,
 						    ctx, &queue_init);
@@ -1744,6 +1762,10 @@ static int vicodec_probe(struct platform_device *pdev)
 			      "videdev-statefull-dec", false))
 		goto unreg_sf_enc;
 
+	if (register_instance(dev, &dev->stateless_dec,
+			      "videdev-stateless-dec", false))
+		goto unreg_sf_dec;
+
 #ifdef CONFIG_MEDIA_CONTROLLER
 	ret = v4l2_m2m_register_media_controller(dev->stateful_enc.m2m_dev,
 						 &dev->stateful_enc.vfd,
@@ -1761,23 +1783,36 @@ static int vicodec_probe(struct platform_device *pdev)
 		goto unreg_m2m_sf_enc_mc;
 	}
 
+	ret = v4l2_m2m_register_media_controller(dev->stateless_dec.m2m_dev,
+						 &dev->stateless_dec.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_sf_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_sf_dec_mc;
+		goto unreg_m2m_sl_dec_mc;
 	}
 #endif
 	return 0;
 
 #ifdef CONFIG_MEDIA_CONTROLLER
+unreg_m2m_sl_dec_mc:
+	v4l2_m2m_unregister_media_controller(dev->stateless_dec.m2m_dev);
 unreg_m2m_sf_dec_mc:
 	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
 unreg_m2m_sf_enc_mc:
 	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
 unreg_m2m:
+	video_unregister_device(&dev->stateless_dec.vfd);
+	v4l2_m2m_release(dev->stateless_dec.m2m_dev);
+#endif
+unreg_sf_dec:
 	video_unregister_device(&dev->stateful_dec.vfd);
 	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
-#endif
 unreg_sf_enc:
 	video_unregister_device(&dev->stateful_enc.vfd);
 	v4l2_m2m_release(dev->stateful_enc.m2m_dev);
@@ -1797,6 +1832,7 @@ static int vicodec_remove(struct platform_device *pdev)
 	media_device_unregister(&dev->mdev);
 	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
 	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
+	v4l2_m2m_unregister_media_controller(dev->stateless_dec.m2m_dev);
 	media_device_cleanup(&dev->mdev);
 #endif
 
@@ -1804,6 +1840,7 @@ static int vicodec_remove(struct platform_device *pdev)
 	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
 	video_unregister_device(&dev->stateful_enc.vfd);
 	video_unregister_device(&dev->stateful_dec.vfd);
+	video_unregister_device(&dev->stateless_dec.vfd);
 	v4l2_device_unregister(&dev->v4l2_dev);
 
 	return 0;
-- 
2.17.1


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

* [PATCH 7/9] media: vb2: Add func that return buffer by timestamp
  2019-02-09 13:54 [PATCH 0/9] media: vicodec: add support to stateless decoder Dafna Hirschfeld
                   ` (5 preceding siblings ...)
  2019-02-09 13:54 ` [PATCH 6/9] media: vicodec: Register another node for stateless decoder Dafna Hirschfeld
@ 2019-02-09 13:54 ` Dafna Hirschfeld
  2019-02-11 10:11   ` Hans Verkuil
  2019-02-09 13:54 ` [PATCH 8/9] media: vicodec: call v4l2_m2m_buf_copy_metadata also upon error Dafna Hirschfeld
  2019-02-09 13:54 ` [PATCH 9/9] media: vicodec: Add support for stateless decoder Dafna Hirschfeld
  8 siblings, 1 reply; 14+ messages in thread
From: Dafna Hirschfeld @ 2019-02-09 13:54 UTC (permalink / raw)
  To: linux-media; +Cc: hverkuil, helen.koike, Dafna Hirschfeld

Add the function 'vb2_find_timestamp_buf' that returns
the vb2 buffer that matches the given timestamp

Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
---
 drivers/media/common/videobuf2/videobuf2-v4l2.c | 14 ++++++++++++++
 include/media/videobuf2-v4l2.h                  |  3 +++
 2 files changed, 17 insertions(+)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 3aeaea3af42a..47c245a76561 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -598,6 +598,20 @@ static const struct vb2_buf_ops v4l2_buf_ops = {
 	.copy_timestamp		= __copy_timestamp,
 };
 
+struct vb2_buffer *vb2_find_timestamp_buf(const struct vb2_queue *q,
+					  u64 timestamp,
+					  unsigned int start_idx)
+{
+	unsigned int i;
+
+	for (i = start_idx; i < q->num_buffers; i++) {
+		if (q->bufs[i]->timestamp == timestamp)
+			return q->bufs[i];
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(vb2_find_timestamp_buf);
+
 int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
 		       unsigned int start_idx)
 {
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index 8a10889dc2fd..7fc2a235064e 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -71,6 +71,9 @@ struct vb2_v4l2_buffer {
 int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
 		       unsigned int start_idx);
 
+struct vb2_buffer *vb2_find_timestamp_buf(const struct vb2_queue *q,
+					  u64 timestamp,
+					  unsigned int start_idx);
 int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b);
 
 /**
-- 
2.17.1


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

* [PATCH 8/9] media: vicodec: call v4l2_m2m_buf_copy_metadata also upon error
  2019-02-09 13:54 [PATCH 0/9] media: vicodec: add support to stateless decoder Dafna Hirschfeld
                   ` (6 preceding siblings ...)
  2019-02-09 13:54 ` [PATCH 7/9] media: vb2: Add func that return buffer by timestamp Dafna Hirschfeld
@ 2019-02-09 13:54 ` Dafna Hirschfeld
  2019-02-09 13:54 ` [PATCH 9/9] media: vicodec: Add support for stateless decoder Dafna Hirschfeld
  8 siblings, 0 replies; 14+ messages in thread
From: Dafna Hirschfeld @ 2019-02-09 13:54 UTC (permalink / raw)
  To: linux-media; +Cc: hverkuil, helen.koike, Dafna Hirschfeld

call v4l2_m2m_buf_copy_metadata also if decoding/encoding
ends with status VB2_BUF_STATE_ERROR.

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

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 324ce566478e..9a6ee593ae19 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -165,12 +165,10 @@ static int device_process(struct vicodec_ctx *ctx,
 			  struct vb2_v4l2_buffer *dst_vb)
 {
 	struct vicodec_dev *dev = ctx->dev;
-	struct vicodec_q_data *q_dst;
 	struct v4l2_fwht_state *state = &ctx->state;
 	u8 *p_src, *p_dst;
 	int ret;
 
-	q_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 	if (ctx->is_enc)
 		p_src = vb2_plane_vaddr(&src_vb->vb2_buf, 0);
 	else
@@ -192,8 +190,10 @@ static int device_process(struct vicodec_ctx *ctx,
 			return ret;
 		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, ret);
 	} else {
+		struct vicodec_q_data *q_dst;
 		unsigned int comp_frame_size = ntohl(ctx->state.header.size);
 
+		q_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 		if (comp_frame_size > ctx->comp_max_size)
 			return -EINVAL;
 		state->info = q_dst->info;
@@ -204,11 +204,6 @@ static int device_process(struct vicodec_ctx *ctx,
 
 		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, q_dst->sizeimage);
 	}
-
-	dst_vb->sequence = q_dst->sequence++;
-	dst_vb->flags &= ~V4L2_BUF_FLAG_LAST;
-	v4l2_m2m_buf_copy_metadata(src_vb, dst_vb, !ctx->is_enc);
-
 	return 0;
 }
 
@@ -282,16 +277,22 @@ static void device_run(void *priv)
 	struct vicodec_ctx *ctx = priv;
 	struct vicodec_dev *dev = ctx->dev;
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
-	struct vicodec_q_data *q_src;
+	struct vicodec_q_data *q_src, *q_dst;
 	u32 state;
 
 	src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
 	dst_buf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 	q_src = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
+	q_dst = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 
 	state = VB2_BUF_STATE_DONE;
 	if (device_process(ctx, src_buf, dst_buf))
 		state = VB2_BUF_STATE_ERROR;
+	else
+		dst_buf->sequence = q_dst->sequence++;
+	dst_buf->flags &= ~V4L2_BUF_FLAG_LAST;
+	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, !ctx->is_enc);
+
 	ctx->last_dst_buf = dst_buf;
 
 	spin_lock(ctx->lock);
-- 
2.17.1


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

* [PATCH 9/9] media: vicodec: Add support for stateless decoder.
  2019-02-09 13:54 [PATCH 0/9] media: vicodec: add support to stateless decoder Dafna Hirschfeld
                   ` (7 preceding siblings ...)
  2019-02-09 13:54 ` [PATCH 8/9] media: vicodec: call v4l2_m2m_buf_copy_metadata also upon error Dafna Hirschfeld
@ 2019-02-09 13:54 ` Dafna Hirschfeld
  2019-02-11 10:09   ` Hans Verkuil
  8 siblings, 1 reply; 14+ messages in thread
From: Dafna Hirschfeld @ 2019-02-09 13:54 UTC (permalink / raw)
  To: linux-media; +Cc: hverkuil, helen.koike, Dafna Hirschfeld

Implement a stateless decoder for the new node.

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

diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
index 9a6ee593ae19..9fda0f9138e8 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -117,6 +117,7 @@ struct vicodec_ctx {
 	bool			is_enc;
 	bool			is_stateless;
 	spinlock_t		*lock;
+	struct media_request	*cur_req;
 
 	struct v4l2_ctrl_handler hdl;
 
@@ -136,6 +137,7 @@ struct vicodec_ctx {
 	bool			comp_has_next_frame;
 	bool			first_source_change_sent;
 	bool			source_changed;
+	bool			bad_statless_params;
 };
 
 static inline struct vicodec_ctx *file2ctx(struct file *file)
@@ -169,10 +171,38 @@ static int device_process(struct vicodec_ctx *ctx,
 	u8 *p_src, *p_dst;
 	int ret;
 
+	if (ctx->is_stateless) {
+		unsigned int comp_frame_size;
+
+		p_src = vb2_plane_vaddr(&src_vb->vb2_buf, 0);
+
+		memcpy(&state->header, p_src, sizeof(struct fwht_cframe_hdr));
+		comp_frame_size = ntohl(ctx->state.header.size);
+		if (comp_frame_size > ctx->comp_max_size)
+			return -EINVAL;
+		memcpy(state->compressed_frame,
+		       p_src + sizeof(struct fwht_cframe_hdr), comp_frame_size);
+	}
+
 	if (ctx->is_enc)
 		p_src = vb2_plane_vaddr(&src_vb->vb2_buf, 0);
 	else
 		p_src = state->compressed_frame;
+
+	if (ctx->is_stateless) {
+		struct media_request *src_req = src_vb->vb2_buf.req_obj.req;
+
+		if (!src_req) {
+			v4l2_err(&dev->v4l2_dev, "%s: request is NULL\n",
+				__func__);
+			return -EFAULT;
+		}
+		ctx->cur_req = src_req;
+		v4l2_ctrl_request_setup(src_req, &ctx->hdl);
+		ctx->cur_req = NULL;
+		if (ctx->bad_statless_params)
+			return -EINVAL;
+	}
 	p_dst = vb2_plane_vaddr(&dst_vb->vb2_buf, 0);
 	if (!p_src || !p_dst) {
 		v4l2_err(&dev->v4l2_dev,
@@ -200,7 +230,8 @@ static int device_process(struct vicodec_ctx *ctx,
 		ret = v4l2_fwht_decode(state, p_src, p_dst);
 		if (ret < 0)
 			return ret;
-		copy_cap_to_ref(p_dst, ctx->state.info, &ctx->state);
+		if (!ctx->is_stateless)
+			copy_cap_to_ref(p_dst, ctx->state.info, &ctx->state);
 
 		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, q_dst->sizeimage);
 	}
@@ -294,13 +325,21 @@ static void device_run(void *priv)
 	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, !ctx->is_enc);
 
 	ctx->last_dst_buf = dst_buf;
+	if (ctx->is_stateless) {
+		struct media_request *src_req;
+
+		src_req = src_buf->vb2_buf.req_obj.req;
+		if (src_req)
+			v4l2_ctrl_request_complete(src_req, &ctx->hdl);
+	}
+
 
 	spin_lock(ctx->lock);
 	if (!ctx->comp_has_next_frame && src_buf == ctx->last_src_buf) {
 		dst_buf->flags |= V4L2_BUF_FLAG_LAST;
 		v4l2_event_queue_fh(&ctx->fh, &eos_event);
 	}
-	if (ctx->is_enc) {
+	if (ctx->is_enc || ctx->is_stateless) {
 		src_buf->sequence = q_src->sequence++;
 		src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
 		v4l2_m2m_buf_done(src_buf, state);
@@ -452,7 +491,7 @@ static int job_ready(void *priv)
 
 	if (ctx->source_changed)
 		return 0;
-	if (ctx->is_enc || ctx->comp_has_frame)
+	if (ctx->is_stateless || ctx->is_enc || ctx->comp_has_frame)
 		return 1;
 
 restart:
@@ -1212,6 +1251,14 @@ static int vicodec_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
 	return 0;
 }
 
+static int vicodec_buf_out_validate(struct vb2_buffer *vb)
+{
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+
+	vbuf->field = V4L2_FIELD_NONE;
+	return 0;
+}
+
 static int vicodec_buf_prepare(struct vb2_buffer *vb)
 {
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -1275,10 +1322,11 @@ static void vicodec_buf_queue(struct vb2_buffer *vb)
 	}
 
 	/*
-	 * source change event is relevant only for the decoder
+	 * source change event is relevant only for the stateful decoder
 	 * in the compressed stream
 	 */
-	if (ctx->is_enc || !V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
+	if (ctx->is_stateless || ctx->is_enc ||
+	    !V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
 		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
 		return;
 	}
@@ -1326,6 +1374,8 @@ static void vicodec_return_bufs(struct vb2_queue *q, u32 state)
 			vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
 		if (vbuf == NULL)
 			return;
+		v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req,
+					   &ctx->hdl);
 		spin_lock(ctx->lock);
 		v4l2_m2m_buf_done(vbuf, state);
 		spin_unlock(ctx->lock);
@@ -1442,14 +1492,24 @@ static void vicodec_stop_streaming(struct vb2_queue *q)
 	}
 }
 
+static void vicodec_buf_request_complete(struct vb2_buffer *vb)
+{
+	struct vicodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
+
+	v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->hdl);
+}
+
+
 static const struct vb2_ops vicodec_qops = {
-	.queue_setup	 = vicodec_queue_setup,
-	.buf_prepare	 = vicodec_buf_prepare,
-	.buf_queue	 = vicodec_buf_queue,
-	.start_streaming = vicodec_start_streaming,
-	.stop_streaming  = vicodec_stop_streaming,
-	.wait_prepare	 = vb2_ops_wait_prepare,
-	.wait_finish	 = vb2_ops_wait_finish,
+	.queue_setup		= vicodec_queue_setup,
+	.buf_out_validate	= vicodec_buf_out_validate,
+	.buf_prepare		= vicodec_buf_prepare,
+	.buf_queue		= vicodec_buf_queue,
+	.buf_request_complete	= vicodec_buf_request_complete,
+	.start_streaming	= vicodec_start_streaming,
+	.stop_streaming		= vicodec_stop_streaming,
+	.wait_prepare		= vb2_ops_wait_prepare,
+	.wait_finish		= vb2_ops_wait_finish,
 };
 
 static int queue_init(void *priv, struct vb2_queue *src_vq,
@@ -1501,6 +1561,13 @@ static int vicodec_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct vicodec_ctx *ctx = container_of(ctrl->handler,
 					       struct vicodec_ctx, hdl);
+	struct media_request *src_req;
+	struct vb2_v4l2_buffer *src_buf;
+	struct vb2_buffer *ref_vb2_buf;
+	u8 *ref_buf;
+	struct v4l2_ctrl_fwht_params *params;
+	struct vb2_queue *vq_cap;
+	struct fwht_raw_frame *ref_frame;
 
 	switch (ctrl->id) {
 	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
@@ -1511,6 +1578,49 @@ static int vicodec_s_ctrl(struct v4l2_ctrl *ctrl)
 		return 0;
 	case VICODEC_CID_P_FRAME_QP:
 		ctx->state.p_frame_qp = ctrl->val;
+		return 0;
+	case VICODEC_CID_STATELESS_FWHT:
+		if (!ctx->cur_req)
+			return 0;
+		params = (struct v4l2_ctrl_fwht_params *) ctrl->p_new.p;
+		vq_cap = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+					  V4L2_BUF_TYPE_VIDEO_CAPTURE);
+		ref_frame = &ctx->state.ref_frame;
+
+		src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
+		if (!src_buf)
+			return 0;
+		src_req = src_buf->vb2_buf.req_obj.req;
+		if (src_req != ctx->cur_req)
+			return 0;
+
+		if (params->width > ctx->state.coded_width ||
+		    params->height > ctx->state.coded_height) {
+			ctx->bad_statless_params = true;
+			return 0;
+		}
+		/*
+		 * if backward_ref_ts is 0, it means there is no
+		 * ref frame, so just return
+		 */
+		if (params->backward_ref_ts == 0) {
+			ctx->state.visible_width = params->width;
+			ctx->state.visible_height = params->height;
+			return 0;
+		}
+		ref_vb2_buf = vb2_find_timestamp_buf(vq_cap,
+						     params->backward_ref_ts, 0);
+
+		if (!ref_vb2_buf) {
+			ctx->bad_statless_params = true;
+			return 0;
+		}
+
+		ref_buf = vb2_plane_vaddr(ref_vb2_buf, 0);
+		ctx->state.visible_width = params->width;
+		ctx->state.visible_height = params->height;
+		copy_cap_to_ref(ref_buf, ctx->state.info, &ctx->state);
+
 		return 0;
 	}
 	return -EINVAL;
@@ -1543,6 +1653,7 @@ static const struct v4l2_ctrl_config vicodec_ctrl_p_frame = {
 };
 
 static const struct v4l2_ctrl_config vicodec_ctrl_stateless_state = {
+	.ops		= &vicodec_ctrl_ops,
 	.id		= VICODEC_CID_STATELESS_FWHT,
 	.elem_size	= sizeof(struct v4l2_ctrl_fwht_params),
 	.name		= "FWHT-Stateless State Params",
@@ -1666,6 +1777,67 @@ static int vicodec_release(struct file *file)
 	return 0;
 }
 
+static int vicodec_request_validate(struct media_request *req)
+{
+	struct media_request_object *obj;
+	struct v4l2_ctrl_handler *parent_hdl, *hdl;
+	struct vicodec_ctx *ctx = NULL;
+	struct v4l2_ctrl *ctrl;
+	unsigned int count;
+
+	list_for_each_entry(obj, &req->objects, list) {
+		struct vb2_buffer *vb;
+
+		if (vb2_request_object_is_buffer(obj)) {
+			vb = container_of(obj, struct vb2_buffer, req_obj);
+			ctx = vb2_get_drv_priv(vb->vb2_queue);
+
+			break;
+		}
+	}
+
+	if (!ctx) {
+		pr_err("No buffer was provided with the request\n");
+		return -ENOENT;
+	}
+
+	count = vb2_request_buffer_cnt(req);
+	if (!count) {
+		v4l2_info(&ctx->dev->v4l2_dev,
+			  "No buffer was provided with the request\n");
+		return -ENOENT;
+	} else if (count > 1) {
+		v4l2_info(&ctx->dev->v4l2_dev,
+			  "More than one buffer was provided with the request\n");
+		return -EINVAL;
+	}
+
+	parent_hdl = &ctx->hdl;
+
+	hdl = v4l2_ctrl_request_hdl_find(req, parent_hdl);
+	if (!hdl) {
+		v4l2_info(&ctx->dev->v4l2_dev, "Missing codec control(s)\n");
+		return -ENOENT;
+	}
+	ctrl = v4l2_ctrl_request_hdl_ctrl_find(hdl,
+					       vicodec_ctrl_stateless_state.id);
+	if (!ctrl) {
+		v4l2_info(&ctx->dev->v4l2_dev,
+			  "Missing required codec control\n");
+		return -ENOENT;
+	}
+
+	if (ctrl->elem_size != vicodec_ctrl_stateless_state.elem_size) {
+		v4l2_info(&ctx->dev->v4l2_dev,
+			  "wrong size, got %u expected %u\n",
+			  ctrl->elem_size,
+			  vicodec_ctrl_stateless_state.elem_size);
+		return -ENOENT;
+
+	}
+	return vb2_request_validate(req);
+}
+
 static const struct v4l2_file_operations vicodec_fops = {
 	.owner		= THIS_MODULE,
 	.open		= vicodec_open,
@@ -1684,6 +1856,11 @@ static const struct video_device vicodec_videodev = {
 	.release	= video_device_release_empty,
 };
 
+static const struct media_device_ops vicodec_m2m_media_ops = {
+	.req_validate	= vicodec_request_validate,
+	.req_queue	= v4l2_m2m_request_queue,
+};
+
 static const struct v4l2_m2m_ops m2m_ops = {
 	.device_run	= device_run,
 	.job_ready	= job_ready,
@@ -1750,6 +1927,7 @@ static int vicodec_probe(struct platform_device *pdev)
 	strscpy(dev->mdev.bus_info, "platform:vicodec",
 		sizeof(dev->mdev.bus_info));
 	media_device_init(&dev->mdev);
+	dev->mdev.ops = &vicodec_m2m_media_ops;
 	dev->v4l2_dev.mdev = &dev->mdev;
 #endif
 
-- 
2.17.1


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

* Re: [PATCH 4/9] media: vicodec: add struct for encoder/decoder instance
  2019-02-09 13:54 ` [PATCH 4/9] media: vicodec: add struct for encoder/decoder instance Dafna Hirschfeld
@ 2019-02-11  8:31   ` Hans Verkuil
  0 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2019-02-11  8:31 UTC (permalink / raw)
  To: Dafna Hirschfeld, linux-media; +Cc: helen.koike

On 2/9/19 2:54 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 | 194 +++++++++---------
>  1 file changed, 92 insertions(+), 102 deletions(-)
> 
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
> index 335a931fdf02..e5987229c5a3 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 stateful_enc;
> +	struct vicodec_dev_instance stateful_dec;
>  #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 {
> @@ -312,9 +312,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->stateful_enc.m2m_dev, ctx->fh.m2m_ctx);
>  	else
> -		v4l2_m2m_job_finish(dev->dec_dev, ctx->fh.m2m_ctx);
> +		v4l2_m2m_job_finish(dev->stateful_dec.m2m_dev, ctx->fh.m2m_ctx);
>  }
>  
>  static void job_remove_src_buf(struct vicodec_ctx *ctx, u32 state)
> @@ -1457,9 +1457,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->stateful_enc.mutex :
> +		&ctx->dev->stateful_dec.mutex;
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
>  		return ret;
> @@ -1547,7 +1546,7 @@ static int vicodec_open(struct file *file)
>  		goto open_unlock;
>  	}
>  
> -	if (vfd == &dev->enc_vfd)
> +	if (vfd == &dev->stateful_enc.vfd)
>  		ctx->is_enc = true;
>  
>  	v4l2_fh_init(&ctx->fh, video_devdata(file));
> @@ -1595,13 +1594,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->stateful_enc.m2m_dev,
> +						    ctx, &queue_init);
> +		ctx->lock = &dev->stateful_enc.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->stateful_dec.m2m_dev,
> +						    ctx, &queue_init);
> +		ctx->lock = &dev->stateful_dec.lock;
>  	}
>  
>  	if (IS_ERR(ctx->fh.m2m_ctx)) {
> @@ -1659,19 +1658,57 @@ 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;
> +
> +	spin_lock_init(&dev_instance->lock);
> +	mutex_init(&dev_instance->mutex);
> +	dev_instance->m2m_dev = v4l2_m2m_init(&m2m_ops);
> +	if (IS_ERR(dev_instance->m2m_dev)) {
> +		v4l2_err(&dev->v4l2_dev, "Failed to init vicodec enc device\n");
> +		return PTR_ERR(dev_instance->m2m_dev);
> +	}
> +
> +	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);
> +		v4l2_m2m_release(dev_instance->m2m_dev);
> +		return ret;
> +	}
> +	v4l2_info(&dev->v4l2_dev, "Device '%s' registered as /dev/video%d\n",
> +		  name, vfd->num);
> +	return 0;
> +}
> +
>  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);
> -
>  	ret = v4l2_device_register(&pdev->dev, &dev->v4l2_dev);
>  	if (ret)
>  		return ret;
> @@ -1685,100 +1722,53 @@ 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);
> -
>  	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);
> +	if (register_instance(dev, &dev->stateful_enc,
> +			      "videdev-enc", true))

I'd rename videdev-enc to "stateful-encoder".

>  		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);
> -		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");
> -		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");
> -		goto unreg_enc;
> -	}
> -	v4l2_info(&dev->v4l2_dev,
> -			"Device registered as /dev/video%d\n", vfd->num);
> +	if (register_instance(dev, &dev->stateful_dec,
> +			      "videdev-statefull-dec", false))

And this to "stateful-decoder".

> +		goto unreg_sf_enc;
>  
>  #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->stateful_enc.m2m_dev,
> +						 &dev->stateful_enc.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->stateful_dec.m2m_dev,
> +						 &dev->stateful_dec.vfd,
> +						 MEDIA_ENT_F_PROC_VIDEO_DECODER);
>  	if (ret) {
> -		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller\n");
> -		goto unreg_m2m_enc_mc;
> +		v4l2_err(&dev->v4l2_dev, "Failed to init mem2mem media controller for dec\n");
> +		goto unreg_m2m_sf_enc_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_sf_dec_mc;
>  	}
>  #endif
>  	return 0;
>  
>  #ifdef CONFIG_MEDIA_CONTROLLER
> -unreg_m2m_dec_mc:
> -	v4l2_m2m_unregister_media_controller(dev->dec_dev);
> -unreg_m2m_enc_mc:
> -	v4l2_m2m_unregister_media_controller(dev->enc_dev);
> +unreg_m2m_sf_dec_mc:
> +	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
> +unreg_m2m_sf_enc_mc:
> +	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
>  unreg_m2m:
> -	video_unregister_device(&dev->dec_vfd);
> +	video_unregister_device(&dev->stateful_dec.vfd);
> +	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
>  #endif
> -unreg_enc:
> -	video_unregister_device(&dev->enc_vfd);
> -err_dec_m2m:
> -	v4l2_m2m_release(dev->dec_dev);
> -err_enc_m2m:
> -	v4l2_m2m_release(dev->enc_dev);
> +unreg_sf_enc:
> +	video_unregister_device(&dev->stateful_enc.vfd);
> +	v4l2_m2m_release(dev->stateful_enc.m2m_dev);
>  unreg_dev:
>  	v4l2_device_unregister(&dev->v4l2_dev);
>  
> @@ -1793,15 +1783,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->stateful_enc.m2m_dev);
> +	v4l2_m2m_unregister_media_controller(dev->stateful_dec.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->stateful_enc.m2m_dev);
> +	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
> +	video_unregister_device(&dev->stateful_enc.vfd);
> +	video_unregister_device(&dev->stateful_dec.vfd);
>  	v4l2_device_unregister(&dev->v4l2_dev);
>  
>  	return 0;
> 

Regards,

	Hans

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

* Re: [PATCH 6/9] media: vicodec: Register another node for stateless decoder
  2019-02-09 13:54 ` [PATCH 6/9] media: vicodec: Register another node for stateless decoder Dafna Hirschfeld
@ 2019-02-11  8:31   ` Hans Verkuil
  0 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2019-02-11  8:31 UTC (permalink / raw)
  To: Dafna Hirschfeld, linux-media; +Cc: helen.koike

On 2/9/19 2:54 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 | 45 +++++++++++++++++--
>  1 file changed, 41 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
> index 95276c09cb9a..324ce566478e 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 stateful_enc;
>  	struct vicodec_dev_instance stateful_dec;
> +	struct vicodec_dev_instance stateless_dec;
>  #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;
>  	spinlock_t		*lock;
>  
>  	struct v4l2_ctrl_handler hdl;
> @@ -317,6 +319,9 @@ static void device_run(void *priv)
>  
>  	if (ctx->is_enc)
>  		v4l2_m2m_job_finish(dev->stateful_enc.m2m_dev, ctx->fh.m2m_ctx);
> +	else if (ctx->is_stateless)
> +		v4l2_m2m_job_finish(dev->stateless_dec.m2m_dev,
> +				    ctx->fh.m2m_ctx);
>  	else
>  		v4l2_m2m_job_finish(dev->stateful_dec.m2m_dev, ctx->fh.m2m_ctx);
>  }
> @@ -1461,8 +1466,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->stateful_enc.mutex :
> -		&ctx->dev->stateful_dec.mutex;
> +	if (ctx->is_enc)
> +		src_vq->lock = &ctx->dev->stateful_enc.mutex;
> +	else if (ctx->is_stateless)
> +		src_vq->lock = &ctx->dev->stateless_dec.mutex;
> +	else
> +		src_vq->lock = &ctx->dev->stateful_dec.mutex;
> +	src_vq->supports_requests = ctx->is_stateless ? true : false;
>  	ret = vb2_queue_init(src_vq);
>  	if (ret)
>  		return ret;
> @@ -1560,6 +1570,8 @@ static int vicodec_open(struct file *file)
>  
>  	if (vfd == &dev->stateful_enc.vfd)
>  		ctx->is_enc = true;
> +	else if (vfd == &dev->stateless_dec.vfd)
> +		ctx->is_stateless = true;
>  
>  	v4l2_fh_init(&ctx->fh, video_devdata(file));
>  	file->private_data = &ctx->fh;
> @@ -1570,6 +1582,8 @@ 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);
> +	if (ctx->is_stateless)
> +		v4l2_ctrl_new_custom(hdl, &vicodec_ctrl_stateless_state, NULL);
>  	if (hdl->error) {
>  		rc = hdl->error;
>  		v4l2_ctrl_handler_free(hdl);
> @@ -1609,6 +1623,10 @@ static int vicodec_open(struct file *file)
>  		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateful_enc.m2m_dev,
>  						    ctx, &queue_init);
>  		ctx->lock = &dev->stateful_enc.lock;
> +	} else if (ctx->is_stateless) {
> +		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateless_dec.m2m_dev,
> +						    ctx, &queue_init);
> +		ctx->lock = &dev->stateless_dec.lock;
>  	} else {
>  		ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->stateful_dec.m2m_dev,
>  						    ctx, &queue_init);
> @@ -1744,6 +1762,10 @@ static int vicodec_probe(struct platform_device *pdev)
>  			      "videdev-statefull-dec", false))
>  		goto unreg_sf_enc;
>  
> +	if (register_instance(dev, &dev->stateless_dec,
> +			      "videdev-stateless-dec", false))

This becomes: "stateless-decoder"

Regards,

	Hans

> +		goto unreg_sf_dec;
> +
>  #ifdef CONFIG_MEDIA_CONTROLLER
>  	ret = v4l2_m2m_register_media_controller(dev->stateful_enc.m2m_dev,
>  						 &dev->stateful_enc.vfd,
> @@ -1761,23 +1783,36 @@ static int vicodec_probe(struct platform_device *pdev)
>  		goto unreg_m2m_sf_enc_mc;
>  	}
>  
> +	ret = v4l2_m2m_register_media_controller(dev->stateless_dec.m2m_dev,
> +						 &dev->stateless_dec.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_sf_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_sf_dec_mc;
> +		goto unreg_m2m_sl_dec_mc;
>  	}
>  #endif
>  	return 0;
>  
>  #ifdef CONFIG_MEDIA_CONTROLLER
> +unreg_m2m_sl_dec_mc:
> +	v4l2_m2m_unregister_media_controller(dev->stateless_dec.m2m_dev);
>  unreg_m2m_sf_dec_mc:
>  	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
>  unreg_m2m_sf_enc_mc:
>  	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
>  unreg_m2m:
> +	video_unregister_device(&dev->stateless_dec.vfd);
> +	v4l2_m2m_release(dev->stateless_dec.m2m_dev);
> +#endif
> +unreg_sf_dec:
>  	video_unregister_device(&dev->stateful_dec.vfd);
>  	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
> -#endif
>  unreg_sf_enc:
>  	video_unregister_device(&dev->stateful_enc.vfd);
>  	v4l2_m2m_release(dev->stateful_enc.m2m_dev);
> @@ -1797,6 +1832,7 @@ static int vicodec_remove(struct platform_device *pdev)
>  	media_device_unregister(&dev->mdev);
>  	v4l2_m2m_unregister_media_controller(dev->stateful_enc.m2m_dev);
>  	v4l2_m2m_unregister_media_controller(dev->stateful_dec.m2m_dev);
> +	v4l2_m2m_unregister_media_controller(dev->stateless_dec.m2m_dev);
>  	media_device_cleanup(&dev->mdev);
>  #endif
>  
> @@ -1804,6 +1840,7 @@ static int vicodec_remove(struct platform_device *pdev)
>  	v4l2_m2m_release(dev->stateful_dec.m2m_dev);
>  	video_unregister_device(&dev->stateful_enc.vfd);
>  	video_unregister_device(&dev->stateful_dec.vfd);
> +	video_unregister_device(&dev->stateless_dec.vfd);
>  	v4l2_device_unregister(&dev->v4l2_dev);
>  
>  	return 0;
> 


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

* Re: [PATCH 9/9] media: vicodec: Add support for stateless decoder.
  2019-02-09 13:54 ` [PATCH 9/9] media: vicodec: Add support for stateless decoder Dafna Hirschfeld
@ 2019-02-11 10:09   ` Hans Verkuil
  0 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2019-02-11 10:09 UTC (permalink / raw)
  To: Dafna Hirschfeld, linux-media; +Cc: helen.koike

On 2/9/19 2:54 PM, Dafna Hirschfeld wrote:
> Implement a stateless decoder for the new node.
> 
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---
>  drivers/media/platform/vicodec/vicodec-core.c | 202 ++++++++++++++++--
>  1 file changed, 190 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
> index 9a6ee593ae19..9fda0f9138e8 100644
> --- a/drivers/media/platform/vicodec/vicodec-core.c
> +++ b/drivers/media/platform/vicodec/vicodec-core.c
> @@ -117,6 +117,7 @@ struct vicodec_ctx {
>  	bool			is_enc;
>  	bool			is_stateless;
>  	spinlock_t		*lock;
> +	struct media_request	*cur_req;
>  
>  	struct v4l2_ctrl_handler hdl;
>  
> @@ -136,6 +137,7 @@ struct vicodec_ctx {
>  	bool			comp_has_next_frame;
>  	bool			first_source_change_sent;
>  	bool			source_changed;
> +	bool			bad_statless_params;

typo: bad_stateless_params

>  };
>  
>  static inline struct vicodec_ctx *file2ctx(struct file *file)
> @@ -169,10 +171,38 @@ static int device_process(struct vicodec_ctx *ctx,
>  	u8 *p_src, *p_dst;
>  	int ret;
>  
> +	if (ctx->is_stateless) {
> +		unsigned int comp_frame_size;
> +
> +		p_src = vb2_plane_vaddr(&src_vb->vb2_buf, 0);
> +
> +		memcpy(&state->header, p_src, sizeof(struct fwht_cframe_hdr));

This isn't right: for the stateless codec the header is no longer part of the
buffer, instead it is passed via the v4l2_ctrl_fwht_params control.

> +		comp_frame_size = ntohl(ctx->state.header.size);
> +		if (comp_frame_size > ctx->comp_max_size)
> +			return -EINVAL;
> +		memcpy(state->compressed_frame,
> +		       p_src + sizeof(struct fwht_cframe_hdr), comp_frame_size);
> +	}
> +
>  	if (ctx->is_enc)
>  		p_src = vb2_plane_vaddr(&src_vb->vb2_buf, 0);
>  	else
>  		p_src = state->compressed_frame;
> +
> +	if (ctx->is_stateless) {
> +		struct media_request *src_req = src_vb->vb2_buf.req_obj.req;
> +
> +		if (!src_req) {
> +			v4l2_err(&dev->v4l2_dev, "%s: request is NULL\n",
> +				__func__);
> +			return -EFAULT;

Actually, this test shouldn't be necessary, but we're missing a 'requires_requests'
bitfield in struct vb2_queue. The current 'supports_requests' just says that
requests are supported, but it is not required. But for stateless codecs this is
actually a requirement and not an option.

I've posted a small patch series for this.

> +		}
> +		ctx->cur_req = src_req;
> +		v4l2_ctrl_request_setup(src_req, &ctx->hdl);
> +		ctx->cur_req = NULL;
> +		if (ctx->bad_statless_params)
> +			return -EINVAL;
> +	}
>  	p_dst = vb2_plane_vaddr(&dst_vb->vb2_buf, 0);
>  	if (!p_src || !p_dst) {
>  		v4l2_err(&dev->v4l2_dev,
> @@ -200,7 +230,8 @@ static int device_process(struct vicodec_ctx *ctx,
>  		ret = v4l2_fwht_decode(state, p_src, p_dst);
>  		if (ret < 0)
>  			return ret;
> -		copy_cap_to_ref(p_dst, ctx->state.info, &ctx->state);
> +		if (!ctx->is_stateless)
> +			copy_cap_to_ref(p_dst, ctx->state.info, &ctx->state);
>  
>  		vb2_set_plane_payload(&dst_vb->vb2_buf, 0, q_dst->sizeimage);
>  	}
> @@ -294,13 +325,21 @@ static void device_run(void *priv)
>  	v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, !ctx->is_enc);
>  
>  	ctx->last_dst_buf = dst_buf;
> +	if (ctx->is_stateless) {
> +		struct media_request *src_req;
> +
> +		src_req = src_buf->vb2_buf.req_obj.req;
> +		if (src_req)
> +			v4l2_ctrl_request_complete(src_req, &ctx->hdl);
> +	}
> +
>  
>  	spin_lock(ctx->lock);
>  	if (!ctx->comp_has_next_frame && src_buf == ctx->last_src_buf) {
>  		dst_buf->flags |= V4L2_BUF_FLAG_LAST;
>  		v4l2_event_queue_fh(&ctx->fh, &eos_event);
>  	}
> -	if (ctx->is_enc) {
> +	if (ctx->is_enc || ctx->is_stateless) {
>  		src_buf->sequence = q_src->sequence++;
>  		src_buf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
>  		v4l2_m2m_buf_done(src_buf, state);
> @@ -452,7 +491,7 @@ static int job_ready(void *priv)
>  
>  	if (ctx->source_changed)
>  		return 0;
> -	if (ctx->is_enc || ctx->comp_has_frame)
> +	if (ctx->is_stateless || ctx->is_enc || ctx->comp_has_frame)
>  		return 1;
>  
>  restart:
> @@ -1212,6 +1251,14 @@ static int vicodec_queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
>  	return 0;
>  }
>  
> +static int vicodec_buf_out_validate(struct vb2_buffer *vb)
> +{
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +
> +	vbuf->field = V4L2_FIELD_NONE;
> +	return 0;
> +}
> +
>  static int vicodec_buf_prepare(struct vb2_buffer *vb)
>  {
>  	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> @@ -1275,10 +1322,11 @@ static void vicodec_buf_queue(struct vb2_buffer *vb)
>  	}
>  
>  	/*
> -	 * source change event is relevant only for the decoder
> +	 * source change event is relevant only for the stateful decoder
>  	 * in the compressed stream
>  	 */
> -	if (ctx->is_enc || !V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
> +	if (ctx->is_stateless || ctx->is_enc ||
> +	    !V4L2_TYPE_IS_OUTPUT(vb->vb2_queue->type)) {
>  		v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf);
>  		return;
>  	}
> @@ -1326,6 +1374,8 @@ static void vicodec_return_bufs(struct vb2_queue *q, u32 state)
>  			vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>  		if (vbuf == NULL)
>  			return;
> +		v4l2_ctrl_request_complete(vbuf->vb2_buf.req_obj.req,
> +					   &ctx->hdl);
>  		spin_lock(ctx->lock);
>  		v4l2_m2m_buf_done(vbuf, state);
>  		spin_unlock(ctx->lock);
> @@ -1442,14 +1492,24 @@ static void vicodec_stop_streaming(struct vb2_queue *q)
>  	}
>  }
>  
> +static void vicodec_buf_request_complete(struct vb2_buffer *vb)
> +{
> +	struct vicodec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
> +
> +	v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->hdl);
> +}
> +
> +
>  static const struct vb2_ops vicodec_qops = {
> -	.queue_setup	 = vicodec_queue_setup,
> -	.buf_prepare	 = vicodec_buf_prepare,
> -	.buf_queue	 = vicodec_buf_queue,
> -	.start_streaming = vicodec_start_streaming,
> -	.stop_streaming  = vicodec_stop_streaming,
> -	.wait_prepare	 = vb2_ops_wait_prepare,
> -	.wait_finish	 = vb2_ops_wait_finish,
> +	.queue_setup		= vicodec_queue_setup,
> +	.buf_out_validate	= vicodec_buf_out_validate,
> +	.buf_prepare		= vicodec_buf_prepare,
> +	.buf_queue		= vicodec_buf_queue,
> +	.buf_request_complete	= vicodec_buf_request_complete,
> +	.start_streaming	= vicodec_start_streaming,
> +	.stop_streaming		= vicodec_stop_streaming,
> +	.wait_prepare		= vb2_ops_wait_prepare,
> +	.wait_finish		= vb2_ops_wait_finish,
>  };
>  
>  static int queue_init(void *priv, struct vb2_queue *src_vq,
> @@ -1501,6 +1561,13 @@ static int vicodec_s_ctrl(struct v4l2_ctrl *ctrl)
>  {
>  	struct vicodec_ctx *ctx = container_of(ctrl->handler,
>  					       struct vicodec_ctx, hdl);
> +	struct media_request *src_req;
> +	struct vb2_v4l2_buffer *src_buf;
> +	struct vb2_buffer *ref_vb2_buf;
> +	u8 *ref_buf;
> +	struct v4l2_ctrl_fwht_params *params;
> +	struct vb2_queue *vq_cap;
> +	struct fwht_raw_frame *ref_frame;
>  
>  	switch (ctrl->id) {
>  	case V4L2_CID_MPEG_VIDEO_GOP_SIZE:
> @@ -1511,6 +1578,49 @@ static int vicodec_s_ctrl(struct v4l2_ctrl *ctrl)
>  		return 0;
>  	case VICODEC_CID_P_FRAME_QP:
>  		ctx->state.p_frame_qp = ctrl->val;
> +		return 0;
> +	case VICODEC_CID_STATELESS_FWHT:
> +		if (!ctx->cur_req)
> +			return 0;

The cur_req field is a hack and should be removed. The core problem is that
this control only makes sense if it is part of a request, and it should never
be set directly.

I'm preparing a patch series for this as well.

> +		params = (struct v4l2_ctrl_fwht_params *) ctrl->p_new.p;
> +		vq_cap = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
> +					  V4L2_BUF_TYPE_VIDEO_CAPTURE);
> +		ref_frame = &ctx->state.ref_frame;
> +
> +		src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> +		if (!src_buf)
> +			return 0;
> +		src_req = src_buf->vb2_buf.req_obj.req;
> +		if (src_req != ctx->cur_req)
> +			return 0;

This can't happen.

> +
> +		if (params->width > ctx->state.coded_width ||
> +		    params->height > ctx->state.coded_height) {
> +			ctx->bad_statless_params = true;
> +			return 0;

This doesn't belong here. You really want to validate the control data
when the request is validated (i.e. when userspace queues the request),
not when the request is actually processed.

Luckily you can do that by implementing a the try_ctrl control op.
Basically the same as this s_ctrl op, but it is called to validate the
control at the time the request is validated.

> +		}
> +		/*
> +		 * if backward_ref_ts is 0, it means there is no
> +		 * ref frame, so just return
> +		 */
> +		if (params->backward_ref_ts == 0) {

A timestamp of 0 is a valid timestamp, so this check doesn't work.

What you want is to check if this is an I frame:

		if (!(param->flags & FWHT_FRAME_PCODED))

> +			ctx->state.visible_width = params->width;
> +			ctx->state.visible_height = params->height;
> +			return 0;
> +		}
> +		ref_vb2_buf = vb2_find_timestamp_buf(vq_cap,
> +						     params->backward_ref_ts, 0);
> +
> +		if (!ref_vb2_buf) {
> +			ctx->bad_statless_params = true;
> +			return 0;
> +		}
> +
> +		ref_buf = vb2_plane_vaddr(ref_vb2_buf, 0);
> +		ctx->state.visible_width = params->width;
> +		ctx->state.visible_height = params->height;
> +		copy_cap_to_ref(ref_buf, ctx->state.info, &ctx->state);

I actually wouldn't do this in s_ctrl. This really belongs after the
v4l2_ctrl_request_setup() call.

There really isn't much that you need to do in s_ctrl. All it really needs
to do is fill in state.header based on the specified control values.

The code in device_process can check the validity of the backward_ref_ts
and do the right error handling if that doesn't point to a valid buffer.

> +
>  		return 0;
>  	}
>  	return -EINVAL;
> @@ -1543,6 +1653,7 @@ static const struct v4l2_ctrl_config vicodec_ctrl_p_frame = {
>  };
>  
>  static const struct v4l2_ctrl_config vicodec_ctrl_stateless_state = {
> +	.ops		= &vicodec_ctrl_ops,
>  	.id		= VICODEC_CID_STATELESS_FWHT,
>  	.elem_size	= sizeof(struct v4l2_ctrl_fwht_params),
>  	.name		= "FWHT-Stateless State Params",
> @@ -1666,6 +1777,67 @@ static int vicodec_release(struct file *file)
>  	return 0;
>  }
>  
> +static int vicodec_request_validate(struct media_request *req)
> +{
> +	struct media_request_object *obj;
> +	struct v4l2_ctrl_handler *parent_hdl, *hdl;
> +	struct vicodec_ctx *ctx = NULL;
> +	struct v4l2_ctrl *ctrl;
> +	unsigned int count;
> +
> +	list_for_each_entry(obj, &req->objects, list) {
> +		struct vb2_buffer *vb;
> +
> +		if (vb2_request_object_is_buffer(obj)) {
> +			vb = container_of(obj, struct vb2_buffer, req_obj);
> +			ctx = vb2_get_drv_priv(vb->vb2_queue);
> +
> +			break;
> +		}
> +	}
> +
> +	if (!ctx) {
> +		pr_err("No buffer was provided with the request\n");
> +		return -ENOENT;
> +	}
> +
> +	count = vb2_request_buffer_cnt(req);
> +	if (!count) {
> +		v4l2_info(&ctx->dev->v4l2_dev,
> +			  "No buffer was provided with the request\n");
> +		return -ENOENT;
> +	} else if (count > 1) {
> +		v4l2_info(&ctx->dev->v4l2_dev,
> +			  "More than one buffer was provided with the request\n");
> +		return -EINVAL;
> +	}
> +
> +	parent_hdl = &ctx->hdl;
> +
> +	hdl = v4l2_ctrl_request_hdl_find(req, parent_hdl);
> +	if (!hdl) {
> +		v4l2_info(&ctx->dev->v4l2_dev, "Missing codec control(s)\n");

No need for (s) since we only have a single control.

> +		return -ENOENT;
> +	}
> +	ctrl = v4l2_ctrl_request_hdl_ctrl_find(hdl,
> +					       vicodec_ctrl_stateless_state.id);
> +	if (!ctrl) {
> +		v4l2_info(&ctx->dev->v4l2_dev,
> +			  "Missing required codec control\n");
> +		return -ENOENT;
> +	}
> +
> +	if (ctrl->elem_size != vicodec_ctrl_stateless_state.elem_size) {
> +		v4l2_info(&ctx->dev->v4l2_dev,
> +			  "wrong size, got %u expected %u\n",
> +			  ctrl->elem_size,
> +			  vicodec_ctrl_stateless_state.elem_size);
> +		return -ENOENT;

No need for this check, this can't happen.

> +
> +	}
> +	return vb2_request_validate(req);
> +}
> +
>  static const struct v4l2_file_operations vicodec_fops = {
>  	.owner		= THIS_MODULE,
>  	.open		= vicodec_open,
> @@ -1684,6 +1856,11 @@ static const struct video_device vicodec_videodev = {
>  	.release	= video_device_release_empty,
>  };
>  
> +static const struct media_device_ops vicodec_m2m_media_ops = {
> +	.req_validate	= vicodec_request_validate,
> +	.req_queue	= v4l2_m2m_request_queue,
> +};
> +
>  static const struct v4l2_m2m_ops m2m_ops = {
>  	.device_run	= device_run,
>  	.job_ready	= job_ready,
> @@ -1750,6 +1927,7 @@ static int vicodec_probe(struct platform_device *pdev)
>  	strscpy(dev->mdev.bus_info, "platform:vicodec",
>  		sizeof(dev->mdev.bus_info));
>  	media_device_init(&dev->mdev);
> +	dev->mdev.ops = &vicodec_m2m_media_ops;
>  	dev->v4l2_dev.mdev = &dev->mdev;
>  #endif
>  
> 

Regards,

	Hans

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

* Re: [PATCH 7/9] media: vb2: Add func that return buffer by timestamp
  2019-02-09 13:54 ` [PATCH 7/9] media: vb2: Add func that return buffer by timestamp Dafna Hirschfeld
@ 2019-02-11 10:11   ` Hans Verkuil
  0 siblings, 0 replies; 14+ messages in thread
From: Hans Verkuil @ 2019-02-11 10:11 UTC (permalink / raw)
  To: Dafna Hirschfeld, linux-media; +Cc: helen.koike

On 2/9/19 2:54 PM, Dafna Hirschfeld wrote:
> Add the function 'vb2_find_timestamp_buf' that returns
> the vb2 buffer that matches the given timestamp
> 
> Signed-off-by: Dafna Hirschfeld <dafna3@gmail.com>
> ---
>  drivers/media/common/videobuf2/videobuf2-v4l2.c | 14 ++++++++++++++
>  include/media/videobuf2-v4l2.h                  |  3 +++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 3aeaea3af42a..47c245a76561 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -598,6 +598,20 @@ static const struct vb2_buf_ops v4l2_buf_ops = {
>  	.copy_timestamp		= __copy_timestamp,
>  };
>  
> +struct vb2_buffer *vb2_find_timestamp_buf(const struct vb2_queue *q,
> +					  u64 timestamp,
> +					  unsigned int start_idx)
> +{
> +	unsigned int i;
> +
> +	for (i = start_idx; i < q->num_buffers; i++) {
> +		if (q->bufs[i]->timestamp == timestamp)
> +			return q->bufs[i];
> +	}
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(vb2_find_timestamp_buf);

There is no need for this function, I don't think it adds anything useful IMHO.

Regards,

	Hans

> +
>  int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
>  		       unsigned int start_idx)
>  {
> diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
> index 8a10889dc2fd..7fc2a235064e 100644
> --- a/include/media/videobuf2-v4l2.h
> +++ b/include/media/videobuf2-v4l2.h
> @@ -71,6 +71,9 @@ struct vb2_v4l2_buffer {
>  int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
>  		       unsigned int start_idx);
>  
> +struct vb2_buffer *vb2_find_timestamp_buf(const struct vb2_queue *q,
> +					  u64 timestamp,
> +					  unsigned int start_idx);
>  int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b);
>  
>  /**
> 


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

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-09 13:54 [PATCH 0/9] media: vicodec: add support to stateless decoder Dafna Hirschfeld
2019-02-09 13:54 ` [PATCH 1/9] media: vicodec: Move raw frame preparation code to a function Dafna Hirschfeld
2019-02-09 13:54 ` [PATCH 2/9] media: vicodec: add field 'buf' to fwht_raw_frame Dafna Hirschfeld
2019-02-09 13:54 ` [PATCH 3/9] media: vicodec: keep the ref frame according to the format in decoder Dafna Hirschfeld
2019-02-09 13:54 ` [PATCH 4/9] media: vicodec: add struct for encoder/decoder instance Dafna Hirschfeld
2019-02-11  8:31   ` Hans Verkuil
2019-02-09 13:54 ` [PATCH 5/9] media: vicodec: Introducing stateless fwht defs and structs Dafna Hirschfeld
2019-02-09 13:54 ` [PATCH 6/9] media: vicodec: Register another node for stateless decoder Dafna Hirschfeld
2019-02-11  8:31   ` Hans Verkuil
2019-02-09 13:54 ` [PATCH 7/9] media: vb2: Add func that return buffer by timestamp Dafna Hirschfeld
2019-02-11 10:11   ` Hans Verkuil
2019-02-09 13:54 ` [PATCH 8/9] media: vicodec: call v4l2_m2m_buf_copy_metadata also upon error Dafna Hirschfeld
2019-02-09 13:54 ` [PATCH 9/9] media: vicodec: Add support for stateless decoder Dafna Hirschfeld
2019-02-11 10:09   ` Hans Verkuil

Linux-Media Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-media/0 linux-media/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-media linux-media/ https://lore.kernel.org/linux-media \
		linux-media@vger.kernel.org linux-media@archiver.kernel.org
	public-inbox-index linux-media


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-media


AGPL code for this site: git clone https://public-inbox.org/ public-inbox