Re: [PATCH v4 08/15] media: rkvdec: Add generic configuration for variants
From: Jonas Karlman <jonas@kwiboo.se>
Date: 2025-10-22 20:57:39
Also in:
linux-media, linux-rockchip, lkml
Hi Detlev, On 10/22/2025 7:45 PM, Detlev Casanova wrote:
quoted hunk ↗ jump to hunk
This is to prepare for adding new versions of the decoder and support specific formats and ops per version. Different rkvdec_variant instances will be able to share generic decoder configs. Tested-by: Diederik de Haas <redacted> # Rock 5B Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> --- .../media/platform/rockchip/rkvdec/rkvdec.c | 37 ++++++++++++------- .../media/platform/rockchip/rkvdec/rkvdec.h | 6 +++ 2 files changed, 30 insertions(+), 13 deletions(-)diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.c b/drivers/media/platform/rockchip/rkvdec/rkvdec.c index 776149f871b0..a7af1e3fdebd 100644 --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.c +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.c@@ -373,15 +373,16 @@ static bool rkvdec_is_capable(struct rkvdec_ctx *ctx, unsigned int capability) static const struct rkvdec_coded_fmt_desc * rkvdec_enum_coded_fmt_desc(struct rkvdec_ctx *ctx, int index) { + const struct rkvdec_config *cfg = ctx->dev->variant->config; int fmt_idx = -1; unsigned int i; - for (i = 0; i < ARRAY_SIZE(rkvdec_coded_fmts); i++) { - if (!rkvdec_is_capable(ctx, rkvdec_coded_fmts[i].capability)) + for (i = 0; i < cfg->coded_fmts_num; i++) { + if (!rkvdec_is_capable(ctx, cfg->coded_fmts[i].capability)) continue; fmt_idx++; if (index == fmt_idx) - return &rkvdec_coded_fmts[i]; + return &cfg->coded_fmts[i]; } return NULL;@@ -390,12 +391,13 @@ rkvdec_enum_coded_fmt_desc(struct rkvdec_ctx *ctx, int index) static const struct rkvdec_coded_fmt_desc * rkvdec_find_coded_fmt_desc(struct rkvdec_ctx *ctx, u32 fourcc) { + const struct rkvdec_config *cfg = ctx->dev->variant->config; unsigned int i; - for (i = 0; i < ARRAY_SIZE(rkvdec_coded_fmts); i++) { - if (rkvdec_is_capable(ctx, rkvdec_coded_fmts[i].capability) && - rkvdec_coded_fmts[i].fourcc == fourcc) - return &rkvdec_coded_fmts[i]; + for (i = 0; i < cfg->coded_fmts_num; i++) { + if (rkvdec_is_capable(ctx, cfg->coded_fmts[i].capability) && + cfg->coded_fmts[i].fourcc == fourcc) + return &cfg->coded_fmts[i]; } return NULL;@@ -1014,18 +1016,19 @@ static int rkvdec_add_ctrls(struct rkvdec_ctx *ctx, static int rkvdec_init_ctrls(struct rkvdec_ctx *ctx) { + const struct rkvdec_config *cfg = ctx->dev->variant->config; unsigned int i, nctrls = 0; int ret; - for (i = 0; i < ARRAY_SIZE(rkvdec_coded_fmts); i++) - if (rkvdec_is_capable(ctx, rkvdec_coded_fmts[i].capability)) - nctrls += rkvdec_coded_fmts[i].ctrls->num_ctrls; + for (i = 0; i < cfg->coded_fmts_num; i++) + if (rkvdec_is_capable(ctx, cfg->coded_fmts[i].capability)) + nctrls += cfg->coded_fmts[i].ctrls->num_ctrls; v4l2_ctrl_handler_init(&ctx->ctrl_hdl, nctrls); - for (i = 0; i < ARRAY_SIZE(rkvdec_coded_fmts); i++) { - if (rkvdec_is_capable(ctx, rkvdec_coded_fmts[i].capability)) { - ret = rkvdec_add_ctrls(ctx, rkvdec_coded_fmts[i].ctrls); + for (i = 0; i < cfg->coded_fmts_num; i++) { + if (rkvdec_is_capable(ctx, cfg->coded_fmts[i].capability)) { + ret = rkvdec_add_ctrls(ctx, cfg->coded_fmts[i].ctrls); if (ret) goto err_free_handler; }@@ -1240,13 +1243,20 @@ static void rkvdec_watchdog_func(struct work_struct *work) } } +static const struct rkvdec_config config_rkvdec = { + .coded_fmts = rkvdec_coded_fmts, + .coded_fmts_num = ARRAY_SIZE(rkvdec_coded_fmts), +}; + static const struct rkvdec_variant rk3288_rkvdec_variant = { .num_regs = 68, + .config = &config_rkvdec, .capabilities = RKVDEC_CAPABILITY_HEVC, }; static const struct rkvdec_variant rk3328_rkvdec_variant = { .num_regs = 109, + .config = &config_rkvdec, .capabilities = RKVDEC_CAPABILITY_HEVC | RKVDEC_CAPABILITY_H264 | RKVDEC_CAPABILITY_VP9,@@ -1255,6 +1265,7 @@ static const struct rkvdec_variant rk3328_rkvdec_variant = { static const struct rkvdec_variant rk3399_rkvdec_variant = { .num_regs = 78, + .config = &config_rkvdec, .capabilities = RKVDEC_CAPABILITY_HEVC | RKVDEC_CAPABILITY_H264 | RKVDEC_CAPABILITY_VP9,diff --git a/drivers/media/platform/rockchip/rkvdec/rkvdec.h b/drivers/media/platform/rockchip/rkvdec/rkvdec.h index f35f6e80ea2e..3b1cc511412e 100644 --- a/drivers/media/platform/rockchip/rkvdec/rkvdec.h +++ b/drivers/media/platform/rockchip/rkvdec/rkvdec.h@@ -71,6 +71,7 @@ vb2_to_rkvdec_decoded_buf(struct vb2_buffer *buf) struct rkvdec_variant { unsigned int num_regs; + const struct rkvdec_config *config; unsigned int capabilities; unsigned int quirks; };@@ -113,6 +114,11 @@ struct rkvdec_coded_fmt_desc { unsigned int capability; }; +struct rkvdec_config { + const struct rkvdec_coded_fmt_desc *coded_fmts; + size_t coded_fmts_num; +};
Do we really need a separate config struct? This chould/should me merged with the variant struct. Using a two layer variant/config mostly seem to complicate things based on an initial review. Regards, Jonas
+
struct rkvdec_dev {
struct v4l2_device v4l2_dev;
struct media_device mdev;