[Freedreno] [PATCH v3 06/15] phy: qualcomm: add MSM8974 HDMI PHY support

[Konrad Dybcio]

On 28.09.2023 13:16, Dmitry Baryshkov wrote:
> Add support for HDMI PHY on Qualcomm MSM8974 / APQ8074 platforms.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> ---
I only have a few style comments (and timers-howto.txt fixes)

[...]

> +#define HDMI_8974_VCO_MAX_FREQ 1800000000UL
> +#define HDMI_8974_VCO_MIN_FREQ  600000000UL
> +
> +#define HDMI_8974_COMMON_DIV 5
> +
> +static void qcom_uniphy_setup(void __iomem *base, unsigned int ref_freq,
> +			      bool sdm_mode,
> +			      bool ref_freq_mult_2,
> +			      bool dither,
> +			      unsigned int refclk_div,
> +			      unsigned int vco_freq)
> +{
> +	unsigned int int_ref_freq = ref_freq * (ref_freq_mult_2 ? 2 : 1);
> +	unsigned int div_in_freq = vco_freq / refclk_div;
> +	unsigned int dc_offset = div_in_freq / int_ref_freq - 1;
> +	unsigned int sdm_freq_seed;
> +	unsigned int val;
> +	unsigned int remain = div_in_freq - (dc_offset + 1) * int_ref_freq;
> +	sdm_freq_seed = mult_frac(remain, 0x10000, int_ref_freq);
> +
> +	val = (ref_freq_mult_2 ? BIT(0) : 0) |
> +		((refclk_div - 1) << 2);
> +	writel(val, base + UNIPHY_PLL_REFCLK_CFG);
> +
> +	writel(sdm_mode ? 0 : 0x40 + dc_offset, base + UNIPHY_PLL_SDM_CFG0);
> +
> +	writel(dither ? 0x40 + dc_offset: 0, base + UNIPHY_PLL_SDM_CFG1);
> +
> +	writel(sdm_freq_seed & 0xff, base + UNIPHY_PLL_SDM_CFG2);
The ternary operator doesn't really improve readability here, imo

> +
> +	writel((sdm_freq_seed >> 8) & 0xff, base + UNIPHY_PLL_SDM_CFG3);
> +
> +	writel(sdm_freq_seed >> 16, base + UNIPHY_PLL_SDM_CFG4);
> +
> +	ref_freq = ref_freq * 5 / 1000;
> +	writel(ref_freq & 0xff, base + UNIPHY_PLL_CAL_CFG8);
> +
> +	writel(ref_freq >> 8, base + UNIPHY_PLL_CAL_CFG9);
> +
> +	vco_freq /= 1000;
> +	writel(vco_freq & 0xff, base + UNIPHY_PLL_CAL_CFG10);
> +
> +	writel(vco_freq >> 8, base + UNIPHY_PLL_CAL_CFG11);
> +}
> +
> +static unsigned long qcom_uniphy_recalc(void __iomem *base, unsigned long parent_rate)
> +{
> +	unsigned long rate;
> +	u32 refclk_cfg;
> +	u32 dc_offset;
> +	u64 fraq_n;
> +	u32 val;
> +
> +	refclk_cfg = readl(base + UNIPHY_PLL_REFCLK_CFG);
> +	if (refclk_cfg & BIT(0))
Can we name this bit?

> +		parent_rate *= 2;
> +
> +	val = readl(base + UNIPHY_PLL_SDM_CFG0);
> +	if (val & 0x40) {
BIT(6)? can we name it?

> +		dc_offset = val & 0x3f;
GENMASK?

> +		fraq_n = 0;
> +	} else {
> +		dc_offset = readl(base + UNIPHY_PLL_SDM_CFG1) & 0x3f;
GENMASK?
> +		fraq_n = readl(base + UNIPHY_PLL_SDM_CFG2) |
> +			(readl(base + UNIPHY_PLL_SDM_CFG3) << 8);
FIELD_PREP?

> +	}
> +
> +	rate = (dc_offset + 1) * parent_rate;
> +	rate += mult_frac(fraq_n, parent_rate, 0x10000);
> +
> +	rate *= (refclk_cfg >> 2) * 0x3 + 1;
3 would be more clear than 0x3 imo

[...]
> +	udelay(50);
usleep_range

[...]

> +	udelay(200);
usleep_range

> +
> +	return 0;
> +}
> +
> +static int qcom_hdmi_msm8974_phy_pll_enable(struct qcom_hdmi_preqmp_phy *hdmi_phy)
> +{
> +	int ret;
> +	unsigned long status;
> +
> +	/* Global enable */
> +	hdmi_phy_write(hdmi_phy, REG_HDMI_8x74_GLB_CFG, 0x81);
> +
> +	/* Power up power gen */
> +	hdmi_phy_write(hdmi_phy, REG_HDMI_8x74_PD_CTRL0, 0x00);
> +	udelay(350);
usleep_range

> +
> +	/* PLL power up */
> +	hdmi_pll_write(hdmi_phy, UNIPHY_PLL_GLB_CFG, 0x01);
> +	udelay(5);
> +
> +	/* Power up PLL LDO */
> +	hdmi_pll_write(hdmi_phy, UNIPHY_PLL_GLB_CFG, 0x03);
> +	udelay(350);
usleep_range

> +
> +	/* PLL power up */
> +	hdmi_pll_write(hdmi_phy, UNIPHY_PLL_GLB_CFG, 0x0f);
> +	udelay(350);
usleep_range

> +
> +	/* Poll for PLL ready status */
> +	ret = readl_poll_timeout(hdmi_phy->pll_reg + UNIPHY_PLL_STATUS,
> +				 status, status & BIT(0),
magic bit name?

> +				 100, 2000);
> +	if (ret) {
> +		dev_warn(hdmi_phy->dev, "HDMI PLL not ready\n");
> +		goto err;
> +	}
> +
> +	udelay(350);
usleep_range

Konrad