[PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[Adrian Ratiu]

Dear all,

This series introduces a regmap infrastructure for the Hantro driver
which is used to compensate for different HW-revision register layouts.
To justify it h264 decoding capability is added for newer VC8000 chips.

This is a gradual conversion to the new infra - a complete conversion
would have been very big and I do not have all the HW yet to test (I'm
expecting a RK3399 shipment next week though ;). I think converting the
h264 decoder provides a nice blueprint for how the other codecs can be
converted and enabled for different HW revisions.

The end goal of this is to make the driver more generic and eliminate
entirely custom boilerplate like `struct hantro_reg` or headers with
core-specific bit manipulations like `hantro_g1_regs.h` and instead rely
on the well-tested albeit more verbose regmap subsytem.

To give just two examples of bugs which are easily discovered by using
more verbose regmap fields (very easy to compare with the datasheets)
instead of relying on bit-magic tricks: G1_REG_DEC_CTRL3_INIT_QP(x) was
off-by-1 and the wrong .clk_gate bit was set in hantro_postproc.c.

Anyway, this series also extends the MMIO regmap API to allow relaxed
writes for the theoretical reason that avoiding unnecessary membarriers
leads to less CPU usage and small improvements to battery life. However,
in practice I could not measure differences between relaxed/non-relaxed
IO, so I'm on the fence whether to keep or remove the relaxed calls.

What I could masure is the performance impact of adding more sub-reg
field acesses: a constant ~ 20 microsecond bump per G1 h264 frame. This
is acceptable considering the total time to decode a frame takes three
orders of magnitude longer, i.e. miliseconds ranges, depending on the
frame size and bitstream params, so it is an acceptable trade-off to
have a more generic driver.

This has been tested on next-20201009 with imx8mq for G1 and an SoC with
VC8000 which has not yet been added (hopefuly support lands soon).

Kind regards,
Adrian

Adrian Ratiu (18):
  media: hantro: document all int reg bits up to vc8000
  media: hantro: make consistent use of decimal register notation
  media: hantro: make G1_REG_SOFT_RESET Rockchip specific
  media: hantro: add reset controller support
  media: hantro: prepare clocks before variant inits are run
  media: hantro: imx8mq: simplify ctrlblk reset logic
  regmap: mmio: add config option to allow relaxed MMIO accesses
  media: hantro: add initial MMIO regmap infrastructure
  media: hantro: default regmap to relaxed MMIO
  media: hantro: convert G1 h264 decoder to regmap fields
  media: hantro: convert G1 postproc to regmap
  media: hantro: add VC8000D h264 decoding
  media: hantro: add VC8000D postproc support
  media: hantro: make PP enablement logic a bit smarter
  media: hantro: add user-selectable, platform-selectable H264 High10
  media: hantro: rename h264_dec as it's not G1 specific anymore
  media: hantro: add dump registers debug option before decode start
  media: hantro: document encoder reg fields

 drivers/base/regmap/regmap-mmio.c             |   34 +-
 drivers/staging/media/hantro/Makefile         |    3 +-
 drivers/staging/media/hantro/hantro.h         |   79 +-
 drivers/staging/media/hantro/hantro_drv.c     |   41 +-
 drivers/staging/media/hantro/hantro_g1_regs.h |   92 +-
 ...hantro_g1_h264_dec.c => hantro_h264_dec.c} |  237 +++-
 drivers/staging/media/hantro/hantro_hw.h      |   23 +-
 .../staging/media/hantro/hantro_postproc.c    |  144 ++-
 drivers/staging/media/hantro/hantro_regmap.c  | 1015 +++++++++++++++++
 drivers/staging/media/hantro/hantro_regmap.h  |  295 +++++
 drivers/staging/media/hantro/hantro_v4l2.c    |    3 +-
 drivers/staging/media/hantro/imx8m_vpu_hw.c   |   75 +-
 drivers/staging/media/hantro/rk3288_vpu_hw.c  |    5 +-
 include/linux/regmap.h                        |    5 +
 14 files changed, 1795 insertions(+), 256 deletions(-)
 rename drivers/staging/media/hantro/{hantro_g1_h264_dec.c => hantro_h264_dec.c} (58%)
 create mode 100644 drivers/staging/media/hantro/hantro_regmap.c
 create mode 100644 drivers/staging/media/hantro/hantro_regmap.h

-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 01/18] media: hantro: document all int reg bits up to vc8000

[Adrian Ratiu]

These do not all strictly belong to the g1 core and even the majority
of previously documented bits were not used (yet) by the driver irq
handlers, but it's still very useful to have an overview of all IRQs,
especially since starting with core versions vc8000 and later the irq
bits previously used by G1 and G2 have been merged at the same address.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/hantro_g1_regs.h | 39 +++++++++++++------
 1 file changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_g1_regs.h b/drivers/staging/media/hantro/hantro_g1_regs.h
index c1756e3d5391..80ff297f6f68 100644
--- a/drivers/staging/media/hantro/hantro_g1_regs.h
+++ b/drivers/staging/media/hantro/hantro_g1_regs.h
@@ -13,17 +13,34 @@
 
 /* Decoder registers. */
 #define G1_REG_INTERRUPT				0x004
-#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)
-#define     G1_REG_INTERRUPT_DEC_ERROR_INT		BIT(16)
-#define     G1_REG_INTERRUPT_DEC_ASO_INT		BIT(15)
-#define     G1_REG_INTERRUPT_DEC_BUFFER_INT		BIT(14)
-#define     G1_REG_INTERRUPT_DEC_BUS_INT		BIT(13)
-#define     G1_REG_INTERRUPT_DEC_RDY_INT		BIT(12)
-#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)
+/* Interrupt bits. Some are present in:
+ *    - all core versions (">= g1")
+ *    - g1, missing in g2, but added back starting with vc8000d ("not in g2")
+ *    - vc8000d and later (">= vc8000d")
+ */
+#define     G1_REG_INTERRUPT_DEC_PIC_INF		BIT(24) /* not in g2 */
+#define     G1_REG_INTERRUPT_DEC_TILE_INT		BIT(23) /* >= vc8000d */
+#define     G1_REG_INTERRUPT_DEC_LINE_CNT_INT		BIT(22) /* >= vc8000d */
+#define     G1_REG_INTERRUPT_DEC_EXT_TIMEOUT_INT	BIT(21) /* >= vc8000d */
+#define     G1_REG_INTERRUPT_DEC_NO_SLICE_INT		BIT(20) /* >= vc8000d */
+#define     G1_REG_INTERRUPT_DEC_LAST_SLICE_INT		BIT(19) /* >= vc8000d */
+#define     G1_REG_INTERRUPT_DEC_TIMEOUT		BIT(18) /* >= g1 */
+#define     G1_REG_INTERRUPT_DEC_SLICE_INT		BIT(17) /* not in g2 */
+#define     G1_REG_INTERRUPT_DEC_ERROR_INT		BIT(16) /* >= g1 */
+#define     G1_REG_INTERRUPT_DEC_ASO_INT		BIT(15) /* not in g2 */
+#define     G1_REG_INTERRUPT_DEC_BUFFER_INT		BIT(14) /* >= g1 */
+#define     G1_REG_INTERRUPT_DEC_BUS_INT		BIT(13) /* >= g1 */
+#define     G1_REG_INTERRUPT_DEC_RDY_INT		BIT(12) /* >= g1 */
+#define     G1_REG_INTERRUPT_DEC_ABORT_INT		BIT(11) /* >= g2 */
+#define     G1_REG_INTERRUPT_DEC_IRQ			BIT(8) /* >= g1 */
+#define     G1_REG_INTERRUPT_DEC_TILE_INT_E		BIT(7) /* >= vc8000d */
+#define     G1_REG_INTERRUPT_DEC_SELF_RESET_DIS		BIT(6) /* >= vc8000d */
+#define     G1_REG_INTERRUPT_DEC_ABORT_E		BIT(5) /* >= vc8000d */
+#define     G1_REG_INTERRUPT_DEC_IRQ_DIS		BIT(4) /* >= g1 */
+#define     G1_REG_INTERRUPT_DEC_TIMEOUT_SOURCE		BIT(3) /* >= vc8000d */
+#define     G1_REG_INTERRUPT_DEC_BUS_INT_DIS		BIT(2) /* >= vc8000d */
+#define     G1_REG_INTERRUPT_DEC_STRM_CORRUPTED		BIT(1) /* >= vc8000d */
+#define     G1_REG_INTERRUPT_DEC_E			BIT(0) /* >= g1 */
 #define G1_REG_CONFIG					0x008
 #define     G1_REG_CONFIG_DEC_AXI_RD_ID(x)		(((x) & 0xff) << 24)
 #define     G1_REG_CONFIG_DEC_TIMEOUT_E			BIT(23)
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 02/18] media: hantro: make consistent use of decimal register notation

[Adrian Ratiu]

This header used a combination of direct hex offsets and decimal register
notation - via the G1_SWREG() macro - which is annoying when comparing with
the ref manuals which always use the equivalent of G1_SWREG(), so convert
the entire file to G1_SWREG() notation.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/hantro_g1_regs.h | 52 +++++++++----------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_g1_regs.h b/drivers/staging/media/hantro/hantro_g1_regs.h
index 80ff297f6f68..073b64cbe295 100644
--- a/drivers/staging/media/hantro/hantro_g1_regs.h
+++ b/drivers/staging/media/hantro/hantro_g1_regs.h
@@ -9,10 +9,10 @@
 #ifndef HANTRO_G1_REGS_H_
 #define HANTRO_G1_REGS_H_
 
-#define G1_SWREG(nr)                 ((nr) * 4)
+#define G1_SWREG(nr)					((nr) * 4)
 
 /* Decoder registers. */
-#define G1_REG_INTERRUPT				0x004
+#define G1_REG_INTERRUPT				G1_SWREG(1)
 /* Interrupt bits. Some are present in:
  *    - all core versions (">= g1")
  *    - g1, missing in g2, but added back starting with vc8000d ("not in g2")
@@ -41,7 +41,7 @@
 #define     G1_REG_INTERRUPT_DEC_BUS_INT_DIS		BIT(2) /* >= vc8000d */
 #define     G1_REG_INTERRUPT_DEC_STRM_CORRUPTED		BIT(1) /* >= vc8000d */
 #define     G1_REG_INTERRUPT_DEC_E			BIT(0) /* >= g1 */
-#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)
@@ -60,7 +60,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)
@@ -85,7 +85,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)
@@ -96,7 +96,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)
@@ -139,13 +139,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)
@@ -182,7 +182,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)
@@ -206,7 +206,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)
@@ -217,7 +217,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)
@@ -230,7 +230,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)
@@ -245,12 +245,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)
@@ -267,11 +267,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)
@@ -288,7 +288,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)
@@ -296,21 +296,21 @@
 #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)
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 03/18] media: hantro: make G1_REG_SOFT_RESET Rockchip specific

[Adrian Ratiu]

This register is not documented in either the G1 or VC8000D register
maps and on VC8000D there is a conflict because at the same offset the
VPU IP defines another register with a very different meaning.

What likely happened is the HW integrator which uses only the G1 IP
core added some reset/control logic at the end of the VPU map, so
it makes sense to make this register RK-specific.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/hantro_g1_regs.h | 1 -
 drivers/staging/media/hantro/rk3288_vpu_hw.c  | 4 +++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_g1_regs.h b/drivers/staging/media/hantro/hantro_g1_regs.h
index 073b64cbe295..a482a2ba6dfe 100644
--- a/drivers/staging/media/hantro/hantro_g1_regs.h
+++ b/drivers/staging/media/hantro/hantro_g1_regs.h
@@ -315,7 +315,6 @@
 #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
 
 /* Post-processor registers. */
 #define G1_REG_PP_INTERRUPT		G1_SWREG(60)
diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
index 7b299ee3e93d..4ad578b1236e 100644
--- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
+++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
@@ -13,6 +13,8 @@
 #include "hantro_g1_regs.h"
 #include "hantro_h1_regs.h"
 
+#define VDPU_REG_SOFT_RESET 0x194
+
 #define RK3288_ACLK_MAX_FREQ (400 * 1000 * 1000)
 
 /*
@@ -167,7 +169,7 @@ static void rk3288_vpu_dec_reset(struct hantro_ctx *ctx)
 
 	vdpu_write(vpu, G1_REG_INTERRUPT_DEC_IRQ_DIS, G1_REG_INTERRUPT);
 	vdpu_write(vpu, G1_REG_CONFIG_DEC_CLK_GATE_E, G1_REG_CONFIG);
-	vdpu_write(vpu, 1, G1_REG_SOFT_RESET);
+	vdpu_write(vpu, 1, VDPU_REG_SOFT_RESET);
 }
 
 /*
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 04/18] media: hantro: add reset controller support

[Adrian Ratiu]

Some SoCs might have a reset controller which disables clocks
by default in reset state which then drivers need to unreset
before being able to ungate a specific clock.

In this specific case, the hantro driver needs to ensure the
peripheral clock can be properly ungated otherwise MMIO reg
values can't be accessed.

If the SoC has no reset controller or there is no "resets" DT
property defined, this new code will have no effect.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/hantro.h     | 1 +
 drivers/staging/media/hantro/hantro_drv.c | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 65f9f7ea7dcf..bb442eb1974e 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -183,6 +183,7 @@ struct hantro_dev {
 	struct platform_device *pdev;
 	struct device *dev;
 	struct clk_bulk_data *clocks;
+	struct reset_control *reset;
 	void __iomem **reg_bases;
 	void __iomem *enc_base;
 	void __iomem *dec_base;
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 3cd00cc0a364..c2ea54552ce9 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -17,6 +17,7 @@
 #include <linux/pm.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
+#include <linux/reset.h>
 #include <linux/videodev2.h>
 #include <linux/workqueue.h>
 #include <media/v4l2-event.h>
@@ -747,6 +748,13 @@ static int hantro_probe(struct platform_device *pdev)
 
 	INIT_DELAYED_WORK(&vpu->watchdog_work, hantro_watchdog);
 
+	vpu->reset = devm_reset_control_get_optional_exclusive(&pdev->dev,
+							       NULL);
+	if (IS_ERR(vpu->reset))
+		vpu->reset = NULL;
+
+	reset_control_reset(vpu->reset);
+
 	vpu->clocks = devm_kcalloc(&pdev->dev, vpu->variant->num_clocks,
 				   sizeof(*vpu->clocks), GFP_KERNEL);
 	if (!vpu->clocks)
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 04/18] media: hantro: add reset controller support

[Philipp Zabel]

Hi Adrian,

On Mon, 2020-10-12 at 23:59 +0300, Adrian Ratiu wrote:
> Some SoCs might have a reset controller which disables clocks
> by default in reset state which then drivers need to unreset
> before being able to ungate a specific clock.
> 
> In this specific case, the hantro driver needs to ensure the
> peripheral clock can be properly ungated otherwise MMIO reg
> values can't be accessed.
> 
> If the SoC has no reset controller or there is no "resets" DT
> property defined, this new code will have no effect.
> 
> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/staging/media/hantro/hantro.h     | 1 +
>  drivers/staging/media/hantro/hantro_drv.c | 8 ++++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
> index 65f9f7ea7dcf..bb442eb1974e 100644
> --- a/drivers/staging/media/hantro/hantro.h
> +++ b/drivers/staging/media/hantro/hantro.h
> @@ -183,6 +183,7 @@ struct hantro_dev {
>  	struct platform_device *pdev;
>  	struct device *dev;
>  	struct clk_bulk_data *clocks;
> +	struct reset_control *reset;
>  	void __iomem **reg_bases;
>  	void __iomem *enc_base;
>  	void __iomem *dec_base;
> diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
> index 3cd00cc0a364..c2ea54552ce9 100644
> --- a/drivers/staging/media/hantro/hantro_drv.c
> +++ b/drivers/staging/media/hantro/hantro_drv.c
> @@ -17,6 +17,7 @@
>  #include <linux/pm.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/slab.h>
> +#include <linux/reset.h>
>  #include <linux/videodev2.h>
>  #include <linux/workqueue.h>
>  #include <media/v4l2-event.h>
> @@ -747,6 +748,13 @@ static int hantro_probe(struct platform_device *pdev)
>  
>  	INIT_DELAYED_WORK(&vpu->watchdog_work, hantro_watchdog);
>  
> +	vpu->reset = devm_reset_control_get_optional_exclusive(&pdev->dev,
> +							       NULL);
> +	if (IS_ERR(vpu->reset))
> +		vpu->reset = NULL;

Please return the error. If the optional reset is missing from the
device tree, devm_reset_control_get_optional_exclusive() returns NULL
already.

regards
Philipp

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 05/18] media: hantro: prepare clocks before variant inits are run

[Adrian Ratiu]

The fundamental idea is: clocks are prepared in the driver probe() then
each use-case will enable/disable them as needed.

Some variants like imx8mq need to have the clocks enabled during the
HW init phase, so they will benefit from having the clocks prepared
before the variant init callback to avoid duing a full prepare_enable/
unprepare_disable, so move the clk prepare a bit earlier.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/hantro_drv.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index c2ea54552ce9..3734efa80a7e 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -813,22 +813,22 @@ static int hantro_probe(struct platform_device *pdev)
 		}
 	}
 
+	ret = clk_bulk_prepare(vpu->variant->num_clocks, vpu->clocks);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to prepare clocks\n");
+		return ret;
+	}
+
 	ret = vpu->variant->init(vpu);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to init VPU hardware\n");
-		return ret;
+		goto err_clk_unprepare;
 	}
 
 	pm_runtime_set_autosuspend_delay(vpu->dev, 100);
 	pm_runtime_use_autosuspend(vpu->dev);
 	pm_runtime_enable(vpu->dev);
 
-	ret = clk_bulk_prepare(vpu->variant->num_clocks, vpu->clocks);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to prepare clocks\n");
-		return ret;
-	}
-
 	ret = v4l2_device_register(&pdev->dev, &vpu->v4l2_dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register v4l2 device\n");
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 06/18] media: hantro: imx8mq: simplify ctrlblk reset logic

[Adrian Ratiu]

The G1 and G2 cores on imx8mq share a common "control block" used
to reset and enable the core clocks as well as enable functioning
via ctrl FUSE registers (these are not the FUSEs on the VPU cores,
they are just used to enable/disable the cores and allow the real
VPU FUSE regs to become available).

The problem is that, while the cores can be operated independently
from one another (different config reg mem regions, separate IRQs),
they can not be reset or powered down independently as the current
code implies. This has been a source for many bugs and frustration
when trying to enable G2 which this driver does not support yet.

So we simplify the ctrlblk reset logic to always reset both cores,
exactly like the vendor linux-imx provided driver "hantrodec" does
for this SoC.

Going forward, this simplified code should be moved in the future to
its own reset controller driver as the reset framework also supports
shared reset resources so the runtime PM logic can disable both cores
when none of them are in use (this is not done yet because only G1
is supported in the driver so there is no need to account for G2).

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/hantro.h       |  2 -
 drivers/staging/media/hantro/imx8m_vpu_hw.c | 74 +++++++--------------
 2 files changed, 24 insertions(+), 52 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index bb442eb1974e..2dd4362d4080 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -167,7 +167,6 @@ hantro_vdev_to_func(struct video_device *vdev)
  * @reg_bases:		Mapped addresses of VPU registers.
  * @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.
  * @vpu_mutex:		Mutex to synchronize V4L2 calls.
  * @irqlock:		Spinlock to synchronize access to data structures
  *			shared with interrupt handlers.
@@ -187,7 +186,6 @@ struct hantro_dev {
 	void __iomem **reg_bases;
 	void __iomem *enc_base;
 	void __iomem *dec_base;
-	void __iomem *ctrl_base;
 
 	struct mutex vpu_mutex;	/* video_device lock */
 	spinlock_t irqlock;
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
index c222de075ef4..b2a401a33992 100644
--- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -24,34 +24,13 @@
 #define CTRL_G1_PP_FUSE		0x0c
 #define CTRL_G2_DEC_FUSE	0x10
 
-static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
-{
-	u32 val;
-
-	/* Assert */
-	val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
-	val &= ~reset_bits;
-	writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
-
-	udelay(2);
-
-	/* Release */
-	val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
-	val |= reset_bits;
-	writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
-}
-
-static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits)
-{
-	u32 val;
-
-	val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE);
-	val |= clock_bits;
-	writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE);
-}
-
-static int imx8mq_runtime_resume(struct hantro_dev *vpu)
+/*
+ * Due to a HW limitation, both G1 and G2 VPU cores on imx8mq need to be reset
+ * together via their unified ctrl block.
+ */
+static int imx8mq_ctrlblk_reset(struct hantro_dev *vpu)
 {
+	void __iomem *ctrl_base = vpu->reg_bases[vpu->variant->num_regs - 1];
 	int ret;
 
 	ret = clk_bulk_prepare_enable(vpu->variant->num_clocks, vpu->clocks);
@@ -60,13 +39,18 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu)
 		return ret;
 	}
 
-	imx8m_soft_reset(vpu, RESET_G1 | RESET_G2);
-	imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2);
+	/* reset HW and ungate clocks via ctrl block */
+	writel(RESET_G1 | RESET_G2, ctrl_base + CTRL_SOFT_RESET);
+	writel(CLOCK_G1 | CLOCK_G2, ctrl_base + CTRL_CLOCK_ENABLE);
 
-	/* Set values of the fuse registers */
-	writel(0xffffffff, vpu->ctrl_base + CTRL_G1_DEC_FUSE);
-	writel(0xffffffff, vpu->ctrl_base + CTRL_G1_PP_FUSE);
-	writel(0xffffffff, vpu->ctrl_base + CTRL_G2_DEC_FUSE);
+	/*
+	 * enable fuse functionalities for each core, these are not real fuses
+	 * but registers which enable the cores and makes accesible their real
+	 * read-only fuse registers describing supported features.
+	 */
+	writel(0xffffffff, ctrl_base + CTRL_G1_DEC_FUSE);
+	writel(0xffffffff, ctrl_base + CTRL_G1_PP_FUSE);
+	writel(0xffffffff, ctrl_base + CTRL_G2_DEC_FUSE);
 
 	clk_bulk_disable_unprepare(vpu->variant->num_clocks, vpu->clocks);
 
@@ -148,19 +132,9 @@ static irqreturn_t imx8m_vpu_g1_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int imx8mq_vpu_hw_init(struct hantro_dev *vpu)
+static void imx8m_vpu_reset(struct hantro_ctx *ctx)
 {
-	vpu->dec_base = vpu->reg_bases[0];
-	vpu->ctrl_base = vpu->reg_bases[vpu->variant->num_regs - 1];
-
-	return 0;
-}
-
-static void imx8m_vpu_g1_reset(struct hantro_ctx *ctx)
-{
-	struct hantro_dev *vpu = ctx->dev;
-
-	imx8m_soft_reset(vpu, RESET_G1);
+	imx8mq_ctrlblk_reset(ctx->dev);
 }
 
 /*
@@ -170,19 +144,19 @@ static void imx8m_vpu_g1_reset(struct hantro_ctx *ctx)
 static const struct hantro_codec_ops imx8mq_vpu_codec_ops[] = {
 	[HANTRO_MODE_MPEG2_DEC] = {
 		.run = hantro_g1_mpeg2_dec_run,
-		.reset = imx8m_vpu_g1_reset,
+		.reset = imx8m_vpu_reset,
 		.init = hantro_mpeg2_dec_init,
 		.exit = hantro_mpeg2_dec_exit,
 	},
 	[HANTRO_MODE_VP8_DEC] = {
 		.run = hantro_g1_vp8_dec_run,
-		.reset = imx8m_vpu_g1_reset,
+		.reset = imx8m_vpu_reset,
 		.init = hantro_vp8_dec_init,
 		.exit = hantro_vp8_dec_exit,
 	},
 	[HANTRO_MODE_H264_DEC] = {
 		.run = hantro_g1_h264_dec_run,
-		.reset = imx8m_vpu_g1_reset,
+		.reset = imx8m_vpu_reset,
 		.init = hantro_h264_dec_init,
 		.exit = hantro_h264_dec_exit,
 	},
@@ -209,8 +183,8 @@ const struct hantro_variant imx8mq_vpu_variant = {
 	.codec = HANTRO_MPEG2_DECODER | HANTRO_VP8_DECODER |
 		 HANTRO_H264_DECODER,
 	.codec_ops = imx8mq_vpu_codec_ops,
-	.init = imx8mq_vpu_hw_init,
-	.runtime_resume = imx8mq_runtime_resume,
+	.init = imx8mq_ctrlblk_reset,
+	.runtime_resume = imx8mq_ctrlblk_reset,
 	.irqs = imx8mq_irqs,
 	.num_irqs = ARRAY_SIZE(imx8mq_irqs),
 	.clk_names = imx8mq_clk_names,
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 07/18] regmap: mmio: add config option to allow relaxed MMIO accesses

[Adrian Ratiu]

On some platforms (eg armv7 due to the CONFIG_ARM_DMA_MEM_BUFFERABLE)
MMIO R/W operations always add memory barriers which can increase load,
decrease battery life or in general reduce performance unnecessarily
on devices which access a lot of configuration registers and where
ordering does not matter (eg. media accelerators like the Verisilicon /
Hantro video decoders).

Drivers used to call the relaxed MMIO variants directly but since they
are now accessing the MMIO registers via regmaps (to compensate for
for different VPU HW reg layouts via regmap fields), there is a need
for a relaxed API / config to preserve their existing behaviour.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/base/regmap/regmap-mmio.c | 34 +++++++++++++++++++++++++++----
 include/linux/regmap.h            |  5 +++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/drivers/base/regmap/regmap-mmio.c b/drivers/base/regmap/regmap-mmio.c
index af967d8f975e..21193ef2a923 100644
--- a/drivers/base/regmap/regmap-mmio.c
+++ b/drivers/base/regmap/regmap-mmio.c
@@ -16,6 +16,7 @@
 struct regmap_mmio_context {
 	void __iomem *regs;
 	unsigned val_bytes;
+	bool relaxed_mmio;
 
 	bool attached_clk;
 	struct clk *clk;
@@ -72,14 +73,20 @@ static void regmap_mmio_write8(struct regmap_mmio_context *ctx,
 				unsigned int reg,
 				unsigned int val)
 {
-	writeb(val, ctx->regs + reg);
+	if (ctx->relaxed_mmio)
+		writeb_relaxed(val, ctx->regs + reg);
+	else
+		writeb(val, ctx->regs + reg);
 }
 
 static void regmap_mmio_write16le(struct regmap_mmio_context *ctx,
 				  unsigned int reg,
 				  unsigned int val)
 {
-	writew(val, ctx->regs + reg);
+	if (ctx->relaxed_mmio)
+		writew_relaxed(val, ctx->regs + reg);
+	else
+		writew(val, ctx->regs + reg);
 }
 
 static void regmap_mmio_write16be(struct regmap_mmio_context *ctx,
@@ -93,7 +100,10 @@ static void regmap_mmio_write32le(struct regmap_mmio_context *ctx,
 				  unsigned int reg,
 				  unsigned int val)
 {
-	writel(val, ctx->regs + reg);
+	if (ctx->relaxed_mmio)
+		writel_relaxed(val, ctx->regs + reg);
+	else
+		writel(val, ctx->regs + reg);
 }
 
 static void regmap_mmio_write32be(struct regmap_mmio_context *ctx,
@@ -108,7 +118,10 @@ static void regmap_mmio_write64le(struct regmap_mmio_context *ctx,
 				  unsigned int reg,
 				  unsigned int val)
 {
-	writeq(val, ctx->regs + reg);
+	if (ctx->relaxed_mmio)
+		writeq_relaxed(val, ctx->regs + reg);
+	else
+		writeq(val, ctx->regs + reg);
 }
 #endif
 
@@ -134,12 +147,18 @@ static int regmap_mmio_write(void *context, unsigned int reg, unsigned int val)
 static unsigned int regmap_mmio_read8(struct regmap_mmio_context *ctx,
 				      unsigned int reg)
 {
+	if (ctx->relaxed_mmio)
+		return readb_relaxed(ctx->regs + reg);
+
 	return readb(ctx->regs + reg);
 }
 
 static unsigned int regmap_mmio_read16le(struct regmap_mmio_context *ctx,
 				         unsigned int reg)
 {
+	if (ctx->relaxed_mmio)
+		return readw_relaxed(ctx->regs + reg);
+
 	return readw(ctx->regs + reg);
 }
 
@@ -152,6 +171,9 @@ static unsigned int regmap_mmio_read16be(struct regmap_mmio_context *ctx,
 static unsigned int regmap_mmio_read32le(struct regmap_mmio_context *ctx,
 				         unsigned int reg)
 {
+	if (ctx->relaxed_mmio)
+		return readl_relaxed(ctx->regs + reg);
+
 	return readl(ctx->regs + reg);
 }
 
@@ -165,6 +187,9 @@ static unsigned int regmap_mmio_read32be(struct regmap_mmio_context *ctx,
 static unsigned int regmap_mmio_read64le(struct regmap_mmio_context *ctx,
 				         unsigned int reg)
 {
+	if (ctx->relaxed_mmio)
+		return readq_relaxed(ctx->regs + reg);
+
 	return readq(ctx->regs + reg);
 }
 #endif
@@ -237,6 +262,7 @@ static struct regmap_mmio_context *regmap_mmio_gen_context(struct device *dev,
 
 	ctx->regs = regs;
 	ctx->val_bytes = config->val_bits / 8;
+	ctx->relaxed_mmio = config->use_relaxed_mmio;
 	ctx->clk = ERR_PTR(-ENODEV);
 
 	switch (regmap_get_val_endian(dev, &regmap_mmio, config)) {
diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index e7834d98207f..126fe700d1d8 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -315,6 +315,10 @@ typedef void (*regmap_unlock)(void *);
  *                   masks are used.
  * @zero_flag_mask: If set, read_flag_mask and write_flag_mask are used even
  *                   if they are both empty.
+ * @use_relaxed_mmio: If set, MMIO R/W operations will not use memory barriers.
+ *                    This can avoid load on devices which don't require strict
+ *                    orderings, but drivers should carefully add any explicit
+ *                    memory barriers when they may require them.
  * @use_single_read: If set, converts the bulk read operation into a series of
  *                   single read operations. This is useful for a device that
  *                   does not support  bulk read.
@@ -388,6 +392,7 @@ struct regmap_config {
 
 	bool use_single_read;
 	bool use_single_write;
+	bool use_relaxed_mmio;
 	bool can_multi_write;
 
 	enum regmap_endian reg_format_endian;
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 07/18] regmap: mmio: add config option to allow relaxed MMIO accesses

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 351 bytes --]

On Mon, Oct 12, 2020 at 11:59:46PM +0300, Adrian Ratiu wrote:

> -	writeb(val, ctx->regs + reg);
> +	if (ctx->relaxed_mmio)
> +		writeb_relaxed(val, ctx->regs + reg);
> +	else
> +		writeb(val, ctx->regs + reg);

There is no point in doing a conditional operation on every I/O, it'd be
better to register a different set of ops when doing relaxed I/O.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 07/18] regmap: mmio: add config option to allow relaxed MMIO accesses

[Adrian Ratiu]

Hello Mark,

On Tue, 13 Oct 2020, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Oct 12, 2020 at 11:59:46PM +0300, Adrian Ratiu wrote: 
> 
>> -	writeb(val, ctx->regs + reg); +	if (ctx->relaxed_mmio) + 
>> writeb_relaxed(val, ctx->regs + reg); +	else + 
>> writeb(val, ctx->regs + reg); 
> 
> There is no point in doing a conditional operation on every I/O, 
> it'd be better to register a different set of ops when doing 
> relaxed I/O. 

Indeed I have considered adding new functions but went with this 
solution because it's easier for the users to only have to define 
a "relaxed" config then test the regmap ctx as above.

Thinking a bit more about it, yes, it makes more sense to have 
dedicated ops: this way users don't have to be explicit about 
adding membarriers and can combine relaxed and non-relaxed more 
easily, so it's also a better API trade-off in addition to 
avoiding the conditional. Thanks!

Question: Do you want me to split this patch from the series and 
send it separately just for the regmap subsystem to be easier to 
review / apply?

Kind regards,
Adrian

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 07/18] regmap: mmio: add config option to allow relaxed MMIO accesses

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1881 bytes --]

On Wed, Oct 14, 2020 at 02:51:14PM +0300, Adrian Ratiu wrote:
> On Tue, 13 Oct 2020, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Oct 12, 2020 at 11:59:46PM +0300, Adrian Ratiu wrote:

> > > -	writeb(val, ctx->regs + reg); +	if (ctx->relaxed_mmio) +
> > > writeb_relaxed(val, ctx->regs + reg); +	else + writeb(val, ctx->regs
> > > + reg);

> > There is no point in doing a conditional operation on every I/O, it'd be
> > better to register a different set of ops when doing relaxed I/O.

> Indeed I have considered adding new functions but went with this solution
> because it's easier for the users to only have to define a "relaxed" config
> then test the regmap ctx as above.

It seems like you've taken this in a direction other than what  I was
thinking of here - defining separate ops doesn't mean we have to do
anything which has any impact on the interface seen by users.  The
regmap config is supplied at registration time, it's just as available
then as it is when doing I/O.

> Thinking a bit more about it, yes, it makes more sense to have dedicated
> ops: this way users don't have to be explicit about adding membarriers and
> can combine relaxed and non-relaxed more easily, so it's also a better API
> trade-off in addition to avoiding the conditional. Thanks!

I'm not sure what you're proposing here - it does seem useful to be able
to combine relaxed and non-relaxed I/O but that seems like it'd break
down the abstraction for regmap since tht's not really a concept other
buses are going to have?  Unless we provide an operation to switch by
setting flags or somethin possibly and integrate it with the cache
perhaps.  Could you be a bit more specific about what you were thinking
of here please?

> Question: Do you want me to split this patch from the series and send it
> separately just for the regmap subsystem to be easier to review / apply?

Sure.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 07/18] regmap: mmio: add config option to allow relaxed MMIO accesses

[Adrian Ratiu]

On Wed, 14 Oct 2020, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Oct 14, 2020 at 02:51:14PM +0300, Adrian Ratiu wrote: 
>> On Tue, 13 Oct 2020, Mark Brown <broonie@kernel.org> wrote: 
>> > On Mon, Oct 12, 2020 at 11:59:46PM +0300, Adrian Ratiu wrote: 
> 
>> > > -	writeb(val, ctx->regs + reg); +	if 
>> > > (ctx->relaxed_mmio) + writeb_relaxed(val, ctx->regs + reg); 
>> > > +	else + writeb(val, ctx->regs + reg); 
> 
>> > There is no point in doing a conditional operation on every 
>> > I/O, it'd be better to register a different set of ops when 
>> > doing relaxed I/O. 
> 
>> Indeed I have considered adding new functions but went with 
>> this solution because it's easier for the users to only have to 
>> define a "relaxed" config then test the regmap ctx as above. 
> 
> It seems like you've taken this in a direction other than what 
> I was thinking of here - defining separate ops doesn't mean we 
> have to do anything which has any impact on the interface seen 
> by users.  The regmap config is supplied at registration time, 
> it's just as available then as it is when doing I/O.

Right. I got confused by the meaning of ops :) Sorry about that.
 
> 
>> Thinking a bit more about it, yes, it makes more sense to have 
>> dedicated ops: this way users don't have to be explicit about 
>> adding membarriers and can combine relaxed and non-relaxed more 
>> easily, so it's also a better API trade-off in addition to 
>> avoiding the conditional. Thanks! 
> 
> I'm not sure what you're proposing here - it does seem useful to 
> be able to combine relaxed and non-relaxed I/O but that seems 
> like it'd break down the abstraction for regmap since tht's not 
> really a concept other buses are going to have?  Unless we 
> provide an operation to switch by setting flags or somethin 
> possibly and integrate it with the cache perhaps.  Could you be 
> a bit more specific about what you were thinking of here please?

I was thinking about exposing a relaxed API like 
regmap_write_relaxed but now that I know what you meant by ops and 
also that it doesn't make sense for other busses / violates the 
abstraction, I realize that is a bad idea and I will continue 
improving this to avoid the conditional and send a separete 
patch. Thanks again!

>
>> Question: Do you want me to split this patch from the series and send it
>> separately just for the regmap subsystem to be easier to review / apply?
>
> Sure.

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 08/18] media: hantro: add initial MMIO regmap infrastructure

[Adrian Ratiu]

This creates regmaps on top of the memory mapped regions for encoders
and decoders and converts the helpers in hantro.h to do their R/W via
these regmaps.

In itself this indirection layer is quite useless, but the key is the
field API also initialized using the regmaps which is currently empty.

Further changes can define any necessary regmap field APIs for various
HW revisions like G1, G2 or configure the fields for different HW reg
layouts to support newer HW revisions like VC8000D.

No regmap is defined for the ctrl registers of imx8m because their
usage is very simple and there is no known register layout divergence.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 drivers/staging/media/hantro/Makefile        |   1 +
 drivers/staging/media/hantro/hantro.h        |  35 +++--
 drivers/staging/media/hantro/hantro_drv.c    |  15 +-
 drivers/staging/media/hantro/hantro_regmap.c | 144 +++++++++++++++++++
 drivers/staging/media/hantro/hantro_regmap.h |  23 +++
 5 files changed, 206 insertions(+), 12 deletions(-)
 create mode 100644 drivers/staging/media/hantro/hantro_regmap.c
 create mode 100644 drivers/staging/media/hantro/hantro_regmap.h

diff --git a/drivers/staging/media/hantro/Makefile b/drivers/staging/media/hantro/Makefile
index 743ce08eb184..52bc0ee73569 100644
--- a/drivers/staging/media/hantro/Makefile
+++ b/drivers/staging/media/hantro/Makefile
@@ -9,6 +9,7 @@ hantro-vpu-y += \
 		hantro_h1_jpeg_enc.o \
 		hantro_g1_h264_dec.o \
 		hantro_g1_mpeg2_dec.o \
+		hantro_regmap.o \
 		hantro_g1_vp8_dec.o \
 		rk3399_vpu_hw_jpeg_enc.o \
 		rk3399_vpu_hw_mpeg2_dec.o \
diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 2dd4362d4080..c5425cd5ac84 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -16,6 +16,7 @@
 #include <linux/videodev2.h>
 #include <linux/wait.h>
 #include <linux/clk.h>
+#include <linux/regmap.h>
 
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-device.h>
@@ -28,6 +29,8 @@
 
 struct hantro_ctx;
 struct hantro_codec_ops;
+struct hantro_regmap_dec_fields;
+struct hantro_regmap_enc_fields;
 
 #define HANTRO_JPEG_ENCODER	BIT(0)
 #define HANTRO_ENCODERS		0x0000ffff
@@ -165,8 +168,12 @@ hantro_vdev_to_func(struct video_device *vdev)
  *			dev_ macros.
  * @clocks:		Array of clock handles.
  * @reg_bases:		Mapped addresses of VPU registers.
- * @enc_base:		Mapped address of VPU encoder register for convenience.
- * @dec_base:		Mapped address of VPU decoder register for convenience.
+ * @regs_enc:		MMIO regmap of VPU encoder block for convenience.
+ * @regs_dec:		MMIO regmap of VPU decoder block for convenience.
+ * @reg_fields_dec:	Decoder regfields inside above regamp region.
+ * @reg_fields_enc:	Encoder regfields inside above regamp region.
+ * @core_hw_dec_rev	Runtime detected HW decoder core revision.
+ * @core_hw_enc_rev	Runtime detected HW encoder core revision.
  * @vpu_mutex:		Mutex to synchronize V4L2 calls.
  * @irqlock:		Spinlock to synchronize access to data structures
  *			shared with interrupt handlers.
@@ -184,8 +191,12 @@ struct hantro_dev {
 	struct clk_bulk_data *clocks;
 	struct reset_control *reset;
 	void __iomem **reg_bases;
-	void __iomem *enc_base;
-	void __iomem *dec_base;
+	struct regmap *regs_dec;
+	struct regmap *regs_enc;
+	struct hantro_regmap_fields_dec *reg_fields_dec;
+	struct hantro_regmap_fields_enc *reg_fields_enc;
+	u32 core_hw_dec_rev;
+	u32 core_hw_enc_rev;
 
 	struct mutex vpu_mutex;	/* video_device lock */
 	spinlock_t irqlock;
