| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <36bbfad9-7169-3dee-a0e1-d722535d6ce8@rock-chips.com>
Date: Tue, 28 Nov 2017 11:32:51 +0800
From: Nickey Yang <nickey.yang@...k-chips.com>
To: Brian Norris <briannorris@...omium.org>
Cc: robh+dt@...nel.org, heiko@...ech.de, mark.rutland@....com,
airlied@...ux.ie, linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org,
linux-rockchip@...ts.infradead.org, seanpaul@...omium.org,
hoegsberg@...il.com, architt@...eaurora.org, philippe.cornu@...com,
yannick.fertre@...com, hl@...k-chips.com, zyw@...k-chips.com,
xbl@...k-chips.com
Subject: Re: [PATCH 2/3] drm/rockchip: Add ROCKCHIP DW MIPI DSI controller
driver
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@...k-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@...k-chips.com>
>> + * Nickey Yang <nickey.yang@...k-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
>>
>
>
Powered by blists - more mailing lists