From: David Laight <david.laight.linux@gmail.com>
To: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
Vinod Koul <vkoul@kernel.org>,
Kishon Vijay Abraham I <kishon@kernel.org>,
Heiko Stuebner <heiko@sntech.de>,
Algea Cao <algea.cao@rock-chips.com>,
Sandor Yu <Sandor.yu@nxp.com>, Maxime Ripard <mripard@kernel.org>,
kernel@collabora.com, linux-kernel@vger.kernel.org,
linux-phy@lists.infradead.org,
linux-arm-kernel@lists.infradead.org,
linux-rockchip@lists.infradead.org
Subject: Re: [PATCH v5 07/12] phy: rockchip: samsung-hdptx: Avoid Hz<->hHz unit conversion overhead
Date: Sun, 9 Mar 2025 14:47:47 +0000 [thread overview]
Message-ID: <20250309144747.744e5197@pumpkin> (raw)
In-Reply-To: <a1a3e86a-1906-4797-932d-8e1aadafedde@collabora.com>
On Sun, 9 Mar 2025 12:13:32 +0200
Cristian Ciocaltea <cristian.ciocaltea@collabora.com> wrote:
> On 3/9/25 11:22 AM, Dmitry Baryshkov wrote:
> > On Sat, 8 Mar 2025 at 14:21, Cristian Ciocaltea
> > <cristian.ciocaltea@collabora.com> 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@collabora.com>
> >> ---
> >> 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.
Isn't 'bit_rate' more descriptive?
But maybe rate_hz to make the units more obvious.
If the max frequency might get near 4Gz then the you need something
bigger that u32 - which might it used hectaHz (a prefix that is pretty much only
used with areas of land!)
Being more explicit with u64 (rather than 'long long') may be better.
Certainly less typing.
David
> >
> >> 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?
>
> Yes, will do the suggested changes in v6.
>
> Thanks,
> Cristian
>
next prev parent reply other threads:[~2025-03-09 14:49 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-08 12:21 [PATCH v5 00/12] phy: rockchip: samsung-hdptx: Support high color depth management Cristian Ciocaltea
2025-03-08 12:21 ` [PATCH v5 01/12] phy: Add HDMI configuration options Cristian Ciocaltea
2025-03-08 12:21 ` [PATCH v5 02/12] phy: hdmi: Add color depth configuration Cristian Ciocaltea
2025-03-09 9:16 ` Dmitry Baryshkov
2025-03-09 10:10 ` Cristian Ciocaltea
2025-03-09 10:10 ` Dmitry Baryshkov
2025-03-08 12:21 ` [PATCH v5 03/12] phy: rockchip: samsung-hdptx: Fix clock ratio setup Cristian Ciocaltea
2025-03-09 9:17 ` Dmitry Baryshkov
2025-03-09 10:11 ` Cristian Ciocaltea
2025-03-09 10:12 ` Dmitry Baryshkov
2025-03-09 10:17 ` Cristian Ciocaltea
2025-03-08 12:21 ` [PATCH v5 04/12] phy: rockchip: samsung-hdptx: Drop unused struct lcpll_config Cristian Ciocaltea
2025-03-09 9:18 ` Dmitry Baryshkov
2025-03-08 12:21 ` [PATCH v5 05/12] phy: rockchip: samsung-hdptx: Drop unused phy_cfg driver data Cristian Ciocaltea
2025-03-09 9:18 ` Dmitry Baryshkov
2025-03-08 12:21 ` [PATCH v5 06/12] phy: rockchip: samsung-hdptx: Drop superfluous cfgs " Cristian Ciocaltea
2025-03-09 9:19 ` Dmitry Baryshkov
2025-03-08 12:21 ` [PATCH v5 07/12] phy: rockchip: samsung-hdptx: Avoid Hz<->hHz unit conversion overhead Cristian Ciocaltea
2025-03-09 9:22 ` Dmitry Baryshkov
2025-03-09 10:13 ` Cristian Ciocaltea
2025-03-09 14:47 ` David Laight [this message]
2025-03-09 22:00 ` Cristian Ciocaltea
2025-03-08 12:21 ` [PATCH v5 08/12] phy: rockchip: samsung-hdptx: Setup TMDS char rate via phy_configure_opts_hdmi Cristian Ciocaltea
2025-03-09 9:26 ` Dmitry Baryshkov
2025-03-09 10:15 ` Cristian Ciocaltea
2025-03-18 12:48 ` Cristian Ciocaltea
2025-03-08 12:21 ` [PATCH v5 09/12] phy: rockchip: samsung-hdptx: Provide config params validation support Cristian Ciocaltea
2025-03-09 9:27 ` Dmitry Baryshkov
2025-03-08 12:21 ` [PATCH v5 10/12] phy: rockchip: samsung-hdptx: Restrict altering TMDS char rate via CCF Cristian Ciocaltea
2025-03-08 12:21 ` [PATCH v5 11/12] phy: rockchip: samsung-hdptx: Optimize internal rate handling Cristian Ciocaltea
2025-03-09 10:17 ` Dmitry Baryshkov
2025-03-09 10:32 ` Cristian Ciocaltea
2025-03-09 10:33 ` Dmitry Baryshkov
2025-03-08 12:21 ` [PATCH v5 12/12] phy: rockchip: samsung-hdptx: Add high color depth management Cristian Ciocaltea
2025-03-09 10:18 ` Dmitry Baryshkov
2025-03-18 12:56 ` Cristian Ciocaltea
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250309144747.744e5197@pumpkin \
--to=david.laight.linux@gmail.com \
--cc=Sandor.yu@nxp.com \
--cc=algea.cao@rock-chips.com \
--cc=cristian.ciocaltea@collabora.com \
--cc=dmitry.baryshkov@linaro.org \
--cc=heiko@sntech.de \
--cc=kernel@collabora.com \
--cc=kishon@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=mripard@kernel.org \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox