Hi Brian, Below comments fixed in patch-v2?https://patchwork.kernel.org/patch/10078527/ but ?"get_drvdata()" Thanks for review. Nickey. On 2017?11?28? 09:51, Brian Norris wrote: > 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 at rock-chips.com> >> --- >> 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 at rock-chips.com> >> + * Nickey Yang <nickey.yang at rock-chips.com> >> + * >> + * 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 >> > >
