Hi Nickey, Several people already made comments on the initial version of this patch [1], and I don't think you've caught them all here yet. I'll repeat a few. Not sure if I've caught them all. [1] https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/780120 On Tue, Nov 28, 2017 at 09:13:35AM +0800, Nickey Yang wrote: > Add the ROCKCHIP DSI controller driver that uses the Synopsys DesignWare > MIPI DSI host controller bridge. > > Signed-off-by: Nickey Yang <nickey.yang@xxxxxxxxxxxxxx> > --- > drivers/gpu/drm/rockchip/Kconfig | 2 +- > drivers/gpu/drm/rockchip/Makefile | 2 +- > drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 1349 ----------------------- > drivers/gpu/drm/rockchip/dw-mipi-dsi_rockchip.c | 756 +++++++++++++ > drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 2 +- > drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 +- > 6 files changed, 760 insertions(+), 1353 deletions(-) > delete mode 100644 drivers/gpu/drm/rockchip/dw-mipi-dsi.c > create mode 100644 drivers/gpu/drm/rockchip/dw-mipi-dsi_rockchip.c > ... > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi_rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi_rockchip.c > new file mode 100644 > index 0000000..32be430 > --- /dev/null > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi_rockchip.c > @@ -0,0 +1,756 @@ > +/* > + * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd > + * Author: > + * Chris Zhong <zyw@xxxxxxxxxxxxxx> > + * Nickey Yang <nickey.yang@xxxxxxxxxxxxxx> > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. > + * > + * License terms: GNU General Public License (GPL), version 2 > + */ > + > +#include <linux/clk.h> > +#include <linux/iopoll.h> > +#include <linux/math64.h> > +#include <linux/module.h> > +#include <linux/of_device.h> > +#include <drm/drmP.h> > +#include <drm/drm_mipi_dsi.h> > +#include <drm/bridge/dw_mipi_dsi.h> > +#include <video/mipi_display.h> > +#include <linux/regmap.h> > +#include <drm/drm_of.h> > +#include <linux/mfd/syscon.h> > + > +#include "rockchip_drm_drv.h" > +#include "rockchip_drm_vop.h" > + > +#define DSI_PHY_TST_CTRL0 0xb4 > +#define PHY_TESTCLK BIT(1) > +#define PHY_UNTESTCLK 0 > +#define PHY_TESTCLR BIT(0) > +#define PHY_UNTESTCLR 0 > + > +#define DSI_PHY_TST_CTRL1 0xb8 > +#define PHY_TESTEN BIT(16) > +#define PHY_UNTESTEN 0 > +#define PHY_TESTDOUT(n) (((n) & 0xff) << 8) > +#define PHY_TESTDIN(n) (((n) & 0xff) << 0) > + > +#define BYPASS_VCO_RANGE BIT(7) > +#define VCO_RANGE_CON_SEL(val) (((val) & 0x7) << 3) > +#define VCO_IN_CAP_CON_DEFAULT (0x0 << 1) > +#define VCO_IN_CAP_CON_LOW (0x1 << 1) > +#define VCO_IN_CAP_CON_HIGH (0x2 << 1) > +#define REF_BIAS_CUR_SEL BIT(0) > + > +#define CP_CURRENT_3UA 0x1 > +#define CP_CURRENT_4_5UA 0x2 > +#define CP_CURRENT_7_5UA 0x6 > +#define CP_CURRENT_6UA 0x9 > +#define CP_CURRENT_12UA 0xb > +#define CP_CURRENT_SEL(val) ((val) & 0xf) > +#define CP_PROGRAM_EN BIT(7) > + > +#define LPF_RESISTORS_15_5KOHM 0x1 > +#define LPF_RESISTORS_13KOHM 0x2 > +#define LPF_RESISTORS_11_5KOHM 0x4 > +#define LPF_RESISTORS_10_5KOHM 0x8 > +#define LPF_RESISTORS_8KOHM 0x10 > +#define LPF_PROGRAM_EN BIT(6) > +#define LPF_RESISTORS_SEL(val) ((val) & 0x3f) > + > +#define HSFREQRANGE_SEL(val) (((val) & 0x3f) << 1) > + > +#define INPUT_DIVIDER(val) (((val) - 1) & 0x7f) > +#define LOW_PROGRAM_EN 0 > +#define HIGH_PROGRAM_EN BIT(7) > +#define LOOP_DIV_LOW_SEL(val) (((val) - 1) & 0x1f) > +#define LOOP_DIV_HIGH_SEL(val) ((((val) - 1) >> 5) & 0xf) > +#define PLL_LOOP_DIV_EN BIT(5) > +#define PLL_INPUT_DIV_EN BIT(4) > + > +#define POWER_CONTROL BIT(6) > +#define INTERNAL_REG_CURRENT BIT(3) > +#define BIAS_BLOCK_ON BIT(2) > +#define BANDGAP_ON BIT(0) > + > +#define TER_RESISTOR_HIGH BIT(7) > +#define TER_RESISTOR_LOW 0 > +#define LEVEL_SHIFTERS_ON BIT(6) > +#define TER_CAL_DONE BIT(5) > +#define SETRD_MAX (0x7 << 2) > +#define POWER_MANAGE BIT(1) > +#define TER_RESISTORS_ON BIT(0) > + > +#define BIASEXTR_SEL(val) ((val) & 0x7) > +#define BANDGAP_SEL(val) ((val) & 0x7) > +#define TLP_PROGRAM_EN BIT(7) > +#define THS_PRE_PROGRAM_EN BIT(7) > +#define THS_ZERO_PROGRAM_EN BIT(6) > + > +#define PLL_BIAS_CUR_SEL_CAP_VCO_CONTROL 0x10 > +#define PLL_CP_CONTROL_PLL_LOCK_BYPASS 0x11 > +#define PLL_LPF_AND_CP_CONTROL 0x12 > +#define PLL_INPUT_DIVIDER_RATIO 0x17 > +#define PLL_LOOP_DIVIDER_RATIO 0x18 > +#define PLL_INPUT_AND_LOOP_DIVIDER_RATIOS_CONTROL 0x19 > +#define BANDGAP_AND_BIAS_CONTROL 0x20 > +#define TERMINATION_RESISTER_CONTROL 0x21 > +#define AFE_BIAS_BANDGAP_ANALOG_PROGRAMMABILITY 0x22 > +#define HS_RX_CONTROL_OF_LANE_0 0x44 > +#define HS_TX_CLOCK_LANE_REQUEST_STATE_TIME_CONTROL 0x60 > +#define HS_TX_CLOCK_LANE_PREPARE_STATE_TIME_CONTROL 0x61 > +#define HS_TX_CLOCK_LANE_HS_ZERO_STATE_TIME_CONTROL 0x62 > +#define HS_TX_CLOCK_LANE_TRAIL_STATE_TIME_CONTROL 0x63 > +#define HS_TX_CLOCK_LANE_EXIT_STATE_TIME_CONTROL 0x64 > +#define HS_TX_CLOCK_LANE_POST_TIME_CONTROL 0x65 > +#define HS_TX_DATA_LANE_REQUEST_STATE_TIME_CONTROL 0x70 > +#define HS_TX_DATA_LANE_PREPARE_STATE_TIME_CONTROL 0x71 > +#define HS_TX_DATA_LANE_HS_ZERO_STATE_TIME_CONTROL 0x72 > +#define HS_TX_DATA_LANE_TRAIL_STATE_TIME_CONTROL 0x73 > +#define HS_TX_DATA_LANE_EXIT_STATE_TIME_CONTROL 0x74 > + > +#define DW_MIPI_NEEDS_PHY_CFG_CLK BIT(0) > +#define DW_MIPI_NEEDS_GRF_CLK BIT(1) > + > +#define RK3288_GRF_SOC_CON6 0x025c > +#define RK3288_DSI0_SEL_VOP_LIT BIT(6) > +#define RK3288_DSI1_SEL_VOP_LIT BIT(9) > + > +#define RK3399_GRF_SOC_CON20 0x6250 > +#define RK3399_DSI0_SEL_VOP_LIT BIT(0) > +#define RK3399_DSI1_SEL_VOP_LIT BIT(4) > + > +/* disable turnrequest, turndisable, forcetxstopmode, forcerxmode */ > +#define RK3399_GRF_SOC_CON22 0x6258 > +#define RK3399_GRF_DSI_MODE 0xffff0000 > + > +#define to_dsi(nm) container_of(nm, struct dw_mipi_dsi_rockchip, nm) > + > +enum { > + BANDGAP_97_07, > + BANDGAP_98_05, > + BANDGAP_99_02, > + BANDGAP_100_00, > + BANDGAP_93_17, > + BANDGAP_94_15, > + BANDGAP_95_12, > + BANDGAP_96_10, > +}; > + > +enum { > + BIASEXTR_87_1, > + BIASEXTR_91_5, > + BIASEXTR_95_9, > + BIASEXTR_100, > + BIASEXTR_105_94, > + BIASEXTR_111_88, > + BIASEXTR_118_8, > + BIASEXTR_127_7, > +}; > + > +struct rockchip_dw_dsi_chip_data { > + u32 dsi0_en_bit; > + u32 dsi1_en_bit; > + u32 grf_switch_reg; > + u32 grf_dsi0_mode; > + u32 grf_dsi0_mode_reg; > + unsigned int flags; > + unsigned int max_data_lanes; > +}; > + > +struct dw_mipi_dsi_rockchip { > + struct device *dev; > + struct drm_encoder encoder; > + void __iomem *base; > + > + struct regmap *grf_regmap; > + struct clk *pllref_clk; > + struct clk *grf_clk; > + struct clk *phy_cfg_clk; > + > + unsigned int lane_mbps; /* per lane */ > + u16 input_div; > + u16 feedback_div; > + u32 format; > + > + const struct rockchip_dw_dsi_chip_data *cdata; > + struct dw_mipi_dsi_plat_data pdata; > +}; > + > +struct dphy_pll_parameter_map { > + unsigned int max_mbps; > + u8 hsfreqrange; > + u8 icpctrl; > + u8 lpfctrl; > +}; > + > +/* The table is based on 27MHz DPHY pll reference clock. */ > +static const struct dphy_pll_parameter_map dppa_map[] = { > + { 89, 0x00, CP_CURRENT_3UA, LPF_RESISTORS_13KOHM}, > + { 99, 0x10, CP_CURRENT_3UA, LPF_RESISTORS_13KOHM}, > + { 109, 0x20, CP_CURRENT_3UA, LPF_RESISTORS_13KOHM}, > + { 129, 0x01, CP_CURRENT_3UA, LPF_RESISTORS_15_5KOHM}, > + { 139, 0x11, CP_CURRENT_3UA, LPF_RESISTORS_15_5KOHM}, > + { 149, 0x21, CP_CURRENT_3UA, LPF_RESISTORS_15_5KOHM}, > + { 169, 0x02, CP_CURRENT_6UA, LPF_RESISTORS_13KOHM}, > + { 179, 0x12, CP_CURRENT_6UA, LPF_RESISTORS_13KOHM}, > + { 199, 0x22, CP_CURRENT_6UA, LPF_RESISTORS_13KOHM}, > + { 219, 0x03, CP_CURRENT_4_5UA, LPF_RESISTORS_13KOHM}, > + { 239, 0x13, CP_CURRENT_4_5UA, LPF_RESISTORS_13KOHM}, > + { 249, 0x23, CP_CURRENT_4_5UA, LPF_RESISTORS_13KOHM}, > + { 269, 0x04, CP_CURRENT_6UA, LPF_RESISTORS_11_5KOHM}, > + { 299, 0x14, CP_CURRENT_6UA, LPF_RESISTORS_11_5KOHM}, > + { 329, 0x05, CP_CURRENT_3UA, LPF_RESISTORS_15_5KOHM}, > + { 359, 0x15, CP_CURRENT_3UA, LPF_RESISTORS_15_5KOHM}, > + { 399, 0x25, CP_CURRENT_3UA, LPF_RESISTORS_15_5KOHM}, > + { 449, 0x06, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 499, 0x16, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 549, 0x07, CP_CURRENT_7_5UA, LPF_RESISTORS_10_5KOHM}, > + { 599, 0x17, CP_CURRENT_7_5UA, LPF_RESISTORS_10_5KOHM}, > + { 649, 0x08, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 699, 0x18, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 749, 0x09, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 799, 0x19, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 849, 0x29, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 899, 0x39, CP_CURRENT_7_5UA, LPF_RESISTORS_11_5KOHM}, > + { 949, 0x0a, CP_CURRENT_12UA, LPF_RESISTORS_8KOHM}, > + { 999, 0x1a, CP_CURRENT_12UA, LPF_RESISTORS_8KOHM}, > + {1049, 0x2a, CP_CURRENT_12UA, LPF_RESISTORS_8KOHM}, > + {1099, 0x3a, CP_CURRENT_12UA, LPF_RESISTORS_8KOHM}, > + {1149, 0x0b, CP_CURRENT_12UA, LPF_RESISTORS_10_5KOHM}, > + {1199, 0x1b, CP_CURRENT_12UA, LPF_RESISTORS_10_5KOHM}, > + {1249, 0x2b, CP_CURRENT_12UA, LPF_RESISTORS_10_5KOHM}, > + {1299, 0x3b, CP_CURRENT_12UA, LPF_RESISTORS_10_5KOHM}, > + {1349, 0x0c, CP_CURRENT_12UA, LPF_RESISTORS_10_5KOHM}, > + {1399, 0x1c, CP_CURRENT_12UA, LPF_RESISTORS_10_5KOHM}, > + {1449, 0x2c, CP_CURRENT_12UA, LPF_RESISTORS_10_5KOHM}, > + {1500, 0x3c, CP_CURRENT_12UA, LPF_RESISTORS_10_5KOHM} > +}; > + > +static int max_mbps_to_parameter(unsigned int max_mbps) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(dppa_map); i++) > + if (dppa_map[i].max_mbps >= max_mbps) > + return i; > + > + return -EINVAL; > +} > + > +static inline void dsi_write(struct dw_mipi_dsi_rockchip *dsi, u32 reg, u32 val) > +{ > + writel(val, dsi->base + reg); > +} > + > +static inline u32 dsi_read(struct dw_mipi_dsi_rockchip *dsi, u32 reg) > +{ > + return readl(dsi->base + reg); > +} > + > +static inline void dsi_set(struct dw_mipi_dsi_rockchip *dsi, u32 reg, u32 mask) > +{ > + dsi_write(dsi, reg, dsi_read(dsi, reg) | mask); > +} > + > +static inline void dsi_update_bits(struct dw_mipi_dsi_rockchip *dsi, u32 reg, > + u32 mask, u32 val) > +{ > + dsi_write(dsi, reg, (dsi_read(dsi, reg) & ~mask) | val); > +} > + > +static void dw_mipi_dsi_phy_write(struct dw_mipi_dsi_rockchip *dsi, > + u8 test_code, > + u8 test_data) > +{ > + /* > + * With the falling edge on TESTCLK, the TESTDIN[7:0] signal content > + * is latched internally as the current test code. Test data is > + * programmed internally by rising edge on TESTCLK. > + */ > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR); > + > + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_TESTEN | PHY_TESTDOUT(0) | > + PHY_TESTDIN(test_code)); > + > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_UNTESTCLK | PHY_UNTESTCLR); > + > + dsi_write(dsi, DSI_PHY_TST_CTRL1, PHY_UNTESTEN | PHY_TESTDOUT(0) | > + PHY_TESTDIN(test_data)); > + > + dsi_write(dsi, DSI_PHY_TST_CTRL0, PHY_TESTCLK | PHY_UNTESTCLR); > +} > + > +/** > + * ns2bc - Nanoseconds to byte clock cycles > + */ > +static inline unsigned int ns2bc(struct dw_mipi_dsi_rockchip *dsi, int ns) > +{ > + return DIV_ROUND_UP(ns * dsi->lane_mbps / 8, 1000); > +} > + > +/** > + * ns2ui - Nanoseconds to UI time periods > + */ > +static inline unsigned int ns2ui(struct dw_mipi_dsi_rockchip *dsi, int ns) > +{ > + return DIV_ROUND_UP(ns * dsi->lane_mbps, 1000); > +} > + > +static int dw_mipi_dsi_phy_init(void *priv_data) > +{ > + struct dw_mipi_dsi_rockchip *dsi = priv_data; > + int ret, i, vco; > + > + vco = (dsi->lane_mbps < 200) ? 0 : (dsi->lane_mbps + 100) / 200; > + > + i = max_mbps_to_parameter(dsi->lane_mbps); > + if (i < 0) { > + DRM_DEV_ERROR(dsi->dev, > + "failed to get parameter for %dmbps clock\n", > + dsi->lane_mbps); > + return i; > + } > + > + ret = clk_prepare_enable(dsi->phy_cfg_clk); > + if (ret) { > + DRM_DEV_ERROR(dsi->dev, "Failed to enable phy_cfg_clk\n"); > + return ret; > + } > + > + dw_mipi_dsi_phy_write(dsi, PLL_BIAS_CUR_SEL_CAP_VCO_CONTROL, > + BYPASS_VCO_RANGE | > + VCO_RANGE_CON_SEL(vco) | > + VCO_IN_CAP_CON_LOW | > + REF_BIAS_CUR_SEL); > + > + dw_mipi_dsi_phy_write(dsi, PLL_CP_CONTROL_PLL_LOCK_BYPASS, > + CP_CURRENT_SEL(dppa_map[i].icpctrl)); > + dw_mipi_dsi_phy_write(dsi, PLL_LPF_AND_CP_CONTROL, > + CP_PROGRAM_EN | LPF_PROGRAM_EN | > + LPF_RESISTORS_SEL(dppa_map[i].lpfctrl)); > + > + dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_0, > + HSFREQRANGE_SEL(dppa_map[i].hsfreqrange)); > + > + dw_mipi_dsi_phy_write(dsi, PLL_INPUT_DIVIDER_RATIO, > + INPUT_DIVIDER(dsi->input_div)); > + dw_mipi_dsi_phy_write(dsi, PLL_LOOP_DIVIDER_RATIO, > + LOOP_DIV_LOW_SEL(dsi->feedback_div) | > + LOW_PROGRAM_EN); > + /* > + * We need set PLL_INPUT_AND_LOOP_DIVIDER_RATIOS_CONTROL immediately > + * to make the configrued LSB effective according to IP simulation "configured" > + * and lab test results. > + * Only in this way can we get correct mipi phy pll frequency. > + */ > + dw_mipi_dsi_phy_write(dsi, PLL_INPUT_AND_LOOP_DIVIDER_RATIOS_CONTROL, > + PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN); > + dw_mipi_dsi_phy_write(dsi, PLL_LOOP_DIVIDER_RATIO, > + LOOP_DIV_HIGH_SEL(dsi->feedback_div) | > + HIGH_PROGRAM_EN); > + dw_mipi_dsi_phy_write(dsi, PLL_INPUT_AND_LOOP_DIVIDER_RATIOS_CONTROL, > + PLL_LOOP_DIV_EN | PLL_INPUT_DIV_EN); > + > + dw_mipi_dsi_phy_write(dsi, AFE_BIAS_BANDGAP_ANALOG_PROGRAMMABILITY, > + LOW_PROGRAM_EN | BIASEXTR_SEL(BIASEXTR_127_7)); > + dw_mipi_dsi_phy_write(dsi, AFE_BIAS_BANDGAP_ANALOG_PROGRAMMABILITY, > + HIGH_PROGRAM_EN | BANDGAP_SEL(BANDGAP_96_10)); > + > + dw_mipi_dsi_phy_write(dsi, BANDGAP_AND_BIAS_CONTROL, > + POWER_CONTROL | INTERNAL_REG_CURRENT | > + BIAS_BLOCK_ON | BANDGAP_ON); > + > + dw_mipi_dsi_phy_write(dsi, TERMINATION_RESISTER_CONTROL, > + TER_RESISTOR_LOW | TER_CAL_DONE | > + SETRD_MAX | TER_RESISTORS_ON); > + dw_mipi_dsi_phy_write(dsi, TERMINATION_RESISTER_CONTROL, > + TER_RESISTOR_HIGH | LEVEL_SHIFTERS_ON | > + SETRD_MAX | POWER_MANAGE | > + TER_RESISTORS_ON); > + > + dw_mipi_dsi_phy_write(dsi, HS_TX_CLOCK_LANE_REQUEST_STATE_TIME_CONTROL, > + TLP_PROGRAM_EN | ns2bc(dsi, 500)); > + dw_mipi_dsi_phy_write(dsi, HS_TX_CLOCK_LANE_PREPARE_STATE_TIME_CONTROL, > + THS_PRE_PROGRAM_EN | ns2ui(dsi, 40)); > + dw_mipi_dsi_phy_write(dsi, HS_TX_CLOCK_LANE_HS_ZERO_STATE_TIME_CONTROL, > + THS_ZERO_PROGRAM_EN | ns2bc(dsi, 300)); > + dw_mipi_dsi_phy_write(dsi, HS_TX_CLOCK_LANE_TRAIL_STATE_TIME_CONTROL, > + THS_PRE_PROGRAM_EN | ns2ui(dsi, 100)); > + dw_mipi_dsi_phy_write(dsi, HS_TX_CLOCK_LANE_EXIT_STATE_TIME_CONTROL, > + BIT(5) | ns2bc(dsi, 100)); > + dw_mipi_dsi_phy_write(dsi, HS_TX_CLOCK_LANE_POST_TIME_CONTROL, > + BIT(5) | (ns2bc(dsi, 60) + 7)); > + > + dw_mipi_dsi_phy_write(dsi, HS_TX_DATA_LANE_REQUEST_STATE_TIME_CONTROL, > + TLP_PROGRAM_EN | ns2bc(dsi, 500)); > + dw_mipi_dsi_phy_write(dsi, HS_TX_DATA_LANE_PREPARE_STATE_TIME_CONTROL, > + THS_PRE_PROGRAM_EN | (ns2ui(dsi, 50) + 20)); > + dw_mipi_dsi_phy_write(dsi, HS_TX_DATA_LANE_HS_ZERO_STATE_TIME_CONTROL, > + THS_ZERO_PROGRAM_EN | (ns2bc(dsi, 140) + 2)); > + dw_mipi_dsi_phy_write(dsi, HS_TX_DATA_LANE_TRAIL_STATE_TIME_CONTROL, > + THS_PRE_PROGRAM_EN | (ns2ui(dsi, 60) + 8)); > + dw_mipi_dsi_phy_write(dsi, HS_TX_DATA_LANE_EXIT_STATE_TIME_CONTROL, > + BIT(5) | ns2bc(dsi, 100)); > + > + clk_disable_unprepare(dsi->phy_cfg_clk); > + > + return ret; > +} > + > +static int > +dw_mipi_dsi_get_lane_mbps(void *priv_data, struct drm_display_mode *mode, > + unsigned long mode_flags, u32 lanes, u32 format, > + unsigned int *lane_mbps) > +{ > + struct dw_mipi_dsi_rockchip *dsi = priv_data; > + int bpp; > + unsigned long mpclk, tmp; > + unsigned int target_mbps = 1000; > + unsigned int max_mbps = dppa_map[ARRAY_SIZE(dppa_map) - 1].max_mbps; > + unsigned long best_freq = 0; > + unsigned long fvco_min, fvco_max, fin, fout; > + unsigned int min_prediv, max_prediv; > + unsigned int _prediv, uninitialized_var(best_prediv); > + unsigned long _fbdiv, uninitialized_var(best_fbdiv); > + unsigned long min_delta = ULONG_MAX; > + > + dsi->format = format; > + bpp = mipi_dsi_pixel_format_to_bpp(dsi->format); > + if (bpp < 0) { > + DRM_DEV_ERROR(dsi->dev, > + "failed to get bpp for pixel format %d\n", > + dsi->format); > + return bpp; > + } > + > + mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC); > + if (mpclk) { > + /* take 1 / 0.8, since mbps must big than bandwidth of RGB */ > + tmp = mpclk * (bpp / lanes) * 10 / 8; > + if (tmp < max_mbps) > + target_mbps = tmp; > + else > + DRM_DEV_ERROR(dsi->dev, > + "DPHY clock frequency is out of range\n"); > + } > + > + fin = clk_get_rate(dsi->pllref_clk); > + fout = target_mbps * USEC_PER_SEC; > + > + /* constraint: 5Mhz <= Fref / N <= 40MHz */ > + min_prediv = DIV_ROUND_UP(fin, 40 * USEC_PER_SEC); > + max_prediv = fin / (5 * USEC_PER_SEC); > + > + /* constraint: 80MHz <= Fvco <= 1500Mhz */ > + fvco_min = 80 * USEC_PER_SEC; > + fvco_max = 1500 * USEC_PER_SEC; > + > + for (_prediv = min_prediv; _prediv <= max_prediv; _prediv++) { > + u64 tmp; > + u32 delta; > + /* Fvco = Fref * M / N */ > + tmp = (u64)fout * _prediv; > + do_div(tmp, fin); > + _fbdiv = tmp; > + /* > + * Due to the use of a "by 2 pre-scaler," the range of the > + * feedback multiplication value M is limited to even division > + * numbers, and m must be greater than 6, less than 512. > + */ > + if (_fbdiv < 6 || _fbdiv > 512) > + continue; > + > + _fbdiv += _fbdiv % 2; > + > + tmp = (u64)_fbdiv * fin; > + do_div(tmp, _prediv); > + if (tmp < fvco_min || tmp > fvco_max) > + continue; > + > + delta = abs(fout - tmp); > + if (delta < min_delta) { > + best_prediv = _prediv; > + best_fbdiv = _fbdiv; > + min_delta = delta; > + best_freq = tmp; > + } > + } > + if (best_freq) { > + dsi->lane_mbps = DIV_ROUND_UP(best_freq, USEC_PER_SEC); > + *lane_mbps = dsi->lane_mbps; > + dsi->input_div = best_prediv; > + dsi->feedback_div = best_fbdiv; > + } else > + DRM_DEV_ERROR(dsi->dev, "Can not find best_freq for DPHY\n"); Return an error? Also, use braces on the 'else' if you had to use them on the 'if'. > + > + return 0; > +} > + > +static const struct dw_mipi_dsi_phy_ops dw_mipi_dsi_rockchip_phy_ops = { > + .init = dw_mipi_dsi_phy_init, > + .get_lane_mbps = dw_mipi_dsi_get_lane_mbps, > +}; > + > +static struct rockchip_dw_dsi_chip_data rk3288_chip_data = { > + .dsi0_en_bit = RK3288_DSI0_SEL_VOP_LIT, > + .dsi1_en_bit = RK3288_DSI1_SEL_VOP_LIT, > + .grf_switch_reg = RK3288_GRF_SOC_CON6, > + .max_data_lanes = 4, > +}; > + > +static struct rockchip_dw_dsi_chip_data rk3399_chip_data = { > + .dsi0_en_bit = RK3399_DSI0_SEL_VOP_LIT, > + .dsi1_en_bit = RK3399_DSI1_SEL_VOP_LIT, > + .grf_switch_reg = RK3399_GRF_SOC_CON20, > + .grf_dsi0_mode = RK3399_GRF_DSI_MODE, > + .grf_dsi0_mode_reg = RK3399_GRF_SOC_CON22, > + .flags = DW_MIPI_NEEDS_PHY_CFG_CLK | DW_MIPI_NEEDS_GRF_CLK, > + .max_data_lanes = 4, > +}; > + > +static const struct of_device_id dw_mipi_dsi_rockchip_dt_ids[] = { > + { > + .compatible = "rockchip,rk3288-mipi-dsi", > + .data = &rk3288_chip_data, > + }, { > + .compatible = "rockchip,rk3399-mipi-dsi", > + .data = &rk3399_chip_data, > + }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, dw_mipi_dsi_rockchip_dt_ids); > + > +static void dw_mipi_dsi_encoder_nop(struct drm_encoder *encoder) > +{ > + /* do nothing */ > +} As of this: 75229eca569f drm: Make drm_encoder_helper_funcs optional these "nop" functions are actually optional. You can just completely remove the .enable/.disable callbacks in this driver. > + > +static void dw_mipi_dsi_encoder_mode_set(struct drm_encoder *encoder, > + struct drm_display_mode *mode, > + struct drm_display_mode *adjusted) > +{ > + struct dw_mipi_dsi_rockchip *dsi = to_dsi(encoder); > + const struct rockchip_dw_dsi_chip_data *cdata = dsi->cdata; > + int val, ret; > + > + /* > + * For the RK3399, the clk of grf must be enabled before writing grf > + * register. And for RK3288 or other soc, this grf_clk must be NULL, > + * the clk_prepare_enable return true directly. > + */ > + ret = clk_prepare_enable(dsi->grf_clk); > + if (ret) { > + DRM_DEV_ERROR(dsi->dev, "Failed to enable grf_clk: %d\n", ret); > + return; > + } > + > + ret = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, > + &dsi->encoder); > + if (ret < 0) > + return; On failure here, you need to disable the GRF clock. Or, you could move drm_of_encoder_active_endpoint_id() before clk_prepare_enable(dsi->grf_clk). > + > + val = cdata->dsi0_en_bit << 16; > + if (ret) > + val |= cdata->dsi0_en_bit; > + regmap_write(dsi->grf_regmap, cdata->grf_switch_reg, val); > + > + if (cdata->grf_dsi0_mode_reg) > + regmap_write(dsi->grf_regmap, cdata->grf_dsi0_mode_reg, > + cdata->grf_dsi0_mode); > + > + clk_disable_unprepare(dsi->grf_clk); > +} > + > +static int > +dw_mipi_dsi_encoder_atomic_check(struct drm_encoder *encoder, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state) > +{ > + struct rockchip_crtc_state *s = to_rockchip_crtc_state(crtc_state); > + struct dw_mipi_dsi_rockchip *dsi = to_dsi(encoder); > + > + switch (dsi->format) { > + case MIPI_DSI_FMT_RGB888: > + s->output_mode = ROCKCHIP_OUT_MODE_P888; > + break; > + case MIPI_DSI_FMT_RGB666: > + s->output_mode = ROCKCHIP_OUT_MODE_P666; > + break; > + case MIPI_DSI_FMT_RGB565: > + s->output_mode = ROCKCHIP_OUT_MODE_P565; > + break; > + default: > + WARN_ON(1); > + return -EINVAL; > + } > + > + s->output_type = DRM_MODE_CONNECTOR_DSI; > + > + return 0; > +} > + > +static const struct drm_encoder_helper_funcs > +dw_mipi_dsi_encoder_helper_funcs = { > + .enable = dw_mipi_dsi_encoder_nop, > + .disable = dw_mipi_dsi_encoder_nop, > + .mode_set = dw_mipi_dsi_encoder_mode_set, > + .atomic_check = dw_mipi_dsi_encoder_atomic_check, > +}; > + > +static const struct drm_encoder_funcs dw_mipi_dsi_encoder_funcs = { > + .destroy = drm_encoder_cleanup, > +}; > + > +static int rockchip_dsi_drm_create_encoder(struct dw_mipi_dsi_rockchip *dsi, > + struct drm_device *drm_dev) > +{ > + struct drm_encoder *encoder = &dsi->encoder; > + int ret; > + > + encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev, > + dsi->dev->of_node); > + > + ret = drm_encoder_init(drm_dev, encoder, &dw_mipi_dsi_encoder_funcs, > + DRM_MODE_ENCODER_DSI, NULL); > + if (ret) { > + DRM_ERROR("Failed to initialize encoder with drm\n"); > + return ret; > + } > + > + drm_encoder_helper_add(encoder, &dw_mipi_dsi_encoder_helper_funcs); > + > + return 0; > +} > + > +static int dw_mipi_dsi_rockchip_bind(struct device *dev, > + struct device *master, > + void *data) > +{ > + const struct of_device_id *of_id = > + of_match_device(dw_mipi_dsi_rockchip_dt_ids, dev); > + const struct rockchip_dw_dsi_chip_data *cdata = of_id->data; > + struct dw_mipi_dsi_rockchip *dsi = dev_get_drvdata(dev); Hmm, I see a "get_drvdata()" but no "set_drvdata()" in this driver. Also, the bridge driver is fighting you on this one. You'll need my patch [1] for this to make sense. [1] https://patchwork.kernel.org/patch/10078493/ > + struct resource *res; > + struct platform_device *pdev = to_platform_device(dev); > + struct drm_device *drm_dev = data; > + struct device_node *np = dev->of_node; > + int ret; > + > + dsi = devm_kzalloc(dev, sizeof(*dsi), GFP_KERNEL); > + if (!dsi) > + return -ENOMEM; > + > + dsi->cdata = cdata; > + dsi->dev = dev; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!res) { > + DRM_ERROR("Unable to get resource\n"); > + return -ENODEV; > + } > + > + dsi->base = devm_ioremap_resource(dev, res); > + if (IS_ERR(dsi->base)) { > + DRM_ERROR("Unable to get dsi registers\n"); > + return PTR_ERR(dsi->base); > + } > + > + dsi->pllref_clk = devm_clk_get(dev, "ref"); > + if (IS_ERR(dsi->pllref_clk)) { > + ret = PTR_ERR(dsi->pllref_clk); > + DRM_DEV_ERROR(dev, > + "Unable to get pll reference clock: %d\n", ret); > + return ret; > + } > + > + if (cdata->flags & DW_MIPI_NEEDS_PHY_CFG_CLK) { > + dsi->phy_cfg_clk = devm_clk_get(dev, "phy_cfg"); > + if (IS_ERR(dsi->phy_cfg_clk)) { > + ret = PTR_ERR(dsi->phy_cfg_clk); > + DRM_DEV_ERROR(dev, > + "Unable to get phy_cfg_clk: %d\n", ret); > + return ret; > + } > + } > + > + if (cdata->flags & DW_MIPI_NEEDS_GRF_CLK) { > + dsi->grf_clk = devm_clk_get(dev, "grf"); > + if (IS_ERR(dsi->grf_clk)) { > + ret = PTR_ERR(dsi->grf_clk); > + DRM_DEV_ERROR(dev, "Unable to get grf_clk: %d\n", ret); > + return ret; > + } > + } > + > + ret = clk_prepare_enable(dsi->pllref_clk); > + if (ret) { > + DRM_DEV_ERROR(dev, > + "%s: Failed to enable pllref_clk\n", __func__); > + return ret; > + } > + > + dsi->grf_regmap = syscon_regmap_lookup_by_phandle(np, "rockchip,grf"); > + if (IS_ERR(dsi->grf_regmap)) { > + DRM_DEV_ERROR(dsi->dev, "Unable to get rockchip,grf\n"); > + return PTR_ERR(dsi->grf_regmap); > + } > + > + dsi->pdata.base = dsi->base; > + dsi->pdata.priv_data = dsi; > + dsi->pdata.max_data_lanes = cdata->max_data_lanes; > + dsi->pdata.phy_ops = &dw_mipi_dsi_rockchip_phy_ops; > + > + ret = rockchip_dsi_drm_create_encoder(dsi, drm_dev); > + if (ret) { > + DRM_ERROR("Failed to create drm encoder\n"); > + return ret; > + } > + ret = dw_mipi_dsi_bind(pdev, &dsi->encoder, &dsi->pdata); > + if (ret) { > + DRM_ERROR("Failed to bind\n"); > + clk_disable_unprepare(dsi->pllref_clk); > + return ret; > + } > + return 0; > +} > + > +static void dw_mipi_dsi_rockchip_unbind(struct device *dev, > + struct device *master, > + void *data) > +{ Are you going to undo anything here? e.g., dw_mipi_dsi_unbind() and clk_disable_unprepare(dsi->pllref_clk)? Brian > +} > + > +static const struct component_ops dw_mipi_dsi_rockchip_ops = { > + .bind = dw_mipi_dsi_rockchip_bind, > + .unbind = dw_mipi_dsi_rockchip_unbind, > +}; > + > +static int dw_mipi_dsi_rockchip_probe(struct platform_device *pdev) > +{ > + return component_add(&pdev->dev, &dw_mipi_dsi_rockchip_ops); > +} > + > +static int dw_mipi_dsi_rockchip_remove(struct platform_device *pdev) > +{ > + component_del(&pdev->dev, &dw_mipi_dsi_rockchip_ops); > + > + return 0; > +} > + > +struct platform_driver dw_mipi_dsi_rockchip_driver = { > + .probe = dw_mipi_dsi_rockchip_probe, > + .remove = dw_mipi_dsi_rockchip_remove, > + .driver = { > + .of_match_table = dw_mipi_dsi_rockchip_dt_ids, > + .name = "dw-mipi-dsi-rockchip", > + }, > +}; > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > index 76d63de..d40cc39 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > @@ -462,7 +462,7 @@ static int __init rockchip_drm_init(void) > ADD_ROCKCHIP_SUB_DRIVER(cdn_dp_driver, CONFIG_ROCKCHIP_CDN_DP); > ADD_ROCKCHIP_SUB_DRIVER(dw_hdmi_rockchip_pltfm_driver, > CONFIG_ROCKCHIP_DW_HDMI); > - ADD_ROCKCHIP_SUB_DRIVER(dw_mipi_dsi_driver, > + ADD_ROCKCHIP_SUB_DRIVER(dw_mipi_dsi_rockchip_driver, > CONFIG_ROCKCHIP_DW_MIPI_DSI); > ADD_ROCKCHIP_SUB_DRIVER(inno_hdmi_driver, CONFIG_ROCKCHIP_INNO_HDMI); > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > index 498dfbc..af235b2 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.h > @@ -66,7 +66,7 @@ void rockchip_drm_dma_detach_device(struct drm_device *drm_dev, > > extern struct platform_driver cdn_dp_driver; > extern struct platform_driver dw_hdmi_rockchip_pltfm_driver; > -extern struct platform_driver dw_mipi_dsi_driver; > +extern struct platform_driver dw_mipi_dsi_rockchip_driver; > extern struct platform_driver inno_hdmi_driver; > extern struct platform_driver rockchip_dp_driver; > extern struct platform_driver rockchip_lvds_driver; > -- > 1.9.1 > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
