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.y...@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 <z...@rock-chips.com>
+ *      Nickey Yang <nickey.y...@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





Reply via email to