Hi Marek, Thanks for the patches. On Fri, Jan 14, 2022 at 9:18 AM Marek Vasut <marex@xxxxxxx> wrote: > > The chip register layout has nothing to do with MIPI DCS, the registers > incorrectly marked as MIPI DCS in the driver are regular chip registers > often with completely different function. > > Fill in the actual register names and bits from [1] and [2] and add the > entire register layout, since the documentation for this chip is hard to > come by. > > [1] https://github.com/rockchip-linux/kernel/blob/develop-4.19/drivers/gpu/drm/bridge/icn6211.c > [2] https://github.com/tdjastrzebski/ICN6211-Configurator > > Fixes: ce517f18944e3 ("drm: bridge: Add Chipone ICN6211 MIPI-DSI to RGB bridge") > Signed-off-by: Marek Vasut <marex@xxxxxxx> > Cc: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx> > Cc: Robert Foss <robert.foss@xxxxxxxxxx> > Cc: Sam Ravnborg <sam@xxxxxxxxxxxx> > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> > To: dri-devel@xxxxxxxxxxxxxxxxxxxxx > --- > drivers/gpu/drm/bridge/chipone-icn6211.c | 134 ++++++++++++++++++++--- > 1 file changed, 117 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/chipone-icn6211.c b/drivers/gpu/drm/bridge/chipone-icn6211.c > index a6151db955868..eb26615b2993a 100644 > --- a/drivers/gpu/drm/bridge/chipone-icn6211.c > +++ b/drivers/gpu/drm/bridge/chipone-icn6211.c > @@ -14,8 +14,19 @@ > #include <linux/of_device.h> > #include <linux/regulator/consumer.h> > > -#include <video/mipi_display.h> > - > +#define VENDOR_ID 0x00 > +#define DEVICE_ID_H 0x01 > +#define DEVICE_ID_L 0x02 > +#define VERSION_ID 0x03 > +#define FIRMWARE_VERSION 0x08 > +#define CONFIG_FINISH 0x09 > +#define PD_CTRL(n) (0x0a + ((n) & 0x3)) /* 0..3 */ > +#define RST_CTRL(n) (0x0e + ((n) & 0x1)) /* 0..1 */ > +#define SYS_CTRL(n) (0x10 + ((n) & 0x7)) /* 0..4 */ > +#define RGB_DRV(n) (0x18 + ((n) & 0x3)) /* 0..3 */ > +#define RGB_DLY(n) (0x1c + ((n) & 0x1)) /* 0..1 */ > +#define RGB_TEST_CTRL 0x1e > +#define ATE_PLL_EN 0x1f > #define HACTIVE_LI 0x20 > #define VACTIVE_LI 0x21 > #define VACTIVE_HACTIVE_HI 0x22 > @@ -26,6 +37,95 @@ > #define VFP 0x27 > #define VSYNC 0x28 > #define VBP 0x29 > +#define BIST_POL 0x2a > +#define BIST_POL_BIST_MODE(n) (((n) & 0xf) << 4) > +#define BIST_POL_BIST_GEN BIT(3) > +#define BIST_POL_HSYNC_POL BIT(2) > +#define BIST_POL_VSYNC_POL BIT(1) > +#define BIST_POL_DE_POL BIT(0) > +#define BIST_RED 0x2b > +#define BIST_GREEN 0x2c > +#define BIST_BLUE 0x2d > +#define BIST_CHESS_X 0x2e > +#define BIST_CHESS_Y 0x2f > +#define BIST_CHESS_XY_H 0x30 > +#define BIST_FRAME_TIME_L 0x31 > +#define BIST_FRAME_TIME_H 0x32 > +#define FIFO_MAX_ADDR_LOW 0x33 > +#define SYNC_EVENT_DLY 0x34 > +#define HSW_MIN 0x35 > +#define HFP_MIN 0x36 > +#define LOGIC_RST_NUM 0x37 > +#define OSC_CTRL(n) (0x48 + ((n) & 0x7)) /* 0..5 */ > +#define BG_CTRL 0x4e > +#define LDO_PLL 0x4f > +#define PLL_CTRL(n) (0x50 + ((n) & 0xf)) /* 0..15 */ > +#define PLL_CTRL_6_EXTERNAL 0x90 > +#define PLL_CTRL_6_MIPI_CLK 0x92 > +#define PLL_CTRL_6_INTERNAL 0x93 > +#define PLL_REM(n) (0x60 + ((n) & 0x3)) /* 0..2 */ > +#define PLL_DIV(n) (0x63 + ((n) & 0x3)) /* 0..2 */ > +#define PLL_FRAC(n) (0x66 + ((n) & 0x3)) /* 0..2 */ > +#define PLL_INT(n) (0x69 + ((n) & 0x1)) /* 0..1 */ > +#define PLL_REF_DIV 0x6b > +#define PLL_REF_DIV_P(n) ((n) & 0xf) > +#define PLL_REF_DIV_Pe BIT(4) > +#define PLL_REF_DIV_S(n) (((n) & 0x7) << 5) > +#define PLL_SSC_P(n) (0x6c + ((n) & 0x3)) /* 0..2 */ > +#define PLL_SSC_STEP(n) (0x6f + ((n) & 0x3)) /* 0..2 */ > +#define PLL_SSC_OFFSET(n) (0x72 + ((n) & 0x3)) /* 0..3 */ > +#define GPIO_OEN 0x79 > +#define MIPI_CFG_PW 0x7a > +#define MIPI_CFG_PW_CONFIG_DSI 0xc1 > +#define MIPI_CFG_PW_CONFIG_I2C 0x3e > +#define GPIO_SEL(n) (0x7b + ((n) & 0x1)) /* 0..1 */ > +#define IRQ_SEL 0x7d > +#define DBG_SEL 0x7e > +#define DBG_SIGNAL 0x7f > +#define MIPI_ERR_VECTOR_L 0x80 > +#define MIPI_ERR_VECTOR_H 0x81 > +#define MIPI_ERR_VECTOR_EN_L 0x82 > +#define MIPI_ERR_VECTOR_EN_H 0x83 > +#define MIPI_MAX_SIZE_L 0x84 > +#define MIPI_MAX_SIZE_H 0x85 > +#define DSI_CTRL 0x86 > +#define DSI_CTRL_UNKNOWN 0x28 > +#define DSI_CTRL_DSI_LANES(n) ((n) & 0x3) > +#define MIPI_PN_SWAP 0x87 > +#define MIPI_PN_SWAP_CLK BIT(4) > +#define MIPI_PN_SWAP_D(n) BIT((n) & 0x3) > +#define MIPI_SOT_SYNC_BIT_(n) (0x88 + ((n) & 0x1)) /* 0..1 */ > +#define MIPI_ULPS_CTRL 0x8a > +#define MIPI_CLK_CHK_VAR 0x8e > +#define MIPI_CLK_CHK_INI 0x8f > +#define MIPI_T_TERM_EN 0x90 > +#define MIPI_T_HS_SETTLE 0x91 > +#define MIPI_T_TA_SURE_PRE 0x92 > +#define MIPI_T_LPX_SET 0x94 > +#define MIPI_T_CLK_MISS 0x95 > +#define MIPI_INIT_TIME_L 0x96 > +#define MIPI_INIT_TIME_H 0x97 > +#define MIPI_T_CLK_TERM_EN 0x99 > +#define MIPI_T_CLK_SETTLE 0x9a > +#define MIPI_TO_HS_RX_L 0x9e > +#define MIPI_TO_HS_RX_H 0x9f > +#define MIPI_PHY_(n) (0xa0 + ((n) & 0x7)) /* 0..5 */ > +#define MIPI_PD_RX 0xb0 > +#define MIPI_PD_TERM 0xb1 > +#define MIPI_PD_HSRX 0xb2 > +#define MIPI_PD_LPTX 0xb3 > +#define MIPI_PD_LPRX 0xb4 > +#define MIPI_PD_CK_LANE 0xb5 > +#define MIPI_FORCE_0 0xb6 > +#define MIPI_RST_CTRL 0xb7 > +#define MIPI_RST_NUM 0xb8 > +#define MIPI_DBG_SET_(n) (0xc0 + ((n) & 0xf)) /* 0..9 */ > +#define MIPI_DBG_SEL 0xe0 > +#define MIPI_DBG_DATA 0xe1 > +#define MIPI_ATE_TEST_SEL 0xe2 > +#define MIPI_ATE_STATUS_(n) (0xe3 + ((n) & 0x1)) /* 0..1 */ > +#define MIPI_ATE_STATUS_1 0xe4 > +#define ICN6211_MAX_REGISTER MIPI_ATE_STATUS(1) > > struct chipone { > struct device *dev; > @@ -66,13 +166,13 @@ static void chipone_enable(struct drm_bridge *bridge) > struct chipone *icn = bridge_to_chipone(bridge); > struct drm_display_mode *mode = bridge_to_mode(bridge); > > - ICN6211_DSI(icn, 0x7a, 0xc1); > + ICN6211_DSI(icn, MIPI_CFG_PW, MIPI_CFG_PW_CONFIG_DSI); > > ICN6211_DSI(icn, HACTIVE_LI, mode->hdisplay & 0xff); > > ICN6211_DSI(icn, VACTIVE_LI, mode->vdisplay & 0xff); > > - /** > + /* > * lsb nibble: 2nd nibble of hdisplay > * msb nibble: 2nd nibble of vdisplay > */ > @@ -95,21 +195,21 @@ static void chipone_enable(struct drm_bridge *bridge) > ICN6211_DSI(icn, VBP, mode->vtotal - mode->vsync_end); > > /* dsi specific sequence */ > - ICN6211_DSI(icn, MIPI_DCS_SET_TEAR_OFF, 0x80); > - ICN6211_DSI(icn, MIPI_DCS_SET_ADDRESS_MODE, 0x28); > - ICN6211_DSI(icn, 0xb5, 0xa0); > - ICN6211_DSI(icn, 0x5c, 0xff); > - ICN6211_DSI(icn, MIPI_DCS_SET_COLUMN_ADDRESS, 0x01); > - ICN6211_DSI(icn, MIPI_DCS_GET_POWER_SAVE, 0x92); > - ICN6211_DSI(icn, 0x6b, 0x71); > - ICN6211_DSI(icn, 0x69, 0x2b); > - ICN6211_DSI(icn, MIPI_DCS_ENTER_SLEEP_MODE, 0x40); > - ICN6211_DSI(icn, MIPI_DCS_EXIT_SLEEP_MODE, 0x98); > + ICN6211_DSI(icn, SYNC_EVENT_DLY, 0x80); > + ICN6211_DSI(icn, HFP_MIN, 0x28); > + ICN6211_DSI(icn, MIPI_PD_CK_LANE, 0xa0); > + ICN6211_DSI(icn, PLL_CTRL(12), 0xff); > + ICN6211_DSI(icn, BIST_POL, BIST_POL_DE_POL); > + ICN6211_DSI(icn, PLL_CTRL(6), PLL_CTRL_6_MIPI_CLK); > + ICN6211_DSI(icn, PLL_REF_DIV, 0x71); > + ICN6211_DSI(icn, PLL_INT(0), 0x2b); > + ICN6211_DSI(icn, SYS_CTRL(0), 0x40); > + ICN6211_DSI(icn, SYS_CTRL(1), 0x98); > > /* icn6211 specific sequence */ > - ICN6211_DSI(icn, 0xb6, 0x20); > - ICN6211_DSI(icn, 0x51, 0x20); > - ICN6211_DSI(icn, 0x09, 0x10); > + ICN6211_DSI(icn, MIPI_FORCE_0, 0x20); > + ICN6211_DSI(icn, PLL_CTRL(1), 0x20); > + ICN6211_DSI(icn, CONFIG_FINISH, 0x10); All these fixes and few of features support are valid only for I2C-based ICN6211. If possible please confirm with the vendor. The driver I've written based on non-I2C-based ICN6211 chip, which is present in BananaPi Panel. Chip part: ICN6211 A59058 1634. Not sure, may be we can have separated bridge driver for I2C-based ICN6211 that I don't have in my design for testing. Thanks, Jagan.
