Re: [PATCH v5 07/12] phy: rockchip: samsung-hdptx: Avoid Hz<->hHz unit conversion overhead
From: Dmitry Baryshkov
Date: Sun Mar 09 2025 - 05:23:12 EST
On Sat, 8 Mar 2025 at 14:21, Cristian Ciocaltea
<cristian.ciocaltea@xxxxxxxxxxxxx> wrote:
>
> The ropll_tmds_cfg table used to identify the configuration params for
> the supported rates expects the search key, i.e. bit_rate member of
> struct ropll_config, to be provided in hHz rather than Hz (1 hHz = 100
> Hz). This requires multiple conversions between these units being
> performed at runtime.
>
> Improve implementation clarity and efficiency by consistently using the
> Hz unit throughout driver's internal data structures and functions.
> Also rename the rather misleading struct member.
>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@xxxxxxxxxxxxx>
> ---
> drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c | 79 +++++++++++------------
> 1 file changed, 39 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> index 2bf525514c1991a1299265d12e1e85f66333c604..e58a01bdb3ce82d66acdcb02c06de2816288b574 100644
> --- a/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> +++ b/drivers/phy/rockchip/phy-rockchip-samsung-hdptx.c
> @@ -330,7 +330,7 @@ enum dp_link_rate {
> };
>
> struct ropll_config {
> - u32 bit_rate;
> + u32 rate;
unsigned long long, please, to match the tmds_char_rate type.
> u8 pms_mdiv;
> u8 pms_mdiv_afc;
> u8 pms_pdiv;
> @@ -410,45 +410,45 @@ struct rk_hdptx_phy {
> };
>
> static const struct ropll_config ropll_tmds_cfg[] = {
> - { 5940000, 124, 124, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
> + { 594000000, 124, 124, 1, 1, 0, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
Use ULL suffix
> 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 3712500, 155, 155, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
> + { 371250000, 155, 155, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
> 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 2970000, 124, 124, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
> + { 297000000, 124, 124, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
> 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 1620000, 135, 135, 1, 1, 3, 1, 1, 0, 1, 1, 1, 1, 4, 0, 3, 5, 5, 0x10,
> + { 162000000, 135, 135, 1, 1, 3, 1, 1, 0, 1, 1, 1, 1, 4, 0, 3, 5, 5, 0x10,
> 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 1856250, 155, 155, 1, 1, 3, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
> + { 185625000, 155, 155, 1, 1, 3, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
> 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 1540000, 193, 193, 1, 1, 5, 1, 1, 1, 1, 1, 1, 1, 193, 1, 32, 2, 1,
> + { 154000000, 193, 193, 1, 1, 5, 1, 1, 1, 1, 1, 1, 1, 193, 1, 32, 2, 1,
> 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 1485000, 0x7b, 0x7b, 1, 1, 3, 1, 1, 1, 1, 1, 1, 1, 4, 0, 3, 5, 5,
> + { 148500000, 0x7b, 0x7b, 1, 1, 3, 1, 1, 1, 1, 1, 1, 1, 4, 0, 3, 5, 5,
> 0x10, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 1462500, 122, 122, 1, 1, 3, 1, 1, 1, 1, 1, 1, 1, 244, 1, 16, 2, 1, 1,
> + { 146250000, 122, 122, 1, 1, 3, 1, 1, 1, 1, 1, 1, 1, 244, 1, 16, 2, 1, 1,
> 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 1190000, 149, 149, 1, 1, 5, 1, 1, 1, 1, 1, 1, 1, 149, 1, 16, 2, 1, 1,
> + { 119000000, 149, 149, 1, 1, 5, 1, 1, 1, 1, 1, 1, 1, 149, 1, 16, 2, 1, 1,
> 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 1065000, 89, 89, 1, 1, 3, 1, 1, 1, 1, 1, 1, 1, 89, 1, 16, 1, 0, 1,
> + { 106500000, 89, 89, 1, 1, 3, 1, 1, 1, 1, 1, 1, 1, 89, 1, 16, 1, 0, 1,
> 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 1080000, 135, 135, 1, 1, 5, 1, 1, 0, 1, 0, 1, 1, 0x9, 0, 0x05, 0,
> + { 108000000, 135, 135, 1, 1, 5, 1, 1, 0, 1, 0, 1, 1, 0x9, 0, 0x05, 0,
> 0x14, 0x18, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 855000, 214, 214, 1, 1, 11, 1, 1, 1, 1, 1, 1, 1, 214, 1, 16, 2, 1,
> + { 85500000, 214, 214, 1, 1, 11, 1, 1, 1, 1, 1, 1, 1, 214, 1, 16, 2, 1,
> 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 835000, 105, 105, 1, 1, 5, 1, 1, 1, 1, 1, 1, 1, 42, 1, 16, 1, 0,
> + { 83500000, 105, 105, 1, 1, 5, 1, 1, 1, 1, 1, 1, 1, 42, 1, 16, 1, 0,
> 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 928125, 155, 155, 1, 1, 7, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
> + { 92812500, 155, 155, 1, 1, 7, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
> 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 742500, 124, 124, 1, 1, 7, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
> + { 74250000, 124, 124, 1, 1, 7, 1, 1, 1, 1, 1, 1, 1, 62, 1, 16, 5, 0,
> 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 650000, 162, 162, 1, 1, 11, 1, 1, 1, 1, 1, 1, 1, 54, 0, 16, 4, 1,
> + { 65000000, 162, 162, 1, 1, 11, 1, 1, 1, 1, 1, 1, 1, 54, 0, 16, 4, 1,
> 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 337500, 0x70, 0x70, 1, 1, 0xf, 1, 1, 1, 1, 1, 1, 1, 0x2, 0, 0x01, 5,
> + { 33750000, 0x70, 0x70, 1, 1, 0xf, 1, 1, 1, 1, 1, 1, 1, 0x2, 0, 0x01, 5,
> 1, 1, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 400000, 100, 100, 1, 1, 11, 1, 1, 0, 1, 0, 1, 1, 0x9, 0, 0x05, 0,
> + { 40000000, 100, 100, 1, 1, 11, 1, 1, 0, 1, 0, 1, 1, 0x9, 0, 0x05, 0,
> 0x14, 0x18, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 270000, 0x5a, 0x5a, 1, 1, 0xf, 1, 1, 0, 1, 0, 1, 1, 0x9, 0, 0x05, 0,
> + { 27000000, 0x5a, 0x5a, 1, 1, 0xf, 1, 1, 0, 1, 0, 1, 1, 0x9, 0, 0x05, 0,
> 0x14, 0x18, 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> - { 251750, 84, 84, 1, 1, 0xf, 1, 1, 1, 1, 1, 1, 1, 168, 1, 16, 4, 1, 1,
> + { 25175000, 84, 84, 1, 1, 0xf, 1, 1, 1, 1, 1, 1, 1, 168, 1, 16, 4, 1, 1,
> 1, 0, 0x20, 0x0c, 1, 0x0e, 0, 0, },
> };
>
> @@ -894,10 +894,10 @@ static void rk_hdptx_phy_disable(struct rk_hdptx_phy *hdptx)
> regmap_write(hdptx->grf, GRF_HDPTX_CON0, val);
> }
>
> -static bool rk_hdptx_phy_clk_pll_calc(unsigned int data_rate,
> +static bool rk_hdptx_phy_clk_pll_calc(unsigned long rate,
here and further, unsigned long long
Also, is it tmds_char_rate?
> struct ropll_config *cfg)
> {
> - const unsigned int fout = data_rate / 2, fref = 24000;
> + const unsigned int fout = rate / 200, fref = 24000;
> unsigned long k = 0, lc, k_sub, lc_sub;
> unsigned int fvco, sdc;
> u32 mdiv, sdiv, n = 8;
--
With best wishes
Dmitry