@@ -329,20 +340,22 @@ static inline void vepu_write_relaxed(struct hantro_dev *vpu,
 				      u32 val, u32 reg)
 {
 	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
-	writel_relaxed(val, vpu->enc_base + reg);
+	regmap_write(vpu->regs_enc, reg, val);
 }
 
 static inline void vepu_write(struct hantro_dev *vpu, u32 val, u32 reg)
 {
 	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
-	writel(val, vpu->enc_base + reg);
+	regmap_write(vpu->regs_enc, reg, val);
 }
 
 static inline u32 vepu_read(struct hantro_dev *vpu, u32 reg)
 {
-	u32 val = readl(vpu->enc_base + reg);
+	u32 val;
 
+	regmap_read(vpu->regs_enc, reg, &val);
 	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
+
 	return val;
 }
 
@@ -350,20 +363,22 @@ static inline void vdpu_write_relaxed(struct hantro_dev *vpu,
 				      u32 val, u32 reg)
 {
 	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
-	writel_relaxed(val, vpu->dec_base + reg);
+	regmap_write(vpu->regs_dec, reg, val);
 }
 
 static inline void vdpu_write(struct hantro_dev *vpu, u32 val, u32 reg)
 {
 	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
-	writel(val, vpu->dec_base + reg);
+	regmap_write(vpu->regs_dec, reg, val);
 }
 
 static inline u32 vdpu_read(struct hantro_dev *vpu, u32 reg)
 {
-	u32 val = readl(vpu->dec_base + reg);
+	u32 val;
 
+	regmap_read(vpu->regs_dec, reg, &val);
 	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
+
 	return val;
 }
 
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 3734efa80a7e..e225515d6985 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -28,6 +28,7 @@
 #include "hantro_v4l2.h"
 #include "hantro.h"
 #include "hantro_hw.h"
+#include "hantro_regmap.h"
 
 #define DRIVER_NAME "hantro-vpu"
 
@@ -782,8 +783,6 @@ static int hantro_probe(struct platform_device *pdev)
 		if (IS_ERR(vpu->reg_bases[i]))
 			return PTR_ERR(vpu->reg_bases[i]);
 	}
-	vpu->enc_base = vpu->reg_bases[0] + vpu->variant->enc_offset;
-	vpu->dec_base = vpu->reg_bases[0] + vpu->variant->dec_offset;
 
 	ret = dma_set_coherent_mask(vpu->dev, DMA_BIT_MASK(32));
 	if (ret) {
@@ -829,6 +828,18 @@ static int hantro_probe(struct platform_device *pdev)
 	pm_runtime_use_autosuspend(vpu->dev);
 	pm_runtime_enable(vpu->dev);
 
+	ret = hantro_regmap_init_dec(vpu);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to init decoder regmap\n");
+		goto err_clk_unprepare;
+	}
+
+	ret = hantro_regmap_init_enc(vpu);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to init encoder regmap\n");
+		goto err_clk_unprepare;
+	}
+
 	ret = v4l2_device_register(&pdev->dev, &vpu->v4l2_dev);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register v4l2 device\n");
diff --git a/drivers/staging/media/hantro/hantro_regmap.c b/drivers/staging/media/hantro/hantro_regmap.c
new file mode 100644
index 000000000000..890e443688e2
--- /dev/null
+++ b/drivers/staging/media/hantro/hantro_regmap.c
@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Hantro VPU codec driver
+ *
+ * Copyright (C) 2020 Collabora, Ltd.
+ *
+ */
+
+#include "hantro.h"
+#include "hantro_regmap.h"
+
+/**
+ * struct regmap_config - hantro regmap configuration, for now just a single cfg
+ * is used for all registers and fields, in the future this can be granularised
+ * to have different configs for eg enc, dec, pp, each with its own checks and
+ * valiations (bounds, read/write enforcement and so on)
+ */
+struct regmap_config hantro_regmap_dec = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	/* all hantro accesses are sequential, even with respect to irq ctx */
+	.disable_locking = true,
+	.name = "hantro_regmap_dec",
+};
+
+struct regmap_config hantro_regmap_enc = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.disable_locking = true,
+	.name = "hantro_regmap_enc",
+};
+
+struct hantro_field_enc {
+	/* TODO: populate encoder fields */
+};
+
+struct hantro_field_dec {
+	/* TODO: populate decoder fields */
+};
+
+#define INIT_FIELD_CFG(f, codec, conf) ({					\
+		vpu->reg_fields_##codec->(f) = devm_regmap_field_alloc(vpu->dev,\
+						     vpu->regs_##codec,		\
+						     field->(conf)); \
+	if (IS_ERR(vpu->reg_fields_##codec->f)) {				\
+		dev_warn(vpu->dev, "Couldn't create regmap field " #f "\n");	\
+		return PTR_ERR(vpu->reg_fields_##codec->f);			\
+	}})
+
+#define INIT_DEC_FIELD(f) INIT_FIELD_CFG(f, dec, cfg_##f)
+#define INIT_ENC_FIELD(f) INIT_FIELD_CFG(f, enc, cfg_##f)
+
+static int hantro_regmap_fields_init_dec(struct hantro_dev *vpu,
+					 const struct hantro_field_dec *field)
+{
+	vpu->reg_fields_dec = devm_kzalloc(vpu->dev, sizeof(*vpu->reg_fields_dec),
+					   GFP_KERNEL);
+	if (!vpu->reg_fields_dec)
+		return -ENOMEM;
+
+	/* TODO: add decoder fields */
+
+	return 0;
+}
+
+static int hantro_regmap_fields_init_enc(struct hantro_dev *vpu,
+					 const struct hantro_field_enc *field)
+{
+	vpu->reg_fields_enc = devm_kzalloc(vpu->dev, sizeof(*vpu->reg_fields_enc),
+					   GFP_KERNEL);
+	if (!vpu->reg_fields_enc)
+		return -ENOMEM;
+
+	/* TODO: add encoder fields */
+
+	return 0;
+}
+
+int hantro_regmap_init_enc(struct hantro_dev *vpu)
+{
+	const struct hantro_field_enc *field = NULL;
+	void __iomem *enc_base = vpu->reg_bases[0] + vpu->variant->enc_offset;
+	int ret;
+
+	if (!vpu->variant->enc_fmts)
+		return 0;
+
+	ret = clk_bulk_enable(vpu->variant->num_clocks, vpu->clocks);
+	if (ret) {
+		dev_err(vpu->dev, "Failed to enable clocks\n");
+		return ret;
+	}
+
+	vpu->core_hw_enc_rev = (readl(enc_base) >> 16) & 0xffff;
+	vpu_debug(0, "Detected hantro encoder revision %x\n",
+		  vpu->core_hw_enc_rev);
+
+	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
+
+	vpu->regs_enc = devm_regmap_init_mmio(vpu->dev, enc_base,
+					      &hantro_regmap_enc);
+	if (IS_ERR(vpu->regs_enc)) {
+		ret = PTR_ERR(vpu->regs_enc);
+		dev_err(vpu->dev, "Failed to create encoder regmap: %d\n", ret);
+		return ret;
+	}
+
+	return hantro_regmap_fields_init_enc(vpu, field);
+}
+
+int hantro_regmap_init_dec(struct hantro_dev *vpu)
+{
+	const struct hantro_field_dec *field = NULL;
+	void __iomem *dec_base = vpu->reg_bases[0] + vpu->variant->dec_offset;
+	int ret;
+
+	if (!vpu->variant->dec_fmts)
+		return 0;
+
+	ret = clk_bulk_enable(vpu->variant->num_clocks, vpu->clocks);
+	if (ret) {
+		dev_err(vpu->dev, "Failed to enable clocks\n");
+		return ret;
+	}
+
+	vpu->core_hw_dec_rev = (readl(dec_base) >> 16) & 0xffff;
+
+	vpu_debug(0, "Detected hantro decoder revision %x\n",
+		  vpu->core_hw_dec_rev);
+
+	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
+
+	vpu->regs_dec = devm_regmap_init_mmio(vpu->dev, dec_base,
+					      &hantro_regmap_dec);
+	if (IS_ERR(vpu->regs_dec)) {
+		ret = PTR_ERR(vpu->regs_dec);
+		dev_err(vpu->dev, "Failed to create decoder regmap: %d\n", ret);
+		return ret;
+	}
+
+	return hantro_regmap_fields_init_dec(vpu, field);
+}
diff --git a/drivers/staging/media/hantro/hantro_regmap.h b/drivers/staging/media/hantro/hantro_regmap.h
new file mode 100644
index 000000000000..52668a8bafb9
--- /dev/null
+++ b/drivers/staging/media/hantro/hantro_regmap.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Hantro VPU codec driver
+ *
+ * Copyright (C) 2020 Collabora, Ltd.
+ *
+ */
+
+#ifndef HANTRO_REGMAP_H_
+#define HANTRO_REGMAP_H_
+
+struct hantro_regmap_fields_dec {
+	/* TODO: populate decoder fields */
+};
+
+struct hantro_regmap_fields_enc {
+	/* TODO: populate encoder fields */
+};
+
+int hantro_regmap_init_dec(struct hantro_dev *vpu);
+int hantro_regmap_init_enc(struct hantro_dev *vpu);
+
+#endif /* HANTRO_REGMAP_H_ */
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 09/18] media: hantro: default regmap to relaxed MMIO

[Adrian Ratiu]

This is done to match the pre-regmap membarrier behaviour, ensuring
default regmap_write calls in _relaxed() are indeed relaxed while
the non-relaxed versions include an explicit mem-barrier call.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/hantro.h        | 4 ++++
 drivers/staging/media/hantro/hantro_regmap.c | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index c5425cd5ac84..5b7fbdc3779d 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -346,6 +346,7 @@ static inline void vepu_write_relaxed(struct hantro_dev *vpu,
 static inline void vepu_write(struct hantro_dev *vpu, u32 val, u32 reg)
 {
 	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
+	wmb(); /* flush encoder previous relaxed writes */
 	regmap_write(vpu->regs_enc, reg, val);
 }
 
@@ -354,6 +355,7 @@ static inline u32 vepu_read(struct hantro_dev *vpu, u32 reg)
 	u32 val;
 
 	regmap_read(vpu->regs_enc, reg, &val);
+	rmb(); /* read encoder swreg data in order */
 	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
 
 	return val;
@@ -369,6 +371,7 @@ static inline void vdpu_write_relaxed(struct hantro_dev *vpu,
 static inline void vdpu_write(struct hantro_dev *vpu, u32 val, u32 reg)
 {
 	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
+	wmb();/* flush decoder previous relaxed writes */
 	regmap_write(vpu->regs_dec, reg, val);
 }
 
@@ -377,6 +380,7 @@ static inline u32 vdpu_read(struct hantro_dev *vpu, u32 reg)
 	u32 val;
 
 	regmap_read(vpu->regs_dec, reg, &val);
+	rmb(); /* read decoder swreg data in order */
 	vpu_debug(6, "0x%04x = 0x%08x\n", reg / 4, val);
 
 	return val;
diff --git a/drivers/staging/media/hantro/hantro_regmap.c b/drivers/staging/media/hantro/hantro_regmap.c
index 890e443688e2..2fc409cbd797 100644
--- a/drivers/staging/media/hantro/hantro_regmap.c
+++ b/drivers/staging/media/hantro/hantro_regmap.c
@@ -21,6 +21,7 @@ struct regmap_config hantro_regmap_dec = {
 	.reg_stride = 4,
 	/* all hantro accesses are sequential, even with respect to irq ctx */
 	.disable_locking = true,
+	.use_relaxed_mmio = true,
 	.name = "hantro_regmap_dec",
 };
 
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 10/18] media: hantro: convert G1 h264 decoder to regmap fields

[Adrian Ratiu]

Populate the regmap field API for G1 h264 decoding and convert the
G1 h264 decoder source to use the new API. This is done because we
will add support for the newer VC8000D core which will configure
the regmap API fields differently to match its own hwreg layout.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 .../staging/media/hantro/hantro_g1_h264_dec.c | 71 ++++++++++-------
 drivers/staging/media/hantro/hantro_regmap.c  | 79 ++++++++++++++++++-
 drivers/staging/media/hantro/hantro_regmap.h  | 26 +++++-
 3 files changed, 145 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
index 845bef73d218..8592dfabbc5e 100644
--- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
@@ -18,6 +18,9 @@
 #include "hantro_g1_regs.h"
 #include "hantro_hw.h"
 #include "hantro_v4l2.h"
+#include "hantro_regmap.h"
+
+extern struct regmap_config hantro_regmap_dec;
 
 static void set_params(struct hantro_ctx *ctx)
 {
@@ -27,10 +30,15 @@ static void set_params(struct hantro_ctx *ctx)
 	const struct v4l2_ctrl_h264_pps *pps = ctrls->pps;
 	struct vb2_v4l2_buffer *src_buf = hantro_get_src_buf(ctx);
 	struct hantro_dev *vpu = ctx->dev;
+	struct hantro_regmap_fields_dec *fields = vpu->reg_fields_dec;
+	u32 width = MB_WIDTH(ctx->src_fmt.width);
+	u32 height = MB_HEIGHT(ctx->src_fmt.height);
 	u32 reg;
 
+	regmap_field_write(fields->dec_axi_wr_id, 0x0);
+
 	/* Decoder control register 0. */
-	reg = G1_REG_DEC_CTRL0_DEC_AXI_WR_ID(0x0);
+	reg = 0;
 	if (sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD)
 		reg |= G1_REG_DEC_CTRL0_SEQ_MBAFF_E;
 	if (sps->profile_idc > 66) {
@@ -50,10 +58,11 @@ static void set_params(struct hantro_ctx *ctx)
 	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL0);
 
 	/* Decoder control register 1. */
-	reg = G1_REG_DEC_CTRL1_PIC_MB_WIDTH(MB_WIDTH(ctx->src_fmt.width)) |
-	      G1_REG_DEC_CTRL1_PIC_MB_HEIGHT_P(MB_HEIGHT(ctx->src_fmt.height)) |
-	      G1_REG_DEC_CTRL1_REF_FRAMES(sps->max_num_ref_frames);
-	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL1);
+	regmap_field_write(fields->dec_pic_width, width);
+	regmap_field_write(fields->dec_pic_height, height);
+
+	regmap_field_write(fields->dec_num_ref_frames,
+			   sps->max_num_ref_frames);
 
 	/* Decoder control register 2. */
 	reg = G1_REG_DEC_CTRL2_CH_QP_OFFSET(pps->chroma_qp_index_offset) |
@@ -66,10 +75,11 @@ static void set_params(struct hantro_ctx *ctx)
 	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL2);
 
 	/* Decoder control register 3. */
-	reg = G1_REG_DEC_CTRL3_START_CODE_E |
-	      G1_REG_DEC_CTRL3_INIT_QP(pps->pic_init_qp_minus26 + 26) |
-	      G1_REG_DEC_CTRL3_STREAM_LEN(vb2_get_plane_payload(&src_buf->vb2_buf, 0));
-	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL3);
+	regmap_field_write(fields->dec_start_code_e, 1);
+	regmap_field_write(fields->dec_init_qp,
+			   pps->pic_init_qp_minus26 + 26);
+	regmap_field_write(fields->dec_stream_len,
+			   vb2_get_plane_payload(&src_buf->vb2_buf, 0));
 
 	/* Decoder control register 4. */
 	reg = G1_REG_DEC_CTRL4_FRAMENUM_LEN(sps->log2_max_frame_num_minus4 + 4) |
@@ -121,8 +131,7 @@ static void set_params(struct hantro_ctx *ctx)
 	vdpu_write_relaxed(vpu, 0, G1_REG_REF_BUF_CTRL);
 
 	/* Reference picture buffer control register 2. */
-	vdpu_write_relaxed(vpu, G1_REG_REF_BUF_CTRL2_APF_THRESHOLD(8),
-			   G1_REG_REF_BUF_CTRL2);
+	regmap_field_write(fields->dec_apf_threshold, 8);
 }
 
 static void set_ref(struct hantro_ctx *ctx)
@@ -221,7 +230,6 @@ static void set_ref(struct hantro_ctx *ctx)
 	/* Set up addresses of DPB buffers. */
 	for (i = 0; i < HANTRO_H264_DPB_SIZE; i++) {
 		dma_addr_t dma_addr = hantro_h264_get_ref_buf(ctx, i);
-
 		vdpu_write_relaxed(vpu, dma_addr, G1_REG_ADDR_REF(i));
 	}
 }
@@ -231,6 +239,7 @@ static void set_buffers(struct hantro_ctx *ctx)
 	const struct hantro_h264_dec_ctrls *ctrls = &ctx->h264_dec.ctrls;
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
 	struct hantro_dev *vpu = ctx->dev;
+	struct hantro_regmap_fields_dec *fields = vpu->reg_fields_dec;
 	dma_addr_t src_dma, dst_dma;
 	size_t offset = 0;
 
@@ -239,14 +248,14 @@ static void set_buffers(struct hantro_ctx *ctx)
 
 	/* Source (stream) buffer. */
 	src_dma = vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
-	vdpu_write_relaxed(vpu, src_dma, G1_REG_ADDR_STR);
+	regmap_field_write(fields->dec_addr_str, src_dma);
 
 	/* Destination (decoded frame) buffer. */
 	dst_dma = hantro_get_dec_buf_addr(ctx, &dst_buf->vb2_buf);
 	/* Adjust dma addr to start at second line for bottom field */
 	if (ctrls->decode->flags & V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD)
 		offset = ALIGN(ctx->src_fmt.width, MB_DIM);
-	vdpu_write_relaxed(vpu, dst_dma + offset, G1_REG_ADDR_DST);
+	regmap_field_write(fields->dec_addr_dst, dst_dma + offset);
 
 	/* Higher profiles require DMV buffer appended to reference frames. */
 	if (ctrls->sps->profile_idc > 66 && ctrls->decode->nal_ref_idc) {
@@ -266,16 +275,18 @@ static void set_buffers(struct hantro_ctx *ctx)
 		if (ctrls->decode->flags & V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD)
 			offset += 32 * MB_WIDTH(ctx->src_fmt.width) *
 				  MB_HEIGHT(ctx->src_fmt.height);
-		vdpu_write_relaxed(vpu, dst_dma + offset, G1_REG_ADDR_DIR_MV);
+		regmap_field_write(fields->dec_addr_dir_mv, dst_dma + offset);
 	}
 
 	/* Auxiliary buffer prepared in hantro_g1_h264_dec_prepare_table(). */
-	vdpu_write_relaxed(vpu, ctx->h264_dec.priv.dma, G1_REG_ADDR_QTABLE);
+	regmap_field_write(fields->dec_addr_qtable, ctx->h264_dec.priv.dma);
 }
 
 void hantro_g1_h264_dec_run(struct hantro_ctx *ctx)
 {
 	struct hantro_dev *vpu = ctx->dev;
+	struct hantro_regmap_fields_dec *fields = vpu->reg_fields_dec;
+	int reg;
 
 	/* Prepare the H264 decoder context. */
 	if (hantro_h264_dec_prepare_run(ctx))
@@ -288,17 +299,23 @@ void hantro_g1_h264_dec_run(struct hantro_ctx *ctx)
 
 	hantro_end_prepare_run(ctx);
 
+	switch (vpu->core_hw_dec_rev) {
+	case HANTRO_G1_REV:
+		reg = G1_REG_CONFIG_DEC_TIMEOUT_E	|
+			G1_REG_CONFIG_DEC_OUT_ENDIAN	|
+			G1_REG_CONFIG_DEC_STRENDIAN_E	|
+			G1_REG_CONFIG_DEC_OUTSWAP32_E	|
+			G1_REG_CONFIG_DEC_INSWAP32_E	|
+			G1_REG_CONFIG_DEC_STRSWAP32_E;
+		vdpu_write_relaxed(vpu, reg, G1_REG_CONFIG);
+		break;
+	/* TODO: add VC8000 support */
+	}
+
+	regmap_field_write(fields->dec_clk_gate_e, 1);
+	regmap_field_write(fields->dec_max_burst, 16);
+	regmap_field_write(fields->dec_axi_rd_id, 16);
+
 	/* Start decoding! */
-	vdpu_write_relaxed(vpu,
-			   G1_REG_CONFIG_DEC_AXI_RD_ID(0xffu) |
-			   G1_REG_CONFIG_DEC_TIMEOUT_E |
-			   G1_REG_CONFIG_DEC_OUT_ENDIAN |
-			   G1_REG_CONFIG_DEC_STRENDIAN_E |
-			   G1_REG_CONFIG_DEC_MAX_BURST(16) |
-			   G1_REG_CONFIG_DEC_OUTSWAP32_E |
-			   G1_REG_CONFIG_DEC_INSWAP32_E |
-			   G1_REG_CONFIG_DEC_STRSWAP32_E |
-			   G1_REG_CONFIG_DEC_CLK_GATE_E,
-			   G1_REG_CONFIG);
 	vdpu_write(vpu, G1_REG_INTERRUPT_DEC_E, G1_REG_INTERRUPT);
 }
diff --git a/drivers/staging/media/hantro/hantro_regmap.c b/drivers/staging/media/hantro/hantro_regmap.c
index 2fc409cbd797..fbc39abedc7d 100644
--- a/drivers/staging/media/hantro/hantro_regmap.c
+++ b/drivers/staging/media/hantro/hantro_regmap.c
@@ -33,12 +33,54 @@ struct regmap_config hantro_regmap_enc = {
 	.name = "hantro_regmap_enc",
 };
 
+struct hantro_field_dec {
+	struct reg_field cfg_dec_axi_rd_id;
+	struct reg_field cfg_dec_axi_wr_id;
+	struct reg_field cfg_dec_rlc_mode_e;
+	struct reg_field cfg_dec_mode;
+	struct reg_field cfg_dec_max_burst;
+	struct reg_field cfg_dec_apf_threshold;
+	struct reg_field cfg_dec_stream_len;
+	struct reg_field cfg_dec_init_qp;
+	struct reg_field cfg_dec_start_code_e;
+	struct reg_field cfg_dec_pic_width;
+	struct reg_field cfg_dec_pic_height;
+	struct reg_field cfg_dec_num_ref_frames;
+	struct reg_field cfg_dec_scaling_list_e;
+	struct reg_field cfg_dec_addr_str;
+	struct reg_field cfg_dec_addr_dst;
+	struct reg_field cfg_dec_ilace_mode;
+	struct reg_field cfg_dec_addr_qtable;
+	struct reg_field cfg_dec_addr_dir_mv;
+	struct reg_field cfg_dec_tiled_mode_lsb;
+	struct reg_field cfg_dec_clk_gate_e;
+};
+
 struct hantro_field_enc {
 	/* TODO: populate encoder fields */
 };
 
-struct hantro_field_dec {
-	/* TODO: populate decoder fields */
+static const struct hantro_field_dec g1_field = {
+	.cfg_dec_tiled_mode_lsb	=	REG_FIELD(SWREG(2), 7, 7),
+	.cfg_dec_clk_gate_e =		REG_FIELD(SWREG(2), 10, 10),
+	.cfg_dec_axi_rd_id =		REG_FIELD(SWREG(2), 24, 31),
+	.cfg_dec_axi_wr_id =		REG_FIELD(SWREG(3), 0, 7),
+	.cfg_dec_rlc_mode_e =		REG_FIELD(SWREG(3), 27, 27),
+	.cfg_dec_mode =			REG_FIELD(SWREG(3), 28, 31),
+	.cfg_dec_max_burst =		REG_FIELD(SWREG(2), 0, 4),
+	.cfg_dec_apf_threshold =	REG_FIELD(SWREG(55), 0, 13),
+	.cfg_dec_stream_len =		REG_FIELD(SWREG(6), 0, 23),
+	.cfg_dec_init_qp =		REG_FIELD(SWREG(6), 25, 30),
+	.cfg_dec_start_code_e =		REG_FIELD(SWREG(6), 31, 31),
+	.cfg_dec_pic_width =		REG_FIELD(SWREG(4), 23, 31),
+	.cfg_dec_pic_height =		REG_FIELD(SWREG(4), 11, 18),
+	.cfg_dec_num_ref_frames =	REG_FIELD(SWREG(4), 0, 4),
+	.cfg_dec_scaling_list_e =	REG_FIELD(SWREG(5), 24, 24),
+	.cfg_dec_addr_str =		REG_FIELD(SWREG(12), 0, 31),
+	.cfg_dec_addr_dst =		REG_FIELD(SWREG(13), 0, 31),
+	.cfg_dec_ilace_mode =		REG_FIELD(SWREG(13), 1, 1),
+	.cfg_dec_addr_qtable =		REG_FIELD(SWREG(40), 0, 31),
+	.cfg_dec_addr_dir_mv =		REG_FIELD(SWREG(41), 0, 31),
 };
 
 #define INIT_FIELD_CFG(f, codec, conf) ({					\
@@ -61,7 +103,27 @@ static int hantro_regmap_fields_init_dec(struct hantro_dev *vpu,
 	if (!vpu->reg_fields_dec)
 		return -ENOMEM;
 
-	/* TODO: add decoder fields */
+	/* Decoder */
+	INIT_DEC_FIELD(dec_axi_wr_id);
+	INIT_DEC_FIELD(dec_axi_rd_id);
+	INIT_DEC_FIELD(dec_rlc_mode_e);
+	INIT_DEC_FIELD(dec_mode);
+	INIT_DEC_FIELD(dec_max_burst);
+	INIT_DEC_FIELD(dec_apf_threshold);
+	INIT_DEC_FIELD(dec_stream_len);
+	INIT_DEC_FIELD(dec_init_qp);
+	INIT_DEC_FIELD(dec_start_code_e);
+	INIT_DEC_FIELD(dec_pic_width);
+	INIT_DEC_FIELD(dec_pic_height);
+	INIT_DEC_FIELD(dec_num_ref_frames);
+	INIT_DEC_FIELD(dec_scaling_list_e);
+	INIT_DEC_FIELD(dec_addr_str);
+	INIT_DEC_FIELD(dec_addr_dst);
+	INIT_DEC_FIELD(dec_ilace_mode);
+	INIT_DEC_FIELD(dec_addr_qtable);
+	INIT_DEC_FIELD(dec_addr_dir_mv);
+	INIT_DEC_FIELD(dec_tiled_mode_lsb);
+	INIT_DEC_FIELD(dec_clk_gate_e);
 
 	return 0;
 }
@@ -133,6 +195,17 @@ int hantro_regmap_init_dec(struct hantro_dev *vpu)
 
 	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
 
+	switch (vpu->core_hw_dec_rev) {
+	case HANTRO_G1_REV:
+		hantro_regmap_dec.max_register = 0x1D8;
+		field = &g1_field;
+		break;
+	default:
+		dev_err(vpu->dev, "Decoder revision 0x%x not supported by driver.\n",
+			vpu->core_hw_dec_rev);
+		return -ENODEV;
+	}
+
 	vpu->regs_dec = devm_regmap_init_mmio(vpu->dev, dec_base,
 					      &hantro_regmap_dec);
 	if (IS_ERR(vpu->regs_dec)) {
diff --git a/drivers/staging/media/hantro/hantro_regmap.h b/drivers/staging/media/hantro/hantro_regmap.h
index 52668a8bafb9..e94fdc055784 100644
--- a/drivers/staging/media/hantro/hantro_regmap.h
+++ b/drivers/staging/media/hantro/hantro_regmap.h
@@ -9,8 +9,32 @@
 #ifndef HANTRO_REGMAP_H_
 #define HANTRO_REGMAP_H_
 
+#define HANTRO_G1_REV		0x6731
+
+#define SWREG(nr)		((nr) << 2)
+
 struct hantro_regmap_fields_dec {
-	/* TODO: populate decoder fields */
+	/* Decoder */
+	struct regmap_field *dec_axi_rd_id;
+	struct regmap_field *dec_axi_wr_id;
+	struct regmap_field *dec_max_burst;
+	struct regmap_field *dec_rlc_mode_e;
+	struct regmap_field *dec_mode;
+	struct regmap_field *dec_apf_threshold;
+	struct regmap_field *dec_stream_len;
+	struct regmap_field *dec_init_qp;
+	struct regmap_field *dec_start_code_e;
+	struct regmap_field *dec_pic_width;
+	struct regmap_field *dec_pic_height;
+	struct regmap_field *dec_num_ref_frames;
+	struct regmap_field *dec_scaling_list_e;
+	struct regmap_field *dec_addr_str;
+	struct regmap_field *dec_addr_dst;
+	struct regmap_field *dec_ilace_mode;
+	struct regmap_field *dec_addr_qtable;
+	struct regmap_field *dec_addr_dir_mv;
+	struct regmap_field *dec_tiled_mode_lsb;
+	struct regmap_field *dec_clk_gate_e;
 };
 
 struct hantro_regmap_fields_enc {
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 11/18] media: hantro: convert G1 postproc to regmap

[Adrian Ratiu]

Postprocessing used the custom hantro_reg structure but now we have
regmap fields which are used for reg layouts which do the same thing,
so PP can be moved to regmap. In the future all hantro_reg references
can be removed, this is just a beginnig.

This converts only the existing G1 PP support, but the fields can be
used for other core revisions like VC8000D which will be added shortly.

While we're at it also document a few more important PP registers for
eg scaling, cropping and rotation.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/hantro.h         | 19 -----
 drivers/staging/media/hantro/hantro_hw.h      |  2 -
 .../staging/media/hantro/hantro_postproc.c    | 72 +++++-------------
 drivers/staging/media/hantro/hantro_regmap.c  | 75 +++++++++++++++++++
 drivers/staging/media/hantro/hantro_regmap.h  | 26 +++++++
 drivers/staging/media/hantro/imx8m_vpu_hw.c   |  1 -
 drivers/staging/media/hantro/rk3288_vpu_hw.c  |  1 -
 7 files changed, 119 insertions(+), 77 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 5b7fbdc3779d..2d507f8d3a1d 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -71,7 +71,6 @@ 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;
@@ -92,7 +91,6 @@ struct hantro_variant {
 	int num_clocks;
 	const char * const *reg_names;
 	int num_regs;
-	const struct hantro_postproc_regs *postproc_regs;
 };
 
 /**
@@ -283,23 +281,6 @@ 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 */
 
 /**
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index 219283a06f52..e0039a15fe85 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -155,8 +155,6 @@ extern const struct hantro_variant rk3328_vpu_variant;
 extern const struct hantro_variant rk3288_vpu_variant;
 extern const struct hantro_variant imx8mq_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
index 6d2a8f2a8f0b..6d1705a60d36 100644
--- a/drivers/staging/media/hantro/hantro_postproc.c
+++ b/drivers/staging/media/hantro/hantro_postproc.c
@@ -11,20 +11,7 @@
 #include "hantro.h"
 #include "hantro_hw.h"
 #include "hantro_g1_regs.h"
-
-#define HANTRO_PP_REG_WRITE(vpu, reg_name, val) \
-{ \
-	hantro_reg_write(vpu, \
-			 &(vpu)->variant->postproc_regs->reg_name, \
-			 val); \
-}
-
-#define HANTRO_PP_REG_WRITE_S(vpu, reg_name, val) \
-{ \
-	hantro_reg_write_s(vpu, \
-			   &(vpu)->variant->postproc_regs->reg_name, \
-			   val); \
-}
+#include "hantro_regmap.h"
 
 #define VPU_PP_IN_YUYV			0x0
 #define VPU_PP_IN_NV12			0x1
@@ -33,35 +20,15 @@
 #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_enable(struct hantro_ctx *ctx)
 {
-	struct hantro_dev *vpu = ctx->dev;
+	struct hantro_regmap_fields_dec *fields = ctx->dev->reg_fields_dec;
 	struct vb2_v4l2_buffer *dst_buf;
 	u32 src_pp_fmt, dst_pp_fmt;
 	dma_addr_t dst_dma;
 
-	if (!vpu->variant->postproc_regs)
-		return;
-
 	/* Turn on pipeline mode. Must be done first. */
-	HANTRO_PP_REG_WRITE_S(vpu, pipeline_en, 0x1);
+	regmap_field_write(fields->pp_pipeline_en, 1);
 
 	src_pp_fmt = VPU_PP_IN_NV12;
 
@@ -79,19 +46,19 @@ void hantro_postproc_enable(struct hantro_ctx *ctx)
 	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);
+	regmap_field_write(fields->pp_clk_gate, 1);
+	regmap_field_write(fields->pp_out_endian, 1);
+	regmap_field_write(fields->pp_out_swap32, 1);
+	regmap_field_write(fields->pp_max_burst, 16);
+	regmap_field_write(fields->pp_out_luma_base, dst_dma);
+	regmap_field_write(fields->pp_input_width, MB_WIDTH(ctx->dst_fmt.width));
+	regmap_field_write(fields->pp_input_height, MB_HEIGHT(ctx->dst_fmt.height));
+	regmap_field_write(fields->pp_input_fmt, src_pp_fmt);
+	regmap_field_write(fields->pp_output_fmt, dst_pp_fmt);
+	regmap_field_write(fields->pp_output_width, ctx->dst_fmt.width);
+	regmap_field_write(fields->pp_output_height, ctx->dst_fmt.height);
+	regmap_field_write(fields->pp_orig_width, MB_WIDTH(ctx->dst_fmt.width));
+	regmap_field_write(fields->pp_display_width, ctx->dst_fmt.width);
 }
 
 void hantro_postproc_free(struct hantro_ctx *ctx)
@@ -141,10 +108,7 @@ int hantro_postproc_alloc(struct hantro_ctx *ctx)
 
 void hantro_postproc_disable(struct hantro_ctx *ctx)
 {
-	struct hantro_dev *vpu = ctx->dev;
-
-	if (!vpu->variant->postproc_regs)
-		return;
+	struct hantro_regmap_fields_dec *fields = ctx->dev->reg_fields_dec;
 
-	HANTRO_PP_REG_WRITE_S(vpu, pipeline_en, 0x0);
+	regmap_field_write(fields->pp_pipeline_en, 0);
 }
