linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support
@ 2022-11-02 18:07 Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 01/11] media: cedrus: remove superfluous call Jernej Skrabec
                   ` (11 more replies)
  0 siblings, 12 replies; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:07 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

While my first intention was to just add 10-bit HEVC handling, I noticed
a few format handling issues and a bit of redundancy in some cases. Final
result is that driver now sticks to stateless decoder rules better.

Format handling improvements:
1. Default format selection is now based on HW capabilities. Before, MPEG2
   was hardcoded but some Cedrus variants don't actually support it.
2. Controls are registered only if related codec is supported by HW.
3. Untiled output format is preferred, if supported, over tiled one. All
   display engine cores support untiled format, but only first generation
   supports tiled one.

I hope this makes Cedrus eligible for destaging.

Best regards,
Jernej

Changes from v1:
- collected acks, except for patch 5, which was changed
- use cedrus_is_capable() for cedrus_find_format() too (patch 4)
- tightly pack control pointers in ctx->ctrls[] (patch 5)

Jernej Skrabec (11):
  media: cedrus: remove superfluous call
  media: cedrus: Add format reset helpers
  media: cedrus: use helper to set default formats
  media: cedrus: Add helper for checking capabilities
  media: cedrus: Filter controls based on capability
  media: cedrus: set codec ops immediately
  media: cedrus: Remove cedrus_codec enum
  media: cedrus: prefer untiled capture format
  media: cedrus: initialize controls a bit later
  media: cedrus: Adjust buffer size based on control values
  media: cedrus: h265: Support decoding 10-bit frames

 drivers/staging/media/sunxi/cedrus/cedrus.c   | 102 +++++----
 drivers/staging/media/sunxi/cedrus/cedrus.h   |  22 +-
 .../staging/media/sunxi/cedrus/cedrus_dec.c   |   4 +-
 .../staging/media/sunxi/cedrus/cedrus_h264.c  |   2 +-
 .../staging/media/sunxi/cedrus/cedrus_h265.c  |  37 +++-
 .../staging/media/sunxi/cedrus/cedrus_hw.c    |  18 +-
 .../staging/media/sunxi/cedrus/cedrus_hw.h    |   2 +-
 .../staging/media/sunxi/cedrus/cedrus_mpeg2.c |   2 +-
 .../staging/media/sunxi/cedrus/cedrus_regs.h  |  16 ++
 .../staging/media/sunxi/cedrus/cedrus_video.c | 200 ++++++++++--------
 .../staging/media/sunxi/cedrus/cedrus_video.h |   2 +
 .../staging/media/sunxi/cedrus/cedrus_vp8.c   |   2 +-
 12 files changed, 244 insertions(+), 165 deletions(-)

--
2.38.1


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

* [PATCH v2 01/11] media: cedrus: remove superfluous call
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
@ 2022-11-02 18:08 ` Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 02/11] media: cedrus: Add format reset helpers Jernej Skrabec
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:08 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

cedrus_try_fmt_vid_out() is called two times inside
cedrus_s_fmt_vid_out(), but nothing changes between calls which would
influence output format. Remove first call, which was added later.

Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus_video.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index 33726175d980..1c3c1d080d31 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -309,10 +309,6 @@ static int cedrus_s_fmt_vid_out(struct file *file, void *priv,
 	struct vb2_queue *peer_vq;
 	int ret;
 
-	ret = cedrus_try_fmt_vid_out(file, priv, f);
-	if (ret)
-		return ret;
-
 	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
 	/*
 	 * In order to support dynamic resolution change,
-- 
2.38.1


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

* [PATCH v2 02/11] media: cedrus: Add format reset helpers
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 01/11] media: cedrus: remove superfluous call Jernej Skrabec
@ 2022-11-02 18:08 ` Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 03/11] media: cedrus: use helper to set default formats Jernej Skrabec
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:08 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

By re-arranging try format handlers and set out format handler, we can
easily implement reset out/cap format helpers.

There is only one subtle, but important functional change. Capture
pixel format will be reset each time output format is set. This is
actually expected behaviour per stateless decoder API.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 .../staging/media/sunxi/cedrus/cedrus_video.c | 100 +++++++++++-------
 .../staging/media/sunxi/cedrus/cedrus_video.h |   2 +
 2 files changed, 66 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index 1c3c1d080d31..27a7120fa6fb 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -241,12 +241,10 @@ static int cedrus_g_fmt_vid_out(struct file *file, void *priv,
 	return 0;
 }
 
-static int cedrus_try_fmt_vid_cap(struct file *file, void *priv,
-				  struct v4l2_format *f)
+static int cedrus_try_fmt_vid_cap_p(struct cedrus_ctx *ctx,
+				    struct v4l2_pix_format *pix_fmt)
 {
-	struct cedrus_ctx *ctx = cedrus_file2ctx(file);
 	struct cedrus_dev *dev = ctx->dev;
-	struct v4l2_pix_format *pix_fmt = &f->fmt.pix;
 	struct cedrus_format *fmt =
 		cedrus_find_format(pix_fmt->pixelformat, CEDRUS_DECODE_DST,
 				   dev->capabilities);
@@ -262,12 +260,16 @@ static int cedrus_try_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
-static int cedrus_try_fmt_vid_out(struct file *file, void *priv,
+static int cedrus_try_fmt_vid_cap(struct file *file, void *priv,
 				  struct v4l2_format *f)
 {
-	struct cedrus_ctx *ctx = cedrus_file2ctx(file);
+	return cedrus_try_fmt_vid_cap_p(cedrus_file2ctx(file), &f->fmt.pix);
+}
+
+static int cedrus_try_fmt_vid_out_p(struct cedrus_ctx *ctx,
+				    struct v4l2_pix_format *pix_fmt)
+{
 	struct cedrus_dev *dev = ctx->dev;
-	struct v4l2_pix_format *pix_fmt = &f->fmt.pix;
 	struct cedrus_format *fmt =
 		cedrus_find_format(pix_fmt->pixelformat, CEDRUS_DECODE_SRC,
 				   dev->capabilities);
@@ -281,6 +283,12 @@ static int cedrus_try_fmt_vid_out(struct file *file, void *priv,
 	return 0;
 }
 
+static int cedrus_try_fmt_vid_out(struct file *file, void *priv,
+				  struct v4l2_format *f)
+{
+	return cedrus_try_fmt_vid_out_p(cedrus_file2ctx(file), &f->fmt.pix);
+}
+
 static int cedrus_s_fmt_vid_cap(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
@@ -301,13 +309,60 @@ static int cedrus_s_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
+void cedrus_reset_cap_format(struct cedrus_ctx *ctx)
+{
+	ctx->dst_fmt.pixelformat = 0;
+	cedrus_try_fmt_vid_cap_p(ctx, &ctx->dst_fmt);
+}
+
+static int cedrus_s_fmt_vid_out_p(struct cedrus_ctx *ctx,
+				  struct v4l2_pix_format *pix_fmt)
+{
+	struct vb2_queue *vq;
+	int ret;
+
+	ret = cedrus_try_fmt_vid_out_p(ctx, pix_fmt);
+	if (ret)
+		return ret;
+
+	ctx->src_fmt = *pix_fmt;
+
+	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT);
+
+	switch (ctx->src_fmt.pixelformat) {
+	case V4L2_PIX_FMT_H264_SLICE:
+	case V4L2_PIX_FMT_HEVC_SLICE:
+		vq->subsystem_flags |=
+			VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF;
+		break;
+	default:
+		vq->subsystem_flags &=
+			~VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF;
+		break;
+	}
+
+	/* Propagate format information to capture. */
+	ctx->dst_fmt.colorspace = pix_fmt->colorspace;
+	ctx->dst_fmt.xfer_func = pix_fmt->xfer_func;
+	ctx->dst_fmt.ycbcr_enc = pix_fmt->ycbcr_enc;
+	ctx->dst_fmt.quantization = pix_fmt->quantization;
+	cedrus_reset_cap_format(ctx);
+
+	return 0;
+}
+
+void cedrus_reset_out_format(struct cedrus_ctx *ctx)
+{
+	ctx->src_fmt.pixelformat = 0;
+	cedrus_s_fmt_vid_out_p(ctx, &ctx->src_fmt);
+}
+
 static int cedrus_s_fmt_vid_out(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
 	struct cedrus_ctx *ctx = cedrus_file2ctx(file);
 	struct vb2_queue *vq;
 	struct vb2_queue *peer_vq;
-	int ret;
 
 	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type);
 	/*
@@ -328,34 +383,7 @@ static int cedrus_s_fmt_vid_out(struct file *file, void *priv,
 	if (vb2_is_busy(peer_vq))
 		return -EBUSY;
 
-	ret = cedrus_try_fmt_vid_out(file, priv, f);
-	if (ret)
-		return ret;
-
-	ctx->src_fmt = f->fmt.pix;
-
-	switch (ctx->src_fmt.pixelformat) {
-	case V4L2_PIX_FMT_H264_SLICE:
-	case V4L2_PIX_FMT_HEVC_SLICE:
-		vq->subsystem_flags |=
-			VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF;
-		break;
-	default:
-		vq->subsystem_flags &=
-			~VB2_V4L2_FL_SUPPORTS_M2M_HOLD_CAPTURE_BUF;
-		break;
-	}
-
-	/* Propagate format information to capture. */
-	ctx->dst_fmt.colorspace = f->fmt.pix.colorspace;
-	ctx->dst_fmt.xfer_func = f->fmt.pix.xfer_func;
-	ctx->dst_fmt.ycbcr_enc = f->fmt.pix.ycbcr_enc;
-	ctx->dst_fmt.quantization = f->fmt.pix.quantization;
-	ctx->dst_fmt.width = ctx->src_fmt.width;
-	ctx->dst_fmt.height = ctx->src_fmt.height;
-	cedrus_prepare_format(&ctx->dst_fmt);
-
-	return 0;
+	return cedrus_s_fmt_vid_out_p(cedrus_file2ctx(file), &f->fmt.pix);
 }
 
 const struct v4l2_ioctl_ops cedrus_ioctl_ops = {
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.h b/drivers/staging/media/sunxi/cedrus/cedrus_video.h
index 05050c0a0921..8e1afc16a6a1 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.h
@@ -27,5 +27,7 @@ extern const struct v4l2_ioctl_ops cedrus_ioctl_ops;
 int cedrus_queue_init(void *priv, struct vb2_queue *src_vq,
 		      struct vb2_queue *dst_vq);
 void cedrus_prepare_format(struct v4l2_pix_format *pix_fmt);
+void cedrus_reset_cap_format(struct cedrus_ctx *ctx);
+void cedrus_reset_out_format(struct cedrus_ctx *ctx);
 
 #endif
-- 
2.38.1


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

* [PATCH v2 03/11] media: cedrus: use helper to set default formats
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 01/11] media: cedrus: remove superfluous call Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 02/11] media: cedrus: Add format reset helpers Jernej Skrabec
@ 2022-11-02 18:08 ` Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 04/11] media: cedrus: Add helper for checking capabilities Jernej Skrabec
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:08 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

Now that set output format helper is available, let's use that for
setting default values. Advantage of this is that values will be always
valid. Current code produced invalid default values for V3s SoC, which
doesn't support MPEG2 decoding.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 55c54dfdc585..2f284a58d787 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -361,16 +361,8 @@ static int cedrus_open(struct file *file)
 		ret = PTR_ERR(ctx->fh.m2m_ctx);
 		goto err_ctrls;
 	}
-	ctx->dst_fmt.pixelformat = V4L2_PIX_FMT_NV12_32L32;
-	cedrus_prepare_format(&ctx->dst_fmt);
-	ctx->src_fmt.pixelformat = V4L2_PIX_FMT_MPEG2_SLICE;
-	/*
-	 * TILED_NV12 has more strict requirements, so copy the width and
-	 * height to src_fmt to ensure that is matches the dst_fmt resolution.
-	 */
-	ctx->src_fmt.width = ctx->dst_fmt.width;
-	ctx->src_fmt.height = ctx->dst_fmt.height;
-	cedrus_prepare_format(&ctx->src_fmt);
+
+	cedrus_reset_out_format(ctx);
 
 	v4l2_fh_add(&ctx->fh);
 
-- 
2.38.1


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

* [PATCH v2 04/11] media: cedrus: Add helper for checking capabilities
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
                   ` (2 preceding siblings ...)
  2022-11-02 18:08 ` [PATCH v2 03/11] media: cedrus: use helper to set default formats Jernej Skrabec
@ 2022-11-02 18:08 ` Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 05/11] media: cedrus: Filter controls based on capability Jernej Skrabec
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:08 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

