From: Dominique Martinet <dominique.martinet@atmark-techno.com>
To: Adam Ford <aford173@gmail.com>
Cc: linux-phy@lists.infradead.org, linux-imx@nxp.com,
festevam@gmail.com, frieder.schrempf@kontron.de,
aford@beaconembedded.com, Sandor.yu@nxp.com,
"Vinod Koul" <vkoul@kernel.org>,
"Kishon Vijay Abraham I" <kishon@kernel.org>,
"Marco Felsch" <m.felsch@pengutronix.de>,
"Lucas Stach" <l.stach@pengutronix.de>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
linux-kernel@vger.kernel.org,
"Makoto Sato" <makoto.sato@atmark-techno.com>
Subject: Re: [PATCH V4 4/5] phy: freescale: fsl-samsung-hdmi: Use closest divider
Date: Tue, 3 Sep 2024 14:12:32 +0900 [thread overview]
Message-ID: <ZtaawD3vb8gmnVmO@atmark-techno.com> (raw)
In-Reply-To: <20240903013113.139698-5-aford173@gmail.com>
Thank you!
Adam Ford wrote on Mon, Sep 02, 2024 at 08:30:46PM -0500:
> + /* Calculate the differences and use the closest one */
> + delta_frac = (rate - phy_pll_cfg[i].pixclk);
> + delta_int = (rate - int_div_clk);
This assumes rate > whatever pixclk was found, that doesn't look true to
me for the integer calculation (`delta = abs(fout - tmp)` so it looks
like it can pick a larger value)
For the LUT, the way the lookup works is by picking the closest smaller
value so this is not a problem, but someone might come fix that later so
I'd rather just use abs() everywhere for future-proofing
That aside it looks good to me, I'll add a 0.5% tolerance patch and test
this all ASAP (might be a few days); will send the tolerance patch
properly after testing but for reference it will probably look like
this:
---
From 12479386c955a59330232c84f4f856606c3a53e0 Mon Sep 17 00:00:00 2001
From: Dominique Martinet <dominique.martinet@atmark-techno.com>
Date: Tue, 3 Sep 2024 13:47:24 +0900
Subject: [PATCH] drm/bridge: imx8mp-hdmi-tx: allow 0.5% margin with selected clock
This allows the hdmi driver to pick e.g. 64.8MHz instead of 65Mhz when we
cannot output the exact frequency, enabling the imx8mp HDMI output to
support more modes
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
index 13bc570c5473..9431cd5e06c3 100644
--- a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
+++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-tx.c
@@ -23,6 +23,7 @@ imx8mp_hdmi_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
const struct drm_display_mode *mode)
{
struct imx8mp_hdmi *hdmi = (struct imx8mp_hdmi *)data;
+ long round_rate;
if (mode->clock < 13500)
return MODE_CLOCK_LOW;
@@ -30,8 +31,9 @@ imx8mp_hdmi_mode_valid(struct dw_hdmi *dw_hdmi, void *data,
if (mode->clock > 297000)
return MODE_CLOCK_HIGH;
- if (clk_round_rate(hdmi->pixclk, mode->clock * 1000) !=
- mode->clock * 1000)
+ round_rate = clk_round_rate(hdmi->pixclk, mode->clock * 1000);
+ /* accept 0.5% = 5/1000 tolerance (mode->clock is 1/1000) */
+ if (abs(round_rate - mode->clock * 1000) > mode->clock * 5)
return MODE_CLOCK_RANGE;
/* We don't support double-clocked and Interlaced modes */
---
--
Dominique
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
next prev parent reply other threads:[~2024-09-03 5:12 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-03 1:30 [PATCH V4 0/5] phy: freescale: fsl-samsung-hdmi: Expand phy clock options Adam Ford
2024-09-03 1:30 ` [PATCH V4 1/5] phy: freescale: fsl-samsung-hdmi: Replace register defines with macro Adam Ford
2024-09-03 7:47 ` Marco Felsch
2024-09-03 1:30 ` [PATCH V4 2/5] phy: freescale: fsl-samsung-hdmi: Simplify REG21_PMS_S_MASK lookup Adam Ford
2024-09-03 7:57 ` Marco Felsch
2024-09-03 1:30 ` [PATCH V4 3/5] phy: freescale: fsl-samsung-hdmi: Support dynamic integer Adam Ford
2024-09-04 8:28 ` Dominique Martinet
2024-09-04 10:37 ` Adam Ford
2024-09-03 1:30 ` [PATCH V4 4/5] phy: freescale: fsl-samsung-hdmi: Use closest divider Adam Ford
2024-09-03 5:12 ` Dominique Martinet [this message]
2024-09-03 1:30 ` [PATCH V4 5/5] phy: freescale: fsl-samsung-hdmi: Remove unnecessary LUT entries Adam Ford
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=ZtaawD3vb8gmnVmO@atmark-techno.com \
--to=dominique.martinet@atmark-techno.com \
--cc=Sandor.yu@nxp.com \
--cc=aford173@gmail.com \
--cc=aford@beaconembedded.com \
--cc=festevam@gmail.com \
--cc=frieder.schrempf@kontron.de \
--cc=kishon@kernel.org \
--cc=l.stach@pengutronix.de \
--cc=linux-imx@nxp.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-phy@lists.infradead.org \
--cc=m.felsch@pengutronix.de \
--cc=makoto.sato@atmark-techno.com \
--cc=u.kleine-koenig@pengutronix.de \
--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;
as well as URLs for NNTP newsgroup(s).