Re: [PATCH v5 4/4] phy: qualcomm: add MSM8974 HDMI PHY support

[Dmitry Baryshkov]

On Wed, Mar 18, 2026 at 10:22:08AM +0100, Konrad Dybcio wrote:
> On 3/14/26 6:06 AM, Dmitry Baryshkov wrote:
> > From: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > 
> > Add support for HDMI PHY on Qualcomm MSM8974 / APQ8074 platforms.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> > ---
> 
> [...]
> 
> > +	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);
> 
> Some beautification (BIT(), FIELD_..(), defined magic values) would be
> really nice to see.. although I'm not sure how much you can do with the
> PLL registers..

I can try doing a bit of it, but not that much. The HDMI PHY is mostly
unspecified for that platform. The code here is mostly based on the
study of the existing values in downstream and corresponding DSI PHY
(which uses the same UNIPHY core).

> 
> [...]
> 
> > +	ref_freq = ref_freq * 5 / 1000;
> 
> mult_frac()

ack

> 
> [...]
> 
> > +	rate = (dc_offset + 1) * parent_rate;
> > +	rate += mult_frac(fraq_n, parent_rate, 0x10000);
> > +
> > +	rate *= (refclk_cfg >> 2) * 0x3 + 1;
> 
> Really strange calculation, but in the end this is (n * 0.75)+1 -
> mult_frac()?

It might be based on some other dividers which I didn't recognize.
Anyway, yes, mult_frac() would work.

> 
> > +
> > +	return rate;
> > +}
> > +
> > +static const unsigned int qcom_hdmi_8974_divs[] = {1, 2, 4, 6};
> > +
> > +static unsigned long qcom_hdmi_8974_pll_recalc_rate(struct clk_hw *hw,
> > +						    unsigned long parent_rate)
> > +{
> > +	struct qcom_hdmi_preqmp_phy *hdmi_phy = hw_clk_to_phy(hw);
> > +	u32 div_idx = hdmi_pll_read(hdmi_phy, UNIPHY_PLL_POSTDIV1_CFG);
> > +	unsigned long rate = qcom_uniphy_recalc(hdmi_phy->pll_reg, parent_rate);
> > +
> > +	return  rate / HDMI_8974_COMMON_DIV / qcom_hdmi_8974_divs[div_idx & 0x3];
> 
> nit: double space
> 
> 
> > +}
> > +
> > +static int qcom_hdmi_8974_pll_determine_rate(struct clk_hw *hw,
> > +					     struct clk_rate_request *req)
> > +{
> > +	req->rate = clamp(req->rate,
> > +			  HDMI_8974_VCO_MIN_FREQ / HDMI_8974_COMMON_DIV / 6,
> > +			  HDMI_8974_VCO_MAX_FREQ / HDMI_8974_COMMON_DIV / 1);
> 
> I don't know if it's a good direction, but maybe:
> 
> const unsigned long max_rate = HDMI_8974_VCO_MAX_FREQ / HDMI_8974_COMMON_DIV;
> 
> clamp(req->rate, max_rate / 6, max_rate)

Note, it is clamp (min_rate / 6, max_rate)

> 
> ?
> 
> [...]
> 
> > +static int qcom_hdmi_msm8974_phy_find_div(unsigned long long pixclk)
> > +{
> > +	int i;
> > +	unsigned long long min_freq = HDMI_8974_VCO_MIN_FREQ / HDMI_8974_COMMON_DIV;
> 
> reverse-Christmas-tree?
> 
> > +
> > +	if (pixclk > HDMI_8974_VCO_MAX_FREQ / HDMI_8974_COMMON_DIV)
> > +		return -E2BIG;
> 
> include/uapi/asm-generic/errno-base.h
> 11:#define      E2BIG            7      /* Argument list too long */
> 
> -EINVAL?

Ok

> 
> Konrad

-- 
With best wishes
Dmitry