* [PATCH v2 1/4] media: hantro: Cleanup format negotiation helpers
2019-10-03 19:08 [PATCH v2 0/4] Enable Hantro G1 post-processor Ezequiel Garcia
@ 2019-10-03 19:08 ` Ezequiel Garcia
2019-10-03 19:08 ` [PATCH v2 2/4] media: hantro: mpeg2_dec: Re-use common register macros Ezequiel Garcia
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2019-10-03 19:08 UTC (permalink / raw)
To: linux-media
Cc: kernel, Tomasz Figa, linux-rockchip, Heiko Stuebner,
Jonas Karlman, Philipp Zabel, Boris Brezillon, Chris Healy,
linux-kernel, Ezequiel Garcia
Format negotation helpers, hantro_find_format()
and hantro_get_default_fmt() can be simplified,
making the code a little bit clearer.
More importantly, this change is preparation work
for the post-processor usage.
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
drivers/staging/media/hantro/hantro_v4l2.c | 53 ++++++++--------------
1 file changed, 20 insertions(+), 33 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index 3dae52abb96c..238e53b28f8f 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -47,23 +47,26 @@ hantro_get_formats(const struct hantro_ctx *ctx, unsigned int *num_fmts)
}
static const struct hantro_fmt *
-hantro_find_format(const struct hantro_fmt *formats, unsigned int num_fmts,
- u32 fourcc)
+hantro_find_format(const struct hantro_ctx *ctx, u32 fourcc)
{
- unsigned int i;
+ const struct hantro_fmt *formats;
+ unsigned int i, num_fmts;
+ formats = hantro_get_formats(ctx, &num_fmts);
for (i = 0; i < num_fmts; i++)
if (formats[i].fourcc == fourcc)
return &formats[i];
+
return NULL;
}
static const struct hantro_fmt *
-hantro_get_default_fmt(const struct hantro_fmt *formats, unsigned int num_fmts,
- bool bitstream)
+hantro_get_default_fmt(const struct hantro_ctx *ctx, bool bitstream)
{
- unsigned int i;
+ const struct hantro_fmt *formats;
+ unsigned int i, num_fmts;
+ formats = hantro_get_formats(ctx, &num_fmts);
for (i = 0; i < num_fmts; i++) {
if (bitstream == (formats[i].codec_mode !=
HANTRO_MODE_NONE))
@@ -89,8 +92,7 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
struct v4l2_frmsizeenum *fsize)
{
struct hantro_ctx *ctx = fh_to_ctx(priv);
- const struct hantro_fmt *formats, *fmt;
- unsigned int num_fmts;
+ const struct hantro_fmt *fmt;
if (fsize->index != 0) {
vpu_debug(0, "invalid frame size index (expected 0, got %d)\n",
@@ -98,8 +100,7 @@ static int vidioc_enum_framesizes(struct file *file, void *priv,
return -EINVAL;
}
- formats = hantro_get_formats(ctx, &num_fmts);
- fmt = hantro_find_format(formats, num_fmts, fsize->pixel_format);
+ fmt = hantro_find_format(ctx, fsize->pixel_format);
if (!fmt) {
vpu_debug(0, "unsupported bitstream format (%08x)\n",
fsize->pixel_format);
@@ -196,8 +197,7 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f,
{
struct hantro_ctx *ctx = fh_to_ctx(priv);
struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
- const struct hantro_fmt *formats, *fmt, *vpu_fmt;
- unsigned int num_fmts;
+ const struct hantro_fmt *fmt, *vpu_fmt;
bool coded;
coded = capture == hantro_is_encoder_ctx(ctx);
@@ -208,10 +208,9 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f,
(pix_mp->pixelformat >> 16) & 0x7f,
(pix_mp->pixelformat >> 24) & 0x7f);
- formats = hantro_get_formats(ctx, &num_fmts);
- fmt = hantro_find_format(formats, num_fmts, pix_mp->pixelformat);
+ fmt = hantro_find_format(ctx, pix_mp->pixelformat);
if (!fmt) {
- fmt = hantro_get_default_fmt(formats, num_fmts, coded);
+ fmt = hantro_get_default_fmt(ctx, coded);
f->fmt.pix_mp.pixelformat = fmt->fourcc;
}
@@ -290,12 +289,10 @@ hantro_reset_fmt(struct v4l2_pix_format_mplane *fmt,
static void
hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
{
- const struct hantro_fmt *vpu_fmt, *formats;
+ const struct hantro_fmt *vpu_fmt;
struct v4l2_pix_format_mplane *fmt;
- unsigned int num_fmts;
- formats = hantro_get_formats(ctx, &num_fmts);
- vpu_fmt = hantro_get_default_fmt(formats, num_fmts, true);
+ vpu_fmt = hantro_get_default_fmt(ctx, true);
if (hantro_is_encoder_ctx(ctx)) {
ctx->vpu_dst_fmt = vpu_fmt;
@@ -316,12 +313,10 @@ hantro_reset_encoded_fmt(struct hantro_ctx *ctx)
static void
hantro_reset_raw_fmt(struct hantro_ctx *ctx)
{
- const struct hantro_fmt *raw_vpu_fmt, *formats;
+ const struct hantro_fmt *raw_vpu_fmt;
struct v4l2_pix_format_mplane *raw_fmt, *encoded_fmt;
- unsigned int num_fmts;
- formats = hantro_get_formats(ctx, &num_fmts);
- raw_vpu_fmt = hantro_get_default_fmt(formats, num_fmts, false);
+ raw_vpu_fmt = hantro_get_default_fmt(ctx, false);
if (hantro_is_encoder_ctx(ctx)) {
ctx->vpu_src_fmt = raw_vpu_fmt;
@@ -367,8 +362,6 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
{
struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
struct hantro_ctx *ctx = fh_to_ctx(priv);
- const struct hantro_fmt *formats;
- unsigned int num_fmts;
struct vb2_queue *vq;
int ret;
@@ -395,9 +388,7 @@ vidioc_s_fmt_out_mplane(struct file *file, void *priv, struct v4l2_format *f)
if (ret)
return ret;
- formats = hantro_get_formats(ctx, &num_fmts);
- ctx->vpu_src_fmt = hantro_find_format(formats, num_fmts,
- pix_mp->pixelformat);
+ ctx->vpu_src_fmt = hantro_find_format(ctx, pix_mp->pixelformat);
ctx->src_fmt = *pix_mp;
/*
@@ -431,9 +422,7 @@ static int vidioc_s_fmt_cap_mplane(struct file *file, void *priv,
{
struct v4l2_pix_format_mplane *pix_mp = &f->fmt.pix_mp;
struct hantro_ctx *ctx = fh_to_ctx(priv);
- const struct hantro_fmt *formats;
struct vb2_queue *vq;
- unsigned int num_fmts;
int ret;
/* Change not allowed if queue is busy. */
@@ -462,9 +451,7 @@ static int vidioc_s_fmt_cap_mplane(struct file *file, void *priv,
if (ret)
return ret;
- formats = hantro_get_formats(ctx, &num_fmts);
- ctx->vpu_dst_fmt = hantro_find_format(formats, num_fmts,
- pix_mp->pixelformat);
+ ctx->vpu_dst_fmt = hantro_find_format(ctx, pix_mp->pixelformat);
ctx->dst_fmt = *pix_mp;
/*
--
2.22.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 2/4] media: hantro: mpeg2_dec: Re-use common register macros
2019-10-03 19:08 [PATCH v2 0/4] Enable Hantro G1 post-processor Ezequiel Garcia
2019-10-03 19:08 ` [PATCH v2 1/4] media: hantro: Cleanup format negotiation helpers Ezequiel Garcia
@ 2019-10-03 19:08 ` Ezequiel Garcia
2019-10-03 19:39 ` Jonas Karlman
2019-10-03 19:08 ` [PATCH v2 3/4] media: hantro: Rework media topology Ezequiel Garcia
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Ezequiel Garcia @ 2019-10-03 19:08 UTC (permalink / raw)
To: linux-media
Cc: kernel, Tomasz Figa, linux-rockchip, Heiko Stuebner,
Jonas Karlman, Philipp Zabel, Boris Brezillon, Chris Healy,
linux-kernel, Ezequiel Garcia
The MPEG-2 decoder register macros can be re-used from hantro_g1_regs.h,
and the re-definitions removed.
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
.../media/hantro/hantro_g1_mpeg2_dec.c | 186 ++++++------------
drivers/staging/media/hantro/hantro_g1_regs.h | 58 +++---
2 files changed, 96 insertions(+), 148 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
index f3bf67d8a289..663bf05459a9 100644
--- a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
@@ -10,60 +10,9 @@
#include <media/v4l2-mem2mem.h>
#include "hantro.h"
#include "hantro_hw.h"
+#include "hantro_g1_regs.h"
-#define G1_SWREG(nr) ((nr) * 4)
-
-#define G1_REG_RLC_VLC_BASE G1_SWREG(12)
-#define G1_REG_DEC_OUT_BASE G1_SWREG(13)
-#define G1_REG_REFER0_BASE G1_SWREG(14)
-#define G1_REG_REFER1_BASE G1_SWREG(15)
-#define G1_REG_REFER2_BASE G1_SWREG(16)
-#define G1_REG_REFER3_BASE G1_SWREG(17)
-#define G1_REG_QTABLE_BASE G1_SWREG(40)
-#define G1_REG_DEC_E(v) ((v) ? BIT(0) : 0)
-
-#define G1_REG_DEC_AXI_RD_ID(v) (((v) << 24) & GENMASK(31, 24))
-#define G1_REG_DEC_TIMEOUT_E(v) ((v) ? BIT(23) : 0)
-#define G1_REG_DEC_STRSWAP32_E(v) ((v) ? BIT(22) : 0)
-#define G1_REG_DEC_STRENDIAN_E(v) ((v) ? BIT(21) : 0)
-#define G1_REG_DEC_INSWAP32_E(v) ((v) ? BIT(20) : 0)
-#define G1_REG_DEC_OUTSWAP32_E(v) ((v) ? BIT(19) : 0)
-#define G1_REG_DEC_DATA_DISC_E(v) ((v) ? BIT(18) : 0)
-#define G1_REG_DEC_LATENCY(v) (((v) << 11) & GENMASK(16, 11))
-#define G1_REG_DEC_CLK_GATE_E(v) ((v) ? BIT(10) : 0)
-#define G1_REG_DEC_IN_ENDIAN(v) ((v) ? BIT(9) : 0)
-#define G1_REG_DEC_OUT_ENDIAN(v) ((v) ? BIT(8) : 0)
-#define G1_REG_DEC_ADV_PRE_DIS(v) ((v) ? BIT(6) : 0)
-#define G1_REG_DEC_SCMD_DIS(v) ((v) ? BIT(5) : 0)
-#define G1_REG_DEC_MAX_BURST(v) (((v) << 0) & GENMASK(4, 0))
-
-#define G1_REG_DEC_MODE(v) (((v) << 28) & GENMASK(31, 28))
-#define G1_REG_RLC_MODE_E(v) ((v) ? BIT(27) : 0)
-#define G1_REG_PIC_INTERLACE_E(v) ((v) ? BIT(23) : 0)
-#define G1_REG_PIC_FIELDMODE_E(v) ((v) ? BIT(22) : 0)
-#define G1_REG_PIC_B_E(v) ((v) ? BIT(21) : 0)
-#define G1_REG_PIC_INTER_E(v) ((v) ? BIT(20) : 0)
-#define G1_REG_PIC_TOPFIELD_E(v) ((v) ? BIT(19) : 0)
-#define G1_REG_FWD_INTERLACE_E(v) ((v) ? BIT(18) : 0)
-#define G1_REG_FILTERING_DIS(v) ((v) ? BIT(14) : 0)
-#define G1_REG_WRITE_MVS_E(v) ((v) ? BIT(12) : 0)
-#define G1_REG_DEC_AXI_WR_ID(v) (((v) << 0) & GENMASK(7, 0))
-
-#define G1_REG_PIC_MB_WIDTH(v) (((v) << 23) & GENMASK(31, 23))
-#define G1_REG_PIC_MB_HEIGHT_P(v) (((v) << 11) & GENMASK(18, 11))
-#define G1_REG_ALT_SCAN_E(v) ((v) ? BIT(6) : 0)
-#define G1_REG_TOPFIELDFIRST_E(v) ((v) ? BIT(5) : 0)
-
-#define G1_REG_STRM_START_BIT(v) (((v) << 26) & GENMASK(31, 26))
-#define G1_REG_QSCALE_TYPE(v) ((v) ? BIT(24) : 0)
-#define G1_REG_CON_MV_E(v) ((v) ? BIT(4) : 0)
-#define G1_REG_INTRA_DC_PREC(v) (((v) << 2) & GENMASK(3, 2))
-#define G1_REG_INTRA_VLC_TAB(v) ((v) ? BIT(1) : 0)
-#define G1_REG_FRAME_PRED_DCT(v) ((v) ? BIT(0) : 0)
-
-#define G1_REG_INIT_QP(v) (((v) << 25) & GENMASK(30, 25))
-#define G1_REG_STREAM_LEN(v) (((v) << 0) & GENMASK(23, 0))
-
+/* These bits seem undocumented. */
#define G1_REG_ALT_SCAN_FLAG_E(v) ((v) ? BIT(19) : 0)
#define G1_REG_FCODE_FWD_HOR(v) (((v) << 15) & GENMASK(18, 15))
#define G1_REG_FCODE_FWD_VER(v) (((v) << 11) & GENMASK(14, 11))
@@ -72,11 +21,6 @@
#define G1_REG_MV_ACCURACY_FWD(v) ((v) ? BIT(2) : 0)
#define G1_REG_MV_ACCURACY_BWD(v) ((v) ? BIT(1) : 0)
-#define G1_REG_STARTMB_X(v) (((v) << 23) & GENMASK(31, 23))
-#define G1_REG_STARTMB_Y(v) (((v) << 15) & GENMASK(22, 15))
-
-#define G1_REG_APF_THRESHOLD(v) (((v) << 0) & GENMASK(13, 0))
-
#define PICT_TOP_FIELD 1
#define PICT_BOTTOM_FIELD 2
#define PICT_FRAME 3
@@ -92,7 +36,7 @@ hantro_g1_mpeg2_dec_set_quantization(struct hantro_dev *vpu,
hantro_mpeg2_dec_copy_qtable(ctx->mpeg2_dec.qtable.cpu,
quantization);
vdpu_write_relaxed(vpu, ctx->mpeg2_dec.qtable.dma,
- G1_REG_QTABLE_BASE);
+ G1_REG_ADDR_QTABLE);
}
static void
@@ -118,7 +62,7 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
/* Source bitstream buffer */
addr = vb2_dma_contig_plane_dma_addr(src_buf, 0);
- vdpu_write_relaxed(vpu, addr, G1_REG_RLC_VLC_BASE);
+ vdpu_write_relaxed(vpu, addr, G1_REG_ADDR_STR);
/* Destination frame buffer */
addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0);
@@ -126,7 +70,7 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
if (picture->picture_structure == PICT_BOTTOM_FIELD)
addr += ALIGN(ctx->dst_fmt.width, 16);
- vdpu_write_relaxed(vpu, addr, G1_REG_DEC_OUT_BASE);
+ vdpu_write_relaxed(vpu, addr, G1_REG_ADDR_DST);
if (!forward_addr)
forward_addr = current_addr;
@@ -140,19 +84,19 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
picture->top_field_first) ||
(picture->picture_structure == PICT_BOTTOM_FIELD &&
!picture->top_field_first)) {
- vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER0_BASE);
- vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER1_BASE);
+ vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(0));
+ vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(1));
} else if (picture->picture_structure == PICT_TOP_FIELD) {
- vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER0_BASE);
- vdpu_write_relaxed(vpu, current_addr, G1_REG_REFER1_BASE);
+ vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(0));
+ vdpu_write_relaxed(vpu, current_addr, G1_REG_ADDR_REF(1));
} else if (picture->picture_structure == PICT_BOTTOM_FIELD) {
- vdpu_write_relaxed(vpu, current_addr, G1_REG_REFER0_BASE);
- vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER1_BASE);
+ vdpu_write_relaxed(vpu, current_addr, G1_REG_ADDR_REF(0));
+ vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(1));
}
/* Set backward ref frame (top/bottom field) */
- vdpu_write_relaxed(vpu, backward_addr, G1_REG_REFER2_BASE);
- vdpu_write_relaxed(vpu, backward_addr, G1_REG_REFER3_BASE);
+ vdpu_write_relaxed(vpu, backward_addr, G1_REG_ADDR_REF(2));
+ vdpu_write_relaxed(vpu, backward_addr, G1_REG_ADDR_REF(3));
}
void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
@@ -175,52 +119,51 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
sequence = &slice_params->sequence;
picture = &slice_params->picture;
- reg = G1_REG_DEC_AXI_RD_ID(0) |
- G1_REG_DEC_TIMEOUT_E(1) |
- G1_REG_DEC_STRSWAP32_E(1) |
- G1_REG_DEC_STRENDIAN_E(1) |
- G1_REG_DEC_INSWAP32_E(1) |
- G1_REG_DEC_OUTSWAP32_E(1) |
- G1_REG_DEC_DATA_DISC_E(0) |
- G1_REG_DEC_LATENCY(0) |
- G1_REG_DEC_CLK_GATE_E(1) |
- G1_REG_DEC_IN_ENDIAN(1) |
- G1_REG_DEC_OUT_ENDIAN(1) |
- G1_REG_DEC_ADV_PRE_DIS(0) |
- G1_REG_DEC_SCMD_DIS(0) |
- G1_REG_DEC_MAX_BURST(16);
- vdpu_write_relaxed(vpu, reg, G1_SWREG(2));
-
- reg = G1_REG_DEC_MODE(5) |
- G1_REG_RLC_MODE_E(0) |
- G1_REG_PIC_INTERLACE_E(!sequence->progressive_sequence) |
- G1_REG_PIC_FIELDMODE_E(picture->picture_structure != PICT_FRAME) |
- G1_REG_PIC_B_E(picture->picture_coding_type == V4L2_MPEG2_PICTURE_CODING_TYPE_B) |
- G1_REG_PIC_INTER_E(picture->picture_coding_type != V4L2_MPEG2_PICTURE_CODING_TYPE_I) |
- G1_REG_PIC_TOPFIELD_E(picture->picture_structure == PICT_TOP_FIELD) |
- G1_REG_FWD_INTERLACE_E(0) |
- G1_REG_FILTERING_DIS(1) |
- G1_REG_WRITE_MVS_E(0) |
- G1_REG_DEC_AXI_WR_ID(0);
- vdpu_write_relaxed(vpu, reg, G1_SWREG(3));
-
- reg = G1_REG_PIC_MB_WIDTH(MB_WIDTH(ctx->dst_fmt.width)) |
- G1_REG_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->dst_fmt.height)) |
- G1_REG_ALT_SCAN_E(picture->alternate_scan) |
- G1_REG_TOPFIELDFIRST_E(picture->top_field_first);
- vdpu_write_relaxed(vpu, reg, G1_SWREG(4));
-
- reg = G1_REG_STRM_START_BIT(slice_params->data_bit_offset) |
- G1_REG_QSCALE_TYPE(picture->q_scale_type) |
- G1_REG_CON_MV_E(picture->concealment_motion_vectors) |
- G1_REG_INTRA_DC_PREC(picture->intra_dc_precision) |
- G1_REG_INTRA_VLC_TAB(picture->intra_vlc_format) |
- G1_REG_FRAME_PRED_DCT(picture->frame_pred_frame_dct);
- vdpu_write_relaxed(vpu, reg, G1_SWREG(5));
-
- reg = G1_REG_INIT_QP(1) |
- G1_REG_STREAM_LEN(slice_params->bit_size >> 3);
- vdpu_write_relaxed(vpu, reg, G1_SWREG(6));
+ reg = G1_REG_CONFIG_DEC_AXI_RD_ID(0) |
+ G1_REG_CONFIG_DEC_TIMEOUT_E |
+ G1_REG_CONFIG_DEC_STRSWAP32_E |
+ G1_REG_CONFIG_DEC_STRENDIAN_E |
+ G1_REG_CONFIG_DEC_INSWAP32_E |
+ G1_REG_CONFIG_DEC_OUTSWAP32_E |
+ G1_REG_CONFIG_DEC_LATENCY(0) |
+ G1_REG_CONFIG_DEC_CLK_GATE_E |
+ G1_REG_CONFIG_DEC_IN_ENDIAN |
+ G1_REG_CONFIG_DEC_OUT_ENDIAN |
+ G1_REG_CONFIG_DEC_MAX_BURST(16);
+ vdpu_write_relaxed(vpu, reg, G1_REG_CONFIG);
+
+ reg = G1_REG_DEC_CTRL0_DEC_MODE(5) |
+ G1_REG_DEC_CTRL0_FILTERING_DIS |
+ G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(0);
+ G1_SWREG_SET_IF(reg, !sequence->progressive_sequence,
+ G1_REG_DEC_CTRL0_PIC_INTERLACE_E);
+ G1_SWREG_SET_IF(reg, picture->picture_coding_type == V4L2_MPEG2_PICTURE_CODING_TYPE_B,
+ G1_REG_DEC_CTRL0_PIC_B_E);
+ G1_SWREG_SET_IF(reg, picture->picture_coding_type != V4L2_MPEG2_PICTURE_CODING_TYPE_I,
+ G1_REG_DEC_CTRL0_PIC_INTER_E);
+ G1_SWREG_SET_IF(reg, picture->picture_structure == PICT_TOP_FIELD,
+ G1_REG_DEC_CTRL0_PIC_TOPFIELD_E);
+ G1_SWREG_SET_IF(reg, picture->picture_structure != PICT_FRAME,
+ G1_REG_DEC_CTRL0_PIC_FIELDMODE_E);
+ vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
+
+ reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(MB_WIDTH(ctx->dst_fmt.width)) |
+ G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->dst_fmt.height));
+ G1_SWREG_SET_IF(reg, picture->alternate_scan, G1_REG_DEC_CTRL1_ALT_SCAN_E);
+ G1_SWREG_SET_IF(reg, picture->top_field_first, G1_REG_DEC_CTRL1_TOPFIELDFIRST_E);
+ vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL1);
+
+ reg = G1_REG_DEC_CTRL2_STRM_START_BIT(slice_params->data_bit_offset) |
+ G1_REG_DEC_CTRL2_INTRA_DC_PREC(picture->intra_dc_precision);
+ G1_SWREG_SET_IF(reg, picture->q_scale_type,G1_REG_DEC_CTRL2_QSCALE_TYPE);
+ G1_SWREG_SET_IF(reg, picture->concealment_motion_vectors, G1_REG_DEC_CTRL2_CON_MV_E);
+ G1_SWREG_SET_IF(reg, picture->intra_vlc_format, G1_REG_DEC_CTRL2_INTRA_VLC_TAB);
+ G1_SWREG_SET_IF(reg, picture->frame_pred_frame_dct, G1_REG_DEC_CTRL2_FRAME_PRED_DCT);
+ vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL2);
+
+ reg = G1_REG_DEC_CTRL3_INIT_QP(1) |
+ G1_REG_DEC_CTRL3_STREAM_LEN(slice_params->bit_size >> 3);
+ vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL3);
reg = G1_REG_ALT_SCAN_FLAG_E(picture->alternate_scan) |
G1_REG_FCODE_FWD_HOR(picture->f_code[0][0]) |
@@ -231,12 +174,12 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
G1_REG_MV_ACCURACY_BWD(1);
vdpu_write_relaxed(vpu, reg, G1_SWREG(18));
- reg = G1_REG_STARTMB_X(0) |
- G1_REG_STARTMB_Y(0);
- vdpu_write_relaxed(vpu, reg, G1_SWREG(48));
+ reg = G1_REG_ERR_CONC_STARTMB_X(0) |
+ G1_REG_ERR_CONC_STARTMB_Y(0);
+ vdpu_write_relaxed(vpu, reg, G1_REG_ERR_CONC);
- reg = G1_REG_APF_THRESHOLD(8);
- vdpu_write_relaxed(vpu, reg, G1_SWREG(55));
+ reg = G1_REG_REF_BUF_CTRL2_APF_THRESHOLD(8);
+ vdpu_write_relaxed(vpu, reg, G1_REG_REF_BUF_CTRL2);
hantro_g1_mpeg2_dec_set_quantization(vpu, ctx);
@@ -246,6 +189,5 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
hantro_finish_run(ctx);
- reg = G1_REG_DEC_E(1);
- vdpu_write(vpu, reg, G1_SWREG(1));
+ vdpu_write(vpu, G1_REG_INTERRUPT_DEC_E, G1_REG_INTERRUPT);
}
diff --git a/drivers/staging/media/hantro/hantro_g1_regs.h b/drivers/staging/media/hantro/hantro_g1_regs.h
index 5c0ea7994336..84b93915a953 100644
--- a/drivers/staging/media/hantro/hantro_g1_regs.h
+++ b/drivers/staging/media/hantro/hantro_g1_regs.h
@@ -9,8 +9,14 @@
#ifndef HANTRO_G1_REGS_H_
#define HANTRO_G1_REGS_H_
+#define G1_SWREG(nr) ((nr) * 4)
+#define G1_SWREG_SET_IF(reg, cond, val) \
+ do \
+ reg |= (cond) ? (val) : 0x0; \
+ while (0)
+
/* Decoder registers. */
-#define G1_REG_INTERRUPT 0x004
+#define G1_REG_INTERRUPT G1_SWREG(1)
#define G1_REG_INTERRUPT_DEC_PIC_INF BIT(24)
#define G1_REG_INTERRUPT_DEC_TIMEOUT BIT(18)
#define G1_REG_INTERRUPT_DEC_SLICE_INT BIT(17)
@@ -22,7 +28,7 @@
#define G1_REG_INTERRUPT_DEC_IRQ BIT(8)
#define G1_REG_INTERRUPT_DEC_IRQ_DIS BIT(4)
#define G1_REG_INTERRUPT_DEC_E BIT(0)
-#define G1_REG_CONFIG 0x008
+#define G1_REG_CONFIG G1_SWREG(2)
#define G1_REG_CONFIG_DEC_AXI_RD_ID(x) (((x) & 0xff) << 24)
#define G1_REG_CONFIG_DEC_TIMEOUT_E BIT(23)
#define G1_REG_CONFIG_DEC_STRSWAP32_E BIT(22)
@@ -41,7 +47,7 @@
#define G1_REG_CONFIG_DEC_ADV_PRE_DIS BIT(6)
#define G1_REG_CONFIG_DEC_SCMD_DIS BIT(5)
#define G1_REG_CONFIG_DEC_MAX_BURST(x) (((x) & 0x1f) << 0)
-#define G1_REG_DEC_CTRL0 0x00c
+#define G1_REG_DEC_CTRL0 G1_SWREG(3)
#define G1_REG_DEC_CTRL0_DEC_MODE(x) (((x) & 0xf) << 28)
#define G1_REG_DEC_CTRL0_RLC_MODE_E BIT(27)
#define G1_REG_DEC_CTRL0_SKIP_MODE BIT(26)
@@ -66,7 +72,7 @@
#define G1_REG_DEC_CTRL0_PICORD_COUNT_E BIT(9)
#define G1_REG_DEC_CTRL0_DEC_AHB_HLOCK_E BIT(8)
#define G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(x) (((x) & 0xff) << 0)
-#define G1_REG_DEC_CTRL1 0x010
+#define G1_REG_DEC_CTRL1 G1_SWREG(4)
#define G1_REG_DEC_CTRL1_PIC_MB_WIDTH(x) (((x) & 0x1ff) << 23)
#define G1_REG_DEC_CTRL1_MB_WIDTH_OFF(x) (((x) & 0xf) << 19)
#define G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(x) (((x) & 0xff) << 11)
@@ -77,7 +83,7 @@
#define G1_REG_DEC_CTRL1_PIC_MB_W_EXT(x) (((x) & 0x7) << 3)
#define G1_REG_DEC_CTRL1_PIC_MB_H_EXT(x) (((x) & 0x7) << 0)
#define G1_REG_DEC_CTRL1_PIC_REFER_FLAG BIT(0)
-#define G1_REG_DEC_CTRL2 0x014
+#define G1_REG_DEC_CTRL2 G1_SWREG(5)
#define G1_REG_DEC_CTRL2_STRM_START_BIT(x) (((x) & 0x3f) << 26)
#define G1_REG_DEC_CTRL2_SYNC_MARKER_E BIT(25)
#define G1_REG_DEC_CTRL2_TYPE1_QUANT_E BIT(24)
@@ -120,13 +126,13 @@
#define G1_REG_DEC_CTRL2_BOOLEAN_RANGE(x) (((x) & 0xff) << 0)
#define G1_REG_DEC_CTRL2_ALPHA_OFFSET(x) (((x) & 0x1f) << 5)
#define G1_REG_DEC_CTRL2_BETA_OFFSET(x) (((x) & 0x1f) << 0)
-#define G1_REG_DEC_CTRL3 0x018
+#define G1_REG_DEC_CTRL3 G1_SWREG(6)
#define G1_REG_DEC_CTRL3_START_CODE_E BIT(31)
#define G1_REG_DEC_CTRL3_INIT_QP(x) (((x) & 0x3f) << 25)
#define G1_REG_DEC_CTRL3_CH_8PIX_ILEAV_E BIT(24)
#define G1_REG_DEC_CTRL3_STREAM_LEN_EXT(x) (((x) & 0xff) << 24)
#define G1_REG_DEC_CTRL3_STREAM_LEN(x) (((x) & 0xffffff) << 0)
-#define G1_REG_DEC_CTRL4 0x01c
+#define G1_REG_DEC_CTRL4 G1_SWREG(7)
#define G1_REG_DEC_CTRL4_CABAC_E BIT(31)
#define G1_REG_DEC_CTRL4_BLACKWHITE_E BIT(30)
#define G1_REG_DEC_CTRL4_DIR_8X8_INFER_E BIT(29)
@@ -163,7 +169,7 @@
#define G1_REG_DEC_CTRL4_INIT_DC_MATCH0(x) (((x) & 0x7) << 9)
#define G1_REG_DEC_CTRL4_INIT_DC_MATCH1(x) (((x) & 0x7) << 6)
#define G1_REG_DEC_CTRL4_VP7_VERSION BIT(5)
-#define G1_REG_DEC_CTRL5 0x020
+#define G1_REG_DEC_CTRL5 G1_SWREG(8)
#define G1_REG_DEC_CTRL5_CONST_INTRA_E BIT(31)
#define G1_REG_DEC_CTRL5_FILT_CTRL_PRES BIT(30)
#define G1_REG_DEC_CTRL5_RDPIC_CNT_PRES BIT(29)
@@ -187,7 +193,7 @@
#define G1_REG_DEC_CTRL5_RV_BWD_SCALE(x) (((x) & 0x3fff) << 0)
#define G1_REG_DEC_CTRL5_INIT_DC_COMP0(x) (((x) & 0xffff) << 16)
#define G1_REG_DEC_CTRL5_INIT_DC_COMP1(x) (((x) & 0xffff) << 0)
-#define G1_REG_DEC_CTRL6 0x024
+#define G1_REG_DEC_CTRL6 G1_SWREG(9)
#define G1_REG_DEC_CTRL6_PPS_ID(x) (((x) & 0xff) << 24)
#define G1_REG_DEC_CTRL6_REFIDX1_ACTIVE(x) (((x) & 0x1f) << 19)
#define G1_REG_DEC_CTRL6_REFIDX0_ACTIVE(x) (((x) & 0x1f) << 14)
@@ -198,7 +204,7 @@
#define G1_REG_DEC_CTRL6_STREAM1_LEN(x) (((x) & 0xffffff) << 0)
#define G1_REG_DEC_CTRL6_PIC_SLICE_AM(x) (((x) & 0x1fff) << 0)
#define G1_REG_DEC_CTRL6_COEFFS_PART_AM(x) (((x) & 0xf) << 24)
-#define G1_REG_FWD_PIC(i) (0x028 + ((i) * 0x4))
+#define G1_REG_FWD_PIC(i) (G1_SWREG(10) + ((i) * 0x4))
#define G1_REG_FWD_PIC_PINIT_RLIST_F5(x) (((x) & 0x1f) << 25)
#define G1_REG_FWD_PIC_PINIT_RLIST_F4(x) (((x) & 0x1f) << 20)
#define G1_REG_FWD_PIC_PINIT_RLIST_F3(x) (((x) & 0x1f) << 15)
@@ -211,7 +217,7 @@
#define G1_REG_FWD_PIC1_SEGMENT_BASE(x) ((x) << 0)
#define G1_REG_FWD_PIC1_SEGMENT_UPD_E BIT(1)
#define G1_REG_FWD_PIC1_SEGMENT_E BIT(0)
-#define G1_REG_DEC_CTRL7 0x02c
+#define G1_REG_DEC_CTRL7 G1_SWREG(11)
#define G1_REG_DEC_CTRL7_PINIT_RLIST_F15(x) (((x) & 0x1f) << 25)
#define G1_REG_DEC_CTRL7_PINIT_RLIST_F14(x) (((x) & 0x1f) << 20)
#define G1_REG_DEC_CTRL7_PINIT_RLIST_F13(x) (((x) & 0x1f) << 15)
@@ -226,12 +232,12 @@
#define G1_REG_DEC_CTRL7_DCT5_START_BIT(x) (((x) & 0x3f) << 12)
#define G1_REG_DEC_CTRL7_DCT6_START_BIT(x) (((x) & 0x3f) << 6)
#define G1_REG_DEC_CTRL7_DCT7_START_BIT(x) (((x) & 0x3f) << 0)
-#define G1_REG_ADDR_STR 0x030
-#define G1_REG_ADDR_DST 0x034
-#define G1_REG_ADDR_REF(i) (0x038 + ((i) * 0x4))
+#define G1_REG_ADDR_STR G1_SWREG(12)
+#define G1_REG_ADDR_DST G1_SWREG(13)
+#define G1_REG_ADDR_REF(i) (G1_SWREG(14) + ((i) * 0x4))
#define G1_REG_ADDR_REF_FIELD_E BIT(1)
#define G1_REG_ADDR_REF_TOPC_E BIT(0)
-#define G1_REG_REF_PIC(i) (0x078 + ((i) * 0x4))
+#define G1_REG_REF_PIC(i) (G1_SWREG(30) + ((i) * 0x4))
#define G1_REG_REF_PIC_FILT_TYPE_E BIT(31)
#define G1_REG_REF_PIC_FILT_SHARPNESS(x) (((x) & 0x7) << 28)
#define G1_REG_REF_PIC_MB_ADJ_0(x) (((x) & 0x7f) << 21)
@@ -248,11 +254,11 @@
#define G1_REG_REF_PIC_QUANT_DELTA_1(x) (((x) & 0x1f) << 22)
#define G1_REG_REF_PIC_QUANT_0(x) (((x) & 0x7ff) << 11)
#define G1_REG_REF_PIC_QUANT_1(x) (((x) & 0x7ff) << 0)
-#define G1_REG_LT_REF 0x098
-#define G1_REG_VALID_REF 0x09c
-#define G1_REG_ADDR_QTABLE 0x0a0
-#define G1_REG_ADDR_DIR_MV 0x0a4
-#define G1_REG_BD_REF_PIC(i) (0x0a8 + ((i) * 0x4))
+#define G1_REG_LT_REF G1_SWREG(38)
+#define G1_REG_VALID_REF G1_SWREG(39)
+#define G1_REG_ADDR_QTABLE G1_SWREG(40)
+#define G1_REG_ADDR_DIR_MV G1_SWREG(41)
+#define G1_REG_BD_REF_PIC(i) (G1_SWREG(42) + ((i) * 0x4))
#define G1_REG_BD_REF_PIC_BINIT_RLIST_B2(x) (((x) & 0x1f) << 25)
#define G1_REG_BD_REF_PIC_BINIT_RLIST_F2(x) (((x) & 0x1f) << 20)
#define G1_REG_BD_REF_PIC_BINIT_RLIST_B1(x) (((x) & 0x1f) << 15)
@@ -269,7 +275,7 @@
#define G1_REG_BD_REF_PIC_QUANT_DELTA_3(x) (((x) & 0x1f) << 22)
#define G1_REG_BD_REF_PIC_QUANT_2(x) (((x) & 0x7ff) << 11)
#define G1_REG_BD_REF_PIC_QUANT_3(x) (((x) & 0x7ff) << 0)
-#define G1_REG_BD_P_REF_PIC 0x0bc
+#define G1_REG_BD_P_REF_PIC G1_SWREG(47)
#define G1_REG_BD_P_REF_PIC_QUANT_DELTA_4(x) (((x) & 0x1f) << 27)
#define G1_REG_BD_P_REF_PIC_PINIT_RLIST_F3(x) (((x) & 0x1f) << 25)
#define G1_REG_BD_P_REF_PIC_PINIT_RLIST_F2(x) (((x) & 0x1f) << 20)
@@ -277,25 +283,25 @@
#define G1_REG_BD_P_REF_PIC_PINIT_RLIST_F0(x) (((x) & 0x1f) << 10)
#define G1_REG_BD_P_REF_PIC_BINIT_RLIST_B15(x) (((x) & 0x1f) << 5)
#define G1_REG_BD_P_REF_PIC_BINIT_RLIST_F15(x) (((x) & 0x1f) << 0)
-#define G1_REG_ERR_CONC 0x0c0
+#define G1_REG_ERR_CONC G1_SWREG(48)
#define G1_REG_ERR_CONC_STARTMB_X(x) (((x) & 0x1ff) << 23)
#define G1_REG_ERR_CONC_STARTMB_Y(x) (((x) & 0xff) << 15)
-#define G1_REG_PRED_FLT 0x0c4
+#define G1_REG_PRED_FLT G1_SWREG(49)
#define G1_REG_PRED_FLT_PRED_BC_TAP_0_0(x) (((x) & 0x3ff) << 22)
#define G1_REG_PRED_FLT_PRED_BC_TAP_0_1(x) (((x) & 0x3ff) << 12)
#define G1_REG_PRED_FLT_PRED_BC_TAP_0_2(x) (((x) & 0x3ff) << 2)
-#define G1_REG_REF_BUF_CTRL 0x0cc
+#define G1_REG_REF_BUF_CTRL G1_SWREG(51)
#define G1_REG_REF_BUF_CTRL_REFBU_E BIT(31)
#define G1_REG_REF_BUF_CTRL_REFBU_THR(x) (((x) & 0xfff) << 19)
#define G1_REG_REF_BUF_CTRL_REFBU_PICID(x) (((x) & 0x1f) << 14)
#define G1_REG_REF_BUF_CTRL_REFBU_EVAL_E BIT(13)
#define G1_REG_REF_BUF_CTRL_REFBU_FPARMOD_E BIT(12)
#define G1_REG_REF_BUF_CTRL_REFBU_Y_OFFSET(x) (((x) & 0x1ff) << 0)
-#define G1_REG_REF_BUF_CTRL2 0x0dc
+#define G1_REG_REF_BUF_CTRL2 G1_SWREG(55)
#define G1_REG_REF_BUF_CTRL2_REFBU2_BUF_E BIT(31)
#define G1_REG_REF_BUF_CTRL2_REFBU2_THR(x) (((x) & 0xfff) << 19)
#define G1_REG_REF_BUF_CTRL2_REFBU2_PICID(x) (((x) & 0x1f) << 14)
#define G1_REG_REF_BUF_CTRL2_APF_THRESHOLD(x) (((x) & 0x3fff) << 0)
-#define G1_REG_SOFT_RESET 0x194
+#define G1_REG_SOFT_RESET G1_SWREG(101)
#endif /* HANTRO_G1_REGS_H_ */
--
2.22.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/4] media: hantro: mpeg2_dec: Re-use common register macros
2019-10-03 19:08 ` [PATCH v2 2/4] media: hantro: mpeg2_dec: Re-use common register macros Ezequiel Garcia
@ 2019-10-03 19:39 ` Jonas Karlman
2019-10-03 20:08 ` Ezequiel Garcia
0 siblings, 1 reply; 10+ messages in thread
From: Jonas Karlman @ 2019-10-03 19:39 UTC (permalink / raw)
To: Ezequiel Garcia, linux-media
Cc: kernel, Tomasz Figa, linux-rockchip, Heiko Stuebner,
Philipp Zabel, Boris Brezillon, Chris Healy, linux-kernel
On 2019-10-03 21:08, Ezequiel Garcia wrote:
> The MPEG-2 decoder register macros can be re-used from hantro_g1_regs.h,
> and the re-definitions removed.
I am not a very big fan of this change as it makes it a little bit harder to spot differences
and to keep the RK3399 version of the code in sync.
The original MPEG-2 code for G1 (RK3288) and RK3399 was generated based on [1], [2] and [3]
with the main purpose to make it easy to keep the code for the different vpu variants in sync.
[1] https://github.com/Kwiboo/rockchip-vpu-regtool/blob/master/vpu1hwtable.h
[2] https://github.com/Kwiboo/rockchip-vpu-regtool/blob/master/vpu2hwtable.h
[3] https://github.com/Kwiboo/rockchip-vpu-regtool/blob/master/mpeg2.txt
Regards,
Jonas
>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
> .../media/hantro/hantro_g1_mpeg2_dec.c | 186 ++++++------------
> drivers/staging/media/hantro/hantro_g1_regs.h | 58 +++---
> 2 files changed, 96 insertions(+), 148 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
> index f3bf67d8a289..663bf05459a9 100644
> --- a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
> +++ b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
> @@ -10,60 +10,9 @@
> #include <media/v4l2-mem2mem.h>
> #include "hantro.h"
> #include "hantro_hw.h"
> +#include "hantro_g1_regs.h"
>
> -#define G1_SWREG(nr) ((nr) * 4)
> -
> -#define G1_REG_RLC_VLC_BASE G1_SWREG(12)
> -#define G1_REG_DEC_OUT_BASE G1_SWREG(13)
> -#define G1_REG_REFER0_BASE G1_SWREG(14)
> -#define G1_REG_REFER1_BASE G1_SWREG(15)
> -#define G1_REG_REFER2_BASE G1_SWREG(16)
> -#define G1_REG_REFER3_BASE G1_SWREG(17)
> -#define G1_REG_QTABLE_BASE G1_SWREG(40)
> -#define G1_REG_DEC_E(v) ((v) ? BIT(0) : 0)
> -
> -#define G1_REG_DEC_AXI_RD_ID(v) (((v) << 24) & GENMASK(31, 24))
> -#define G1_REG_DEC_TIMEOUT_E(v) ((v) ? BIT(23) : 0)
> -#define G1_REG_DEC_STRSWAP32_E(v) ((v) ? BIT(22) : 0)
> -#define G1_REG_DEC_STRENDIAN_E(v) ((v) ? BIT(21) : 0)
> -#define G1_REG_DEC_INSWAP32_E(v) ((v) ? BIT(20) : 0)
> -#define G1_REG_DEC_OUTSWAP32_E(v) ((v) ? BIT(19) : 0)
> -#define G1_REG_DEC_DATA_DISC_E(v) ((v) ? BIT(18) : 0)
> -#define G1_REG_DEC_LATENCY(v) (((v) << 11) & GENMASK(16, 11))
> -#define G1_REG_DEC_CLK_GATE_E(v) ((v) ? BIT(10) : 0)
> -#define G1_REG_DEC_IN_ENDIAN(v) ((v) ? BIT(9) : 0)
> -#define G1_REG_DEC_OUT_ENDIAN(v) ((v) ? BIT(8) : 0)
> -#define G1_REG_DEC_ADV_PRE_DIS(v) ((v) ? BIT(6) : 0)
> -#define G1_REG_DEC_SCMD_DIS(v) ((v) ? BIT(5) : 0)
> -#define G1_REG_DEC_MAX_BURST(v) (((v) << 0) & GENMASK(4, 0))
> -
> -#define G1_REG_DEC_MODE(v) (((v) << 28) & GENMASK(31, 28))
> -#define G1_REG_RLC_MODE_E(v) ((v) ? BIT(27) : 0)
> -#define G1_REG_PIC_INTERLACE_E(v) ((v) ? BIT(23) : 0)
> -#define G1_REG_PIC_FIELDMODE_E(v) ((v) ? BIT(22) : 0)
> -#define G1_REG_PIC_B_E(v) ((v) ? BIT(21) : 0)
> -#define G1_REG_PIC_INTER_E(v) ((v) ? BIT(20) : 0)
> -#define G1_REG_PIC_TOPFIELD_E(v) ((v) ? BIT(19) : 0)
> -#define G1_REG_FWD_INTERLACE_E(v) ((v) ? BIT(18) : 0)
> -#define G1_REG_FILTERING_DIS(v) ((v) ? BIT(14) : 0)
> -#define G1_REG_WRITE_MVS_E(v) ((v) ? BIT(12) : 0)
> -#define G1_REG_DEC_AXI_WR_ID(v) (((v) << 0) & GENMASK(7, 0))
> -
> -#define G1_REG_PIC_MB_WIDTH(v) (((v) << 23) & GENMASK(31, 23))
> -#define G1_REG_PIC_MB_HEIGHT_P(v) (((v) << 11) & GENMASK(18, 11))
> -#define G1_REG_ALT_SCAN_E(v) ((v) ? BIT(6) : 0)
> -#define G1_REG_TOPFIELDFIRST_E(v) ((v) ? BIT(5) : 0)
> -
> -#define G1_REG_STRM_START_BIT(v) (((v) << 26) & GENMASK(31, 26))
> -#define G1_REG_QSCALE_TYPE(v) ((v) ? BIT(24) : 0)
> -#define G1_REG_CON_MV_E(v) ((v) ? BIT(4) : 0)
> -#define G1_REG_INTRA_DC_PREC(v) (((v) << 2) & GENMASK(3, 2))
> -#define G1_REG_INTRA_VLC_TAB(v) ((v) ? BIT(1) : 0)
> -#define G1_REG_FRAME_PRED_DCT(v) ((v) ? BIT(0) : 0)
> -
> -#define G1_REG_INIT_QP(v) (((v) << 25) & GENMASK(30, 25))
> -#define G1_REG_STREAM_LEN(v) (((v) << 0) & GENMASK(23, 0))
> -
> +/* These bits seem undocumented. */
> #define G1_REG_ALT_SCAN_FLAG_E(v) ((v) ? BIT(19) : 0)
> #define G1_REG_FCODE_FWD_HOR(v) (((v) << 15) & GENMASK(18, 15))
> #define G1_REG_FCODE_FWD_VER(v) (((v) << 11) & GENMASK(14, 11))
> @@ -72,11 +21,6 @@
> #define G1_REG_MV_ACCURACY_FWD(v) ((v) ? BIT(2) : 0)
> #define G1_REG_MV_ACCURACY_BWD(v) ((v) ? BIT(1) : 0)
>
> -#define G1_REG_STARTMB_X(v) (((v) << 23) & GENMASK(31, 23))
> -#define G1_REG_STARTMB_Y(v) (((v) << 15) & GENMASK(22, 15))
> -
> -#define G1_REG_APF_THRESHOLD(v) (((v) << 0) & GENMASK(13, 0))
> -
> #define PICT_TOP_FIELD 1
> #define PICT_BOTTOM_FIELD 2
> #define PICT_FRAME 3
> @@ -92,7 +36,7 @@ hantro_g1_mpeg2_dec_set_quantization(struct hantro_dev *vpu,
> hantro_mpeg2_dec_copy_qtable(ctx->mpeg2_dec.qtable.cpu,
> quantization);
> vdpu_write_relaxed(vpu, ctx->mpeg2_dec.qtable.dma,
> - G1_REG_QTABLE_BASE);
> + G1_REG_ADDR_QTABLE);
> }
>
> static void
> @@ -118,7 +62,7 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
>
> /* Source bitstream buffer */
> addr = vb2_dma_contig_plane_dma_addr(src_buf, 0);
> - vdpu_write_relaxed(vpu, addr, G1_REG_RLC_VLC_BASE);
> + vdpu_write_relaxed(vpu, addr, G1_REG_ADDR_STR);
>
> /* Destination frame buffer */
> addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0);
> @@ -126,7 +70,7 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
>
> if (picture->picture_structure == PICT_BOTTOM_FIELD)
> addr += ALIGN(ctx->dst_fmt.width, 16);
> - vdpu_write_relaxed(vpu, addr, G1_REG_DEC_OUT_BASE);
> + vdpu_write_relaxed(vpu, addr, G1_REG_ADDR_DST);
>
> if (!forward_addr)
> forward_addr = current_addr;
> @@ -140,19 +84,19 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
> picture->top_field_first) ||
> (picture->picture_structure == PICT_BOTTOM_FIELD &&
> !picture->top_field_first)) {
> - vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER0_BASE);
> - vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER1_BASE);
> + vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(0));
> + vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(1));
> } else if (picture->picture_structure == PICT_TOP_FIELD) {
> - vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER0_BASE);
> - vdpu_write_relaxed(vpu, current_addr, G1_REG_REFER1_BASE);
> + vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(0));
> + vdpu_write_relaxed(vpu, current_addr, G1_REG_ADDR_REF(1));
> } else if (picture->picture_structure == PICT_BOTTOM_FIELD) {
> - vdpu_write_relaxed(vpu, current_addr, G1_REG_REFER0_BASE);
> - vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER1_BASE);
> + vdpu_write_relaxed(vpu, current_addr, G1_REG_ADDR_REF(0));
> + vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(1));
> }
>
> /* Set backward ref frame (top/bottom field) */
> - vdpu_write_relaxed(vpu, backward_addr, G1_REG_REFER2_BASE);
> - vdpu_write_relaxed(vpu, backward_addr, G1_REG_REFER3_BASE);
> + vdpu_write_relaxed(vpu, backward_addr, G1_REG_ADDR_REF(2));
> + vdpu_write_relaxed(vpu, backward_addr, G1_REG_ADDR_REF(3));
> }
>
> void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
> @@ -175,52 +119,51 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
> sequence = &slice_params->sequence;
> picture = &slice_params->picture;
>
> - reg = G1_REG_DEC_AXI_RD_ID(0) |
> - G1_REG_DEC_TIMEOUT_E(1) |
> - G1_REG_DEC_STRSWAP32_E(1) |
> - G1_REG_DEC_STRENDIAN_E(1) |
> - G1_REG_DEC_INSWAP32_E(1) |
> - G1_REG_DEC_OUTSWAP32_E(1) |
> - G1_REG_DEC_DATA_DISC_E(0) |
> - G1_REG_DEC_LATENCY(0) |
> - G1_REG_DEC_CLK_GATE_E(1) |
> - G1_REG_DEC_IN_ENDIAN(1) |
> - G1_REG_DEC_OUT_ENDIAN(1) |
> - G1_REG_DEC_ADV_PRE_DIS(0) |
> - G1_REG_DEC_SCMD_DIS(0) |
> - G1_REG_DEC_MAX_BURST(16);
> - vdpu_write_relaxed(vpu, reg, G1_SWREG(2));
> -
> - reg = G1_REG_DEC_MODE(5) |
> - G1_REG_RLC_MODE_E(0) |
> - G1_REG_PIC_INTERLACE_E(!sequence->progressive_sequence) |
> - G1_REG_PIC_FIELDMODE_E(picture->picture_structure != PICT_FRAME) |
> - G1_REG_PIC_B_E(picture->picture_coding_type == V4L2_MPEG2_PICTURE_CODING_TYPE_B) |
> - G1_REG_PIC_INTER_E(picture->picture_coding_type != V4L2_MPEG2_PICTURE_CODING_TYPE_I) |
> - G1_REG_PIC_TOPFIELD_E(picture->picture_structure == PICT_TOP_FIELD) |
> - G1_REG_FWD_INTERLACE_E(0) |
> - G1_REG_FILTERING_DIS(1) |
> - G1_REG_WRITE_MVS_E(0) |
> - G1_REG_DEC_AXI_WR_ID(0);
> - vdpu_write_relaxed(vpu, reg, G1_SWREG(3));
> -
> - reg = G1_REG_PIC_MB_WIDTH(MB_WIDTH(ctx->dst_fmt.width)) |
> - G1_REG_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->dst_fmt.height)) |
> - G1_REG_ALT_SCAN_E(picture->alternate_scan) |
> - G1_REG_TOPFIELDFIRST_E(picture->top_field_first);
> - vdpu_write_relaxed(vpu, reg, G1_SWREG(4));
> -
> - reg = G1_REG_STRM_START_BIT(slice_params->data_bit_offset) |
> - G1_REG_QSCALE_TYPE(picture->q_scale_type) |
> - G1_REG_CON_MV_E(picture->concealment_motion_vectors) |
> - G1_REG_INTRA_DC_PREC(picture->intra_dc_precision) |
> - G1_REG_INTRA_VLC_TAB(picture->intra_vlc_format) |
> - G1_REG_FRAME_PRED_DCT(picture->frame_pred_frame_dct);
> - vdpu_write_relaxed(vpu, reg, G1_SWREG(5));
> -
> - reg = G1_REG_INIT_QP(1) |
> - G1_REG_STREAM_LEN(slice_params->bit_size >> 3);
> - vdpu_write_relaxed(vpu, reg, G1_SWREG(6));
> + reg = G1_REG_CONFIG_DEC_AXI_RD_ID(0) |
> + G1_REG_CONFIG_DEC_TIMEOUT_E |
> + G1_REG_CONFIG_DEC_STRSWAP32_E |
> + G1_REG_CONFIG_DEC_STRENDIAN_E |
> + G1_REG_CONFIG_DEC_INSWAP32_E |
> + G1_REG_CONFIG_DEC_OUTSWAP32_E |
> + G1_REG_CONFIG_DEC_LATENCY(0) |
> + G1_REG_CONFIG_DEC_CLK_GATE_E |
> + G1_REG_CONFIG_DEC_IN_ENDIAN |
> + G1_REG_CONFIG_DEC_OUT_ENDIAN |
> + G1_REG_CONFIG_DEC_MAX_BURST(16);
> + vdpu_write_relaxed(vpu, reg, G1_REG_CONFIG);
> +
> + reg = G1_REG_DEC_CTRL0_DEC_MODE(5) |
> + G1_REG_DEC_CTRL0_FILTERING_DIS |
> + G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(0);
> + G1_SWREG_SET_IF(reg, !sequence->progressive_sequence,
> + G1_REG_DEC_CTRL0_PIC_INTERLACE_E);
> + G1_SWREG_SET_IF(reg, picture->picture_coding_type == V4L2_MPEG2_PICTURE_CODING_TYPE_B,
> + G1_REG_DEC_CTRL0_PIC_B_E);
> + G1_SWREG_SET_IF(reg, picture->picture_coding_type != V4L2_MPEG2_PICTURE_CODING_TYPE_I,
> + G1_REG_DEC_CTRL0_PIC_INTER_E);
> + G1_SWREG_SET_IF(reg, picture->picture_structure == PICT_TOP_FIELD,
> + G1_REG_DEC_CTRL0_PIC_TOPFIELD_E);
> + G1_SWREG_SET_IF(reg, picture->picture_structure != PICT_FRAME,
> + G1_REG_DEC_CTRL0_PIC_FIELDMODE_E);
> + vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
> +
> + reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(MB_WIDTH(ctx->dst_fmt.width)) |
> + G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->dst_fmt.height));
> + G1_SWREG_SET_IF(reg, picture->alternate_scan, G1_REG_DEC_CTRL1_ALT_SCAN_E);
> + G1_SWREG_SET_IF(reg, picture->top_field_first, G1_REG_DEC_CTRL1_TOPFIELDFIRST_E);
> + vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL1);
> +
> + reg = G1_REG_DEC_CTRL2_STRM_START_BIT(slice_params->data_bit_offset) |
> + G1_REG_DEC_CTRL2_INTRA_DC_PREC(picture->intra_dc_precision);
> + G1_SWREG_SET_IF(reg, picture->q_scale_type,G1_REG_DEC_CTRL2_QSCALE_TYPE);
> + G1_SWREG_SET_IF(reg, picture->concealment_motion_vectors, G1_REG_DEC_CTRL2_CON_MV_E);
> + G1_SWREG_SET_IF(reg, picture->intra_vlc_format, G1_REG_DEC_CTRL2_INTRA_VLC_TAB);
> + G1_SWREG_SET_IF(reg, picture->frame_pred_frame_dct, G1_REG_DEC_CTRL2_FRAME_PRED_DCT);
> + vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL2);
> +
> + reg = G1_REG_DEC_CTRL3_INIT_QP(1) |
> + G1_REG_DEC_CTRL3_STREAM_LEN(slice_params->bit_size >> 3);
> + vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL3);
>
> reg = G1_REG_ALT_SCAN_FLAG_E(picture->alternate_scan) |
> G1_REG_FCODE_FWD_HOR(picture->f_code[0][0]) |
> @@ -231,12 +174,12 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
> G1_REG_MV_ACCURACY_BWD(1);
> vdpu_write_relaxed(vpu, reg, G1_SWREG(18));
>
> - reg = G1_REG_STARTMB_X(0) |
> - G1_REG_STARTMB_Y(0);
> - vdpu_write_relaxed(vpu, reg, G1_SWREG(48));
> + reg = G1_REG_ERR_CONC_STARTMB_X(0) |
> + G1_REG_ERR_CONC_STARTMB_Y(0);
> + vdpu_write_relaxed(vpu, reg, G1_REG_ERR_CONC);
>
> - reg = G1_REG_APF_THRESHOLD(8);
> - vdpu_write_relaxed(vpu, reg, G1_SWREG(55));
> + reg = G1_REG_REF_BUF_CTRL2_APF_THRESHOLD(8);
> + vdpu_write_relaxed(vpu, reg, G1_REG_REF_BUF_CTRL2);
>
> hantro_g1_mpeg2_dec_set_quantization(vpu, ctx);
>
> @@ -246,6 +189,5 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
>
> hantro_finish_run(ctx);
>
> - reg = G1_REG_DEC_E(1);
> - vdpu_write(vpu, reg, G1_SWREG(1));
> + vdpu_write(vpu, G1_REG_INTERRUPT_DEC_E, G1_REG_INTERRUPT);
> }
> diff --git a/drivers/staging/media/hantro/hantro_g1_regs.h b/drivers/staging/media/hantro/hantro_g1_regs.h
> index 5c0ea7994336..84b93915a953 100644
> --- a/drivers/staging/media/hantro/hantro_g1_regs.h
> +++ b/drivers/staging/media/hantro/hantro_g1_regs.h
> @@ -9,8 +9,14 @@
> #ifndef HANTRO_G1_REGS_H_
> #define HANTRO_G1_REGS_H_
>
> +#define G1_SWREG(nr) ((nr) * 4)
> +#define G1_SWREG_SET_IF(reg, cond, val) \
> + do \
> + reg |= (cond) ? (val) : 0x0; \
> + while (0)
> +
> /* Decoder registers. */
> -#define G1_REG_INTERRUPT 0x004
> +#define G1_REG_INTERRUPT G1_SWREG(1)
> #define G1_REG_INTERRUPT_DEC_PIC_INF BIT(24)
> #define G1_REG_INTERRUPT_DEC_TIMEOUT BIT(18)
> #define G1_REG_INTERRUPT_DEC_SLICE_INT BIT(17)
> @@ -22,7 +28,7 @@
> #define G1_REG_INTERRUPT_DEC_IRQ BIT(8)
> #define G1_REG_INTERRUPT_DEC_IRQ_DIS BIT(4)
> #define G1_REG_INTERRUPT_DEC_E BIT(0)
> -#define G1_REG_CONFIG 0x008
> +#define G1_REG_CONFIG G1_SWREG(2)
> #define G1_REG_CONFIG_DEC_AXI_RD_ID(x) (((x) & 0xff) << 24)
> #define G1_REG_CONFIG_DEC_TIMEOUT_E BIT(23)
> #define G1_REG_CONFIG_DEC_STRSWAP32_E BIT(22)
> @@ -41,7 +47,7 @@
> #define G1_REG_CONFIG_DEC_ADV_PRE_DIS BIT(6)
> #define G1_REG_CONFIG_DEC_SCMD_DIS BIT(5)
> #define G1_REG_CONFIG_DEC_MAX_BURST(x) (((x) & 0x1f) << 0)
> -#define G1_REG_DEC_CTRL0 0x00c
> +#define G1_REG_DEC_CTRL0 G1_SWREG(3)
> #define G1_REG_DEC_CTRL0_DEC_MODE(x) (((x) & 0xf) << 28)
> #define G1_REG_DEC_CTRL0_RLC_MODE_E BIT(27)
> #define G1_REG_DEC_CTRL0_SKIP_MODE BIT(26)
> @@ -66,7 +72,7 @@
> #define G1_REG_DEC_CTRL0_PICORD_COUNT_E BIT(9)
> #define G1_REG_DEC_CTRL0_DEC_AHB_HLOCK_E BIT(8)
> #define G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(x) (((x) & 0xff) << 0)
> -#define G1_REG_DEC_CTRL1 0x010
> +#define G1_REG_DEC_CTRL1 G1_SWREG(4)
> #define G1_REG_DEC_CTRL1_PIC_MB_WIDTH(x) (((x) & 0x1ff) << 23)
> #define G1_REG_DEC_CTRL1_MB_WIDTH_OFF(x) (((x) & 0xf) << 19)
> #define G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(x) (((x) & 0xff) << 11)
> @@ -77,7 +83,7 @@
> #define G1_REG_DEC_CTRL1_PIC_MB_W_EXT(x) (((x) & 0x7) << 3)
> #define G1_REG_DEC_CTRL1_PIC_MB_H_EXT(x) (((x) & 0x7) << 0)
> #define G1_REG_DEC_CTRL1_PIC_REFER_FLAG BIT(0)
> -#define G1_REG_DEC_CTRL2 0x014
> +#define G1_REG_DEC_CTRL2 G1_SWREG(5)
> #define G1_REG_DEC_CTRL2_STRM_START_BIT(x) (((x) & 0x3f) << 26)
> #define G1_REG_DEC_CTRL2_SYNC_MARKER_E BIT(25)
> #define G1_REG_DEC_CTRL2_TYPE1_QUANT_E BIT(24)
> @@ -120,13 +126,13 @@
> #define G1_REG_DEC_CTRL2_BOOLEAN_RANGE(x) (((x) & 0xff) << 0)
> #define G1_REG_DEC_CTRL2_ALPHA_OFFSET(x) (((x) & 0x1f) << 5)
> #define G1_REG_DEC_CTRL2_BETA_OFFSET(x) (((x) & 0x1f) << 0)
> -#define G1_REG_DEC_CTRL3 0x018
> +#define G1_REG_DEC_CTRL3 G1_SWREG(6)
> #define G1_REG_DEC_CTRL3_START_CODE_E BIT(31)
> #define G1_REG_DEC_CTRL3_INIT_QP(x) (((x) & 0x3f) << 25)
> #define G1_REG_DEC_CTRL3_CH_8PIX_ILEAV_E BIT(24)
> #define G1_REG_DEC_CTRL3_STREAM_LEN_EXT(x) (((x) & 0xff) << 24)
> #define G1_REG_DEC_CTRL3_STREAM_LEN(x) (((x) & 0xffffff) << 0)
> -#define G1_REG_DEC_CTRL4 0x01c
> +#define G1_REG_DEC_CTRL4 G1_SWREG(7)
> #define G1_REG_DEC_CTRL4_CABAC_E BIT(31)
> #define G1_REG_DEC_CTRL4_BLACKWHITE_E BIT(30)
> #define G1_REG_DEC_CTRL4_DIR_8X8_INFER_E BIT(29)
> @@ -163,7 +169,7 @@
> #define G1_REG_DEC_CTRL4_INIT_DC_MATCH0(x) (((x) & 0x7) << 9)
> #define G1_REG_DEC_CTRL4_INIT_DC_MATCH1(x) (((x) & 0x7) << 6)
> #define G1_REG_DEC_CTRL4_VP7_VERSION BIT(5)
> -#define G1_REG_DEC_CTRL5 0x020
> +#define G1_REG_DEC_CTRL5 G1_SWREG(8)
> #define G1_REG_DEC_CTRL5_CONST_INTRA_E BIT(31)
> #define G1_REG_DEC_CTRL5_FILT_CTRL_PRES BIT(30)
> #define G1_REG_DEC_CTRL5_RDPIC_CNT_PRES BIT(29)
> @@ -187,7 +193,7 @@
> #define G1_REG_DEC_CTRL5_RV_BWD_SCALE(x) (((x) & 0x3fff) << 0)
> #define G1_REG_DEC_CTRL5_INIT_DC_COMP0(x) (((x) & 0xffff) << 16)
> #define G1_REG_DEC_CTRL5_INIT_DC_COMP1(x) (((x) & 0xffff) << 0)
> -#define G1_REG_DEC_CTRL6 0x024
> +#define G1_REG_DEC_CTRL6 G1_SWREG(9)
> #define G1_REG_DEC_CTRL6_PPS_ID(x) (((x) & 0xff) << 24)
> #define G1_REG_DEC_CTRL6_REFIDX1_ACTIVE(x) (((x) & 0x1f) << 19)
> #define G1_REG_DEC_CTRL6_REFIDX0_ACTIVE(x) (((x) & 0x1f) << 14)
> @@ -198,7 +204,7 @@
> #define G1_REG_DEC_CTRL6_STREAM1_LEN(x) (((x) & 0xffffff) << 0)
> #define G1_REG_DEC_CTRL6_PIC_SLICE_AM(x) (((x) & 0x1fff) << 0)
> #define G1_REG_DEC_CTRL6_COEFFS_PART_AM(x) (((x) & 0xf) << 24)
> -#define G1_REG_FWD_PIC(i) (0x028 + ((i) * 0x4))
> +#define G1_REG_FWD_PIC(i) (G1_SWREG(10) + ((i) * 0x4))
> #define G1_REG_FWD_PIC_PINIT_RLIST_F5(x) (((x) & 0x1f) << 25)
> #define G1_REG_FWD_PIC_PINIT_RLIST_F4(x) (((x) & 0x1f) << 20)
> #define G1_REG_FWD_PIC_PINIT_RLIST_F3(x) (((x) & 0x1f) << 15)
> @@ -211,7 +217,7 @@
> #define G1_REG_FWD_PIC1_SEGMENT_BASE(x) ((x) << 0)
> #define G1_REG_FWD_PIC1_SEGMENT_UPD_E BIT(1)
> #define G1_REG_FWD_PIC1_SEGMENT_E BIT(0)
> -#define G1_REG_DEC_CTRL7 0x02c
> +#define G1_REG_DEC_CTRL7 G1_SWREG(11)
> #define G1_REG_DEC_CTRL7_PINIT_RLIST_F15(x) (((x) & 0x1f) << 25)
> #define G1_REG_DEC_CTRL7_PINIT_RLIST_F14(x) (((x) & 0x1f) << 20)
> #define G1_REG_DEC_CTRL7_PINIT_RLIST_F13(x) (((x) & 0x1f) << 15)
> @@ -226,12 +232,12 @@
> #define G1_REG_DEC_CTRL7_DCT5_START_BIT(x) (((x) & 0x3f) << 12)
> #define G1_REG_DEC_CTRL7_DCT6_START_BIT(x) (((x) & 0x3f) << 6)
> #define G1_REG_DEC_CTRL7_DCT7_START_BIT(x) (((x) & 0x3f) << 0)
> -#define G1_REG_ADDR_STR 0x030
> -#define G1_REG_ADDR_DST 0x034
> -#define G1_REG_ADDR_REF(i) (0x038 + ((i) * 0x4))
> +#define G1_REG_ADDR_STR G1_SWREG(12)
> +#define G1_REG_ADDR_DST G1_SWREG(13)
> +#define G1_REG_ADDR_REF(i) (G1_SWREG(14) + ((i) * 0x4))
> #define G1_REG_ADDR_REF_FIELD_E BIT(1)
> #define G1_REG_ADDR_REF_TOPC_E BIT(0)
> -#define G1_REG_REF_PIC(i) (0x078 + ((i) * 0x4))
> +#define G1_REG_REF_PIC(i) (G1_SWREG(30) + ((i) * 0x4))
> #define G1_REG_REF_PIC_FILT_TYPE_E BIT(31)
> #define G1_REG_REF_PIC_FILT_SHARPNESS(x) (((x) & 0x7) << 28)
> #define G1_REG_REF_PIC_MB_ADJ_0(x) (((x) & 0x7f) << 21)
> @@ -248,11 +254,11 @@
> #define G1_REG_REF_PIC_QUANT_DELTA_1(x) (((x) & 0x1f) << 22)
> #define G1_REG_REF_PIC_QUANT_0(x) (((x) & 0x7ff) << 11)
> #define G1_REG_REF_PIC_QUANT_1(x) (((x) & 0x7ff) << 0)
> -#define G1_REG_LT_REF 0x098
> -#define G1_REG_VALID_REF 0x09c
> -#define G1_REG_ADDR_QTABLE 0x0a0
> -#define G1_REG_ADDR_DIR_MV 0x0a4
> -#define G1_REG_BD_REF_PIC(i) (0x0a8 + ((i) * 0x4))
> +#define G1_REG_LT_REF G1_SWREG(38)
> +#define G1_REG_VALID_REF G1_SWREG(39)
> +#define G1_REG_ADDR_QTABLE G1_SWREG(40)
> +#define G1_REG_ADDR_DIR_MV G1_SWREG(41)
> +#define G1_REG_BD_REF_PIC(i) (G1_SWREG(42) + ((i) * 0x4))
> #define G1_REG_BD_REF_PIC_BINIT_RLIST_B2(x) (((x) & 0x1f) << 25)
> #define G1_REG_BD_REF_PIC_BINIT_RLIST_F2(x) (((x) & 0x1f) << 20)
> #define G1_REG_BD_REF_PIC_BINIT_RLIST_B1(x) (((x) & 0x1f) << 15)
> @@ -269,7 +275,7 @@
> #define G1_REG_BD_REF_PIC_QUANT_DELTA_3(x) (((x) & 0x1f) << 22)
> #define G1_REG_BD_REF_PIC_QUANT_2(x) (((x) & 0x7ff) << 11)
> #define G1_REG_BD_REF_PIC_QUANT_3(x) (((x) & 0x7ff) << 0)
> -#define G1_REG_BD_P_REF_PIC 0x0bc
> +#define G1_REG_BD_P_REF_PIC G1_SWREG(47)
> #define G1_REG_BD_P_REF_PIC_QUANT_DELTA_4(x) (((x) & 0x1f) << 27)
> #define G1_REG_BD_P_REF_PIC_PINIT_RLIST_F3(x) (((x) & 0x1f) << 25)
> #define G1_REG_BD_P_REF_PIC_PINIT_RLIST_F2(x) (((x) & 0x1f) << 20)
> @@ -277,25 +283,25 @@
> #define G1_REG_BD_P_REF_PIC_PINIT_RLIST_F0(x) (((x) & 0x1f) << 10)
> #define G1_REG_BD_P_REF_PIC_BINIT_RLIST_B15(x) (((x) & 0x1f) << 5)
> #define G1_REG_BD_P_REF_PIC_BINIT_RLIST_F15(x) (((x) & 0x1f) << 0)
> -#define G1_REG_ERR_CONC 0x0c0
> +#define G1_REG_ERR_CONC G1_SWREG(48)
> #define G1_REG_ERR_CONC_STARTMB_X(x) (((x) & 0x1ff) << 23)
> #define G1_REG_ERR_CONC_STARTMB_Y(x) (((x) & 0xff) << 15)
> -#define G1_REG_PRED_FLT 0x0c4
> +#define G1_REG_PRED_FLT G1_SWREG(49)
> #define G1_REG_PRED_FLT_PRED_BC_TAP_0_0(x) (((x) & 0x3ff) << 22)
> #define G1_REG_PRED_FLT_PRED_BC_TAP_0_1(x) (((x) & 0x3ff) << 12)
> #define G1_REG_PRED_FLT_PRED_BC_TAP_0_2(x) (((x) & 0x3ff) << 2)
> -#define G1_REG_REF_BUF_CTRL 0x0cc
> +#define G1_REG_REF_BUF_CTRL G1_SWREG(51)
> #define G1_REG_REF_BUF_CTRL_REFBU_E BIT(31)
> #define G1_REG_REF_BUF_CTRL_REFBU_THR(x) (((x) & 0xfff) << 19)
> #define G1_REG_REF_BUF_CTRL_REFBU_PICID(x) (((x) & 0x1f) << 14)
> #define G1_REG_REF_BUF_CTRL_REFBU_EVAL_E BIT(13)
> #define G1_REG_REF_BUF_CTRL_REFBU_FPARMOD_E BIT(12)
> #define G1_REG_REF_BUF_CTRL_REFBU_Y_OFFSET(x) (((x) & 0x1ff) << 0)
> -#define G1_REG_REF_BUF_CTRL2 0x0dc
> +#define G1_REG_REF_BUF_CTRL2 G1_SWREG(55)
> #define G1_REG_REF_BUF_CTRL2_REFBU2_BUF_E BIT(31)
> #define G1_REG_REF_BUF_CTRL2_REFBU2_THR(x) (((x) & 0xfff) << 19)
> #define G1_REG_REF_BUF_CTRL2_REFBU2_PICID(x) (((x) & 0x1f) << 14)
> #define G1_REG_REF_BUF_CTRL2_APF_THRESHOLD(x) (((x) & 0x3fff) << 0)
> -#define G1_REG_SOFT_RESET 0x194
> +#define G1_REG_SOFT_RESET G1_SWREG(101)
>
> #endif /* HANTRO_G1_REGS_H_ */
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/4] media: hantro: mpeg2_dec: Re-use common register macros
2019-10-03 19:39 ` Jonas Karlman
@ 2019-10-03 20:08 ` Ezequiel Garcia
0 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2019-10-03 20:08 UTC (permalink / raw)
To: Jonas Karlman, linux-media
Cc: kernel, Tomasz Figa, linux-rockchip, Heiko Stuebner,
Philipp Zabel, Boris Brezillon, Chris Healy, linux-kernel
On Thu, 2019-10-03 at 19:39 +0000, Jonas Karlman wrote:
> On 2019-10-03 21:08, Ezequiel Garcia wrote:
> > The MPEG-2 decoder register macros can be re-used from hantro_g1_regs.h,
> > and the re-definitions removed.
>
> I am not a very big fan of this change as it makes it a little bit harder to spot differences
> and to keep the RK3399 version of the code in sync.
>
> The original MPEG-2 code for G1 (RK3288) and RK3399 was generated based on [1], [2] and [3]
> with the main purpose to make it easy to keep the code for the different vpu variants in sync.
>
> [1] https://github.com/Kwiboo/rockchip-vpu-regtool/blob/master/vpu1hwtable.h
> [2] https://github.com/Kwiboo/rockchip-vpu-regtool/blob/master/vpu2hwtable.h
> [3] https://github.com/Kwiboo/rockchip-vpu-regtool/blob/master/mpeg2.txt
>
Yeah, that maybe true. A more useful improvement, would be to get rid of the rk3399
implementation and have a single one using, following the example in hantro_postproc.c,
introduced in patch 4.
I guess we can drop this patch, and instead work towards a unification goal.
OTOH, it's a bit annoying to have hantro_g1_regs.h and then not use it
and roll our own, which is what motivated this.
Thanks for reviewing!
Ezequiel
> Regards,
> Jonas
>
> > Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> > ---
> > .../media/hantro/hantro_g1_mpeg2_dec.c | 186 ++++++------------
> > drivers/staging/media/hantro/hantro_g1_regs.h | 58 +++---
> > 2 files changed, 96 insertions(+), 148 deletions(-)
> >
> > diff --git a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
> > index f3bf67d8a289..663bf05459a9 100644
> > --- a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
> > +++ b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
> > @@ -10,60 +10,9 @@
> > #include <media/v4l2-mem2mem.h>
> > #include "hantro.h"
> > #include "hantro_hw.h"
> > +#include "hantro_g1_regs.h"
> >
> > -#define G1_SWREG(nr) ((nr) * 4)
> > -
> > -#define G1_REG_RLC_VLC_BASE G1_SWREG(12)
> > -#define G1_REG_DEC_OUT_BASE G1_SWREG(13)
> > -#define G1_REG_REFER0_BASE G1_SWREG(14)
> > -#define G1_REG_REFER1_BASE G1_SWREG(15)
> > -#define G1_REG_REFER2_BASE G1_SWREG(16)
> > -#define G1_REG_REFER3_BASE G1_SWREG(17)
> > -#define G1_REG_QTABLE_BASE G1_SWREG(40)
> > -#define G1_REG_DEC_E(v) ((v) ? BIT(0) : 0)
> > -
> > -#define G1_REG_DEC_AXI_RD_ID(v) (((v) << 24) & GENMASK(31, 24))
> > -#define G1_REG_DEC_TIMEOUT_E(v) ((v) ? BIT(23) : 0)
> > -#define G1_REG_DEC_STRSWAP32_E(v) ((v) ? BIT(22) : 0)
> > -#define G1_REG_DEC_STRENDIAN_E(v) ((v) ? BIT(21) : 0)
> > -#define G1_REG_DEC_INSWAP32_E(v) ((v) ? BIT(20) : 0)
> > -#define G1_REG_DEC_OUTSWAP32_E(v) ((v) ? BIT(19) : 0)
> > -#define G1_REG_DEC_DATA_DISC_E(v) ((v) ? BIT(18) : 0)
> > -#define G1_REG_DEC_LATENCY(v) (((v) << 11) & GENMASK(16, 11))
> > -#define G1_REG_DEC_CLK_GATE_E(v) ((v) ? BIT(10) : 0)
> > -#define G1_REG_DEC_IN_ENDIAN(v) ((v) ? BIT(9) : 0)
> > -#define G1_REG_DEC_OUT_ENDIAN(v) ((v) ? BIT(8) : 0)
> > -#define G1_REG_DEC_ADV_PRE_DIS(v) ((v) ? BIT(6) : 0)
> > -#define G1_REG_DEC_SCMD_DIS(v) ((v) ? BIT(5) : 0)
> > -#define G1_REG_DEC_MAX_BURST(v) (((v) << 0) & GENMASK(4, 0))
> > -
> > -#define G1_REG_DEC_MODE(v) (((v) << 28) & GENMASK(31, 28))
> > -#define G1_REG_RLC_MODE_E(v) ((v) ? BIT(27) : 0)
> > -#define G1_REG_PIC_INTERLACE_E(v) ((v) ? BIT(23) : 0)
> > -#define G1_REG_PIC_FIELDMODE_E(v) ((v) ? BIT(22) : 0)
> > -#define G1_REG_PIC_B_E(v) ((v) ? BIT(21) : 0)
> > -#define G1_REG_PIC_INTER_E(v) ((v) ? BIT(20) : 0)
> > -#define G1_REG_PIC_TOPFIELD_E(v) ((v) ? BIT(19) : 0)
> > -#define G1_REG_FWD_INTERLACE_E(v) ((v) ? BIT(18) : 0)
> > -#define G1_REG_FILTERING_DIS(v) ((v) ? BIT(14) : 0)
> > -#define G1_REG_WRITE_MVS_E(v) ((v) ? BIT(12) : 0)
> > -#define G1_REG_DEC_AXI_WR_ID(v) (((v) << 0) & GENMASK(7, 0))
> > -
> > -#define G1_REG_PIC_MB_WIDTH(v) (((v) << 23) & GENMASK(31, 23))
> > -#define G1_REG_PIC_MB_HEIGHT_P(v) (((v) << 11) & GENMASK(18, 11))
> > -#define G1_REG_ALT_SCAN_E(v) ((v) ? BIT(6) : 0)
> > -#define G1_REG_TOPFIELDFIRST_E(v) ((v) ? BIT(5) : 0)
> > -
> > -#define G1_REG_STRM_START_BIT(v) (((v) << 26) & GENMASK(31, 26))
> > -#define G1_REG_QSCALE_TYPE(v) ((v) ? BIT(24) : 0)
> > -#define G1_REG_CON_MV_E(v) ((v) ? BIT(4) : 0)
> > -#define G1_REG_INTRA_DC_PREC(v) (((v) << 2) & GENMASK(3, 2))
> > -#define G1_REG_INTRA_VLC_TAB(v) ((v) ? BIT(1) : 0)
> > -#define G1_REG_FRAME_PRED_DCT(v) ((v) ? BIT(0) : 0)
> > -
> > -#define G1_REG_INIT_QP(v) (((v) << 25) & GENMASK(30, 25))
> > -#define G1_REG_STREAM_LEN(v) (((v) << 0) & GENMASK(23, 0))
> > -
> > +/* These bits seem undocumented. */
> > #define G1_REG_ALT_SCAN_FLAG_E(v) ((v) ? BIT(19) : 0)
> > #define G1_REG_FCODE_FWD_HOR(v) (((v) << 15) & GENMASK(18, 15))
> > #define G1_REG_FCODE_FWD_VER(v) (((v) << 11) & GENMASK(14, 11))
> > @@ -72,11 +21,6 @@
> > #define G1_REG_MV_ACCURACY_FWD(v) ((v) ? BIT(2) : 0)
> > #define G1_REG_MV_ACCURACY_BWD(v) ((v) ? BIT(1) : 0)
> >
> > -#define G1_REG_STARTMB_X(v) (((v) << 23) & GENMASK(31, 23))
> > -#define G1_REG_STARTMB_Y(v) (((v) << 15) & GENMASK(22, 15))
> > -
> > -#define G1_REG_APF_THRESHOLD(v) (((v) << 0) & GENMASK(13, 0))
> > -
> > #define PICT_TOP_FIELD 1
> > #define PICT_BOTTOM_FIELD 2
> > #define PICT_FRAME 3
> > @@ -92,7 +36,7 @@ hantro_g1_mpeg2_dec_set_quantization(struct hantro_dev *vpu,
> > hantro_mpeg2_dec_copy_qtable(ctx->mpeg2_dec.qtable.cpu,
> > quantization);
> > vdpu_write_relaxed(vpu, ctx->mpeg2_dec.qtable.dma,
> > - G1_REG_QTABLE_BASE);
> > + G1_REG_ADDR_QTABLE);
> > }
> >
> > static void
> > @@ -118,7 +62,7 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
> >
> > /* Source bitstream buffer */
> > addr = vb2_dma_contig_plane_dma_addr(src_buf, 0);
> > - vdpu_write_relaxed(vpu, addr, G1_REG_RLC_VLC_BASE);
> > + vdpu_write_relaxed(vpu, addr, G1_REG_ADDR_STR);
> >
> > /* Destination frame buffer */
> > addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0);
> > @@ -126,7 +70,7 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
> >
> > if (picture->picture_structure == PICT_BOTTOM_FIELD)
> > addr += ALIGN(ctx->dst_fmt.width, 16);
> > - vdpu_write_relaxed(vpu, addr, G1_REG_DEC_OUT_BASE);
> > + vdpu_write_relaxed(vpu, addr, G1_REG_ADDR_DST);
> >
> > if (!forward_addr)
> > forward_addr = current_addr;
> > @@ -140,19 +84,19 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
> > picture->top_field_first) ||
> > (picture->picture_structure == PICT_BOTTOM_FIELD &&
> > !picture->top_field_first)) {
> > - vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER0_BASE);
> > - vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER1_BASE);
> > + vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(0));
> > + vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(1));
> > } else if (picture->picture_structure == PICT_TOP_FIELD) {
> > - vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER0_BASE);
> > - vdpu_write_relaxed(vpu, current_addr, G1_REG_REFER1_BASE);
> > + vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(0));
> > + vdpu_write_relaxed(vpu, current_addr, G1_REG_ADDR_REF(1));
> > } else if (picture->picture_structure == PICT_BOTTOM_FIELD) {
> > - vdpu_write_relaxed(vpu, current_addr, G1_REG_REFER0_BASE);
> > - vdpu_write_relaxed(vpu, forward_addr, G1_REG_REFER1_BASE);
> > + vdpu_write_relaxed(vpu, current_addr, G1_REG_ADDR_REF(0));
> > + vdpu_write_relaxed(vpu, forward_addr, G1_REG_ADDR_REF(1));
> > }
> >
> > /* Set backward ref frame (top/bottom field) */
> > - vdpu_write_relaxed(vpu, backward_addr, G1_REG_REFER2_BASE);
> > - vdpu_write_relaxed(vpu, backward_addr, G1_REG_REFER3_BASE);
> > + vdpu_write_relaxed(vpu, backward_addr, G1_REG_ADDR_REF(2));
> > + vdpu_write_relaxed(vpu, backward_addr, G1_REG_ADDR_REF(3));
> > }
> >
> > void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
> > @@ -175,52 +119,51 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
> > sequence = &slice_params->sequence;
> > picture = &slice_params->picture;
> >
> > - reg = G1_REG_DEC_AXI_RD_ID(0) |
> > - G1_REG_DEC_TIMEOUT_E(1) |
> > - G1_REG_DEC_STRSWAP32_E(1) |
> > - G1_REG_DEC_STRENDIAN_E(1) |
> > - G1_REG_DEC_INSWAP32_E(1) |
> > - G1_REG_DEC_OUTSWAP32_E(1) |
> > - G1_REG_DEC_DATA_DISC_E(0) |
> > - G1_REG_DEC_LATENCY(0) |
> > - G1_REG_DEC_CLK_GATE_E(1) |
> > - G1_REG_DEC_IN_ENDIAN(1) |
> > - G1_REG_DEC_OUT_ENDIAN(1) |
> > - G1_REG_DEC_ADV_PRE_DIS(0) |
> > - G1_REG_DEC_SCMD_DIS(0) |
> > - G1_REG_DEC_MAX_BURST(16);
> > - vdpu_write_relaxed(vpu, reg, G1_SWREG(2));
> > -
> > - reg = G1_REG_DEC_MODE(5) |
> > - G1_REG_RLC_MODE_E(0) |
> > - G1_REG_PIC_INTERLACE_E(!sequence->progressive_sequence) |
> > - G1_REG_PIC_FIELDMODE_E(picture->picture_structure != PICT_FRAME) |
> > - G1_REG_PIC_B_E(picture->picture_coding_type == V4L2_MPEG2_PICTURE_CODING_TYPE_B) |
> > - G1_REG_PIC_INTER_E(picture->picture_coding_type != V4L2_MPEG2_PICTURE_CODING_TYPE_I) |
> > - G1_REG_PIC_TOPFIELD_E(picture->picture_structure == PICT_TOP_FIELD) |
> > - G1_REG_FWD_INTERLACE_E(0) |
> > - G1_REG_FILTERING_DIS(1) |
> > - G1_REG_WRITE_MVS_E(0) |
> > - G1_REG_DEC_AXI_WR_ID(0);
> > - vdpu_write_relaxed(vpu, reg, G1_SWREG(3));
> > -
> > - reg = G1_REG_PIC_MB_WIDTH(MB_WIDTH(ctx->dst_fmt.width)) |
> > - G1_REG_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->dst_fmt.height)) |
> > - G1_REG_ALT_SCAN_E(picture->alternate_scan) |
> > - G1_REG_TOPFIELDFIRST_E(picture->top_field_first);
> > - vdpu_write_relaxed(vpu, reg, G1_SWREG(4));
> > -
> > - reg = G1_REG_STRM_START_BIT(slice_params->data_bit_offset) |
> > - G1_REG_QSCALE_TYPE(picture->q_scale_type) |
> > - G1_REG_CON_MV_E(picture->concealment_motion_vectors) |
> > - G1_REG_INTRA_DC_PREC(picture->intra_dc_precision) |
> > - G1_REG_INTRA_VLC_TAB(picture->intra_vlc_format) |
> > - G1_REG_FRAME_PRED_DCT(picture->frame_pred_frame_dct);
> > - vdpu_write_relaxed(vpu, reg, G1_SWREG(5));
> > -
> > - reg = G1_REG_INIT_QP(1) |
> > - G1_REG_STREAM_LEN(slice_params->bit_size >> 3);
> > - vdpu_write_relaxed(vpu, reg, G1_SWREG(6));
> > + reg = G1_REG_CONFIG_DEC_AXI_RD_ID(0) |
> > + G1_REG_CONFIG_DEC_TIMEOUT_E |
> > + G1_REG_CONFIG_DEC_STRSWAP32_E |
> > + G1_REG_CONFIG_DEC_STRENDIAN_E |
> > + G1_REG_CONFIG_DEC_INSWAP32_E |
> > + G1_REG_CONFIG_DEC_OUTSWAP32_E |
> > + G1_REG_CONFIG_DEC_LATENCY(0) |
> > + G1_REG_CONFIG_DEC_CLK_GATE_E |
> > + G1_REG_CONFIG_DEC_IN_ENDIAN |
> > + G1_REG_CONFIG_DEC_OUT_ENDIAN |
> > + G1_REG_CONFIG_DEC_MAX_BURST(16);
> > + vdpu_write_relaxed(vpu, reg, G1_REG_CONFIG);
> > +
> > + reg = G1_REG_DEC_CTRL0_DEC_MODE(5) |
> > + G1_REG_DEC_CTRL0_FILTERING_DIS |
> > + G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(0);
> > + G1_SWREG_SET_IF(reg, !sequence->progressive_sequence,
> > + G1_REG_DEC_CTRL0_PIC_INTERLACE_E);
> > + G1_SWREG_SET_IF(reg, picture->picture_coding_type == V4L2_MPEG2_PICTURE_CODING_TYPE_B,
> > + G1_REG_DEC_CTRL0_PIC_B_E);
> > + G1_SWREG_SET_IF(reg, picture->picture_coding_type != V4L2_MPEG2_PICTURE_CODING_TYPE_I,
> > + G1_REG_DEC_CTRL0_PIC_INTER_E);
> > + G1_SWREG_SET_IF(reg, picture->picture_structure == PICT_TOP_FIELD,
> > + G1_REG_DEC_CTRL0_PIC_TOPFIELD_E);
> > + G1_SWREG_SET_IF(reg, picture->picture_structure != PICT_FRAME,
> > + G1_REG_DEC_CTRL0_PIC_FIELDMODE_E);
> > + vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
> > +
> > + reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(MB_WIDTH(ctx->dst_fmt.width)) |
> > + G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->dst_fmt.height));
> > + G1_SWREG_SET_IF(reg, picture->alternate_scan, G1_REG_DEC_CTRL1_ALT_SCAN_E);
> > + G1_SWREG_SET_IF(reg, picture->top_field_first, G1_REG_DEC_CTRL1_TOPFIELDFIRST_E);
> > + vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL1);
> > +
> > + reg = G1_REG_DEC_CTRL2_STRM_START_BIT(slice_params->data_bit_offset) |
> > + G1_REG_DEC_CTRL2_INTRA_DC_PREC(picture->intra_dc_precision);
> > + G1_SWREG_SET_IF(reg, picture->q_scale_type,G1_REG_DEC_CTRL2_QSCALE_TYPE);
> > + G1_SWREG_SET_IF(reg, picture->concealment_motion_vectors, G1_REG_DEC_CTRL2_CON_MV_E);
> > + G1_SWREG_SET_IF(reg, picture->intra_vlc_format, G1_REG_DEC_CTRL2_INTRA_VLC_TAB);
> > + G1_SWREG_SET_IF(reg, picture->frame_pred_frame_dct, G1_REG_DEC_CTRL2_FRAME_PRED_DCT);
> > + vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL2);
> > +
> > + reg = G1_REG_DEC_CTRL3_INIT_QP(1) |
> > + G1_REG_DEC_CTRL3_STREAM_LEN(slice_params->bit_size >> 3);
> > + vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL3);
> >
> > reg = G1_REG_ALT_SCAN_FLAG_E(picture->alternate_scan) |
> > G1_REG_FCODE_FWD_HOR(picture->f_code[0][0]) |
> > @@ -231,12 +174,12 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
> > G1_REG_MV_ACCURACY_BWD(1);
> > vdpu_write_relaxed(vpu, reg, G1_SWREG(18));
> >
> > - reg = G1_REG_STARTMB_X(0) |
> > - G1_REG_STARTMB_Y(0);
> > - vdpu_write_relaxed(vpu, reg, G1_SWREG(48));
> > + reg = G1_REG_ERR_CONC_STARTMB_X(0) |
> > + G1_REG_ERR_CONC_STARTMB_Y(0);
> > + vdpu_write_relaxed(vpu, reg, G1_REG_ERR_CONC);
> >
> > - reg = G1_REG_APF_THRESHOLD(8);
> > - vdpu_write_relaxed(vpu, reg, G1_SWREG(55));
> > + reg = G1_REG_REF_BUF_CTRL2_APF_THRESHOLD(8);
> > + vdpu_write_relaxed(vpu, reg, G1_REG_REF_BUF_CTRL2);
> >
> > hantro_g1_mpeg2_dec_set_quantization(vpu, ctx);
> >
> > @@ -246,6 +189,5 @@ void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx)
> >
> > hantro_finish_run(ctx);
> >
> > - reg = G1_REG_DEC_E(1);
> > - vdpu_write(vpu, reg, G1_SWREG(1));
> > + vdpu_write(vpu, G1_REG_INTERRUPT_DEC_E, G1_REG_INTERRUPT);
> > }
> > diff --git a/drivers/staging/media/hantro/hantro_g1_regs.h b/drivers/staging/media/hantro/hantro_g1_regs.h
> > index 5c0ea7994336..84b93915a953 100644
> > --- a/drivers/staging/media/hantro/hantro_g1_regs.h
> > +++ b/drivers/staging/media/hantro/hantro_g1_regs.h
> > @@ -9,8 +9,14 @@
> > #ifndef HANTRO_G1_REGS_H_
> > #define HANTRO_G1_REGS_H_
> >
> > +#define G1_SWREG(nr) ((nr) * 4)
> > +#define G1_SWREG_SET_IF(reg, cond, val) \
> > + do \
> > + reg |= (cond) ? (val) : 0x0; \
> > + while (0)
> > +
> > /* Decoder registers. */
> > -#define G1_REG_INTERRUPT 0x004
> > +#define G1_REG_INTERRUPT G1_SWREG(1)
> > #define G1_REG_INTERRUPT_DEC_PIC_INF BIT(24)
> > #define G1_REG_INTERRUPT_DEC_TIMEOUT BIT(18)
> > #define G1_REG_INTERRUPT_DEC_SLICE_INT BIT(17)
> > @@ -22,7 +28,7 @@
> > #define G1_REG_INTERRUPT_DEC_IRQ BIT(8)
> > #define G1_REG_INTERRUPT_DEC_IRQ_DIS BIT(4)
> > #define G1_REG_INTERRUPT_DEC_E BIT(0)
> > -#define G1_REG_CONFIG 0x008
> > +#define G1_REG_CONFIG G1_SWREG(2)
> > #define G1_REG_CONFIG_DEC_AXI_RD_ID(x) (((x) & 0xff) << 24)
> > #define G1_REG_CONFIG_DEC_TIMEOUT_E BIT(23)
> > #define G1_REG_CONFIG_DEC_STRSWAP32_E BIT(22)
> > @@ -41,7 +47,7 @@
> > #define G1_REG_CONFIG_DEC_ADV_PRE_DIS BIT(6)
> > #define G1_REG_CONFIG_DEC_SCMD_DIS BIT(5)
> > #define G1_REG_CONFIG_DEC_MAX_BURST(x) (((x) & 0x1f) << 0)
> > -#define G1_REG_DEC_CTRL0 0x00c
> > +#define G1_REG_DEC_CTRL0 G1_SWREG(3)
> > #define G1_REG_DEC_CTRL0_DEC_MODE(x) (((x) & 0xf) << 28)
> > #define G1_REG_DEC_CTRL0_RLC_MODE_E BIT(27)
> > #define G1_REG_DEC_CTRL0_SKIP_MODE BIT(26)
> > @@ -66,7 +72,7 @@
> > #define G1_REG_DEC_CTRL0_PICORD_COUNT_E BIT(9)
> > #define G1_REG_DEC_CTRL0_DEC_AHB_HLOCK_E BIT(8)
> > #define G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(x) (((x) & 0xff) << 0)
> > -#define G1_REG_DEC_CTRL1 0x010
> > +#define G1_REG_DEC_CTRL1 G1_SWREG(4)
> > #define G1_REG_DEC_CTRL1_PIC_MB_WIDTH(x) (((x) & 0x1ff) << 23)
> > #define G1_REG_DEC_CTRL1_MB_WIDTH_OFF(x) (((x) & 0xf) << 19)
> > #define G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(x) (((x) & 0xff) << 11)
> > @@ -77,7 +83,7 @@
> > #define G1_REG_DEC_CTRL1_PIC_MB_W_EXT(x) (((x) & 0x7) << 3)
> > #define G1_REG_DEC_CTRL1_PIC_MB_H_EXT(x) (((x) & 0x7) << 0)
> > #define G1_REG_DEC_CTRL1_PIC_REFER_FLAG BIT(0)
> > -#define G1_REG_DEC_CTRL2 0x014
> > +#define G1_REG_DEC_CTRL2 G1_SWREG(5)
> > #define G1_REG_DEC_CTRL2_STRM_START_BIT(x) (((x) & 0x3f) << 26)
> > #define G1_REG_DEC_CTRL2_SYNC_MARKER_E BIT(25)
> > #define G1_REG_DEC_CTRL2_TYPE1_QUANT_E BIT(24)
> > @@ -120,13 +126,13 @@
> > #define G1_REG_DEC_CTRL2_BOOLEAN_RANGE(x) (((x) & 0xff) << 0)
> > #define G1_REG_DEC_CTRL2_ALPHA_OFFSET(x) (((x) & 0x1f) << 5)
> > #define G1_REG_DEC_CTRL2_BETA_OFFSET(x) (((x) & 0x1f) << 0)
> > -#define G1_REG_DEC_CTRL3 0x018
> > +#define G1_REG_DEC_CTRL3 G1_SWREG(6)
> > #define G1_REG_DEC_CTRL3_START_CODE_E BIT(31)
> > #define G1_REG_DEC_CTRL3_INIT_QP(x) (((x) & 0x3f) << 25)
> > #define G1_REG_DEC_CTRL3_CH_8PIX_ILEAV_E BIT(24)
> > #define G1_REG_DEC_CTRL3_STREAM_LEN_EXT(x) (((x) & 0xff) << 24)
> > #define G1_REG_DEC_CTRL3_STREAM_LEN(x) (((x) & 0xffffff) << 0)
> > -#define G1_REG_DEC_CTRL4 0x01c
> > +#define G1_REG_DEC_CTRL4 G1_SWREG(7)
> > #define G1_REG_DEC_CTRL4_CABAC_E BIT(31)
> > #define G1_REG_DEC_CTRL4_BLACKWHITE_E BIT(30)
> > #define G1_REG_DEC_CTRL4_DIR_8X8_INFER_E BIT(29)
> > @@ -163,7 +169,7 @@
> > #define G1_REG_DEC_CTRL4_INIT_DC_MATCH0(x) (((x) & 0x7) << 9)
> > #define G1_REG_DEC_CTRL4_INIT_DC_MATCH1(x) (((x) & 0x7) << 6)
> > #define G1_REG_DEC_CTRL4_VP7_VERSION BIT(5)
> > -#define G1_REG_DEC_CTRL5 0x020
> > +#define G1_REG_DEC_CTRL5 G1_SWREG(8)
> > #define G1_REG_DEC_CTRL5_CONST_INTRA_E BIT(31)
> > #define G1_REG_DEC_CTRL5_FILT_CTRL_PRES BIT(30)
> > #define G1_REG_DEC_CTRL5_RDPIC_CNT_PRES BIT(29)
> > @@ -187,7 +193,7 @@
> > #define G1_REG_DEC_CTRL5_RV_BWD_SCALE(x) (((x) & 0x3fff) << 0)
> > #define G1_REG_DEC_CTRL5_INIT_DC_COMP0(x) (((x) & 0xffff) << 16)
> > #define G1_REG_DEC_CTRL5_INIT_DC_COMP1(x) (((x) & 0xffff) << 0)
> > -#define G1_REG_DEC_CTRL6 0x024
> > +#define G1_REG_DEC_CTRL6 G1_SWREG(9)
> > #define G1_REG_DEC_CTRL6_PPS_ID(x) (((x) & 0xff) << 24)
> > #define G1_REG_DEC_CTRL6_REFIDX1_ACTIVE(x) (((x) & 0x1f) << 19)
> > #define G1_REG_DEC_CTRL6_REFIDX0_ACTIVE(x) (((x) & 0x1f) << 14)
> > @@ -198,7 +204,7 @@
> > #define G1_REG_DEC_CTRL6_STREAM1_LEN(x) (((x) & 0xffffff) << 0)
> > #define G1_REG_DEC_CTRL6_PIC_SLICE_AM(x) (((x) & 0x1fff) << 0)
> > #define G1_REG_DEC_CTRL6_COEFFS_PART_AM(x) (((x) & 0xf) << 24)
> > -#define G1_REG_FWD_PIC(i) (0x028 + ((i) * 0x4))
> > +#define G1_REG_FWD_PIC(i) (G1_SWREG(10) + ((i) * 0x4))
> > #define G1_REG_FWD_PIC_PINIT_RLIST_F5(x) (((x) & 0x1f) << 25)
> > #define G1_REG_FWD_PIC_PINIT_RLIST_F4(x) (((x) & 0x1f) << 20)
> > #define G1_REG_FWD_PIC_PINIT_RLIST_F3(x) (((x) & 0x1f) << 15)
> > @@ -211,7 +217,7 @@
> > #define G1_REG_FWD_PIC1_SEGMENT_BASE(x) ((x) << 0)
> > #define G1_REG_FWD_PIC1_SEGMENT_UPD_E BIT(1)
> > #define G1_REG_FWD_PIC1_SEGMENT_E BIT(0)
> > -#define G1_REG_DEC_CTRL7 0x02c
> > +#define G1_REG_DEC_CTRL7 G1_SWREG(11)
> > #define G1_REG_DEC_CTRL7_PINIT_RLIST_F15(x) (((x) & 0x1f) << 25)
> > #define G1_REG_DEC_CTRL7_PINIT_RLIST_F14(x) (((x) & 0x1f) << 20)
> > #define G1_REG_DEC_CTRL7_PINIT_RLIST_F13(x) (((x) & 0x1f) << 15)
> > @@ -226,12 +232,12 @@
> > #define G1_REG_DEC_CTRL7_DCT5_START_BIT(x) (((x) & 0x3f) << 12)
> > #define G1_REG_DEC_CTRL7_DCT6_START_BIT(x) (((x) & 0x3f) << 6)
> > #define G1_REG_DEC_CTRL7_DCT7_START_BIT(x) (((x) & 0x3f) << 0)
> > -#define G1_REG_ADDR_STR 0x030
> > -#define G1_REG_ADDR_DST 0x034
> > -#define G1_REG_ADDR_REF(i) (0x038 + ((i) * 0x4))
> > +#define G1_REG_ADDR_STR G1_SWREG(12)
> > +#define G1_REG_ADDR_DST G1_SWREG(13)
> > +#define G1_REG_ADDR_REF(i) (G1_SWREG(14) + ((i) * 0x4))
> > #define G1_REG_ADDR_REF_FIELD_E BIT(1)
> > #define G1_REG_ADDR_REF_TOPC_E BIT(0)
> > -#define G1_REG_REF_PIC(i) (0x078 + ((i) * 0x4))
> > +#define G1_REG_REF_PIC(i) (G1_SWREG(30) + ((i) * 0x4))
> > #define G1_REG_REF_PIC_FILT_TYPE_E BIT(31)
> > #define G1_REG_REF_PIC_FILT_SHARPNESS(x) (((x) & 0x7) << 28)
> > #define G1_REG_REF_PIC_MB_ADJ_0(x) (((x) & 0x7f) << 21)
> > @@ -248,11 +254,11 @@
> > #define G1_REG_REF_PIC_QUANT_DELTA_1(x) (((x) & 0x1f) << 22)
> > #define G1_REG_REF_PIC_QUANT_0(x) (((x) & 0x7ff) << 11)
> > #define G1_REG_REF_PIC_QUANT_1(x) (((x) & 0x7ff) << 0)
> > -#define G1_REG_LT_REF 0x098
> > -#define G1_REG_VALID_REF 0x09c
> > -#define G1_REG_ADDR_QTABLE 0x0a0
> > -#define G1_REG_ADDR_DIR_MV 0x0a4
> > -#define G1_REG_BD_REF_PIC(i) (0x0a8 + ((i) * 0x4))
> > +#define G1_REG_LT_REF G1_SWREG(38)
> > +#define G1_REG_VALID_REF G1_SWREG(39)
> > +#define G1_REG_ADDR_QTABLE G1_SWREG(40)
> > +#define G1_REG_ADDR_DIR_MV G1_SWREG(41)
> > +#define G1_REG_BD_REF_PIC(i) (G1_SWREG(42) + ((i) * 0x4))
> > #define G1_REG_BD_REF_PIC_BINIT_RLIST_B2(x) (((x) & 0x1f) << 25)
> > #define G1_REG_BD_REF_PIC_BINIT_RLIST_F2(x) (((x) & 0x1f) << 20)
> > #define G1_REG_BD_REF_PIC_BINIT_RLIST_B1(x) (((x) & 0x1f) << 15)
> > @@ -269,7 +275,7 @@
> > #define G1_REG_BD_REF_PIC_QUANT_DELTA_3(x) (((x) & 0x1f) << 22)
> > #define G1_REG_BD_REF_PIC_QUANT_2(x) (((x) & 0x7ff) << 11)
> > #define G1_REG_BD_REF_PIC_QUANT_3(x) (((x) & 0x7ff) << 0)
> > -#define G1_REG_BD_P_REF_PIC 0x0bc
> > +#define G1_REG_BD_P_REF_PIC G1_SWREG(47)
> > #define G1_REG_BD_P_REF_PIC_QUANT_DELTA_4(x) (((x) & 0x1f) << 27)
> > #define G1_REG_BD_P_REF_PIC_PINIT_RLIST_F3(x) (((x) & 0x1f) << 25)
> > #define G1_REG_BD_P_REF_PIC_PINIT_RLIST_F2(x) (((x) & 0x1f) << 20)
> > @@ -277,25 +283,25 @@
> > #define G1_REG_BD_P_REF_PIC_PINIT_RLIST_F0(x) (((x) & 0x1f) << 10)
> > #define G1_REG_BD_P_REF_PIC_BINIT_RLIST_B15(x) (((x) & 0x1f) << 5)
> > #define G1_REG_BD_P_REF_PIC_BINIT_RLIST_F15(x) (((x) & 0x1f) << 0)
> > -#define G1_REG_ERR_CONC 0x0c0
> > +#define G1_REG_ERR_CONC G1_SWREG(48)
> > #define G1_REG_ERR_CONC_STARTMB_X(x) (((x) & 0x1ff) << 23)
> > #define G1_REG_ERR_CONC_STARTMB_Y(x) (((x) & 0xff) << 15)
> > -#define G1_REG_PRED_FLT 0x0c4
> > +#define G1_REG_PRED_FLT G1_SWREG(49)
> > #define G1_REG_PRED_FLT_PRED_BC_TAP_0_0(x) (((x) & 0x3ff) << 22)
> > #define G1_REG_PRED_FLT_PRED_BC_TAP_0_1(x) (((x) & 0x3ff) << 12)
> > #define G1_REG_PRED_FLT_PRED_BC_TAP_0_2(x) (((x) & 0x3ff) << 2)
> > -#define G1_REG_REF_BUF_CTRL 0x0cc
> > +#define G1_REG_REF_BUF_CTRL G1_SWREG(51)
> > #define G1_REG_REF_BUF_CTRL_REFBU_E BIT(31)
> > #define G1_REG_REF_BUF_CTRL_REFBU_THR(x) (((x) & 0xfff) << 19)
> > #define G1_REG_REF_BUF_CTRL_REFBU_PICID(x) (((x) & 0x1f) << 14)
> > #define G1_REG_REF_BUF_CTRL_REFBU_EVAL_E BIT(13)
> > #define G1_REG_REF_BUF_CTRL_REFBU_FPARMOD_E BIT(12)
> > #define G1_REG_REF_BUF_CTRL_REFBU_Y_OFFSET(x) (((x) & 0x1ff) << 0)
> > -#define G1_REG_REF_BUF_CTRL2 0x0dc
> > +#define G1_REG_REF_BUF_CTRL2 G1_SWREG(55)
> > #define G1_REG_REF_BUF_CTRL2_REFBU2_BUF_E BIT(31)
> > #define G1_REG_REF_BUF_CTRL2_REFBU2_THR(x) (((x) & 0xfff) << 19)
> > #define G1_REG_REF_BUF_CTRL2_REFBU2_PICID(x) (((x) & 0x1f) << 14)
> > #define G1_REG_REF_BUF_CTRL2_APF_THRESHOLD(x) (((x) & 0x3fff) << 0)
> > -#define G1_REG_SOFT_RESET 0x194
> > +#define G1_REG_SOFT_RESET G1_SWREG(101)
> >
> > #endif /* HANTRO_G1_REGS_H_ */
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/4] media: hantro: Rework media topology
2019-10-03 19:08 [PATCH v2 0/4] Enable Hantro G1 post-processor Ezequiel Garcia
2019-10-03 19:08 ` [PATCH v2 1/4] media: hantro: Cleanup format negotiation helpers Ezequiel Garcia
2019-10-03 19:08 ` [PATCH v2 2/4] media: hantro: mpeg2_dec: Re-use common register macros Ezequiel Garcia
@ 2019-10-03 19:08 ` Ezequiel Garcia
2019-10-03 19:08 ` [PATCH v2 4/4] media: hantro: Support color conversion via post-processing Ezequiel Garcia
2019-10-04 6:07 ` [PATCH v2 0/4] Enable Hantro G1 post-processor Tomasz Figa
4 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2019-10-03 19:08 UTC (permalink / raw)
To: linux-media
Cc: kernel, Tomasz Figa, linux-rockchip, Heiko Stuebner,
Jonas Karlman, Philipp Zabel, Boris Brezillon, Chris Healy,
linux-kernel, Ezequiel Garcia, Helen Koike
As suggested by Helen Koike, the decoder processing entity can be
modeled as a V4L2 subdevice.
This change will allow to describe more complex topology
and/or behavior to userspace, starting with the post-processing
feature, which will be soon introduced.
For now, introduce a simple subdevice, maintaining an immutable
topology, and now exposing the subdevices to userspace.
Suggested-by: Helen Koike <helen.koike@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
drivers/staging/media/hantro/hantro.h | 21 +-
drivers/staging/media/hantro/hantro_drv.c | 250 +++++++++++++++------
drivers/staging/media/hantro/hantro_v4l2.c | 18 +-
3 files changed, 205 insertions(+), 84 deletions(-)
diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index deb90ae37859..15506b9a34f4 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -124,25 +124,19 @@ struct hantro_ctrl {
* %MEDIA_ENT_F_PROC_VIDEO_DECODER)
* @vdev: &struct video_device that exposes the encoder or
* decoder functionality
- * @source_pad: &struct media_pad with the source pad.
- * @sink: &struct media_entity pointer with the sink entity
- * @sink_pad: &struct media_pad with the sink pad.
- * @proc: &struct media_entity pointer with the M2M device itself.
- * @proc_pads: &struct media_pad with the @proc pads.
- * @intf_devnode: &struct media_intf devnode pointer with the interface
- * with controls the M2M device.
+ * @vdev_pads: &struct media_pad with the @vdev pads.
+ * @sd_proc: &struct v4l2_subdev exposing the encoder or decoder sub-device
+ * @sd_proc_pads: &struct media_pad with the @sd_proc pads.
*
* Contains everything needed to attach the video device to the media device.
*/
struct hantro_func {
unsigned int id;
struct video_device vdev;
- struct media_pad source_pad;
- struct media_entity sink;
- struct media_pad sink_pad;
- struct media_entity proc;
- struct media_pad proc_pads[2];
- struct media_intf_devnode *intf_devnode;
+ struct media_pad vdev_pads[2];
+
+ struct v4l2_subdev sd_proc;
+ struct media_pad sd_proc_pads[2];
};
static inline struct hantro_func *
@@ -220,6 +214,7 @@ struct hantro_dev {
struct hantro_ctx {
struct hantro_dev *dev;
struct v4l2_fh fh;
+ struct media_pipeline pipe;
u32 sequence_cap;
u32 sequence_out;
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 26108c96b674..35beb5a9bf52 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -488,9 +488,67 @@ static const struct of_device_id of_hantro_match[] = {
};
MODULE_DEVICE_TABLE(of, of_hantro_match);
+static int link_setup(struct media_entity *entity,
+ const struct media_pad *local,
+ const struct media_pad *remote, u32 flags)
+{
+ /* empty for now */
+ return 0;
+}
+
+static const struct media_entity_operations sd_mops = {
+ .link_setup = link_setup,
+};
+
+static const struct v4l2_subdev_ops sd_ops = {
+ /* empty */
+};
+
+static int
+hantro_subdev_register(struct hantro_dev *vpu,
+ struct hantro_func *func,
+ struct v4l2_subdev *sd,
+ const char *const name,
+ u32 function,
+ struct media_pad *pads,
+ const struct v4l2_subdev_internal_ops *sd_int_ops,
+ const struct v4l2_subdev_ops *sd_ops)
+{
+ int ret;
+
+ /* Initialize the subdev */
+ v4l2_subdev_init(sd, sd_ops);
+ sd->internal_ops = sd_int_ops;
+ sd->entity.function = function;
+ sd->entity.ops = &sd_mops;
+ sd->owner = THIS_MODULE;
+ strscpy(sd->name, name, sizeof(sd->name));
+ v4l2_set_subdevdata(sd, vpu);
+
+ if (sd->ctrl_handler)
+ sd->flags |= V4L2_SUBDEV_FL_HAS_EVENTS;
+
+ /* Initialize the media entity */
+ pads[0].flags = MEDIA_PAD_FL_SINK;
+ pads[1].flags = MEDIA_PAD_FL_SOURCE;
+ ret = media_entity_pads_init(&sd->entity, 2, pads);
+ if (ret)
+ return ret;
+
+ /* Register the subdev with the v4l2 and the media frameworks */
+ ret = v4l2_device_register_subdev(&vpu->v4l2_dev, sd);
+ if (ret) {
+ v4l2_err(&vpu->v4l2_dev,
+ "%s: subdev register failed (err=%d)\n",
+ name, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
static int hantro_register_entity(struct media_device *mdev,
struct media_entity *entity,
- const char *entity_name,
struct media_pad *pads, int num_pads,
int function, struct video_device *vdev)
{
@@ -503,8 +561,7 @@ static int hantro_register_entity(struct media_device *mdev,
entity->info.dev.minor = vdev->minor;
}
- name = devm_kasprintf(mdev->dev, GFP_KERNEL, "%s-%s", vdev->name,
- entity_name);
+ name = devm_kasprintf(mdev->dev, GFP_KERNEL, "%s", vdev->name);
if (!name)
return -ENOMEM;
@@ -522,61 +579,132 @@ static int hantro_register_entity(struct media_device *mdev,
return 0;
}
+#define HANTRO_LINK_(_src, _sink, link_flags) { \
+ .source = _src, \
+ .sink = _sink, \
+ .flags = link_flags, \
+}
+
+#define HANTRO_LINK_IM(_src, _sink) \
+ HANTRO_LINK_(_src, _sink, MEDIA_LNK_FL_ENABLED | MEDIA_LNK_FL_IMMUTABLE)
+
+#define HANTRO_LINK_EN(_src, _sink) \
+ HANTRO_LINK_(_src, _sink, MEDIA_LNK_FL_ENABLED)
+
+#define HANTRO_LINK(_src, _sink) \
+ HANTRO_LINK_(_src, _sink, 0)
+
+#define HANTRO_SUBDEV(_subdev, _name, _function, _pads) { \
+ .subdev = _subdev, \
+ .name = _name, \
+ .function = _function, \
+ .pads = _pads, \
+}
+
static int hantro_attach_func(struct hantro_dev *vpu,
struct hantro_func *func)
{
struct media_device *mdev = &vpu->mdev;
struct media_link *link;
+ unsigned int i, num_subdevs, num_links;
int ret;
- /* Create the three encoder entities with their pads */
- func->source_pad.flags = MEDIA_PAD_FL_SOURCE;
- ret = hantro_register_entity(mdev, &func->vdev.entity, "source",
- &func->source_pad, 1, MEDIA_ENT_F_IO_V4L,
- &func->vdev);
- if (ret)
- return ret;
+ /*
+ * In order to ease the media topology setup,
+ * define a couple compound types. Keep these
+ * types local, as they are not needed them
+ * elsewhere.
+ */
+ struct hantro_subdev {
+ struct v4l2_subdev *subdev;
+ struct media_pad *pads;
+ const char *name;
+ u32 function;
+ };
+ struct hantro_link {
+ struct media_entity *source;
+ struct media_entity *sink;
+ u32 flags;
+ };
+ const struct hantro_subdev decoder_subdevs[] = {
+ HANTRO_SUBDEV(&func->sd_proc, "decoder", func->id,
+ func->sd_proc_pads),
+ };
+ const struct hantro_subdev encoder_subdevs[] = {
+ HANTRO_SUBDEV(&func->sd_proc, "encoder", func->id,
+ func->sd_proc_pads),
+ };
+ const struct hantro_link decoder_links[] = {
+ /* decoder -> vdev */
+ HANTRO_LINK_IM(&func->sd_proc.entity, &func->vdev.entity),
+ /* vdev -> decoder */
+ HANTRO_LINK_IM(&func->vdev.entity, &func->sd_proc.entity),
+ };
+ const struct hantro_link encoder_links[] = {
+ /* encoder -> vdev */
+ HANTRO_LINK_IM(&func->sd_proc.entity, &func->vdev.entity),
+ /* vdev -> encoder */
+ HANTRO_LINK_IM(&func->vdev.entity, &func->sd_proc.entity),
+ };
+ const struct hantro_subdev *subdevs;
+ const struct hantro_link *links;
- func->proc_pads[0].flags = MEDIA_PAD_FL_SINK;
- func->proc_pads[1].flags = MEDIA_PAD_FL_SOURCE;
- ret = hantro_register_entity(mdev, &func->proc, "proc",
- func->proc_pads, 2, func->id,
- &func->vdev);
- if (ret)
- goto err_rel_entity0;
+ if (func->id == MEDIA_ENT_F_PROC_VIDEO_ENCODER) {
+ subdevs = encoder_subdevs;
+ links = encoder_links;
+ num_subdevs = ARRAY_SIZE(encoder_subdevs);
+ num_links = ARRAY_SIZE(encoder_links);
+ } else {
+ subdevs = decoder_subdevs;
+ links = decoder_links;
+ num_subdevs = ARRAY_SIZE(decoder_subdevs);
+ num_links = ARRAY_SIZE(decoder_links);
+ }
- func->sink_pad.flags = MEDIA_PAD_FL_SINK;
- ret = hantro_register_entity(mdev, &func->sink, "sink",
- &func->sink_pad, 1, MEDIA_ENT_F_IO_V4L,
+ for (i = 0; i < num_subdevs; i++) {
+ const struct hantro_subdev *subdev = &subdevs[i];
+
+ ret = hantro_subdev_register(vpu, func, subdev->subdev,
+ subdev->name, subdev->function,
+ subdev->pads,
+ NULL, &sd_ops);
+ if (ret)
+ goto err_unreg_subdevs;
+ }
+
+ func->vdev_pads[0].flags = MEDIA_PAD_FL_SINK;
+ func->vdev_pads[1].flags = MEDIA_PAD_FL_SOURCE;
+ ret = hantro_register_entity(mdev, &func->vdev.entity,
+ func->vdev_pads, 2,
+ MEDIA_ENT_F_IO_V4L,
&func->vdev);
if (ret)
- goto err_rel_entity1;
+ goto err_unreg_subdevs;
- /* Connect the three entities */
- ret = media_create_pad_link(&func->vdev.entity, 0, &func->proc, 1,
- MEDIA_LNK_FL_IMMUTABLE |
- MEDIA_LNK_FL_ENABLED);
- if (ret)
- goto err_rel_entity2;
+ for (i = 0; i < num_links; i++) {
+ const struct hantro_link *link = &links[i];
- ret = media_create_pad_link(&func->proc, 0, &func->sink, 0,
- MEDIA_LNK_FL_IMMUTABLE |
- MEDIA_LNK_FL_ENABLED);
- if (ret)
- goto err_rm_links0;
+ ret = media_create_pad_link(link->source, 1,
+ link->sink, 0,
+ link->flags);
+ if (ret) {
+ ret = -ENOMEM;
+ goto err_unreg_entity;
+ }
+ }
- /* Create video interface */
- func->intf_devnode = media_devnode_create(mdev, MEDIA_INTF_T_V4L_VIDEO,
- 0, VIDEO_MAJOR,
- func->vdev.minor);
- if (!func->intf_devnode) {
+ /* Create the video device interface and link it. */
+ func->vdev.intf_devnode =
+ media_devnode_create(mdev, MEDIA_INTF_T_V4L_VIDEO,
+ 0, VIDEO_MAJOR,
+ func->vdev.minor);
+ if (!func->vdev.intf_devnode) {
ret = -ENOMEM;
- goto err_rm_links1;
+ goto err_unreg_entity;
}
- /* Connect the two DMA engines to the interface */
link = media_create_intf_link(&func->vdev.entity,
- &func->intf_devnode->intf,
+ &func->vdev.intf_devnode->intf,
MEDIA_LNK_FL_IMMUTABLE |
MEDIA_LNK_FL_ENABLED);
if (!link) {
@@ -584,45 +712,22 @@ static int hantro_attach_func(struct hantro_dev *vpu,
goto err_rm_devnode;
}
- link = media_create_intf_link(&func->sink, &func->intf_devnode->intf,
- MEDIA_LNK_FL_IMMUTABLE |
- MEDIA_LNK_FL_ENABLED);
- if (!link) {
- ret = -ENOMEM;
- goto err_rm_devnode;
- }
return 0;
-
err_rm_devnode:
- media_devnode_remove(func->intf_devnode);
-
-err_rm_links1:
- media_entity_remove_links(&func->sink);
-
-err_rm_links0:
- media_entity_remove_links(&func->proc);
- media_entity_remove_links(&func->vdev.entity);
-
-err_rel_entity2:
- media_device_unregister_entity(&func->sink);
-
-err_rel_entity1:
- media_device_unregister_entity(&func->proc);
-
-err_rel_entity0:
+ media_devnode_remove(func->vdev.intf_devnode);
+err_unreg_entity:
media_device_unregister_entity(&func->vdev.entity);
+err_unreg_subdevs:
+ for (i = 0; i < num_subdevs; i++)
+ v4l2_device_unregister_subdev(subdevs[i].subdev);
return ret;
}
static void hantro_detach_func(struct hantro_func *func)
{
- media_devnode_remove(func->intf_devnode);
- media_entity_remove_links(&func->sink);
- media_entity_remove_links(&func->proc);
- media_entity_remove_links(&func->vdev.entity);
- media_device_unregister_entity(&func->sink);
- media_device_unregister_entity(&func->proc);
+ media_devnode_remove(func->vdev.intf_devnode);
media_device_unregister_entity(&func->vdev.entity);
+ v4l2_device_unregister_subdev(&func->sd_proc);
}
static int hantro_add_func(struct hantro_dev *vpu, unsigned int funcid)
@@ -866,6 +971,11 @@ static int hantro_probe(struct platform_device *pdev)
goto err_rm_dec_func;
}
+ ret = v4l2_device_register_subdev_nodes(&vpu->v4l2_dev);
+ if (ret) {
+ v4l2_err(&vpu->v4l2_dev, "Failed to register subdev nodes\n");
+ goto err_rm_dec_func;
+ }
return 0;
err_rm_dec_func:
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index 238e53b28f8f..58fa4b52275b 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -594,6 +594,7 @@ static bool hantro_vq_is_coded(struct vb2_queue *q)
static int hantro_start_streaming(struct vb2_queue *q, unsigned int count)
{
struct hantro_ctx *ctx = vb2_get_drv_priv(q);
+ struct media_entity *entity = &ctx->fh.vdev->entity;
int ret = 0;
if (V4L2_TYPE_IS_OUTPUT(q->type))
@@ -601,6 +602,11 @@ static int hantro_start_streaming(struct vb2_queue *q, unsigned int count)
else
ctx->sequence_cap = 0;
+ /* Start the media pipeline */
+ ret = media_pipeline_start(entity, &ctx->pipe);
+ if (ret)
+ return ret;
+
if (hantro_vq_is_coded(q)) {
enum hantro_codec_mode codec_mode;
@@ -611,11 +617,18 @@ static int hantro_start_streaming(struct vb2_queue *q, unsigned int count)
vpu_debug(4, "Codec mode = %d\n", codec_mode);
ctx->codec_ops = &ctx->dev->variant->codec_ops[codec_mode];
- if (ctx->codec_ops->init)
+ if (ctx->codec_ops->init) {
ret = ctx->codec_ops->init(ctx);
+ if (ret)
+ goto err_pipe_stop;
+ }
}
return ret;
+
+err_pipe_stop:
+ media_pipeline_stop(entity);
+ return ret;
}
static void
@@ -639,12 +652,15 @@ hantro_return_bufs(struct vb2_queue *q,
static void hantro_stop_streaming(struct vb2_queue *q)
{
struct hantro_ctx *ctx = vb2_get_drv_priv(q);
+ struct media_entity *entity = &ctx->fh.vdev->entity;
if (hantro_vq_is_coded(q)) {
if (ctx->codec_ops && ctx->codec_ops->exit)
ctx->codec_ops->exit(ctx);
}
+ media_pipeline_stop(entity);
+
/*
* The mem2mem framework calls v4l2_m2m_cancel_job before
* .stop_streaming, so there isn't any job running and
--
2.22.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 4/4] media: hantro: Support color conversion via post-processing
2019-10-03 19:08 [PATCH v2 0/4] Enable Hantro G1 post-processor Ezequiel Garcia
` (2 preceding siblings ...)
2019-10-03 19:08 ` [PATCH v2 3/4] media: hantro: Rework media topology Ezequiel Garcia
@ 2019-10-03 19:08 ` Ezequiel Garcia
2019-10-04 6:07 ` [PATCH v2 0/4] Enable Hantro G1 post-processor Tomasz Figa
4 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2019-10-03 19:08 UTC (permalink / raw)
To: linux-media
Cc: kernel, Tomasz Figa, linux-rockchip, Heiko Stuebner,
Jonas Karlman, Philipp Zabel, Boris Brezillon, Chris Healy,
linux-kernel, Ezequiel Garcia
The Hantro G1 decoder is able to enable a post-processor
on the decoding pipeline, which can be used to perform
scaling and color conversion.
The post-processor is integrated to the decoder, and it's
possible to use it in a way that is completely transparent
to the user.
This commit enables color conversion via post-processing,
which means the driver now exposes YUV packed, in addition to NV12.
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
drivers/staging/media/hantro/Makefile | 1 +
drivers/staging/media/hantro/hantro.h | 84 ++++++++++-
drivers/staging/media/hantro/hantro_drv.c | 90 ++++++++++-
.../staging/media/hantro/hantro_g1_h264_dec.c | 2 +-
.../media/hantro/hantro_g1_mpeg2_dec.c | 2 +-
drivers/staging/media/hantro/hantro_g1_regs.h | 51 +++++++
.../staging/media/hantro/hantro_g1_vp8_dec.c | 2 +-
drivers/staging/media/hantro/hantro_h264.c | 6 +-
drivers/staging/media/hantro/hantro_hw.h | 13 ++
.../staging/media/hantro/hantro_postproc.c | 141 ++++++++++++++++++
drivers/staging/media/hantro/hantro_v4l2.c | 47 +++++-
drivers/staging/media/hantro/rk3288_vpu_hw.c | 10 ++
12 files changed, 436 insertions(+), 13 deletions(-)
create mode 100644 drivers/staging/media/hantro/hantro_postproc.c
diff --git a/drivers/staging/media/hantro/Makefile b/drivers/staging/media/hantro/Makefile
index 5d6b0383d280..496b30c3c396 100644
--- a/drivers/staging/media/hantro/Makefile
+++ b/drivers/staging/media/hantro/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_VIDEO_HANTRO) += hantro-vpu.o
hantro-vpu-y += \
hantro_drv.o \
hantro_v4l2.o \
+ hantro_postproc.o \
hantro_h1_jpeg_enc.o \
hantro_g1_h264_dec.o \
hantro_g1_mpeg2_dec.o \
diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 15506b9a34f4..9b8b3a01b0cc 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -60,6 +60,8 @@ struct hantro_irq {
* @num_enc_fmts: Number of encoder formats.
* @dec_fmts: Decoder formats.
* @num_dec_fmts: Number of decoder formats.
+ * @postproc_fmts: Post-processor formats.
+ * @num_postproc_fmts: Number of post-processor formats.
* @codec: Supported codecs
* @codec_ops: Codec ops.
* @init: Initialize hardware.
@@ -70,6 +72,7 @@ struct hantro_irq {
* @num_clocks: number of clocks in the array
* @reg_names: array of register range names
* @num_regs: number of register range names in the array
+ * @postproc_regs: &struct hantro_postproc_regs pointer
*/
struct hantro_variant {
unsigned int enc_offset;
@@ -78,6 +81,8 @@ struct hantro_variant {
unsigned int num_enc_fmts;
const struct hantro_fmt *dec_fmts;
unsigned int num_dec_fmts;
+ const struct hantro_fmt *postproc_fmts;
+ unsigned int num_postproc_fmts;
unsigned int codec;
const struct hantro_codec_ops *codec_ops;
int (*init)(struct hantro_dev *vpu);
@@ -88,6 +93,7 @@ struct hantro_variant {
int num_clocks;
const char * const *reg_names;
int num_regs;
+ const struct hantro_postproc_regs *postproc_regs;
};
/**
@@ -127,6 +133,8 @@ struct hantro_ctrl {
* @vdev_pads: &struct media_pad with the @vdev pads.
* @sd_proc: &struct v4l2_subdev exposing the encoder or decoder sub-device
* @sd_proc_pads: &struct media_pad with the @sd_proc pads.
+ * @sd_postproc: &struct v4l2_subdev exposing the post-proc sub-device
+ * @sd_postproc_pads: &struct media_pad with the @sd_postproc pads.
*
* Contains everything needed to attach the video device to the media device.
*/
@@ -137,6 +145,9 @@ struct hantro_func {
struct v4l2_subdev sd_proc;
struct media_pad sd_proc_pads[2];
+
+ struct v4l2_subdev sd_postproc;
+ struct media_pad sd_postproc_pads[2];
};
static inline struct hantro_func *
@@ -145,6 +156,12 @@ hantro_vdev_to_func(struct video_device *vdev)
return container_of(vdev, struct hantro_func, vdev);
}
+enum hantro_decoder_mode {
+ HANTRO_NONE_MODE = 0x0,
+ HANTRO_DECODER_ONLY_MODE = 0x1,
+ HANTRO_POSTPROC_MODE = 0x2,
+};
+
/**
* struct hantro_dev - driver data
* @v4l2_dev: V4L2 device to register video devices for.
@@ -160,6 +177,7 @@ hantro_vdev_to_func(struct video_device *vdev)
* @enc_base: Mapped address of VPU encoder register for convenience.
* @dec_base: Mapped address of VPU decoder register for convenience.
* @ctrl_base: Mapped address of VPU control block.
+ * @dec_mode: Decoder mode, specifies if post-processor is pipelined.
* @vpu_mutex: Mutex to synchronize V4L2 calls.
* @irqlock: Spinlock to synchronize access to data structures
* shared with interrupt handlers.
@@ -180,6 +198,7 @@ struct hantro_dev {
void __iomem *dec_base;
void __iomem *ctrl_base;
+ enum hantro_decoder_mode dec_mode;
struct mutex vpu_mutex; /* video_device lock */
spinlock_t irqlock;
const struct hantro_variant *variant;
@@ -207,6 +226,7 @@ struct hantro_dev {
* context, and it's called right before
* calling v4l2_m2m_job_finish.
* @codec_ops: Set of operations related to codec mode.
+ * @postproc: Post-processing context.
* @jpeg_enc: JPEG-encoding context.
* @mpeg2_dec: MPEG-2-decoding context.
* @vp8_dec: VP8-decoding context.
@@ -232,6 +252,7 @@ struct hantro_ctx {
unsigned int bytesused);
const struct hantro_codec_ops *codec_ops;
+ struct hantro_postproc_ctx postproc;
/* Specific for particular codec modes. */
union {
@@ -269,6 +290,23 @@ struct hantro_reg {
u32 mask;
};
+struct hantro_postproc_regs {
+ struct hantro_reg pipeline_en;
+ struct hantro_reg max_burst;
+ struct hantro_reg clk_gate;
+ struct hantro_reg out_swap32;
+ struct hantro_reg out_endian;
+ struct hantro_reg out_luma_base;
+ struct hantro_reg input_width;
+ struct hantro_reg input_height;
+ struct hantro_reg output_width;
+ struct hantro_reg output_height;
+ struct hantro_reg input_fmt;
+ struct hantro_reg output_fmt;
+ struct hantro_reg orig_width;
+ struct hantro_reg display_width;
+};
+
/* Logging helpers */
/**
@@ -347,9 +385,23 @@ static inline u32 vdpu_read(struct hantro_dev *vpu, u32 reg)
return val;
}
-static inline void hantro_reg_write(struct hantro_dev *vpu,
- const struct hantro_reg *reg,
- u32 val)
+static inline void
+hantro_reg_write(struct hantro_dev *vpu,
+ const struct hantro_reg *reg,
+ u32 val)
+{
+ u32 v;
+
+ v = vdpu_read(vpu, reg->base);
+ v &= ~(reg->mask << reg->shift);
+ v |= ((val & reg->mask) << reg->shift);
+ vdpu_write(vpu, v, reg->base);
+}
+
+static inline void
+hantro_reg_write_relaxed(struct hantro_dev *vpu,
+ const struct hantro_reg *reg,
+ u32 val)
{
u32 v;
@@ -376,4 +428,30 @@ hantro_get_dst_buf(struct hantro_ctx *ctx)
return v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
}
+static inline bool
+hantro_postproc_enabled(const struct hantro_ctx *ctx)
+{
+ return ctx->dev->dec_mode == HANTRO_POSTPROC_MODE;
+}
+
+static inline bool
+hantro_needs_postproc(struct hantro_ctx *ctx)
+{
+ return hantro_postproc_enabled(ctx) &&
+ ctx->vpu_dst_fmt->fourcc != V4L2_PIX_FMT_NV12;
+}
+
+static inline dma_addr_t
+hantro_get_dec_buf_addr(struct hantro_ctx *ctx, struct vb2_buffer *vb)
+{
+ if (hantro_needs_postproc(ctx))
+ return ctx->postproc.dec_q[vb->index].dma;
+ return vb2_dma_contig_plane_dma_addr(vb, 0);
+}
+
+void hantro_postproc_disable(struct hantro_ctx *ctx);
+void hantro_postproc_setup(struct hantro_ctx *ctx);
+void hantro_postproc_free(struct hantro_ctx *ctx);
+int hantro_postproc_alloc(struct hantro_ctx *ctx);
+
#endif /* HANTRO_H_ */
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 35beb5a9bf52..e72915cd3d99 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -53,7 +53,7 @@ dma_addr_t hantro_get_ref(struct hantro_ctx *ctx, u64 ts)
if (index < 0)
return 0;
buf = vb2_get_buffer(q, index);
- return vb2_dma_contig_plane_dma_addr(buf, 0);
+ return hantro_get_dec_buf_addr(ctx, buf);
}
static int
@@ -159,12 +159,18 @@ void hantro_prepare_run(struct hantro_ctx *ctx)
src_buf = hantro_get_src_buf(ctx);
v4l2_ctrl_request_setup(src_buf->vb2_buf.req_obj.req,
&ctx->ctrl_handler);
+
+ if (!hantro_needs_postproc(ctx))
+ hantro_postproc_disable(ctx);
}
void hantro_finish_run(struct hantro_ctx *ctx)
{
struct vb2_v4l2_buffer *src_buf;
+ if (hantro_needs_postproc(ctx))
+ hantro_postproc_setup(ctx);
+
src_buf = hantro_get_src_buf(ctx);
v4l2_ctrl_request_complete(src_buf->vb2_buf.req_obj.req,
&ctx->ctrl_handler);
@@ -382,6 +388,7 @@ static int hantro_ctrls_setup(struct hantro_dev *vpu,
return ctx->ctrl_handler.error;
}
}
+
return v4l2_ctrl_handler_setup(&ctx->ctrl_handler);
}
@@ -492,7 +499,65 @@ static int link_setup(struct media_entity *entity,
const struct media_pad *local,
const struct media_pad *remote, u32 flags)
{
- /* empty for now */
+ struct v4l2_subdev *sd = media_entity_to_v4l2_subdev(entity);
+ struct hantro_dev *vpu = v4l2_get_subdevdata(sd);
+ bool enable = flags & MEDIA_LNK_FL_ENABLED;
+ unsigned int new_mode, mode_mask, mode_set;
+
+ /*
+ * This device topology is quite simple, and the non-immutable
+ * links are just two: decoder to v4l and post-processor to v4l.
+ *
+ * There are tree possible states:
+ * - [Decoder] -> [Post-processor] -> V4L
+ * - [Decoder] -> V4L
+ * - [] -> V4L
+ *
+ * In the first case, the post-processor is enabled in the data path
+ * and can produce other pixel formats. When the post-processor is
+ * used, more memory is in-use due to the extra buffers required.
+ *
+ * In the second case, only the decoder is enabled, so only the decoder
+ * native pixel format is available.
+ *
+ * The last case is when neither the decoder,
+ * nor the post-processor is connected to the V4L device,
+ * and enabled.
+ *
+ * Notes:
+ * - The post-processor can be used or not used depending
+ * on the pixel format. Therefore, enabling the link doesn't
+ * necesarily mean it's gonna be used.
+ *
+ * - The post-processor can be used or not on a per-request,
+ * per-context way. However, because the topology
+ * is described at the media controller level, the enable
+ * is done at a device level.
+ *
+ * - Topology is only used to expose the post-processor feature
+ * to the application (and allow enabling it). Topology changes
+ * while a context is streaming is allowed, and will be noticed
+ * by applications on the next format negotiation (S_FMT,TRY_FMT).
+ */
+ if (local->entity->function == MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER &&
+ remote->entity->function == MEDIA_ENT_F_IO_V4L) {
+ mode_mask = HANTRO_POSTPROC_MODE;
+ } else if (local->entity->function == MEDIA_ENT_F_PROC_VIDEO_DECODER &&
+ remote->entity->function == MEDIA_ENT_F_IO_V4L) {
+ mode_mask = HANTRO_DECODER_ONLY_MODE;
+ } else {
+ return -EINVAL;
+ }
+
+ mode_set = enable ? mode_mask : 0x0;
+ new_mode = vpu->dec_mode & ~mode_mask;
+ new_mode |= (mode_mask & mode_set);
+
+ /* Reject an illegal setup. */
+ if (new_mode == (HANTRO_DECODER_ONLY_MODE | HANTRO_POSTPROC_MODE))
+ return -EINVAL;
+
+ vpu->dec_mode = new_mode;
return 0;
}
@@ -629,14 +694,29 @@ static int hantro_attach_func(struct hantro_dev *vpu,
const struct hantro_subdev decoder_subdevs[] = {
HANTRO_SUBDEV(&func->sd_proc, "decoder", func->id,
func->sd_proc_pads),
+ HANTRO_SUBDEV(&func->sd_postproc, "postproc",
+ MEDIA_ENT_F_PROC_VIDEO_PIXEL_FORMATTER,
+ func->sd_postproc_pads),
};
const struct hantro_subdev encoder_subdevs[] = {
HANTRO_SUBDEV(&func->sd_proc, "encoder", func->id,
func->sd_proc_pads),
};
+ /*
+ * By default, the decoder is connected, but the post-proc
+ * is disconnected. The application is expected to connect
+ * the post-processor explicitly in order to use it.
+ *
+ * The encoder has no post-processor, so in this case the link
+ * is enabled and immutable.
+ */
const struct hantro_link decoder_links[] = {
/* decoder -> vdev */
- HANTRO_LINK_IM(&func->sd_proc.entity, &func->vdev.entity),
+ HANTRO_LINK_EN(&func->sd_proc.entity, &func->vdev.entity),
+ /* decoder -> post-proc */
+ HANTRO_LINK_IM(&func->sd_proc.entity, &func->sd_postproc.entity),
+ /* post-proc -> vdev */
+ HANTRO_LINK(&func->sd_postproc.entity, &func->vdev.entity),
/* vdev -> decoder */
HANTRO_LINK_IM(&func->vdev.entity, &func->sd_proc.entity),
};
@@ -712,6 +792,9 @@ static int hantro_attach_func(struct hantro_dev *vpu,
goto err_rm_devnode;
}
+ /* The default mode. */
+ vpu->dec_mode = HANTRO_DECODER_ONLY_MODE;
+
return 0;
err_rm_devnode:
media_devnode_remove(func->vdev.intf_devnode);
@@ -728,6 +811,7 @@ static void hantro_detach_func(struct hantro_func *func)
media_devnode_remove(func->vdev.intf_devnode);
media_device_unregister_entity(&func->vdev.entity);
v4l2_device_unregister_subdev(&func->sd_proc);
+ v4l2_device_unregister_subdev(&func->sd_postproc);
}
static int hantro_add_func(struct hantro_dev *vpu, unsigned int funcid)
diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
index 29130946dea4..e263a6b50651 100644
--- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
@@ -241,7 +241,7 @@ static void set_buffers(struct hantro_ctx *ctx)
vdpu_write_relaxed(vpu, src_dma, G1_REG_ADDR_STR);
/* Destination (decoded frame) buffer. */
- dst_dma = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
+ dst_dma = hantro_get_dec_buf_addr(ctx, &dst_buf->vb2_buf);
vdpu_write_relaxed(vpu, dst_dma, G1_REG_ADDR_DST);
/* Higher profiles require DMV buffer appended to reference frames. */
diff --git a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
index 663bf05459a9..6fdced7d7f4b 100644
--- a/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_mpeg2_dec.c
@@ -65,7 +65,7 @@ hantro_g1_mpeg2_dec_set_buffers(struct hantro_dev *vpu, struct hantro_ctx *ctx,
vdpu_write_relaxed(vpu, addr, G1_REG_ADDR_STR);
/* Destination frame buffer */
- addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0);
+ addr = hantro_get_dec_buf_addr(ctx, dst_buf);
current_addr = addr;
if (picture->picture_structure == PICT_BOTTOM_FIELD)
diff --git a/drivers/staging/media/hantro/hantro_g1_regs.h b/drivers/staging/media/hantro/hantro_g1_regs.h
index 84b93915a953..68f2a53a3680 100644
--- a/drivers/staging/media/hantro/hantro_g1_regs.h
+++ b/drivers/staging/media/hantro/hantro_g1_regs.h
@@ -304,4 +304,55 @@
#define G1_REG_REF_BUF_CTRL2_APF_THRESHOLD(x) (((x) & 0x3fff) << 0)
#define G1_REG_SOFT_RESET G1_SWREG(101)
+/* Post-processor registers. */
+#define G1_REG_PP_INTERRUPT G1_SWREG(60)
+#define G1_REG_PP_READY_IRQ BIT(12)
+#define G1_REG_PP_IRQ BIT(8)
+#define G1_REG_PP_IRQ_DIS BIT(4)
+#define G1_REG_PP_PIPELINE_EN BIT(1)
+#define G1_REG_PP_EXTERNAL_TRIGGER BIT(0)
+#define G1_REG_PP_DEV_CONFIG G1_SWREG(61)
+#define G1_REG_PP_AXI_RD_ID(v) (((v) << 24) & GENMASK(31, 24))
+#define G1_REG_PP_AXI_WR_ID(v) (((v) << 16) & GENMASK(23, 16))
+#define G1_REG_PP_INSWAP32_E(v) ((v) ? BIT(10) : 0)
+#define G1_REG_PP_DATA_DISC_E(v) ((v) ? BIT(9) : 0)
+#define G1_REG_PP_CLK_GATE_E(v) ((v) ? BIT(8) : 0)
+#define G1_REG_PP_IN_ENDIAN(v) ((v) ? BIT(7) : 0)
+#define G1_REG_PP_OUT_ENDIAN(v) ((v) ? BIT(6) : 0)
+#define G1_REG_PP_OUTSWAP32_E(v) ((v) ? BIT(5) : 0)
+#define G1_REG_PP_MAX_BURST(v) (((v) << 0) & GENMASK(4, 0))
+#define G1_REG_PP_IN_LUMA_BASE G1_SWREG(63)
+#define G1_REG_PP_IN_CB_BASE G1_SWREG(64)
+#define G1_REG_PP_IN_CR_BASE G1_SWREG(65)
+#define G1_REG_PP_OUT_LUMA_BASE G1_SWREG(66)
+#define G1_REG_PP_OUT_CHROMA_BASE G1_SWREG(67)
+#define G1_REG_PP_CONTRAST_ADJUST G1_SWREG(68)
+#define G1_REG_PP_COLOR_CONVERSION G1_SWREG(69)
+#define G1_REG_PP_COLOR_CONVERSION0 G1_SWREG(70)
+#define G1_REG_PP_COLOR_CONVERSION1 G1_SWREG(71)
+#define G1_REG_PP_INPUT_SIZE G1_SWREG(72)
+#define G1_REG_PP_INPUT_SIZE_HEIGHT(v) (((v) << 9) & GENMASK(16, 9))
+#define G1_REG_PP_INPUT_SIZE_WIDTH(v) (((v) << 0) & GENMASK(8, 0))
+#define G1_REG_PP_SCALING0 G1_SWREG(79)
+#define G1_REG_PP_PADD_R(v) (((v) << 23) & GENMASK(27, 23))
+#define G1_REG_PP_PADD_G(v) (((v) << 18) & GENMASK(22, 18))
+#define G1_REG_PP_RANGEMAP_Y(v) ((v) ? BIT(31) : 0)
+#define G1_REG_PP_RANGEMAP_C(v) ((v) ? BIT(30) : 0)
+#define G1_REG_PP_YCBCR_RANGE(v) ((v) ? BIT(29) : 0)
+#define G1_REG_PP_RGB_16(v) ((v) ? BIT(28) : 0)
+#define G1_REG_PP_SCALING1 G1_SWREG(80)
+#define G1_REG_PP_PADD_B(v) (((v) << 18) & GENMASK(22, 18))
+#define G1_REG_PP_MASK_R G1_SWREG(82)
+#define G1_REG_PP_MASK_G G1_SWREG(83)
+#define G1_REG_PP_MASK_B G1_SWREG(84)
+#define G1_REG_PP_CONTROL G1_SWREG(85)
+#define G1_REG_PP_CONTROL_IN_FMT(v) (((v) << 29) & GENMASK(31, 29))
+#define G1_REG_PP_CONTROL_OUT_FMT(v) (((v) << 26) & GENMASK(28, 26))
+#define G1_REG_PP_CONTROL_OUT_HEIGHT(v) (((v) << 15) & GENMASK(25, 15))
+#define G1_REG_PP_CONTROL_OUT_WIDTH(v) (((v) << 4) & GENMASK(14, 4))
+#define G1_REG_PP_MASK1_ORIG_WIDTH G1_SWREG(88)
+#define G1_REG_PP_ORIG_WIDTH(v) (((v) << 23) & GENMASK(31, 23))
+#define G1_REG_PP_DISPLAY_WIDTH G1_SWREG(92)
+#define G1_REG_PP_FUSE G1_SWREG(99)
+
#endif /* HANTRO_G1_REGS_H_ */
diff --git a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
index cad18094fee0..e708994d1aba 100644
--- a/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_vp8_dec.c
@@ -422,7 +422,7 @@ static void cfg_buffers(struct hantro_ctx *ctx,
}
vdpu_write_relaxed(vpu, reg, G1_REG_FWD_PIC(0));
- dst_dma = vb2_dma_contig_plane_dma_addr(&vb2_dst->vb2_buf, 0);
+ dst_dma = hantro_get_dec_buf_addr(ctx, &vb2_dst->vb2_buf);
vdpu_write_relaxed(vpu, dst_dma, G1_REG_ADDR_DST);
}
diff --git a/drivers/staging/media/hantro/hantro_h264.c b/drivers/staging/media/hantro/hantro_h264.c
index 02cbe7761769..ab6f9d04dcc1 100644
--- a/drivers/staging/media/hantro/hantro_h264.c
+++ b/drivers/staging/media/hantro/hantro_h264.c
@@ -656,7 +656,11 @@ int hantro_h264_dec_init(struct hantro_ctx *ctx)
tbl = priv->cpu;
memcpy(tbl->cabac_table, h264_cabac_table, sizeof(tbl->cabac_table));
- v4l2_fill_pixfmt_mp(&pix_mp, ctx->dst_fmt.pixelformat,
+ /*
+ * For the decoder picture size, we want the decoder
+ * native pixel format.
+ */
+ v4l2_fill_pixfmt_mp(&pix_mp, V4L2_PIX_FMT_NV12,
ctx->dst_fmt.width, ctx->dst_fmt.height);
h264_dec->pic_size = pix_mp.plane_fmt[0].sizeimage;
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index 69b88f4d3fb3..18e7d9e1f469 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -28,11 +28,13 @@ struct hantro_variant;
* @cpu: CPU pointer to the buffer.
* @dma: DMA address of the buffer.
* @size: Size of the buffer.
+ * @attrs: Attributes of the DMA mapping.
*/
struct hantro_aux_buf {
void *cpu;
dma_addr_t dma;
size_t size;
+ unsigned long attrs;
};
/**
@@ -109,6 +111,15 @@ struct hantro_vp8_dec_hw_ctx {
struct hantro_aux_buf prob_tbl;
};
+/**
+ * struct hantro_postproc_ctx
+ *
+ * @dec_q: References buffers, in decoder format.
+ */
+struct hantro_postproc_ctx {
+ struct hantro_aux_buf dec_q[VB2_MAX_FRAME];
+};
+
/**
* struct hantro_codec_ops - codec mode specific operations
*
@@ -144,6 +155,8 @@ extern const struct hantro_variant rk3399_vpu_variant;
extern const struct hantro_variant rk3328_vpu_variant;
extern const struct hantro_variant rk3288_vpu_variant;
+extern const struct hantro_postproc_regs hantro_g1_postproc_regs;
+
extern const u32 hantro_vp8_dec_mc_filter[8][6];
void hantro_watchdog(struct work_struct *work);
diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
new file mode 100644
index 000000000000..e95d38a20963
--- /dev/null
+++ b/drivers/staging/media/hantro/hantro_postproc.c
@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hantro G1 post-processor support
+ *
+ * Copyright (C) 2019 Collabora, Ltd.
+ */
+
+#include <linux/dma-mapping.h>
+#include <linux/types.h>
+
+#include "hantro.h"
+#include "hantro_hw.h"
+#include "hantro_g1_regs.h"
+
+#define HANTRO_PP_REG_WRITE(vpu, reg_name, val) \
+ do { \
+ if ((vpu)->variant->postproc_regs->reg_name.base) \
+ hantro_reg_write_relaxed(vpu, \
+ &vpu->variant->postproc_regs->reg_name, \
+ (val)); \
+ } while (0)
+
+#define HANTRO_PP_REG_WRITE_S(vpu, reg_name, val) \
+ do { \
+ if ((vpu)->variant->postproc_regs->reg_name.base) \
+ hantro_reg_write(vpu, \
+ &vpu->variant->postproc_regs->reg_name, \
+ (val)); \
+ } while (0)
+
+#define VPU_PP_IN_YUYV 0x0
+#define VPU_PP_IN_NV12 0x1
+#define VPU_PP_IN_YUV420 0x2
+#define VPU_PP_IN_YUV240_TILED 0x5
+#define VPU_PP_OUT_RGB 0x0
+#define VPU_PP_OUT_YUYV 0x3
+
+const struct hantro_postproc_regs hantro_g1_postproc_regs = {
+ .pipeline_en = {G1_REG_PP_INTERRUPT, 1, 0x1},
+ .max_burst = {G1_REG_PP_DEV_CONFIG, 0, 0x1f},
+ .clk_gate = {G1_REG_PP_DEV_CONFIG, 1, 0x1},
+ .out_swap32 = {G1_REG_PP_DEV_CONFIG, 5, 0x1},
+ .out_endian = {G1_REG_PP_DEV_CONFIG, 6, 0x1},
+ .out_luma_base = {G1_REG_PP_OUT_LUMA_BASE, 0, 0xffffffff},
+ .input_width = {G1_REG_PP_INPUT_SIZE, 0, 0x1ff},
+ .input_height = {G1_REG_PP_INPUT_SIZE, 9, 0x1ff},
+ .output_width = {G1_REG_PP_CONTROL, 4, 0x7ff},
+ .output_height = {G1_REG_PP_CONTROL, 15, 0x7ff},
+ .input_fmt = {G1_REG_PP_CONTROL, 29, 0x7},
+ .output_fmt = {G1_REG_PP_CONTROL, 26, 0x7},
+ .orig_width = {G1_REG_PP_MASK1_ORIG_WIDTH, 23, 0x1ff},
+ .display_width = {G1_REG_PP_DISPLAY_WIDTH, 0, 0xfff},
+};
+
+void hantro_postproc_setup(struct hantro_ctx *ctx)
+{
+ struct hantro_dev *vpu = ctx->dev;
+ struct vb2_v4l2_buffer *dst_buf;
+ u32 src_pp_fmt, dst_pp_fmt;
+ dma_addr_t dst_dma;
+
+ /* Turn on pipeline mode. Must be done first. */
+ HANTRO_PP_REG_WRITE_S(vpu, pipeline_en, 0x1);
+
+ src_pp_fmt = VPU_PP_IN_NV12;
+
+ switch (ctx->vpu_dst_fmt->fourcc) {
+ case V4L2_PIX_FMT_YUYV:
+ dst_pp_fmt = VPU_PP_OUT_YUYV;
+ break;
+ default:
+ WARN(1, "output format %d not supported by the post-processor, this wasn't expected.",
+ ctx->vpu_dst_fmt->fourcc);
+ dst_pp_fmt = 0;
+ break;
+ }
+
+ dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
+ dst_dma = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
+
+ HANTRO_PP_REG_WRITE(vpu, clk_gate, 0x1);
+ HANTRO_PP_REG_WRITE(vpu, out_endian, 0x1);
+ HANTRO_PP_REG_WRITE(vpu, out_swap32, 0x1);
+ HANTRO_PP_REG_WRITE(vpu, max_burst, 16);
+ HANTRO_PP_REG_WRITE(vpu, out_luma_base, dst_dma);
+ HANTRO_PP_REG_WRITE(vpu, input_width, MB_WIDTH(ctx->dst_fmt.width));
+ HANTRO_PP_REG_WRITE(vpu, input_height, MB_HEIGHT(ctx->dst_fmt.height));
+ HANTRO_PP_REG_WRITE(vpu, input_fmt, src_pp_fmt);
+ HANTRO_PP_REG_WRITE(vpu, output_fmt, dst_pp_fmt);
+ HANTRO_PP_REG_WRITE(vpu, output_width, ctx->dst_fmt.width);
+ HANTRO_PP_REG_WRITE(vpu, output_height, ctx->dst_fmt.height);
+ HANTRO_PP_REG_WRITE(vpu, orig_width, MB_WIDTH(ctx->dst_fmt.width));
+ HANTRO_PP_REG_WRITE(vpu, display_width, ctx->dst_fmt.width);
+}
+
+void hantro_postproc_free(struct hantro_ctx *ctx)
+{
+ struct hantro_dev *vpu = ctx->dev;
+ unsigned int i;
+
+ for (i = 0; i < VB2_MAX_FRAME; ++i) {
+ struct hantro_aux_buf *priv = &ctx->postproc.dec_q[i];
+
+ if (priv->cpu) {
+ dma_free_attrs(vpu->dev, priv->size, priv->cpu,
+ priv->dma, priv->attrs);
+ priv->cpu = NULL;
+ }
+ }
+}
+
+int hantro_postproc_alloc(struct hantro_ctx *ctx)
+{
+ struct hantro_dev *vpu = ctx->dev;
+ unsigned int i, buf_size;
+
+ buf_size = ctx->dst_fmt.plane_fmt[0].sizeimage;
+
+ for (i = 0; i < VB2_MAX_FRAME; ++i) {
+ struct hantro_aux_buf *priv = &ctx->postproc.dec_q[i];
+
+ /*
+ * The buffers on this queue are meant as intermediate
+ * buffers for the decoder, so no mapping is needed.
+ */
+ priv->attrs = DMA_ATTR_NO_KERNEL_MAPPING;
+ priv->cpu = dma_alloc_attrs(vpu->dev, buf_size, &priv->dma,
+ GFP_KERNEL, priv->attrs);
+ if (!priv->cpu)
+ return -ENOMEM;
+ priv->size = buf_size;
+ }
+ return 0;
+}
+
+void hantro_postproc_disable(struct hantro_ctx *ctx)
+{
+ struct hantro_dev *vpu = ctx->dev;
+
+ HANTRO_PP_REG_WRITE_S(vpu, pipeline_en, 0x0);
+}
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index 58fa4b52275b..ddd180278c8b 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -46,6 +46,19 @@ hantro_get_formats(const struct hantro_ctx *ctx, unsigned int *num_fmts)
return formats;
}
+static const struct hantro_fmt *
+hantro_get_postproc_formats(const struct hantro_ctx *ctx,
+ unsigned int *num_fmts)
+{
+ if (!hantro_postproc_enabled(ctx) || hantro_is_encoder_ctx(ctx)) {
+ *num_fmts = 0;
+ return NULL;
+ }
+
+ *num_fmts = ctx->dev->variant->num_postproc_fmts;
+ return ctx->dev->variant->postproc_fmts;
+}
+
static const struct hantro_fmt *
hantro_find_format(const struct hantro_ctx *ctx, u32 fourcc)
{
@@ -57,6 +70,10 @@ hantro_find_format(const struct hantro_ctx *ctx, u32 fourcc)
if (formats[i].fourcc == fourcc)
return &formats[i];
+ formats = hantro_get_postproc_formats(ctx, &num_fmts);
+ for (i = 0; i < num_fmts; i++)
+ if (formats[i].fourcc == fourcc)
+ return &formats[i];
return NULL;
}
@@ -151,6 +168,20 @@ static int vidioc_enum_fmt(struct file *file, void *priv,
}
++j;
}
+
+ /* Enumerate post-processed formats. */
+ if (!capture)
+ return -EINVAL;
+ formats = hantro_get_postproc_formats(ctx, &num_fmts);
+ for (i = 0; i < num_fmts; i++) {
+ if (j == f->index) {
+ fmt = &formats[i];
+ f->pixelformat = fmt->fourcc;
+ return 0;
+ }
+ ++j;
+ }
+
return -EINVAL;
}
@@ -241,9 +272,10 @@ static int vidioc_try_fmt(struct file *file, void *priv, struct v4l2_format *f,
/*
* The H264 decoder needs extra space on the output buffers
* to store motion vectors. This is needed for reference
- * frames.
+ * frames and only if the format is non-post-processed (NV12).
*/
- if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_H264_SLICE)
+ if (ctx->vpu_src_fmt->fourcc == V4L2_PIX_FMT_H264_SLICE &&
+ fmt->fourcc == V4L2_PIX_FMT_NV12)
pix_mp->plane_fmt[0].sizeimage +=
128 * DIV_ROUND_UP(pix_mp->width, 16) *
DIV_ROUND_UP(pix_mp->height, 16);
@@ -622,10 +654,18 @@ static int hantro_start_streaming(struct vb2_queue *q, unsigned int count)
if (ret)
goto err_pipe_stop;
}
- }
+ if (hantro_needs_postproc(ctx)) {
+ ret = hantro_postproc_alloc(ctx);
+ if (ret)
+ goto err_codec_exit;
+ }
+ }
return ret;
+err_codec_exit:
+ if (ctx->codec_ops && ctx->codec_ops->exit)
+ ctx->codec_ops->exit(ctx);
err_pipe_stop:
media_pipeline_stop(entity);
return ret;
@@ -655,6 +695,7 @@ static void hantro_stop_streaming(struct vb2_queue *q)
struct media_entity *entity = &ctx->fh.vdev->entity;
if (hantro_vq_is_coded(q)) {
+ hantro_postproc_free(ctx);
if (ctx->codec_ops && ctx->codec_ops->exit)
ctx->codec_ops->exit(ctx);
}
diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
index c0bdd6c02520..7d65fb75ac94 100644
--- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
+++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
@@ -56,6 +56,13 @@ static const struct hantro_fmt rk3288_vpu_enc_fmts[] = {
},
};
+static const struct hantro_fmt rk3288_vpu_postproc_fmts[] = {
+ {
+ .fourcc = V4L2_PIX_FMT_YUYV,
+ .codec_mode = HANTRO_MODE_NONE,
+ },
+};
+
static const struct hantro_fmt rk3288_vpu_dec_fmts[] = {
{
.fourcc = V4L2_PIX_FMT_NV12,
@@ -215,6 +222,9 @@ const struct hantro_variant rk3288_vpu_variant = {
.dec_offset = 0x400,
.dec_fmts = rk3288_vpu_dec_fmts,
.num_dec_fmts = ARRAY_SIZE(rk3288_vpu_dec_fmts),
+ .postproc_fmts = rk3288_vpu_postproc_fmts,
+ .num_postproc_fmts = ARRAY_SIZE(rk3288_vpu_postproc_fmts),
+ .postproc_regs = &hantro_g1_postproc_regs,
.codec = HANTRO_JPEG_ENCODER | HANTRO_MPEG2_DECODER |
HANTRO_VP8_DECODER | HANTRO_H264_DECODER,
.codec_ops = rk3288_vpu_codec_ops,
--
2.22.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 0/4] Enable Hantro G1 post-processor
2019-10-03 19:08 [PATCH v2 0/4] Enable Hantro G1 post-processor Ezequiel Garcia
` (3 preceding siblings ...)
2019-10-03 19:08 ` [PATCH v2 4/4] media: hantro: Support color conversion via post-processing Ezequiel Garcia
@ 2019-10-04 6:07 ` Tomasz Figa
2019-11-09 10:40 ` Hans Verkuil
4 siblings, 1 reply; 10+ messages in thread
From: Tomasz Figa @ 2019-10-04 6:07 UTC (permalink / raw)
To: Ezequiel Garcia
Cc: Linux Media Mailing List, kernel, open list:ARM/Rockchip SoC...,
Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
Chris Healy, Linux Kernel Mailing List
Hi Ezequiel,
On Fri, Oct 4, 2019 at 4:09 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>
> Hi all,
>
> The Hantro G1 VPU post-processor block can be pipelined with
> the decoder hardware, allowing to perform operations such as
> color conversion, scaling, rotation, cropping, among others.
>
> When the post-processor is enabled, the decoder hardware
> gets its own set of NV12 buffers (the native decoder format),
> and the post-processor is the owner of the CAPTURE buffers.
>
> This allows the application get processed
> (scaled, converted, etc) buffers, completely transparently.
>
> This feature is implemented by exposing the post-processed pixel
> formats on ENUM_FMT. When the application sets a pixel format
> other than NV12, and if the post-processor is MC-linked,
> the driver will make use of it, transparently.
>
> On this v2, the media controller API is now required
> to "enable" (aka link, in topology jargon) the post-processor.
> Therefore, applications now have to explicitly request this feature.
>
> For instance, the post-processor is enabled using the media-ctl
> tool:
>
> media-ctl -l "'decoder':1 -> 'rockchip,rk3288-vpu-dec':0[0]"
> media-ctl -l "'postproc':1 -> 'rockchip,rk3288-vpu-dec':0[1]"
>
> v4l2-ctl -d 1 --list-formats
> ioctl: VIDIOC_ENUM_FMT
> Type: Video Capture Multiplanar
>
> [0]: 'NV12' (Y/CbCr 4:2:0)
> [1]: 'YUYV' (YUYV 4:2:2)
>
> Patches 1 and 2 are simply cleanups needed to easier integrate the
> post-processor. Patch 2 is a bit invasive, but it's a step
> forward towards merging the implementation for RK3399 and RK3288.
>
> Patch 3 re-works the media-topology, making the decoder
> a v4l2_subdevice, instead of a bare entity. This allows
> to introduce a more accurate of the decoder+post-processor complex.
>
> Patch 4 introduces the post-processing support.
>
> This is tested on RK3288 platforms with MPEG-2, VP8 and
> H264 streams, decoding to YUY2 surfaces. For now, this series
> is only adding support for NV12-to-YUY2 conversion.
>
> The design and implementation is quite different from v1:
>
> * The decoder->post-processor topology is now exposed
> explicitly and applications need to configure the pipeline.
> By default, the decoder is enabled and the post-processor
> is disabled.
>
First of all, thanks for working on this. While from Chromium point of
view there isn't any practical use case for this feature, it could
possibly be valuable for some other platforms.
I still see a problem with the current design. Mem-to-mem decoders are
commonly used in a multi-instance fashion, but media topology is
global. That means that when having two applications on the system
decoding their own videos, one might be stepping on the other by
changing the topology.
I believe that if we want this to be really usable, we would need to
make the media topology per instance, but that's a significant change
to the media subsystem. Maybe we could pursue the other options I
suggested in the previous revision instead, like ordering the formats
returned by enum_fmt by native first and adding format flags that
would tell the userspace that the format can have some performance
and/or power penalty?
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 0/4] Enable Hantro G1 post-processor
2019-10-04 6:07 ` [PATCH v2 0/4] Enable Hantro G1 post-processor Tomasz Figa
@ 2019-11-09 10:40 ` Hans Verkuil
2019-11-09 16:02 ` Ezequiel Garcia
0 siblings, 1 reply; 10+ messages in thread
From: Hans Verkuil @ 2019-11-09 10:40 UTC (permalink / raw)
To: Tomasz Figa, Ezequiel Garcia
Cc: Linux Media Mailing List, kernel, open list:ARM/Rockchip SoC...,
Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
Chris Healy, Linux Kernel Mailing List
On 10/4/19 8:07 AM, Tomasz Figa wrote:
> Hi Ezequiel,
>
> On Fri, Oct 4, 2019 at 4:09 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
>>
>> Hi all,
>>
>> The Hantro G1 VPU post-processor block can be pipelined with
>> the decoder hardware, allowing to perform operations such as
>> color conversion, scaling, rotation, cropping, among others.
>>
>> When the post-processor is enabled, the decoder hardware
>> gets its own set of NV12 buffers (the native decoder format),
>> and the post-processor is the owner of the CAPTURE buffers.
>>
>> This allows the application get processed
>> (scaled, converted, etc) buffers, completely transparently.
>>
>> This feature is implemented by exposing the post-processed pixel
>> formats on ENUM_FMT. When the application sets a pixel format
>> other than NV12, and if the post-processor is MC-linked,
>> the driver will make use of it, transparently.
>>
>> On this v2, the media controller API is now required
>> to "enable" (aka link, in topology jargon) the post-processor.
>> Therefore, applications now have to explicitly request this feature.
>>
>> For instance, the post-processor is enabled using the media-ctl
>> tool:
>>
>> media-ctl -l "'decoder':1 -> 'rockchip,rk3288-vpu-dec':0[0]"
>> media-ctl -l "'postproc':1 -> 'rockchip,rk3288-vpu-dec':0[1]"
>>
>> v4l2-ctl -d 1 --list-formats
>> ioctl: VIDIOC_ENUM_FMT
>> Type: Video Capture Multiplanar
>>
>> [0]: 'NV12' (Y/CbCr 4:2:0)
>> [1]: 'YUYV' (YUYV 4:2:2)
>>
>> Patches 1 and 2 are simply cleanups needed to easier integrate the
>> post-processor. Patch 2 is a bit invasive, but it's a step
>> forward towards merging the implementation for RK3399 and RK3288.
>>
>> Patch 3 re-works the media-topology, making the decoder
>> a v4l2_subdevice, instead of a bare entity. This allows
>> to introduce a more accurate of the decoder+post-processor complex.
>>
>> Patch 4 introduces the post-processing support.
>>
>> This is tested on RK3288 platforms with MPEG-2, VP8 and
>> H264 streams, decoding to YUY2 surfaces. For now, this series
>> is only adding support for NV12-to-YUY2 conversion.
>>
>> The design and implementation is quite different from v1:
>>
>> * The decoder->post-processor topology is now exposed
>> explicitly and applications need to configure the pipeline.
>> By default, the decoder is enabled and the post-processor
>> is disabled.
>>
>
> First of all, thanks for working on this. While from Chromium point of
> view there isn't any practical use case for this feature, it could
> possibly be valuable for some other platforms.
>
> I still see a problem with the current design. Mem-to-mem decoders are
> commonly used in a multi-instance fashion, but media topology is
> global. That means that when having two applications on the system
> decoding their own videos, one might be stepping on the other by
> changing the topology.
>
> I believe that if we want this to be really usable, we would need to
> make the media topology per instance, but that's a significant change
> to the media subsystem. Maybe we could pursue the other options I
> suggested in the previous revision instead, like ordering the formats
> returned by enum_fmt by native first and adding format flags that
> would tell the userspace that the format can have some performance
> and/or power penalty?
I discussed this with Ezequiel in Lyon, and my preference is also to
not rely on the media controller, but instead order the formats with
the native formats first, followed by the formats that need this additional
processing step. A format flag can be added, but I feel that it is better
to wait with that until it is clear that there is a real need for it.
It would be good to document in the ENUM_FMT ioctl that formats that
require additional processing are at the end of the format list.
Ezequiel, I'm marking this series as "Changes Requested" in patchwork.
Regards,
Hans
>
> Best regards,
> Tomasz
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/4] Enable Hantro G1 post-processor
2019-11-09 10:40 ` Hans Verkuil
@ 2019-11-09 16:02 ` Ezequiel Garcia
0 siblings, 0 replies; 10+ messages in thread
From: Ezequiel Garcia @ 2019-11-09 16:02 UTC (permalink / raw)
To: Hans Verkuil, Tomasz Figa
Cc: Linux Media Mailing List, kernel, open list:ARM/Rockchip SoC...,
Heiko Stuebner, Jonas Karlman, Philipp Zabel, Boris Brezillon,
Chris Healy, Linux Kernel Mailing List
On Sat, 2019-11-09 at 11:40 +0100, Hans Verkuil wrote:
> On 10/4/19 8:07 AM, Tomasz Figa wrote:
> > Hi Ezequiel,
> >
> > On Fri, Oct 4, 2019 at 4:09 AM Ezequiel Garcia <ezequiel@collabora.com> wrote:
> > > Hi all,
> > >
> > > The Hantro G1 VPU post-processor block can be pipelined with
> > > the decoder hardware, allowing to perform operations such as
> > > color conversion, scaling, rotation, cropping, among others.
> > >
> > > When the post-processor is enabled, the decoder hardware
> > > gets its own set of NV12 buffers (the native decoder format),
> > > and the post-processor is the owner of the CAPTURE buffers.
> > >
> > > This allows the application get processed
> > > (scaled, converted, etc) buffers, completely transparently.
> > >
> > > This feature is implemented by exposing the post-processed pixel
> > > formats on ENUM_FMT. When the application sets a pixel format
> > > other than NV12, and if the post-processor is MC-linked,
> > > the driver will make use of it, transparently.
> > >
> > > On this v2, the media controller API is now required
> > > to "enable" (aka link, in topology jargon) the post-processor.
> > > Therefore, applications now have to explicitly request this feature.
> > >
> > > For instance, the post-processor is enabled using the media-ctl
> > > tool:
> > >
> > > media-ctl -l "'decoder':1 -> 'rockchip,rk3288-vpu-dec':0[0]"
> > > media-ctl -l "'postproc':1 -> 'rockchip,rk3288-vpu-dec':0[1]"
> > >
> > > v4l2-ctl -d 1 --list-formats
> > > ioctl: VIDIOC_ENUM_FMT
> > > Type: Video Capture Multiplanar
> > >
> > > [0]: 'NV12' (Y/CbCr 4:2:0)
> > > [1]: 'YUYV' (YUYV 4:2:2)
> > >
> > > Patches 1 and 2 are simply cleanups needed to easier integrate the
> > > post-processor. Patch 2 is a bit invasive, but it's a step
> > > forward towards merging the implementation for RK3399 and RK3288.
> > >
> > > Patch 3 re-works the media-topology, making the decoder
> > > a v4l2_subdevice, instead of a bare entity. This allows
> > > to introduce a more accurate of the decoder+post-processor complex.
> > >
> > > Patch 4 introduces the post-processing support.
> > >
> > > This is tested on RK3288 platforms with MPEG-2, VP8 and
> > > H264 streams, decoding to YUY2 surfaces. For now, this series
> > > is only adding support for NV12-to-YUY2 conversion.
> > >
> > > The design and implementation is quite different from v1:
> > >
> > > * The decoder->post-processor topology is now exposed
> > > explicitly and applications need to configure the pipeline.
> > > By default, the decoder is enabled and the post-processor
> > > is disabled.
> > >
> >
> > First of all, thanks for working on this. While from Chromium point of
> > view there isn't any practical use case for this feature, it could
> > possibly be valuable for some other platforms.
> >
> > I still see a problem with the current design. Mem-to-mem decoders are
> > commonly used in a multi-instance fashion, but media topology is
> > global. That means that when having two applications on the system
> > decoding their own videos, one might be stepping on the other by
> > changing the topology.
> >
> > I believe that if we want this to be really usable, we would need to
> > make the media topology per instance, but that's a significant change
> > to the media subsystem. Maybe we could pursue the other options I
> > suggested in the previous revision instead, like ordering the formats
> > returned by enum_fmt by native first and adding format flags that
> > would tell the userspace that the format can have some performance
> > and/or power penalty?
>
> I discussed this with Ezequiel in Lyon, and my preference is also to
> not rely on the media controller, but instead order the formats with
> the native formats first, followed by the formats that need this additional
> processing step. A format flag can be added, but I feel that it is better
> to wait with that until it is clear that there is a real need for it.
>
> It would be good to document in the ENUM_FMT ioctl that formats that
> require additional processing are at the end of the format list.
>
> Ezequiel, I'm marking this series as "Changes Requested" in patchwork.
>
Thanks! I should revisit this and post a new series soon.
Ezequiel
^ permalink raw reply [flat|nested] 10+ messages in thread