diff --git a/drivers/staging/media/hantro/hantro_regmap.c b/drivers/staging/media/hantro/hantro_regmap.c
index fbc39abedc7d..c0344b0ec8de 100644
--- a/drivers/staging/media/hantro/hantro_regmap.c
+++ b/drivers/staging/media/hantro/hantro_regmap.c
@@ -54,6 +54,31 @@ struct hantro_field_dec {
 	struct reg_field cfg_dec_addr_dir_mv;
 	struct reg_field cfg_dec_tiled_mode_lsb;
 	struct reg_field cfg_dec_clk_gate_e;
+
+	struct reg_field cfg_pp_pipeline_en;
+	struct reg_field cfg_pp_max_burst;
+	struct reg_field cfg_pp_clk_gate;
+	struct reg_field cfg_pp_out_swap32;
+	struct reg_field cfg_pp_out_endian;
+	struct reg_field cfg_pp_out_luma_base;
+	struct reg_field cfg_pp_input_width;
+	struct reg_field cfg_pp_input_height;
+	struct reg_field cfg_pp_output_width;
+	struct reg_field cfg_pp_output_height;
+	struct reg_field cfg_pp_input_fmt;
+	struct reg_field cfg_pp_output_fmt;
+	struct reg_field cfg_pp_orig_width;
+	struct reg_field cfg_pp_display_width;
+	struct reg_field cfg_pp_crop_startx;
+	struct reg_field cfg_pp_crop_starty;
+	struct reg_field cfg_pp_rotation_mode;
+	struct reg_field cfg_pp_fast_scale_e;
+	struct reg_field cfg_pp_vscale_mode;
+	struct reg_field cfg_pp_hscale_mode;
+	struct reg_field cfg_pp_scale_wratio;
+	struct reg_field cfg_pp_scale_hratio;
+	struct reg_field cfg_pp_scale_inv_wratio;
+	struct reg_field cfg_pp_scale_inv_hratio;
 };
 
 struct hantro_field_enc {
@@ -81,6 +106,30 @@ static const struct hantro_field_dec g1_field = {
 	.cfg_dec_ilace_mode =		REG_FIELD(SWREG(13), 1, 1),
 	.cfg_dec_addr_qtable =		REG_FIELD(SWREG(40), 0, 31),
 	.cfg_dec_addr_dir_mv =		REG_FIELD(SWREG(41), 0, 31),
+	.cfg_pp_pipeline_en =		REG_FIELD(SWREG(60), 1, 1),
+	.cfg_pp_max_burst =		REG_FIELD(SWREG(61), 0, 4),
+	.cfg_pp_out_swap32 =		REG_FIELD(SWREG(61), 5, 5),
+	.cfg_pp_out_endian =		REG_FIELD(SWREG(61), 6, 6),
+	.cfg_pp_clk_gate =		REG_FIELD(SWREG(61), 8, 8),
+	.cfg_pp_out_luma_base =		REG_FIELD(SWREG(66), 0, 31),
+	.cfg_pp_input_width =		REG_FIELD(SWREG(72), 0, 8),
+	.cfg_pp_input_height =		REG_FIELD(SWREG(72), 9, 16),
+	.cfg_pp_output_width =		REG_FIELD(SWREG(85), 4, 14),
+	.cfg_pp_output_height =		REG_FIELD(SWREG(85), 15, 25),
+	.cfg_pp_output_fmt =		REG_FIELD(SWREG(85), 26, 28),
+	.cfg_pp_input_fmt =		REG_FIELD(SWREG(85), 29, 31),
+	.cfg_pp_orig_width =		REG_FIELD(SWREG(88), 23, 31),
+	.cfg_pp_display_width =		REG_FIELD(SWREG(92), 0, 12),
+	.cfg_pp_crop_startx =		REG_FIELD(SWREG(71), 21, 29),
+	.cfg_pp_crop_starty =		REG_FIELD(SWREG(72), 24, 31),
+	.cfg_pp_rotation_mode =		REG_FIELD(SWREG(71), 18, 20),
+	.cfg_pp_fast_scale_e =		REG_FIELD(SWREG(80), 30, 30),
+	.cfg_pp_vscale_mode =		REG_FIELD(SWREG(80), 23, 24),
+	.cfg_pp_hscale_mode =		REG_FIELD(SWREG(80), 25, 26),
+	.cfg_pp_scale_wratio =		REG_FIELD(SWREG(79), 0, 17),
+	.cfg_pp_scale_hratio =		REG_FIELD(SWREG(80), 0, 17),
+	.cfg_pp_scale_inv_wratio =	REG_FIELD(SWREG(81), 16, 31),
+	.cfg_pp_scale_inv_hratio =	REG_FIELD(SWREG(81), 0, 15),
 };
 
 #define INIT_FIELD_CFG(f, codec, conf) ({					\
@@ -125,6 +174,32 @@ static int hantro_regmap_fields_init_dec(struct hantro_dev *vpu,
 	INIT_DEC_FIELD(dec_tiled_mode_lsb);
 	INIT_DEC_FIELD(dec_clk_gate_e);
 
+	/* Post-processor */
+	INIT_DEC_FIELD(pp_pipeline_en);
+	INIT_DEC_FIELD(pp_max_burst);
+	INIT_DEC_FIELD(pp_clk_gate);
+	INIT_DEC_FIELD(pp_out_swap32);
+	INIT_DEC_FIELD(pp_out_endian);
+	INIT_DEC_FIELD(pp_out_luma_base);
+	INIT_DEC_FIELD(pp_input_width);
+	INIT_DEC_FIELD(pp_input_height);
+	INIT_DEC_FIELD(pp_output_width);
+	INIT_DEC_FIELD(pp_output_height);
+	INIT_DEC_FIELD(pp_input_fmt);
+	INIT_DEC_FIELD(pp_output_fmt);
+	INIT_DEC_FIELD(pp_orig_width);
+	INIT_DEC_FIELD(pp_display_width);
+	INIT_DEC_FIELD(pp_crop_startx);
+	INIT_DEC_FIELD(pp_crop_starty);
+	INIT_DEC_FIELD(pp_rotation_mode);
+	INIT_DEC_FIELD(pp_fast_scale_e);
+	INIT_DEC_FIELD(pp_vscale_mode);
+	INIT_DEC_FIELD(pp_hscale_mode);
+	INIT_DEC_FIELD(pp_scale_wratio);
+	INIT_DEC_FIELD(pp_scale_hratio);
+	INIT_DEC_FIELD(pp_scale_inv_wratio);
+	INIT_DEC_FIELD(pp_scale_inv_hratio);
+
 	return 0;
 }
 
diff --git a/drivers/staging/media/hantro/hantro_regmap.h b/drivers/staging/media/hantro/hantro_regmap.h
index e94fdc055784..1e6e3b2478ae 100644
--- a/drivers/staging/media/hantro/hantro_regmap.h
+++ b/drivers/staging/media/hantro/hantro_regmap.h
@@ -35,6 +35,32 @@ struct hantro_regmap_fields_dec {
 	struct regmap_field *dec_addr_dir_mv;
 	struct regmap_field *dec_tiled_mode_lsb;
 	struct regmap_field *dec_clk_gate_e;
+
+	/* Post-processor */
+	struct regmap_field *pp_pipeline_en;
+	struct regmap_field *pp_max_burst;
+	struct regmap_field *pp_clk_gate;
+	struct regmap_field *pp_out_swap32;
+	struct regmap_field *pp_out_endian;
+	struct regmap_field *pp_out_luma_base;
+	struct regmap_field *pp_input_width;
+	struct regmap_field *pp_input_height;
+	struct regmap_field *pp_output_width;
+	struct regmap_field *pp_output_height;
+	struct regmap_field *pp_input_fmt;
+	struct regmap_field *pp_output_fmt;
+	struct regmap_field *pp_orig_width;
+	struct regmap_field *pp_display_width;
+	struct regmap_field *pp_crop_startx;
+	struct regmap_field *pp_crop_starty;
+	struct regmap_field *pp_rotation_mode;
+	struct regmap_field *pp_fast_scale_e;
+	struct regmap_field *pp_vscale_mode;
+	struct regmap_field *pp_hscale_mode;
+	struct regmap_field *pp_scale_wratio;
+	struct regmap_field *pp_scale_hratio;
+	struct regmap_field *pp_scale_inv_wratio;
+	struct regmap_field *pp_scale_inv_hratio;
 };
 
 struct hantro_regmap_fields_enc {
diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c b/drivers/staging/media/hantro/imx8m_vpu_hw.c
index b2a401a33992..20394a568f65 100644
--- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
+++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
@@ -179,7 +179,6 @@ const struct hantro_variant imx8mq_vpu_variant = {
 	.num_dec_fmts = ARRAY_SIZE(imx8m_vpu_dec_fmts),
 	.postproc_fmts = imx8m_vpu_postproc_fmts,
 	.num_postproc_fmts = ARRAY_SIZE(imx8m_vpu_postproc_fmts),
-	.postproc_regs = &hantro_g1_postproc_regs,
 	.codec = HANTRO_MPEG2_DECODER | HANTRO_VP8_DECODER |
 		 HANTRO_H264_DECODER,
 	.codec_ops = imx8mq_vpu_codec_ops,
diff --git a/drivers/staging/media/hantro/rk3288_vpu_hw.c b/drivers/staging/media/hantro/rk3288_vpu_hw.c
index 4ad578b1236e..48dea5756098 100644
--- a/drivers/staging/media/hantro/rk3288_vpu_hw.c
+++ b/drivers/staging/media/hantro/rk3288_vpu_hw.c
@@ -226,7 +226,6 @@ const struct hantro_variant rk3288_vpu_variant = {
 	.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.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 12/18] media: hantro: add VC8000D h264 decoding

[Adrian Ratiu]

VC8000D is a newer core combining both previous G1 and G2 cores into
one chip. As a result of this register layouts took a hit but the HW
functions mostly the same, so we can use regmap fields to compensate.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 .../staging/media/hantro/hantro_g1_h264_dec.c | 29 +++++++-
 drivers/staging/media/hantro/hantro_regmap.c  | 69 +++++++++++++++++++
 drivers/staging/media/hantro/hantro_regmap.h  | 22 ++++++
 3 files changed, 117 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
index 8592dfabbc5e..a04cb616d628 100644
--- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
@@ -20,6 +20,8 @@
 #include "hantro_v4l2.h"
 #include "hantro_regmap.h"
 
+#define VC8KD_TIMEOUT 0x500000
+
 extern struct regmap_config hantro_regmap_dec;
 
 static void set_params(struct hantro_ctx *ctx)
@@ -33,10 +35,23 @@ static void set_params(struct hantro_ctx *ctx)
 	struct hantro_regmap_fields_dec *fields = vpu->reg_fields_dec;
 	u32 width = MB_WIDTH(ctx->src_fmt.width);
 	u32 height = MB_HEIGHT(ctx->src_fmt.height);
-	u32 reg;
+	u32 reg, stride;
 
 	regmap_field_write(fields->dec_axi_wr_id, 0x0);
 
+	if (vpu->core_hw_dec_rev == HANTRO_VC8000_REV) {
+		/* stride should be computed in hantro_try_fmt() and set here */
+		stride = width * 4 * 16;
+		regmap_field_write(fields->dec_out_y_stride, stride);
+		regmap_field_write(fields->dec_out_c_stride, stride);
+
+		/* on VC8KD the pic sizes changed from MB to CBS */
+		regmap_field_write(fields->dec_min_cb_size, 3);
+		regmap_field_write(fields->dec_max_cb_size, 4);
+		width <<= 1;
+		height <<= 1;
+	}
+
 	/* Decoder control register 0. */
 	reg = 0;
 	if (sps->flags & V4L2_H264_SPS_FLAG_MB_ADAPTIVE_FRAME_FIELD)
@@ -230,7 +245,7 @@ static void set_ref(struct hantro_ctx *ctx)
 	/* Set up addresses of DPB buffers. */
 	for (i = 0; i < HANTRO_H264_DPB_SIZE; i++) {
 		dma_addr_t dma_addr = hantro_h264_get_ref_buf(ctx, i);
-		vdpu_write_relaxed(vpu, dma_addr, G1_REG_ADDR_REF(i));
+		vdpu_write_relaxed(vpu, dma_addr, REG_ADDR_REF(i));
 	}
 }
 
@@ -309,7 +324,15 @@ void hantro_g1_h264_dec_run(struct hantro_ctx *ctx)
 			G1_REG_CONFIG_DEC_STRSWAP32_E;
 		vdpu_write_relaxed(vpu, reg, G1_REG_CONFIG);
 		break;
-	/* TODO: add VC8000 support */
+	case HANTRO_VC8000_REV:
+		regmap_field_write(fields->dec_ext_timeout_e, 1);
+		regmap_field_write(fields->dec_ext_timeout_cycles, VC8KD_TIMEOUT);
+		regmap_field_write(fields->dec_timeout_e, 1);
+		regmap_field_write(fields->dec_timeout_cycles, VC8KD_TIMEOUT);
+		regmap_field_write(fields->dec_buswidth, 2);
+		regmap_field_write(fields->dec_tab_swap, 3);
+		regmap_field_write(fields->dec_tiled_mode_lsb, 1);
+		break;
 	}
 
 	regmap_field_write(fields->dec_clk_gate_e, 1);
diff --git a/drivers/staging/media/hantro/hantro_regmap.c b/drivers/staging/media/hantro/hantro_regmap.c
index c0344b0ec8de..0e74ba69034f 100644
--- a/drivers/staging/media/hantro/hantro_regmap.c
+++ b/drivers/staging/media/hantro/hantro_regmap.c
@@ -37,8 +37,13 @@ struct hantro_field_dec {
 	struct reg_field cfg_dec_axi_rd_id;
 	struct reg_field cfg_dec_axi_wr_id;
 	struct reg_field cfg_dec_rlc_mode_e;
+	struct reg_field cfg_dec_strm_swap;
+	struct reg_field cfg_dec_pic_swap;
+	struct reg_field cfg_dec_dirmv_swap;
 	struct reg_field cfg_dec_mode;
+	struct reg_field cfg_dec_buffer_empty_int_e;
 	struct reg_field cfg_dec_max_burst;
+	struct reg_field cfg_dec_buswidth;
 	struct reg_field cfg_dec_apf_threshold;
 	struct reg_field cfg_dec_stream_len;
 	struct reg_field cfg_dec_init_qp;
@@ -51,9 +56,18 @@ struct hantro_field_dec {
 	struct reg_field cfg_dec_addr_dst;
 	struct reg_field cfg_dec_ilace_mode;
 	struct reg_field cfg_dec_addr_qtable;
+	struct reg_field cfg_dec_max_cb_size;
+	struct reg_field cfg_dec_min_cb_size;
+	struct reg_field cfg_dec_out_y_stride;
+	struct reg_field cfg_dec_out_c_stride;
 	struct reg_field cfg_dec_addr_dir_mv;
 	struct reg_field cfg_dec_tiled_mode_lsb;
 	struct reg_field cfg_dec_clk_gate_e;
+	struct reg_field cfg_dec_tab_swap;
+	struct reg_field cfg_dec_ext_timeout_cycles;
+	struct reg_field cfg_dec_ext_timeout_e;
+	struct reg_field cfg_dec_timeout_cycles;
+	struct reg_field cfg_dec_timeout_e;
 
 	struct reg_field cfg_pp_pipeline_en;
 	struct reg_field cfg_pp_max_burst;
@@ -132,6 +146,43 @@ static const struct hantro_field_dec g1_field = {
 	.cfg_pp_scale_inv_hratio =	REG_FIELD(SWREG(81), 0, 15),
 };
 
+static const struct hantro_field_dec vc8000d_field = {
+	.cfg_dec_tiled_mode_lsb	=	REG_FIELD(SWREG(2), 7, 7),
+	.cfg_dec_clk_gate_e =		REG_FIELD(SWREG(2), 10, 10),
+	.cfg_dec_tab_swap =		REG_FIELD(SWREG(2), 12, 15),
+	.cfg_dec_axi_rd_id =		REG_FIELD(SWREG(60), 0, 15),
+	.cfg_dec_axi_wr_id =		REG_FIELD(SWREG(60), 16, 31),
+	.cfg_dec_rlc_mode_e =		REG_FIELD(SWREG(3), 24, 24),
+	.cfg_dec_strm_swap =		REG_FIELD(SWREG(2), 28, 31),
+	.cfg_dec_pic_swap =		REG_FIELD(SWREG(2), 24, 27),
+	.cfg_dec_dirmv_swap =		REG_FIELD(SWREG(2), 20, 23),
+	.cfg_dec_mode =			REG_FIELD(SWREG(3), 27, 31),
+	.cfg_dec_buffer_empty_int_e =	REG_FIELD(SWREG(3), 2, 2),
+	.cfg_dec_max_burst =		REG_FIELD(SWREG(58), 0, 7),
+	.cfg_dec_buswidth =		REG_FIELD(SWREG(58), 8, 10),
+	.cfg_dec_apf_threshold =	REG_FIELD(SWREG(55), 0, 15),
+	.cfg_dec_stream_len =		REG_FIELD(SWREG(6), 0, 31),
+	.cfg_dec_init_qp =		REG_FIELD(SWREG(13), 24, 30),
+	.cfg_dec_start_code_e =		REG_FIELD(SWREG(13), 31, 31),
+	.cfg_dec_pic_width =		REG_FIELD(SWREG(4), 19, 31),
+	.cfg_dec_pic_height =		REG_FIELD(SWREG(4), 6, 18),
+	.cfg_dec_num_ref_frames =	REG_FIELD(SWREG(4), 0, 4),
+	.cfg_dec_scaling_list_e =	REG_FIELD(SWREG(5), 24, 24),
+	.cfg_dec_addr_str =		REG_FIELD(SWREG(169), 0, 31),
+	.cfg_dec_addr_dst =		REG_FIELD(SWREG(65), 0, 31),
+	.cfg_dec_ilace_mode =		REG_FIELD(SWREG(65), 1, 1),
+	.cfg_dec_addr_qtable =		REG_FIELD(SWREG(175), 0, 31),
+	.cfg_dec_addr_dir_mv =		REG_FIELD(SWREG(133), 0, 31),
+	.cfg_dec_max_cb_size =		REG_FIELD(SWREG(12), 10, 12),
+	.cfg_dec_min_cb_size =		REG_FIELD(SWREG(12), 13, 15),
+	.cfg_dec_out_y_stride =		REG_FIELD(SWREG(314), 16, 31),
+	.cfg_dec_out_c_stride =		REG_FIELD(SWREG(314), 0, 15),
+	.cfg_dec_ext_timeout_cycles =	REG_FIELD(SWREG(318), 0, 30),
+	.cfg_dec_ext_timeout_e =	REG_FIELD(SWREG(318), 31, 31),
+	.cfg_dec_timeout_cycles =	REG_FIELD(SWREG(319), 0, 30),
+	.cfg_dec_timeout_e =		REG_FIELD(SWREG(319), 31, 31),
+};
+
 #define INIT_FIELD_CFG(f, codec, conf) ({					\
 		vpu->reg_fields_##codec->(f) = devm_regmap_field_alloc(vpu->dev,\
 						     vpu->regs_##codec,		\
@@ -156,8 +207,13 @@ static int hantro_regmap_fields_init_dec(struct hantro_dev *vpu,
 	INIT_DEC_FIELD(dec_axi_wr_id);
 	INIT_DEC_FIELD(dec_axi_rd_id);
 	INIT_DEC_FIELD(dec_rlc_mode_e);
+	INIT_DEC_FIELD(dec_strm_swap);
+	INIT_DEC_FIELD(dec_pic_swap);
+	INIT_DEC_FIELD(dec_dirmv_swap);
 	INIT_DEC_FIELD(dec_mode);
+	INIT_DEC_FIELD(dec_buffer_empty_int_e);
 	INIT_DEC_FIELD(dec_max_burst);
+	INIT_DEC_FIELD(dec_buswidth);
 	INIT_DEC_FIELD(dec_apf_threshold);
 	INIT_DEC_FIELD(dec_stream_len);
 	INIT_DEC_FIELD(dec_init_qp);
@@ -170,9 +226,18 @@ static int hantro_regmap_fields_init_dec(struct hantro_dev *vpu,
 	INIT_DEC_FIELD(dec_addr_dst);
 	INIT_DEC_FIELD(dec_ilace_mode);
 	INIT_DEC_FIELD(dec_addr_qtable);
+	INIT_DEC_FIELD(dec_max_cb_size);
+	INIT_DEC_FIELD(dec_min_cb_size);
+	INIT_DEC_FIELD(dec_out_y_stride);
+	INIT_DEC_FIELD(dec_out_c_stride);
 	INIT_DEC_FIELD(dec_addr_dir_mv);
 	INIT_DEC_FIELD(dec_tiled_mode_lsb);
 	INIT_DEC_FIELD(dec_clk_gate_e);
+	INIT_DEC_FIELD(dec_tab_swap);
+	INIT_DEC_FIELD(dec_ext_timeout_cycles);
+	INIT_DEC_FIELD(dec_ext_timeout_e);
+	INIT_DEC_FIELD(dec_timeout_cycles);
+	INIT_DEC_FIELD(dec_timeout_e);
 
 	/* Post-processor */
 	INIT_DEC_FIELD(pp_pipeline_en);
@@ -275,6 +340,10 @@ int hantro_regmap_init_dec(struct hantro_dev *vpu)
 		hantro_regmap_dec.max_register = 0x1D8;
 		field = &g1_field;
 		break;
+	case HANTRO_VC8000_REV:
+		hantro_regmap_dec.max_register = 0x554;
+		field = &vc8000d_field;
+		break;
 	default:
 		dev_err(vpu->dev, "Decoder revision 0x%x not supported by driver.\n",
 			vpu->core_hw_dec_rev);
diff --git a/drivers/staging/media/hantro/hantro_regmap.h b/drivers/staging/media/hantro/hantro_regmap.h
index 1e6e3b2478ae..8ec0020f40c3 100644
--- a/drivers/staging/media/hantro/hantro_regmap.h
+++ b/drivers/staging/media/hantro/hantro_regmap.h
@@ -10,16 +10,29 @@
 #define HANTRO_REGMAP_H_
 
 #define HANTRO_G1_REV		0x6731
+#define HANTRO_VC8000_REV	0x8001
 
 #define SWREG(nr)		((nr) << 2)
 
+#define SWREG_ITER_G1(n, i)	(SWREG(n) + ((i) << 2))
+#define SWREG_ITER_VC8000(n, i)	(SWREG(n) + ((i) << 3))
+
+#define REG_ADDR_REF(i)		(vpu->core_hw_dec_rev == HANTRO_G1_REV ? \
+				 SWREG_ITER_G1(14, i) : \
+				 SWREG_ITER_VC8000(67, i))
+
 struct hantro_regmap_fields_dec {
 	/* Decoder */
 	struct regmap_field *dec_axi_rd_id;
 	struct regmap_field *dec_axi_wr_id;
 	struct regmap_field *dec_max_burst;
 	struct regmap_field *dec_rlc_mode_e;
+	struct regmap_field *dec_strm_swap;
+	struct regmap_field *dec_pic_swap;
+	struct regmap_field *dec_dirmv_swap;
 	struct regmap_field *dec_mode;
+	struct regmap_field *dec_buffer_empty_int_e;
+	struct regmap_field *dec_buswidth;
 	struct regmap_field *dec_apf_threshold;
 	struct regmap_field *dec_stream_len;
 	struct regmap_field *dec_init_qp;
@@ -32,9 +45,18 @@ struct hantro_regmap_fields_dec {
 	struct regmap_field *dec_addr_dst;
 	struct regmap_field *dec_ilace_mode;
 	struct regmap_field *dec_addr_qtable;
+	struct regmap_field *dec_max_cb_size;
+	struct regmap_field *dec_min_cb_size;
+	struct regmap_field *dec_out_y_stride;
+	struct regmap_field *dec_out_c_stride;
 	struct regmap_field *dec_addr_dir_mv;
 	struct regmap_field *dec_tiled_mode_lsb;
 	struct regmap_field *dec_clk_gate_e;
+	struct regmap_field *dec_tab_swap;
+	struct regmap_field *dec_ext_timeout_cycles;
+	struct regmap_field *dec_ext_timeout_e;
+	struct regmap_field *dec_timeout_cycles;
+	struct regmap_field *dec_timeout_e;
 
 	/* Post-processor */
 	struct regmap_field *pp_pipeline_en;
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 13/18] media: hantro: add VC8000D postproc support

[Adrian Ratiu]

VC8000D decodes only to 4x4 tiled NV12 format and the attached PP
can be used to de-tile its output. This can bo done in two modes:

1. Pipeline mode, using the same decoder "done" irq
2. External mode, with a separate irq and input setup.

This adds the relevant postprocessor fields and support for pipeline
mode de-tiling.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../staging/media/hantro/hantro_postproc.c    | 58 ++++++++++++++++---
 drivers/staging/media/hantro/hantro_regmap.c  | 41 +++++++++++++
 drivers/staging/media/hantro/hantro_regmap.h  |  8 +++
 3 files changed, 98 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
index 6d1705a60d36..a6b3e243dc39 100644
--- a/drivers/staging/media/hantro/hantro_postproc.c
+++ b/drivers/staging/media/hantro/hantro_postproc.c
@@ -20,22 +20,35 @@
 #define VPU_PP_OUT_RGB			0x0
 #define VPU_PP_OUT_YUYV			0x3
 
+#define VC8000D_PP_OUT_NV12		0x0
+
 void hantro_postproc_enable(struct hantro_ctx *ctx)
 {
 	struct hantro_regmap_fields_dec *fields = ctx->dev->reg_fields_dec;
 	struct vb2_v4l2_buffer *dst_buf;
-	u32 src_pp_fmt, dst_pp_fmt;
+	u32 src_pp_fmt, dst_pp_fmt, in_width, in_height;
 	dma_addr_t dst_dma;
 
 	/* Turn on pipeline mode. Must be done first. */
 	regmap_field_write(fields->pp_pipeline_en, 1);
 
+	/*
+	 * use NV12 as input format for pipeline mode as that's what decoder
+	 * outputs, on VC8000D it is 4x4 tiled NV12.
+	 */
 	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;
+	case V4L2_PIX_FMT_NV12:
+		/* src == dst == NV12 only makes sense to de-tile on VC8000D */
+		if (ctx->dev->core_hw_dec_rev == HANTRO_VC8000_REV) {
+			dst_pp_fmt = VC8000D_PP_OUT_NV12;
+			break;
+		}
+		fallthrough;
 	default:
 		WARN(1, "output format %d not supported by the post-processor, this wasn't expected.",
 		     ctx->vpu_dst_fmt->fourcc);
@@ -46,19 +59,46 @@ void hantro_postproc_enable(struct hantro_ctx *ctx)
 	dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
 	dst_dma = vb2_dma_contig_plane_dma_addr(&dst_buf->vb2_buf, 0);
 
-	regmap_field_write(fields->pp_clk_gate, 1);
-	regmap_field_write(fields->pp_out_endian, 1);
-	regmap_field_write(fields->pp_out_swap32, 1);
-	regmap_field_write(fields->pp_max_burst, 16);
+	switch (ctx->dev->core_hw_dec_rev) {
+	case HANTRO_G1_REV:
+		regmap_field_write(fields->pp_clk_gate, 1);
+		regmap_field_write(fields->pp_out_endian, 1);
+		regmap_field_write(fields->pp_out_swap32, 1);
+		regmap_field_write(fields->pp_max_burst, 16);
+		regmap_field_write(fields->pp_orig_width, MB_WIDTH(ctx->dst_fmt.width));
+		regmap_field_write(fields->pp_display_width, ctx->dst_fmt.width);
+		in_width = MB_WIDTH(ctx->src_fmt.width);
+		in_height = MB_WIDTH(ctx->src_fmt.height);
+		break;
+	case HANTRO_VC8000_REV:
+		/* on VC8000D the PP is used to de-tile decoder output */
+		regmap_field_write(fields->pp_out_tile_e, 0);
+
+		regmap_field_write(fields->pp_out_y_stride, ctx->dst_fmt.width);
+		regmap_field_write(fields->pp_out_c_stride, ctx->dst_fmt.width);
+
+		regmap_field_write(fields->pp_out_chroma_base, dst_dma +
+				   ctx->dst_fmt.width * ctx->dst_fmt.height);
+
+		/* VC8000D input resolution is a 2-pixels length. */
+		in_width = ctx->src_fmt.width / 2;
+		in_height = ctx->src_fmt.height / 2;
+
+		break;
+	default:
+		vpu_err("PP does not recognize HW revision: %x, disabling\n",
+			ctx->dev->core_hw_dec_rev);
+		hantro_postproc_disable(ctx);
+		return;
+	}
+
 	regmap_field_write(fields->pp_out_luma_base, dst_dma);
-	regmap_field_write(fields->pp_input_width, MB_WIDTH(ctx->dst_fmt.width));
-	regmap_field_write(fields->pp_input_height, MB_HEIGHT(ctx->dst_fmt.height));
+	regmap_field_write(fields->pp_input_width, in_width);
+	regmap_field_write(fields->pp_input_height, in_height);
 	regmap_field_write(fields->pp_input_fmt, src_pp_fmt);
 	regmap_field_write(fields->pp_output_fmt, dst_pp_fmt);
 	regmap_field_write(fields->pp_output_width, ctx->dst_fmt.width);
 	regmap_field_write(fields->pp_output_height, ctx->dst_fmt.height);
-	regmap_field_write(fields->pp_orig_width, MB_WIDTH(ctx->dst_fmt.width));
-	regmap_field_write(fields->pp_display_width, ctx->dst_fmt.width);
 }
 
 void hantro_postproc_free(struct hantro_ctx *ctx)
diff --git a/drivers/staging/media/hantro/hantro_regmap.c b/drivers/staging/media/hantro/hantro_regmap.c
index 0e74ba69034f..b87fe809f2f7 100644
--- a/drivers/staging/media/hantro/hantro_regmap.c
+++ b/drivers/staging/media/hantro/hantro_regmap.c
@@ -75,6 +75,10 @@ struct hantro_field_dec {
 	struct reg_field cfg_pp_out_swap32;
 	struct reg_field cfg_pp_out_endian;
 	struct reg_field cfg_pp_out_luma_base;
+	struct reg_field cfg_pp_out_chroma_base;
+	struct reg_field cfg_pp_in_luma_base;
+	struct reg_field cfg_pp_in_chroma_base;
+	struct reg_field cfg_pp_out_tile_e;
 	struct reg_field cfg_pp_input_width;
 	struct reg_field cfg_pp_input_height;
 	struct reg_field cfg_pp_output_width;
@@ -93,6 +97,10 @@ struct hantro_field_dec {
 	struct reg_field cfg_pp_scale_hratio;
 	struct reg_field cfg_pp_scale_inv_wratio;
 	struct reg_field cfg_pp_scale_inv_hratio;
+	struct reg_field cfg_pp_out_c_stride;
+	struct reg_field cfg_pp_out_y_stride;
+	struct reg_field cfg_pp_in_c_stride;
+	struct reg_field cfg_pp_in_y_stride;
 };
 
 struct hantro_field_enc {
@@ -181,6 +189,31 @@ static const struct hantro_field_dec vc8000d_field = {
 	.cfg_dec_ext_timeout_e =	REG_FIELD(SWREG(318), 31, 31),
 	.cfg_dec_timeout_cycles =	REG_FIELD(SWREG(319), 0, 30),
 	.cfg_dec_timeout_e =		REG_FIELD(SWREG(319), 31, 31),
+	.cfg_pp_pipeline_en =		REG_FIELD(SWREG(320), 0, 0),
+	.cfg_pp_out_tile_e =		REG_FIELD(SWREG(320), 3, 3),
+	.cfg_pp_output_fmt =		REG_FIELD(SWREG(322), 18, 22),
+	.cfg_pp_input_fmt =		REG_FIELD(SWREG(322), 27, 31),
+	.cfg_pp_out_luma_base =		REG_FIELD(SWREG(326), 0, 31),
+	.cfg_pp_out_chroma_base =	REG_FIELD(SWREG(328), 0, 31),
+	.cfg_pp_in_luma_base =		REG_FIELD(SWREG(339), 0, 31),
+	.cfg_pp_in_chroma_base =	REG_FIELD(SWREG(341), 0, 31),
+	.cfg_pp_input_height =		REG_FIELD(SWREG(331), 0, 15),
+	.cfg_pp_input_width =		REG_FIELD(SWREG(331), 16, 31),
+	.cfg_pp_output_height =		REG_FIELD(SWREG(332), 0, 15),
+	.cfg_pp_output_width =		REG_FIELD(SWREG(332), 16, 31),
+	.cfg_pp_crop_starty =		REG_FIELD(SWREG(330), 0, 12),
+	.cfg_pp_rotation_mode =		REG_FIELD(SWREG(330), 13, 14),
+	.cfg_pp_crop_startx =		REG_FIELD(SWREG(330), 16, 28),
+	.cfg_pp_vscale_mode =		REG_FIELD(SWREG(322), 23, 24),
+	.cfg_pp_hscale_mode =		REG_FIELD(SWREG(322), 25, 26),
+	.cfg_pp_scale_wratio =		REG_FIELD(SWREG(323), 0, 17),
+	.cfg_pp_scale_hratio =		REG_FIELD(SWREG(322), 0, 17),
+	.cfg_pp_scale_inv_wratio =	REG_FIELD(SWREG(324), 16, 31),
+	.cfg_pp_scale_inv_hratio =	REG_FIELD(SWREG(324), 0, 15),
+	.cfg_pp_out_c_stride =		REG_FIELD(SWREG(329), 0, 15),
+	.cfg_pp_out_y_stride =		REG_FIELD(SWREG(329), 16, 31),
+	.cfg_pp_in_c_stride =		REG_FIELD(SWREG(337), 0, 15),
+	.cfg_pp_in_y_stride =		REG_FIELD(SWREG(337), 16, 31),
 };
 
 #define INIT_FIELD_CFG(f, codec, conf) ({					\
@@ -246,6 +279,14 @@ static int hantro_regmap_fields_init_dec(struct hantro_dev *vpu,
 	INIT_DEC_FIELD(pp_out_swap32);
 	INIT_DEC_FIELD(pp_out_endian);
 	INIT_DEC_FIELD(pp_out_luma_base);
+	INIT_DEC_FIELD(pp_out_chroma_base);
+	INIT_DEC_FIELD(pp_out_tile_e);
+	INIT_DEC_FIELD(pp_out_c_stride);
+	INIT_DEC_FIELD(pp_out_y_stride);
+	INIT_DEC_FIELD(pp_in_c_stride);
+	INIT_DEC_FIELD(pp_in_y_stride);
+	INIT_DEC_FIELD(pp_in_luma_base);
+	INIT_DEC_FIELD(pp_in_chroma_base);
 	INIT_DEC_FIELD(pp_input_width);
 	INIT_DEC_FIELD(pp_input_height);
 	INIT_DEC_FIELD(pp_output_width);
diff --git a/drivers/staging/media/hantro/hantro_regmap.h b/drivers/staging/media/hantro/hantro_regmap.h
index 8ec0020f40c3..0a39bae83f85 100644
--- a/drivers/staging/media/hantro/hantro_regmap.h
+++ b/drivers/staging/media/hantro/hantro_regmap.h
@@ -65,6 +65,10 @@ struct hantro_regmap_fields_dec {
 	struct regmap_field *pp_out_swap32;
 	struct regmap_field *pp_out_endian;
 	struct regmap_field *pp_out_luma_base;
+	struct regmap_field *pp_out_chroma_base;
+	struct regmap_field *pp_in_luma_base;
+	struct regmap_field *pp_in_chroma_base;
+	struct regmap_field *pp_out_tile_e;
 	struct regmap_field *pp_input_width;
 	struct regmap_field *pp_input_height;
 	struct regmap_field *pp_output_width;
@@ -83,6 +87,10 @@ struct hantro_regmap_fields_dec {
 	struct regmap_field *pp_scale_hratio;
 	struct regmap_field *pp_scale_inv_wratio;
 	struct regmap_field *pp_scale_inv_hratio;
+	struct regmap_field *pp_out_y_stride;
+	struct regmap_field *pp_out_c_stride;
+	struct regmap_field *pp_in_y_stride;
+	struct regmap_field *pp_in_c_stride;
 };
 
 struct hantro_regmap_fields_enc {
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 14/18] media: hantro: make PP enablement logic a bit smarter

[Adrian Ratiu]

Now that we support two cores with different PP operations we need
to make the condition to enable PP a bit smarter based on what is
actually supported by each core.

While doing this also move the needs_postproc() test inside the
postproc .c file instead of cluttering the header.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/hantro.h         | 10 ++-----
 .../staging/media/hantro/hantro_postproc.c    | 29 +++++++++++++++++++
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 2d507f8d3a1d..05e59bc83b71 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -393,6 +393,9 @@ static inline void hantro_reg_write_s(struct hantro_dev *vpu,
 	vdpu_write(vpu, vdpu_read_mask(vpu, reg, val), reg->base);
 }
 
+bool hantro_needs_postproc(const struct hantro_ctx *ctx,
+			   const struct hantro_fmt *fmt);
+
 void *hantro_get_ctrl(struct hantro_ctx *ctx, u32 id);
 dma_addr_t hantro_get_ref(struct hantro_ctx *ctx, u64 ts);
 
@@ -408,13 +411,6 @@ hantro_get_dst_buf(struct hantro_ctx *ctx)
 	return v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
 }
 
-static inline bool
-hantro_needs_postproc(const struct hantro_ctx *ctx,
-		      const struct hantro_fmt *fmt)
-{
-	return !ctx->is_encoder && fmt->fourcc != V4L2_PIX_FMT_NV12;
-}
-
 static inline dma_addr_t
 hantro_get_dec_buf_addr(struct hantro_ctx *ctx, struct vb2_buffer *vb)
 {
diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
index a6b3e243dc39..653bae37eed9 100644
--- a/drivers/staging/media/hantro/hantro_postproc.c
+++ b/drivers/staging/media/hantro/hantro_postproc.c
@@ -22,6 +22,35 @@
 
 #define VC8000D_PP_OUT_NV12		0x0
 
+bool hantro_needs_postproc(const struct hantro_ctx *ctx,
+			   const struct hantro_fmt *fmt)
+{
+	bool ret = false;
+
+	/* postproc is only available for decoders */
+	if (ctx->is_encoder)
+		return false;
+
+	switch (ctx->dev->core_hw_dec_rev) {
+	case HANTRO_G1_REV:
+		/*
+		 * for now the G1 PP is only used for NV12 -> YUYV conversion
+		 * so if the dst format is already NV12 we don't need it
+		 */
+		ret = fmt->fourcc != V4L2_PIX_FMT_NV12;
+		break;
+	case HANTRO_VC8000_REV:
+		/*
+		 * for now the VC8000D PP is only used to de-tile 4x4 NV12, so
+		 * enabling it for something else doesn't make sense.
+		 */
+		ret = fmt->fourcc == V4L2_PIX_FMT_NV12;
+		break;
+	}
+
+	return ret;
+}
+
 void hantro_postproc_enable(struct hantro_ctx *ctx)
 {
 	struct hantro_regmap_fields_dec *fields = ctx->dev->reg_fields_dec;
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 15/18] media: hantro: add user-selectable, platform-selectable H264 High10

[Adrian Ratiu]

VPU cores starting with VC8000D feature a separate decoding mode named
"high10", capable of decoding both 8bit and 10bit streams, alongside the
previous (still supported) "normal / classic" h264 decoding mode.

The new kernel module param h264_high10 can be used to switch modes,
otherwise the driver will use the platform configured default.

Currently only 8bit decoding is implemented in the high10 mode.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/hantro.h         |   7 +
 drivers/staging/media/hantro/hantro_drv.c     |  10 ++
 .../staging/media/hantro/hantro_g1_h264_dec.c | 142 ++++++++++++++----
 drivers/staging/media/hantro/hantro_hw.h      |  21 ++-
 .../staging/media/hantro/hantro_postproc.c    |   3 +-
 drivers/staging/media/hantro/hantro_regmap.c  |  36 +++++
 drivers/staging/media/hantro/hantro_regmap.h  |  17 +++
 drivers/staging/media/hantro/hantro_v4l2.c    |   3 +-
 8 files changed, 203 insertions(+), 36 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 05e59bc83b71..70aeb11b1149 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -71,6 +71,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
+ * @has_h264_high10:		platform has support for high10 decoding mode
  */
 struct hantro_variant {
 	unsigned int enc_offset;
@@ -91,6 +92,8 @@ struct hantro_variant {
 	int num_clocks;
 	const char * const *reg_names;
 	int num_regs;
+
+	bool has_h264_high10;
 };
 
 /**
@@ -177,6 +180,8 @@ hantro_vdev_to_func(struct video_device *vdev)
  *			shared with interrupt handlers.
  * @variant:		Hardware variant-specific parameters.
  * @watchdog_work:	Delayed work for hardware timeout handling.
+ *
+ * @h264_hw_mode:	H264 mode: legacy, high10 supported.
  */
 struct hantro_dev {
 	struct v4l2_device v4l2_dev;
@@ -200,6 +205,8 @@ struct hantro_dev {
 	spinlock_t irqlock;
 	const struct hantro_variant *variant;
 	struct delayed_work watchdog_work;
+
+	enum hantro_h264_hw_mode h264_hw_mode;
 };
 
 /**
diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index e225515d6985..afb4e201fa42 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -32,6 +32,10 @@
 
 #define DRIVER_NAME "hantro-vpu"
 
+static bool hantro_h264_high10 = true;
+module_param_named(h264_high10, hantro_h264_high10, bool, 0444);
+MODULE_PARM_DESC(h264_high10, "Enable High10 decoding mode");
+
 int hantro_debug;
 module_param_named(debug, hantro_debug, int, 0644);
 MODULE_PARM_DESC(debug,
@@ -824,6 +828,12 @@ static int hantro_probe(struct platform_device *pdev)
 		goto err_clk_unprepare;
 	}
 
+	/* Small quirk: check if H264 High10 mode can be used */
+	if (hantro_h264_high10 && vpu->variant->has_h264_high10)
+		vpu->h264_hw_mode = HANTRO_H264_HIGH10;
+	else
+		vpu->h264_hw_mode = HANTRO_H264_LEGACY;
+
 	pm_runtime_set_autosuspend_delay(vpu->dev, 100);
 	pm_runtime_use_autosuspend(vpu->dev);
 	pm_runtime_enable(vpu->dev);
diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
index a04cb616d628..e64b59c84111 100644
--- a/drivers/staging/media/hantro/hantro_g1_h264_dec.c
+++ b/drivers/staging/media/hantro/hantro_g1_h264_dec.c
@@ -2,6 +2,8 @@
 /*
  * Rockchip RK3288 VPU codec driver
  *
+ * Copyright (c) 2020 Collabora, Ltd.
+ *
  * Copyright (c) 2014 Rockchip Electronics Co., Ltd.
  *	Hertz Wong <hertz.wong@rock-chips.com>
  *	Herman Chen <herman.chen@rock-chips.com>
@@ -10,6 +12,7 @@
  *	Tomasz Figa <tfiga@chromium.org>
  */
 
+#include <linux/moduleparam.h>
 #include <linux/types.h>
 #include <linux/sort.h>
 
@@ -20,6 +23,8 @@
 #include "hantro_v4l2.h"
 #include "hantro_regmap.h"
 
+/* TODO: remove this harcoded pixel size when adding 10bit streams */
+#define VC8KD_PIXEL_SIZE 8
 #define VC8KD_TIMEOUT 0x500000
 
 extern struct regmap_config hantro_regmap_dec;
@@ -30,7 +35,6 @@ static void set_params(struct hantro_ctx *ctx)
 	const struct v4l2_ctrl_h264_decode_params *dec_param = ctrls->decode;
 	const struct v4l2_ctrl_h264_sps *sps = ctrls->sps;
 	const struct v4l2_ctrl_h264_pps *pps = ctrls->pps;
-	struct vb2_v4l2_buffer *src_buf = hantro_get_src_buf(ctx);
 	struct hantro_dev *vpu = ctx->dev;
 	struct hantro_regmap_fields_dec *fields = vpu->reg_fields_dec;
 	u32 width = MB_WIDTH(ctx->src_fmt.width);
@@ -40,8 +44,26 @@ static void set_params(struct hantro_ctx *ctx)
 	regmap_field_write(fields->dec_axi_wr_id, 0x0);
 
 	if (vpu->core_hw_dec_rev == HANTRO_VC8000_REV) {
-		/* stride should be computed in hantro_try_fmt() and set here */
-		stride = width * 4 * 16;
+		/*
+		 * TODO: For now we only support 8bit pixel depth even in high10
+		 * decoding mode, so this is why PIXEL_SIZE is always defined 8
+		 */
+		regmap_field_write(fields->dec_bit_depth_c_minus8,
+				   VC8KD_PIXEL_SIZE - 8);
+		regmap_field_write(fields->dec_bit_depth_y_minus8,
+				   VC8KD_PIXEL_SIZE - 8);
+
+		regmap_field_write(fields->dec_pic_height_4x4,
+				   ctx->src_fmt.height / 4);
+		regmap_field_write(fields->dec_pic_width_4x4,
+				   ctx->src_fmt.width / 4);
+
+		/*
+		 * This depends on tiled_stride_enable.
+		 * It's a weird math, we still don't know
+		 * what's the rationale.
+		 */
+		stride = width * MB_DIM * 4;
 		regmap_field_write(fields->dec_out_y_stride, stride);
 		regmap_field_write(fields->dec_out_c_stride, stride);
 
@@ -93,8 +115,6 @@ static void set_params(struct hantro_ctx *ctx)
 	regmap_field_write(fields->dec_start_code_e, 1);
 	regmap_field_write(fields->dec_init_qp,
 			   pps->pic_init_qp_minus26 + 26);
-	regmap_field_write(fields->dec_stream_len,
-			   vb2_get_plane_payload(&src_buf->vb2_buf, 0));
 
 	/* Decoder control register 4. */
 	reg = G1_REG_DEC_CTRL4_FRAMENUM_LEN(sps->log2_max_frame_num_minus4 + 4) |
@@ -111,8 +131,7 @@ static void set_params(struct hantro_ctx *ctx)
 	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL4);
 
 	/* Decoder control register 5. */
-	reg = G1_REG_DEC_CTRL5_REFPIC_MK_LEN(dec_param->dec_ref_pic_marking_bit_size) |
-	      G1_REG_DEC_CTRL5_IDR_PIC_ID(dec_param->idr_pic_id);
+	reg = G1_REG_DEC_CTRL5_REFPIC_MK_LEN(dec_param->dec_ref_pic_marking_bit_size);
 	if (pps->flags & V4L2_H264_PPS_FLAG_CONSTRAINED_INTRA_PRED)
 		reg |= G1_REG_DEC_CTRL5_CONST_INTRA_E;
 	if (pps->flags & V4L2_H264_PPS_FLAG_DEBLOCKING_FILTER_CONTROL_PRESENT)
@@ -125,6 +144,8 @@ static void set_params(struct hantro_ctx *ctx)
 		reg |= G1_REG_DEC_CTRL5_IDR_PIC_E;
 	vdpu_write_relaxed(vpu, reg, G1_REG_DEC_CTRL5);
 
+	regmap_field_write(fields->dec_idr_pic_id_h10, dec_param->idr_pic_id);
+
 	/* Decoder control register 6. */
 	reg = G1_REG_DEC_CTRL6_PPS_ID(pps->pic_parameter_set_id) |
 	      G1_REG_DEC_CTRL6_REFIDX0_ACTIVE(pps->num_ref_idx_l0_default_active_minus1 + 1) |
@@ -149,11 +170,46 @@ static void set_params(struct hantro_ctx *ctx)
 	regmap_field_write(fields->dec_apf_threshold, 8);
 }
 
+static size_t get_mv_offset(struct hantro_ctx *ctx)
+{
+	const struct hantro_h264_dec_ctrls *ctrls = &ctx->h264_dec.ctrls;
+	u32 bytes_per_mb = 384;
+	size_t mv_offset = 0;
+
+	/* DMV buffer for monochrome start directly after Y-plane */
+	if (ctrls->sps->profile_idc >= 100 &&
+	    ctrls->sps->chroma_format_idc == 0)
+		bytes_per_mb = 256;
+
+	mv_offset = bytes_per_mb * MB_WIDTH(ctx->src_fmt.width) *
+		MB_HEIGHT(ctx->src_fmt.height) * (VC8KD_PIXEL_SIZE / 8);
+
+	/*
+	 * Allocate 32 bytes for multicore status fields
+	 * locate it after picture and before direct MV.
+	 * TODO: This should be constrained to multicore?
+	 */
+	mv_offset += 32;
+
+	/*
+	 * DMV buffer is split in two for field encoded frames,
+	 * adjust offset for bottom field
+	 */
+	if (ctrls->decode->flags & V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD)
+		mv_offset += 32 * MB_WIDTH(ctx->src_fmt.width) *
+			MB_HEIGHT(ctx->src_fmt.height);
+
+	return mv_offset;
+}
+
 static void set_ref(struct hantro_ctx *ctx)
 {
 	struct v4l2_h264_dpb_entry *dpb = ctx->h264_dec.dpb;
 	const u8 *b0_reflist, *b1_reflist, *p_reflist;
 	struct hantro_dev *vpu = ctx->dev;
+	const struct hantro_h264_dec_ctrls *ctrls = &ctx->h264_dec.ctrls;
+	struct hantro_regmap_fields_dec *fields = vpu->reg_fields_dec;
+	bool do_high10 = (vpu->h264_hw_mode == HANTRO_H264_HIGH10);
 	u32 dpb_longterm = 0;
 	u32 dpb_valid = 0;
 	int reg_num;
@@ -200,7 +256,7 @@ static void set_ref(struct hantro_ctx *ctx)
 	p_reflist = ctx->h264_dec.reflists.p;
 
 	/*
-	 * Each G1_REG_BD_REF_PIC(x) register contains three entries
+	 * Each REG_BD_REF_PIC(x) register contains three entries
 	 * of each forward and backward picture list.
 	 */
 	reg_num = 0;
@@ -211,7 +267,7 @@ static void set_ref(struct hantro_ctx *ctx)
 		      G1_REG_BD_REF_PIC_BINIT_RLIST_B0(b1_reflist[i]) |
 		      G1_REG_BD_REF_PIC_BINIT_RLIST_B1(b1_reflist[i + 1]) |
 		      G1_REG_BD_REF_PIC_BINIT_RLIST_B2(b1_reflist[i + 2]);
-		vdpu_write_relaxed(vpu, reg, G1_REG_BD_REF_PIC(reg_num++));
+		vdpu_write_relaxed(vpu, reg, REG_BD_REF_PIC(reg_num++));
 	}
 
 	/*
@@ -219,14 +275,19 @@ static void set_ref(struct hantro_ctx *ctx)
 	 * of forward and backward reference picture lists and first 4 entries
 	 * of P forward picture list.
 	 */
-	reg = G1_REG_BD_P_REF_PIC_BINIT_RLIST_F15(b0_reflist[15]) |
-	      G1_REG_BD_P_REF_PIC_BINIT_RLIST_B15(b1_reflist[15]) |
-	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F0(p_reflist[0]) |
+	reg = G1_REG_BD_P_REF_PIC_PINIT_RLIST_F0(p_reflist[0]) |
 	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F1(p_reflist[1]) |
 	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F2(p_reflist[2]) |
 	      G1_REG_BD_P_REF_PIC_PINIT_RLIST_F3(p_reflist[3]);
 	vdpu_write_relaxed(vpu, reg, G1_REG_BD_P_REF_PIC);
 
+	/*
+	 * The last fw/bw refpic lists (index 15) have actually moved register
+	 * locations between decoder revisions, so set them using regmap fields
+	 */
+	regmap_field_write(fields->dec_init_rlist_f15, b0_reflist[15]);
+	regmap_field_write(fields->dec_init_rlist_b15, b1_reflist[15]);
+
 	/*
 	 * Each G1_REG_FWD_PIC(x) register contains six consecutive
 	 * entries of P forward picture list, starting from index 4.
@@ -246,6 +307,22 @@ static void set_ref(struct hantro_ctx *ctx)
 	for (i = 0; i < HANTRO_H264_DPB_SIZE; i++) {
 		dma_addr_t dma_addr = hantro_h264_get_ref_buf(ctx, i);
 		vdpu_write_relaxed(vpu, dma_addr, REG_ADDR_REF(i));
+
+		if (vpu->core_hw_dec_rev == HANTRO_VC8000_REV) {
+			if (ctrls->sps->profile_idc > 66) {
+				size_t mv_offset = get_mv_offset(ctx);
+
+				vdpu_write_relaxed(vpu, dma_addr + mv_offset,
+						   REG_DMV_REF(i));
+			}
+
+			if (do_high10) {
+				size_t chroma_offset = ctx->src_fmt.width *
+					ctx->src_fmt.height;
+				vdpu_write_relaxed(vpu, dma_addr + chroma_offset,
+						   REG_CHR_REF(i));
+			}
+		}
 	}
 }
 
@@ -255,15 +332,26 @@ static void set_buffers(struct hantro_ctx *ctx)
 	struct vb2_v4l2_buffer *src_buf, *dst_buf;
 	struct hantro_dev *vpu = ctx->dev;
 	struct hantro_regmap_fields_dec *fields = vpu->reg_fields_dec;
+	bool do_high10 = (vpu->h264_hw_mode == HANTRO_H264_HIGH10);
 	dma_addr_t src_dma, dst_dma;
 	size_t offset = 0;
+	u32 src_len, src_buf_len;
 
 	src_buf = hantro_get_src_buf(ctx);
 	dst_buf = hantro_get_dst_buf(ctx);
 
 	/* Source (stream) buffer. */
 	src_dma = vb2_dma_contig_plane_dma_addr(&src_buf->vb2_buf, 0);
+	src_len = vb2_get_plane_payload(&src_buf->vb2_buf, 0);
+
 	regmap_field_write(fields->dec_addr_str, src_dma);
+	regmap_field_write(fields->dec_stream_len, src_len);
+
+	if (do_high10) {
+		src_buf_len = vb2_plane_size(&src_buf->vb2_buf, 0);
+		regmap_field_write(fields->dec_strm_buffer_len, src_buf_len);
+		regmap_field_write(fields->dec_strm_start_offset, 0);
+	}
 
 	/* Destination (decoded frame) buffer. */
 	dst_dma = hantro_get_dec_buf_addr(ctx, &dst_buf->vb2_buf);
@@ -272,27 +360,18 @@ static void set_buffers(struct hantro_ctx *ctx)
 		offset = ALIGN(ctx->src_fmt.width, MB_DIM);
 	regmap_field_write(fields->dec_addr_dst, dst_dma + offset);
 
-	/* Higher profiles require DMV buffer appended to reference frames. */
-	if (ctrls->sps->profile_idc > 66 && ctrls->decode->nal_ref_idc) {
-		unsigned int bytes_per_mb = 384;
-
-		/* DMV buffer for monochrome start directly after Y-plane */
-		if (ctrls->sps->profile_idc >= 100 &&
-		    ctrls->sps->chroma_format_idc == 0)
-			bytes_per_mb = 256;
-		offset = bytes_per_mb * MB_WIDTH(ctx->src_fmt.width) *
-			 MB_HEIGHT(ctx->src_fmt.height);
+	if (do_high10) {
+		size_t chroma_offset = ctx->src_fmt.width * ctx->src_fmt.height;
 
-		/*
-		 * DMV buffer is split in two for field encoded frames,
-		 * adjust offset for bottom field
-		 */
-		if (ctrls->decode->flags & V4L2_H264_DECODE_PARAM_FLAG_BOTTOM_FIELD)
-			offset += 32 * MB_WIDTH(ctx->src_fmt.width) *
-				  MB_HEIGHT(ctx->src_fmt.height);
-		regmap_field_write(fields->dec_addr_dir_mv, dst_dma + offset);
+		regmap_field_write(fields->dec_addr_dst_chr,
+				   dst_dma + chroma_offset);
 	}
 
+	/* Higher profiles require DMV buffer appended to reference frames. */
+	if (ctrls->sps->profile_idc > 66 && ctrls->decode->nal_ref_idc)
+		regmap_field_write(fields->dec_addr_dir_mv,
+				   dst_dma + get_mv_offset(ctx));
+
 	/* Auxiliary buffer prepared in hantro_g1_h264_dec_prepare_table(). */
 	regmap_field_write(fields->dec_addr_qtable, ctx->h264_dec.priv.dma);
 }
@@ -301,6 +380,7 @@ void hantro_g1_h264_dec_run(struct hantro_ctx *ctx)
 {
 	struct hantro_dev *vpu = ctx->dev;
 	struct hantro_regmap_fields_dec *fields = vpu->reg_fields_dec;
+	bool do_high10 = (vpu->h264_hw_mode == HANTRO_H264_HIGH10);
 	int reg;
 
 	/* Prepare the H264 decoder context. */
@@ -332,6 +412,8 @@ void hantro_g1_h264_dec_run(struct hantro_ctx *ctx)
 		regmap_field_write(fields->dec_buswidth, 2);
 		regmap_field_write(fields->dec_tab_swap, 3);
 		regmap_field_write(fields->dec_tiled_mode_lsb, 1);
+		regmap_field_write(fields->dec_ref_compress_bypass, 1);
+		regmap_field_write(fields->dec_mode, do_high10 ? 0xf : 0x0);
 		break;
 	}
 
diff --git a/drivers/staging/media/hantro/hantro_hw.h b/drivers/staging/media/hantro/hantro_hw.h
index e0039a15fe85..7a79d6c8e4e1 100644
--- a/drivers/staging/media/hantro/hantro_hw.h
+++ b/drivers/staging/media/hantro/hantro_hw.h
@@ -49,6 +49,11 @@ struct hantro_jpeg_enc_hw_ctx {
 	struct hantro_aux_buf bounce_buffer;
 };
 
+enum hantro_h264_hw_mode {
+	HANTRO_H264_LEGACY,
+	HANTRO_H264_HIGH10,
+};
+
 /* Max. number of entries in the DPB (HW limitation). */
 #define HANTRO_H264_DPB_SIZE		16
 
@@ -178,8 +183,11 @@ int hantro_h264_dec_init(struct hantro_ctx *ctx);
 void hantro_h264_dec_exit(struct hantro_ctx *ctx);
 
 static inline size_t
-hantro_h264_mv_size(unsigned int width, unsigned int height)
+hantro_h264_mv_size(unsigned int width, unsigned int height,
+		    enum hantro_h264_hw_mode hw_mode)
 {
+	unsigned int mv_bytes_per_mb;
+
 	/*
 	 * A decoded 8-bit 4:2:0 NV12 frame may need memory for up to
 	 * 448 bytes per macroblock with additional 32 bytes on
@@ -196,12 +204,17 @@ hantro_h264_mv_size(unsigned int width, unsigned int height)
 	 * +---------------------------+
 	 * | UV-plane  128 bytes x MBs |
 	 * +---------------------------+
-	 * | MV buffer  64 bytes x MBs |
-	 * +---------------------------+
 	 * | MC sync          32 bytes |
 	 * +---------------------------+
+	 * | MV buffer  64 bytes x MBs |
+	 * +---------------------------+
 	 */
-	return 64 * MB_WIDTH(width) * MB_WIDTH(height) + 32;
+	if (hw_mode == HANTRO_H264_LEGACY)
+		mv_bytes_per_mb = 64;
+	else
+		mv_bytes_per_mb = 80;
+
+	return mv_bytes_per_mb * MB_WIDTH(width) * MB_WIDTH(height) + 32;
 }
 
 void hantro_g1_mpeg2_dec_run(struct hantro_ctx *ctx);
diff --git a/drivers/staging/media/hantro/hantro_postproc.c b/drivers/staging/media/hantro/hantro_postproc.c
index 653bae37eed9..893f226ec301 100644
--- a/drivers/staging/media/hantro/hantro_postproc.c
+++ b/drivers/staging/media/hantro/hantro_postproc.c
@@ -156,7 +156,8 @@ int hantro_postproc_alloc(struct hantro_ctx *ctx)
 
 	buf_size = ctx->dst_fmt.plane_fmt[0].sizeimage +
 		   hantro_h264_mv_size(ctx->dst_fmt.width,
-				       ctx->dst_fmt.height);
+				       ctx->dst_fmt.height,
+				       vpu->h264_hw_mode);
 
 	for (i = 0; i < num_buffers; ++i) {
 		struct hantro_aux_buf *priv = &ctx->postproc.dec_q[i];
diff --git a/drivers/staging/media/hantro/hantro_regmap.c b/drivers/staging/media/hantro/hantro_regmap.c
index b87fe809f2f7..62280b873859 100644
--- a/drivers/staging/media/hantro/hantro_regmap.c
+++ b/drivers/staging/media/hantro/hantro_regmap.c
@@ -42,6 +42,7 @@ struct hantro_field_dec {
 	struct reg_field cfg_dec_dirmv_swap;
 	struct reg_field cfg_dec_mode;
 	struct reg_field cfg_dec_buffer_empty_int_e;
+	struct reg_field cfg_dec_ref_compress_bypass;
 	struct reg_field cfg_dec_max_burst;
 	struct reg_field cfg_dec_buswidth;
 	struct reg_field cfg_dec_apf_threshold;
@@ -54,6 +55,7 @@ struct hantro_field_dec {
 	struct reg_field cfg_dec_scaling_list_e;
 	struct reg_field cfg_dec_addr_str;
 	struct reg_field cfg_dec_addr_dst;
+	struct reg_field cfg_dec_addr_dst_chr;
 	struct reg_field cfg_dec_ilace_mode;
 	struct reg_field cfg_dec_addr_qtable;
 	struct reg_field cfg_dec_max_cb_size;
@@ -61,6 +63,7 @@ struct hantro_field_dec {
 	struct reg_field cfg_dec_out_y_stride;
 	struct reg_field cfg_dec_out_c_stride;
 	struct reg_field cfg_dec_addr_dir_mv;
+	struct reg_field cfg_dec_idr_pic_id_h10;
 	struct reg_field cfg_dec_tiled_mode_lsb;
 	struct reg_field cfg_dec_clk_gate_e;
 	struct reg_field cfg_dec_tab_swap;
@@ -68,6 +71,14 @@ struct hantro_field_dec {
 	struct reg_field cfg_dec_ext_timeout_e;
 	struct reg_field cfg_dec_timeout_cycles;
 	struct reg_field cfg_dec_timeout_e;
+	struct reg_field cfg_dec_strm_start_offset;
+	struct reg_field cfg_dec_strm_buffer_len;
+	struct reg_field cfg_dec_bit_depth_y_minus8;
+	struct reg_field cfg_dec_bit_depth_c_minus8;
+	struct reg_field cfg_dec_init_rlist_b15;
+	struct reg_field cfg_dec_init_rlist_f15;
+	struct reg_field cfg_dec_pic_height_4x4;
+	struct reg_field cfg_dec_pic_width_4x4;
 
 	struct reg_field cfg_pp_pipeline_en;
 	struct reg_field cfg_pp_max_burst;
@@ -128,6 +139,9 @@ static const struct hantro_field_dec g1_field = {
 	.cfg_dec_ilace_mode =		REG_FIELD(SWREG(13), 1, 1),
 	.cfg_dec_addr_qtable =		REG_FIELD(SWREG(40), 0, 31),
 	.cfg_dec_addr_dir_mv =		REG_FIELD(SWREG(41), 0, 31),
+	.cfg_dec_idr_pic_id_h10 =	REG_FIELD(SWREG(8), 0, 15),
+	.cfg_dec_init_rlist_f15 =	REG_FIELD(SWREG(47), 0, 4),
+	.cfg_dec_init_rlist_b15 =	REG_FIELD(SWREG(47), 5, 9),
 	.cfg_pp_pipeline_en =		REG_FIELD(SWREG(60), 1, 1),
 	.cfg_pp_max_burst =		REG_FIELD(SWREG(61), 0, 4),
 	.cfg_pp_out_swap32 =		REG_FIELD(SWREG(61), 5, 5),
@@ -166,6 +180,7 @@ static const struct hantro_field_dec vc8000d_field = {
 	.cfg_dec_dirmv_swap =		REG_FIELD(SWREG(2), 20, 23),
 	.cfg_dec_mode =			REG_FIELD(SWREG(3), 27, 31),
 	.cfg_dec_buffer_empty_int_e =	REG_FIELD(SWREG(3), 2, 2),
+	.cfg_dec_ref_compress_bypass =	REG_FIELD(SWREG(3), 8, 8),
 	.cfg_dec_max_burst =		REG_FIELD(SWREG(58), 0, 7),
 	.cfg_dec_buswidth =		REG_FIELD(SWREG(58), 8, 10),
 	.cfg_dec_apf_threshold =	REG_FIELD(SWREG(55), 0, 15),
@@ -178,9 +193,11 @@ static const struct hantro_field_dec vc8000d_field = {
 	.cfg_dec_scaling_list_e =	REG_FIELD(SWREG(5), 24, 24),
 	.cfg_dec_addr_str =		REG_FIELD(SWREG(169), 0, 31),
 	.cfg_dec_addr_dst =		REG_FIELD(SWREG(65), 0, 31),
+	.cfg_dec_addr_dst_chr =		REG_FIELD(SWREG(99), 0, 31),
 	.cfg_dec_ilace_mode =		REG_FIELD(SWREG(65), 1, 1),
 	.cfg_dec_addr_qtable =		REG_FIELD(SWREG(175), 0, 31),
 	.cfg_dec_addr_dir_mv =		REG_FIELD(SWREG(133), 0, 31),
+	.cfg_dec_idr_pic_id_h10 =	REG_FIELD(SWREG(12), 16, 31),
 	.cfg_dec_max_cb_size =		REG_FIELD(SWREG(12), 10, 12),
 	.cfg_dec_min_cb_size =		REG_FIELD(SWREG(12), 13, 15),
 	.cfg_dec_out_y_stride =		REG_FIELD(SWREG(314), 16, 31),
@@ -189,6 +206,14 @@ static const struct hantro_field_dec vc8000d_field = {
 	.cfg_dec_ext_timeout_e =	REG_FIELD(SWREG(318), 31, 31),
 	.cfg_dec_timeout_cycles =	REG_FIELD(SWREG(319), 0, 30),
 	.cfg_dec_timeout_e =		REG_FIELD(SWREG(319), 31, 31),
+	.cfg_dec_strm_buffer_len =	REG_FIELD(SWREG(258), 0, 31),
+	.cfg_dec_strm_start_offset =	REG_FIELD(SWREG(259), 0, 31),
+	.cfg_dec_bit_depth_y_minus8 =	REG_FIELD(SWREG(8), 6, 7),
+	.cfg_dec_bit_depth_c_minus8 =	REG_FIELD(SWREG(8), 4, 5),
+	.cfg_dec_init_rlist_f15 =	REG_FIELD(SWREG(19), 0, 4),
+	.cfg_dec_init_rlist_b15 =	REG_FIELD(SWREG(19), 5, 9),
+	.cfg_dec_pic_height_4x4 =	REG_FIELD(SWREG(20), 0, 11),
+	.cfg_dec_pic_width_4x4 =	REG_FIELD(SWREG(20), 16, 27),
 	.cfg_pp_pipeline_en =		REG_FIELD(SWREG(320), 0, 0),
 	.cfg_pp_out_tile_e =		REG_FIELD(SWREG(320), 3, 3),
 	.cfg_pp_output_fmt =		REG_FIELD(SWREG(322), 18, 22),
@@ -245,6 +270,7 @@ static int hantro_regmap_fields_init_dec(struct hantro_dev *vpu,
 	INIT_DEC_FIELD(dec_dirmv_swap);
 	INIT_DEC_FIELD(dec_mode);
 	INIT_DEC_FIELD(dec_buffer_empty_int_e);
+	INIT_DEC_FIELD(dec_ref_compress_bypass);
 	INIT_DEC_FIELD(dec_max_burst);
 	INIT_DEC_FIELD(dec_buswidth);
 	INIT_DEC_FIELD(dec_apf_threshold);
@@ -257,6 +283,7 @@ static int hantro_regmap_fields_init_dec(struct hantro_dev *vpu,
 	INIT_DEC_FIELD(dec_scaling_list_e);
 	INIT_DEC_FIELD(dec_addr_str);
 	INIT_DEC_FIELD(dec_addr_dst);
+	INIT_DEC_FIELD(dec_addr_dst_chr);
 	INIT_DEC_FIELD(dec_ilace_mode);
 	INIT_DEC_FIELD(dec_addr_qtable);
 	INIT_DEC_FIELD(dec_max_cb_size);
@@ -264,6 +291,7 @@ static int hantro_regmap_fields_init_dec(struct hantro_dev *vpu,
 	INIT_DEC_FIELD(dec_out_y_stride);
 	INIT_DEC_FIELD(dec_out_c_stride);
 	INIT_DEC_FIELD(dec_addr_dir_mv);
+	INIT_DEC_FIELD(dec_idr_pic_id_h10);
 	INIT_DEC_FIELD(dec_tiled_mode_lsb);
 	INIT_DEC_FIELD(dec_clk_gate_e);
 	INIT_DEC_FIELD(dec_tab_swap);
@@ -271,6 +299,14 @@ static int hantro_regmap_fields_init_dec(struct hantro_dev *vpu,
 	INIT_DEC_FIELD(dec_ext_timeout_e);
 	INIT_DEC_FIELD(dec_timeout_cycles);
 	INIT_DEC_FIELD(dec_timeout_e);
+	INIT_DEC_FIELD(dec_strm_buffer_len);
+	INIT_DEC_FIELD(dec_strm_start_offset);
+	INIT_DEC_FIELD(dec_bit_depth_c_minus8);
+	INIT_DEC_FIELD(dec_bit_depth_y_minus8);
+	INIT_DEC_FIELD(dec_init_rlist_b15);
+	INIT_DEC_FIELD(dec_init_rlist_f15);
+	INIT_DEC_FIELD(dec_pic_height_4x4);
+	INIT_DEC_FIELD(dec_pic_width_4x4);
 
 	/* Post-processor */
 	INIT_DEC_FIELD(pp_pipeline_en);
diff --git a/drivers/staging/media/hantro/hantro_regmap.h b/drivers/staging/media/hantro/hantro_regmap.h
index 0a39bae83f85..083c4e92c4bd 100644
--- a/drivers/staging/media/hantro/hantro_regmap.h
+++ b/drivers/staging/media/hantro/hantro_regmap.h
@@ -17,9 +17,15 @@
 #define SWREG_ITER_G1(n, i)	(SWREG(n) + ((i) << 2))
 #define SWREG_ITER_VC8000(n, i)	(SWREG(n) + ((i) << 3))
 
+#define REG_CHR_REF(i)		(SWREG_ITER_VC8000(101, i))
+#define REG_DMV_REF(i)		(SWREG_ITER_VC8000(135, i))
+
 #define REG_ADDR_REF(i)		(vpu->core_hw_dec_rev == HANTRO_G1_REV ? \
 				 SWREG_ITER_G1(14, i) : \
 				 SWREG_ITER_VC8000(67, i))
+#define REG_BD_REF_PIC(i)	(vpu->core_hw_dec_rev == HANTRO_G1_REV ? \
+				 SWREG_ITER_G1(42, i) : \
+				 SWREG_ITER_VC8000(14, i))
 
 struct hantro_regmap_fields_dec {
 	/* Decoder */
@@ -32,6 +38,7 @@ struct hantro_regmap_fields_dec {
 	struct regmap_field *dec_dirmv_swap;
 	struct regmap_field *dec_mode;
 	struct regmap_field *dec_buffer_empty_int_e;
+	struct regmap_field *dec_ref_compress_bypass;
 	struct regmap_field *dec_buswidth;
 	struct regmap_field *dec_apf_threshold;
 	struct regmap_field *dec_stream_len;
@@ -43,6 +50,7 @@ struct hantro_regmap_fields_dec {
 	struct regmap_field *dec_scaling_list_e;
 	struct regmap_field *dec_addr_str;
 	struct regmap_field *dec_addr_dst;
+	struct regmap_field *dec_addr_dst_chr;
 	struct regmap_field *dec_ilace_mode;
 	struct regmap_field *dec_addr_qtable;
 	struct regmap_field *dec_max_cb_size;
@@ -50,6 +58,7 @@ struct hantro_regmap_fields_dec {
 	struct regmap_field *dec_out_y_stride;
 	struct regmap_field *dec_out_c_stride;
 	struct regmap_field *dec_addr_dir_mv;
+	struct regmap_field *dec_idr_pic_id_h10;
 	struct regmap_field *dec_tiled_mode_lsb;
 	struct regmap_field *dec_clk_gate_e;
 	struct regmap_field *dec_tab_swap;
@@ -57,6 +66,14 @@ struct hantro_regmap_fields_dec {
 	struct regmap_field *dec_ext_timeout_e;
 	struct regmap_field *dec_timeout_cycles;
 	struct regmap_field *dec_timeout_e;
+	struct regmap_field *dec_strm_buffer_len;
+	struct regmap_field *dec_strm_start_offset;
+	struct regmap_field *dec_bit_depth_y_minus8;
+	struct regmap_field *dec_bit_depth_c_minus8;
+	struct regmap_field *dec_init_rlist_b15;
+	struct regmap_field *dec_init_rlist_f15;
+	struct regmap_field *dec_pic_height_4x4;
+	struct regmap_field *dec_pic_width_4x4;
 
 	/* Post-processor */
 	struct regmap_field *pp_pipeline_en;
diff --git a/drivers/staging/media/hantro/hantro_v4l2.c b/drivers/staging/media/hantro/hantro_v4l2.c
index b668a82d40ad..dc811e256181 100644
--- a/drivers/staging/media/hantro/hantro_v4l2.c
+++ b/drivers/staging/media/hantro/hantro_v4l2.c
@@ -282,7 +282,8 @@ static int hantro_try_fmt(const struct hantro_ctx *ctx,
 		    !hantro_needs_postproc(ctx, fmt))
 			pix_mp->plane_fmt[0].sizeimage +=
 				hantro_h264_mv_size(pix_mp->width,
-						    pix_mp->height);
+						    pix_mp->height,
+						    ctx->dev->h264_hw_mode);
 	} else if (!pix_mp->plane_fmt[0].sizeimage) {
 		/*
 		 * For coded formats the application can specify
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 16/18] media: hantro: rename h264_dec as it's not G1 specific anymore

[Adrian Ratiu]

The h264 decoder is now capable of decoding on both G1 and VC8000 and
other HW revisions can be added in the future by extending the hantro
regmap config, so we rename it to reflect the new status.

All other core-specific files like "hantro_g1_mpeg2_dec.c" should be
renamed as well after they have been ported to the new regmap API.

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/Makefile                           | 2 +-
 .../media/hantro/{hantro_g1_h264_dec.c => hantro_h264_dec.c}    | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/staging/media/hantro/{hantro_g1_h264_dec.c => hantro_h264_dec.c} (100%)

diff --git a/drivers/staging/media/hantro/Makefile b/drivers/staging/media/hantro/Makefile
index 52bc0ee73569..94f1e454c495 100644
--- a/drivers/staging/media/hantro/Makefile
+++ b/drivers/staging/media/hantro/Makefile
@@ -7,7 +7,7 @@ hantro-vpu-y += \
 		hantro_v4l2.o \
 		hantro_postproc.o \
 		hantro_h1_jpeg_enc.o \
-		hantro_g1_h264_dec.o \
+		hantro_h264_dec.o \
 		hantro_g1_mpeg2_dec.o \
 		hantro_regmap.o \
 		hantro_g1_vp8_dec.o \
diff --git a/drivers/staging/media/hantro/hantro_g1_h264_dec.c b/drivers/staging/media/hantro/hantro_h264_dec.c
similarity index 100%
rename from drivers/staging/media/hantro/hantro_g1_h264_dec.c
rename to drivers/staging/media/hantro/hantro_h264_dec.c
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 17/18] media: hantro: add dump registers debug option before decode start

[Adrian Ratiu]

It is very useful to know the status of all the decoder configuration
registers right before starting a decode operation, so add an option
to print them if register debugging is enabled (debug bit 7 is set).

Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/hantro.h          | 1 +
 drivers/staging/media/hantro/hantro_h264_dec.c | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/hantro/hantro.h b/drivers/staging/media/hantro/hantro.h
index 70aeb11b1149..1b0c441ff15a 100644
--- a/drivers/staging/media/hantro/hantro.h
+++ b/drivers/staging/media/hantro/hantro.h
@@ -304,6 +304,7 @@ struct hantro_reg {
  * bit 4 - detail fmt, ctrl, buffer q/dq information
  * bit 5 - detail function enter/leave trace information
  * bit 6 - register write/read information
+ * bit 7 - dump
  */
 extern int hantro_debug;
 
diff --git a/drivers/staging/media/hantro/hantro_h264_dec.c b/drivers/staging/media/hantro/hantro_h264_dec.c
index e64b59c84111..2c53394cbb0c 100644
--- a/drivers/staging/media/hantro/hantro_h264_dec.c
+++ b/drivers/staging/media/hantro/hantro_h264_dec.c
@@ -381,7 +381,9 @@ void hantro_g1_h264_dec_run(struct hantro_ctx *ctx)
 	struct hantro_dev *vpu = ctx->dev;
 	struct hantro_regmap_fields_dec *fields = vpu->reg_fields_dec;
 	bool do_high10 = (vpu->h264_hw_mode == HANTRO_H264_HIGH10);
-	int reg;
+	u32 max_reg = hantro_regmap_dec.max_register;
+	u32 reg_stride = hantro_regmap_dec.reg_stride;
+	int reg, i;
 
 	/* Prepare the H264 decoder context. */
 	if (hantro_h264_dec_prepare_run(ctx))
@@ -421,6 +423,11 @@ void hantro_g1_h264_dec_run(struct hantro_ctx *ctx)
 	regmap_field_write(fields->dec_max_burst, 16);
 	regmap_field_write(fields->dec_axi_rd_id, 16);
 
+	vpu_debug(7, "Reg dump at decoding start\n");
+	for (i = 0; hantro_debug & BIT(7) && i <= max_reg; i += reg_stride)
+		vpu_debug(7, "swreg %03d: %08x\n", i / 4, vdpu_read(vpu, i));
+	vpu_debug(7, "Reg dump end\n");
+
 	/* Start decoding! */
 	vdpu_write(vpu, G1_REG_INTERRUPT_DEC_E, G1_REG_INTERRUPT);
 }
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH 18/18] media: hantro: document encoder reg fields

[Adrian Ratiu]

Even though these fields are currently unused it is still a good
idea to have them documented for future encoder implementations.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 drivers/staging/media/hantro/hantro_regmap.c | 580 ++++++++++++++++++-
 drivers/staging/media/hantro/hantro_regmap.h | 177 +++++-
 2 files changed, 754 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_regmap.c b/drivers/staging/media/hantro/hantro_regmap.c
index 62280b873859..f15884f29ed6 100644
--- a/drivers/staging/media/hantro/hantro_regmap.c
+++ b/drivers/staging/media/hantro/hantro_regmap.c
@@ -115,7 +115,394 @@ struct hantro_field_dec {
 };
 
 struct hantro_field_enc {
-	/* TODO: populate encoder fields */
+	struct reg_field cfg_enc_timeout_e;
+	struct reg_field cfg_enc_timeout_cycles;
+	struct reg_field cfg_enc_mode;
+	struct reg_field cfg_enc_stream_mode;
+	struct reg_field cfg_enc_enable;
+	struct reg_field cfg_enc_pic_type;
+	struct reg_field cfg_enc_pic_width;
+	struct reg_field cfg_enc_pic_height;
+	struct reg_field cfg_enc_burst_len;
+	struct reg_field cfg_enc_clk_gate_en;
+	struct reg_field cfg_enc_TODO_swap;
+	struct reg_field cfg_enc_stream_buf_limit;
+	struct reg_field cfg_enc_row_len;
+	struct reg_field cfg_enc_overfill_r;
+	struct reg_field cfg_enc_overfill_b;
+	struct reg_field cfg_enc_src_format;
+	struct reg_field cfg_enc_init_qp;
+	struct reg_field cfg_enc_chroma_qp_offset;
+	struct reg_field cfg_enc_idr_pic_id;
+	struct reg_field cfg_enc_nal_ref_idc;
+	struct reg_field cfg_enc_pps_id;
+	struct reg_field cfg_enc_nal_unit_type;
+	struct reg_field cfg_enc_frame_num;
+	struct reg_field cfg_enc_min_cb_size;
+	struct reg_field cfg_enc_max_cb_size;
+	struct reg_field cfg_enc_max_trb_size;
+	struct reg_field cfg_enc_min_trb_size;
+	struct reg_field cfg_enc_deblocking_filter_dis;
+	struct reg_field cfg_enc_slice_deblocking_filter_override;
+	struct reg_field cfg_enc_slice_deblocking_filter_dis;
+	struct reg_field cfg_enc_pps_deblocking_filter_override;
+	struct reg_field cfg_enc_slice_alpha_div2;
+	struct reg_field cfg_enc_slice_beta_div2;
+	struct reg_field cfg_enc_slice_size;
+	struct reg_field cfg_enc_nal_size_write;
+	struct reg_field cfg_enc_cabac_init_idc;
+	struct reg_field cfg_enc_pic_qp;
+	struct reg_field cfg_enc_qp_frac;
+	struct reg_field cfg_enc_entropy_coding_mode;
+	struct reg_field cfg_enc_axi_r_outstanding_num;
+	struct reg_field cfg_enc_axi_w_outstanding_num;
+	struct reg_field cfg_enc_trans8x8_mode_en;
+	struct reg_field cfg_enc_inter4x4_mode;
+	struct reg_field cfg_enc_quarter_pixmv_dis;
+	struct reg_field cfg_enc_addr_cabac;
+	struct reg_field cfg_enc_addr_str;
+	struct reg_field cfg_enc_addr_size_table;
+	struct reg_field cfg_enc_addr_rec_luma;
+	struct reg_field cfg_enc_addr_rec_luma_4n;
+	struct reg_field cfg_enc_addr_ref_luma_l0_4n0;
+	struct reg_field cfg_enc_addr_rec_chroma;
+	struct reg_field cfg_enc_addr_ref_luma;
+	struct reg_field cfg_enc_addr_ref_chroma;
+	struct reg_field cfg_enc_addr_src_y;
+	struct reg_field cfg_enc_addr_src_cb;
+	struct reg_field cfg_enc_addr_src_cr;
+	struct reg_field cfg_enc_log2_max_pic_order_cnt_lsb;
+	struct reg_field cfg_enc_log2_max_frame_num;
+	struct reg_field cfg_enc_pic_order_cnt_type;
+	struct reg_field cfg_enc_l0_delta_framenum0;
+	struct reg_field cfg_enc_l0_used_by_next_pic0;
+	struct reg_field cfg_enc_l0_used_by_next_pic1;
+
+	struct reg_field cfg_enc_lu_stride;
+	struct reg_field cfg_enc_cr_stride;
+	struct reg_field cfg_enc_ref_lu_stride;
+	struct reg_field cfg_enc_ref_ds_lu_stride;
+	struct reg_field cfg_enc_ref_cr_stride;
+	struct reg_field cfg_enc_ipcm2_left;
+	struct reg_field cfg_enc_ipcm2_right;
+	struct reg_field cfg_enc_ipcm2_top;
+	struct reg_field cfg_enc_ipcm2_bottom;
+
+	struct reg_field cfg_enc_slice_qp_offset;
+	struct reg_field cfg_enc_qp_min;
+	struct reg_field cfg_enc_qp_max;
+
+	struct reg_field cfg_enc_lambda_satd_me_0;
+	struct reg_field cfg_enc_lambda_satd_me_1;
+	struct reg_field cfg_enc_lambda_satd_me_2;
+	struct reg_field cfg_enc_lambda_satd_me_3;
+	struct reg_field cfg_enc_lambda_satd_me_4;
+	struct reg_field cfg_enc_lambda_satd_me_5;
+	struct reg_field cfg_enc_lambda_satd_me_6;
+	struct reg_field cfg_enc_lambda_satd_me_7;
+	struct reg_field cfg_enc_lambda_satd_me_8;
+	struct reg_field cfg_enc_lambda_satd_me_9;
+	struct reg_field cfg_enc_lambda_satd_me_10;
+	struct reg_field cfg_enc_lambda_satd_me_11;
+	struct reg_field cfg_enc_lambda_satd_me_12;
+	struct reg_field cfg_enc_lambda_satd_me_13;
+	struct reg_field cfg_enc_lambda_satd_me_14;
+	struct reg_field cfg_enc_lambda_satd_me_15;
+	struct reg_field cfg_enc_lambda_satd_me_16;
+	struct reg_field cfg_enc_lambda_satd_me_17;
+	struct reg_field cfg_enc_lambda_satd_me_18;
+	struct reg_field cfg_enc_lambda_satd_me_19;
+	struct reg_field cfg_enc_lambda_satd_me_20;
+	struct reg_field cfg_enc_lambda_satd_me_21;
+	struct reg_field cfg_enc_lambda_satd_me_22;
+	struct reg_field cfg_enc_lambda_satd_me_23;
+	struct reg_field cfg_enc_lambda_satd_me_24;
+	struct reg_field cfg_enc_lambda_satd_me_25;
+	struct reg_field cfg_enc_lambda_satd_me_26;
+	struct reg_field cfg_enc_lambda_satd_me_27;
+	struct reg_field cfg_enc_lambda_satd_me_28;
+	struct reg_field cfg_enc_lambda_satd_me_29;
+	struct reg_field cfg_enc_lambda_satd_me_30;
+	struct reg_field cfg_enc_lambda_satd_me_31;
+
+	struct reg_field cfg_enc_lambda_satd_0;
+	struct reg_field cfg_enc_lambda_satd_1;
+	struct reg_field cfg_enc_lambda_satd_2;
+	struct reg_field cfg_enc_lambda_satd_3;
+	struct reg_field cfg_enc_lambda_satd_4;
+	struct reg_field cfg_enc_lambda_satd_5;
+	struct reg_field cfg_enc_lambda_satd_6;
+	struct reg_field cfg_enc_lambda_satd_7;
+	struct reg_field cfg_enc_lambda_satd_8;
+	struct reg_field cfg_enc_lambda_satd_9;
+	struct reg_field cfg_enc_lambda_satd_10;
+	struct reg_field cfg_enc_lambda_satd_11;
+	struct reg_field cfg_enc_lambda_satd_12;
+	struct reg_field cfg_enc_lambda_satd_13;
+	struct reg_field cfg_enc_lambda_satd_14;
+	struct reg_field cfg_enc_lambda_satd_15;
+	struct reg_field cfg_enc_lambda_satd_16;
+	struct reg_field cfg_enc_lambda_satd_17;
+	struct reg_field cfg_enc_lambda_satd_18;
+	struct reg_field cfg_enc_lambda_satd_19;
+	struct reg_field cfg_enc_lambda_satd_20;
+	struct reg_field cfg_enc_lambda_satd_21;
+	struct reg_field cfg_enc_lambda_satd_22;
+	struct reg_field cfg_enc_lambda_satd_23;
+	struct reg_field cfg_enc_lambda_satd_24;
+	struct reg_field cfg_enc_lambda_satd_25;
+	struct reg_field cfg_enc_lambda_satd_26;
+	struct reg_field cfg_enc_lambda_satd_27;
+	struct reg_field cfg_enc_lambda_satd_28;
+	struct reg_field cfg_enc_lambda_satd_29;
+	struct reg_field cfg_enc_lambda_satd_30;
+	struct reg_field cfg_enc_lambda_satd_31;
+
+	struct reg_field cfg_enc_lambda_sse_me_0;
+	struct reg_field cfg_enc_lambda_sse_me_1;
+	struct reg_field cfg_enc_lambda_sse_me_2;
+	struct reg_field cfg_enc_lambda_sse_me_3;
+	struct reg_field cfg_enc_lambda_sse_me_4;
+	struct reg_field cfg_enc_lambda_sse_me_5;
+	struct reg_field cfg_enc_lambda_sse_me_6;
+	struct reg_field cfg_enc_lambda_sse_me_7;
+	struct reg_field cfg_enc_lambda_sse_me_8;
+	struct reg_field cfg_enc_lambda_sse_me_9;
+	struct reg_field cfg_enc_lambda_sse_me_10;
+	struct reg_field cfg_enc_lambda_sse_me_11;
+	struct reg_field cfg_enc_lambda_sse_me_12;
+	struct reg_field cfg_enc_lambda_sse_me_13;
+	struct reg_field cfg_enc_lambda_sse_me_14;
+	struct reg_field cfg_enc_lambda_sse_me_15;
+	struct reg_field cfg_enc_lambda_sse_me_16;
+	struct reg_field cfg_enc_lambda_sse_me_17;
+	struct reg_field cfg_enc_lambda_sse_me_18;
+	struct reg_field cfg_enc_lambda_sse_me_19;
+	struct reg_field cfg_enc_lambda_sse_me_20;
+	struct reg_field cfg_enc_lambda_sse_me_21;
+	struct reg_field cfg_enc_lambda_sse_me_22;
+	struct reg_field cfg_enc_lambda_sse_me_23;
+	struct reg_field cfg_enc_lambda_sse_me_24;
+	struct reg_field cfg_enc_lambda_sse_me_25;
+	struct reg_field cfg_enc_lambda_sse_me_26;
+	struct reg_field cfg_enc_lambda_sse_me_27;
+	struct reg_field cfg_enc_lambda_sse_me_28;
+	struct reg_field cfg_enc_lambda_sse_me_29;
+	struct reg_field cfg_enc_lambda_sse_me_30;
+	struct reg_field cfg_enc_lambda_sse_me_31;
+};
+
+static const struct hantro_field_enc h1_field = {
+	.cfg_enc_mode =			REG_FIELD(SWREG(14), 1, 2),
+	.cfg_enc_enable =		REG_FIELD(SWREG(14), 0, 0),
+	.cfg_enc_pic_type =		REG_FIELD(SWREG(14), 3, 4),
+	.cfg_enc_pic_width =		REG_FIELD(SWREG(14), 19, 27),
+	.cfg_enc_pic_height =		REG_FIELD(SWREG(14), 10, 18),
+	.cfg_enc_nal_size_write =	REG_FIELD(SWREG(14), 29, 29),
+	.cfg_enc_stream_mode =		REG_FIELD(SWREG(18), 16, 16),
+	.cfg_enc_burst_len =		REG_FIELD(SWREG(2), 8, 13),
+	.cfg_enc_clk_gate_en =		REG_FIELD(SWREG(2), 4, 4),
+	.cfg_enc_stream_buf_limit =	REG_FIELD(SWREG(48), 0, 31),
+	.cfg_enc_row_len =		REG_FIELD(SWREG(15), 12, 25),
+	.cfg_enc_overfill_r =		REG_FIELD(SWREG(15), 10, 11),
+	.cfg_enc_overfill_b =		REG_FIELD(SWREG(15), 6, 9),
+	.cfg_enc_src_format =		REG_FIELD(SWREG(15), 2, 5),
+	.cfg_enc_init_qp =		REG_FIELD(SWREG(16), 26, 31),
+	.cfg_enc_slice_alpha_div2 =	REG_FIELD(SWREG(16), 22, 25),
+	.cfg_enc_slice_beta_div2 =	REG_FIELD(SWREG(16), 18, 21),
+	.cfg_enc_idr_pic_id =		REG_FIELD(SWREG(16), 1, 4),
+	.cfg_enc_pps_id =		REG_FIELD(SWREG(17), 24, 31),
+	.cfg_enc_frame_num =		REG_FIELD(SWREG(17), 0, 15),
+	.cfg_enc_chroma_qp_offset =	REG_FIELD(SWREG(16), 13, 17),
+	.cfg_enc_deblocking_filter_dis = REG_FIELD(SWREG(18), 30, 31),
+	.cfg_enc_slice_size =		REG_FIELD(SWREG(18), 23, 29),
+	.cfg_enc_cabac_init_idc =	REG_FIELD(SWREG(18), 19, 20),
+	.cfg_enc_entropy_coding_mode =	REG_FIELD(SWREG(18), 18, 18),
+	.cfg_enc_trans8x8_mode_en =	REG_FIELD(SWREG(18), 21, 21),
+	.cfg_enc_inter4x4_mode =	REG_FIELD(SWREG(18), 17, 17),
+	.cfg_enc_quarter_pixmv_dis =	REG_FIELD(SWREG(18), 22, 22),
+	.cfg_enc_addr_str =		REG_FIELD(SWREG(5), 0, 31),
+	.cfg_enc_addr_size_table =	REG_FIELD(SWREG(6), 0, 31),
+	.cfg_enc_addr_rec_luma =	REG_FIELD(SWREG(9), 0, 31),
+	.cfg_enc_addr_rec_chroma =	REG_FIELD(SWREG(10), 0, 31),
+	.cfg_enc_addr_ref_luma =	REG_FIELD(SWREG(7), 0, 31),
+	.cfg_enc_addr_ref_chroma =	REG_FIELD(SWREG(8), 0, 31),
+	.cfg_enc_addr_src_y =		REG_FIELD(SWREG(11), 0, 31),
+	.cfg_enc_addr_src_cb =		REG_FIELD(SWREG(12), 0, 31),
+	.cfg_enc_addr_src_cr =		REG_FIELD(SWREG(13), 0, 31),
+};
+
+static const struct hantro_field_enc vc8000e_field = {
+	.cfg_enc_burst_len =		REG_FIELD(SWREG(81), 24, 31),
+	.cfg_enc_timeout_e =		REG_FIELD(SWREG(81), 23, 23),
+	.cfg_enc_timeout_cycles =	REG_FIELD(SWREG(81), 0, 22),
+	.cfg_enc_mode =			REG_FIELD(SWREG(4), 29, 31),
+	.cfg_enc_stream_mode =		REG_FIELD(SWREG(4), 18, 18),
+	.cfg_enc_enable =		REG_FIELD(SWREG(5), 0, 0),
+	.cfg_enc_pic_type =		REG_FIELD(SWREG(5), 1, 2),
+	.cfg_enc_pic_width =		REG_FIELD(SWREG(5), 22, 31),
+	.cfg_enc_pic_height =		REG_FIELD(SWREG(5), 11, 21),
+	.cfg_enc_stream_buf_limit =	REG_FIELD(SWREG(9), 0, 31),
+	.cfg_enc_row_len =		REG_FIELD(SWREG(38), 6, 19),
+	.cfg_enc_overfill_r =		REG_FIELD(SWREG(38), 4, 5),
+	.cfg_enc_overfill_b =		REG_FIELD(SWREG(38), 1, 3),
+	.cfg_enc_src_format =		REG_FIELD(SWREG(38), 28, 31),
+	.cfg_enc_init_qp =		REG_FIELD(SWREG(7), 26, 31),
+	.cfg_enc_idr_pic_id =		REG_FIELD(SWREG(193), 2, 2),
+	.cfg_enc_nal_ref_idc =		REG_FIELD(SWREG(193), 3, 3),
+	.cfg_enc_pic_qp =		REG_FIELD(SWREG(7), 8, 13),
+	.cfg_enc_qp_frac =		REG_FIELD(SWREG(182), 16, 31),
+	.cfg_enc_nal_unit_type =	REG_FIELD(SWREG(191), 26, 31),
+	.cfg_enc_pps_id =		REG_FIELD(SWREG(191), 17, 22),
+	.cfg_enc_frame_num =		REG_FIELD(SWREG(192), 0, 31),
+	.cfg_enc_min_cb_size =		REG_FIELD(SWREG(4), 25, 26),
+	.cfg_enc_max_cb_size =		REG_FIELD(SWREG(4), 23, 24),
+	.cfg_enc_min_trb_size =		REG_FIELD(SWREG(4), 21, 22),
+	.cfg_enc_max_trb_size =		REG_FIELD(SWREG(4), 19, 20),
+	.cfg_enc_chroma_qp_offset =	REG_FIELD(SWREG(4), 13, 17),
+	.cfg_enc_slice_deblocking_filter_dis = REG_FIELD(SWREG(6), 15, 15),
+	.cfg_enc_pps_deblocking_filter_override = REG_FIELD(SWREG(5), 9, 9),
+	.cfg_enc_slice_deblocking_filter_override = REG_FIELD(SWREG(5), 8, 8),
+	.cfg_enc_slice_alpha_div2 =	REG_FIELD(SWREG(6), 11, 14),
+	.cfg_enc_slice_beta_div2 =	REG_FIELD(SWREG(6), 7, 10),
+	.cfg_enc_slice_size =		REG_FIELD(SWREG(6), 25, 31),
+	.cfg_enc_nal_size_write =	REG_FIELD(SWREG(6), 1, 1),
+	.cfg_enc_cabac_init_idc =	REG_FIELD(SWREG(7), 25, 25),
+	.cfg_enc_entropy_coding_mode =	REG_FIELD(SWREG(193), 0, 0),
+	.cfg_enc_axi_r_outstanding_num = REG_FIELD(SWREG(261), 4, 11),
+	.cfg_enc_axi_w_outstanding_num = REG_FIELD(SWREG(246), 6, 13),
+	.cfg_enc_trans8x8_mode_en =	REG_FIELD(SWREG(193), 1, 1),
+	.cfg_enc_addr_str =		REG_FIELD(SWREG(8), 0, 31),
+	.cfg_enc_addr_size_table =	REG_FIELD(SWREG(10), 0, 31),
+	.cfg_enc_addr_rec_luma_4n =	REG_FIELD(SWREG(72), 0, 31),
+	.cfg_enc_addr_ref_luma_l0_4n0 =	REG_FIELD(SWREG(74), 0, 31),
+	.cfg_enc_addr_rec_luma =	REG_FIELD(SWREG(15), 0, 31),
+	.cfg_enc_addr_rec_chroma =	REG_FIELD(SWREG(16), 0, 31),
+	.cfg_enc_addr_ref_luma =	REG_FIELD(SWREG(18), 0, 31),
+	.cfg_enc_addr_ref_chroma =	REG_FIELD(SWREG(19), 0, 31),
+	.cfg_enc_addr_src_y =		REG_FIELD(SWREG(12), 0, 31),
+	.cfg_enc_addr_src_cb =		REG_FIELD(SWREG(13), 0, 31),
+	.cfg_enc_addr_src_cr =		REG_FIELD(SWREG(14), 0, 31),
+	.cfg_enc_log2_max_pic_order_cnt_lsb = REG_FIELD(SWREG(277), 27, 31),
+	.cfg_enc_log2_max_frame_num =	REG_FIELD(SWREG(277), 22, 26),
+	.cfg_enc_pic_order_cnt_type =	REG_FIELD(SWREG(277), 20, 21),
+	.cfg_enc_l0_delta_framenum0 =	REG_FIELD(SWREG(193), 21, 31),
+	.cfg_enc_l0_used_by_next_pic0 = REG_FIELD(SWREG(193), 20, 20),
+	.cfg_enc_l0_used_by_next_pic1 = REG_FIELD(SWREG(193), 8, 8),
+
+	.cfg_enc_lu_stride =		REG_FIELD(SWREG(210), 12, 31),
+	.cfg_enc_cr_stride =		REG_FIELD(SWREG(211), 12, 31),
+	.cfg_enc_ref_lu_stride =	REG_FIELD(SWREG(212), 12, 31),
+	.cfg_enc_ref_cr_stride =	REG_FIELD(SWREG(237), 12, 31),
+	.cfg_enc_ref_ds_lu_stride =	REG_FIELD(SWREG(213), 14, 31),
+
+	.cfg_enc_ipcm2_left =		REG_FIELD(SWREG(210), 3, 11),
+	.cfg_enc_ipcm2_right =		REG_FIELD(SWREG(211), 3, 11),
+	.cfg_enc_ipcm2_top =		REG_FIELD(SWREG(212), 3, 11),
+	.cfg_enc_ipcm2_bottom =		REG_FIELD(SWREG(213), 5, 13),
+
+	.cfg_enc_slice_qp_offset =	REG_FIELD(SWREG(170), 5, 10),
+	.cfg_enc_qp_min =		REG_FIELD(SWREG(172), 5, 10),
+	.cfg_enc_qp_max =		REG_FIELD(SWREG(173), 5, 10),
+
+	.cfg_enc_lambda_satd_me_0 =	REG_FIELD(SWREG(28), 19, 31),
+	.cfg_enc_lambda_satd_me_1 =	REG_FIELD(SWREG(28), 6,  18),
+	.cfg_enc_lambda_satd_me_2 =	REG_FIELD(SWREG(29), 19, 31),
+	.cfg_enc_lambda_satd_me_3 =	REG_FIELD(SWREG(29), 6,  18),
+	.cfg_enc_lambda_satd_me_4 =	REG_FIELD(SWREG(30), 19, 31),
+	.cfg_enc_lambda_satd_me_5 =	REG_FIELD(SWREG(30), 6,  18),
+	.cfg_enc_lambda_satd_me_6 =	REG_FIELD(SWREG(31), 19, 31),
+	.cfg_enc_lambda_satd_me_7 =	REG_FIELD(SWREG(31), 6,  18),
+	.cfg_enc_lambda_satd_me_8 =	REG_FIELD(SWREG(32), 19, 31),
+	.cfg_enc_lambda_satd_me_9 =	REG_FIELD(SWREG(32), 6,  18),
+	.cfg_enc_lambda_satd_me_10 =	REG_FIELD(SWREG(33), 19, 31),
+	.cfg_enc_lambda_satd_me_11 =	REG_FIELD(SWREG(33), 6,  18),
+	.cfg_enc_lambda_satd_me_12 =	REG_FIELD(SWREG(34), 19, 31),
+	.cfg_enc_lambda_satd_me_13 =	REG_FIELD(SWREG(34), 6,  18),
+	.cfg_enc_lambda_satd_me_14 =	REG_FIELD(SWREG(78), 19, 31),
+	.cfg_enc_lambda_satd_me_15 =	REG_FIELD(SWREG(78), 6,  18),
+	.cfg_enc_lambda_satd_me_16 =	REG_FIELD(SWREG(150), 19, 31),
+	.cfg_enc_lambda_satd_me_17 =	REG_FIELD(SWREG(150), 6,  18),
+	.cfg_enc_lambda_satd_me_18 =	REG_FIELD(SWREG(151), 19, 31),
+	.cfg_enc_lambda_satd_me_19 =	REG_FIELD(SWREG(151), 6,  18),
+	.cfg_enc_lambda_satd_me_20 =	REG_FIELD(SWREG(152), 19, 31),
+	.cfg_enc_lambda_satd_me_21 =	REG_FIELD(SWREG(152), 6,  18),
+	.cfg_enc_lambda_satd_me_22 =	REG_FIELD(SWREG(153), 19, 31),
+	.cfg_enc_lambda_satd_me_23 =	REG_FIELD(SWREG(153), 6,  18),
+	.cfg_enc_lambda_satd_me_24 =	REG_FIELD(SWREG(154), 19, 31),
+	.cfg_enc_lambda_satd_me_25 =	REG_FIELD(SWREG(154), 6,  18),
+	.cfg_enc_lambda_satd_me_26 =	REG_FIELD(SWREG(155), 19, 31),
+	.cfg_enc_lambda_satd_me_27 =	REG_FIELD(SWREG(155), 6,  18),
+	.cfg_enc_lambda_satd_me_28 =	REG_FIELD(SWREG(156), 19, 31),
+	.cfg_enc_lambda_satd_me_29 =	REG_FIELD(SWREG(156), 6,  18),
+	.cfg_enc_lambda_satd_me_30 =	REG_FIELD(SWREG(157), 19, 31),
+	.cfg_enc_lambda_satd_me_31 =	REG_FIELD(SWREG(157), 6,  18),
+
+	.cfg_enc_lambda_satd_0 =	REG_FIELD(SWREG(125), 18, 31),
+	.cfg_enc_lambda_satd_1 =	REG_FIELD(SWREG(125), 4, 17),
+	.cfg_enc_lambda_satd_2 =	REG_FIELD(SWREG(126), 18, 31),
+	.cfg_enc_lambda_satd_3 =	REG_FIELD(SWREG(126), 4, 17),
+	.cfg_enc_lambda_satd_4 =	REG_FIELD(SWREG(127), 18, 31),
+	.cfg_enc_lambda_satd_5 =	REG_FIELD(SWREG(127), 4, 17),
+	.cfg_enc_lambda_satd_6 =	REG_FIELD(SWREG(128), 18, 31),
+	.cfg_enc_lambda_satd_7 =	REG_FIELD(SWREG(128), 4, 17),
+	.cfg_enc_lambda_satd_8 =	REG_FIELD(SWREG(129), 18, 31),
+	.cfg_enc_lambda_satd_9 =	REG_FIELD(SWREG(129), 4, 17),
+	.cfg_enc_lambda_satd_10 =	REG_FIELD(SWREG(130), 18, 31),
+	.cfg_enc_lambda_satd_11 =	REG_FIELD(SWREG(130), 4, 17),
+	.cfg_enc_lambda_satd_12 =	REG_FIELD(SWREG(131), 18, 31),
+	.cfg_enc_lambda_satd_13 =	REG_FIELD(SWREG(131), 4, 17),
+	.cfg_enc_lambda_satd_14 =	REG_FIELD(SWREG(132), 18, 31),
+	.cfg_enc_lambda_satd_15 =	REG_FIELD(SWREG(132), 4, 17),
+	.cfg_enc_lambda_satd_16 =	REG_FIELD(SWREG(174), 18, 31),
+	.cfg_enc_lambda_satd_17 =	REG_FIELD(SWREG(174), 4, 17),
+	.cfg_enc_lambda_satd_18 =	REG_FIELD(SWREG(175), 18, 31),
+	.cfg_enc_lambda_satd_19 =	REG_FIELD(SWREG(175), 4, 17),
+	.cfg_enc_lambda_satd_20 =	REG_FIELD(SWREG(176), 18, 31),
+	.cfg_enc_lambda_satd_21 =	REG_FIELD(SWREG(176), 4, 17),
+	.cfg_enc_lambda_satd_22 =	REG_FIELD(SWREG(177), 18, 31),
+	.cfg_enc_lambda_satd_23 =	REG_FIELD(SWREG(177), 4, 17),
+	.cfg_enc_lambda_satd_24 =	REG_FIELD(SWREG(178), 18, 31),
+	.cfg_enc_lambda_satd_25 =	REG_FIELD(SWREG(178), 4, 17),
+	.cfg_enc_lambda_satd_26 =	REG_FIELD(SWREG(179), 18, 31),
+	.cfg_enc_lambda_satd_27 =	REG_FIELD(SWREG(179), 4, 17),
+	.cfg_enc_lambda_satd_28 =	REG_FIELD(SWREG(180), 18, 31),
+	.cfg_enc_lambda_satd_29 =	REG_FIELD(SWREG(180), 4, 17),
+	.cfg_enc_lambda_satd_30 =	REG_FIELD(SWREG(181), 18, 31),
+	.cfg_enc_lambda_satd_31 =	REG_FIELD(SWREG(181), 4, 17),
+
+	.cfg_enc_lambda_sse_me_0 =	REG_FIELD(SWREG(79), 11, 31),
+	.cfg_enc_lambda_sse_me_1 =	REG_FIELD(SWREG(122), 11, 31),
+	.cfg_enc_lambda_sse_me_2 =	REG_FIELD(SWREG(123), 11, 31),
+	.cfg_enc_lambda_sse_me_3 =	REG_FIELD(SWREG(124), 11, 31),
+	.cfg_enc_lambda_sse_me_4 =	REG_FIELD(SWREG(138), 11, 31),
+	.cfg_enc_lambda_sse_me_5 =	REG_FIELD(SWREG(139), 11, 31),
+	.cfg_enc_lambda_sse_me_6 =	REG_FIELD(SWREG(140), 11, 31),
+	.cfg_enc_lambda_sse_me_7 =	REG_FIELD(SWREG(141), 11, 31),
+	.cfg_enc_lambda_sse_me_8 =	REG_FIELD(SWREG(142), 11, 31),
+	.cfg_enc_lambda_sse_me_9 =	REG_FIELD(SWREG(143), 11, 31),
+	.cfg_enc_lambda_sse_me_10 =	REG_FIELD(SWREG(144), 11, 31),
+	.cfg_enc_lambda_sse_me_11 =	REG_FIELD(SWREG(145), 11, 31),
+	.cfg_enc_lambda_sse_me_12 =	REG_FIELD(SWREG(146), 11, 31),
+	.cfg_enc_lambda_sse_me_13 =	REG_FIELD(SWREG(147), 11, 31),
+	.cfg_enc_lambda_sse_me_14 =	REG_FIELD(SWREG(148), 11, 31),
+	.cfg_enc_lambda_sse_me_15 =	REG_FIELD(SWREG(149), 11, 31),
+	.cfg_enc_lambda_sse_me_16 =	REG_FIELD(SWREG(158), 11, 31),
+	.cfg_enc_lambda_sse_me_17 =	REG_FIELD(SWREG(159), 11, 31),
+	.cfg_enc_lambda_sse_me_18 =	REG_FIELD(SWREG(160), 11, 31),
+	.cfg_enc_lambda_sse_me_19 =	REG_FIELD(SWREG(161), 11, 31),
+	.cfg_enc_lambda_sse_me_20 =	REG_FIELD(SWREG(162), 11, 31),
+	.cfg_enc_lambda_sse_me_21 =	REG_FIELD(SWREG(163), 11, 31),
+	.cfg_enc_lambda_sse_me_22 =	REG_FIELD(SWREG(164), 11, 31),
+	.cfg_enc_lambda_sse_me_23 =	REG_FIELD(SWREG(165), 11, 31),
+	.cfg_enc_lambda_sse_me_24 =	REG_FIELD(SWREG(166), 11, 31),
+	.cfg_enc_lambda_sse_me_25 =	REG_FIELD(SWREG(167), 11, 31),
+	.cfg_enc_lambda_sse_me_26 =	REG_FIELD(SWREG(168), 11, 31),
+	.cfg_enc_lambda_sse_me_27 =	REG_FIELD(SWREG(169), 11, 31),
+	.cfg_enc_lambda_sse_me_28 =	REG_FIELD(SWREG(170), 11, 31),
+	.cfg_enc_lambda_sse_me_29 =	REG_FIELD(SWREG(171), 11, 31),
+	.cfg_enc_lambda_sse_me_30 =	REG_FIELD(SWREG(172), 11, 31),
+	.cfg_enc_lambda_sse_me_31 =	REG_FIELD(SWREG(173), 11, 31),
 };
 
 static const struct hantro_field_dec g1_field = {
@@ -353,7 +740,181 @@ static int hantro_regmap_fields_init_enc(struct hantro_dev *vpu,
 	if (!vpu->reg_fields_enc)
 		return -ENOMEM;
 
-	/* TODO: add encoder fields */
+	/* Encoder */
+	INIT_ENC_FIELD(enc_timeout_cycles);
+	INIT_ENC_FIELD(enc_timeout_e);
+	INIT_ENC_FIELD(enc_mode);
+	INIT_ENC_FIELD(enc_stream_mode);
+	INIT_ENC_FIELD(enc_enable);
+	INIT_ENC_FIELD(enc_pic_type);
+	INIT_ENC_FIELD(enc_pic_width);
+	INIT_ENC_FIELD(enc_pic_height);
+	INIT_ENC_FIELD(enc_burst_len);
+	INIT_ENC_FIELD(enc_clk_gate_en);
+	INIT_ENC_FIELD(enc_stream_buf_limit);
+	INIT_ENC_FIELD(enc_row_len);
+	INIT_ENC_FIELD(enc_overfill_r);
+	INIT_ENC_FIELD(enc_overfill_b);
+	INIT_ENC_FIELD(enc_src_format);
+	INIT_ENC_FIELD(enc_init_qp);
+	INIT_ENC_FIELD(enc_chroma_qp_offset);
+	INIT_ENC_FIELD(enc_deblocking_filter_dis);
+	INIT_ENC_FIELD(enc_slice_deblocking_filter_dis);
+	INIT_ENC_FIELD(enc_pps_deblocking_filter_override);
+	INIT_ENC_FIELD(enc_slice_deblocking_filter_override);
+	INIT_ENC_FIELD(enc_axi_r_outstanding_num);
+	INIT_ENC_FIELD(enc_axi_w_outstanding_num);
+	INIT_ENC_FIELD(enc_slice_alpha_div2);
+	INIT_ENC_FIELD(enc_slice_beta_div2);
+	INIT_ENC_FIELD(enc_idr_pic_id);
+	INIT_ENC_FIELD(enc_nal_ref_idc);
+	INIT_ENC_FIELD(enc_pps_id);
+	INIT_ENC_FIELD(enc_nal_unit_type);
+	INIT_ENC_FIELD(enc_frame_num);
+	INIT_ENC_FIELD(enc_min_cb_size);
+	INIT_ENC_FIELD(enc_max_cb_size);
+	INIT_ENC_FIELD(enc_min_trb_size);
+	INIT_ENC_FIELD(enc_max_trb_size);
+	INIT_ENC_FIELD(enc_slice_size);
+	INIT_ENC_FIELD(enc_nal_size_write);
+	INIT_ENC_FIELD(enc_cabac_init_idc);
+	INIT_ENC_FIELD(enc_pic_qp);
+	INIT_ENC_FIELD(enc_qp_frac);
+	INIT_ENC_FIELD(enc_entropy_coding_mode);
+	INIT_ENC_FIELD(enc_trans8x8_mode_en);
+	INIT_ENC_FIELD(enc_inter4x4_mode);
+	INIT_ENC_FIELD(enc_quarter_pixmv_dis);
+	INIT_ENC_FIELD(enc_addr_str);
+	INIT_ENC_FIELD(enc_addr_size_table);
+	INIT_ENC_FIELD(enc_addr_rec_luma);
+	INIT_ENC_FIELD(enc_addr_rec_chroma);
+	INIT_ENC_FIELD(enc_addr_rec_luma_4n);
+	INIT_ENC_FIELD(enc_addr_ref_luma_l0_4n0);
+	INIT_ENC_FIELD(enc_addr_ref_luma);
+	INIT_ENC_FIELD(enc_addr_ref_chroma);
+	INIT_ENC_FIELD(enc_addr_src_y);
+	INIT_ENC_FIELD(enc_addr_src_cb);
+	INIT_ENC_FIELD(enc_addr_src_cr);
+	INIT_ENC_FIELD(enc_log2_max_pic_order_cnt_lsb);
+	INIT_ENC_FIELD(enc_log2_max_frame_num);
+	INIT_ENC_FIELD(enc_pic_order_cnt_type);
+	INIT_ENC_FIELD(enc_l0_delta_framenum0);
+	INIT_ENC_FIELD(enc_l0_used_by_next_pic0);
+	INIT_ENC_FIELD(enc_l0_used_by_next_pic1);
+
+	INIT_ENC_FIELD(enc_lu_stride);
+	INIT_ENC_FIELD(enc_cr_stride);
+	INIT_ENC_FIELD(enc_ref_lu_stride);
+	INIT_ENC_FIELD(enc_ref_ds_lu_stride);
+	INIT_ENC_FIELD(enc_ref_cr_stride);
+	INIT_ENC_FIELD(enc_ipcm2_left);
+	INIT_ENC_FIELD(enc_ipcm2_right);
+	INIT_ENC_FIELD(enc_ipcm2_top);
+	INIT_ENC_FIELD(enc_ipcm2_bottom);
+
+	/* Quantization fields */
+	INIT_ENC_FIELD(enc_slice_qp_offset);
+	INIT_ENC_FIELD(enc_qp_min);
+	INIT_ENC_FIELD(enc_qp_max);
+
+	INIT_ENC_FIELD(enc_lambda_satd_me_0);
+	INIT_ENC_FIELD(enc_lambda_satd_me_1);
+	INIT_ENC_FIELD(enc_lambda_satd_me_2);
+	INIT_ENC_FIELD(enc_lambda_satd_me_3);
+	INIT_ENC_FIELD(enc_lambda_satd_me_4);
+	INIT_ENC_FIELD(enc_lambda_satd_me_5);
+	INIT_ENC_FIELD(enc_lambda_satd_me_6);
+	INIT_ENC_FIELD(enc_lambda_satd_me_7);
+	INIT_ENC_FIELD(enc_lambda_satd_me_8);
+	INIT_ENC_FIELD(enc_lambda_satd_me_9);
+	INIT_ENC_FIELD(enc_lambda_satd_me_10);
+	INIT_ENC_FIELD(enc_lambda_satd_me_11);
+	INIT_ENC_FIELD(enc_lambda_satd_me_12);
+	INIT_ENC_FIELD(enc_lambda_satd_me_13);
+	INIT_ENC_FIELD(enc_lambda_satd_me_14);
+	INIT_ENC_FIELD(enc_lambda_satd_me_15);
+	INIT_ENC_FIELD(enc_lambda_satd_me_16);
+	INIT_ENC_FIELD(enc_lambda_satd_me_17);
+	INIT_ENC_FIELD(enc_lambda_satd_me_18);
+	INIT_ENC_FIELD(enc_lambda_satd_me_19);
+	INIT_ENC_FIELD(enc_lambda_satd_me_20);
+	INIT_ENC_FIELD(enc_lambda_satd_me_21);
+	INIT_ENC_FIELD(enc_lambda_satd_me_22);
+	INIT_ENC_FIELD(enc_lambda_satd_me_23);
+	INIT_ENC_FIELD(enc_lambda_satd_me_24);
+	INIT_ENC_FIELD(enc_lambda_satd_me_25);
+	INIT_ENC_FIELD(enc_lambda_satd_me_26);
+	INIT_ENC_FIELD(enc_lambda_satd_me_27);
+	INIT_ENC_FIELD(enc_lambda_satd_me_28);
+	INIT_ENC_FIELD(enc_lambda_satd_me_29);
+	INIT_ENC_FIELD(enc_lambda_satd_me_30);
+	INIT_ENC_FIELD(enc_lambda_satd_me_31);
+
+	INIT_ENC_FIELD(enc_lambda_satd_0);
+	INIT_ENC_FIELD(enc_lambda_satd_1);
+	INIT_ENC_FIELD(enc_lambda_satd_2);
+	INIT_ENC_FIELD(enc_lambda_satd_3);
+	INIT_ENC_FIELD(enc_lambda_satd_4);
+	INIT_ENC_FIELD(enc_lambda_satd_5);
+	INIT_ENC_FIELD(enc_lambda_satd_6);
+	INIT_ENC_FIELD(enc_lambda_satd_7);
+	INIT_ENC_FIELD(enc_lambda_satd_8);
+	INIT_ENC_FIELD(enc_lambda_satd_9);
+	INIT_ENC_FIELD(enc_lambda_satd_10);
+	INIT_ENC_FIELD(enc_lambda_satd_11);
+	INIT_ENC_FIELD(enc_lambda_satd_12);
+	INIT_ENC_FIELD(enc_lambda_satd_13);
+	INIT_ENC_FIELD(enc_lambda_satd_14);
+	INIT_ENC_FIELD(enc_lambda_satd_15);
+	INIT_ENC_FIELD(enc_lambda_satd_16);
+	INIT_ENC_FIELD(enc_lambda_satd_17);
+	INIT_ENC_FIELD(enc_lambda_satd_18);
+	INIT_ENC_FIELD(enc_lambda_satd_19);
+	INIT_ENC_FIELD(enc_lambda_satd_20);
+	INIT_ENC_FIELD(enc_lambda_satd_21);
+	INIT_ENC_FIELD(enc_lambda_satd_22);
+	INIT_ENC_FIELD(enc_lambda_satd_23);
+	INIT_ENC_FIELD(enc_lambda_satd_24);
+	INIT_ENC_FIELD(enc_lambda_satd_25);
+	INIT_ENC_FIELD(enc_lambda_satd_26);
+	INIT_ENC_FIELD(enc_lambda_satd_27);
+	INIT_ENC_FIELD(enc_lambda_satd_28);
+	INIT_ENC_FIELD(enc_lambda_satd_29);
+	INIT_ENC_FIELD(enc_lambda_satd_30);
+	INIT_ENC_FIELD(enc_lambda_satd_31);
+
+	INIT_ENC_FIELD(enc_lambda_sse_me_0);
+	INIT_ENC_FIELD(enc_lambda_sse_me_1);
+	INIT_ENC_FIELD(enc_lambda_sse_me_2);
+	INIT_ENC_FIELD(enc_lambda_sse_me_3);
+	INIT_ENC_FIELD(enc_lambda_sse_me_4);
+	INIT_ENC_FIELD(enc_lambda_sse_me_5);
+	INIT_ENC_FIELD(enc_lambda_sse_me_6);
+	INIT_ENC_FIELD(enc_lambda_sse_me_7);
+	INIT_ENC_FIELD(enc_lambda_sse_me_8);
+	INIT_ENC_FIELD(enc_lambda_sse_me_9);
+	INIT_ENC_FIELD(enc_lambda_sse_me_10);
+	INIT_ENC_FIELD(enc_lambda_sse_me_11);
+	INIT_ENC_FIELD(enc_lambda_sse_me_12);
+	INIT_ENC_FIELD(enc_lambda_sse_me_13);
+	INIT_ENC_FIELD(enc_lambda_sse_me_14);
+	INIT_ENC_FIELD(enc_lambda_sse_me_15);
+	INIT_ENC_FIELD(enc_lambda_sse_me_16);
+	INIT_ENC_FIELD(enc_lambda_sse_me_17);
+	INIT_ENC_FIELD(enc_lambda_sse_me_18);
+	INIT_ENC_FIELD(enc_lambda_sse_me_19);
+	INIT_ENC_FIELD(enc_lambda_sse_me_20);
+	INIT_ENC_FIELD(enc_lambda_sse_me_21);
+	INIT_ENC_FIELD(enc_lambda_sse_me_22);
+	INIT_ENC_FIELD(enc_lambda_sse_me_23);
+	INIT_ENC_FIELD(enc_lambda_sse_me_24);
+	INIT_ENC_FIELD(enc_lambda_sse_me_25);
+	INIT_ENC_FIELD(enc_lambda_sse_me_26);
+	INIT_ENC_FIELD(enc_lambda_sse_me_27);
+	INIT_ENC_FIELD(enc_lambda_sse_me_28);
+	INIT_ENC_FIELD(enc_lambda_sse_me_29);
+	INIT_ENC_FIELD(enc_lambda_sse_me_30);
+	INIT_ENC_FIELD(enc_lambda_sse_me_31);
 
 	return 0;
 }
@@ -379,6 +940,21 @@ int hantro_regmap_init_enc(struct hantro_dev *vpu)
 
 	clk_bulk_disable(vpu->variant->num_clocks, vpu->clocks);
 
+	switch (vpu->core_hw_enc_rev) {
+	case HANTRO_H1_REV:
+		hantro_regmap_enc.max_register = 0x300;
+		field = &h1_field;
+		break;
+	case HANTRO_VC8000_REV:
+		hantro_regmap_enc.max_register = 0x640;
+		field = &vc8000e_field;
+		break;
+	default:
+		dev_err(vpu->dev, "Encoder revision 0x%x not supported by driver.\n",
+			vpu->core_hw_enc_rev);
+		return -ENODEV;
+	}
+
 	vpu->regs_enc = devm_regmap_init_mmio(vpu->dev, enc_base,
 					      &hantro_regmap_enc);
 	if (IS_ERR(vpu->regs_enc)) {
diff --git a/drivers/staging/media/hantro/hantro_regmap.h b/drivers/staging/media/hantro/hantro_regmap.h
index 083c4e92c4bd..549c16f72919 100644
--- a/drivers/staging/media/hantro/hantro_regmap.h
+++ b/drivers/staging/media/hantro/hantro_regmap.h
@@ -10,6 +10,7 @@
 #define HANTRO_REGMAP_H_
 
 #define HANTRO_G1_REV		0x6731
+#define HANTRO_H1_REV		0x4831
 #define HANTRO_VC8000_REV	0x8001
 
 #define SWREG(nr)		((nr) << 2)
@@ -111,7 +112,181 @@ struct hantro_regmap_fields_dec {
 };
 
 struct hantro_regmap_fields_enc {
-	/* TODO: populate encoder fields */
+	/* Encoder */
+	struct regmap_field *enc_timeout_cycles;
+	struct regmap_field *enc_timeout_e;
+	struct regmap_field *enc_mode;
+	struct regmap_field *enc_stream_mode;
+	struct regmap_field *enc_enable;
+	struct regmap_field *enc_pic_type;
+	struct regmap_field *enc_pic_width;
+	struct regmap_field *enc_pic_height;
+	struct regmap_field *enc_burst_len;
+	struct regmap_field *enc_clk_gate_en;
+	struct regmap_field *enc_stream_buf_limit;
+	struct regmap_field *enc_row_len;
+	struct regmap_field *enc_overfill_r;
+	struct regmap_field *enc_overfill_b;
+	struct regmap_field *enc_src_format;
+	struct regmap_field *enc_init_qp;
+	struct regmap_field *enc_qp_frac;
+	struct regmap_field *enc_chroma_qp_offset;
+	struct regmap_field *enc_deblocking_filter_dis;
+	struct regmap_field *enc_slice_deblocking_filter_dis;
+	struct regmap_field *enc_slice_deblocking_filter_override;
+	struct regmap_field *enc_pps_deblocking_filter_override;
+	struct regmap_field *enc_slice_alpha_div2;
+	struct regmap_field *enc_slice_beta_div2;
+	struct regmap_field *enc_idr_pic_id;
+	struct regmap_field *enc_nal_ref_idc;
+	struct regmap_field *enc_pps_id;
+	struct regmap_field *enc_nal_unit_type;
+	struct regmap_field *enc_frame_num;
+	struct regmap_field *enc_min_cb_size;
+	struct regmap_field *enc_max_cb_size;
+	struct regmap_field *enc_min_trb_size;
+	struct regmap_field *enc_max_trb_size;
+	struct regmap_field *enc_slice_size;
+	struct regmap_field *enc_nal_size_write;
+	struct regmap_field *enc_cabac_init_idc;
+	struct regmap_field *enc_pic_qp;
+	struct regmap_field *enc_entropy_coding_mode;
+	struct regmap_field *enc_axi_r_outstanding_num;
+	struct regmap_field *enc_axi_w_outstanding_num;
+	struct regmap_field *enc_trans8x8_mode_en;
+	struct regmap_field *enc_inter4x4_mode;
+	struct regmap_field *enc_quarter_pixmv_dis;
+	struct regmap_field *enc_addr_cabac;
+	struct regmap_field *enc_addr_str;
+	struct regmap_field *enc_addr_size_table;
+	struct regmap_field *enc_addr_rec_luma;
+	struct regmap_field *enc_addr_rec_luma_4n;
+	struct regmap_field *enc_addr_ref_luma_l0_4n0;
+	struct regmap_field *enc_addr_rec_chroma;
+	struct regmap_field *enc_addr_ref_luma;
+	struct regmap_field *enc_addr_ref_chroma;
+	struct regmap_field *enc_addr_src_y;
+	struct regmap_field *enc_addr_src_cb;
+	struct regmap_field *enc_addr_src_cr;
+	struct regmap_field *enc_log2_max_pic_order_cnt_lsb;
+	struct regmap_field *enc_log2_max_frame_num;
+	struct regmap_field *enc_pic_order_cnt_type;
+	struct regmap_field *enc_l0_delta_framenum0;
+	struct regmap_field *enc_l0_used_by_next_pic0;
+	struct regmap_field *enc_l0_used_by_next_pic1;
+
+	struct regmap_field *enc_lu_stride;
+	struct regmap_field *enc_cr_stride;
+	struct regmap_field *enc_ref_lu_stride;
+	struct regmap_field *enc_ref_ds_lu_stride;
+	struct regmap_field *enc_ref_cr_stride;
+	struct regmap_field *enc_ipcm2_left;
+	struct regmap_field *enc_ipcm2_right;
+	struct regmap_field *enc_ipcm2_top;
+	struct regmap_field *enc_ipcm2_bottom;
+
+	struct regmap_field *enc_slice_qp_offset;
+	struct regmap_field *enc_qp_min;
+	struct regmap_field *enc_qp_max;
+
+	struct regmap_field *enc_lambda_satd_me_0;
+	struct regmap_field *enc_lambda_satd_me_1;
+	struct regmap_field *enc_lambda_satd_me_2;
+	struct regmap_field *enc_lambda_satd_me_3;
+	struct regmap_field *enc_lambda_satd_me_4;
+	struct regmap_field *enc_lambda_satd_me_5;
+	struct regmap_field *enc_lambda_satd_me_6;
+	struct regmap_field *enc_lambda_satd_me_7;
+	struct regmap_field *enc_lambda_satd_me_8;
+	struct regmap_field *enc_lambda_satd_me_9;
+	struct regmap_field *enc_lambda_satd_me_10;
+	struct regmap_field *enc_lambda_satd_me_11;
+	struct regmap_field *enc_lambda_satd_me_12;
+	struct regmap_field *enc_lambda_satd_me_13;
+	struct regmap_field *enc_lambda_satd_me_14;
+	struct regmap_field *enc_lambda_satd_me_15;
+	struct regmap_field *enc_lambda_satd_me_16;
+	struct regmap_field *enc_lambda_satd_me_17;
+	struct regmap_field *enc_lambda_satd_me_18;
+	struct regmap_field *enc_lambda_satd_me_19;
+	struct regmap_field *enc_lambda_satd_me_20;
+	struct regmap_field *enc_lambda_satd_me_21;
+	struct regmap_field *enc_lambda_satd_me_22;
+	struct regmap_field *enc_lambda_satd_me_23;
+	struct regmap_field *enc_lambda_satd_me_24;
+	struct regmap_field *enc_lambda_satd_me_25;
+	struct regmap_field *enc_lambda_satd_me_26;
+	struct regmap_field *enc_lambda_satd_me_27;
+	struct regmap_field *enc_lambda_satd_me_28;
+	struct regmap_field *enc_lambda_satd_me_29;
+	struct regmap_field *enc_lambda_satd_me_30;
+	struct regmap_field *enc_lambda_satd_me_31;
+
+	struct regmap_field *enc_lambda_satd_0;
+	struct regmap_field *enc_lambda_satd_1;
+	struct regmap_field *enc_lambda_satd_2;
+	struct regmap_field *enc_lambda_satd_3;
+	struct regmap_field *enc_lambda_satd_4;
+	struct regmap_field *enc_lambda_satd_5;
+	struct regmap_field *enc_lambda_satd_6;
+	struct regmap_field *enc_lambda_satd_7;
+	struct regmap_field *enc_lambda_satd_8;
+	struct regmap_field *enc_lambda_satd_9;
+	struct regmap_field *enc_lambda_satd_10;
+	struct regmap_field *enc_lambda_satd_11;
+	struct regmap_field *enc_lambda_satd_12;
+	struct regmap_field *enc_lambda_satd_13;
+	struct regmap_field *enc_lambda_satd_14;
+	struct regmap_field *enc_lambda_satd_15;
+	struct regmap_field *enc_lambda_satd_16;
+	struct regmap_field *enc_lambda_satd_17;
+	struct regmap_field *enc_lambda_satd_18;
+	struct regmap_field *enc_lambda_satd_19;
+	struct regmap_field *enc_lambda_satd_20;
+	struct regmap_field *enc_lambda_satd_21;
+	struct regmap_field *enc_lambda_satd_22;
+	struct regmap_field *enc_lambda_satd_23;
+	struct regmap_field *enc_lambda_satd_24;
+	struct regmap_field *enc_lambda_satd_25;
+	struct regmap_field *enc_lambda_satd_26;
+	struct regmap_field *enc_lambda_satd_27;
+	struct regmap_field *enc_lambda_satd_28;
+	struct regmap_field *enc_lambda_satd_29;
+	struct regmap_field *enc_lambda_satd_30;
+	struct regmap_field *enc_lambda_satd_31;
+
+	struct regmap_field *enc_lambda_sse_me_0;
+	struct regmap_field *enc_lambda_sse_me_1;
+	struct regmap_field *enc_lambda_sse_me_2;
+	struct regmap_field *enc_lambda_sse_me_3;
+	struct regmap_field *enc_lambda_sse_me_4;
+	struct regmap_field *enc_lambda_sse_me_5;
+	struct regmap_field *enc_lambda_sse_me_6;
+	struct regmap_field *enc_lambda_sse_me_7;
+	struct regmap_field *enc_lambda_sse_me_8;
+	struct regmap_field *enc_lambda_sse_me_9;
+	struct regmap_field *enc_lambda_sse_me_10;
+	struct regmap_field *enc_lambda_sse_me_11;
+	struct regmap_field *enc_lambda_sse_me_12;
+	struct regmap_field *enc_lambda_sse_me_13;
+	struct regmap_field *enc_lambda_sse_me_14;
+	struct regmap_field *enc_lambda_sse_me_15;
+	struct regmap_field *enc_lambda_sse_me_16;
+	struct regmap_field *enc_lambda_sse_me_17;
+	struct regmap_field *enc_lambda_sse_me_18;
+	struct regmap_field *enc_lambda_sse_me_19;
+	struct regmap_field *enc_lambda_sse_me_20;
+	struct regmap_field *enc_lambda_sse_me_21;
+	struct regmap_field *enc_lambda_sse_me_22;
+	struct regmap_field *enc_lambda_sse_me_23;
+	struct regmap_field *enc_lambda_sse_me_24;
+	struct regmap_field *enc_lambda_sse_me_25;
+	struct regmap_field *enc_lambda_sse_me_26;
+	struct regmap_field *enc_lambda_sse_me_27;
+	struct regmap_field *enc_lambda_sse_me_28;
+	struct regmap_field *enc_lambda_sse_me_29;
+	struct regmap_field *enc_lambda_sse_me_30;
+	struct regmap_field *enc_lambda_sse_me_31;
 };
 
 int hantro_regmap_init_dec(struct hantro_dev *vpu);
-- 
2.28.0


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[Jonas Karlman]

Hi,

On 2020-10-12 22:59, Adrian Ratiu wrote:
> Dear all,
> 
> This series introduces a regmap infrastructure for the Hantro driver
> which is used to compensate for different HW-revision register layouts.
> To justify it h264 decoding capability is added for newer VC8000 chips.
> 
> This is a gradual conversion to the new infra - a complete conversion
> would have been very big and I do not have all the HW yet to test (I'm
> expecting a RK3399 shipment next week though ;). I think converting the
> h264 decoder provides a nice blueprint for how the other codecs can be
> converted and enabled for different HW revisions.
> 
> The end goal of this is to make the driver more generic and eliminate
> entirely custom boilerplate like `struct hantro_reg` or headers with
> core-specific bit manipulations like `hantro_g1_regs.h` and instead rely
> on the well-tested albeit more verbose regmap subsytem.
> 
> To give just two examples of bugs which are easily discovered by using
> more verbose regmap fields (very easy to compare with the datasheets)
> instead of relying on bit-magic tricks: G1_REG_DEC_CTRL3_INIT_QP(x) was
> off-by-1 and the wrong .clk_gate bit was set in hantro_postproc.c.
> 
> Anyway, this series also extends the MMIO regmap API to allow relaxed
> writes for the theoretical reason that avoiding unnecessary membarriers
> leads to less CPU usage and small improvements to battery life. However,
> in practice I could not measure differences between relaxed/non-relaxed
> IO, so I'm on the fence whether to keep or remove the relaxed calls.
> 
> What I could masure is the performance impact of adding more sub-reg
> field acesses: a constant ~ 20 microsecond bump per G1 h264 frame. This
> is acceptable considering the total time to decode a frame takes three
> orders of magnitude longer, i.e. miliseconds ranges, depending on the
> frame size and bitstream params, so it is an acceptable trade-off to
> have a more generic driver.

In the RK3399 variant all fields use completely different positions so
in order to make the driver fully generic all around 145 sub-reg fields
used for h264 needs to be converted, see [1] for a quick generation of
field mappings used for h264 decoding.

Any indication on how the performance will be impacted with 145 fields
compared to around 20 fields used in this series?

Another issue with RK3399 variant is that some fields use different
position depending on the codec used, e.g. two dec_ref_frames in [2].
Should we use codec specific field maps? or any other suggestion on
how we can handle such case?

[1] https://github.com/Kwiboo/rockchip-vpu-regtool/commit/8b88d94d2ed966c7d88d9a735c0c97368eb6c92d
[2] https://github.com/Kwiboo/rockchip-vpu-regtool/blob/master/rk3399_dec_regs.c#L1065
[3] https://github.com/Kwiboo/rockchip-vpu-regtool/commit/9498326296445a9ce153b585cc48e0cea05d3c93

Best regards,
Jonas

> 
> This has been tested on next-20201009 with imx8mq for G1 and an SoC with
> VC8000 which has not yet been added (hopefuly support lands soon).
> 
> Kind regards,
> Adrian
> 
> Adrian Ratiu (18):
>   media: hantro: document all int reg bits up to vc8000
>   media: hantro: make consistent use of decimal register notation
>   media: hantro: make G1_REG_SOFT_RESET Rockchip specific
>   media: hantro: add reset controller support
>   media: hantro: prepare clocks before variant inits are run
>   media: hantro: imx8mq: simplify ctrlblk reset logic
>   regmap: mmio: add config option to allow relaxed MMIO accesses
>   media: hantro: add initial MMIO regmap infrastructure
>   media: hantro: default regmap to relaxed MMIO
>   media: hantro: convert G1 h264 decoder to regmap fields
>   media: hantro: convert G1 postproc to regmap
>   media: hantro: add VC8000D h264 decoding
>   media: hantro: add VC8000D postproc support
>   media: hantro: make PP enablement logic a bit smarter
>   media: hantro: add user-selectable, platform-selectable H264 High10
>   media: hantro: rename h264_dec as it's not G1 specific anymore
>   media: hantro: add dump registers debug option before decode start
>   media: hantro: document encoder reg fields
> 
>  drivers/base/regmap/regmap-mmio.c             |   34 +-
>  drivers/staging/media/hantro/Makefile         |    3 +-
>  drivers/staging/media/hantro/hantro.h         |   79 +-
>  drivers/staging/media/hantro/hantro_drv.c     |   41 +-
>  drivers/staging/media/hantro/hantro_g1_regs.h |   92 +-
>  ...hantro_g1_h264_dec.c => hantro_h264_dec.c} |  237 +++-
>  drivers/staging/media/hantro/hantro_hw.h      |   23 +-
>  .../staging/media/hantro/hantro_postproc.c    |  144 ++-
>  drivers/staging/media/hantro/hantro_regmap.c  | 1015 +++++++++++++++++
>  drivers/staging/media/hantro/hantro_regmap.h  |  295 +++++
>  drivers/staging/media/hantro/hantro_v4l2.c    |    3 +-
>  drivers/staging/media/hantro/imx8m_vpu_hw.c   |   75 +-
>  drivers/staging/media/hantro/rk3288_vpu_hw.c  |    5 +-
>  include/linux/regmap.h                        |    5 +
>  14 files changed, 1795 insertions(+), 256 deletions(-)
>  rename drivers/staging/media/hantro/{hantro_g1_h264_dec.c => hantro_h264_dec.c} (58%)
>  create mode 100644 drivers/staging/media/hantro/hantro_regmap.c
>  create mode 100644 drivers/staging/media/hantro/hantro_regmap.h
> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[Adrian Ratiu]

Hi Jonas,

On Mon, 12 Oct 2020, Jonas Karlman <jonas@kwiboo.se> wrote:
> Hi, 
> 
> On 2020-10-12 22:59, Adrian Ratiu wrote: 
>> Dear all,  This series introduces a regmap infrastructure for 
>> the Hantro driver which is used to compensate for different 
>> HW-revision register layouts.  To justify it h264 decoding 
>> capability is added for newer VC8000 chips.   This is a gradual 
>> conversion to the new infra - a complete conversion would have 
>> been very big and I do not have all the HW yet to test (I'm 
>> expecting a RK3399 shipment next week though ;). I think 
>> converting the h264 decoder provides a nice blueprint for how 
>> the other codecs can be converted and enabled for different HW 
>> revisions.   The end goal of this is to make the driver more 
>> generic and eliminate entirely custom boilerplate like `struct 
>> hantro_reg` or headers with core-specific bit manipulations 
>> like `hantro_g1_regs.h` and instead rely on the well-tested 
>> albeit more verbose regmap subsytem.   To give just two 
>> examples of bugs which are easily discovered by using more 
>> verbose regmap fields (very easy to compare with the 
>> datasheets) instead of relying on bit-magic tricks: 
>> G1_REG_DEC_CTRL3_INIT_QP(x) was off-by-1 and the wrong 
>> .clk_gate bit was set in hantro_postproc.c.   Anyway, this 
>> series also extends the MMIO regmap API to allow relaxed writes 
>> for the theoretical reason that avoiding unnecessary 
>> membarriers leads to less CPU usage and small improvements to 
>> battery life. However, in practice I could not measure 
>> differences between relaxed/non-relaxed IO, so I'm on the fence 
>> whether to keep or remove the relaxed calls.   What I could 
>> masure is the performance impact of adding more sub-reg field 
>> acesses: a constant ~ 20 microsecond bump per G1 h264 
>> frame. This is acceptable considering the total time to decode 
>> a frame takes three orders of magnitude longer, 
>> i.e. miliseconds ranges, depending on the frame size and 
>> bitstream params, so it is an acceptable trade-off to have a 
>> more generic driver. 
> 
> In the RK3399 variant all fields use completely different 
> positions so in order to make the driver fully generic all 
> around 145 sub-reg fields used for h264 needs to be converted, 
> see [1] for a quick generation of field mappings used for h264 
> decoding. 
> 
> Any indication on how the performance will be impacted with 145 
> fields compared to around 20 fields used in this series? 

I'm aware of the RK3399 bigger layout divergence and have some 
commits converting more of the reg fields, but not all that is 
required for h264 on rk3399. I haven't seen a huge perf 
degradation but more measurements are needed, basically it depends 
on how often we go from writing a reg once to multiple times due 
to splitting.

I tried some benchmarks using regmap caching (both the default 
backends provided by the regmap subsystem, and a custom one I 
wrote) but they were not helping, perhaps if we had more fields 
then that would have more of an impact.

(btw some good news is I'm having a RK3399 SoC in the mail for an 
unrelated project and expect to receive it soon :D)

IMO there will always be a trade-off between optimizing the driver 
to squeeze the most perf out of the HW, eg optimize reg writes at 
low microsec level (which I think here is unnecessary) and making 
it more generic to support more HW.

In this case a fundamental question we need to ask ourselves is if 
the RK3399 "looks like another/different-enough HW" due to its 
bigger reg shuffling to warrant a separate driver or 
driver-within-a-driver architecture instead trying to bring it 
into the fold with the others, possibly degrading perf for 
everyone. I guess we'll have to see some benchmark numbers and an 
actual h264 implementation before deciding how to proceed with 
RK3399.

> 
> Another issue with RK3399 variant is that some fields use 
> different position depending on the codec used, e.g. two 
> dec_ref_frames in [2].  Should we use codec specific field maps? 
> or any other suggestion on how we can handle such case?

Yes, codec specific fields would be one idea, but I'd try to avoid 
it if possible to avoid unnecessary field definitions.

The regmap field API and config we currently use are just a flat 
structs (see hantro_regmap.[h|c]) but it doesn't have to be like 
that. Maybe we could organize it a bit better and in the future 
have some codec-level configs going on due to the regmap subsystem 
allowing de-coupling of the API (struct regmap_field) from the reg 
defs/configs (struct reg_field).

That is just an idea of the top of my head :) Will have to think a 
bit more about how to handle that specific use case in the 
future. Thanks!

>
> [1] https://github.com/Kwiboo/rockchip-vpu-regtool/commit/8b88d94d2ed966c7d88d9a735c0c97368eb6c92d
> [2] https://github.com/Kwiboo/rockchip-vpu-regtool/blob/master/rk3399_dec_regs.c#L1065
> [3] https://github.com/Kwiboo/rockchip-vpu-regtool/commit/9498326296445a9ce153b585cc48e0cea05d3c93
>
> Best regards,
> Jonas
>
>> 
>> This has been tested on next-20201009 with imx8mq for G1 and an SoC with
>> VC8000 which has not yet been added (hopefuly support lands soon).
>> 
>> Kind regards,
>> Adrian
>> 
>> Adrian Ratiu (18):
>>   media: hantro: document all int reg bits up to vc8000
>>   media: hantro: make consistent use of decimal register notation
>>   media: hantro: make G1_REG_SOFT_RESET Rockchip specific
>>   media: hantro: add reset controller support
>>   media: hantro: prepare clocks before variant inits are run
>>   media: hantro: imx8mq: simplify ctrlblk reset logic
>>   regmap: mmio: add config option to allow relaxed MMIO accesses
>>   media: hantro: add initial MMIO regmap infrastructure
>>   media: hantro: default regmap to relaxed MMIO
>>   media: hantro: convert G1 h264 decoder to regmap fields
>>   media: hantro: convert G1 postproc to regmap
>>   media: hantro: add VC8000D h264 decoding
>>   media: hantro: add VC8000D postproc support
>>   media: hantro: make PP enablement logic a bit smarter
>>   media: hantro: add user-selectable, platform-selectable H264 High10
>>   media: hantro: rename h264_dec as it's not G1 specific anymore
>>   media: hantro: add dump registers debug option before decode start
>>   media: hantro: document encoder reg fields
>> 
>>  drivers/base/regmap/regmap-mmio.c             |   34 +-
>>  drivers/staging/media/hantro/Makefile         |    3 +-
>>  drivers/staging/media/hantro/hantro.h         |   79 +-
>>  drivers/staging/media/hantro/hantro_drv.c     |   41 +-
>>  drivers/staging/media/hantro/hantro_g1_regs.h |   92 +-
>>  ...hantro_g1_h264_dec.c => hantro_h264_dec.c} |  237 +++-
>>  drivers/staging/media/hantro/hantro_hw.h      |   23 +-
>>  .../staging/media/hantro/hantro_postproc.c    |  144 ++-
>>  drivers/staging/media/hantro/hantro_regmap.c  | 1015 +++++++++++++++++
>>  drivers/staging/media/hantro/hantro_regmap.h  |  295 +++++
>>  drivers/staging/media/hantro/hantro_v4l2.c    |    3 +-
>>  drivers/staging/media/hantro/imx8m_vpu_hw.c   |   75 +-
>>  drivers/staging/media/hantro/rk3288_vpu_hw.c  |    5 +-
>>  include/linux/regmap.h                        |    5 +
>>  14 files changed, 1795 insertions(+), 256 deletions(-)
>>  rename drivers/staging/media/hantro/{hantro_g1_h264_dec.c => hantro_h264_dec.c} (58%)
>>  create mode 100644 drivers/staging/media/hantro/hantro_regmap.c
>>  create mode 100644 drivers/staging/media/hantro/hantro_regmap.h
>> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[Ezequiel Garcia]

On Mon, 2020-10-12 at 23:39 +0000, Jonas Karlman wrote:
> Hi,
> 
> On 2020-10-12 22:59, Adrian Ratiu wrote:
> > Dear all,
> > 
> > This series introduces a regmap infrastructure for the Hantro driver
> > which is used to compensate for different HW-revision register layouts.
> > To justify it h264 decoding capability is added for newer VC8000 chips.
> > 
> > This is a gradual conversion to the new infra - a complete conversion
> > would have been very big and I do not have all the HW yet to test (I'm
> > expecting a RK3399 shipment next week though ;). I think converting the
> > h264 decoder provides a nice blueprint for how the other codecs can be
> > converted and enabled for different HW revisions.
> > 
> > The end goal of this is to make the driver more generic and eliminate
> > entirely custom boilerplate like `struct hantro_reg` or headers with
> > core-specific bit manipulations like `hantro_g1_regs.h` and instead rely
> > on the well-tested albeit more verbose regmap subsytem.
> > 
> > To give just two examples of bugs which are easily discovered by using
> > more verbose regmap fields (very easy to compare with the datasheets)
> > instead of relying on bit-magic tricks: G1_REG_DEC_CTRL3_INIT_QP(x) was
> > off-by-1 and the wrong .clk_gate bit was set in hantro_postproc.c.
> > 
> > Anyway, this series also extends the MMIO regmap API to allow relaxed
> > writes for the theoretical reason that avoiding unnecessary membarriers
> > leads to less CPU usage and small improvements to battery life. However,
> > in practice I could not measure differences between relaxed/non-relaxed
> > IO, so I'm on the fence whether to keep or remove the relaxed calls.
> > 
> > What I could masure is the performance impact of adding more sub-reg
> > field acesses: a constant ~ 20 microsecond bump per G1 h264 frame. This
> > is acceptable considering the total time to decode a frame takes three
> > orders of magnitude longer, i.e. miliseconds ranges, depending on the
> > frame size and bitstream params, so it is an acceptable trade-off to
> > have a more generic driver.
> 
> In the RK3399 variant all fields use completely different positions so
> in order to make the driver fully generic all around 145 sub-reg fields
> used for h264 needs to be converted, see [1] for a quick generation of
> field mappings used for h264 decoding.
> 

Currently, we've only decided to support H.264 decoding via he RKVDEC
core on RK3399.

What your thoughts here Jonas, have you tested H.264 on RK3399 with
the G1 core? If it works, what benefits do we get from enabling both
cores?

Thanks!
Ezequiel

> Any indication on how the performance will be impacted with 145 fields
> compared to around 20 fields used in this series?
> 
> Another issue with RK3399 variant is that some fields use different
> position depending on the codec used, e.g. two dec_ref_frames in [2].
> Should we use codec specific field maps? or any other suggestion on
> how we can handle such case?
> 
> [1] https://github.com/Kwiboo/rockchip-vpu-regtool/commit/8b88d94d2ed966c7d88d9a735c0c97368eb6c92d
> [2] https://github.com/Kwiboo/rockchip-vpu-regtool/blob/master/rk3399_dec_regs.c#L1065
> [3] https://github.com/Kwiboo/rockchip-vpu-regtool/commit/9498326296445a9ce153b585cc48e0cea05d3c93
> 
> Best regards,
> Jonas
> 
> > This has been tested on next-20201009 with imx8mq for G1 and an SoC with
> > VC8000 which has not yet been added (hopefuly support lands soon).
> > 
> > Kind regards,
> > Adrian
> > 
> > Adrian Ratiu (18):
> >   media: hantro: document all int reg bits up to vc8000
> >   media: hantro: make consistent use of decimal register notation
> >   media: hantro: make G1_REG_SOFT_RESET Rockchip specific
> >   media: hantro: add reset controller support
> >   media: hantro: prepare clocks before variant inits are run
> >   media: hantro: imx8mq: simplify ctrlblk reset logic
> >   regmap: mmio: add config option to allow relaxed MMIO accesses
> >   media: hantro: add initial MMIO regmap infrastructure
> >   media: hantro: default regmap to relaxed MMIO
> >   media: hantro: convert G1 h264 decoder to regmap fields
> >   media: hantro: convert G1 postproc to regmap
> >   media: hantro: add VC8000D h264 decoding
> >   media: hantro: add VC8000D postproc support
> >   media: hantro: make PP enablement logic a bit smarter
> >   media: hantro: add user-selectable, platform-selectable H264 High10
> >   media: hantro: rename h264_dec as it's not G1 specific anymore
> >   media: hantro: add dump registers debug option before decode start
> >   media: hantro: document encoder reg fields
> > 
> >  drivers/base/regmap/regmap-mmio.c             |   34 +-
> >  drivers/staging/media/hantro/Makefile         |    3 +-
> >  drivers/staging/media/hantro/hantro.h         |   79 +-
> >  drivers/staging/media/hantro/hantro_drv.c     |   41 +-
> >  drivers/staging/media/hantro/hantro_g1_regs.h |   92 +-
> >  ...hantro_g1_h264_dec.c => hantro_h264_dec.c} |  237 +++-
> >  drivers/staging/media/hantro/hantro_hw.h      |   23 +-
> >  .../staging/media/hantro/hantro_postproc.c    |  144 ++-
> >  drivers/staging/media/hantro/hantro_regmap.c  | 1015 +++++++++++++++++
> >  drivers/staging/media/hantro/hantro_regmap.h  |  295 +++++
> >  drivers/staging/media/hantro/hantro_v4l2.c    |    3 +-
> >  drivers/staging/media/hantro/imx8m_vpu_hw.c   |   75 +-
> >  drivers/staging/media/hantro/rk3288_vpu_hw.c  |    5 +-
> >  include/linux/regmap.h                        |    5 +
> >  14 files changed, 1795 insertions(+), 256 deletions(-)
> >  rename drivers/staging/media/hantro/{hantro_g1_h264_dec.c => hantro_h264_dec.c} (58%)
> >  create mode 100644 drivers/staging/media/hantro/hantro_regmap.c
> >  create mode 100644 drivers/staging/media/hantro/hantro_regmap.h
> > 



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[Jonas Karlman]

On 2020-10-29 13:38, Ezequiel Garcia wrote:
> On Mon, 2020-10-12 at 23:39 +0000, Jonas Karlman wrote:
>> Hi,
>>
>> On 2020-10-12 22:59, Adrian Ratiu wrote:
>>> Dear all,
>>>
>>> This series introduces a regmap infrastructure for the Hantro driver
>>> which is used to compensate for different HW-revision register layouts.
>>> To justify it h264 decoding capability is added for newer VC8000 chips.
>>>
>>> This is a gradual conversion to the new infra - a complete conversion
>>> would have been very big and I do not have all the HW yet to test (I'm
>>> expecting a RK3399 shipment next week though ;). I think converting the
>>> h264 decoder provides a nice blueprint for how the other codecs can be
>>> converted and enabled for different HW revisions.
>>>
>>> The end goal of this is to make the driver more generic and eliminate
>>> entirely custom boilerplate like `struct hantro_reg` or headers with
>>> core-specific bit manipulations like `hantro_g1_regs.h` and instead rely
>>> on the well-tested albeit more verbose regmap subsytem.
>>>
>>> To give just two examples of bugs which are easily discovered by using
>>> more verbose regmap fields (very easy to compare with the datasheets)
>>> instead of relying on bit-magic tricks: G1_REG_DEC_CTRL3_INIT_QP(x) was
>>> off-by-1 and the wrong .clk_gate bit was set in hantro_postproc.c.
>>>
>>> Anyway, this series also extends the MMIO regmap API to allow relaxed
>>> writes for the theoretical reason that avoiding unnecessary membarriers
>>> leads to less CPU usage and small improvements to battery life. However,
>>> in practice I could not measure differences between relaxed/non-relaxed
>>> IO, so I'm on the fence whether to keep or remove the relaxed calls.
>>>
>>> What I could masure is the performance impact of adding more sub-reg
>>> field acesses: a constant ~ 20 microsecond bump per G1 h264 frame. This
>>> is acceptable considering the total time to decode a frame takes three
>>> orders of magnitude longer, i.e. miliseconds ranges, depending on the
>>> frame size and bitstream params, so it is an acceptable trade-off to
>>> have a more generic driver.
>>
>> In the RK3399 variant all fields use completely different positions so
>> in order to make the driver fully generic all around 145 sub-reg fields
>> used for h264 needs to be converted, see [1] for a quick generation of
>> field mappings used for h264 decoding.
>>
> 
> Currently, we've only decided to support H.264 decoding via he RKVDEC
> core on RK3399.
> 
> What your thoughts here Jonas, have you tested H.264 on RK3399 with
> the G1 core? If it works, what benefits do we get from enabling both
> cores?

The G1 core was working back in Dec/Jan/Feb and was used for H.264 decoding in
LibreELEC nightly images until the rkvdec h264 driver was submitted/merged.

For RK3399 and other SoCs that both contain RKVDEC and VDPU2 IP it may not be
much of a benefit. Possible for decoding multiple videos in parallel,
it is unclear to me if both IP can be used at the same time.

There are however SoCs that only have VDPU2 IP (px30/rk3326 and rk1808)
that could benefit from adding support for the VDPU2 IP, see [1].

Should I submit the rk3399 variant in similar style as the rk3399 mpeg2 decoder?
Or should I try and adopt it to be based on this series and use regmap?

[1] https://github.com/HermanChen/mpp/blob/develop/osal/mpp_platform.cpp#L80-L82

Best regards,
Jonas

> 
> Thanks!
> Ezequiel
> 
>> Any indication on how the performance will be impacted with 145 fields
>> compared to around 20 fields used in this series?
>>
>> Another issue with RK3399 variant is that some fields use different
>> position depending on the codec used, e.g. two dec_ref_frames in [2].
>> Should we use codec specific field maps? or any other suggestion on
>> how we can handle such case?
>>
>> [1] https://github.com/Kwiboo/rockchip-vpu-regtool/commit/8b88d94d2ed966c7d88d9a735c0c97368eb6c92d
>> [2] https://github.com/Kwiboo/rockchip-vpu-regtool/blob/master/rk3399_dec_regs.c#L1065
>> [3] https://github.com/Kwiboo/rockchip-vpu-regtool/commit/9498326296445a9ce153b585cc48e0cea05d3c93
>>
>> Best regards,
>> Jonas
>>
>>> This has been tested on next-20201009 with imx8mq for G1 and an SoC with
>>> VC8000 which has not yet been added (hopefuly support lands soon).
>>>
>>> Kind regards,
>>> Adrian
>>>
>>> Adrian Ratiu (18):
>>>   media: hantro: document all int reg bits up to vc8000
>>>   media: hantro: make consistent use of decimal register notation
>>>   media: hantro: make G1_REG_SOFT_RESET Rockchip specific
>>>   media: hantro: add reset controller support
>>>   media: hantro: prepare clocks before variant inits are run
>>>   media: hantro: imx8mq: simplify ctrlblk reset logic
>>>   regmap: mmio: add config option to allow relaxed MMIO accesses
>>>   media: hantro: add initial MMIO regmap infrastructure
>>>   media: hantro: default regmap to relaxed MMIO
>>>   media: hantro: convert G1 h264 decoder to regmap fields
>>>   media: hantro: convert G1 postproc to regmap
>>>   media: hantro: add VC8000D h264 decoding
>>>   media: hantro: add VC8000D postproc support
>>>   media: hantro: make PP enablement logic a bit smarter
>>>   media: hantro: add user-selectable, platform-selectable H264 High10
>>>   media: hantro: rename h264_dec as it's not G1 specific anymore
>>>   media: hantro: add dump registers debug option before decode start
>>>   media: hantro: document encoder reg fields
>>>
>>>  drivers/base/regmap/regmap-mmio.c             |   34 +-
>>>  drivers/staging/media/hantro/Makefile         |    3 +-
>>>  drivers/staging/media/hantro/hantro.h         |   79 +-
>>>  drivers/staging/media/hantro/hantro_drv.c     |   41 +-
>>>  drivers/staging/media/hantro/hantro_g1_regs.h |   92 +-
>>>  ...hantro_g1_h264_dec.c => hantro_h264_dec.c} |  237 +++-
>>>  drivers/staging/media/hantro/hantro_hw.h      |   23 +-
>>>  .../staging/media/hantro/hantro_postproc.c    |  144 ++-
>>>  drivers/staging/media/hantro/hantro_regmap.c  | 1015 +++++++++++++++++
>>>  drivers/staging/media/hantro/hantro_regmap.h  |  295 +++++
>>>  drivers/staging/media/hantro/hantro_v4l2.c    |    3 +-
>>>  drivers/staging/media/hantro/imx8m_vpu_hw.c   |   75 +-
>>>  drivers/staging/media/hantro/rk3288_vpu_hw.c  |    5 +-
>>>  include/linux/regmap.h                        |    5 +
>>>  14 files changed, 1795 insertions(+), 256 deletions(-)
>>>  rename drivers/staging/media/hantro/{hantro_g1_h264_dec.c => hantro_h264_dec.c} (58%)
>>>  create mode 100644 drivers/staging/media/hantro/hantro_regmap.c
>>>  create mode 100644 drivers/staging/media/hantro/hantro_regmap.h
>>>
> 
> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[Ezequiel Garcia]

On Thu, 2020-10-29 at 16:21 +0000, Jonas Karlman wrote:
> On 2020-10-29 13:38, Ezequiel Garcia wrote:
> > On Mon, 2020-10-12 at 23:39 +0000, Jonas Karlman wrote:
> > > Hi,
> > > 
> > > On 2020-10-12 22:59, Adrian Ratiu wrote:
> > > > Dear all,
> > > > 
> > > > This series introduces a regmap infrastructure for the Hantro driver
> > > > which is used to compensate for different HW-revision register layouts.
> > > > To justify it h264 decoding capability is added for newer VC8000 chips.
> > > > 
> > > > This is a gradual conversion to the new infra - a complete conversion
> > > > would have been very big and I do not have all the HW yet to test (I'm
> > > > expecting a RK3399 shipment next week though ;). I think converting the
> > > > h264 decoder provides a nice blueprint for how the other codecs can be
> > > > converted and enabled for different HW revisions.
> > > > 
> > > > The end goal of this is to make the driver more generic and eliminate
> > > > entirely custom boilerplate like `struct hantro_reg` or headers with
> > > > core-specific bit manipulations like `hantro_g1_regs.h` and instead rely
> > > > on the well-tested albeit more verbose regmap subsytem.
> > > > 
> > > > To give just two examples of bugs which are easily discovered by using
> > > > more verbose regmap fields (very easy to compare with the datasheets)
> > > > instead of relying on bit-magic tricks: G1_REG_DEC_CTRL3_INIT_QP(x) was
> > > > off-by-1 and the wrong .clk_gate bit was set in hantro_postproc.c.
> > > > 
> > > > Anyway, this series also extends the MMIO regmap API to allow relaxed
> > > > writes for the theoretical reason that avoiding unnecessary membarriers
> > > > leads to less CPU usage and small improvements to battery life. However,
> > > > in practice I could not measure differences between relaxed/non-relaxed
> > > > IO, so I'm on the fence whether to keep or remove the relaxed calls.
> > > > 
> > > > What I could masure is the performance impact of adding more sub-reg
> > > > field acesses: a constant ~ 20 microsecond bump per G1 h264 frame. This
> > > > is acceptable considering the total time to decode a frame takes three
> > > > orders of magnitude longer, i.e. miliseconds ranges, depending on the
> > > > frame size and bitstream params, so it is an acceptable trade-off to
> > > > have a more generic driver.
> > > 
> > > In the RK3399 variant all fields use completely different positions so
> > > in order to make the driver fully generic all around 145 sub-reg fields
> > > used for h264 needs to be converted, see [1] for a quick generation of
> > > field mappings used for h264 decoding.
> > > 
> > 
> > Currently, we've only decided to support H.264 decoding via he RKVDEC
> > core on RK3399.
> > 
> > What your thoughts here Jonas, have you tested H.264 on RK3399 with
> > the G1 core? If it works, what benefits do we get from enabling both
> > cores?
> 
> The G1 core was working back in Dec/Jan/Feb and was used for H.264 decoding in
> LibreELEC nightly images until the rkvdec h264 driver was submitted/merged.
> 
> For RK3399 and other SoCs that both contain RKVDEC and VDPU2 IP it may not be
> much of a benefit. Possible for decoding multiple videos in parallel,
> it is unclear to me if both IP can be used at the same time.
> 
> There are however SoCs that only have VDPU2 IP (px30/rk3326 and rk1808)
> that could benefit from adding support for the VDPU2 IP, see [1].
> 
> Should I submit the rk3399 variant in similar style as the rk3399 mpeg2 decoder?
> Or should I try and adopt it to be based on this series and use regmap?
> 

I'm inclined to take a patch that is as uninvasive as possible first.
If it's easy enough to submit something that adds just the minimum for
VDPU2, then please go for it.

We can do the cleaning later.

As a stretch, if you could add px30 (which seems to be an alias for
rk3326, in terms of codec support), that would be nice as well.
(I have ordered an odroid with rk3326 but shipment is taking ages). 

Thanks,
Ezequiel

> [1] https://github.com/HermanChen/mpp/blob/develop/osal/mpp_platform.cpp#L80-L82
> 
> Best regards,
> Jonas
> 
> > Thanks!
> > Ezequiel
> > 
> > > Any indication on how the performance will be impacted with 145 fields
> > > compared to around 20 fields used in this series?
> > > 
> > > Another issue with RK3399 variant is that some fields use different
> > > position depending on the codec used, e.g. two dec_ref_frames in [2].
> > > Should we use codec specific field maps? or any other suggestion on
> > > how we can handle such case?
> > > 
> > > [1] https://github.com/Kwiboo/rockchip-vpu-regtool/commit/8b88d94d2ed966c7d88d9a735c0c97368eb6c92d
> > > [2] https://github.com/Kwiboo/rockchip-vpu-regtool/blob/master/rk3399_dec_regs.c#L1065
> > > [3] https://github.com/Kwiboo/rockchip-vpu-regtool/commit/9498326296445a9ce153b585cc48e0cea05d3c93
> > > 
> > > Best regards,
> > > Jonas
> > > 
> > > > This has been tested on next-20201009 with imx8mq for G1 and an SoC with
> > > > VC8000 which has not yet been added (hopefuly support lands soon).
> > > > 
> > > > Kind regards,
> > > > Adrian
> > > > 
> > > > Adrian Ratiu (18):
> > > >   media: hantro: document all int reg bits up to vc8000
> > > >   media: hantro: make consistent use of decimal register notation
> > > >   media: hantro: make G1_REG_SOFT_RESET Rockchip specific
> > > >   media: hantro: add reset controller support
> > > >   media: hantro: prepare clocks before variant inits are run
> > > >   media: hantro: imx8mq: simplify ctrlblk reset logic
> > > >   regmap: mmio: add config option to allow relaxed MMIO accesses
> > > >   media: hantro: add initial MMIO regmap infrastructure
> > > >   media: hantro: default regmap to relaxed MMIO
> > > >   media: hantro: convert G1 h264 decoder to regmap fields
> > > >   media: hantro: convert G1 postproc to regmap
> > > >   media: hantro: add VC8000D h264 decoding
> > > >   media: hantro: add VC8000D postproc support
> > > >   media: hantro: make PP enablement logic a bit smarter
> > > >   media: hantro: add user-selectable, platform-selectable H264 High10
> > > >   media: hantro: rename h264_dec as it's not G1 specific anymore
> > > >   media: hantro: add dump registers debug option before decode start
> > > >   media: hantro: document encoder reg fields
> > > > 
> > > >  drivers/base/regmap/regmap-mmio.c             |   34 +-
> > > >  drivers/staging/media/hantro/Makefile         |    3 +-
> > > >  drivers/staging/media/hantro/hantro.h         |   79 +-
> > > >  drivers/staging/media/hantro/hantro_drv.c     |   41 +-
> > > >  drivers/staging/media/hantro/hantro_g1_regs.h |   92 +-
> > > >  ...hantro_g1_h264_dec.c => hantro_h264_dec.c} |  237 +++-
> > > >  drivers/staging/media/hantro/hantro_hw.h      |   23 +-
> > > >  .../staging/media/hantro/hantro_postproc.c    |  144 ++-
> > > >  drivers/staging/media/hantro/hantro_regmap.c  | 1015 +++++++++++++++++
> > > >  drivers/staging/media/hantro/hantro_regmap.h  |  295 +++++
> > > >  drivers/staging/media/hantro/hantro_v4l2.c    |    3 +-
> > > >  drivers/staging/media/hantro/imx8m_vpu_hw.c   |   75 +-
> > > >  drivers/staging/media/hantro/rk3288_vpu_hw.c  |    5 +-
> > > >  include/linux/regmap.h                        |    5 +
> > > >  14 files changed, 1795 insertions(+), 256 deletions(-)
> > > >  rename drivers/staging/media/hantro/{hantro_g1_h264_dec.c => hantro_h264_dec.c} (58%)
> > > >  create mode 100644 drivers/staging/media/hantro/hantro_regmap.c
> > > >  create mode 100644 drivers/staging/media/hantro/hantro_regmap.h
> > > > 



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[Ezequiel Garcia]

Hello Adrian,

On Mon, 2020-10-12 at 23:59 +0300, Adrian Ratiu wrote:
> Dear all,
> 
> This series introduces a regmap infrastructure for the Hantro driver
> which is used to compensate for different HW-revision register layouts.
> To justify it h264 decoding capability is added for newer VC8000 chips.
> 
> This is a gradual conversion to the new infra - a complete conversion
> would have been very big and I do not have all the HW yet to test (I'm
> expecting a RK3399 shipment next week though ;). I think converting the
> h264 decoder provides a nice blueprint for how the other codecs can be
> converted and enabled for different HW revisions.
> 
> The end goal of this is to make the driver more generic and eliminate
> entirely custom boilerplate like `struct hantro_reg` or headers with
> core-specific bit manipulations like `hantro_g1_regs.h` and instead rely
> on the well-tested albeit more verbose regmap subsytem.
> 
> To give just two examples of bugs which are easily discovered by using
> more verbose regmap fields (very easy to compare with the datasheets)
> instead of relying on bit-magic tricks: G1_REG_DEC_CTRL3_INIT_QP(x) was
> off-by-1 and the wrong .clk_gate bit was set in hantro_postproc.c.
> 
> Anyway, this series also extends the MMIO regmap API to allow relaxed
> writes for the theoretical reason that avoiding unnecessary membarriers
> leads to less CPU usage and small improvements to battery life. However,
> in practice I could not measure differences between relaxed/non-relaxed
> IO, so I'm on the fence whether to keep or remove the relaxed calls.
> 
> What I could masure is the performance impact of adding more sub-reg
> field acesses: a constant ~ 20 microsecond bump per G1 h264 frame. This
> is acceptable considering the total time to decode a frame takes three
> orders of magnitude longer, i.e. miliseconds ranges, depending on the
> frame size and bitstream params, so it is an acceptable trade-off to
> have a more generic driver.
> 

Before going forward with using regmap, I would like to have a sense
of the footprint it adds, and see if we can avoid that 20 us penalty.

I'd also like to try another approach, something that has less
memory footprint and less runtime penalty.

How about something like this:

#define G1_PIC_WIDTH 4, 0xff8, 23                                                
#define ...
                                 
struct hantro_swreg {                                                            
        u32 value[399 /*whatever size goes here*/];                                                          
};                                                                               
                                                                                 
void hantro_reg_write(struct hantro_swreg *r,                                   
                      unsigned int swreg, u32 mask, u32 offset, u32 new_val)     
{                                                                                
        r->value[swreg] = (r->value[swreg] & ~(mask)) |                          
                          ((new_val << offset) & mask);                          
}

Which you can then use in a very similar way as the current proposal:

hantro_reg_write(&swreg, G1_PIC_WIDTH, width);

The first advantage here is that we no longer have any
footprint for the fields.

The ugly macros for "4, 0xff8, 23" can be auto-generated from
existing vendor headers, when possible, so that shouldn't
bother us.

The register set is "flushed" using _relaxed, but it
could be still costly.

If that is indeed costly, perhaps we can avoid writing
the entire set by having a dirty bit somewhere.

In any case, it's worth exploring our options first, I think.

PS: Another option is to just fork RK3399 to its
own driver and call the day, given how different it is :)

Thanks!
Ezequiel



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[Robin Murphy]

On 2020-10-29 13:07, Ezequiel Garcia wrote:
> Hello Adrian,
> 
> On Mon, 2020-10-12 at 23:59 +0300, Adrian Ratiu wrote:
>> Dear all,
>>
>> This series introduces a regmap infrastructure for the Hantro driver
>> which is used to compensate for different HW-revision register layouts.
>> To justify it h264 decoding capability is added for newer VC8000 chips.
>>
>> This is a gradual conversion to the new infra - a complete conversion
>> would have been very big and I do not have all the HW yet to test (I'm
>> expecting a RK3399 shipment next week though ;). I think converting the
>> h264 decoder provides a nice blueprint for how the other codecs can be
>> converted and enabled for different HW revisions.
>>
>> The end goal of this is to make the driver more generic and eliminate
>> entirely custom boilerplate like `struct hantro_reg` or headers with
>> core-specific bit manipulations like `hantro_g1_regs.h` and instead rely
>> on the well-tested albeit more verbose regmap subsytem.
>>
>> To give just two examples of bugs which are easily discovered by using
>> more verbose regmap fields (very easy to compare with the datasheets)
>> instead of relying on bit-magic tricks: G1_REG_DEC_CTRL3_INIT_QP(x) was
>> off-by-1 and the wrong .clk_gate bit was set in hantro_postproc.c.
>>
>> Anyway, this series also extends the MMIO regmap API to allow relaxed
>> writes for the theoretical reason that avoiding unnecessary membarriers
>> leads to less CPU usage and small improvements to battery life. However,
>> in practice I could not measure differences between relaxed/non-relaxed
>> IO, so I'm on the fence whether to keep or remove the relaxed calls.
>>
>> What I could masure is the performance impact of adding more sub-reg
>> field acesses: a constant ~ 20 microsecond bump per G1 h264 frame. This
>> is acceptable considering the total time to decode a frame takes three
>> orders of magnitude longer, i.e. miliseconds ranges, depending on the
>> frame size and bitstream params, so it is an acceptable trade-off to
>> have a more generic driver.
>>
> 
> Before going forward with using regmap, I would like to have a sense
> of the footprint it adds, and see if we can avoid that 20 us penalty.
> 
> I'd also like to try another approach, something that has less
> memory footprint and less runtime penalty.
> 
> How about something like this:
> 
> #define G1_PIC_WIDTH 4, 0xff8, 23
> #define ...
>                                   
> struct hantro_swreg {
>          u32 value[399 /*whatever size goes here*/];
> };
>                                                                                   
> void hantro_reg_write(struct hantro_swreg *r,
>                        unsigned int swreg, u32 mask, u32 offset, u32 new_val)
> {
>          r->value[swreg] = (r->value[swreg] & ~(mask)) |
>                            ((new_val << offset) & mask);
> }
> 
> Which you can then use in a very similar way as the current proposal:
> 
> hantro_reg_write(&swreg, G1_PIC_WIDTH, width);
> 
> The first advantage here is that we no longer have any
> footprint for the fields.
> 
> The ugly macros for "4, 0xff8, 23" can be auto-generated from
> existing vendor headers, when possible, so that shouldn't
> bother us.
> 
> The register set is "flushed" using _relaxed, but it
> could be still costly.
> 
> If that is indeed costly, perhaps we can avoid writing
> the entire set by having a dirty bit somewhere.
> 
> In any case, it's worth exploring our options first, I think.

Or maybe the regmap API itself deserves extending with a "deferred" 
operating mode where updates to the cached state can be separated from 
committing that state to the underlying hardware.

...which, after a brief code search out of curiosity, apparently already 
exists in the form of regcache_cache_only()/regcache_sync(), so there's 
probably no need to reinvent it :)

Robin.

> 
> PS: Another option is to just fork RK3399 to its
> own driver and call the day, given how different it is :)
> 
> Thanks!
> Ezequiel
> 
> 
> 
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 642 bytes --]

On Thu, Oct 29, 2020 at 02:15:10PM +0000, Robin Murphy wrote:

> Or maybe the regmap API itself deserves extending with a "deferred"
> operating mode where updates to the cached state can be separated from
> committing that state to the underlying hardware.

> ...which, after a brief code search out of curiosity, apparently already
> exists in the form of regcache_cache_only()/regcache_sync(), so there's
> probably no need to reinvent it :)

Yes, exactly.  One of the big use cases for regmap on MMIO devices is
being able to access the register map without the hardware being there,
this would be another application of the cache stuff.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[Ezequiel Garcia]

Hi Robin,

On Thu, 2020-10-29 at 14:15 +0000, Robin Murphy wrote:
> On 2020-10-29 13:07, Ezequiel Garcia wrote:
> > Hello Adrian,
> > 
> > On Mon, 2020-10-12 at 23:59 +0300, Adrian Ratiu wrote:
> > > Dear all,
> > > 
> > > This series introduces a regmap infrastructure for the Hantro driver
> > > which is used to compensate for different HW-revision register layouts.
> > > To justify it h264 decoding capability is added for newer VC8000 chips.
> > > 
> > > This is a gradual conversion to the new infra - a complete conversion
> > > would have been very big and I do not have all the HW yet to test (I'm
> > > expecting a RK3399 shipment next week though ;). I think converting the
> > > h264 decoder provides a nice blueprint for how the other codecs can be
> > > converted and enabled for different HW revisions.
> > > 
> > > The end goal of this is to make the driver more generic and eliminate
> > > entirely custom boilerplate like `struct hantro_reg` or headers with
> > > core-specific bit manipulations like `hantro_g1_regs.h` and instead rely
> > > on the well-tested albeit more verbose regmap subsytem.
> > > 
> > > To give just two examples of bugs which are easily discovered by using
> > > more verbose regmap fields (very easy to compare with the datasheets)
> > > instead of relying on bit-magic tricks: G1_REG_DEC_CTRL3_INIT_QP(x) was
> > > off-by-1 and the wrong .clk_gate bit was set in hantro_postproc.c.
> > > 
> > > Anyway, this series also extends the MMIO regmap API to allow relaxed
> > > writes for the theoretical reason that avoiding unnecessary membarriers
> > > leads to less CPU usage and small improvements to battery life. However,
> > > in practice I could not measure differences between relaxed/non-relaxed
> > > IO, so I'm on the fence whether to keep or remove the relaxed calls.
> > > 
> > > What I could masure is the performance impact of adding more sub-reg
> > > field acesses: a constant ~ 20 microsecond bump per G1 h264 frame. This
> > > is acceptable considering the total time to decode a frame takes three
> > > orders of magnitude longer, i.e. miliseconds ranges, depending on the
> > > frame size and bitstream params, so it is an acceptable trade-off to
> > > have a more generic driver.
> > > 
> > 
> > Before going forward with using regmap, I would like to have a sense
> > of the footprint it adds, and see if we can avoid that 20 us penalty.
> > 
> > I'd also like to try another approach, something that has less
> > memory footprint and less runtime penalty.
> > 
> > How about something like this:
> > 
> > #define G1_PIC_WIDTH 4, 0xff8, 23
> > #define ...
> >                                   
> > struct hantro_swreg {
> >          u32 value[399 /*whatever size goes here*/];
> > };
> >                                                                                   
> > void hantro_reg_write(struct hantro_swreg *r,
> >                        unsigned int swreg, u32 mask, u32 offset, u32 new_val)
> > {
> >          r->value[swreg] = (r->value[swreg] & ~(mask)) |
> >                            ((new_val << offset) & mask);
> > }
> > 
> > Which you can then use in a very similar way as the current proposal:
> > 
> > hantro_reg_write(&swreg, G1_PIC_WIDTH, width);
> > 
> > The first advantage here is that we no longer have any
> > footprint for the fields.
> > 
> > The ugly macros for "4, 0xff8, 23" can be auto-generated from
> > existing vendor headers, when possible, so that shouldn't
> > bother us.
> > 
> > The register set is "flushed" using _relaxed, but it
> > could be still costly.
> > 
> > If that is indeed costly, perhaps we can avoid writing
> > the entire set by having a dirty bit somewhere.
> > 
> > In any case, it's worth exploring our options first, I think.
> 
> Or maybe the regmap API itself deserves extending with a "deferred" 
> operating mode where updates to the cached state can be separated from 
> committing that state to the underlying hardware.
> 
> ...which, after a brief code search out of curiosity, apparently already 
> exists in the form of regcache_cache_only()/regcache_sync(), so there's 
> probably no need to reinvent it :)
> 

To be fair, and despite it could seem an anti-pattern, this particular
wheel is so tiny and trivial, that I'm starting to seriously consider
reinventing it.

I've been thinking long about this but just can't seem to see exactly
what benefit we're getting from using MMIO regmaps here,
as opposed to just a simple macro with an index, a mask, and an offset.

Ezequiel



_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 00/18] Add Hantro regmap and VC8000 h264 decode support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1075 bytes --]

On Thu, Oct 29, 2020 at 01:27:08PM -0300, Ezequiel Garcia wrote:
> On Thu, 2020-10-29 at 14:15 +0000, Robin Murphy wrote:

> > Or maybe the regmap API itself deserves extending with a "deferred" 
> > operating mode where updates to the cached state can be separated from 
> > committing that state to the underlying hardware.

> > ...which, after a brief code search out of curiosity, apparently already 
> > exists in the form of regcache_cache_only()/regcache_sync(), so there's 
> > probably no need to reinvent it :)

> To be fair, and despite it could seem an anti-pattern, this particular
> wheel is so tiny and trivial, that I'm starting to seriously consider
> reinventing it.

> I've been thinking long about this but just can't seem to see exactly
> what benefit we're getting from using MMIO regmaps here,
> as opposed to just a simple macro with an index, a mask, and an offset.

As a rule of thumb if you're not using a cache or fitting into some
other higher level framework stuff that uses regmap then I wouldn't
bother for MMIO devices.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip