Re: [PATCH 1/5] drm/msm/dsi: add support for DSI-PHY on SM8350 and SM8450

[Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx>]

On 9/22/2022 4:30 AM, Dmitry Baryshkov wrote:
> SM8350 and SM8450 use 5nm DSI PHYs, which share register definitions
> with 7nm DSI PHYs. Rather than duplicating the driver, handle 5nm
> variants inside the common 5+7nm driver.

I do realize that there is common code across PHYs but i am concerned 
about this type of unification of phy drivers.

If we have features which are PHY sequence dependent such as ULPS, this 
will just complicate things for us.

Also some PHY registers might get added some might get removed across 
chipsets as this is the most frequently changed component.

Even in this patch, I see this added to dsi_7nm_phy_disable()

> +	/* Turn off REFGEN Vote */
> +        dsi_phy_write(base + 
REG_DSI_7nm_PHY_CMN_GLBL_DIGTOP_SPARE10, 0x0);
> +        wmb();
> +        /* Delay to ensure HW removes vote before PHY shut down */
> +        udelay(2);
> +

What would be the impact of writing this for the existing 7nm PHY?

I am having some access issues to check the software interface so wanted 
to check.

> Co-developed-by: Robert Foss <robert.foss@xxxxxxxxxx>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> ---
>   drivers/gpu/drm/msm/Kconfig               |   6 +-
>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.c     |   4 +
>   drivers/gpu/drm/msm/dsi/phy/dsi_phy.h     |   2 +
>   drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 132 ++++++++++++++++++++--
>   4 files changed, 131 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
> index 4e0cbd682725..e6c5dfbad009 100644
> --- a/drivers/gpu/drm/msm/Kconfig
> +++ b/drivers/gpu/drm/msm/Kconfig
> @@ -140,12 +140,12 @@ config DRM_MSM_DSI_10NM_PHY
>   	  Choose this option if DSI PHY on SDM845 is used on the platform.
>   
>   config DRM_MSM_DSI_7NM_PHY
> -	bool "Enable DSI 7nm PHY driver in MSM DRM"
> +	bool "Enable DSI 7nm/5nm PHY driver in MSM DRM"
>   	depends on DRM_MSM_DSI
>   	default y
>   	help
> -	  Choose this option if DSI PHY on SM8150/SM8250/SC7280 is used on
> -	  the platform.
> +	  Choose this option if DSI PHY on SM8150/SM8250/SM8350/SM8450/SC7280
> +	  is used on the platform.
>   
>   config DRM_MSM_HDMI
>   	bool "Enable HDMI support in MSM DRM driver"
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> index 7fc0975cb869..97cf6b8b34cc 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
> @@ -567,6 +567,10 @@ static const struct of_device_id dsi_phy_dt_match[] = {
>   	  .data = &dsi_phy_7nm_8150_cfgs },
>   	{ .compatible = "qcom,sc7280-dsi-phy-7nm",
>   	  .data = &dsi_phy_7nm_7280_cfgs },
> +	{ .compatible = "qcom,dsi-phy-5nm-8350",
> +	  .data = &dsi_phy_5nm_8350_cfgs },
> +	{ .compatible = "qcom,dsi-phy-5nm-8450",
> +	  .data = &dsi_phy_5nm_8450_cfgs },
>   #endif
>   	{}
>   };
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> index 60a99c6525b2..654cbfa14d6e 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
> @@ -56,6 +56,8 @@ extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs;
>   extern const struct msm_dsi_phy_cfg dsi_phy_7nm_cfgs;
>   extern const struct msm_dsi_phy_cfg dsi_phy_7nm_8150_cfgs;
>   extern const struct msm_dsi_phy_cfg dsi_phy_7nm_7280_cfgs;
> +extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8350_cfgs;
> +extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8450_cfgs;
>   
>   struct msm_dsi_dphy_timing {
>   	u32 clk_zero;
> diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> index 9e7fa7d88ead..1696ff150b9e 100644
> --- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> +++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
> @@ -39,8 +39,14 @@
>   #define VCO_REF_CLK_RATE		19200000
>   #define FRAC_BITS 18
>   
> +/* Hardware is pre V4.1 */
> +#define DSI_PHY_7NM_QUIRK_PRE_V4_1	BIT(0)
>   /* Hardware is V4.1 */
> -#define DSI_PHY_7NM_QUIRK_V4_1		BIT(0)
> +#define DSI_PHY_7NM_QUIRK_V4_1		BIT(1)
> +/* Hardware is V4.2 */
> +#define DSI_PHY_7NM_QUIRK_V4_2		BIT(2)
> +/* Hardware is V4.3 */
> +#define DSI_PHY_7NM_QUIRK_V4_3		BIT(3)
>   
>   struct dsi_pll_config {
>   	bool enable_ssc;
> @@ -116,7 +122,7 @@ static void dsi_pll_calc_dec_frac(struct dsi_pll_7nm *pll, struct dsi_pll_config
>   	dec_multiple = div_u64(pll_freq * multiplier, divider);
>   	dec = div_u64_rem(dec_multiple, multiplier, &frac);
>   
> -	if (!(pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1))
> +	if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_PRE_V4_1)
>   		config->pll_clock_inverters = 0x28;
>   	else if (pll_freq <= 1000000000ULL)
>   		config->pll_clock_inverters = 0xa0;
> @@ -197,16 +203,25 @@ static void dsi_pll_config_hzindep_reg(struct dsi_pll_7nm *pll)
>   	void __iomem *base = pll->phy->pll_base;
>   	u8 analog_controls_five_1 = 0x01, vco_config_1 = 0x00;
>   
> -	if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1) {
> +	if (!(pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_PRE_V4_1))
>   		if (pll->vco_current_rate >= 3100000000ULL)
>   			analog_controls_five_1 = 0x03;
>   
> +	if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1) {
>   		if (pll->vco_current_rate < 1520000000ULL)
>   			vco_config_1 = 0x08;
>   		else if (pll->vco_current_rate < 2990000000ULL)
>   			vco_config_1 = 0x01;
>   	}
>   
> +	if ((pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_2) ||
> +	    (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3)) {
> +		if (pll->vco_current_rate < 1520000000ULL)
> +			vco_config_1 = 0x08;
> +		else if (pll->vco_current_rate >= 2990000000ULL)
> +			vco_config_1 = 0x01;
> +	}
> +
>   	dsi_phy_write(base + REG_DSI_7nm_PHY_PLL_ANALOG_CONTROLS_FIVE_1,
>   		      analog_controls_five_1);
>   	dsi_phy_write(base + REG_DSI_7nm_PHY_PLL_VCO_CONFIG_1, vco_config_1);
> @@ -231,9 +246,9 @@ static void dsi_pll_config_hzindep_reg(struct dsi_pll_7nm *pll)
>   	dsi_phy_write(base + REG_DSI_7nm_PHY_PLL_PFILT, 0x2f);
>   	dsi_phy_write(base + REG_DSI_7nm_PHY_PLL_IFILT, 0x2a);
>   	dsi_phy_write(base + REG_DSI_7nm_PHY_PLL_IFILT,
> -		  pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1 ? 0x3f : 0x22);
> +		  !(pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_PRE_V4_1) ? 0x3f : 0x22);
>   
> -	if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1) {
> +	if (!(pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_PRE_V4_1)) {
>   		dsi_phy_write(base + REG_DSI_7nm_PHY_PLL_PERF_OPTIMIZE, 0x22);
>   		if (pll->slave)
>   			dsi_phy_write(pll->slave->phy->pll_base + REG_DSI_7nm_PHY_PLL_PERF_OPTIMIZE, 0x22);
> @@ -788,7 +803,7 @@ static void dsi_phy_hw_v4_0_lane_settings(struct msm_dsi_phy *phy)
>   	const u8 *tx_dctrl = tx_dctrl_0;
>   	void __iomem *lane_base = phy->lane_base;
>   
> -	if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1)
> +	if (!(phy->cfg->quirks & DSI_PHY_7NM_QUIRK_PRE_V4_1))
>   		tx_dctrl = tx_dctrl_1;
>   
>   	/* Strength ctrl settings */
> @@ -844,6 +859,12 @@ static int dsi_7nm_phy_enable(struct msm_dsi_phy *phy,
>   	if (dsi_phy_hw_v4_0_is_pll_on(phy))
>   		pr_warn("PLL turned on before configuring PHY\n");
>   
> +	/* Request for REFGEN READY */
> +	if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) {
> +		dsi_phy_write(phy->base + REG_DSI_7nm_PHY_CMN_GLBL_DIGTOP_SPARE10, 0x1);
> +		udelay(500);
> +	}
> +
>   	/* wait for REFGEN READY */
>   	ret = readl_poll_timeout_atomic(base + REG_DSI_7nm_PHY_CMN_PHY_STATUS,
>   					status, (status & BIT(0)),
> @@ -858,23 +879,53 @@ static int dsi_7nm_phy_enable(struct msm_dsi_phy *phy,
>   	/* Alter PHY configurations if data rate less than 1.5GHZ*/
>   	less_than_1500_mhz = (clk_req->bitclk_rate <= 1500000000);
>   
> -	if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1) {
> -		vreg_ctrl_0 = less_than_1500_mhz ? 0x53 : 0x52;
> +	if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3) {
> +		if (phy->cphy_mode) {
> +			vreg_ctrl_0 = 0x51;
> +			glbl_rescode_top_ctrl = less_than_1500_mhz ? 0x3d :  0x01;
> +			glbl_rescode_bot_ctrl = less_than_1500_mhz ? 0x38 :  0x3b;
> +			glbl_str_swi_cal_sel_ctrl = 0x00;
> +			glbl_hstx_str_ctrl_0 = 0x00;
> +		} else {
> +			vreg_ctrl_0 = less_than_1500_mhz ? 0x53 : 0x52;
> +			glbl_rescode_top_ctrl = less_than_1500_mhz ? 0x3d :  0x01;
> +			glbl_rescode_bot_ctrl = less_than_1500_mhz ? 0x38 :  0x39;
> +			glbl_str_swi_cal_sel_ctrl = 0x00;
> +			glbl_hstx_str_ctrl_0 = 0x88;
> +		}
> +	} else if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_2) {
> +		if (phy->cphy_mode) {
> +			vreg_ctrl_0 = 0x51;
> +			glbl_rescode_top_ctrl = less_than_1500_mhz ? 0x3d :  0x01;
> +			glbl_rescode_bot_ctrl = less_than_1500_mhz ? 0x38 :  0x3b;
> +			glbl_str_swi_cal_sel_ctrl = 0x00;
> +			glbl_hstx_str_ctrl_0 = 0x00;
> +		} else {
> +			vreg_ctrl_0 = less_than_1500_mhz ? 0x53 : 0x52;
> +			glbl_rescode_top_ctrl = less_than_1500_mhz ? 0x3c :  0x00;
> +			glbl_rescode_bot_ctrl = less_than_1500_mhz ? 0x38 :  0x39;
> +			glbl_str_swi_cal_sel_ctrl = 0x00;
> +			glbl_hstx_str_ctrl_0 = 0x88;
> +		}
> +	} else if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1) {
>   		if (phy->cphy_mode) {
> +			vreg_ctrl_0 = 0x51;
>   			glbl_rescode_top_ctrl = 0x00;
>   			glbl_rescode_bot_ctrl = 0x3c;
>   		} else {
> +			vreg_ctrl_0 = less_than_1500_mhz ? 0x53 : 0x52;
>   			glbl_rescode_top_ctrl = less_than_1500_mhz ? 0x3d :  0x00;
>   			glbl_rescode_bot_ctrl = less_than_1500_mhz ? 0x39 :  0x3c;
>   		}
>   		glbl_str_swi_cal_sel_ctrl = 0x00;
>   		glbl_hstx_str_ctrl_0 = 0x88;
>   	} else {
> -		vreg_ctrl_0 = less_than_1500_mhz ? 0x5B : 0x59;
>   		if (phy->cphy_mode) {
> +			vreg_ctrl_0 = 0x51;
>   			glbl_str_swi_cal_sel_ctrl = 0x03;
>   			glbl_hstx_str_ctrl_0 = 0x66;
>   		} else {
> +			vreg_ctrl_0 = less_than_1500_mhz ? 0x5B : 0x59;
>   			glbl_str_swi_cal_sel_ctrl = less_than_1500_mhz ? 0x03 : 0x00;
>   			glbl_hstx_str_ctrl_0 = less_than_1500_mhz ? 0x66 : 0x88;
>   		}
> @@ -883,7 +934,6 @@ static int dsi_7nm_phy_enable(struct msm_dsi_phy *phy,
>   	}
>   
>   	if (phy->cphy_mode) {
> -		vreg_ctrl_0 = 0x51;
>   		vreg_ctrl_1 = 0x55;
>   		glbl_pemph_ctrl_0 = 0x11;
>   		lane_ctrl0 = 0x17;
> @@ -1017,6 +1067,13 @@ static void dsi_7nm_phy_disable(struct msm_dsi_phy *phy)
>   		pr_warn("Turning OFF PHY while PLL is on\n");
>   
>   	dsi_phy_hw_v4_0_config_lpcdrx(phy, false);
> +
> +	/* Turn off REFGEN Vote */
> +        dsi_phy_write(base + REG_DSI_7nm_PHY_CMN_GLBL_DIGTOP_SPARE10, 0x0);
> +        wmb();
> +        /* Delay to ensure HW removes vote before PHY shut down */
> +        udelay(2);
> +
>   	data = dsi_phy_read(base + REG_DSI_7nm_PHY_CMN_CTRL_0);
>   
>   	/* disable all lanes */
> @@ -1079,6 +1136,7 @@ const struct msm_dsi_phy_cfg dsi_phy_7nm_8150_cfgs = {
>   	.max_pll_rate = 3500000000UL,
>   	.io_start = { 0xae94400, 0xae96400 },
>   	.num_dsi_phy = 2,
> +	.quirks = DSI_PHY_7NM_QUIRK_PRE_V4_1,
>   };
>   
>   const struct msm_dsi_phy_cfg dsi_phy_7nm_7280_cfgs = {
> @@ -1102,3 +1160,57 @@ const struct msm_dsi_phy_cfg dsi_phy_7nm_7280_cfgs = {
>   	.num_dsi_phy = 1,
>   	.quirks = DSI_PHY_7NM_QUIRK_V4_1,
>   };
> +
> +const struct msm_dsi_phy_cfg dsi_phy_5nm_8350_cfgs = {
> +	.has_phy_lane = true,
> +	.reg_cfg = {
> +		.num = 1,
> +		.regs = {
> +			{"vdds", 37550, 0},
> +		},
> +	},
> +	.ops = {
> +		.enable = dsi_7nm_phy_enable,
> +		.disable = dsi_7nm_phy_disable,
> +		.pll_init = dsi_pll_7nm_init,
> +		.save_pll_state = dsi_7nm_pll_save_state,
> +		.restore_pll_state = dsi_7nm_pll_restore_state,
> +		.set_continuous_clock = dsi_7nm_set_continuous_clock,
> +	},
> +	.min_pll_rate = 600000000UL,
> +#ifdef CONFIG_64BIT
> +	.max_pll_rate = 5000000000UL,
> +#else
> +	.max_pll_rate = ULONG_MAX,
> +#endif
> +	.io_start = { 0xae94400, 0xae96400 },
> +	.num_dsi_phy = 2,
> +	.quirks = DSI_PHY_7NM_QUIRK_V4_2,
> +};
> +
> +const struct msm_dsi_phy_cfg dsi_phy_5nm_8450_cfgs = {
> +	.has_phy_lane = true,
> +	.reg_cfg = {
> +		.num = 1,
> +		.regs = {
> +			{"vdds", 97800, 0},
> +		},
> +	},
> +	.ops = {
> +		.enable = dsi_7nm_phy_enable,
> +		.disable = dsi_7nm_phy_disable,
> +		.pll_init = dsi_pll_7nm_init,
> +		.save_pll_state = dsi_7nm_pll_save_state,
> +		.restore_pll_state = dsi_7nm_pll_restore_state,
> +		.set_continuous_clock = dsi_7nm_set_continuous_clock,
> +	},
> +	.min_pll_rate = 600000000UL,
> +#ifdef CONFIG_64BIT
> +	.max_pll_rate = 5000000000UL,
> +#else
> +	.max_pll_rate = ULONG_MAX,
> +#endif
> +	.io_start = { 0xae94400, 0xae96400 },
> +	.num_dsi_phy = 2,
> +	.quirks = DSI_PHY_7NM_QUIRK_V4_3,
> +};