There is several different Cedrus cores with varying capabilities, so
some operations like listing formats depends on checks if feature is
supported or not.

Currently check for capabilities is only in format handling functions,
but it will be used also elsewhere later. Let's convert this check to
helper and while at it, also simplify it. There is no need to check if
capability mask is zero, condition will still work properly.

No functional changes intended.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.h   |  6 +++++
 .../staging/media/sunxi/cedrus/cedrus_video.c | 24 +++++++------------
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 9cffaf228422..1a98790a99af 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -268,6 +268,12 @@ vb2_to_cedrus_buffer(const struct vb2_buffer *p)
 	return vb2_v4l2_to_cedrus_buffer(to_vb2_v4l2_buffer(p));
 }
 
+static inline bool
+cedrus_is_capable(struct cedrus_ctx *ctx, unsigned int capabilities)
+{
+	return (ctx->dev->capabilities & capabilities) == capabilities;
+}
+
 void *cedrus_find_control_data(struct cedrus_ctx *ctx, u32 id);
 u32 cedrus_get_num_of_controls(struct cedrus_ctx *ctx, u32 id);
 
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index 27a7120fa6fb..192d51397fd2 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -73,8 +73,8 @@ static inline struct cedrus_ctx *cedrus_file2ctx(struct file *file)
 	return container_of(file->private_data, struct cedrus_ctx, fh);
 }
 
-static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions,
-						unsigned int capabilities)
+static struct cedrus_format *cedrus_find_format(struct cedrus_ctx *ctx,
+						u32 pixelformat, u32 directions)
 {
 	struct cedrus_format *first_valid_fmt = NULL;
 	struct cedrus_format *fmt;
@@ -83,7 +83,7 @@ static struct cedrus_format *cedrus_find_format(u32 pixelformat, u32 directions,
 	for (i = 0; i < CEDRUS_FORMATS_COUNT; i++) {
 		fmt = &cedrus_formats[i];
 
-		if ((fmt->capabilities & capabilities) != fmt->capabilities ||
+		if (!cedrus_is_capable(ctx, fmt->capabilities) ||
 		    !(fmt->directions & directions))
 			continue;
 
@@ -177,19 +177,13 @@ static int cedrus_enum_fmt(struct file *file, struct v4l2_fmtdesc *f,
 			   u32 direction)
 {
 	struct cedrus_ctx *ctx = cedrus_file2ctx(file);
-	struct cedrus_dev *dev = ctx->dev;
-	unsigned int capabilities = dev->capabilities;
-	struct cedrus_format *fmt;
 	unsigned int i, index;
 
 	/* Index among formats that match the requested direction. */
 	index = 0;
 
 	for (i = 0; i < CEDRUS_FORMATS_COUNT; i++) {
-		fmt = &cedrus_formats[i];
-
-		if (fmt->capabilities && (fmt->capabilities & capabilities) !=
-		    fmt->capabilities)
+		if (!cedrus_is_capable(ctx, cedrus_formats[i].capabilities))
 			continue;
 
 		if (!(cedrus_formats[i].directions & direction))
@@ -244,10 +238,9 @@ static int cedrus_g_fmt_vid_out(struct file *file, void *priv,
 static int cedrus_try_fmt_vid_cap_p(struct cedrus_ctx *ctx,
 				    struct v4l2_pix_format *pix_fmt)
 {
-	struct cedrus_dev *dev = ctx->dev;
 	struct cedrus_format *fmt =
-		cedrus_find_format(pix_fmt->pixelformat, CEDRUS_DECODE_DST,
-				   dev->capabilities);
+		cedrus_find_format(ctx, pix_fmt->pixelformat,
+				   CEDRUS_DECODE_DST);
 
 	if (!fmt)
 		return -EINVAL;
@@ -269,10 +262,9 @@ static int cedrus_try_fmt_vid_cap(struct file *file, void *priv,
 static int cedrus_try_fmt_vid_out_p(struct cedrus_ctx *ctx,
 				    struct v4l2_pix_format *pix_fmt)
 {
-	struct cedrus_dev *dev = ctx->dev;
 	struct cedrus_format *fmt =
-		cedrus_find_format(pix_fmt->pixelformat, CEDRUS_DECODE_SRC,
-				   dev->capabilities);
+		cedrus_find_format(ctx, pix_fmt->pixelformat,
+				   CEDRUS_DECODE_SRC);
 
 	if (!fmt)
 		return -EINVAL;
-- 
2.38.1


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

* [PATCH v2 05/11] media: cedrus: Filter controls based on capability
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
                   ` (3 preceding siblings ...)
  2022-11-02 18:08 ` [PATCH v2 04/11] media: cedrus: Add helper for checking capabilities Jernej Skrabec
@ 2022-11-02 18:08 ` Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 06/11] media: cedrus: set codec ops immediately Jernej Skrabec
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:08 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

Because not all Cedrus variants supports all codecs, controls should be
registered only if codec related to individual control is supported by
Cedrus.

Replace codec enum, which is not used at all, with capabilities flags
and register control only if capabilities are met. We have to be careful
though, controls have to be tightly packed in ctx->ctrls array.
Otherwise functions cedrus_find_control_data() and
cedrus_get_num_of_controls() won't work properly.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c | 50 +++++++++++----------
 drivers/staging/media/sunxi/cedrus/cedrus.h |  2 +-
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 2f284a58d787..a88ca89d966d 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -77,56 +77,56 @@ static const struct cedrus_control cedrus_controls[] = {
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_MPEG2_SEQUENCE,
 		},
-		.codec		= CEDRUS_CODEC_MPEG2,
+		.capabilities	= CEDRUS_CAPABILITY_MPEG2_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_MPEG2_PICTURE,
 		},
-		.codec		= CEDRUS_CODEC_MPEG2,
+		.capabilities	= CEDRUS_CAPABILITY_MPEG2_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_MPEG2_QUANTISATION,
 		},
-		.codec		= CEDRUS_CODEC_MPEG2,
+		.capabilities	= CEDRUS_CAPABILITY_MPEG2_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_H264_DECODE_PARAMS,
 		},
-		.codec		= CEDRUS_CODEC_H264,
+		.capabilities	= CEDRUS_CAPABILITY_H264_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_H264_SLICE_PARAMS,
 		},
-		.codec		= CEDRUS_CODEC_H264,
+		.capabilities	= CEDRUS_CAPABILITY_H264_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_H264_SPS,
 			.ops	= &cedrus_ctrl_ops,
 		},
-		.codec		= CEDRUS_CODEC_H264,
+		.capabilities	= CEDRUS_CAPABILITY_H264_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_H264_PPS,
 		},
-		.codec		= CEDRUS_CODEC_H264,
+		.capabilities	= CEDRUS_CAPABILITY_H264_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_H264_SCALING_MATRIX,
 		},
-		.codec		= CEDRUS_CODEC_H264,
+		.capabilities	= CEDRUS_CAPABILITY_H264_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_H264_PRED_WEIGHTS,
 		},
-		.codec		= CEDRUS_CODEC_H264,
+		.capabilities	= CEDRUS_CAPABILITY_H264_DEC,
 	},
 	{
 		.cfg = {
@@ -134,7 +134,7 @@ static const struct cedrus_control cedrus_controls[] = {
 			.max	= V4L2_STATELESS_H264_DECODE_MODE_SLICE_BASED,
 			.def	= V4L2_STATELESS_H264_DECODE_MODE_SLICE_BASED,
 		},
-		.codec		= CEDRUS_CODEC_H264,
+		.capabilities	= CEDRUS_CAPABILITY_H264_DEC,
 	},
 	{
 		.cfg = {
@@ -142,7 +142,7 @@ static const struct cedrus_control cedrus_controls[] = {
 			.max	= V4L2_STATELESS_H264_START_CODE_NONE,
 			.def	= V4L2_STATELESS_H264_START_CODE_NONE,
 		},
-		.codec		= CEDRUS_CODEC_H264,
+		.capabilities	= CEDRUS_CAPABILITY_H264_DEC,
 	},
 	/*
 	 * We only expose supported profiles information,
@@ -160,20 +160,20 @@ static const struct cedrus_control cedrus_controls[] = {
 			.menu_skip_mask =
 				BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED),
 		},
-		.codec		= CEDRUS_CODEC_H264,
+		.capabilities	= CEDRUS_CAPABILITY_H264_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_HEVC_SPS,
 			.ops	= &cedrus_ctrl_ops,
 		},
-		.codec		= CEDRUS_CODEC_H265,
+		.capabilities	= CEDRUS_CAPABILITY_H265_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_HEVC_PPS,
 		},
-		.codec		= CEDRUS_CODEC_H265,
+		.capabilities	= CEDRUS_CAPABILITY_H265_DEC,
 	},
 	{
 		.cfg = {
@@ -181,13 +181,13 @@ static const struct cedrus_control cedrus_controls[] = {
 			/* The driver can only handle 1 entry per slice for now */
 			.dims   = { 1 },
 		},
-		.codec		= CEDRUS_CODEC_H265,
+		.capabilities	= CEDRUS_CAPABILITY_H265_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_HEVC_SCALING_MATRIX,
 		},
-		.codec		= CEDRUS_CODEC_H265,
+		.capabilities	= CEDRUS_CAPABILITY_H265_DEC,
 	},
 	{
 		.cfg = {
@@ -197,7 +197,7 @@ static const struct cedrus_control cedrus_controls[] = {
 			.max = 0xffffffff,
 			.step = 1,
 		},
-		.codec		= CEDRUS_CODEC_H265,
+		.capabilities	= CEDRUS_CAPABILITY_H265_DEC,
 	},
 	{
 		.cfg = {
@@ -205,7 +205,7 @@ static const struct cedrus_control cedrus_controls[] = {
 			.max	= V4L2_STATELESS_HEVC_DECODE_MODE_SLICE_BASED,
 			.def	= V4L2_STATELESS_HEVC_DECODE_MODE_SLICE_BASED,
 		},
-		.codec		= CEDRUS_CODEC_H265,
+		.capabilities	= CEDRUS_CAPABILITY_H265_DEC,
 	},
 	{
 		.cfg = {
@@ -213,19 +213,19 @@ static const struct cedrus_control cedrus_controls[] = {
 			.max	= V4L2_STATELESS_HEVC_START_CODE_NONE,
 			.def	= V4L2_STATELESS_HEVC_START_CODE_NONE,
 		},
-		.codec		= CEDRUS_CODEC_H265,
+		.capabilities	= CEDRUS_CAPABILITY_H265_DEC,
 	},
 	{
 		.cfg = {
 			.id	= V4L2_CID_STATELESS_VP8_FRAME,
 		},
-		.codec		= CEDRUS_CODEC_VP8,
+		.capabilities	= CEDRUS_CAPABILITY_VP8_DEC,
 	},
 	{
 		.cfg = {
 			.id = V4L2_CID_STATELESS_HEVC_DECODE_PARAMS,
 		},
-		.codec		= CEDRUS_CODEC_H265,
+		.capabilities	= CEDRUS_CAPABILITY_H265_DEC,
 	},
 };
 
@@ -258,7 +258,7 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
 	struct v4l2_ctrl_handler *hdl = &ctx->hdl;
 	struct v4l2_ctrl *ctrl;
 	unsigned int ctrl_size;
-	unsigned int i;
+	unsigned int i, j;
 
 	v4l2_ctrl_handler_init(hdl, CEDRUS_CONTROLS_COUNT);
 	if (hdl->error) {
@@ -274,7 +274,11 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
 	if (!ctx->ctrls)
 		return -ENOMEM;
 
+	j = 0;
 	for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
+		if (!cedrus_is_capable(ctx, cedrus_controls[i].capabilities))
+			continue;
+
 		ctrl = v4l2_ctrl_new_custom(hdl, &cedrus_controls[i].cfg,
 					    NULL);
 		if (hdl->error) {
@@ -289,7 +293,7 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
 			return hdl->error;
 		}
 
-		ctx->ctrls[i] = ctrl;
+		ctx->ctrls[j++] = ctrl;
 	}
 
 	ctx->fh.ctrl_handler = hdl;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 1a98790a99af..7a1619967513 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -57,7 +57,7 @@ enum cedrus_h264_pic_type {
 
 struct cedrus_control {
 	struct v4l2_ctrl_config cfg;
-	enum cedrus_codec	codec;
+	unsigned int		capabilities;
 };
 
 struct cedrus_h264_run {
-- 
2.38.1


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

* [PATCH v2 06/11] media: cedrus: set codec ops immediately
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
                   ` (4 preceding siblings ...)
  2022-11-02 18:08 ` [PATCH v2 05/11] media: cedrus: Filter controls based on capability Jernej Skrabec
@ 2022-11-02 18:08 ` Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 07/11] media: cedrus: Remove cedrus_codec enum Jernej Skrabec
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:08 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

We'll need codec ops soon after output format is set in following
commits. Let's move current codec setup to set output format callback.
While at it, let's remove one level of indirection by changing
current_codec to point to codec ops structure directly.

Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c   |  5 ---
 drivers/staging/media/sunxi/cedrus/cedrus.h   |  3 +-
 .../staging/media/sunxi/cedrus/cedrus_dec.c   |  4 +-
 .../staging/media/sunxi/cedrus/cedrus_hw.c    |  6 +--
 .../staging/media/sunxi/cedrus/cedrus_video.c | 44 ++++++++-----------
 5 files changed, 25 insertions(+), 37 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index a88ca89d966d..37b1df9a9d6a 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -456,11 +456,6 @@ static int cedrus_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	dev->dec_ops[CEDRUS_CODEC_MPEG2] = &cedrus_dec_ops_mpeg2;
-	dev->dec_ops[CEDRUS_CODEC_H264] = &cedrus_dec_ops_h264;
-	dev->dec_ops[CEDRUS_CODEC_H265] = &cedrus_dec_ops_h265;
-	dev->dec_ops[CEDRUS_CODEC_VP8] = &cedrus_dec_ops_vp8;
-
 	mutex_init(&dev->dev_mutex);
 
 	INIT_DELAYED_WORK(&dev->watchdog_work, cedrus_watchdog);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 7a1619967513..0b082b1fae22 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -126,7 +126,7 @@ struct cedrus_ctx {
 
 	struct v4l2_pix_format		src_fmt;
 	struct v4l2_pix_format		dst_fmt;
-	enum cedrus_codec		current_codec;
+	struct cedrus_dec_ops		*current_codec;
 
 	struct v4l2_ctrl_handler	hdl;
 	struct v4l2_ctrl		**ctrls;
@@ -185,7 +185,6 @@ struct cedrus_dev {
 	struct platform_device	*pdev;
 	struct device		*dev;
 	struct v4l2_m2m_dev	*m2m_dev;
-	struct cedrus_dec_ops	*dec_ops[CEDRUS_CODEC_LAST];
 
 	/* Device file mutex */
 	struct mutex		dev_mutex;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
index e7f7602a5ab4..fbbf9e6f0f50 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
@@ -94,7 +94,7 @@ void cedrus_device_run(void *priv)
 
 	cedrus_dst_format_set(dev, &ctx->dst_fmt);
 
-	error = dev->dec_ops[ctx->current_codec]->setup(ctx, &run);
+	error = ctx->current_codec->setup(ctx, &run);
 	if (error)
 		v4l2_err(&ctx->dev->v4l2_dev,
 			 "Failed to setup decoding job: %d\n", error);
@@ -110,7 +110,7 @@ void cedrus_device_run(void *priv)
 		schedule_delayed_work(&dev->watchdog_work,
 				      msecs_to_jiffies(2000));
 
-		dev->dec_ops[ctx->current_codec]->trigger(ctx);
+		ctx->current_codec->trigger(ctx);
 	} else {
 		v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev,
 						 ctx->fh.m2m_ctx,
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
index a6470a89851e..c3387cd1e80f 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
@@ -132,12 +132,12 @@ static irqreturn_t cedrus_irq(int irq, void *data)
 		return IRQ_NONE;
 	}
 
-	status = dev->dec_ops[ctx->current_codec]->irq_status(ctx);
+	status = ctx->current_codec->irq_status(ctx);
 	if (status == CEDRUS_IRQ_NONE)
 		return IRQ_NONE;
 
-	dev->dec_ops[ctx->current_codec]->irq_disable(ctx);
-	dev->dec_ops[ctx->current_codec]->irq_clear(ctx);
+	ctx->current_codec->irq_disable(ctx);
+	ctx->current_codec->irq_clear(ctx);
 
 	if (status == CEDRUS_IRQ_ERROR)
 		state = VB2_BUF_STATE_ERROR;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index 192d51397fd2..f6305ffe2c4f 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -333,6 +333,21 @@ static int cedrus_s_fmt_vid_out_p(struct cedrus_ctx *ctx,
 		break;
 	}
 
+	switch (ctx->src_fmt.pixelformat) {
+	case V4L2_PIX_FMT_MPEG2_SLICE:
+		ctx->current_codec = &cedrus_dec_ops_mpeg2;
+		break;
+	case V4L2_PIX_FMT_H264_SLICE:
+		ctx->current_codec = &cedrus_dec_ops_h264;
+		break;
+	case V4L2_PIX_FMT_HEVC_SLICE:
+		ctx->current_codec = &cedrus_dec_ops_h265;
+		break;
+	case V4L2_PIX_FMT_VP8_FRAME:
+		ctx->current_codec = &cedrus_dec_ops_vp8;
+		break;
+	}
+
 	/* Propagate format information to capture. */
 	ctx->dst_fmt.colorspace = pix_fmt->colorspace;
 	ctx->dst_fmt.xfer_func = pix_fmt->xfer_func;
@@ -491,34 +506,13 @@ static int cedrus_start_streaming(struct vb2_queue *vq, unsigned int count)
 	struct cedrus_dev *dev = ctx->dev;
 	int ret = 0;
 
-	switch (ctx->src_fmt.pixelformat) {
-	case V4L2_PIX_FMT_MPEG2_SLICE:
-		ctx->current_codec = CEDRUS_CODEC_MPEG2;
-		break;
-
-	case V4L2_PIX_FMT_H264_SLICE:
-		ctx->current_codec = CEDRUS_CODEC_H264;
-		break;
-
-	case V4L2_PIX_FMT_HEVC_SLICE:
-		ctx->current_codec = CEDRUS_CODEC_H265;
-		break;
-
-	case V4L2_PIX_FMT_VP8_FRAME:
-		ctx->current_codec = CEDRUS_CODEC_VP8;
-		break;
-
-	default:
-		return -EINVAL;
-	}
-
 	if (V4L2_TYPE_IS_OUTPUT(vq->type)) {
 		ret = pm_runtime_resume_and_get(dev->dev);
 		if (ret < 0)
 			goto err_cleanup;
 
-		if (dev->dec_ops[ctx->current_codec]->start) {
-			ret = dev->dec_ops[ctx->current_codec]->start(ctx);
+		if (ctx->current_codec->start) {
+			ret = ctx->current_codec->start(ctx);
 			if (ret)
 				goto err_pm;
 		}
@@ -540,8 +534,8 @@ static void cedrus_stop_streaming(struct vb2_queue *vq)
 	struct cedrus_dev *dev = ctx->dev;
 
 	if (V4L2_TYPE_IS_OUTPUT(vq->type)) {
-		if (dev->dec_ops[ctx->current_codec]->stop)
-			dev->dec_ops[ctx->current_codec]->stop(ctx);
+		if (ctx->current_codec->stop)
+			ctx->current_codec->stop(ctx);
 
 		pm_runtime_put(dev->dev);
 	}
-- 
2.38.1


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

* [PATCH v2 07/11] media: cedrus: Remove cedrus_codec enum
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
                   ` (5 preceding siblings ...)
  2022-11-02 18:08 ` [PATCH v2 06/11] media: cedrus: set codec ops immediately Jernej Skrabec
@ 2022-11-02 18:08 ` Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 08/11] media: cedrus: prefer untiled capture format Jernej Skrabec
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:08 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

Last user of cedrus_codec enum is cedrus_engine_enable() but this
argument is completely redundant. Same information can be obtained via
source pixel format. Let's remove this argument and enum.

No functional changes intended.

Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.h       |  8 --------
 drivers/staging/media/sunxi/cedrus/cedrus_h264.c  |  2 +-
 drivers/staging/media/sunxi/cedrus/cedrus_h265.c  |  2 +-
 drivers/staging/media/sunxi/cedrus/cedrus_hw.c    | 12 ++++++------
 drivers/staging/media/sunxi/cedrus/cedrus_hw.h    |  2 +-
 drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c |  2 +-
 drivers/staging/media/sunxi/cedrus/cedrus_vp8.c   |  2 +-
 7 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 0b082b1fae22..5904294f3108 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -35,14 +35,6 @@
 #define CEDRUS_CAPABILITY_VP8_DEC	BIT(4)
 #define CEDRUS_CAPABILITY_H265_10_DEC	BIT(5)
 
-enum cedrus_codec {
-	CEDRUS_CODEC_MPEG2,
-	CEDRUS_CODEC_H264,
-	CEDRUS_CODEC_H265,
-	CEDRUS_CODEC_VP8,
-	CEDRUS_CODEC_LAST,
-};
-
 enum cedrus_irq_status {
 	CEDRUS_IRQ_NONE,
 	CEDRUS_IRQ_ERROR,
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
index c92dec21c1ac..dfb401df138a 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
@@ -518,7 +518,7 @@ static int cedrus_h264_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	struct cedrus_dev *dev = ctx->dev;
 	int ret;
 
-	cedrus_engine_enable(ctx, CEDRUS_CODEC_H264);
+	cedrus_engine_enable(ctx);
 
 	cedrus_write(dev, VE_H264_SDROT_CTRL, 0);
 	cedrus_write(dev, VE_H264_EXTRA_BUFFER1,
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
index 7a438cd22c34..5d3da50ce46a 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
@@ -471,7 +471,7 @@ static int cedrus_h265_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	}
 
 	/* Activate H265 engine. */
-	cedrus_engine_enable(ctx, CEDRUS_CODEC_H265);
+	cedrus_engine_enable(ctx);
 
 	/* Source offset and length in bits. */
 
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
index c3387cd1e80f..fa86a658fdc6 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
@@ -31,7 +31,7 @@
 #include "cedrus_hw.h"
 #include "cedrus_regs.h"
 
-int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec codec)
+int cedrus_engine_enable(struct cedrus_ctx *ctx)
 {
 	u32 reg = 0;
 
@@ -42,18 +42,18 @@ int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec codec)
 	reg |= VE_MODE_REC_WR_MODE_2MB;
 	reg |= VE_MODE_DDR_MODE_BW_128;
 
-	switch (codec) {
-	case CEDRUS_CODEC_MPEG2:
+	switch (ctx->src_fmt.pixelformat) {
+	case V4L2_PIX_FMT_MPEG2_SLICE:
 		reg |= VE_MODE_DEC_MPEG;
 		break;
 
 	/* H.264 and VP8 both use the same decoding mode bit. */
-	case CEDRUS_CODEC_H264:
-	case CEDRUS_CODEC_VP8:
+	case V4L2_PIX_FMT_H264_SLICE:
+	case V4L2_PIX_FMT_VP8_FRAME:
 		reg |= VE_MODE_DEC_H264;
 		break;
 
-	case CEDRUS_CODEC_H265:
+	case V4L2_PIX_FMT_HEVC_SLICE:
 		reg |= VE_MODE_DEC_H265;
 		break;
 
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
index 7c92f00e36da..6f1e701b1ea8 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.h
@@ -16,7 +16,7 @@
 #ifndef _CEDRUS_HW_H_
 #define _CEDRUS_HW_H_
 
-int cedrus_engine_enable(struct cedrus_ctx *ctx, enum cedrus_codec codec);
+int cedrus_engine_enable(struct cedrus_ctx *ctx);
 void cedrus_engine_disable(struct cedrus_dev *dev);
 
 void cedrus_dst_format_set(struct cedrus_dev *dev,
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
index c1128d2cd555..10e98f08aafc 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
@@ -66,7 +66,7 @@ static int cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	quantisation = run->mpeg2.quantisation;
 
 	/* Activate MPEG engine. */
-	cedrus_engine_enable(ctx, CEDRUS_CODEC_MPEG2);
+	cedrus_engine_enable(ctx);
 
 	/* Set intra quantisation matrix. */
 	matrix = quantisation->intra_quantiser_matrix;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
index f7714baae37d..969677a3bbf9 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
@@ -662,7 +662,7 @@ static int cedrus_vp8_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	int header_size;
 	u32 reg;
 
-	cedrus_engine_enable(ctx, CEDRUS_CODEC_VP8);
+	cedrus_engine_enable(ctx);
 
 	cedrus_write(dev, VE_H264_CTRL, VE_H264_CTRL_VP8);
 
-- 
2.38.1


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

* [PATCH v2 08/11] media: cedrus: prefer untiled capture format
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
                   ` (6 preceding siblings ...)
  2022-11-02 18:08 ` [PATCH v2 07/11] media: cedrus: Remove cedrus_codec enum Jernej Skrabec
@ 2022-11-02 18:08 ` Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 09/11] media: cedrus: initialize controls a bit later Jernej Skrabec
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:08 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

While all generations of display engine on Allwinner SoCs support
untiled format, only first generation supports tiled format.  Let's
move untiled format up, so it can be picked before tiled one. If
Cedrus variant doesn't support untiled format, tiled will still be
picked as default format.

Acked-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus_video.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index f6305ffe2c4f..dec5d3ae4871 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -55,15 +55,15 @@ static struct cedrus_format cedrus_formats[] = {
 		.directions	= CEDRUS_DECODE_SRC,
 		.capabilities	= CEDRUS_CAPABILITY_VP8_DEC,
 	},
-	{
-		.pixelformat	= V4L2_PIX_FMT_NV12_32L32,
-		.directions	= CEDRUS_DECODE_DST,
-	},
 	{
 		.pixelformat	= V4L2_PIX_FMT_NV12,
 		.directions	= CEDRUS_DECODE_DST,
 		.capabilities	= CEDRUS_CAPABILITY_UNTILED,
 	},
+	{
+		.pixelformat	= V4L2_PIX_FMT_NV12_32L32,
+		.directions	= CEDRUS_DECODE_DST,
+	},
 };
 
 #define CEDRUS_FORMATS_COUNT	ARRAY_SIZE(cedrus_formats)
-- 
2.38.1


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

* [PATCH v2 09/11] media: cedrus: initialize controls a bit later
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
                   ` (7 preceding siblings ...)
  2022-11-02 18:08 ` [PATCH v2 08/11] media: cedrus: prefer untiled capture format Jernej Skrabec
@ 2022-11-02 18:08 ` Jernej Skrabec
  2022-11-02 18:08 ` [PATCH v2 10/11] media: cedrus: Adjust buffer size based on control values Jernej Skrabec
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:08 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

While it doesn't matter if controls are initialized before or after
queues and formats from open handler standpoint, initializing them last
helps keeping s_ctrl handler simpler, since everything has already valid
values.

This is just preparation for follow up changes. No functional change is
intended.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 37b1df9a9d6a..6a2c08928613 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -355,27 +355,27 @@ static int cedrus_open(struct file *file)
 	file->private_data = &ctx->fh;
 	ctx->dev = dev;
 
-	ret = cedrus_init_ctrls(dev, ctx);
-	if (ret)
-		goto err_free;
-
 	ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(dev->m2m_dev, ctx,
 					    &cedrus_queue_init);
 	if (IS_ERR(ctx->fh.m2m_ctx)) {
 		ret = PTR_ERR(ctx->fh.m2m_ctx);
-		goto err_ctrls;
+		goto err_free;
 	}
 
 	cedrus_reset_out_format(ctx);
 
+	ret = cedrus_init_ctrls(dev, ctx);
+	if (ret)
+		goto err_m2m_release;
+
 	v4l2_fh_add(&ctx->fh);
 
 	mutex_unlock(&dev->dev_mutex);
 
 	return 0;
 
-err_ctrls:
-	v4l2_ctrl_handler_free(&ctx->hdl);
+err_m2m_release:
+	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
 err_free:
 	kfree(ctx);
 	mutex_unlock(&dev->dev_mutex);
-- 
2.38.1


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

* [PATCH v2 10/11] media: cedrus: Adjust buffer size based on control values
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
                   ` (8 preceding siblings ...)
  2022-11-02 18:08 ` [PATCH v2 09/11] media: cedrus: initialize controls a bit later Jernej Skrabec
@ 2022-11-02 18:08 ` Jernej Skrabec
  2022-11-04 15:27   ` Hans Verkuil
  2022-11-02 18:08 ` [PATCH v2 11/11] media: cedrus: h265: Support decoding 10-bit frames Jernej Skrabec
  2022-11-04 15:31 ` [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Hans Verkuil
  11 siblings, 1 reply; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:08 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

In some cases decoding engine needs extra space in capture buffers. This
is the case for decoding 10-bit HEVC frames into 8-bit capture format.
This commit only adds infrastructure for such cases.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c       | 14 ++++++++++++++
 drivers/staging/media/sunxi/cedrus/cedrus.h       |  2 ++
 drivers/staging/media/sunxi/cedrus/cedrus_video.c |  4 ++++
 3 files changed, 20 insertions(+)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index 6a2c08928613..c36999e47591 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -68,8 +68,22 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
 	return 0;
 }
 
+static int cedrus_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+	struct cedrus_ctx *ctx = container_of(ctrl->handler,
+					      struct cedrus_ctx, hdl);
+	struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
+					       V4L2_BUF_TYPE_VIDEO_CAPTURE);
+
+	if (!vb2_is_busy(vq) && !vb2_is_streaming(vq))
+		cedrus_reset_cap_format(ctx);
+
+	return 0;
+}
+
 static const struct v4l2_ctrl_ops cedrus_ctrl_ops = {
 	.try_ctrl = cedrus_try_ctrl,
+	.s_ctrl = cedrus_s_ctrl,
 };
 
 static const struct cedrus_control cedrus_controls[] = {
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 5904294f3108..774fe8048ce3 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -162,6 +162,8 @@ struct cedrus_dec_ops {
 	int (*start)(struct cedrus_ctx *ctx);
 	void (*stop)(struct cedrus_ctx *ctx);
 	void (*trigger)(struct cedrus_ctx *ctx);
+	unsigned int (*extra_cap_size)(struct cedrus_ctx *ctx,
+				       struct v4l2_pix_format *pix_fmt);
 };
 
 struct cedrus_variant {
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
index dec5d3ae4871..dc67940f1ade 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
@@ -250,6 +250,10 @@ static int cedrus_try_fmt_vid_cap_p(struct cedrus_ctx *ctx,
 	pix_fmt->height = ctx->src_fmt.height;
 	cedrus_prepare_format(pix_fmt);
 
+	if (ctx->current_codec->extra_cap_size)
+		pix_fmt->sizeimage +=
+			ctx->current_codec->extra_cap_size(ctx, pix_fmt);
+
 	return 0;
 }
 
-- 
2.38.1


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

* [PATCH v2 11/11] media: cedrus: h265: Support decoding 10-bit frames
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
                   ` (9 preceding siblings ...)
  2022-11-02 18:08 ` [PATCH v2 10/11] media: cedrus: Adjust buffer size based on control values Jernej Skrabec
@ 2022-11-02 18:08 ` Jernej Skrabec
  2022-11-04 15:31 ` [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Hans Verkuil
  11 siblings, 0 replies; 15+ messages in thread
From: Jernej Skrabec @ 2022-11-02 18:08 UTC (permalink / raw)
  To: mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, hverkuil-cisco, linux-media,
	linux-staging, linux-arm-kernel, linux-sunxi, linux-kernel,
	Jernej Skrabec

10-bit frames needs extra buffer space when 8-bit capture format is
used. Use previously prepared infrastructure to adjust buffer size.

Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/staging/media/sunxi/cedrus/cedrus.c   |  7 ++++
 drivers/staging/media/sunxi/cedrus/cedrus.h   |  1 +
 .../staging/media/sunxi/cedrus/cedrus_h265.c  | 35 +++++++++++++++++++
 .../staging/media/sunxi/cedrus/cedrus_regs.h  | 16 +++++++++
 4 files changed, 59 insertions(+)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index c36999e47591..909f769b3ede 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -75,6 +75,13 @@ static int cedrus_s_ctrl(struct v4l2_ctrl *ctrl)
 	struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
 					       V4L2_BUF_TYPE_VIDEO_CAPTURE);
 
+	if (ctrl->id == V4L2_CID_STATELESS_HEVC_SPS) {
+		const struct v4l2_ctrl_hevc_sps *sps = ctrl->p_new.p_hevc_sps;
+
+		ctx->bit_depth = max(sps->bit_depth_luma_minus8,
+				     sps->bit_depth_chroma_minus8) + 8;
+	}
+
 	if (!vb2_is_busy(vq) && !vb2_is_streaming(vq))
 		cedrus_reset_cap_format(ctx);
 
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 774fe8048ce3..522c184e2afc 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -119,6 +119,7 @@ struct cedrus_ctx {
 	struct v4l2_pix_format		src_fmt;
 	struct v4l2_pix_format		dst_fmt;
 	struct cedrus_dec_ops		*current_codec;
+	unsigned int			bit_depth;
 
 	struct v4l2_ctrl_handler	hdl;
 	struct v4l2_ctrl		**ctrls;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
index 5d3da50ce46a..fc9297232456 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
@@ -41,6 +41,19 @@ struct cedrus_h265_sram_pred_weight {
 	__s8	offset;
 } __packed;
 
+static unsigned int cedrus_h265_2bit_size(unsigned int width,
+					  unsigned int height)
+{
+	/*
+	 * Vendor library additionally aligns width and height to 16,
+	 * but all capture formats are already aligned to that anyway,
+	 * so we can skip that here. All formats are also one form of
+	 * YUV 4:2:0 or another, so we can safely assume multiplication
+	 * factor of 1.5.
+	 */
+	return ALIGN(width / 4, 32) * height * 3 / 2;
+}
+
 static enum cedrus_irq_status cedrus_h265_irq_status(struct cedrus_ctx *ctx)
 {
 	struct cedrus_dev *dev = ctx->dev;
@@ -802,6 +815,18 @@ static int cedrus_h265_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 						      VE_DEC_H265_SRAM_OFFSET_PRED_WEIGHT_CHROMA_L1);
 	}
 
+	if (ctx->bit_depth > 8) {
+		unsigned int stride = ALIGN(ctx->dst_fmt.width / 4, 32);
+
+		reg = ctx->dst_fmt.sizeimage -
+		      cedrus_h265_2bit_size(ctx->dst_fmt.width,
+					    ctx->dst_fmt.height);
+		cedrus_write(dev, VE_DEC_H265_OFFSET_ADDR_FIRST_OUT, reg);
+
+		reg = VE_DEC_H265_10BIT_CONFIGURE_FIRST_2BIT_STRIDE(stride);
+		cedrus_write(dev, VE_DEC_H265_10BIT_CONFIGURE, reg);
+	}
+
 	/* Enable appropriate interruptions. */
 	cedrus_write(dev, VE_DEC_H265_CTRL, VE_DEC_H265_CTRL_IRQ_MASK);
 
@@ -874,6 +899,15 @@ static void cedrus_h265_trigger(struct cedrus_ctx *ctx)
 	cedrus_write(dev, VE_DEC_H265_TRIGGER, VE_DEC_H265_TRIGGER_DEC_SLICE);
 }
 
+static unsigned int cedrus_h265_extra_cap_size(struct cedrus_ctx *ctx,
+					       struct v4l2_pix_format *pix_fmt)
+{
+	if (ctx->bit_depth > 8)
+		return cedrus_h265_2bit_size(pix_fmt->width, pix_fmt->height);
+
+	return 0;
+}
+
 struct cedrus_dec_ops cedrus_dec_ops_h265 = {
 	.irq_clear	= cedrus_h265_irq_clear,
 	.irq_disable	= cedrus_h265_irq_disable,
@@ -882,4 +916,5 @@ struct cedrus_dec_ops cedrus_dec_ops_h265 = {
 	.start		= cedrus_h265_start,
 	.stop		= cedrus_h265_stop,
 	.trigger	= cedrus_h265_trigger,
+	.extra_cap_size	= cedrus_h265_extra_cap_size,
 };
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
index 655c05b389cf..05e6cbc548ab 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_regs.h
@@ -498,6 +498,22 @@
 
 #define VE_DEC_H265_LOW_ADDR			(VE_ENGINE_DEC_H265 + 0x80)
 
+#define VE_DEC_H265_OFFSET_ADDR_FIRST_OUT	(VE_ENGINE_DEC_H265 + 0x84)
+#define VE_DEC_H265_OFFSET_ADDR_SECOND_OUT	(VE_ENGINE_DEC_H265 + 0x88)
+
+#define VE_DEC_H265_SECOND_OUT_FMT_8BIT_PLUS_2BIT	0
+#define VE_DEC_H265_SECOND_OUT_FMT_P010			1
+#define VE_DEC_H265_SECOND_OUT_FMT_10BIT_4x4_TILED	2
+
+#define VE_DEC_H265_10BIT_CONFIGURE_SECOND_OUT_FMT(v) \
+	SHIFT_AND_MASK_BITS(v, 24, 23)
+#define VE_DEC_H265_10BIT_CONFIGURE_SECOND_2BIT_ENABLE	BIT(22)
+#define VE_DEC_H265_10BIT_CONFIGURE_SECOND_2BIT_STRIDE(v) \
+	SHIFT_AND_MASK_BITS(v, 21, 11)
+#define VE_DEC_H265_10BIT_CONFIGURE_FIRST_2BIT_STRIDE(v) \
+	SHIFT_AND_MASK_BITS(v, 10, 0)
+#define VE_DEC_H265_10BIT_CONFIGURE		(VE_ENGINE_DEC_H265 + 0x8c)
+
 #define VE_DEC_H265_LOW_ADDR_PRIMARY_CHROMA(a) \
 	SHIFT_AND_MASK_BITS(a, 31, 24)
 #define VE_DEC_H265_LOW_ADDR_SECONDARY_CHROMA(a) \
-- 
2.38.1


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

* Re: [PATCH v2 10/11] media: cedrus: Adjust buffer size based on control values
  2022-11-02 18:08 ` [PATCH v2 10/11] media: cedrus: Adjust buffer size based on control values Jernej Skrabec
@ 2022-11-04 15:27   ` Hans Verkuil
  2022-11-04 18:10     ` Jernej Škrabec
  0 siblings, 1 reply; 15+ messages in thread
From: Hans Verkuil @ 2022-11-04 15:27 UTC (permalink / raw)
  To: Jernej Skrabec, mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, linux-media, linux-staging,
	linux-arm-kernel, linux-sunxi, linux-kernel

Hi Jernej,

On 02/11/2022 19:08, Jernej Skrabec wrote:
> In some cases decoding engine needs extra space in capture buffers. This
> is the case for decoding 10-bit HEVC frames into 8-bit capture format.
> This commit only adds infrastructure for such cases.
> 
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus.c       | 14 ++++++++++++++
>  drivers/staging/media/sunxi/cedrus/cedrus.h       |  2 ++
>  drivers/staging/media/sunxi/cedrus/cedrus_video.c |  4 ++++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index 6a2c08928613..c36999e47591 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -68,8 +68,22 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
>  	return 0;
>  }
>  
> +static int cedrus_s_ctrl(struct v4l2_ctrl *ctrl)
> +{
> +	struct cedrus_ctx *ctx = container_of(ctrl->handler,
> +					      struct cedrus_ctx, hdl);
> +	struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
> +					       V4L2_BUF_TYPE_VIDEO_CAPTURE);
> +
> +	if (!vb2_is_busy(vq) && !vb2_is_streaming(vq))

You can drop the !vb2_is_streaming part. If you are streaming, then
it will also be 'busy'.

> +		cedrus_reset_cap_format(ctx);

This is weird. I would expect that this is only called for specific controls,
since presumably not all will change the sizeimage value. It looks like
it applies only to V4L2_CID_STATELESS_HEVC_SPS, so I see no reason why
you would call cedrus_reset_cap_format() for other controls as well.

And what happens if someone makes such a change *while streaming*?
Shouldn't cedrus_try_ctrl detect that and fail with -EBUSY in that case?

Regardless, this is unexpected behavior, so some explanatory comments
would be useful.

I'm also not sure whether it isn't better to add cedrus_s_ctrl in the next
patch, I think it would be more understandable when seen with an actual
control.

Regards,

	Hans

> +
> +	return 0;
> +}
> +
>  static const struct v4l2_ctrl_ops cedrus_ctrl_ops = {
>  	.try_ctrl = cedrus_try_ctrl,
> +	.s_ctrl = cedrus_s_ctrl,
>  };
>  
>  static const struct cedrus_control cedrus_controls[] = {
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index 5904294f3108..774fe8048ce3 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -162,6 +162,8 @@ struct cedrus_dec_ops {
>  	int (*start)(struct cedrus_ctx *ctx);
>  	void (*stop)(struct cedrus_ctx *ctx);
>  	void (*trigger)(struct cedrus_ctx *ctx);
> +	unsigned int (*extra_cap_size)(struct cedrus_ctx *ctx,
> +				       struct v4l2_pix_format *pix_fmt);
>  };
>  
>  struct cedrus_variant {
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> index dec5d3ae4871..dc67940f1ade 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> @@ -250,6 +250,10 @@ static int cedrus_try_fmt_vid_cap_p(struct cedrus_ctx *ctx,
>  	pix_fmt->height = ctx->src_fmt.height;
>  	cedrus_prepare_format(pix_fmt);
>  
> +	if (ctx->current_codec->extra_cap_size)
> +		pix_fmt->sizeimage +=
> +			ctx->current_codec->extra_cap_size(ctx, pix_fmt);
> +
>  	return 0;
>  }
>  


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

* Re: [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support
  2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
                   ` (10 preceding siblings ...)
  2022-11-02 18:08 ` [PATCH v2 11/11] media: cedrus: h265: Support decoding 10-bit frames Jernej Skrabec
@ 2022-11-04 15:31 ` Hans Verkuil
  11 siblings, 0 replies; 15+ messages in thread
From: Hans Verkuil @ 2022-11-04 15:31 UTC (permalink / raw)
  To: Jernej Skrabec, mripard, paul.kocialkowski
  Cc: mchehab, gregkh, wens, samuel, linux-media, linux-staging,
	linux-arm-kernel, linux-sunxi, linux-kernel

Hi Jernej,

On 02/11/2022 19:07, Jernej Skrabec wrote:
> While my first intention was to just add 10-bit HEVC handling, I noticed
> a few format handling issues and a bit of redundancy in some cases. Final
> result is that driver now sticks to stateless decoder rules better.
> 
> Format handling improvements:
> 1. Default format selection is now based on HW capabilities. Before, MPEG2
>    was hardcoded but some Cedrus variants don't actually support it.
> 2. Controls are registered only if related codec is supported by HW.
> 3. Untiled output format is preferred, if supported, over tiled one. All
>    display engine cores support untiled format, but only first generation
>    supports tiled one.
> 
> I hope this makes Cedrus eligible for destaging.
> 
> Best regards,
> Jernej
> 
> Changes from v1:
> - collected acks, except for patch 5, which was changed
> - use cedrus_is_capable() for cedrus_find_format() too (patch 4)
> - tightly pack control pointers in ctx->ctrls[] (patch 5)
> 
> Jernej Skrabec (11):
>   media: cedrus: remove superfluous call
>   media: cedrus: Add format reset helpers
>   media: cedrus: use helper to set default formats
>   media: cedrus: Add helper for checking capabilities
>   media: cedrus: Filter controls based on capability
>   media: cedrus: set codec ops immediately
>   media: cedrus: Remove cedrus_codec enum
>   media: cedrus: prefer untiled capture format
>   media: cedrus: initialize controls a bit later

I'm going to merge the first 9 patches, but leave out the last
two. It's weird what happens there, and I think those two need a
bit more work.

Regards,

	Hans

>   media: cedrus: Adjust buffer size based on control values
>   media: cedrus: h265: Support decoding 10-bit frames
> 
>  drivers/staging/media/sunxi/cedrus/cedrus.c   | 102 +++++----
>  drivers/staging/media/sunxi/cedrus/cedrus.h   |  22 +-
>  .../staging/media/sunxi/cedrus/cedrus_dec.c   |   4 +-
>  .../staging/media/sunxi/cedrus/cedrus_h264.c  |   2 +-
>  .../staging/media/sunxi/cedrus/cedrus_h265.c  |  37 +++-
>  .../staging/media/sunxi/cedrus/cedrus_hw.c    |  18 +-
>  .../staging/media/sunxi/cedrus/cedrus_hw.h    |   2 +-
>  .../staging/media/sunxi/cedrus/cedrus_mpeg2.c |   2 +-
>  .../staging/media/sunxi/cedrus/cedrus_regs.h  |  16 ++
>  .../staging/media/sunxi/cedrus/cedrus_video.c | 200 ++++++++++--------
>  .../staging/media/sunxi/cedrus/cedrus_video.h |   2 +
>  .../staging/media/sunxi/cedrus/cedrus_vp8.c   |   2 +-
>  12 files changed, 244 insertions(+), 165 deletions(-)
> 
> --
> 2.38.1
> 


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

* Re: [PATCH v2 10/11] media: cedrus: Adjust buffer size based on control values
  2022-11-04 15:27   ` Hans Verkuil
@ 2022-11-04 18:10     ` Jernej Škrabec
  0 siblings, 0 replies; 15+ messages in thread
From: Jernej Škrabec @ 2022-11-04 18:10 UTC (permalink / raw)
  To: mripard, paul.kocialkowski, Hans Verkuil
  Cc: mchehab, gregkh, wens, samuel, linux-media, linux-staging,
	linux-arm-kernel, linux-sunxi, linux-kernel

Dne petek, 04. november 2022 ob 16:27:29 CET je Hans Verkuil napisal(a):
> Hi Jernej,
> 
> On 02/11/2022 19:08, Jernej Skrabec wrote:
> > In some cases decoding engine needs extra space in capture buffers. This
> > is the case for decoding 10-bit HEVC frames into 8-bit capture format.
> > This commit only adds infrastructure for such cases.
> > 
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > ---
> > 
> >  drivers/staging/media/sunxi/cedrus/cedrus.c       | 14 ++++++++++++++
> >  drivers/staging/media/sunxi/cedrus/cedrus.h       |  2 ++
> >  drivers/staging/media/sunxi/cedrus/cedrus_video.c |  4 ++++
> >  3 files changed, 20 insertions(+)
> > 
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus.c index
> > 6a2c08928613..c36999e47591 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> > @@ -68,8 +68,22 @@ static int cedrus_try_ctrl(struct v4l2_ctrl *ctrl)
> > 
> >  	return 0;
> >  
> >  }
> > 
> > +static int cedrus_s_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > +	struct cedrus_ctx *ctx = container_of(ctrl->handler,
> > +					      struct 
cedrus_ctx, hdl);
> > +	struct vb2_queue *vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
> > +					       
V4L2_BUF_TYPE_VIDEO_CAPTURE);
> > +
> > +	if (!vb2_is_busy(vq) && !vb2_is_streaming(vq))
> 
> You can drop the !vb2_is_streaming part. If you are streaming, then
> it will also be 'busy'.
> 
> > +		cedrus_reset_cap_format(ctx);
> 
> This is weird. I would expect that this is only called for specific
> controls, since presumably not all will change the sizeimage value. It
> looks like it applies only to V4L2_CID_STATELESS_HEVC_SPS, so I see no
> reason why you would call cedrus_reset_cap_format() for other controls as
> well.

It's more general approach. It's harmless for other cases, still within specs. 
Anyway, I can make it more specific.

> 
> And what happens if someone makes such a change *while streaming*?
> Shouldn't cedrus_try_ctrl detect that and fail with -EBUSY in that case?

SPS can also be changed during streaming, so we can't outright deny setting it 
during streaming. Also, if it changes from 10 to 8, that also works, since 
capture buffers are still big enough. Only change from 8 to 10 during streaming 
won't work and that check can be added.

> 
> Regardless, this is unexpected behavior, so some explanatory comments
> would be useful.
> 
> I'm also not sure whether it isn't better to add cedrus_s_ctrl in the next
> patch, I think it would be more understandable when seen with an actual
> control.

I guess I can move logic to cedrus_try_ctrl(), where some of this logic 
already exists and it would be just a little bit expanded and more selective.

Best regards,
Jernej

> 
> Regards,
> 
> 	Hans
> 
> > +
> > +	return 0;
> > +}
> > +
> > 
> >  static const struct v4l2_ctrl_ops cedrus_ctrl_ops = {
> >  
> >  	.try_ctrl = cedrus_try_ctrl,
> > 
> > +	.s_ctrl = cedrus_s_ctrl,
> > 
> >  };
> >  
> >  static const struct cedrus_control cedrus_controls[] = {
> > 
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > b/drivers/staging/media/sunxi/cedrus/cedrus.h index
> > 5904294f3108..774fe8048ce3 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> > @@ -162,6 +162,8 @@ struct cedrus_dec_ops {
> > 
> >  	int (*start)(struct cedrus_ctx *ctx);
> >  	void (*stop)(struct cedrus_ctx *ctx);
> >  	void (*trigger)(struct cedrus_ctx *ctx);
> > 
> > +	unsigned int (*extra_cap_size)(struct cedrus_ctx *ctx,
> > +				       struct v4l2_pix_format 
*pix_fmt);
> > 
> >  };
> >  
> >  struct cedrus_variant {
> > 
> > diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > b/drivers/staging/media/sunxi/cedrus/cedrus_video.c index
> > dec5d3ae4871..dc67940f1ade 100644
> > --- a/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > +++ b/drivers/staging/media/sunxi/cedrus/cedrus_video.c
> > @@ -250,6 +250,10 @@ static int cedrus_try_fmt_vid_cap_p(struct cedrus_ctx
> > *ctx,> 
> >  	pix_fmt->height = ctx->src_fmt.height;
> >  	cedrus_prepare_format(pix_fmt);
> > 
> > +	if (ctx->current_codec->extra_cap_size)
> > +		pix_fmt->sizeimage +=
> > +			ctx->current_codec->extra_cap_size(ctx, 
pix_fmt);
> > +
> > 
> >  	return 0;
> >  
> >  }





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

end of thread, other threads:[~2022-11-04 18:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-02 18:07 [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Jernej Skrabec
2022-11-02 18:08 ` [PATCH v2 01/11] media: cedrus: remove superfluous call Jernej Skrabec
2022-11-02 18:08 ` [PATCH v2 02/11] media: cedrus: Add format reset helpers Jernej Skrabec
2022-11-02 18:08 ` [PATCH v2 03/11] media: cedrus: use helper to set default formats Jernej Skrabec
2022-11-02 18:08 ` [PATCH v2 04/11] media: cedrus: Add helper for checking capabilities Jernej Skrabec
2022-11-02 18:08 ` [PATCH v2 05/11] media: cedrus: Filter controls based on capability Jernej Skrabec
2022-11-02 18:08 ` [PATCH v2 06/11] media: cedrus: set codec ops immediately Jernej Skrabec
2022-11-02 18:08 ` [PATCH v2 07/11] media: cedrus: Remove cedrus_codec enum Jernej Skrabec
2022-11-02 18:08 ` [PATCH v2 08/11] media: cedrus: prefer untiled capture format Jernej Skrabec
2022-11-02 18:08 ` [PATCH v2 09/11] media: cedrus: initialize controls a bit later Jernej Skrabec
2022-11-02 18:08 ` [PATCH v2 10/11] media: cedrus: Adjust buffer size based on control values Jernej Skrabec
2022-11-04 15:27   ` Hans Verkuil
2022-11-04 18:10     ` Jernej Škrabec
2022-11-02 18:08 ` [PATCH v2 11/11] media: cedrus: h265: Support decoding 10-bit frames Jernej Skrabec
2022-11-04 15:31 ` [PATCH v2 00/11] media: cedrus: Format handling improvements and 10-bit HEVC support Hans Verkuil

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