On 11/11/2020 15:20, Hans Verkuil wrote: > On 30/10/2020 15:37, Neil Armstrong wrote: >> The GE2D is a 2D accelerator with various features like configurable blitter >> with alpha blending, frame rotation, scaling, format conversion and colorspace >> conversion. >> >> The driver implements a Memory2Memory VB2 V4L2 streaming device permitting: >> - 0, 90, 180, 270deg rotation >> - horizontal/vertical flipping >> - source cropping >> - destination compositing >> - 32bit/24bit/16bit format conversion >> >> This adds the support for the GE2D version found in the AXG SoCs Family. >> >> The missing features are: >> - Source scaling >> - Colorspace conversion >> - Advanced alpha blending & blitting options >> >> Is passes v4l2-compliance SHA: ea16a7ef13a902793a5c2626b0cefc4d956147f3, 64 bits, 64-bit time_t >> >> Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> >> --- >> drivers/media/platform/Kconfig | 13 + >> drivers/media/platform/Makefile | 2 + >> drivers/media/platform/meson/ge2d/Makefile | 3 + >> drivers/media/platform/meson/ge2d/ge2d-regs.h | 360 ++++++ >> drivers/media/platform/meson/ge2d/ge2d.c | 1105 +++++++++++++++++ >> 5 files changed, 1483 insertions(+) >> create mode 100644 drivers/media/platform/meson/ge2d/Makefile >> create mode 100644 drivers/media/platform/meson/ge2d/ge2d-regs.h >> create mode 100644 drivers/media/platform/meson/ge2d/ge2d.c >> >> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig >> index a3cb104956d5..0522a5a76b86 100644 >> --- a/drivers/media/platform/Kconfig >> +++ b/drivers/media/platform/Kconfig >> @@ -274,6 +274,19 @@ config VIDEO_MEM2MEM_DEINTERLACE >> help >> Generic deinterlacing V4L2 driver. >> >> +config VIDEO_MESON_GE2D >> + tristate "Amlogic 2D Graphic Acceleration Unit" >> + depends on VIDEO_DEV && VIDEO_V4L2 >> + depends on ARCH_MESON || COMPILE_TEST >> + select VIDEOBUF2_DMA_CONTIG >> + select V4L2_MEM2MEM_DEV >> + help >> + This is a v4l2 driver for Amlogic GE2D 2D graphics accelerator. >> + GE2D is a standalone 2D graphic acceleration unit, with color converter, >> + image scaling, BitBLT & alpha blending operations. >> + >> + To compile this driver as a module choose m here. >> + >> config VIDEO_SAMSUNG_S5P_G2D >> tristate "Samsung S5P and EXYNOS4 G2D 2d graphics accelerator driver" >> depends on VIDEO_DEV && VIDEO_V4L2 >> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile >> index 62b6cdc8c730..d47e0dcc65d7 100644 >> --- a/drivers/media/platform/Makefile >> +++ b/drivers/media/platform/Makefile >> @@ -80,3 +80,5 @@ obj-$(CONFIG_VIDEO_QCOM_CAMSS) += qcom/camss/ >> obj-$(CONFIG_VIDEO_QCOM_VENUS) += qcom/venus/ >> >> obj-y += sunxi/ >> + >> +obj-$(CONFIG_VIDEO_MESON_GE2D) += meson/ge2d/ >> diff --git a/drivers/media/platform/meson/ge2d/Makefile b/drivers/media/platform/meson/ge2d/Makefile >> new file mode 100644 >> index 000000000000..450586df27d7 >> --- /dev/null >> +++ b/drivers/media/platform/meson/ge2d/Makefile >> @@ -0,0 +1,3 @@ >> +# SPDX-License-Identifier: GPL-2.0-only >> + >> +obj-$(CONFIG_VIDEO_MESON_GE2D) += ge2d.o >> diff --git a/drivers/media/platform/meson/ge2d/ge2d-regs.h b/drivers/media/platform/meson/ge2d/ge2d-regs.h >> new file mode 100644 >> index 000000000000..2a76dd4c0ccb >> --- /dev/null >> +++ b/drivers/media/platform/meson/ge2d/ge2d-regs.h >> @@ -0,0 +1,360 @@ >> +/* SPDX-License-Identifier: GPL-2.0-or-later */ >> +/* >> + * Copyright (C) 2020 BayLibre, SAS >> + * Author: Neil Armstrong <narmstrong@xxxxxxxxxxxx> >> + * Copyright (C) 2017 Amlogic, Inc. All rights reserved. >> + */ >> + >> +#ifndef __GE2D_REGS__ >> +#define __GE2D_REGS__ >> + >> +/* Registers starts at (GE2D_REG(0x8a0 * 4) */ >> +#define GE2D_REG(x) ((0x8a0 + (x)) * 4) >> + >> +#define GE2D_GEN_CTRL0 GE2D_REG(0x00) >> + >> +#define GE2D_DST_BYTEMASK_ONLY BIT(31) >> +#define GE2D_DST_BITMASK_EN BIT(30) >> +#define GE2D_SRC2_KEY_EN BIT(29) >> +#define GE2D_SRC2_KEY_MODE BIT(28) >> +#define GE2D_SRC1_KEY_EN BIT(27) >> +#define GE2D_SRC1_KEY_MODE BIT(26) >> +#define GE2D_DST1_8B_MODE_SEL GENMASK(25, 24) >> +#define GE2D_DST_CLIP_MODE BIT(23) >> +#define GE2D_SRC2_8B_MODE_SEL GENMASK(16, 15) >> +#define GE2D_SRC2_FILL_MODE BIT(14) >> +#define GE2D_SRC2_PIC_STRUCT GENMASK(13, 12) >> +#define GE2D_SRC2_X_YC_RATIO BIT(11) >> +#define GE2D_SRC1_8B_MODE_SEL GENMASK(6, 5) >> +#define GE2D_SRC1_FILL_MODE BIT(4) >> +#define GE2D_SRC1_LUT_EN BIT(3) >> +#define GE2D_SRC1_PIC_STRUCT GENMASK(2, 1) >> + >> +#define GE2D_GEN_CTRL1 GE2D_REG(0x01) >> + >> +#define GE2D_SOFT_RST BIT(31) >> +#define GE2D_DST_WRITE_RESP_CNT_RST BIT(30) >> +#define GE2D_DST_WRITE_RESP_CNT_ADD_DIS BIT(29) >> +#define GE2D_COLOR_CONVERSION_MODE1 BIT(26) >> +#define GE2D_INTERRUPT_CTRL GENMASK(25, 24) >> +#define GE2D_SRC2_BURST_SIZE_CTRL GENMASK(23, 22) >> +#define GE2D_SRC1_BURST_SIZE_CTRL GENMASK(21, 16) >> +#define GE2D_DST1_PIC_STRUCT GENMASK(15, 14) >> +#define GE2D_SRC_RD_CTRL GENMASK(13, 12) >> +#define GE2D_DST2_URGENT_EN BIT(11) >> +#define GE2D_SRC1_URGENT_EN BIT(10) >> +#define GE2D_SRC2_URGENT_EN BIT(9) >> +#define GE2D_DST1_URGENT_EN BIT(8) >> +#define GE2D_SRC1_GB_ALPHA GENMASK(7, 0) >> + >> +#define GE2D_GEN_CTRL2 GE2D_REG(0x02) >> + >> +#define GE2D_ALPHA_CONVERSION_MODE0 BIT(31) >> +#define GE2D_COLOR_CONVERSION_MODE0 BIT(30) >> +#define GE2D_SRC1_GB_ALPHA_EN BIT(29) >> +#define GE2D_DST1_COLOR_ROUND_MODE BIT(28) >> +#define GE2D_SRC2_COLOR_EXPAND_MODE BIT(27) >> +#define GE2D_SRC2_ALPHA_EXPAND_MODE BIT(26) >> +#define GE2D_SRC1_COLOR_EXPAND_MODE BIT(25) >> +#define GE2D_SRC1_ALPHA_EXPAND_MODE BIT(24) >> +#define GE2D_DST_LITTLE_ENDIAN BIT(23) >> +#define GE2D_DST1_COLOR_MAP GENMASK(22, 19) >> +#define GE2D_ALU_MULT_MODE BIT(18) >> +#define GE2D_DST1_FORMAT GENMASK(17, 16) >> +#define GE2D_SRC2_LITTLE_ENDIAN BIT(15) >> +#define GE2D_SRC2_COLOR_MAP GENMASK(14, 11) >> +#define GE2D_ALPHA_CONVERSION_MODE1 BIT(10) >> +#define GE2D_SRC2_FORMAT GENMASK(9, 8) >> +#define GE2D_SRC1_LITTLE_ENDIAN BIT(7) >> +#define GE2D_SRC1_COLOR_MAP GENMASK(6, 3) >> +#define GE2D_SRC1_DEEPCOLOR BIT(2) >> +#define GE2D_SRC1_FORMAT GENMASK(1, 0) >> + >> +#define GE2D_FORMAT_8BIT 0 >> +#define GE2D_FORMAT_16BIT 1 >> +#define GE2D_FORMAT_24BIT 2 >> +#define GE2D_FORMAT_32BIT 3 >> + >> +/* 16 bit */ >> +#define GE2D_COLOR_MAP_YUV422 0 >> +#define GE2D_COLOR_MAP_RGB655 1 >> +#define GE2D_COLOR_MAP_YUV655 1 >> +#define GE2D_COLOR_MAP_RGB844 2 >> +#define GE2D_COLOR_MAP_YUV844 2 >> +#define GE2D_COLOR_MAP_RGBA6442 3 >> +#define GE2D_COLOR_MAP_YUVA6442 3 >> +#define GE2D_COLOR_MAP_RGBA4444 4 >> +#define GE2D_COLOR_MAP_YUVA4444 4 >> +#define GE2D_COLOR_MAP_RGB565 5 >> +#define GE2D_COLOR_MAP_YUV565 5 >> +#define GE2D_COLOR_MAP_ARGB4444 6 >> +#define GE2D_COLOR_MAP_AYUV4444 6 >> +#define GE2D_COLOR_MAP_ARGB1555 7 >> +#define GE2D_COLOR_MAP_AYUV1555 7 >> +#define GE2D_COLOR_MAP_RGBA4642 8 >> +#define GE2D_COLOR_MAP_YUVA4642 8 >> + >> +/* 24 bit */ >> +#define GE2D_COLOR_MAP_RGB888 0 >> +#define GE2D_COLOR_MAP_YUV444 0 >> +#define GE2D_COLOR_MAP_RGBA5658 1 >> +#define GE2D_COLOR_MAP_YUVA5658 1 >> +#define GE2D_COLOR_MAP_ARGB8565 2 >> +#define GE2D_COLOR_MAP_AYUV8565 2 >> +#define GE2D_COLOR_MAP_RGBA6666 3 >> +#define GE2D_COLOR_MAP_YUVA6666 3 >> +#define GE2D_COLOR_MAP_ARGB6666 4 >> +#define GE2D_COLOR_MAP_AYUV6666 4 >> +#define GE2D_COLOR_MAP_BGR888 5 >> +#define GE2D_COLOR_MAP_VUY888 5 >> + >> +/* 32 bit */ >> +#define GE2D_COLOR_MAP_RGBA8888 0 >> +#define GE2D_COLOR_MAP_YUVA8888 0 >> +#define GE2D_COLOR_MAP_ARGB8888 1 >> +#define GE2D_COLOR_MAP_AYUV8888 1 >> +#define GE2D_COLOR_MAP_ABGR8888 2 >> +#define GE2D_COLOR_MAP_AVUY8888 2 >> +#define GE2D_COLOR_MAP_BGRA8888 3 >> +#define GE2D_COLOR_MAP_VUYA8888 3 >> + >> +#define GE2D_CMD_CTRL GE2D_REG(0x03) >> + >> +#define GE2D_SRC2_FILL_COLOR_EN BIT(9) >> +#define GE2D_SRC1_FILL_COLOR_EN BIT(8) >> +#define GE2D_DST_XY_SWAP BIT(7) >> +#define GE2D_DST_X_REV BIT(6) >> +#define GE2D_DST_Y_REV BIT(5) >> +#define GE2D_SRC2_X_REV BIT(4) >> +#define GE2D_SRC2_Y_REV BIT(3) >> +#define GE2D_SRC1_X_REV BIT(2) >> +#define GE2D_SRC1_Y_REV BIT(1) >> +#define GE2D_CBUS_CMD_WR BIT(0) >> + >> +#define GE2D_STATUS0 GE2D_REG(0x04) >> + >> +#define GE2D_DST_WRITE_RSP_CNT GENMASK(28, 17) >> +#define GE2D_DP_STATUS GENMASK(16, 7) >> +#define GE2D_R1CMD_RDY BIT(6) >> +#define GE2D_R2CMD_RDY BIT(5) >> +#define GE2D_PDPCMD_VALID BIT(4) >> +#define GE2D_DPCMD_RDY BIT(3) >> +#define GE2D_BUF_CMD_VALID BIT(2) >> +#define GE2D_CURR_CMD_VALID BIT(1) >> +#define GE2D_GE2D_BUSY BIT(0) >> + >> +#define GE2D_STATUS1 GE2D_REG(0x05) >> + >> +#define GE2D_WR_DST1_STATUS GENMASK(29, 16) >> +#define GE2D_RD_SRC2_FIFO_EMPTY BIT(15) >> +#define GE2D_RD_SRC2_FIFO_OVERFLOW BIT(14) >> +#define GE2D_RD_SRC2_STATE_Y GENMASK(13, 12) >> +#define GE2D_RD_SRC2_WIN_ERR BIT(11) >> +#define GE2D_RD_SRC2_CMD_BUSY BIT(10) >> +#define GE2D_RD_SRC1_FIFO_EMPTY BIT(9) >> +#define GE2D_RD_SRC1_FIFO_OVERFLOW BIT(8) >> +#define GE2D_RD_SRC1_STATE_CR GENMASK(7, 6) >> +#define GE2D_RD_SRC1_STATE_CB GENMASK(5, 4) >> +#define GE2D_RD_SRC1_STATE_Y GENMASK(3, 2) >> +#define GE2D_RD_SRC1_WIN_ERR BIT(1) >> +#define GE2D_RD_SRC1_CMD_BUSY BIT(0) >> + >> +#define GE2D_SRC1_DEF_COLOR GE2D_REG(0x06) >> + >> +#define GE2D_COLOR_R_Y GENMASK(31, 24) >> +#define GE2D_COLOR_B_CB GENMASK(23, 16) >> +#define GE2D_COLOR_B_CR GENMASK(15, 8) >> +#define GE2D_COLOR_ALPHA GENMASK(7, 0) >> + >> +#define GE2D_SRC1_CLIPX_START_END GE2D_REG(0x07) >> + >> +#define GE2D_START_EXTRA BIT(31) /* For GE2D_SRC1_CLIPX/Y_START_END */ >> +#define GE2D_START_EXTRA0 BIT(30) /* For GE2D_SRC1_X/Y_START_END */ >> +#define GE2D_START GENMASK(28, 16) >> +#define GE2D_END_EXTRA BIT(15) /* For GE2D_SRC1_CLIPX/Y_START_END */ >> +#define GE2D_END_EXTRA0 BIT(14) /* For GE2D_SRC1_X/Y_START_END */ >> +#define GE2D_END GENMASK(12, 0) >> + >> +#define GE2D_SRC1_CLIPY_START_END GE2D_REG(0x08) >> +#define GE2D_SRC1_CANVAS GE2D_REG(0x09) >> + >> +#define GE2D_SRC1_CANVAS_ADDR GENMASK(31, 24) >> + >> +#define GE2D_SRC1_X_START_END GE2D_REG(0x0a) >> +#define GE2D_SRC1_Y_START_END GE2D_REG(0x0b) >> +#define GE2D_SRC1_LUT_ADDR GE2D_REG(0x0c) >> + >> +#define GE2D_LUT_READ BIT(8) >> +#define GE2D_LUT_ADDR GENMASK(7, 0) >> + >> +#define GE2D_SRC1_LUT_DAT GE2D_REG(0x0d) >> +#define GE2D_SRC1_FMT_CTRL GE2D_REG(0x0e) >> +#define GE2D_SRC2_DEF_COLOR GE2D_REG(0x0f) >> +#define GE2D_SRC2_CLIPX_START_END GE2D_REG(0x10) >> +#define GE2D_SRC2_CLIPY_START_END GE2D_REG(0x11) >> +#define GE2D_SRC2_X_START_END GE2D_REG(0x12) >> +#define GE2D_SRC2_Y_START_END GE2D_REG(0x13) >> +#define GE2D_DST_CLIPX_START_END GE2D_REG(0x14) >> +#define GE2D_DST_CLIPY_START_END GE2D_REG(0x15) >> +#define GE2D_DST_X_START_END GE2D_REG(0x16) >> +#define GE2D_DST_Y_START_END GE2D_REG(0x17) >> +#define GE2D_SRC2_DST_CANVAS GE2D_REG(0x18) >> + >> +#define GE2D_DST2_CANVAS_ADDR GENMASK(23, 16) >> +#define GE2D_SRC2_CANVAS_ADDR GENMASK(15, 8) >> +#define GE2D_DST1_CANVAS_ADDR GENMASK(7, 0) >> + >> +#define GE2D_VSC_START_PHASE_STEP GE2D_REG(0x19) >> +#define GE2D_VSC_PHASE_SLOPE GE2D_REG(0x1a) >> +#define GE2D_VSC_INI_CTRL GE2D_REG(0x1b) >> +#define GE2D_HSC_START_PHASE_STEP GE2D_REG(0x1c) >> +#define GE2D_HSC_PHASE_SLOPE GE2D_REG(0x1d) >> +#define GE2D_HSC_INI_CTRL GE2D_REG(0x1e) >> +#define GE2D_HSC_ADV_CTRL GE2D_REG(0x1f) >> +#define GE2D_SC_MISC_CTRL GE2D_REG(0x20) >> +#define GE2D_VSC_NRND_POINT GE2D_REG(0x21) >> +#define GE2D_VSC_NRND_PHASE GE2D_REG(0x22) >> +#define GE2D_HSC_NRND_POINT GE2D_REG(0x23) >> +#define GE2D_HSC_NRND_PHASE GE2D_REG(0x24) >> +#define GE2D_MATRIX_PRE_OFFSET GE2D_REG(0x25) >> +#define GE2D_MATRIX_COEF00_01 GE2D_REG(0x26) >> +#define GE2D_MATRIX_COEF02_10 GE2D_REG(0x27) >> +#define GE2D_MATRIX_COEF11_12 GE2D_REG(0x28) >> +#define GE2D_MATRIX_COEF20_21 GE2D_REG(0x29) >> +#define GE2D_MATRIX_COEF22_CTRL GE2D_REG(0x2a) >> +#define GE2D_MATRIX_OFFSET GE2D_REG(0x2b) >> +#define GE2D_ALU_OP_CTRL GE2D_REG(0x2c) >> + >> +#define GE2D_SRC1_COLOR_MULT_ALPHA_SEL GENMASK(26, 25) >> +#define GE2D_SRC2_COLOR_MULT_ALPHA_SEL BIT(24) >> +#define GE2D_ALU_BLEND_MODE GENMASK(22, 20) >> + >> +#define OPERATION_ADD 0 /* Cd = Cs*Fs+Cd*Fd */ >> +#define OPERATION_SUB 1 /* Cd = Cs*Fs-Cd*Fd */ >> +#define OPERATION_REVERSE_SUB 2 /* Cd = Cd*Fd-Cs*Fs */ >> +#define OPERATION_MIN 3 /* Cd = Min(Cd*Fd,Cs*Fs) */ >> +#define OPERATION_MAX 4 /* Cd = Max(Cd*Fd,Cs*Fs) */ >> +#define OPERATION_LOGIC 5 >> + >> +#define GE2D_ALU_SRC_COLOR_BLEND_FACTOR GENMASK(19, 16) >> +#define GE2D_ALU_DST_COLOR_BLEND_FACTOR GENMASK(15, 12) >> + >> +#define COLOR_FACTOR_ZERO 0 >> +#define COLOR_FACTOR_ONE 1 >> +#define COLOR_FACTOR_SRC_COLOR 2 >> +#define COLOR_FACTOR_ONE_MINUS_SRC_COLOR 3 >> +#define COLOR_FACTOR_DST_COLOR 4 >> +#define COLOR_FACTOR_ONE_MINUS_DST_COLOR 5 >> +#define COLOR_FACTOR_SRC_ALPHA 6 >> +#define COLOR_FACTOR_ONE_MINUS_SRC_ALPHA 7 >> +#define COLOR_FACTOR_DST_ALPHA 8 >> +#define COLOR_FACTOR_ONE_MINUS_DST_ALPHA 9 >> +#define COLOR_FACTOR_CONST_COLOR 10 >> +#define COLOR_FACTOR_ONE_MINUS_CONST_COLOR 11 >> +#define COLOR_FACTOR_CONST_ALPHA 12 >> +#define COLOR_FACTOR_ONE_MINUS_CONST_ALPHA 13 >> +#define COLOR_FACTOR_SRC_ALPHA_SATURATE 14 >> + >> +#define GE2D_ALU_OPERATION_LOGIC GENMASK(15, 12) >> + >> +#define LOGIC_OPERATION_CLEAR 0 >> +#define LOGIC_OPERATION_COPY 1 >> +#define LOGIC_OPERATION_NOOP 2 >> +#define LOGIC_OPERATION_SET 3 >> +#define LOGIC_OPERATION_COPY_INVERT 4 >> +#define LOGIC_OPERATION_INVERT 5 >> +#define LOGIC_OPERATION_AND_REVERSE 6 >> +#define LOGIC_OPERATION_OR_REVERSE 7 >> +#define LOGIC_OPERATION_AND 8 >> +#define LOGIC_OPERATION_OR 9 >> +#define LOGIC_OPERATION_NAND 10 >> +#define LOGIC_OPERATION_NOR 11 >> +#define LOGIC_OPERATION_XOR 12 >> +#define LOGIC_OPERATION_EQUIV 13 >> +#define LOGIC_OPERATION_AND_INVERT 14 >> +#define LOGIC_OPERATION_OR_INVERT 15 >> + >> +#define GE2D_ALU_ALPHA_BLEND_MODE GENMASK(10, 8) >> +#define GE2D_ALU_SRC_ALPHA_BLEND_FACTOR GENMASK(7, 4) >> +#define GE2D_ALU_DST_ALPHA_BLEND_FACTOR GENMASK(3, 0) >> + >> +#define ALPHA_FACTOR_ZERO 0 >> +#define ALPHA_FACTOR_ONE 1 >> +#define ALPHA_FACTOR_SRC_ALPHA 2 >> +#define ALPHA_FACTOR_ONE_MINUS_SRC_ALPHA 3 >> +#define ALPHA_FACTOR_DST_ALPHA 4 >> +#define ALPHA_FACTOR_ONE_MINUS_DST_ALPHA 5 >> +#define ALPHA_FACTOR_CONST_ALPHA 6 >> +#define ALPHA_FACTOR_ONE_MINUS_CONST_ALPHA 7 >> + >> +#define GE2D_ALU_ALPHA_OPERATION_LOGIC GENMASK(3, 0) >> + >> +#define GE2D_ALU_COLOR_OP(__op, __src_factor, __dst_factor) \ >> + (FIELD_PREP(GE2D_ALU_BLEND_MODE, __op) | \ >> + FIELD_PREP(GE2D_ALU_SRC_COLOR_BLEND_FACTOR, __src_factor) | \ >> + FIELD_PREP(GE2D_ALU_DST_COLOR_BLEND_FACTOR, __dst_factor)) >> + >> +#define GE2D_ALU_DO_COLOR_OPERATION_LOGIC(__op, __src_factor) \ >> + GE2D_ALU_COLOR_OP(OPERATION_LOGIC, __src_factor, __op) >> + >> +#define GE2D_ALU_ALPHA_OP(__op, __src_factor, __dst_factor) \ >> + (FIELD_PREP(GE2D_ALU_ALPHA_BLEND_MODE, __op) | \ >> + FIELD_PREP(GE2D_ALU_SRC_ALPHA_BLEND_FACTOR, __src_factor) | \ >> + FIELD_PREP(GE2D_ALU_DST_ALPHA_BLEND_FACTOR, __dst_factor)) >> + >> +#define GE2D_ALU_DO_ALPHA_OPERATION_LOGIC(__op, __src_factor) \ >> + GE2D_ALU_ALPHA_OP(OPERATION_LOGIC, __src_factor, __op) >> + >> +#define GE2D_ALU_CONST_COLOR GE2D_REG(0x2d) >> +#define GE2D_SRC1_KEY GE2D_REG(0x2e) >> +#define GE2D_SRC1_KEY_MASK GE2D_REG(0x2f) >> +#define GE2D_SRC2_KEY GE2D_REG(0x30) >> +#define GE2D_SRC2_KEY_MASK GE2D_REG(0x31) >> +#define GE2D_DST_BITMASK GE2D_REG(0x32) >> +#define GE2D_DP_ONOFF_CTRL GE2D_REG(0x33) >> +#define GE2D_SCALE_COEF_IDX GE2D_REG(0x34) >> +#define GE2D_SCALE_COEF GE2D_REG(0x35) >> +#define GE2D_SRC_OUTSIDE_ALPHA GE2D_REG(0x36) >> +#define GE2D_ANTIFLICK_CTRL0 GE2D_REG(0x38) >> +#define GE2D_ANTIFLICK_CTRL1 GE2D_REG(0x39) >> +#define GE2D_ANTIFLICK_COLOR_FILT0 GE2D_REG(0x3a) >> +#define GE2D_ANTIFLICK_COLOR_FILT1 GE2D_REG(0x3b) >> +#define GE2D_ANTIFLICK_COLOR_FILT2 GE2D_REG(0x3c) >> +#define GE2D_ANTIFLICK_COLOR_FILT3 GE2D_REG(0x3d) >> +#define GE2D_ANTIFLICK_ALPHA_FILT0 GE2D_REG(0x3e) >> +#define GE2D_ANTIFLICK_ALPHA_FILT1 GE2D_REG(0x3f) >> +#define GE2D_ANTIFLICK_ALPHA_FILT2 GE2D_REG(0x40) >> +#define GE2D_ANTIFLICK_ALPHA_FILT3 GE2D_REG(0x41) >> +#define GE2D_SRC1_RANGE_MAP_Y_CTRL GE2D_REG(0x43) >> +#define GE2D_SRC1_RANGE_MAP_CB_CTRL GE2D_REG(0x44) >> +#define GE2D_SRC1_RANGE_MAP_CR_CTRL GE2D_REG(0x45) >> +#define GE2D_ARB_BURST_NUM GE2D_REG(0x46) >> +#define GE2D_TID_TOKEN GE2D_REG(0x47) >> +#define GE2D_GEN_CTRL3 GE2D_REG(0x48) >> + >> +#define GE2D_DST2_BYTEMASK_VAL GENMASK(31, 28) >> +#define GE2D_DST2_PIC_STRUCT GENMASK(27, 26) >> +#define GE2D_DST2_8B_MODE_SEL GENMASK(25, 24) >> +#define GE2D_DST2_COLOR_MAP GENMASK(22, 19) >> +#define GE2D_DST2_FORMAT GENMASK(17, 16) >> +#define GE2D_DST2_COLOR_ROUND_MODE BIT(14) >> +#define GE2D_DST2_X_DISCARD_MODE GENMASK(13, 12) >> +#define GE2D_DST2_Y_DISCARD_MODE GENMASK(11, 10) >> +#define GE2D_DST2_ENABLE BIT(8) >> +#define GE2D_DST1_X_DISCARD_MODE GENMASK(5, 4) >> +#define GE2D_DST1_Y_DISCARD_MODE GENMASK(3, 2) >> +#define GE2D_DST1_ENABLE BIT(0) >> + >> +#define GE2D_STATUS2 GE2D_REG(0x49) >> +#define GE2D_GEN_CTRL4 GE2D_REG(0x4a) >> +#define GE2D_DST1_BADDR_CTRL GE2D_REG(0x51) >> +#define GE2D_DST1_STRIDE_CTRL GE2D_REG(0x52) >> + >> +#define GE2D_STRIDE_SIZE GENMASK(19, 0) >> + >> +#define GE2D_SRC1_BADDR_CTRL GE2D_REG(0x53) >> +#define GE2D_SRC1_STRIDE_CTRL GE2D_REG(0x54) >> +#define GE2D_SRC2_BADDR_CTRL GE2D_REG(0x55) >> +#define GE2D_SRC2_STRIDE_CTRL GE2D_REG(0x56) >> + >> +#endif /* __GE2D_REGS__ */ >> diff --git a/drivers/media/platform/meson/ge2d/ge2d.c b/drivers/media/platform/meson/ge2d/ge2d.c >> new file mode 100644 >> index 000000000000..439bdc5151fa >> --- /dev/null >> +++ b/drivers/media/platform/meson/ge2d/ge2d.c >> @@ -0,0 +1,1105 @@ >> +// SPDX-License-Identifier: GPL-2.0-or-later >> +/* >> + * Copyright (C) 2020 BayLibre, SAS >> + * Author: Neil Armstrong <narmstrong@xxxxxxxxxxxx> >> + */ >> + >> +#include <linux/clk.h> >> +#include <linux/delay.h> >> +#include <linux/interrupt.h> >> +#include <linux/module.h> >> +#include <linux/of.h> >> +#include <linux/reset.h> >> +#include <linux/sched.h> >> +#include <linux/slab.h> >> +#include <linux/timer.h> >> +#include <linux/regmap.h> >> + >> +#include <linux/platform_device.h> >> +#include <media/v4l2-device.h> >> +#include <media/v4l2-event.h> >> +#include <media/v4l2-ioctl.h> >> +#include <media/v4l2-mem2mem.h> >> +#include <media/v4l2-ctrls.h> >> +#include <media/videobuf2-v4l2.h> >> +#include <media/videobuf2-dma-contig.h> >> + >> +#include "ge2d-regs.h" >> + >> +#define GE2D_NAME "meson-ge2d" >> + >> +#define DEFAULT_WIDTH 100 >> +#define DEFAULT_HEIGHT 100 >> + >> +#define MAX_WIDTH 8191 >> +#define MAX_HEIGHT 8191 >> + >> +/* >> + * Missing features: >> + * - Scaling >> + * - Simple 1/2 vertical scaling >> + * - YUV input support >> + * - Source global alpha >> + * - Colorspace conversion >> + */ >> + >> +struct ge2d_fmt { >> + u32 fourcc; >> + bool alpha; >> + bool le; >> + unsigned int depth; >> + unsigned int hw_fmt; >> + unsigned int hw_map; >> +}; >> + >> +struct ge2d_frame { >> + struct vb2_v4l2_buffer *buf; >> + >> + /* Image Format */ >> + struct v4l2_pix_format pix_fmt; >> + >> + /* Crop */ >> + struct v4l2_rect crop; >> + >> + /* Image format */ >> + const struct ge2d_fmt *fmt; >> +}; >> + >> +struct ge2d_ctx { >> + struct v4l2_fh fh; >> + struct meson_ge2d *ge2d; >> + struct ge2d_frame in; >> + struct ge2d_frame out; >> + struct v4l2_ctrl_handler ctrl_handler; >> + >> + bool streamon_out, streamon_cap; >> + unsigned long sequence_out, sequence_cap; >> + >> + /* Control values */ >> + u32 hflip; >> + u32 vflip; >> + u32 xy_swap; >> +}; >> + >> +struct meson_ge2d { >> + struct v4l2_device v4l2_dev; >> + struct v4l2_m2m_dev *m2m_dev; >> + struct video_device *vfd; >> + >> + struct device *dev; >> + struct regmap *map; >> + struct clk *clk; >> + >> + /* vb2 queue lock */ >> + struct mutex mutex; >> + /* device access spinlock */ >> + spinlock_t ctrl_lock; >> + >> + struct ge2d_ctx *curr; >> +}; >> + >> +static void vidioc_setup_cap_fmt(struct ge2d_ctx *ctx, struct v4l2_pix_format *f); >> + >> +#define FMT(_fourcc, _alpha, _depth, _map) \ >> +{ \ >> + .fourcc = _fourcc, \ >> + .alpha = (_alpha), \ >> + .depth = (_depth), \ >> + .hw_fmt = GE2D_FORMAT_ ## _depth ## BIT, \ >> + .hw_map = GE2D_COLOR_MAP_ ## _map, \ >> +} >> + >> +/* TOFIX Handle the YUV input formats */ >> +static const struct ge2d_fmt formats[] = { >> + /* FOURCC Alpha HW FMT HW MAP */ >> + FMT(V4L2_PIX_FMT_XRGB32, false, 32, BGRA8888), >> + FMT(V4L2_PIX_FMT_RGB32, true, 32, BGRA8888), >> + FMT(V4L2_PIX_FMT_ARGB32, true, 32, BGRA8888), >> + FMT(V4L2_PIX_FMT_RGBX32, false, 32, ABGR8888), >> + FMT(V4L2_PIX_FMT_RGBA32, true, 32, ABGR8888), >> + FMT(V4L2_PIX_FMT_BGRX32, false, 32, RGBA8888), >> + FMT(V4L2_PIX_FMT_BGRA32, true, 32, RGBA8888), >> + FMT(V4L2_PIX_FMT_BGR32, true, 32, ARGB8888), >> + FMT(V4L2_PIX_FMT_ABGR32, true, 32, ARGB8888), >> + FMT(V4L2_PIX_FMT_XBGR32, false, 32, ARGB8888), >> + >> + FMT(V4L2_PIX_FMT_RGB24, false, 24, BGR888), >> + FMT(V4L2_PIX_FMT_BGR24, false, 24, RGB888), >> + >> + FMT(V4L2_PIX_FMT_XRGB555X, false, 16, ARGB1555), >> + FMT(V4L2_PIX_FMT_ARGB555X, true, 16, ARGB1555), >> + FMT(V4L2_PIX_FMT_RGB565, false, 16, RGB565), >> + FMT(V4L2_PIX_FMT_RGBX444, false, 16, RGBA4444), >> + FMT(V4L2_PIX_FMT_RGBA444, true, 16, RGBA4444), >> + FMT(V4L2_PIX_FMT_XRGB444, false, 16, ARGB4444), >> + FMT(V4L2_PIX_FMT_ARGB444, true, 16, ARGB4444), >> +}; >> + >> +#define NUM_FORMATS ARRAY_SIZE(formats) >> + >> +static const struct ge2d_fmt *find_fmt(struct v4l2_format *f) >> +{ >> + unsigned int i; >> + >> + for (i = 0; i < NUM_FORMATS; i++) { >> + if (formats[i].fourcc == f->fmt.pix.pixelformat) >> + return &formats[i]; >> + } >> + >> + /* >> + * TRY_FMT/S_FMT should never return an error when the requested format >> + * is not supported. Drivers should always return a valid format, >> + * preferably a format that is as widely supported by applications as >> + * possible. >> + */ >> + return &formats[0]; >> +} >> + >> +static struct ge2d_frame *get_frame(struct ge2d_ctx *ctx, >> + enum v4l2_buf_type type) >> +{ >> + switch (type) { >> + case V4L2_BUF_TYPE_VIDEO_OUTPUT: >> + return &ctx->in; >> + case V4L2_BUF_TYPE_VIDEO_CAPTURE: >> + return &ctx->out; >> + default: >> + return ERR_PTR(-EINVAL); > > I wouldn't mess with returning an error pointer here. For the default > case just add a WARN_ON and return &ctx->in or ->out. > > It's really a driver bug if you end up there. ok > >> + } >> +} >> + >> +static void ge2d_hw_start(struct meson_ge2d *ge2d) >> +{ >> + struct ge2d_ctx *ctx = ge2d->curr; >> + u32 reg; >> + >> + /* Reset */ >> + regmap_update_bits(ge2d->map, GE2D_GEN_CTRL1, >> + GE2D_SOFT_RST, GE2D_SOFT_RST); >> + regmap_update_bits(ge2d->map, GE2D_GEN_CTRL1, >> + GE2D_SOFT_RST, 0); >> + >> + usleep_range(100, 200); >> + >> + /* Implement CANVAS for non-AXG */ >> + regmap_write(ge2d->map, GE2D_SRC1_BADDR_CTRL, >> + (vb2_dma_contig_plane_dma_addr(&ctx->in.buf->vb2_buf, 0) + 7) >> 3); >> + regmap_write(ge2d->map, GE2D_SRC1_STRIDE_CTRL, >> + (ctx->in.pix_fmt.bytesperline + 7) >> 3); >> + regmap_write(ge2d->map, GE2D_SRC2_BADDR_CTRL, >> + (vb2_dma_contig_plane_dma_addr(&ctx->out.buf->vb2_buf, 0) + 7) >> 3); >> + regmap_write(ge2d->map, GE2D_SRC2_STRIDE_CTRL, >> + (ctx->out.pix_fmt.bytesperline + 7) >> 3); >> + regmap_write(ge2d->map, GE2D_DST1_BADDR_CTRL, >> + (vb2_dma_contig_plane_dma_addr(&ctx->out.buf->vb2_buf, 0) + 7) >> 3); >> + regmap_write(ge2d->map, GE2D_DST1_STRIDE_CTRL, >> + (ctx->out.pix_fmt.bytesperline + 7) >> 3); >> + >> + regmap_write(ge2d->map, GE2D_GEN_CTRL0, 0); >> + regmap_write(ge2d->map, GE2D_GEN_CTRL1, >> + FIELD_PREP(GE2D_INTERRUPT_CTRL, 2) | >> + FIELD_PREP(GE2D_SRC2_BURST_SIZE_CTRL, 3) | >> + FIELD_PREP(GE2D_SRC1_BURST_SIZE_CTRL, 0x3f)); >> + >> + regmap_write(ge2d->map, GE2D_GEN_CTRL2, >> + GE2D_SRC1_LITTLE_ENDIAN | >> + GE2D_SRC2_LITTLE_ENDIAN | >> + GE2D_DST_LITTLE_ENDIAN | >> + FIELD_PREP(GE2D_DST1_COLOR_MAP, ctx->out.fmt->hw_map) | >> + FIELD_PREP(GE2D_DST1_FORMAT, ctx->out.fmt->hw_fmt) | >> + FIELD_PREP(GE2D_SRC2_COLOR_MAP, ctx->out.fmt->hw_map) | >> + FIELD_PREP(GE2D_SRC2_FORMAT, ctx->out.fmt->hw_fmt) | >> + FIELD_PREP(GE2D_SRC1_COLOR_MAP, ctx->in.fmt->hw_map) | >> + FIELD_PREP(GE2D_SRC1_FORMAT, ctx->in.fmt->hw_fmt)); >> + regmap_write(ge2d->map, GE2D_GEN_CTRL3, >> + GE2D_DST1_ENABLE); >> + >> + regmap_write(ge2d->map, GE2D_SRC1_CLIPY_START_END, >> + FIELD_PREP(GE2D_START, ctx->in.crop.top) | >> + FIELD_PREP(GE2D_END, ctx->in.crop.top + ctx->in.crop.height)); >> + regmap_write(ge2d->map, GE2D_SRC1_CLIPX_START_END, >> + FIELD_PREP(GE2D_START, ctx->in.crop.left) | >> + FIELD_PREP(GE2D_END, ctx->in.crop.left + ctx->in.crop.width)); >> + regmap_write(ge2d->map, GE2D_SRC2_CLIPY_START_END, >> + FIELD_PREP(GE2D_START, ctx->out.crop.top) | >> + FIELD_PREP(GE2D_END, ctx->out.crop.top + ctx->out.crop.height)); >> + regmap_write(ge2d->map, GE2D_SRC2_CLIPX_START_END, >> + FIELD_PREP(GE2D_START, ctx->out.crop.left) | >> + FIELD_PREP(GE2D_END, ctx->out.crop.left + ctx->out.crop.width)); >> + regmap_write(ge2d->map, GE2D_DST_CLIPY_START_END, >> + FIELD_PREP(GE2D_START, ctx->out.crop.top) | >> + FIELD_PREP(GE2D_END, ctx->out.crop.top + ctx->out.crop.height)); >> + regmap_write(ge2d->map, GE2D_DST_CLIPX_START_END, >> + FIELD_PREP(GE2D_START, ctx->out.crop.left) | >> + FIELD_PREP(GE2D_END, ctx->out.crop.left + ctx->out.crop.width)); >> + >> + regmap_write(ge2d->map, GE2D_SRC1_Y_START_END, >> + FIELD_PREP(GE2D_END, ctx->in.pix_fmt.height)); >> + regmap_write(ge2d->map, GE2D_SRC1_X_START_END, >> + FIELD_PREP(GE2D_END, ctx->in.pix_fmt.width)); >> + regmap_write(ge2d->map, GE2D_SRC2_Y_START_END, >> + FIELD_PREP(GE2D_END, ctx->out.pix_fmt.height)); >> + regmap_write(ge2d->map, GE2D_SRC2_X_START_END, >> + FIELD_PREP(GE2D_END, ctx->out.pix_fmt.width)); >> + regmap_write(ge2d->map, GE2D_DST_Y_START_END, >> + FIELD_PREP(GE2D_END, ctx->out.pix_fmt.height)); >> + regmap_write(ge2d->map, GE2D_DST_X_START_END, >> + FIELD_PREP(GE2D_END, ctx->out.pix_fmt.width)); >> + >> + /* Color, no blend, use source color */ >> + reg = GE2D_ALU_DO_COLOR_OPERATION_LOGIC(LOGIC_OPERATION_COPY, >> + COLOR_FACTOR_SRC_COLOR); >> + >> + if (ctx->in.fmt->alpha && ctx->out.fmt->alpha) >> + /* Take source alpha */ >> + reg |= GE2D_ALU_DO_ALPHA_OPERATION_LOGIC(LOGIC_OPERATION_COPY, >> + COLOR_FACTOR_SRC_ALPHA); >> + else if (!ctx->out.fmt->alpha) >> + /* Set alpha to 0 */ >> + reg |= GE2D_ALU_DO_ALPHA_OPERATION_LOGIC(LOGIC_OPERATION_SET, >> + COLOR_FACTOR_ZERO); >> + else >> + /* Keep original alpha */ >> + reg |= GE2D_ALU_DO_ALPHA_OPERATION_LOGIC(LOGIC_OPERATION_COPY, >> + COLOR_FACTOR_DST_ALPHA); >> + >> + regmap_write(ge2d->map, GE2D_ALU_OP_CTRL, reg); >> + >> + /* Start */ >> + regmap_write(ge2d->map, GE2D_CMD_CTRL, >> + (ctx->xy_swap ? GE2D_DST_XY_SWAP : 0) | >> + (ctx->hflip ? GE2D_SRC1_Y_REV : 0) | >> + (ctx->vflip ? GE2D_SRC1_X_REV : 0) | >> + GE2D_CBUS_CMD_WR); >> +} >> + >> +static void device_run(void *priv) >> +{ >> + struct ge2d_ctx *ctx = priv; >> + struct meson_ge2d *ge2d = ctx->ge2d; >> + unsigned long flags; >> + >> + spin_lock_irqsave(&ge2d->ctrl_lock, flags); >> + >> + ge2d->curr = ctx; >> + >> + ctx->in.buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx); >> + ctx->out.buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx); >> + >> + ge2d_hw_start(ge2d); >> + >> + spin_unlock_irqrestore(&ge2d->ctrl_lock, flags); >> +} >> + >> +static irqreturn_t ge2d_isr(int irq, void *priv) >> +{ >> + struct meson_ge2d *ge2d = priv; >> + u32 intr; >> + >> + regmap_read(ge2d->map, GE2D_STATUS0, &intr); >> + >> + if (!(intr & GE2D_GE2D_BUSY)) { >> + struct vb2_v4l2_buffer *src, *dst; >> + struct ge2d_ctx *ctx = ge2d->curr; >> + >> + ge2d->curr = NULL; >> + >> + src = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx); >> + dst = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx); >> + >> + src->sequence = ctx->sequence_out++; >> + dst->sequence = ctx->sequence_cap++; >> + >> + dst->timecode = src->timecode; >> + dst->vb2_buf.timestamp = src->vb2_buf.timestamp; >> + dst->flags = src->flags; >> + >> + v4l2_m2m_buf_done(src, VB2_BUF_STATE_DONE); >> + v4l2_m2m_buf_done(dst, VB2_BUF_STATE_DONE); >> + v4l2_m2m_job_finish(ge2d->m2m_dev, ctx->fh.m2m_ctx); >> + } >> + >> + return IRQ_HANDLED; >> +} >> + >> +static const struct v4l2_m2m_ops ge2d_m2m_ops = { >> + .device_run = device_run, >> +}; >> + >> +static int ge2d_queue_setup(struct vb2_queue *vq, >> + unsigned int *nbuffers, unsigned int *nplanes, >> + unsigned int sizes[], struct device *alloc_devs[]) >> +{ >> + struct ge2d_ctx *ctx = vb2_get_drv_priv(vq); >> + struct ge2d_frame *f = get_frame(ctx, vq->type); >> + >> + if (IS_ERR(f)) >> + return PTR_ERR(f); [for me: remove] >> + >> + if (*nplanes) >> + return sizes[0] < f->pix_fmt.sizeimage ? -EINVAL : 0; >> + >> + sizes[0] = f->pix_fmt.sizeimage; >> + *nplanes = 1; >> + >> + return 0; >> +} >> + >> +static int ge2d_buf_prepare(struct vb2_buffer *vb) >> +{ >> + struct ge2d_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); >> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); >> + struct ge2d_frame *f = get_frame(ctx, vb->vb2_queue->type); >> + >> + if (IS_ERR(f)) >> + return PTR_ERR(f); [for me: remove] >> + >> + vbuf->field = V4L2_FIELD_NONE; >> + >> + vb2_set_plane_payload(vb, 0, f->pix_fmt.sizeimage); >> + >> + return 0; >> +} >> + >> +static void ge2d_buf_queue(struct vb2_buffer *vb) >> +{ >> + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); >> + struct ge2d_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue); >> + >> + v4l2_m2m_buf_queue(ctx->fh.m2m_ctx, vbuf); >> +} >> + >> +static int ge2d_start_streaming(struct vb2_queue *vq, unsigned int count) >> +{ >> + struct ge2d_ctx *ctx = vb2_get_drv_priv(vq); >> + >> + if (V4L2_TYPE_IS_OUTPUT(vq->type)) >> + ctx->streamon_out = true; >> + else >> + ctx->streamon_cap = true; >> + >> + if (!ctx->streamon_out) >> + return 0; >> + >> + ctx->sequence_out = 0; >> + ctx->sequence_cap = 0; > > This is wrong, which sequence counter you zero depends on vq->type. > It would only be an issue if userspace stops and restarts just one of the > queues. > > This function can just be: > > static int ge2d_start_streaming(struct vb2_queue *vq, unsigned int count) > { > struct ge2d_ctx *ctx = vb2_get_drv_priv(vq); > > if (V4L2_TYPE_IS_OUTPUT(vq->type)) { > ctx->streamon_out = true; > ctx->sequence_out = 0; > } else { > ctx->streamon_cap = true; > ctx->sequence_cap = 0; > } > return 0; > } > ok > >> + >> + return 0; >> +} >> + >> +static void ge2d_stop_streaming(struct vb2_queue *vq) >> +{ >> + struct ge2d_ctx *ctx = vb2_get_drv_priv(vq); >> + struct vb2_v4l2_buffer *vbuf; >> + >> + if (V4L2_TYPE_IS_OUTPUT(vq->type)) >> + ctx->streamon_out = false; >> + else >> + ctx->streamon_cap = false; >> + >> + for (;;) { >> + if (V4L2_TYPE_IS_OUTPUT(vq->type)) >> + vbuf = v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx); >> + else >> + vbuf = v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx); >> + if (!vbuf) >> + break; >> + v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR); >> + } >> +} >> + >> +static const struct vb2_ops ge2d_qops = { >> + .queue_setup = ge2d_queue_setup, >> + .buf_prepare = ge2d_buf_prepare, >> + .buf_queue = ge2d_buf_queue, >> + .start_streaming = ge2d_start_streaming, >> + .stop_streaming = ge2d_stop_streaming, >> + .wait_prepare = vb2_ops_wait_prepare, >> + .wait_finish = vb2_ops_wait_finish, >> +}; >> + >> +static int >> +queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *dst_vq) >> +{ >> + struct ge2d_ctx *ctx = priv; >> + int ret; >> + >> + src_vq->type = V4L2_BUF_TYPE_VIDEO_OUTPUT; >> + src_vq->io_modes = VB2_MMAP | VB2_DMABUF; >> + src_vq->drv_priv = ctx; >> + src_vq->ops = &ge2d_qops; >> + src_vq->mem_ops = &vb2_dma_contig_memops; >> + src_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); >> + src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY; >> + src_vq->lock = &ctx->ge2d->mutex; >> + src_vq->dev = ctx->ge2d->v4l2_dev.dev; >> + >> + ret = vb2_queue_init(src_vq); >> + if (ret) >> + return ret; >> + >> + dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; >> + dst_vq->io_modes = VB2_MMAP | VB2_DMABUF; >> + dst_vq->drv_priv = ctx; >> + dst_vq->ops = &ge2d_qops; >> + dst_vq->mem_ops = &vb2_dma_contig_memops; >> + dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer); >> + dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY; >> + dst_vq->lock = &ctx->ge2d->mutex; >> + dst_vq->dev = ctx->ge2d->v4l2_dev.dev; >> + >> + return vb2_queue_init(dst_vq); >> +} >> + >> +static int ge2d_s_ctrl(struct v4l2_ctrl *ctrl) >> +{ >> + struct ge2d_ctx *ctx = container_of(ctrl->handler, struct ge2d_ctx, >> + ctrl_handler); >> + struct v4l2_pix_format fmt; > > Shouldn't this be zeroed? I.e.: fmt = {}; > Unless vidioc_setup_cap_fmt() does that already? Sort of ! > >> + struct vb2_queue *vq; >> + unsigned long flags; >> + >> + spin_lock_irqsave(&ctx->ge2d->ctrl_lock, flags); >> + switch (ctrl->id) { >> + case V4L2_CID_HFLIP: >> + ctx->hflip = ctrl->val; >> + break; >> + case V4L2_CID_VFLIP: >> + ctx->vflip = ctrl->val; >> + break; >> + case V4L2_CID_ROTATE: >> + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE); >> + if (vb2_is_busy(vq)) >> + return -EBUSY; >> + >> + if (ctrl->val == 90) { >> + ctx->hflip = 0; >> + ctx->vflip = 0; >> + ctx->xy_swap = 1; >> + } else if (ctrl->val == 180) { >> + ctx->hflip = 1; >> + ctx->vflip = 1; >> + ctx->xy_swap = 0; >> + } else if (ctrl->val == 270) { >> + ctx->hflip = 1; >> + ctx->vflip = 1; >> + ctx->xy_swap = 1; >> + } else { >> + ctx->hflip = 0; >> + ctx->vflip = 0; >> + ctx->xy_swap = 0; >> + } >> + >> + vidioc_setup_cap_fmt(ctx, &fmt); > > Ah, this is why you needed the forward reference to this function. > > Why not just move the ctrl-related functions further down in the > source so you don't need that forward reference? Sure > >> + >> + if (fmt.width != ctx->out.pix_fmt.width || >> + fmt.height != ctx->out.pix_fmt.width || >> + fmt.bytesperline > ctx->out.pix_fmt.bytesperline || >> + fmt.sizeimage > ctx->out.pix_fmt.sizeimage) >> + memcpy(&ctx->out.pix_fmt, &fmt, sizeof(struct v4l2_pix_format)); > > This can be replaced by 'ctx->out.pix_fmt = pix;' > > This 'if' needs a comment, it's not clear what is happening here. ok > >> + >> + break; >> + } >> + spin_unlock_irqrestore(&ctx->ge2d->ctrl_lock, flags); >> + >> + return 0; >> +} >> + >> +static const struct v4l2_ctrl_ops ge2d_ctrl_ops = { >> + .s_ctrl = ge2d_s_ctrl, >> +}; >> + >> +static int ge2d_setup_ctrls(struct ge2d_ctx *ctx) >> +{ >> + struct meson_ge2d *ge2d = ctx->ge2d; >> + >> + v4l2_ctrl_handler_init(&ctx->ctrl_handler, 4); >> + >> + v4l2_ctrl_new_std(&ctx->ctrl_handler, &ge2d_ctrl_ops, >> + V4L2_CID_HFLIP, 0, 1, 1, 0); >> + >> + v4l2_ctrl_new_std(&ctx->ctrl_handler, &ge2d_ctrl_ops, >> + V4L2_CID_VFLIP, 0, 1, 1, 0); >> + >> + v4l2_ctrl_new_std(&ctx->ctrl_handler, &ge2d_ctrl_ops, >> + V4L2_CID_ROTATE, 0, 270, 90, 0); >> + >> + if (ctx->ctrl_handler.error) { >> + int err = ctx->ctrl_handler.error; >> + >> + v4l2_err(&ge2d->v4l2_dev, "%s failed\n", __func__); >> + v4l2_ctrl_handler_free(&ctx->ctrl_handler); >> + return err; >> + } >> + >> + return 0; >> +} >> + >> +static const struct ge2d_frame def_frame = { >> + .pix_fmt = { >> + .width = DEFAULT_WIDTH, >> + .height = DEFAULT_HEIGHT, >> + .field = V4L2_FIELD_NONE, >> + }, >> + .crop.left = 0, >> + .crop.top = 0, > > No need for these two fields. ok > >> + .crop.width = DEFAULT_WIDTH, >> + .crop.height = DEFAULT_HEIGHT, >> + .fmt = &formats[0], >> +}; >> + >> +static int ge2d_open(struct file *file) >> +{ >> + struct meson_ge2d *ge2d = video_drvdata(file); >> + struct ge2d_ctx *ctx = NULL; >> + int ret = 0; >> + >> + ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); >> + if (!ctx) >> + return -ENOMEM; >> + ctx->ge2d = ge2d; >> + >> + /* Set default formats */ >> + ctx->in = def_frame; >> + ctx->out = def_frame; >> + >> + /* Initialize default frames */ >> + ctx->in.pix_fmt.bytesperline = ALIGN((ctx->in.pix_fmt.width * ctx->in.fmt->depth) / 8, 8); >> + ctx->in.pix_fmt.sizeimage = ctx->in.pix_fmt.height * ctx->in.pix_fmt.bytesperline; >> + memcpy(&ctx->out.pix_fmt, &ctx->in.pix_fmt, sizeof(struct v4l2_pix_format)); > > ctx->out.pix_fmt = ctx->in.pix_fmt; ok > > Actually, why not just set these two fields (bytesperline and sizeimage) in def_frame? > Then you can delete these four lines and def_frame is always correct. Because it's not static and depends on the default pix fmt, it was the smartest way I found... > >> + >> + if (mutex_lock_interruptible(&ge2d->mutex)) { >> + kfree(ctx); >> + return -ERESTARTSYS; >> + } >> + ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(ge2d->m2m_dev, ctx, &queue_init); >> + if (IS_ERR(ctx->fh.m2m_ctx)) { >> + ret = PTR_ERR(ctx->fh.m2m_ctx); >> + mutex_unlock(&ge2d->mutex); >> + kfree(ctx); >> + return ret; >> + } >> + v4l2_fh_init(&ctx->fh, video_devdata(file)); >> + file->private_data = &ctx->fh; >> + v4l2_fh_add(&ctx->fh); >> + >> + ge2d_setup_ctrls(ctx); >> + >> + /* Write the default values to the ctx struct */ >> + v4l2_ctrl_handler_setup(&ctx->ctrl_handler); >> + >> + ctx->fh.ctrl_handler = &ctx->ctrl_handler; >> + mutex_unlock(&ge2d->mutex); >> + >> + return 0; >> +} >> + >> +static int ge2d_release(struct file *file) >> +{ >> + struct ge2d_ctx *ctx = >> + container_of(file->private_data, struct ge2d_ctx, fh); >> + struct meson_ge2d *ge2d = ctx->ge2d; >> + >> + mutex_lock(&ge2d->mutex); >> + >> + v4l2_m2m_ctx_release(ctx->fh.m2m_ctx); >> + >> + v4l2_ctrl_handler_free(&ctx->ctrl_handler); >> + v4l2_fh_del(&ctx->fh); >> + v4l2_fh_exit(&ctx->fh); >> + kfree(ctx); >> + >> + mutex_unlock(&ge2d->mutex); >> + >> + return 0; >> +} >> + >> +static const struct v4l2_file_operations ge2d_fops = { >> + .owner = THIS_MODULE, >> + .open = ge2d_open, >> + .release = ge2d_release, >> + .poll = v4l2_m2m_fop_poll, >> + .unlocked_ioctl = video_ioctl2, >> + .mmap = v4l2_m2m_fop_mmap, >> +}; >> + >> +static int >> +vidioc_querycap(struct file *file, void *priv, struct v4l2_capability *cap) >> +{ >> + strscpy(cap->driver, GE2D_NAME, sizeof(cap->driver)); >> + strscpy(cap->card, GE2D_NAME, sizeof(cap->card)); >> + strscpy(cap->bus_info, "platform:" GE2D_NAME, sizeof(cap->bus_info)); >> + >> + return 0; >> +} >> + >> +static int vidioc_enum_fmt(struct file *file, void *priv, struct v4l2_fmtdesc *f) >> +{ >> + const struct ge2d_fmt *fmt; >> + >> + if (f->index >= NUM_FORMATS) >> + return -EINVAL; >> + >> + fmt = &formats[f->index]; >> + f->pixelformat = fmt->fourcc; >> + >> + return 0; >> +} >> + >> +static int vidioc_g_selection(struct file *file, void *priv, >> + struct v4l2_selection *s) >> +{ >> + struct ge2d_ctx *ctx = priv; >> + struct ge2d_frame *f; >> + bool use_frame = false; >> + >> + f = get_frame(ctx, s->type); >> + if (IS_ERR(f)) >> + return PTR_ERR(f); > > Rather than relying on get_frame errors, just add a check for s->type > here. Ditto for s_selection. ok > >> + >> + switch (s->target) { >> + case V4L2_SEL_TGT_COMPOSE_DEFAULT: >> + case V4L2_SEL_TGT_COMPOSE_BOUNDS: >> + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) >> + return -EINVAL; >> + break; >> + case V4L2_SEL_TGT_CROP_DEFAULT: >> + case V4L2_SEL_TGT_CROP_BOUNDS: >> + if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) >> + return -EINVAL; >> + break; >> + case V4L2_SEL_TGT_COMPOSE: >> + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) >> + return -EINVAL; >> + use_frame = true; >> + break; >> + case V4L2_SEL_TGT_CROP: >> + if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) >> + return -EINVAL; >> + use_frame = true; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + if (use_frame) { >> + s->r = f->crop; >> + } else { >> + s->r.left = 0; >> + s->r.top = 0; >> + s->r.width = f->pix_fmt.width; >> + s->r.height = f->pix_fmt.height; >> + } >> + >> + return 0; >> +} >> + >> +static int vidioc_s_selection(struct file *file, void *priv, >> + struct v4l2_selection *s) >> +{ >> + struct ge2d_ctx *ctx = priv; >> + struct meson_ge2d *ge2d = ctx->ge2d; >> + struct ge2d_frame *f; >> + int ret = 0; >> + >> + f = get_frame(ctx, s->type); >> + if (IS_ERR(f)) >> + return PTR_ERR(f); [for me: remove] >> + >> + switch (s->target) { >> + case V4L2_SEL_TGT_COMPOSE: >> + /* >> + * COMPOSE target is only valid for capture buffer type, return >> + * error for output buffer type >> + */ >> + if (s->type != V4L2_BUF_TYPE_VIDEO_CAPTURE) >> + return -EINVAL; >> + break; >> + case V4L2_SEL_TGT_CROP: >> + /* >> + * CROP target is only valid for output buffer type, return >> + * error for capture buffer type >> + */ >> + if (s->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) >> + return -EINVAL; >> + break; >> + /* >> + * bound and default crop/compose targets are invalid targets to >> + * try/set >> + */ >> + default: >> + return -EINVAL; >> + } >> + >> + if (s->r.top < 0 || s->r.left < 0) { >> + v4l2_err(&ge2d->v4l2_dev, >> + "doesn't support negative values for top & left.\n"); >> + return -EINVAL; >> + } >> + >> + if (s->r.left + s->r.width > f->pix_fmt.width || >> + s->r.top + s->r.height > f->pix_fmt.height || >> + s->r.width < 0 || s->r.height < 0) { > > width & height in v4l2_rect are unsigned, so can't be negative. ok > >> + v4l2_err(&ge2d->v4l2_dev, "unsupported crop value.\n"); > > crop -> rectangle ok > >> + return -EINVAL; >> + } >> + >> + f->crop = s->r; >> + >> + return ret; >> +} >> + >> +static void vidioc_setup_cap_fmt(struct ge2d_ctx *ctx, struct v4l2_pix_format *f) >> +{ >> + struct ge2d_frame *frm_out = get_frame(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT); >> + >> + memcpy(f, &frm_out->pix_fmt, sizeof(struct v4l2_pix_format)); > > *f = frm_out->pix_fmt; ok > >> + >> + if (ctx->xy_swap) { >> + f->width = frm_out->pix_fmt.height; >> + f->height = frm_out->pix_fmt.width; >> + } >> +} >> + >> +static int vidioc_try_fmt_cap(struct file *file, void *priv, struct v4l2_format *f) >> +{ >> + const struct ge2d_fmt *fmt = find_fmt(f); >> + struct ge2d_ctx *ctx = priv; >> + struct v4l2_pix_format fmt_cap; >> + >> + vidioc_setup_cap_fmt(ctx, &fmt_cap); >> + >> + fmt_cap.pixelformat = fmt->fourcc; >> + >> + fmt_cap.bytesperline = max(f->fmt.pix.bytesperline, >> + ALIGN((fmt_cap.width * fmt->depth) >> 3, 8)); >> + >> + fmt_cap.sizeimage = max(f->fmt.pix.sizeimage, >> + fmt_cap.height * fmt_cap.bytesperline); >> + >> + memcpy(&f->fmt.pix, &fmt_cap, sizeof(struct v4l2_pix_format)); > > f->fmt.pix = fmt_cap; ok > >> + >> + return 0; >> +} >> + >> +static int vidioc_s_fmt_cap(struct file *file, void *priv, struct v4l2_format *f) >> +{ >> + struct ge2d_ctx *ctx = priv; >> + struct meson_ge2d *ge2d = ctx->ge2d; >> + struct vb2_queue *vq; >> + struct ge2d_frame *frm; >> + int ret = 0; >> + >> + /* Adjust all values accordingly to the hardware capabilities >> + * and chosen format. >> + */ >> + ret = vidioc_try_fmt_cap(file, priv, f); >> + if (ret) >> + return ret; >> + >> + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type); >> + if (vb2_is_busy(vq)) { >> + v4l2_err(&ge2d->v4l2_dev, "queue (%d) bust\n", f->type); >> + return -EBUSY; >> + } >> + >> + frm = get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE); >> + if (IS_ERR(frm)) >> + return PTR_ERR(frm); > > Can't happen. ok > >> + >> + memcpy(&frm->pix_fmt, &f->fmt.pix, sizeof(struct v4l2_pix_format)); >> + frm->fmt = find_fmt(f); >> + f->fmt.pix.pixelformat = frm->fmt->fourcc; >> + >> + /* Reset crop settings */ >> + frm->crop.left = 0; >> + frm->crop.top = 0; >> + frm->crop.width = frm->pix_fmt.width; >> + frm->crop.height = frm->pix_fmt.height; >> + >> + return 0; >> +} >> + >> +static int vidioc_g_fmt(struct file *file, void *priv, struct v4l2_format *f) >> +{ >> + struct ge2d_ctx *ctx = priv; >> + struct vb2_queue *vq; >> + struct ge2d_frame *frm; >> + >> + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type); >> + if (!vq) >> + return -EINVAL; >> + >> + frm = get_frame(ctx, f->type); >> + if (IS_ERR(frm)) >> + return PTR_ERR(frm); > > Ditto. > > I won't repeat myself, the same thing happens several times below. ok will remove all this dead code > >> + >> + memcpy(&f->fmt.pix, &frm->pix_fmt, sizeof(struct v4l2_pix_format)); > > Use assignment. Same several times below. ok > >> + f->fmt.pix.pixelformat = frm->fmt->fourcc; >> + >> + return 0; >> +} >> + >> +static int vidioc_try_fmt_out(struct file *file, void *priv, struct v4l2_format *f) >> +{ >> + const struct ge2d_fmt *fmt = find_fmt(f); >> + >> + f->fmt.pix.field = V4L2_FIELD_NONE; >> + f->fmt.pix.pixelformat = fmt->fourcc; >> + >> + if (f->fmt.pix.width > MAX_WIDTH) >> + f->fmt.pix.width = MAX_WIDTH; >> + if (f->fmt.pix.height > MAX_HEIGHT) >> + f->fmt.pix.height = MAX_HEIGHT; >> + >> + if (f->fmt.pix.width < 0) >> + f->fmt.pix.width = 0; >> + if (f->fmt.pix.height < 0) >> + f->fmt.pix.height = 0; >> + >> + f->fmt.pix.bytesperline = max(f->fmt.pix.bytesperline, >> + ALIGN((f->fmt.pix.width * fmt->depth) >> 3, 8)); >> + >> + f->fmt.pix.sizeimage = max(f->fmt.pix.sizeimage, >> + f->fmt.pix.height * f->fmt.pix.bytesperline); >> + >> + return 0; >> +} >> + >> +static int vidioc_s_fmt_out(struct file *file, void *priv, struct v4l2_format *f) >> +{ >> + struct ge2d_ctx *ctx = priv; >> + struct meson_ge2d *ge2d = ctx->ge2d; >> + struct vb2_queue *vq; >> + struct ge2d_frame *frm, *frm_cap; >> + int ret = 0; >> + >> + /* Adjust all values accordingly to the hardware capabilities >> + * and chosen format. >> + */ >> + ret = vidioc_try_fmt_out(file, priv, f); >> + if (ret) >> + return ret; >> + >> + vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, f->type); >> + if (vb2_is_busy(vq)) { >> + v4l2_err(&ge2d->v4l2_dev, "queue (%d) bust\n", f->type); >> + return -EBUSY; >> + } >> + >> + frm = get_frame(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT); >> + if (IS_ERR(frm)) >> + return PTR_ERR(frm); [for me: remove] >> + frm_cap = get_frame(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE); >> + if (IS_ERR(frm_cap)) >> + return PTR_ERR(frm_cap); [for me: remove] >> + >> + memcpy(&frm->pix_fmt, &f->fmt.pix, sizeof(struct v4l2_pix_format)); [for me: rm->pix_fmt = f->fmt.pix] >> + frm->fmt = find_fmt(f); >> + f->fmt.pix.pixelformat = frm->fmt->fourcc; >> + >> + /* Reset crop settings */ >> + frm->crop.left = 0; >> + frm->crop.top = 0; >> + frm->crop.width = frm->pix_fmt.width; >> + frm->crop.height = frm->pix_fmt.height; >> + >> + /* Propagate settings to capture */ >> + vidioc_setup_cap_fmt(ctx, &frm_cap->pix_fmt); >> + >> + return 0; >> +} >> + >> +static const struct v4l2_ioctl_ops ge2d_ioctl_ops = { >> + .vidioc_querycap = vidioc_querycap, >> + >> + .vidioc_enum_fmt_vid_cap = vidioc_enum_fmt, >> + .vidioc_g_fmt_vid_cap = vidioc_g_fmt, >> + .vidioc_try_fmt_vid_cap = vidioc_try_fmt_cap, >> + .vidioc_s_fmt_vid_cap = vidioc_s_fmt_cap, >> + >> + .vidioc_enum_fmt_vid_out = vidioc_enum_fmt, >> + .vidioc_g_fmt_vid_out = vidioc_g_fmt, >> + .vidioc_try_fmt_vid_out = vidioc_try_fmt_out, >> + .vidioc_s_fmt_vid_out = vidioc_s_fmt_out, >> + >> + .vidioc_reqbufs = v4l2_m2m_ioctl_reqbufs, >> + .vidioc_querybuf = v4l2_m2m_ioctl_querybuf, >> + .vidioc_qbuf = v4l2_m2m_ioctl_qbuf, >> + .vidioc_dqbuf = v4l2_m2m_ioctl_dqbuf, >> + .vidioc_prepare_buf = v4l2_m2m_ioctl_prepare_buf, >> + .vidioc_create_bufs = v4l2_m2m_ioctl_create_bufs, >> + .vidioc_expbuf = v4l2_m2m_ioctl_expbuf, >> + >> + .vidioc_subscribe_event = v4l2_ctrl_subscribe_event, >> + .vidioc_unsubscribe_event = v4l2_event_unsubscribe, >> + >> + .vidioc_streamon = v4l2_m2m_ioctl_streamon, >> + .vidioc_streamoff = v4l2_m2m_ioctl_streamoff, >> + >> + .vidioc_g_selection = vidioc_g_selection, >> + .vidioc_s_selection = vidioc_s_selection, >> +}; >> + >> +static const struct video_device ge2d_videodev = { >> + .name = "meson-ge2d", >> + .fops = &ge2d_fops, >> + .ioctl_ops = &ge2d_ioctl_ops, >> + .minor = -1, >> + .release = video_device_release, >> + .vfl_dir = VFL_DIR_M2M, >> + .device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING, >> +}; >> + >> +static const struct regmap_config meson_ge2d_regmap_conf = { >> + .reg_bits = 8, >> + .val_bits = 32, >> + .reg_stride = 4, >> + .max_register = GE2D_SRC2_STRIDE_CTRL, >> +}; >> + >> +static int ge2d_probe(struct platform_device *pdev) >> +{ >> + struct reset_control *rst; >> + struct video_device *vfd; >> + struct meson_ge2d *ge2d; >> + struct resource *res; >> + void __iomem *regs; >> + int ret = 0; >> + int irq; >> + >> + if (!pdev->dev.of_node) >> + return -ENODEV; >> + >> + ge2d = devm_kzalloc(&pdev->dev, sizeof(*ge2d), GFP_KERNEL); >> + if (!ge2d) >> + return -ENOMEM; >> + >> + ge2d->dev = &pdev->dev; >> + spin_lock_init(&ge2d->ctrl_lock); >> + mutex_init(&ge2d->mutex); >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + regs = devm_ioremap_resource(ge2d->dev, res); >> + if (IS_ERR(regs)) >> + return PTR_ERR(regs); >> + >> + ge2d->map = devm_regmap_init_mmio(ge2d->dev, regs, >> + &meson_ge2d_regmap_conf); >> + if (IS_ERR(ge2d->map)) >> + return PTR_ERR(ge2d->map); >> + >> + irq = platform_get_irq(pdev, 0); >> + ret = devm_request_irq(ge2d->dev, irq, ge2d_isr, 0, >> + dev_name(ge2d->dev), ge2d); >> + if (ret < 0) { >> + dev_err(ge2d->dev, "failed to request irq\n"); >> + return ret; >> + } >> + >> + rst = devm_reset_control_get(ge2d->dev, NULL); >> + if (IS_ERR(rst)) { >> + dev_err(ge2d->dev, "failed to get core reset controller\n"); >> + return PTR_ERR(rst); >> + } >> + >> + ge2d->clk = devm_clk_get(ge2d->dev, NULL); >> + if (IS_ERR(ge2d->clk)) { >> + dev_err(ge2d->dev, "failed to get clock\n"); >> + return PTR_ERR(ge2d->clk); >> + } >> + >> + reset_control_assert(rst); >> + udelay(1); >> + reset_control_deassert(rst); >> + >> + ret = clk_prepare_enable(ge2d->clk); >> + if (ret) { >> + dev_err(ge2d->dev, "Cannot enable ge2d sclk: %d\n", ret); >> + return ret; >> + } >> + >> + ret = v4l2_device_register(&pdev->dev, &ge2d->v4l2_dev); >> + if (ret) >> + goto disable_clks; >> + >> + vfd = video_device_alloc(); >> + if (!vfd) { >> + v4l2_err(&ge2d->v4l2_dev, "Failed to allocate video device\n"); >> + goto unreg_v4l2_dev; >> + } >> + >> + *vfd = ge2d_videodev; >> + vfd->lock = &ge2d->mutex; >> + vfd->v4l2_dev = &ge2d->v4l2_dev; >> + >> + video_set_drvdata(vfd, ge2d); >> + ge2d->vfd = vfd; >> + >> + platform_set_drvdata(pdev, ge2d); >> + ge2d->m2m_dev = v4l2_m2m_init(&ge2d_m2m_ops); >> + if (IS_ERR(ge2d->m2m_dev)) { >> + v4l2_err(&ge2d->v4l2_dev, "Failed to init mem2mem device\n"); >> + ret = PTR_ERR(ge2d->m2m_dev); >> + goto unreg_video_dev; >> + } >> + >> + ret = video_register_device(vfd, VFL_TYPE_VIDEO, -1); >> + if (ret) { >> + v4l2_err(&ge2d->v4l2_dev, "Failed to register video device\n"); >> + goto rel_vdev; >> + } >> + >> + v4l2_info(&ge2d->v4l2_dev, "Registered %s as /dev/%s\n", >> + vfd->name, video_device_node_name(vfd)); >> + >> + return 0; >> + >> +rel_vdev: >> + video_device_release(vfd); >> +unreg_video_dev: >> + video_unregister_device(ge2d->vfd); >> +unreg_v4l2_dev: >> + v4l2_device_unregister(&ge2d->v4l2_dev); >> +disable_clks: >> + clk_disable_unprepare(ge2d->clk); >> + >> + return ret; >> +} >> + >> +static int ge2d_remove(struct platform_device *pdev) >> +{ >> + struct meson_ge2d *ge2d = platform_get_drvdata(pdev); >> + >> + v4l2_m2m_release(ge2d->m2m_dev); >> + video_unregister_device(ge2d->vfd); >> + v4l2_device_unregister(&ge2d->v4l2_dev); >> + >> + clk_disable_unprepare(ge2d->clk); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id meson_ge2d_match[] = { >> + { >> + .compatible = "amlogic,axg-ge2d", >> + }, >> + {}, >> +}; >> + >> +MODULE_DEVICE_TABLE(of, meson_ge2d_match); >> + >> +static struct platform_driver ge2d_drv = { >> + .probe = ge2d_probe, >> + .remove = ge2d_remove, >> + .driver = { >> + .name = "meson-ge2d", >> + .of_match_table = meson_ge2d_match, >> + }, >> +}; >> + >> +module_platform_driver(ge2d_drv); >> + >> +MODULE_AUTHOR("Neil Armstrong <narmstrong@xxxxxxxxxxxx>"); >> +MODULE_DESCRIPTION("Amlogic 2D Graphic Acceleration Unit"); >> +MODULE_LICENSE("GPL"); >> > Thanks for the review ! Neil > Regards, > > Hans >