* [RFC,WIP PATCH 0/2] cedrus: h264: add support for dynamically allocated ctrl arrays
@ 2021-07-02 2:01 daniel.almeida
2021-07-02 2:01 ` [RFC,WIP PATCH 1/2] media: cedrus: fix double free daniel.almeida
2021-07-02 2:01 ` [RFC,WIP PATCH 2/2] Cedrus: add support for dynamic arrays in H264 daniel.almeida
0 siblings, 2 replies; 6+ messages in thread
From: daniel.almeida @ 2021-07-02 2:01 UTC (permalink / raw)
To: hverkuil, jernej.skrabec, ezequiel, paul.kocialkowski, mripard
Cc: kernel, linux-media, Daniel Almeida
From: Daniel Almeida <daniel.almeida@collabora.com>
So far the Cedrus driver is able to decode a slice at a time in slice
mode.
Use the new flag "V4L2_CTRL_FLAG_DYNAMIC_ARRAY" and the new h264 slice
array decode mode to support passing an array with all the slices at once
from userspace.
The device will process all slices in this array before calling
v4l2_m2m_buf_done_and_job_finish, significantly reducing the
amount of back and forth of data.
This is marked as WIP because currently only the first slice will
decode, all subsequent slices in the same frame will return
CEDRUS_IRQ_ERROR. Also I haven't quite polished this yet.
It is marked as RFC because I am not sure whether adding a new entry
in v4l2_stateless_h264_decode_mode was the right call. Also, apparently
only the slice offset is needed for subsequent slices (i.e. slice->header_bit_size),
so I am a bit unsure whether userspace has to fill all fields for slices
2..n
Daniel Almeida (2):
media: cedrus: fix double free
Cedrus: add support for dynamic arrays in H264
drivers/media/v4l2-core/v4l2-ctrls-defs.c | 1 +
drivers/staging/media/sunxi/cedrus/cedrus.c | 35 ++++++++++++--
drivers/staging/media/sunxi/cedrus/cedrus.h | 18 ++++++++
.../staging/media/sunxi/cedrus/cedrus_dec.c | 33 +++++++++++++
.../staging/media/sunxi/cedrus/cedrus_h264.c | 46 ++++++++++++++++---
.../staging/media/sunxi/cedrus/cedrus_hw.c | 35 +++++++++++++-
include/uapi/linux/v4l2-controls.h | 7 +++
7 files changed, 163 insertions(+), 12 deletions(-)
--
2.32.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC,WIP PATCH 1/2] media: cedrus: fix double free
2021-07-02 2:01 [RFC,WIP PATCH 0/2] cedrus: h264: add support for dynamically allocated ctrl arrays daniel.almeida
@ 2021-07-02 2:01 ` daniel.almeida
2021-07-19 8:17 ` Hans Verkuil
2021-07-02 2:01 ` [RFC,WIP PATCH 2/2] Cedrus: add support for dynamic arrays in H264 daniel.almeida
1 sibling, 1 reply; 6+ messages in thread
From: daniel.almeida @ 2021-07-02 2:01 UTC (permalink / raw)
To: hverkuil, jernej.skrabec, ezequiel, paul.kocialkowski, mripard
Cc: kernel, linux-media, Daniel Almeida
From: Daniel Almeida <daniel.almeida@collabora.com>
If v4l2_ctrl_new_custom fails in cedrus_init_ctrls the error path will
free ctx->ctrls, which is also freed in cedrus release. Fix this by
setting ctx->ctrls to NULL instead of inadvertently removing kfree
calls.
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
drivers/staging/media/sunxi/cedrus/cedrus.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index c0d005dafc6c..e72810ace40f 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -207,6 +207,7 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
v4l2_ctrl_handler_free(hdl);
kfree(ctx->ctrls);
+ ctx->ctrls = NULL;
return hdl->error;
}
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC,WIP PATCH 2/2] Cedrus: add support for dynamic arrays in H264
2021-07-02 2:01 [RFC,WIP PATCH 0/2] cedrus: h264: add support for dynamically allocated ctrl arrays daniel.almeida
2021-07-02 2:01 ` [RFC,WIP PATCH 1/2] media: cedrus: fix double free daniel.almeida
@ 2021-07-02 2:01 ` daniel.almeida
2021-07-05 18:10 ` [RFC, WIP " kernel test robot
2021-07-19 8:23 ` [RFC,WIP " Hans Verkuil
1 sibling, 2 replies; 6+ messages in thread
From: daniel.almeida @ 2021-07-02 2:01 UTC (permalink / raw)
To: hverkuil, jernej.skrabec, ezequiel, paul.kocialkowski, mripard
Cc: kernel, linux-media, Daniel Almeida
From: Daniel Almeida <daniel.almeida@collabora.com>
So far the Cedrus driver is able to decode a slice at a time in slice
mode.
Use the new flag "V4L2_CTRL_FLAG_DYNAMIC_ARRAY" and the new h264 slice
decode mode to support passing an array with all the slices at once from
userspace.
The device will process all slices in this array before calling
v4l2_m2m_buf_done_and_job_finish, significantly reducing the
amount of back and forth of data.
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
---
drivers/media/v4l2-core/v4l2-ctrls-defs.c | 1 +
drivers/staging/media/sunxi/cedrus/cedrus.c | 34 ++++++++++++--
drivers/staging/media/sunxi/cedrus/cedrus.h | 18 ++++++++
.../staging/media/sunxi/cedrus/cedrus_dec.c | 33 +++++++++++++
.../staging/media/sunxi/cedrus/cedrus_h264.c | 46 ++++++++++++++++---
.../staging/media/sunxi/cedrus/cedrus_hw.c | 35 +++++++++++++-
include/uapi/linux/v4l2-controls.h | 7 +++
7 files changed, 162 insertions(+), 12 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
index b6344bbf1e00..5c7fe69d7097 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
@@ -367,6 +367,7 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
static const char * const h264_decode_mode[] = {
"Slice-Based",
"Frame-Based",
+ "Slice Array Based",
NULL,
};
static const char * const h264_start_code[] = {
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index e72810ace40f..3b55c1e5a288 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -56,6 +56,11 @@ static const struct cedrus_control cedrus_controls[] = {
{
.cfg = {
.id = V4L2_CID_STATELESS_H264_SLICE_PARAMS,
+ .flags = V4L2_CTRL_FLAG_DYNAMIC_ARRAY,
+ .dims = {32},
+ /* FIXME: I suppose these last two will not be necessary */
+ .type = V4L2_CTRL_TYPE_H264_SLICE_PARAMS,
+ .name = "H264 Slice Parameters",
},
.codec = CEDRUS_CODEC_H264,
},
@@ -86,7 +91,7 @@ static const struct cedrus_control cedrus_controls[] = {
{
.cfg = {
.id = V4L2_CID_STATELESS_H264_DECODE_MODE,
- .max = V4L2_STATELESS_H264_DECODE_MODE_SLICE_BASED,
+ .max = V4L2_STATELESS_H264_DECODE_MODE_SLICE_ARRAY_BASED,
.def = V4L2_STATELESS_H264_DECODE_MODE_SLICE_BASED,
},
.codec = CEDRUS_CODEC_H264,
@@ -167,17 +172,40 @@ static const struct cedrus_control cedrus_controls[] = {
#define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)
-void *cedrus_find_control_data(struct cedrus_ctx *ctx, u32 id)
+struct v4l2_ctrl *cedrus_find_control(struct cedrus_ctx *ctx, u32 id)
{
unsigned int i;
for (i = 0; ctx->ctrls[i]; i++)
if (ctx->ctrls[i]->id == id)
- return ctx->ctrls[i]->p_cur.p;
+ return ctx->ctrls[i];
+
+ return NULL;
+}
+
+void *cedrus_find_control_data(struct cedrus_ctx *ctx, u32 id)
+{
+ struct v4l2_ctrl *ctrl;
+
+ ctrl = cedrus_find_control(ctx, id);
+ if (ctrl)
+ return ctrl->p_cur.p;
return NULL;
}
+u32 cedrus_control_num_elems(struct cedrus_ctx *ctx, u32 id)
+{
+ struct v4l2_ctrl *ctrl;
+
+ ctrl = cedrus_find_control(ctx, id);
+ if (ctrl) {
+ return ctrl->elems;
+ }
+
+ return 0;
+}
+
static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
{
struct v4l2_ctrl_handler *hdl = &ctx->hdl;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 88afba17b78b..b445c1dc2a29 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -58,6 +58,18 @@ struct cedrus_control {
enum cedrus_codec codec;
};
+struct cedrus_h264_slice_ctx {
+ u32 num_slices;
+ u32 cur_slice;
+};
+
+struct cedrus_slice_ctx {
+ void *priv;
+ union {
+ struct cedrus_h264_slice_ctx h264;
+ };
+};
+
struct cedrus_h264_run {
const struct v4l2_ctrl_h264_decode_params *decode_params;
const struct v4l2_ctrl_h264_pps *pps;
@@ -118,6 +130,9 @@ struct cedrus_ctx {
struct v4l2_ctrl_handler hdl;
struct v4l2_ctrl **ctrls;
+ struct cedrus_slice_ctx slice_ctx;
+ bool slice_array_decode_mode;
+
union {
struct {
void *mv_col_buf;
@@ -160,6 +175,7 @@ struct cedrus_dec_ops {
void (*irq_disable)(struct cedrus_ctx *ctx);
enum cedrus_irq_status (*irq_status)(struct cedrus_ctx *ctx);
void (*setup)(struct cedrus_ctx *ctx, struct cedrus_run *run);
+ void (*setup_next_slice)(struct cedrus_ctx *ctx);
int (*start)(struct cedrus_ctx *ctx);
void (*stop)(struct cedrus_ctx *ctx);
void (*trigger)(struct cedrus_ctx *ctx);
@@ -256,5 +272,7 @@ vb2_to_cedrus_buffer(const struct vb2_buffer *p)
}
void *cedrus_find_control_data(struct cedrus_ctx *ctx, u32 id);
+u32 cedrus_control_num_elems(struct cedrus_ctx *ctx, u32 id);
+struct v4l2_ctrl *cedrus_find_control(struct cedrus_ctx *ctx, u32 id);
#endif
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
index 40e8c4123f76..9b9034044aa4 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
@@ -22,6 +22,37 @@
#include "cedrus_dec.h"
#include "cedrus_hw.h"
+static void fill_slice_ctx(struct cedrus_ctx *ctx, struct cedrus_run *run)
+{
+ struct v4l2_ctrl *ctrl;
+
+ ctrl = cedrus_find_control(ctx, V4L2_CID_STATELESS_H264_DECODE_MODE);
+ if (!ctrl)
+ return;
+
+ switch (ctx->current_codec) {
+ case CEDRUS_CODEC_H264:
+ ctx->slice_array_decode_mode =
+ ctrl->cur.val == V4L2_STATELESS_H264_DECODE_MODE_SLICE_ARRAY_BASED;
+
+ if (!ctx->slice_array_decode_mode)
+ return;
+
+ ctx->slice_ctx.h264.num_slices =
+ cedrus_control_num_elems(ctx, V4L2_CID_STATELESS_H264_SLICE_PARAMS);
+ ctx->slice_ctx.priv = kmalloc(sizeof(struct v4l2_ctrl_h264_slice_params) *
+ ctx->slice_ctx.h264.num_slices, GFP_KERNEL);
+
+ memcpy(ctx->slice_ctx.priv,
+ run->h264.slice_params, sizeof(struct v4l2_ctrl_h264_slice_params) *
+ ctx->slice_ctx.h264.num_slices);
+ break;
+
+ default:
+ break;
+ }
+}
+
void cedrus_device_run(void *priv)
{
struct cedrus_ctx *ctx = priv;
@@ -83,6 +114,8 @@ void cedrus_device_run(void *priv)
break;
}
+ fill_slice_ctx(ctx, &run);
+
v4l2_m2m_buf_copy_metadata(run.src, run.dst, true);
cedrus_dst_format_set(dev, &ctx->dst_fmt);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
index de7442d4834d..bada2bf04f24 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
@@ -15,6 +15,8 @@
#include "cedrus_hw.h"
#include "cedrus_regs.h"
+static void cedrus_h264_trigger(struct cedrus_ctx *ctx);
+
enum cedrus_h264_sram_off {
CEDRUS_SRAM_H264_PRED_WEIGHT_TABLE = 0x000,
CEDRUS_SRAM_H264_FRAMEBUFFER_LIST = 0x100,
@@ -318,6 +320,31 @@ static void cedrus_skip_bits(struct cedrus_dev *dev, int num)
}
}
+static void cedrus_h264_setup_next_slice(struct cedrus_ctx *ctx)
+{
+ struct v4l2_ctrl_h264_slice_params *sp;
+ struct cedrus_h264_slice_ctx *sctx;
+ u32 offset;
+
+ sp = ctx->slice_ctx.priv;
+ sctx = &ctx->slice_ctx.h264;
+ offset = sp[sctx->cur_slice++].header_bit_size;
+
+ /* skip to the next slice */
+ cedrus_skip_bits(ctx->dev, offset);
+
+ /* clear status flags */
+ cedrus_write(ctx->dev, VE_H264_STATUS, cedrus_read(ctx->dev, VE_H264_STATUS));
+
+ /* enable int */
+ cedrus_write(ctx->dev, VE_H264_CTRL,
+ VE_H264_CTRL_SLICE_DECODE_INT |
+ VE_H264_CTRL_DECODE_ERR_INT |
+ VE_H264_CTRL_VLD_DATA_REQ_INT);
+
+ cedrus_h264_trigger(ctx);
+}
+
static void cedrus_set_params(struct cedrus_ctx *ctx,
struct cedrus_run *run)
{
@@ -510,6 +537,10 @@ static void cedrus_h264_setup(struct cedrus_ctx *ctx,
cedrus_write_frame_list(ctx, run);
cedrus_set_params(ctx, run);
+
+ if (ctx->slice_array_decode_mode)
+ ctx->slice_ctx.h264.cur_slice++;
+
}
static int cedrus_h264_start(struct cedrus_ctx *ctx)
@@ -682,11 +713,12 @@ static void cedrus_h264_trigger(struct cedrus_ctx *ctx)
}
struct cedrus_dec_ops cedrus_dec_ops_h264 = {
- .irq_clear = cedrus_h264_irq_clear,
- .irq_disable = cedrus_h264_irq_disable,
- .irq_status = cedrus_h264_irq_status,
- .setup = cedrus_h264_setup,
- .start = cedrus_h264_start,
- .stop = cedrus_h264_stop,
- .trigger = cedrus_h264_trigger,
+ .irq_clear = cedrus_h264_irq_clear,
+ .irq_disable = cedrus_h264_irq_disable,
+ .irq_status = cedrus_h264_irq_status,
+ .setup = cedrus_h264_setup,
+ .setup_next_slice = cedrus_h264_setup_next_slice,
+ .start = cedrus_h264_start,
+ .stop = cedrus_h264_stop,
+ .trigger = cedrus_h264_trigger,
};
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
index e2f2ff609c7e..4ff3da210e50 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
@@ -111,12 +111,25 @@ void cedrus_dst_format_set(struct cedrus_dev *dev,
}
}
+static void clear_slice_ctx(struct cedrus_ctx *ctx)
+{
+ switch (ctx->current_codec) {
+ case CEDRUS_CODEC_H264:
+ kfree(ctx->slice_ctx.priv);
+ memset(&ctx->slice_ctx, 0, sizeof(ctx->slice_ctx));
+ break;
+ default:
+ break;
+ }
+}
+
static irqreturn_t cedrus_irq(int irq, void *data)
{
struct cedrus_dev *dev = data;
struct cedrus_ctx *ctx;
enum vb2_buffer_state state;
enum cedrus_irq_status status;
+ struct cedrus_h264_slice_ctx *h264sctx;
ctx = v4l2_m2m_get_curr_priv(dev->m2m_dev);
if (!ctx) {
@@ -132,10 +145,28 @@ static irqreturn_t cedrus_irq(int irq, void *data)
dev->dec_ops[ctx->current_codec]->irq_disable(ctx);
dev->dec_ops[ctx->current_codec]->irq_clear(ctx);
- if (status == CEDRUS_IRQ_ERROR)
+ if (status == CEDRUS_IRQ_OK && ctx->slice_array_decode_mode) {
+ switch (ctx->current_codec) {
+ case CEDRUS_CODEC_H264:
+ h264sctx = &ctx->slice_ctx.h264;
+ if (h264sctx->cur_slice < h264sctx->num_slices - 1) {
+ dev->dec_ops[ctx->current_codec]->setup_next_slice(ctx);
+ return IRQ_HANDLED;
+ }
+ default:
+ break;
+ }
+
+ clear_slice_ctx(ctx);
+ }
+
+ if (status == CEDRUS_IRQ_ERROR) {
state = VB2_BUF_STATE_ERROR;
- else
+ if (ctx->slice_array_decode_mode)
+ clear_slice_ctx(ctx);
+ } else {
state = VB2_BUF_STATE_DONE;
+ }
v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx,
state);
diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
index fdf97a6d7d18..8907076d0ddf 100644
--- a/include/uapi/linux/v4l2-controls.h
+++ b/include/uapi/linux/v4l2-controls.h
@@ -1231,10 +1231,17 @@ enum v4l2_detect_md_mode {
* by device drivers that are able to parse the slice(s) header(s)
* in hardware. When this mode is selected,
* V4L2_CID_STATELESS_H264_SLICE_PARAMS is not used.
+ * @V4L2_STATELESS_H264_DECODE_MODE_SLICE_ARRAY_BASED: indicates that
+ * decoding is performed for the entire frame using a slice array.
+ * When this mode is selected, a pointer to a contiguous memory region
+ * of v4l2_ctrl_h264_slice elements is expected.
+
*/
enum v4l2_stateless_h264_decode_mode {
V4L2_STATELESS_H264_DECODE_MODE_SLICE_BASED,
V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
+ V4L2_STATELESS_H264_DECODE_MODE_SLICE_ARRAY_BASED,
+
};
#define V4L2_CID_STATELESS_H264_START_CODE (V4L2_CID_CODEC_STATELESS_BASE + 1)
--
2.32.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC, WIP PATCH 2/2] Cedrus: add support for dynamic arrays in H264
2021-07-02 2:01 ` [RFC,WIP PATCH 2/2] Cedrus: add support for dynamic arrays in H264 daniel.almeida
@ 2021-07-05 18:10 ` kernel test robot
2021-07-19 8:23 ` [RFC,WIP " Hans Verkuil
1 sibling, 0 replies; 6+ messages in thread
From: kernel test robot @ 2021-07-05 18:10 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 18444 bytes --]
Hi,
[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on linuxtv-media/master]
[also build test ERROR on next-20210701]
[cannot apply to sunxi/sunxi/for-next v5.13]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/daniel-almeida-collabora-com/cedrus-h264-add-support-for-dynamically-allocated-ctrl-arrays/20210702-100300
base: git://linuxtv.org/media_tree.git master
config: riscv-randconfig-r011-20210705 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 3f9bf9f42a9043e20c6d2a74dd4f47a90a7e2b41)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/a7e6f08c21b9153ac6bd1b556f389d658a3b8cc8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review daniel-almeida-collabora-com/cedrus-h264-add-support-for-dynamically-allocated-ctrl-arrays/20210702-100300
git checkout a7e6f08c21b9153ac6bd1b556f389d658a3b8cc8
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=riscv SHELL=/bin/bash drivers/staging/media/sunxi/cedrus/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:87:48: note: expanded from macro 'readb_cpu'
#define readb_cpu(c) ({ u8 __r = __raw_readb(c); __r; })
^
In file included from drivers/staging/media/sunxi/cedrus/cedrus.c:21:
In file included from include/media/v4l2-device.h:13:
In file included from include/media/v4l2-subdev.h:15:
In file included from include/media/v4l2-common.h:270:
In file included from include/linux/spi/spi.h:15:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:564:9: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return inw(addr);
^~~~~~~~~
arch/riscv/include/asm/io.h:56:76: note: expanded from macro 'inw'
#define inw(c) ({ u16 __v; __io_pbr(); __v = readw_cpu((void*)(PCI_IOBASE + (c))); __io_par(__v); __v; })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:88:76: note: expanded from macro 'readw_cpu'
#define readw_cpu(c) ({ u16 __r = le16_to_cpu((__force __le16)__raw_readw(c)); __r; })
^
include/uapi/linux/byteorder/little_endian.h:36:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/staging/media/sunxi/cedrus/cedrus.c:21:
In file included from include/media/v4l2-device.h:13:
In file included from include/media/v4l2-subdev.h:15:
In file included from include/media/v4l2-common.h:270:
In file included from include/linux/spi/spi.h:15:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:572:9: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return inl(addr);
^~~~~~~~~
arch/riscv/include/asm/io.h:57:76: note: expanded from macro 'inl'
#define inl(c) ({ u32 __v; __io_pbr(); __v = readl_cpu((void*)(PCI_IOBASE + (c))); __io_par(__v); __v; })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:89:76: note: expanded from macro 'readl_cpu'
#define readl_cpu(c) ({ u32 __r = le32_to_cpu((__force __le32)__raw_readl(c)); __r; })
^
include/uapi/linux/byteorder/little_endian.h:34:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/staging/media/sunxi/cedrus/cedrus.c:21:
In file included from include/media/v4l2-device.h:13:
In file included from include/media/v4l2-subdev.h:15:
In file included from include/media/v4l2-common.h:270:
In file included from include/linux/spi/spi.h:15:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:580:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
outb(value, addr);
^~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:59:68: note: expanded from macro 'outb'
#define outb(v,c) ({ __io_pbw(); writeb_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:91:52: note: expanded from macro 'writeb_cpu'
#define writeb_cpu(v, c) ((void)__raw_writeb((v), (c)))
^
In file included from drivers/staging/media/sunxi/cedrus/cedrus.c:21:
In file included from include/media/v4l2-device.h:13:
In file included from include/media/v4l2-subdev.h:15:
In file included from include/media/v4l2-common.h:270:
In file included from include/linux/spi/spi.h:15:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:588:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
outw(value, addr);
^~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:60:68: note: expanded from macro 'outw'
#define outw(v,c) ({ __io_pbw(); writew_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:92:76: note: expanded from macro 'writew_cpu'
#define writew_cpu(v, c) ((void)__raw_writew((__force u16)cpu_to_le16(v), (c)))
^
In file included from drivers/staging/media/sunxi/cedrus/cedrus.c:21:
In file included from include/media/v4l2-device.h:13:
In file included from include/media/v4l2-subdev.h:15:
In file included from include/media/v4l2-common.h:270:
In file included from include/linux/spi/spi.h:15:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:596:2: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
outl(value, addr);
^~~~~~~~~~~~~~~~~
arch/riscv/include/asm/io.h:61:68: note: expanded from macro 'outl'
#define outl(v,c) ({ __io_pbw(); writel_cpu((v),(void*)(PCI_IOBASE + (c))); __io_paw(); })
~~~~~~~~~~ ^
arch/riscv/include/asm/mmio.h:93:76: note: expanded from macro 'writel_cpu'
#define writel_cpu(v, c) ((void)__raw_writel((__force u32)cpu_to_le32(v), (c)))
^
In file included from drivers/staging/media/sunxi/cedrus/cedrus.c:21:
In file included from include/media/v4l2-device.h:13:
In file included from include/media/v4l2-subdev.h:15:
In file included from include/media/v4l2-common.h:270:
In file included from include/linux/spi/spi.h:15:
In file included from include/linux/scatterlist.h:9:
In file included from arch/riscv/include/asm/io.h:149:
include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
~~~~~~~~~~ ^
>> drivers/staging/media/sunxi/cedrus/cedrus.c:59:14: error: use of undeclared identifier 'V4L2_CTRL_FLAG_DYNAMIC_ARRAY'
.flags = V4L2_CTRL_FLAG_DYNAMIC_ARRAY,
^
>> drivers/staging/media/sunxi/cedrus/cedrus.c:216:30: error: invalid application of 'sizeof' to an incomplete type 'const struct cedrus_control []'
v4l2_ctrl_handler_init(hdl, CEDRUS_CONTROLS_COUNT);
~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
drivers/staging/media/sunxi/cedrus/cedrus.c:173:31: note: expanded from macro 'CEDRUS_CONTROLS_COUNT'
#define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)
^
include/linux/kernel.h:42:32: note: expanded from macro 'ARRAY_SIZE'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^
include/media/v4l2-ctrls.h:513:37: note: expanded from macro 'v4l2_ctrl_handler_init'
v4l2_ctrl_handler_init_class(hdl, nr_of_controls_hint, \
^~~~~~~~~~~~~~~~~~~
drivers/staging/media/sunxi/cedrus/cedrus.c:223:29: error: invalid application of 'sizeof' to an incomplete type 'const struct cedrus_control []'
ctrl_size = sizeof(ctrl) * CEDRUS_CONTROLS_COUNT + 1;
^~~~~~~~~~~~~~~~~~~~~
drivers/staging/media/sunxi/cedrus/cedrus.c:173:31: note: expanded from macro 'CEDRUS_CONTROLS_COUNT'
#define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/kernel.h:42:32: note: expanded from macro 'ARRAY_SIZE'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^~~~~
drivers/staging/media/sunxi/cedrus/cedrus.c:229:18: error: invalid application of 'sizeof' to an incomplete type 'const struct cedrus_control []'
for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
^~~~~~~~~~~~~~~~~~~~~
drivers/staging/media/sunxi/cedrus/cedrus.c:173:31: note: expanded from macro 'CEDRUS_CONTROLS_COUNT'
#define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/kernel.h:42:32: note: expanded from macro 'ARRAY_SIZE'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^~~~~
7 warnings and 4 errors generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for LOCKDEP
Depends on DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT && (FRAME_POINTER || MIPS || PPC || S390 || MICROBLAZE || ARM || ARC || X86)
Selected by
- PROVE_LOCKING && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
- DEBUG_LOCK_ALLOC && DEBUG_KERNEL && LOCK_DEBUGGING_SUPPORT
vim +/V4L2_CTRL_FLAG_DYNAMIC_ARRAY +59 drivers/staging/media/sunxi/cedrus/cedrus.c
30
31 static const struct cedrus_control cedrus_controls[] = {
32 {
33 .cfg = {
34 .id = V4L2_CID_STATELESS_MPEG2_SEQUENCE,
35 },
36 .codec = CEDRUS_CODEC_MPEG2,
37 },
38 {
39 .cfg = {
40 .id = V4L2_CID_STATELESS_MPEG2_PICTURE,
41 },
42 .codec = CEDRUS_CODEC_MPEG2,
43 },
44 {
45 .cfg = {
46 .id = V4L2_CID_STATELESS_MPEG2_QUANTISATION,
47 },
48 .codec = CEDRUS_CODEC_MPEG2,
49 },
50 {
51 .cfg = {
52 .id = V4L2_CID_STATELESS_H264_DECODE_PARAMS,
53 },
54 .codec = CEDRUS_CODEC_H264,
55 },
56 {
57 .cfg = {
58 .id = V4L2_CID_STATELESS_H264_SLICE_PARAMS,
> 59 .flags = V4L2_CTRL_FLAG_DYNAMIC_ARRAY,
60 .dims = {32},
61 /* FIXME: I suppose these last two will not be necessary */
62 .type = V4L2_CTRL_TYPE_H264_SLICE_PARAMS,
63 .name = "H264 Slice Parameters",
64 },
65 .codec = CEDRUS_CODEC_H264,
66 },
67 {
68 .cfg = {
69 .id = V4L2_CID_STATELESS_H264_SPS,
70 },
71 .codec = CEDRUS_CODEC_H264,
72 },
73 {
74 .cfg = {
75 .id = V4L2_CID_STATELESS_H264_PPS,
76 },
77 .codec = CEDRUS_CODEC_H264,
78 },
79 {
80 .cfg = {
81 .id = V4L2_CID_STATELESS_H264_SCALING_MATRIX,
82 },
83 .codec = CEDRUS_CODEC_H264,
84 },
85 {
86 .cfg = {
87 .id = V4L2_CID_STATELESS_H264_PRED_WEIGHTS,
88 },
89 .codec = CEDRUS_CODEC_H264,
90 },
91 {
92 .cfg = {
93 .id = V4L2_CID_STATELESS_H264_DECODE_MODE,
94 .max = V4L2_STATELESS_H264_DECODE_MODE_SLICE_ARRAY_BASED,
95 .def = V4L2_STATELESS_H264_DECODE_MODE_SLICE_BASED,
96 },
97 .codec = CEDRUS_CODEC_H264,
98 },
99 {
100 .cfg = {
101 .id = V4L2_CID_STATELESS_H264_START_CODE,
102 .max = V4L2_STATELESS_H264_START_CODE_NONE,
103 .def = V4L2_STATELESS_H264_START_CODE_NONE,
104 },
105 .codec = CEDRUS_CODEC_H264,
106 },
107 /*
108 * We only expose supported profiles information,
109 * and not levels as it's not clear what is supported
110 * for each hardware/core version.
111 * In any case, TRY/S_FMT will clamp the format resolution
112 * to the maximum supported.
113 */
114 {
115 .cfg = {
116 .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
117 .min = V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE,
118 .def = V4L2_MPEG_VIDEO_H264_PROFILE_MAIN,
119 .max = V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
120 .menu_skip_mask =
121 BIT(V4L2_MPEG_VIDEO_H264_PROFILE_EXTENDED),
122 },
123 .codec = CEDRUS_CODEC_H264,
124 },
125 {
126 .cfg = {
127 .id = V4L2_CID_MPEG_VIDEO_HEVC_SPS,
128 },
129 .codec = CEDRUS_CODEC_H265,
130 },
131 {
132 .cfg = {
133 .id = V4L2_CID_MPEG_VIDEO_HEVC_PPS,
134 },
135 .codec = CEDRUS_CODEC_H265,
136 },
137 {
138 .cfg = {
139 .id = V4L2_CID_MPEG_VIDEO_HEVC_SLICE_PARAMS,
140 },
141 .codec = CEDRUS_CODEC_H265,
142 },
143 {
144 .cfg = {
145 .id = V4L2_CID_MPEG_VIDEO_HEVC_DECODE_MODE,
146 .max = V4L2_MPEG_VIDEO_HEVC_DECODE_MODE_SLICE_BASED,
147 .def = V4L2_MPEG_VIDEO_HEVC_DECODE_MODE_SLICE_BASED,
148 },
149 .codec = CEDRUS_CODEC_H265,
150 },
151 {
152 .cfg = {
153 .id = V4L2_CID_MPEG_VIDEO_HEVC_START_CODE,
154 .max = V4L2_MPEG_VIDEO_HEVC_START_CODE_NONE,
155 .def = V4L2_MPEG_VIDEO_HEVC_START_CODE_NONE,
156 },
157 .codec = CEDRUS_CODEC_H265,
158 },
159 {
160 .cfg = {
161 .id = V4L2_CID_STATELESS_VP8_FRAME,
162 },
163 .codec = CEDRUS_CODEC_VP8,
164 },
165 {
166 .cfg = {
167 .id = V4L2_CID_MPEG_VIDEO_HEVC_DECODE_PARAMS,
168 },
169 .codec = CEDRUS_CODEC_H265,
170 },
171 };
172
173 #define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)
174
175 struct v4l2_ctrl *cedrus_find_control(struct cedrus_ctx *ctx, u32 id)
176 {
177 unsigned int i;
178
179 for (i = 0; ctx->ctrls[i]; i++)
180 if (ctx->ctrls[i]->id == id)
181 return ctx->ctrls[i];
182
183 return NULL;
184 }
185
186 void *cedrus_find_control_data(struct cedrus_ctx *ctx, u32 id)
187 {
188 struct v4l2_ctrl *ctrl;
189
190 ctrl = cedrus_find_control(ctx, id);
191 if (ctrl)
192 return ctrl->p_cur.p;
193
194 return NULL;
195 }
196
197 u32 cedrus_control_num_elems(struct cedrus_ctx *ctx, u32 id)
198 {
199 struct v4l2_ctrl *ctrl;
200
201 ctrl = cedrus_find_control(ctx, id);
202 if (ctrl) {
203 return ctrl->elems;
204 }
205
206 return 0;
207 }
208
209 static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
210 {
211 struct v4l2_ctrl_handler *hdl = &ctx->hdl;
212 struct v4l2_ctrl *ctrl;
213 unsigned int ctrl_size;
214 unsigned int i;
215
> 216 v4l2_ctrl_handler_init(hdl, CEDRUS_CONTROLS_COUNT);
217 if (hdl->error) {
218 v4l2_err(&dev->v4l2_dev,
219 "Failed to initialize control handler\n");
220 return hdl->error;
221 }
222
223 ctrl_size = sizeof(ctrl) * CEDRUS_CONTROLS_COUNT + 1;
224
225 ctx->ctrls = kzalloc(ctrl_size, GFP_KERNEL);
226 if (!ctx->ctrls)
227 return -ENOMEM;
228
229 for (i = 0; i < CEDRUS_CONTROLS_COUNT; i++) {
230 ctrl = v4l2_ctrl_new_custom(hdl, &cedrus_controls[i].cfg,
231 NULL);
232 if (hdl->error) {
233 v4l2_err(&dev->v4l2_dev,
234 "Failed to create new custom control\n");
235
236 v4l2_ctrl_handler_free(hdl);
237 kfree(ctx->ctrls);
238 ctx->ctrls = NULL;
239 return hdl->error;
240 }
241
242 ctx->ctrls[i] = ctrl;
243 }
244
245 ctx->fh.ctrl_handler = hdl;
246 v4l2_ctrl_handler_setup(hdl);
247
248 return 0;
249 }
250
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30857 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC,WIP PATCH 1/2] media: cedrus: fix double free
2021-07-02 2:01 ` [RFC,WIP PATCH 1/2] media: cedrus: fix double free daniel.almeida
@ 2021-07-19 8:17 ` Hans Verkuil
0 siblings, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2021-07-19 8:17 UTC (permalink / raw)
To: daniel.almeida, jernej.skrabec, ezequiel, paul.kocialkowski, mripard
Cc: kernel, linux-media
On 02/07/2021 04:01, daniel.almeida@collabora.com wrote:
> From: Daniel Almeida <daniel.almeida@collabora.com>
>
> If v4l2_ctrl_new_custom fails in cedrus_init_ctrls the error path will
> free ctx->ctrls, which is also freed in cedrus release. Fix this by
> setting ctx->ctrls to NULL instead of inadvertently removing kfree
> calls.
This is just a bug fix, right? Not really an RFC,WIP.
Regards,
Hans
>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> ---
> drivers/staging/media/sunxi/cedrus/cedrus.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index c0d005dafc6c..e72810ace40f 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -207,6 +207,7 @@ static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
>
> v4l2_ctrl_handler_free(hdl);
> kfree(ctx->ctrls);
> + ctx->ctrls = NULL;
> return hdl->error;
> }
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC,WIP PATCH 2/2] Cedrus: add support for dynamic arrays in H264
2021-07-02 2:01 ` [RFC,WIP PATCH 2/2] Cedrus: add support for dynamic arrays in H264 daniel.almeida
2021-07-05 18:10 ` [RFC, WIP " kernel test robot
@ 2021-07-19 8:23 ` Hans Verkuil
1 sibling, 0 replies; 6+ messages in thread
From: Hans Verkuil @ 2021-07-19 8:23 UTC (permalink / raw)
To: daniel.almeida, jernej.skrabec, ezequiel, paul.kocialkowski, mripard
Cc: kernel, linux-media
On 02/07/2021 04:01, daniel.almeida@collabora.com wrote:
> From: Daniel Almeida <daniel.almeida@collabora.com>
>
> So far the Cedrus driver is able to decode a slice at a time in slice
> mode.
>
> Use the new flag "V4L2_CTRL_FLAG_DYNAMIC_ARRAY" and the new h264 slice
> decode mode to support passing an array with all the slices at once from
> userspace.
>
> The device will process all slices in this array before calling
> v4l2_m2m_buf_done_and_job_finish, significantly reducing the
> amount of back and forth of data.
>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> ---
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 1 +
> drivers/staging/media/sunxi/cedrus/cedrus.c | 34 ++++++++++++--
> drivers/staging/media/sunxi/cedrus/cedrus.h | 18 ++++++++
> .../staging/media/sunxi/cedrus/cedrus_dec.c | 33 +++++++++++++
> .../staging/media/sunxi/cedrus/cedrus_h264.c | 46 ++++++++++++++++---
> .../staging/media/sunxi/cedrus/cedrus_hw.c | 35 +++++++++++++-
> include/uapi/linux/v4l2-controls.h | 7 +++
> 7 files changed, 162 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-defs.c b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> index b6344bbf1e00..5c7fe69d7097 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-defs.c
> @@ -367,6 +367,7 @@ const char * const *v4l2_ctrl_get_menu(u32 id)
> static const char * const h264_decode_mode[] = {
> "Slice-Based",
> "Frame-Based",
> + "Slice Array Based",
> NULL,
> };
> static const char * const h264_start_code[] = {
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
> index e72810ace40f..3b55c1e5a288 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
> @@ -56,6 +56,11 @@ static const struct cedrus_control cedrus_controls[] = {
> {
> .cfg = {
> .id = V4L2_CID_STATELESS_H264_SLICE_PARAMS,
This will have to be a new id (V4L2_CID_STATELESS_H264_SLICES_PARAMS?) since
you cannot change the type of an existing control. So you will have to support
both V4L2_CID_STATELESS_H264_SLICE_PARAMS and the new id for slice arrays.
> + .flags = V4L2_CTRL_FLAG_DYNAMIC_ARRAY,
> + .dims = {32},
Add space after/before { and }
I think a comment explaining where the 32 comes from would be useful as well.
I assume it is the max number of slices that the HW can handle?
> + /* FIXME: I suppose these last two will not be necessary */
> + .type = V4L2_CTRL_TYPE_H264_SLICE_PARAMS,
> + .name = "H264 Slice Parameters",
> },
> .codec = CEDRUS_CODEC_H264,
> },
> @@ -86,7 +91,7 @@ static const struct cedrus_control cedrus_controls[] = {
> {
> .cfg = {
> .id = V4L2_CID_STATELESS_H264_DECODE_MODE,
> - .max = V4L2_STATELESS_H264_DECODE_MODE_SLICE_BASED,
> + .max = V4L2_STATELESS_H264_DECODE_MODE_SLICE_ARRAY_BASED,
> .def = V4L2_STATELESS_H264_DECODE_MODE_SLICE_BASED,
> },
> .codec = CEDRUS_CODEC_H264,
> @@ -167,17 +172,40 @@ static const struct cedrus_control cedrus_controls[] = {
>
> #define CEDRUS_CONTROLS_COUNT ARRAY_SIZE(cedrus_controls)
>
> -void *cedrus_find_control_data(struct cedrus_ctx *ctx, u32 id)
> +struct v4l2_ctrl *cedrus_find_control(struct cedrus_ctx *ctx, u32 id)
> {
> unsigned int i;
>
> for (i = 0; ctx->ctrls[i]; i++)
> if (ctx->ctrls[i]->id == id)
> - return ctx->ctrls[i]->p_cur.p;
> + return ctx->ctrls[i];
> +
> + return NULL;
> +}
> +
> +void *cedrus_find_control_data(struct cedrus_ctx *ctx, u32 id)
> +{
> + struct v4l2_ctrl *ctrl;
> +
> + ctrl = cedrus_find_control(ctx, id);
> + if (ctrl)
> + return ctrl->p_cur.p;
>
> return NULL;
> }
>
> +u32 cedrus_control_num_elems(struct cedrus_ctx *ctx, u32 id)
> +{
> + struct v4l2_ctrl *ctrl;
> +
> + ctrl = cedrus_find_control(ctx, id);
> + if (ctrl) {
> + return ctrl->elems;
> + }
> +
> + return 0;
> +}
> +
> static int cedrus_init_ctrls(struct cedrus_dev *dev, struct cedrus_ctx *ctx)
> {
> struct v4l2_ctrl_handler *hdl = &ctx->hdl;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
> index 88afba17b78b..b445c1dc2a29 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -58,6 +58,18 @@ struct cedrus_control {
> enum cedrus_codec codec;
> };
>
> +struct cedrus_h264_slice_ctx {
> + u32 num_slices;
> + u32 cur_slice;
> +};
> +
> +struct cedrus_slice_ctx {
> + void *priv;
> + union {
> + struct cedrus_h264_slice_ctx h264;
> + };
> +};
> +
> struct cedrus_h264_run {
> const struct v4l2_ctrl_h264_decode_params *decode_params;
> const struct v4l2_ctrl_h264_pps *pps;
> @@ -118,6 +130,9 @@ struct cedrus_ctx {
> struct v4l2_ctrl_handler hdl;
> struct v4l2_ctrl **ctrls;
>
> + struct cedrus_slice_ctx slice_ctx;
> + bool slice_array_decode_mode;
> +
> union {
> struct {
> void *mv_col_buf;
> @@ -160,6 +175,7 @@ struct cedrus_dec_ops {
> void (*irq_disable)(struct cedrus_ctx *ctx);
> enum cedrus_irq_status (*irq_status)(struct cedrus_ctx *ctx);
> void (*setup)(struct cedrus_ctx *ctx, struct cedrus_run *run);
> + void (*setup_next_slice)(struct cedrus_ctx *ctx);
> int (*start)(struct cedrus_ctx *ctx);
> void (*stop)(struct cedrus_ctx *ctx);
> void (*trigger)(struct cedrus_ctx *ctx);
> @@ -256,5 +272,7 @@ vb2_to_cedrus_buffer(const struct vb2_buffer *p)
> }
>
> void *cedrus_find_control_data(struct cedrus_ctx *ctx, u32 id);
> +u32 cedrus_control_num_elems(struct cedrus_ctx *ctx, u32 id);
> +struct v4l2_ctrl *cedrus_find_control(struct cedrus_ctx *ctx, u32 id);
>
> #endif
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> index 40e8c4123f76..9b9034044aa4 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c
> @@ -22,6 +22,37 @@
> #include "cedrus_dec.h"
> #include "cedrus_hw.h"
>
> +static void fill_slice_ctx(struct cedrus_ctx *ctx, struct cedrus_run *run)
> +{
> + struct v4l2_ctrl *ctrl;
> +
> + ctrl = cedrus_find_control(ctx, V4L2_CID_STATELESS_H264_DECODE_MODE);
> + if (!ctrl)
> + return;
> +
> + switch (ctx->current_codec) {
> + case CEDRUS_CODEC_H264:
> + ctx->slice_array_decode_mode =
> + ctrl->cur.val == V4L2_STATELESS_H264_DECODE_MODE_SLICE_ARRAY_BASED;
> +
> + if (!ctx->slice_array_decode_mode)
> + return;
> +
> + ctx->slice_ctx.h264.num_slices =
> + cedrus_control_num_elems(ctx, V4L2_CID_STATELESS_H264_SLICE_PARAMS);
> + ctx->slice_ctx.priv = kmalloc(sizeof(struct v4l2_ctrl_h264_slice_params) *
> + ctx->slice_ctx.h264.num_slices, GFP_KERNEL);
> +
> + memcpy(ctx->slice_ctx.priv,
> + run->h264.slice_params, sizeof(struct v4l2_ctrl_h264_slice_params) *
> + ctx->slice_ctx.h264.num_slices);
> + break;
> +
> + default:
> + break;
> + }
> +}
> +
> void cedrus_device_run(void *priv)
> {
> struct cedrus_ctx *ctx = priv;
> @@ -83,6 +114,8 @@ void cedrus_device_run(void *priv)
> break;
> }
>
> + fill_slice_ctx(ctx, &run);
> +
> v4l2_m2m_buf_copy_metadata(run.src, run.dst, true);
>
> cedrus_dst_format_set(dev, &ctx->dst_fmt);
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> index de7442d4834d..bada2bf04f24 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> @@ -15,6 +15,8 @@
> #include "cedrus_hw.h"
> #include "cedrus_regs.h"
>
> +static void cedrus_h264_trigger(struct cedrus_ctx *ctx);
> +
> enum cedrus_h264_sram_off {
> CEDRUS_SRAM_H264_PRED_WEIGHT_TABLE = 0x000,
> CEDRUS_SRAM_H264_FRAMEBUFFER_LIST = 0x100,
> @@ -318,6 +320,31 @@ static void cedrus_skip_bits(struct cedrus_dev *dev, int num)
> }
> }
>
> +static void cedrus_h264_setup_next_slice(struct cedrus_ctx *ctx)
> +{
> + struct v4l2_ctrl_h264_slice_params *sp;
> + struct cedrus_h264_slice_ctx *sctx;
> + u32 offset;
> +
> + sp = ctx->slice_ctx.priv;
> + sctx = &ctx->slice_ctx.h264;
> + offset = sp[sctx->cur_slice++].header_bit_size;
> +
> + /* skip to the next slice */
> + cedrus_skip_bits(ctx->dev, offset);
> +
> + /* clear status flags */
> + cedrus_write(ctx->dev, VE_H264_STATUS, cedrus_read(ctx->dev, VE_H264_STATUS));
> +
> + /* enable int */
> + cedrus_write(ctx->dev, VE_H264_CTRL,
> + VE_H264_CTRL_SLICE_DECODE_INT |
> + VE_H264_CTRL_DECODE_ERR_INT |
> + VE_H264_CTRL_VLD_DATA_REQ_INT);
> +
> + cedrus_h264_trigger(ctx);
> +}
> +
> static void cedrus_set_params(struct cedrus_ctx *ctx,
> struct cedrus_run *run)
> {
> @@ -510,6 +537,10 @@ static void cedrus_h264_setup(struct cedrus_ctx *ctx,
> cedrus_write_frame_list(ctx, run);
>
> cedrus_set_params(ctx, run);
> +
> + if (ctx->slice_array_decode_mode)
> + ctx->slice_ctx.h264.cur_slice++;
> +
> }
>
> static int cedrus_h264_start(struct cedrus_ctx *ctx)
> @@ -682,11 +713,12 @@ static void cedrus_h264_trigger(struct cedrus_ctx *ctx)
> }
>
> struct cedrus_dec_ops cedrus_dec_ops_h264 = {
> - .irq_clear = cedrus_h264_irq_clear,
> - .irq_disable = cedrus_h264_irq_disable,
> - .irq_status = cedrus_h264_irq_status,
> - .setup = cedrus_h264_setup,
> - .start = cedrus_h264_start,
> - .stop = cedrus_h264_stop,
> - .trigger = cedrus_h264_trigger,
> + .irq_clear = cedrus_h264_irq_clear,
> + .irq_disable = cedrus_h264_irq_disable,
> + .irq_status = cedrus_h264_irq_status,
> + .setup = cedrus_h264_setup,
> + .setup_next_slice = cedrus_h264_setup_next_slice,
> + .start = cedrus_h264_start,
> + .stop = cedrus_h264_stop,
> + .trigger = cedrus_h264_trigger,
> };
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> index e2f2ff609c7e..4ff3da210e50 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_hw.c
> @@ -111,12 +111,25 @@ void cedrus_dst_format_set(struct cedrus_dev *dev,
> }
> }
>
> +static void clear_slice_ctx(struct cedrus_ctx *ctx)
> +{
> + switch (ctx->current_codec) {
> + case CEDRUS_CODEC_H264:
> + kfree(ctx->slice_ctx.priv);
> + memset(&ctx->slice_ctx, 0, sizeof(ctx->slice_ctx));
> + break;
> + default:
> + break;
> + }
> +}
> +
> static irqreturn_t cedrus_irq(int irq, void *data)
> {
> struct cedrus_dev *dev = data;
> struct cedrus_ctx *ctx;
> enum vb2_buffer_state state;
> enum cedrus_irq_status status;
> + struct cedrus_h264_slice_ctx *h264sctx;
>
> ctx = v4l2_m2m_get_curr_priv(dev->m2m_dev);
> if (!ctx) {
> @@ -132,10 +145,28 @@ static irqreturn_t cedrus_irq(int irq, void *data)
> dev->dec_ops[ctx->current_codec]->irq_disable(ctx);
> dev->dec_ops[ctx->current_codec]->irq_clear(ctx);
>
> - if (status == CEDRUS_IRQ_ERROR)
> + if (status == CEDRUS_IRQ_OK && ctx->slice_array_decode_mode) {
> + switch (ctx->current_codec) {
> + case CEDRUS_CODEC_H264:
> + h264sctx = &ctx->slice_ctx.h264;
> + if (h264sctx->cur_slice < h264sctx->num_slices - 1) {
> + dev->dec_ops[ctx->current_codec]->setup_next_slice(ctx);
> + return IRQ_HANDLED;
> + }
> + default:
> + break;
> + }
> +
> + clear_slice_ctx(ctx);
> + }
> +
> + if (status == CEDRUS_IRQ_ERROR) {
> state = VB2_BUF_STATE_ERROR;
> - else
> + if (ctx->slice_array_decode_mode)
> + clear_slice_ctx(ctx);
> + } else {
> state = VB2_BUF_STATE_DONE;
> + }
>
> v4l2_m2m_buf_done_and_job_finish(ctx->dev->m2m_dev, ctx->fh.m2m_ctx,
> state);
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index fdf97a6d7d18..8907076d0ddf 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -1231,10 +1231,17 @@ enum v4l2_detect_md_mode {
> * by device drivers that are able to parse the slice(s) header(s)
> * in hardware. When this mode is selected,
> * V4L2_CID_STATELESS_H264_SLICE_PARAMS is not used.
> + * @V4L2_STATELESS_H264_DECODE_MODE_SLICE_ARRAY_BASED: indicates that
> + * decoding is performed for the entire frame using a slice array.
> + * When this mode is selected, a pointer to a contiguous memory region
> + * of v4l2_ctrl_h264_slice elements is expected.
> +
> */
> enum v4l2_stateless_h264_decode_mode {
> V4L2_STATELESS_H264_DECODE_MODE_SLICE_BASED,
> V4L2_STATELESS_H264_DECODE_MODE_FRAME_BASED,
> + V4L2_STATELESS_H264_DECODE_MODE_SLICE_ARRAY_BASED,
> +
> };
>
> #define V4L2_CID_STATELESS_H264_START_CODE (V4L2_CID_CODEC_STATELESS_BASE + 1)
>
Regards,
Hans
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-07-19 9:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-02 2:01 [RFC,WIP PATCH 0/2] cedrus: h264: add support for dynamically allocated ctrl arrays daniel.almeida
2021-07-02 2:01 ` [RFC,WIP PATCH 1/2] media: cedrus: fix double free daniel.almeida
2021-07-19 8:17 ` Hans Verkuil
2021-07-02 2:01 ` [RFC,WIP PATCH 2/2] Cedrus: add support for dynamic arrays in H264 daniel.almeida
2021-07-05 18:10 ` [RFC, WIP " kernel test robot
2021-07-19 8:23 ` [RFC,WIP " Hans Verkuil
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.