[PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

[PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

[Frieder Schrempf]

This small series depends on Adam's series here [1].

The parameters have been determined using the script here [2] and validated by
trying the modes on a Kontron BL i.MX8MP with a HDMI USB grabber.

[1] https://patchwork.kernel.org/project/linux-phy/cover/20240904233100.114611-1-aford173@gmail.com/
[2] https://codeberg.org/fschrempf/samsung-hdmi-phy-pll-calculator/src/branch/main/pll.py

Frieder Schrempf (2):
  phy: freescale: fsl-samsung-hdmi: Add references for calculating LUT
    parameters to comment
  phy: freescale: fsl-samsung-hdmi: Add PLL LUT entries for some
    non-CEA-861 modes

 drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

-- 
2.46.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH 1/2] phy: freescale: fsl-samsung-hdmi: Add references for calculating LUT parameters to comment

[Frieder Schrempf]

Mention the Reference Manual and the scripts and tools available
to help calculating the PLL parameters for additional LUT entries.

Suggested-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
Signed-off-by: Frieder Schrempf <frieder@fris.de>
---
 drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
index 1203143bad7a..401178bfcdda 100644
--- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
+++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
@@ -61,7 +61,13 @@ static struct phy_config calculated_phy_pll_cfg = {
 	.pll_div_regs = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x80, 0x00 },
 };
 
-/* The lookup table contains values for which the fractional divder is used */
+/*
+ * The lookup table contains values for which the fractional divder is used.
+ * Please see the i.MX8MP Reference Manual and the resources at [1] for
+ * additional information on calculating values for table entries.
+ *
+ * [1] https://codeberg.org/fschrempf/samsung-hdmi-phy-pll-calculator
+ */
 static const struct phy_config phy_pll_cfg[] = {
 	{
 		.pixclk = 22250000,
-- 
2.46.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH 2/2] phy: freescale: fsl-samsung-hdmi: Add PLL LUT entries for some non-CEA-861 modes

[Frieder Schrempf]

In order to support some pixel clock values not specified in CEA-861
and not achievable by the integer PLL algorithm, we add some new LUT
entries. This allows to use the fractional-N PLL to achieve pixel
clocks of 75 MHz, 88.75 MHz and 296.703 MHz with high accuracy.

Signed-off-by: Frieder Schrempf <frieder@fris.de>
---
 drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
index 401178bfcdda..6b36318630b2 100644
--- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
+++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
@@ -156,6 +156,9 @@ static const struct phy_config phy_pll_cfg[] = {
 	}, {
 		.pixclk = 74250000,
 		.pll_div_regs = { 0xd1, 0x5c, 0x52, 0x90, 0x0d, 0x84, 0x41 },
+	}, {
+		.pixclk = 75000000,
+		.pll_div_regs = { 0xD1, 0x3E, 0x30, 0x9F, 0x0E, 0x82, 0x41 },
 	}, {
 		.pixclk = 78500000,
 		.pll_div_regs = { 0xd1, 0x62, 0x54, 0x87, 0x01, 0x80, 0x40 },
@@ -165,6 +168,9 @@ static const struct phy_config phy_pll_cfg[] = {
 	}, {
 		.pixclk = 82500000,
 		.pll_div_regs = { 0xd1, 0x67, 0x54, 0x88, 0x01, 0x90, 0x49 },
+	}, {
+		.pixclk = 88750000,
+		.pll_div_regs = { 0xD1, 0x6E, 0x50, 0x8B, 0x06, 0x81, 0x40 },
 	}, {
 		.pixclk = 89000000,
 		.pll_div_regs = { 0xd1, 0x70, 0x54, 0x84, 0x83, 0x80, 0x40 },
@@ -277,6 +283,9 @@ static const struct phy_config phy_pll_cfg[] = {
 		.pixclk = 277500000,
 		.pll_div_regs = { 0xd1, 0x73, 0x15, 0x88, 0x05, 0x90, 0x4d },
 	},  {
+		.pixclk = 296703000,
+		.pll_div_regs = { 0xd1, 0x7b, 0x18, 0xe0, 0x3d, 0x8a, 0x41 },
+	}, {
 		.pixclk = 297000000,
 		.pll_div_regs = { 0xd1, 0x7b, 0x15, 0x84, 0x03, 0x90, 0x45 },
 	},
-- 
2.46.0


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

[Dominique Martinet]

Frieder Schrempf wrote on Tue, Sep 10, 2024 at 08:14:51PM +0200:
> [2] https://codeberg.org/fschrempf/samsung-hdmi-phy-pll-calculator/src/branch/main/pll.py

Great work! Thanks!

I was curious about the existing table entries, recomputing existing
values doesn't yield the same values. For example, the first entry is
{
        .pixclk = 22250000,
        .pll_div_regs = { 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40 },
}
but computing it yields
{
    .pixclk = 22250000,
    .pll_div_regs = { 0xd1, 0x4a, 0xf0, 0xef, 0x10, 0x81, 0x40 },
}

I assume there just are multiple ways to generate the same frequencies,
which is fine in itself, but it'd be great to be able to "back-compute"
the entries as a sanity check.

I've played a bit with your script and spent more time on it than I'd
like to admit, but something like this seems to do the trick, plugging
in the regs from the kernel:

---
pll = FractionalNPLL(freq_ref)

regs = [0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40]
# assume fractional
if not regs[0] & 0xD0:
    print("reg[0] missing 0xD0")
    sys.exit(1)
pll.freq_frac = True
pll.params["p"] = regs[0] & 0x2f
pll.params["m"] = regs[1]
pll.params["s"] = (regs[2] >> 4) + 1
pll.params["n2"] = ((regs[2] >> 3) & 0x1) + 1
pll.params["n"] = (regs[2] & 0x7) + 4
pll.params["lc"] = regs[3] & 0x7f
if regs[4] & 0x80:
    pll.params["lc"] = - pll.params["lc"]
pll.params["k"] = regs[4] & 0x7f
pll.params["lc_s"] = regs[5] & 0x7f
pll.params["k_s"] = regs[6] & 0xbf


f_vco = int(pll.params["m"] * pll.f_ref / pll.params["p"])
if f_vco < 750000000 or f_vco > 3000000000:
    print(f"f_vco {f_vco} out of range")
    sys.exit(1)
f_calc = f_vco / pll.params["s"] / 5
pll.freq_int = round(f_calc)
print(f_calc)
sdc = pll.calc_sdc(pll.params)
frac = pll.calc_f_frac(sdc, pll.params)
print(frac)
freq = pll.freq_int + frac
print(freq)
pll.print_reg_driver_data(freq)
exit(0);
---
yields this back (comments added manually)
---
22500000.0 (integer part)
-250000.0 (fractional part)
22250000.0 (summed)

PHY Driver Table Entry:
{
    .pixclk = 22250000.0,
    .pll_div_regs = { 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x81, 0x40 },
}
---

so if I find some time I'll whip some loop to check all other values...




That aside, I see no problem with this, just one meta-comment about
adding a link to the script in an external repo: I see some other
drivers have python scripts in their trees e.g.
drivers/comedi/drivers/ni_routing/tools/*py
drivers/gpu/drm/ci/xfails/update-xfails.py
drivers/gpu/drm/msm/registers/gen_header.py

would it make sense to commit the script here instead of linking to a
repo that might be lost in the future?

I'm not quite sure what policy the linux repo has here, so leaving that
as an open question.
-- 
Dominique

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

[Adam Ford]

On Tue, Sep 10, 2024 at 7:23 PM Dominique Martinet
<dominique.martinet@atmark-techno.com> wrote:
>
> Frieder Schrempf wrote on Tue, Sep 10, 2024 at 08:14:51PM +0200:
> > [2] https://codeberg.org/fschrempf/samsung-hdmi-phy-pll-calculator/src/branch/main/pll.py
>
> Great work! Thanks!
>
> I was curious about the existing table entries, recomputing existing
> values doesn't yield the same values. For example, the first entry is
> {
>         .pixclk = 22250000,
>         .pll_div_regs = { 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40 },
> }
> but computing it yields
> {
>     .pixclk = 22250000,
>     .pll_div_regs = { 0xd1, 0x4a, 0xf0, 0xef, 0x10, 0x81, 0x40 },
> }
>
> I assume there just are multiple ways to generate the same frequencies,
> which is fine in itself, but it'd be great to be able to "back-compute"
> the entries as a sanity check.
>
> I've played a bit with your script and spent more time on it than I'd
> like to admit, but something like this seems to do the trick, plugging
> in the regs from the kernel:
>
> ---
> pll = FractionalNPLL(freq_ref)
>
> regs = [0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40]
> # assume fractional
> if not regs[0] & 0xD0:
>     print("reg[0] missing 0xD0")
>     sys.exit(1)
> pll.freq_frac = True
> pll.params["p"] = regs[0] & 0x2f
> pll.params["m"] = regs[1]
> pll.params["s"] = (regs[2] >> 4) + 1
> pll.params["n2"] = ((regs[2] >> 3) & 0x1) + 1
> pll.params["n"] = (regs[2] & 0x7) + 4
> pll.params["lc"] = regs[3] & 0x7f
> if regs[4] & 0x80:
>     pll.params["lc"] = - pll.params["lc"]
> pll.params["k"] = regs[4] & 0x7f
> pll.params["lc_s"] = regs[5] & 0x7f
> pll.params["k_s"] = regs[6] & 0xbf
>
>
> f_vco = int(pll.params["m"] * pll.f_ref / pll.params["p"])
> if f_vco < 750000000 or f_vco > 3000000000:
>     print(f"f_vco {f_vco} out of range")
>     sys.exit(1)
> f_calc = f_vco / pll.params["s"] / 5
> pll.freq_int = round(f_calc)
> print(f_calc)
> sdc = pll.calc_sdc(pll.params)
> frac = pll.calc_f_frac(sdc, pll.params)
> print(frac)
> freq = pll.freq_int + frac
> print(freq)
> pll.print_reg_driver_data(freq)
> exit(0);
> ---
> yields this back (comments added manually)
> ---
> 22500000.0 (integer part)
> -250000.0 (fractional part)
> 22250000.0 (summed)
>
> PHY Driver Table Entry:
> {
>     .pixclk = 22250000.0,
>     .pll_div_regs = { 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x81, 0x40 },
> }
> ---
>
> so if I find some time I'll whip some loop to check all other values...
>
>
>
>
> That aside, I see no problem with this, just one meta-comment about
> adding a link to the script in an external repo: I see some other
> drivers have python scripts in their trees e.g.
> drivers/comedi/drivers/ni_routing/tools/*py
> drivers/gpu/drm/ci/xfails/update-xfails.py
> drivers/gpu/drm/msm/registers/gen_header.py
>
> would it make sense to commit the script here instead of linking to a
> repo that might be lost in the future?
>
> I'm not quite sure what policy the linux repo has here, so leaving that
> as an open question.

Is there a reason this couldn't be coded in C and used to expand my
integer calculator series?  With that, we could drop the lookup table.

adam
> --
> Dominique

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

[Dominique Martinet]

Adam Ford wrote on Tue, Sep 10, 2024 at 08:16:04PM -0500:
> > That aside, I see no problem with this, just one meta-comment about
> > adding a link to the script in an external repo: I see some other
> > drivers have python scripts in their trees e.g.
> > drivers/comedi/drivers/ni_routing/tools/*py
> > drivers/gpu/drm/ci/xfails/update-xfails.py
> > drivers/gpu/drm/msm/registers/gen_header.py
> >
> > would it make sense to commit the script here instead of linking to a
> > repo that might be lost in the future?
> >
> > I'm not quite sure what policy the linux repo has here, so leaving that
> > as an open question.
> 
> Is there a reason this couldn't be coded in C and used to expand my
> integer calculator series?  With that, we could drop the lookup table.

Quoting a previous mail from Frieder:
> I will clean things up a bit and then share what I have. I hope that this
> allows anyone to calculate parameters for their non-standard displays if
> required.
> 
> If someone feels extra motivated they could try to calculate the fractional
> parameters at runtime. However I'm not sure that this is feasible. The
> numerical computation of a large number of parameters is quite heavy and it's
> probably not easy to strip the algorithm down to something that can be run on
> the target without too much overhead.

Trying a random frequency with the algorithm he has implemented it
easily takes 10 seconds to run on my imx8mp board, so even if we asssume
C is 3-4 times faster I think the current algorithm is too slow for
runtime and it makes more sense to extended the LUT to me (as long as
the values can be & are checked at least once, which we now can)

The current algorithm brute-forces its way through so there could be a
better way of computing the fractional part of the divider, but I'm not
sure it's worth the effort at this point; I guess it's a good
intellectual challenge though so someone might do it in the future.

-- 
Dominique

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

[Dominique Martinet]

Dominique Martinet wrote on Wed, Sep 11, 2024 at 09:23:31AM +0900:
> so if I find some time I'll whip some loop to check all other values...

There were more differences than I thought there'd be (see diff below)

they're all minor enough to probably not care, but your script finds
exact values for them so it might be worth updating the regs if we keep
the table...
(OTOH these seem to work, and "if it works don't touch it" -- I have no
further opinion now I've checked, curiosity is sated)

Might want to just check 154000000 though, reg6's 0x80 was not set so
SDC was disabled? and it'd get the integer fraction (153600000) instead
if I understand this correctly

---------
diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
index a5ad51425ee3..72facc7bd045 100644
--- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
+++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
@@ -73,7 +73,7 @@ static const struct phy_config phy_pll_cfg[] = {
 		.pixclk = 24024000,
 		.pll_div_regs = { 0xd1, 0x50, 0xf1, 0x99, 0x02, 0x80, 0x40 },
 	}, {
-		.pixclk = 25175000,
+		.pixclk = 25177981,
 		.pll_div_regs = { 0xd1, 0x54, 0xfc, 0xcc, 0x91, 0x80, 0x40 },
 	},  {
 		.pixclk = 26750000,
@@ -82,16 +82,16 @@ static const struct phy_config phy_pll_cfg[] = {
 		.pixclk = 27027000,
 		.pll_div_regs = { 0xd1, 0x5a, 0xf2, 0xfd, 0x0c, 0x80, 0x40 },
 	}, {
-		.pixclk = 29500000,
+		.pixclk = 29487500,
 		.pll_div_regs = { 0xd1, 0x62, 0xf4, 0x95, 0x08, 0x80, 0x40 },
 	}, {
-		.pixclk = 30750000,
+		.pixclk = 30729661,
 		.pll_div_regs = { 0xd1, 0x66, 0xf4, 0x82, 0x01, 0x88, 0x45 },
 	}, {
-		.pixclk = 30888000,
+		.pixclk = 30848949,
 		.pll_div_regs = { 0xd1, 0x66, 0xf4, 0x99, 0x18, 0x88, 0x45 },
 	}, {
-		.pixclk = 33750000,
+		.pixclk = 33731250,
 		.pll_div_regs = { 0xd1, 0x70, 0xf4, 0x82, 0x01, 0x80, 0x40 },
 	}, {
 		.pixclk = 35000000,
@@ -106,13 +106,13 @@ static const struct phy_config phy_pll_cfg[] = {
 		.pixclk = 44500000,
 		.pll_div_regs = { 0xd1, 0x5c, 0x92, 0x98, 0x11, 0x84, 0x41 },
 	}, {
-		.pixclk = 47000000,
+		.pixclk = 47005000,
 		.pll_div_regs = { 0xd1, 0x62, 0x94, 0x95, 0x82, 0x80, 0x40 },
 	}, {
-		.pixclk = 47500000,
+		.pixclk = 47502000,
 		.pll_div_regs = { 0xd1, 0x63, 0x96, 0xa1, 0x82, 0x80, 0x40 },
 	}, {
-		.pixclk = 50349650,
+		.pixclk = 50355924,
 		.pll_div_regs = { 0xd1, 0x54, 0x7c, 0xc3, 0x8f, 0x80, 0x40 },
 	}, {
 		.pixclk = 53250000,
@@ -124,16 +124,16 @@ static const struct phy_config phy_pll_cfg[] = {
 		.pixclk = 54054000,
 		.pll_div_regs = { 0xd1, 0x5a, 0x72, 0xfd, 0x0c, 0x80, 0x40 },
 	}, {
-		.pixclk = 59000000,
+		.pixclk = 58975000,
 		.pll_div_regs = { 0xd1, 0x62, 0x74, 0x95, 0x08, 0x80, 0x40 },
 	}, {
-		.pixclk = 59340659,
+		.pixclk = 59264777,
 		.pll_div_regs = { 0xd1, 0x62, 0x74, 0xdb, 0x52, 0x88, 0x47 },
 	},  {
-		.pixclk = 61500000,
+		.pixclk = 61459322,
 		.pll_div_regs = { 0xd1, 0x66, 0x74, 0x82, 0x01, 0x88, 0x45 },
 	}, {
-		.pixclk = 63500000,
+		.pixclk = 63437500,
 		.pll_div_regs = { 0xd1, 0x69, 0x74, 0x89, 0x08, 0x80, 0x40 },
 	}, {
 		.pixclk = 67500000,
@@ -145,25 +145,25 @@ static const struct phy_config phy_pll_cfg[] = {
 		.pixclk = 72072000,
 		.pll_div_regs = { 0xd1, 0x5a, 0x52, 0xfd, 0x0c, 0x80, 0x40 },
 	}, {
-		.pixclk = 74176000,
+		.pixclk = 74175824,
 		.pll_div_regs = { 0xd1, 0x5d, 0x58, 0xdb, 0xA2, 0x88, 0x41 },
 	}, {
 		.pixclk = 74250000,
 		.pll_div_regs = { 0xd1, 0x5c, 0x52, 0x90, 0x0d, 0x84, 0x41 },
 	}, {
-		.pixclk = 78500000,
+		.pixclk = 78487500,
 		.pll_div_regs = { 0xd1, 0x62, 0x54, 0x87, 0x01, 0x80, 0x40 },
 	},  {
-		.pixclk = 82000000,
+		.pixclk = 81945763,
 		.pll_div_regs = { 0xd1, 0x66, 0x54, 0x82, 0x01, 0x88, 0x45 },
 	}, {
-		.pixclk = 82500000,
+		.pixclk = 82486555,
 		.pll_div_regs = { 0xd1, 0x67, 0x54, 0x88, 0x01, 0x90, 0x49 },
 	}, {
-		.pixclk = 89000000,
+		.pixclk = 89075000,
 		.pll_div_regs = { 0xd1, 0x70, 0x54, 0x84, 0x83, 0x80, 0x40 },
 	}, {
-		.pixclk = 90000000,
+		.pixclk = 89950000,
 		.pll_div_regs = { 0xd1, 0x70, 0x54, 0x82, 0x01, 0x80, 0x40 },
 	}, {
 		.pixclk = 94000000,
@@ -178,7 +178,7 @@ static const struct phy_config phy_pll_cfg[] = {
 		.pixclk = 99000000,
 		.pll_div_regs = { 0xd1, 0x52, 0x32, 0x82, 0x01, 0x88, 0x47 },
 	}, {
-		.pixclk = 100699300,
+		.pixclk = 100711847,
 		.pll_div_regs = { 0xd1, 0x54, 0x3c, 0xc3, 0x8f, 0x80, 0x40 },
 	},  {
 		.pixclk = 102500000,
@@ -196,16 +196,16 @@ static const struct phy_config phy_pll_cfg[] = {
 		.pixclk = 108108000,
 		.pll_div_regs = { 0xd1, 0x5a, 0x32, 0xfd, 0x0c, 0x80, 0x40 },
 	}, {
-		.pixclk = 118000000,
+		.pixclk = 117950000,
 		.pll_div_regs = { 0xd1, 0x62, 0x34, 0x95, 0x08, 0x80, 0x40 },
 	},  {
-		.pixclk = 123000000,
+		.pixclk = 122918644,
 		.pll_div_regs = { 0xd1, 0x66, 0x34, 0x82, 0x01, 0x88, 0x45 },
 	}, {
-		.pixclk = 127000000,
+		.pixclk = 126875000,
 		.pll_div_regs = { 0xd1, 0x69, 0x34, 0x89, 0x08, 0x80, 0x40 },
 	}, {
-		.pixclk = 135000000,
+		.pixclk = 134925000,
 		.pll_div_regs = { 0xd1, 0x70, 0x34, 0x82, 0x01, 0x80, 0x40 },
 	}, {
 		.pixclk = 135580000,
@@ -214,20 +214,20 @@ static const struct phy_config phy_pll_cfg[] = {
 		.pixclk = 137520000,
 		.pll_div_regs = { 0xd1, 0x72, 0x38, 0x99, 0x10, 0x85, 0x41 },
 	}, {
-		.pixclk = 138750000,
+		.pixclk = 138658397,
 		.pll_div_regs = { 0xd1, 0x73, 0x35, 0x88, 0x05, 0x90, 0x4d },
 	}, {
-		.pixclk = 140000000,
+		.pixclk = 140040000,
 		.pll_div_regs = { 0xd1, 0x75, 0x36, 0xa7, 0x90, 0x80, 0x40 },
 	},  {
-		.pixclk = 148352000,
+		.pixclk = 148265128,
 		.pll_div_regs = { 0xd1, 0x7b, 0x35, 0xdb, 0x39, 0x90, 0x45 },
 	}, {
-		.pixclk = 148500000,
+		.pixclk = 148396403,
 		.pll_div_regs = { 0xd1, 0x7b, 0x35, 0x84, 0x03, 0x90, 0x45 },
 	}, {
 		.pixclk = 154000000,
-		.pll_div_regs = { 0xd1, 0x40, 0x18, 0x83, 0x01, 0x00, 0x40 },
+		.pll_div_regs = { 0xd1, 0x40, 0x18, 0x83, 0x01, 0x80, 0x40 },
 	}, {
 		.pixclk = 157000000,
 		.pll_div_regs = { 0xd1, 0x41, 0x11, 0xa7, 0x14, 0x80, 0x40 },
@@ -268,10 +268,10 @@ static const struct phy_config phy_pll_cfg[] = {
 		.pixclk = 254000000,
 		.pll_div_regs = { 0xd1, 0x69, 0x14, 0x89, 0x08, 0x80, 0x40 },
 	}, {
-		.pixclk = 277500000,
+		.pixclk = 277316794,
 		.pll_div_regs = { 0xd1, 0x73, 0x15, 0x88, 0x05, 0x90, 0x4d },
 	},  {
-		.pixclk = 297000000,
+		.pixclk = 296792806,
 		.pll_div_regs = { 0xd1, 0x7b, 0x15, 0x84, 0x03, 0x90, 0x45 },
 	},
 };
-------------
-- 
Dominique

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

[Frieder Schrempf]

On 11.09.24 02:23, Dominique Martinet wrote:
> Frieder Schrempf wrote on Tue, Sep 10, 2024 at 08:14:51PM +0200:
>> [2] https://codeberg.org/fschrempf/samsung-hdmi-phy-pll-calculator/src/branch/main/pll.py
> 
> Great work! Thanks!
> 
> I was curious about the existing table entries, recomputing existing
> values doesn't yield the same values. For example, the first entry is
> {
>          .pixclk = 22250000,
>          .pll_div_regs = { 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40 },
> }
> but computing it yields
> {
>      .pixclk = 22250000,
>      .pll_div_regs = { 0xd1, 0x4a, 0xf0, 0xef, 0x10, 0x81, 0x40 },
> }
> 
> I assume there just are multiple ways to generate the same frequencies,
> which is fine in itself, but it'd be great to be able to "back-compute"
> the entries as a sanity check.
> 
> I've played a bit with your script and spent more time on it than I'd
> like to admit, but something like this seems to do the trick, plugging
> in the regs from the kernel:

Thanks for the feedback and the additional code. Yes, the script yields 
different results for the existing table entries.

For the reverse operation I used the spreadsheet (pll.ods, also in the 
repo). Actually I wrote that before the Python code and used it for 
verification.

> 
> ---
> pll = FractionalNPLL(freq_ref)
> 
> regs = [0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x80, 0x40]
> # assume fractional
> if not regs[0] & 0xD0:
>      print("reg[0] missing 0xD0")
>      sys.exit(1)
> pll.freq_frac = True
> pll.params["p"] = regs[0] & 0x2f
> pll.params["m"] = regs[1]
> pll.params["s"] = (regs[2] >> 4) + 1
> pll.params["n2"] = ((regs[2] >> 3) & 0x1) + 1
> pll.params["n"] = (regs[2] & 0x7) + 4
> pll.params["lc"] = regs[3] & 0x7f
> if regs[4] & 0x80:
>      pll.params["lc"] = - pll.params["lc"]
> pll.params["k"] = regs[4] & 0x7f
> pll.params["lc_s"] = regs[5] & 0x7f
> pll.params["k_s"] = regs[6] & 0xbf
> 
> 
> f_vco = int(pll.params["m"] * pll.f_ref / pll.params["p"])
> if f_vco < 750000000 or f_vco > 3000000000:
>      print(f"f_vco {f_vco} out of range")
>      sys.exit(1)
> f_calc = f_vco / pll.params["s"] / 5
> pll.freq_int = round(f_calc)
> print(f_calc)
> sdc = pll.calc_sdc(pll.params)
> frac = pll.calc_f_frac(sdc, pll.params)
> print(frac)
> freq = pll.freq_int + frac
> print(freq)
> pll.print_reg_driver_data(freq)
> exit(0);
> ---
> yields this back (comments added manually)
> ---
> 22500000.0 (integer part)
> -250000.0 (fractional part)
> 22250000.0 (summed)
> 
> PHY Driver Table Entry:
> {
>      .pixclk = 22250000.0,
>      .pll_div_regs = { 0xd1, 0x4b, 0xf1, 0x89, 0x88, 0x81, 0x40 },
> }
> ---
> 
> so if I find some time I'll whip some loop to check all other values...

If you look at the second sheet in the pll.ods file, you can see that I 
already started to create a table for verifying the existing values. I 
gave up at some point when I saw my calculations were giving reasonable 
results and continued with writing the script. But it's easy to extend 
the first columns with the values from the driver and copy the other 
columns to provide the calculations.

> 
> That aside, I see no problem with this, just one meta-comment about
> adding a link to the script in an external repo: I see some other
> drivers have python scripts in their trees e.g.
> drivers/comedi/drivers/ni_routing/tools/*py
> drivers/gpu/drm/ci/xfails/update-xfails.py
> drivers/gpu/drm/msm/registers/gen_header.py
> 
> would it make sense to commit the script here instead of linking to a
> repo that might be lost in the future?
> 
> I'm not quite sure what policy the linux repo has here, so leaving that
> as an open question.

I was thinking about this as well and I really have no idea what is 
preferred by the maintainers. So I will just wait for some feedback here.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

[Frieder Schrempf]

On 11.09.24 03:27, Dominique Martinet wrote:
> Adam Ford wrote on Tue, Sep 10, 2024 at 08:16:04PM -0500:
>>> That aside, I see no problem with this, just one meta-comment about
>>> adding a link to the script in an external repo: I see some other
>>> drivers have python scripts in their trees e.g.
>>> drivers/comedi/drivers/ni_routing/tools/*py
>>> drivers/gpu/drm/ci/xfails/update-xfails.py
>>> drivers/gpu/drm/msm/registers/gen_header.py
>>>
>>> would it make sense to commit the script here instead of linking to a
>>> repo that might be lost in the future?
>>>
>>> I'm not quite sure what policy the linux repo has here, so leaving that
>>> as an open question.
>>
>> Is there a reason this couldn't be coded in C and used to expand my
>> integer calculator series?  With that, we could drop the lookup table.
> 
> Quoting a previous mail from Frieder:
>> I will clean things up a bit and then share what I have. I hope that this
>> allows anyone to calculate parameters for their non-standard displays if
>> required.
>>
>> If someone feels extra motivated they could try to calculate the fractional
>> parameters at runtime. However I'm not sure that this is feasible. The
>> numerical computation of a large number of parameters is quite heavy and it's
>> probably not easy to strip the algorithm down to something that can be run on
>> the target without too much overhead.
> 
> Trying a random frequency with the algorithm he has implemented it
> easily takes 10 seconds to run on my imx8mp board, so even if we asssume
> C is 3-4 times faster I think the current algorithm is too slow for
> runtime and it makes more sense to extended the LUT to me (as long as
> the values can be & are checked at least once, which we now can)
> 
> The current algorithm brute-forces its way through so there could be a
> better way of computing the fractional part of the divider, but I'm not
> sure it's worth the effort at this point; I guess it's a good
> intellectual challenge though so someone might do it in the future.

Second this. My algorithm is far from optimal and there might be more 
elegant solutions. If anyone feels like creating something that can be 
added to the driver that would be interesting and welcome for sure.

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

[Frieder Schrempf]

On 11.09.24 08:08, Dominique Martinet wrote:
> Dominique Martinet wrote on Wed, Sep 11, 2024 at 09:23:31AM +0900:
>> so if I find some time I'll whip some loop to check all other values...
> 
> There were more differences than I thought there'd be (see diff below)
> 
> they're all minor enough to probably not care, but your script finds
> exact values for them so it might be worth updating the regs if we keep
> the table...
> (OTOH these seem to work, and "if it works don't touch it" -- I have no
> further opinion now I've checked, curiosity is sated)

Yeah, that's interesting. A lot of those values seem to be a little off. 
Or my calculation has some problem, but it's unlikely as everything else 
seems to add up so nicely.

The deviations seem very minor, only in the per mill ballpark or even 
lower. I wonder how they got these values initially.

It would still be nice to verify the clocks with some real measurements 
on the hardware level before we change the existing values.

On a side note, there is drivers/gpu/drm/exynos/exynos_hdmi.c which is 
basically for the same kind of PHY, probably an older version of the IP 
core. It also has some LUTs for the PLL registers. But I didn't compare 
them so far.

> 
> Might want to just check 154000000 though, reg6's 0x80 was not set so
> SDC was disabled? and it'd get the integer fraction (153600000) instead
> if I understand this correctly

I think there are two switches. The fractional-N part can be enabled but 
the sigma-delta-part can still be disabled. That's different than having 
the fractional-N completely turned off.

My script doesn't cover this case. It assumes the SDC is always enabled 
if the fractional-N is enabled.

The problem is that I don't know what part of the calculation is the SDC 
value exactly. The reference manual is confusing and probably defective 
in some way in regard to this. So I don't really know when to 
enable/disable the SDC part.

For the 154 MHz this means even with SDC turned off the fractional-N 
part will provide the 400 kHZ to be added to the integer part. At least 
that's how I understand it. I might be wrong.

> 
> ---------
> diff --git a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> index a5ad51425ee3..72facc7bd045 100644
> --- a/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> +++ b/drivers/phy/freescale/phy-fsl-samsung-hdmi.c
> @@ -73,7 +73,7 @@ static const struct phy_config phy_pll_cfg[] = {
>   		.pixclk = 24024000,
>   		.pll_div_regs = { 0xd1, 0x50, 0xf1, 0x99, 0x02, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 25175000,
> +		.pixclk = 25177981,
>   		.pll_div_regs = { 0xd1, 0x54, 0xfc, 0xcc, 0x91, 0x80, 0x40 },
>   	},  {
>   		.pixclk = 26750000,
> @@ -82,16 +82,16 @@ static const struct phy_config phy_pll_cfg[] = {
>   		.pixclk = 27027000,
>   		.pll_div_regs = { 0xd1, 0x5a, 0xf2, 0xfd, 0x0c, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 29500000,
> +		.pixclk = 29487500,
>   		.pll_div_regs = { 0xd1, 0x62, 0xf4, 0x95, 0x08, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 30750000,
> +		.pixclk = 30729661,
>   		.pll_div_regs = { 0xd1, 0x66, 0xf4, 0x82, 0x01, 0x88, 0x45 },
>   	}, {
> -		.pixclk = 30888000,
> +		.pixclk = 30848949,
>   		.pll_div_regs = { 0xd1, 0x66, 0xf4, 0x99, 0x18, 0x88, 0x45 },
>   	}, {
> -		.pixclk = 33750000,
> +		.pixclk = 33731250,
>   		.pll_div_regs = { 0xd1, 0x70, 0xf4, 0x82, 0x01, 0x80, 0x40 },
>   	}, {
>   		.pixclk = 35000000,
> @@ -106,13 +106,13 @@ static const struct phy_config phy_pll_cfg[] = {
>   		.pixclk = 44500000,
>   		.pll_div_regs = { 0xd1, 0x5c, 0x92, 0x98, 0x11, 0x84, 0x41 },
>   	}, {
> -		.pixclk = 47000000,
> +		.pixclk = 47005000,
>   		.pll_div_regs = { 0xd1, 0x62, 0x94, 0x95, 0x82, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 47500000,
> +		.pixclk = 47502000,
>   		.pll_div_regs = { 0xd1, 0x63, 0x96, 0xa1, 0x82, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 50349650,
> +		.pixclk = 50355924,
>   		.pll_div_regs = { 0xd1, 0x54, 0x7c, 0xc3, 0x8f, 0x80, 0x40 },
>   	}, {
>   		.pixclk = 53250000,
> @@ -124,16 +124,16 @@ static const struct phy_config phy_pll_cfg[] = {
>   		.pixclk = 54054000,
>   		.pll_div_regs = { 0xd1, 0x5a, 0x72, 0xfd, 0x0c, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 59000000,
> +		.pixclk = 58975000,
>   		.pll_div_regs = { 0xd1, 0x62, 0x74, 0x95, 0x08, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 59340659,
> +		.pixclk = 59264777,
>   		.pll_div_regs = { 0xd1, 0x62, 0x74, 0xdb, 0x52, 0x88, 0x47 },
>   	},  {
> -		.pixclk = 61500000,
> +		.pixclk = 61459322,
>   		.pll_div_regs = { 0xd1, 0x66, 0x74, 0x82, 0x01, 0x88, 0x45 },
>   	}, {
> -		.pixclk = 63500000,
> +		.pixclk = 63437500,
>   		.pll_div_regs = { 0xd1, 0x69, 0x74, 0x89, 0x08, 0x80, 0x40 },
>   	}, {
>   		.pixclk = 67500000,
> @@ -145,25 +145,25 @@ static const struct phy_config phy_pll_cfg[] = {
>   		.pixclk = 72072000,
>   		.pll_div_regs = { 0xd1, 0x5a, 0x52, 0xfd, 0x0c, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 74176000,
> +		.pixclk = 74175824,
>   		.pll_div_regs = { 0xd1, 0x5d, 0x58, 0xdb, 0xA2, 0x88, 0x41 },
>   	}, {
>   		.pixclk = 74250000,
>   		.pll_div_regs = { 0xd1, 0x5c, 0x52, 0x90, 0x0d, 0x84, 0x41 },
>   	}, {
> -		.pixclk = 78500000,
> +		.pixclk = 78487500,
>   		.pll_div_regs = { 0xd1, 0x62, 0x54, 0x87, 0x01, 0x80, 0x40 },
>   	},  {
> -		.pixclk = 82000000,
> +		.pixclk = 81945763,
>   		.pll_div_regs = { 0xd1, 0x66, 0x54, 0x82, 0x01, 0x88, 0x45 },
>   	}, {
> -		.pixclk = 82500000,
> +		.pixclk = 82486555,
>   		.pll_div_regs = { 0xd1, 0x67, 0x54, 0x88, 0x01, 0x90, 0x49 },
>   	}, {
> -		.pixclk = 89000000,
> +		.pixclk = 89075000,
>   		.pll_div_regs = { 0xd1, 0x70, 0x54, 0x84, 0x83, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 90000000,
> +		.pixclk = 89950000,
>   		.pll_div_regs = { 0xd1, 0x70, 0x54, 0x82, 0x01, 0x80, 0x40 },
>   	}, {
>   		.pixclk = 94000000,
> @@ -178,7 +178,7 @@ static const struct phy_config phy_pll_cfg[] = {
>   		.pixclk = 99000000,
>   		.pll_div_regs = { 0xd1, 0x52, 0x32, 0x82, 0x01, 0x88, 0x47 },
>   	}, {
> -		.pixclk = 100699300,
> +		.pixclk = 100711847,
>   		.pll_div_regs = { 0xd1, 0x54, 0x3c, 0xc3, 0x8f, 0x80, 0x40 },
>   	},  {
>   		.pixclk = 102500000,
> @@ -196,16 +196,16 @@ static const struct phy_config phy_pll_cfg[] = {
>   		.pixclk = 108108000,
>   		.pll_div_regs = { 0xd1, 0x5a, 0x32, 0xfd, 0x0c, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 118000000,
> +		.pixclk = 117950000,
>   		.pll_div_regs = { 0xd1, 0x62, 0x34, 0x95, 0x08, 0x80, 0x40 },
>   	},  {
> -		.pixclk = 123000000,
> +		.pixclk = 122918644,
>   		.pll_div_regs = { 0xd1, 0x66, 0x34, 0x82, 0x01, 0x88, 0x45 },
>   	}, {
> -		.pixclk = 127000000,
> +		.pixclk = 126875000,
>   		.pll_div_regs = { 0xd1, 0x69, 0x34, 0x89, 0x08, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 135000000,
> +		.pixclk = 134925000,
>   		.pll_div_regs = { 0xd1, 0x70, 0x34, 0x82, 0x01, 0x80, 0x40 },
>   	}, {
>   		.pixclk = 135580000,
> @@ -214,20 +214,20 @@ static const struct phy_config phy_pll_cfg[] = {
>   		.pixclk = 137520000,
>   		.pll_div_regs = { 0xd1, 0x72, 0x38, 0x99, 0x10, 0x85, 0x41 },
>   	}, {
> -		.pixclk = 138750000,
> +		.pixclk = 138658397,
>   		.pll_div_regs = { 0xd1, 0x73, 0x35, 0x88, 0x05, 0x90, 0x4d },
>   	}, {
> -		.pixclk = 140000000,
> +		.pixclk = 140040000,
>   		.pll_div_regs = { 0xd1, 0x75, 0x36, 0xa7, 0x90, 0x80, 0x40 },
>   	},  {
> -		.pixclk = 148352000,
> +		.pixclk = 148265128,
>   		.pll_div_regs = { 0xd1, 0x7b, 0x35, 0xdb, 0x39, 0x90, 0x45 },
>   	}, {
> -		.pixclk = 148500000,
> +		.pixclk = 148396403,
>   		.pll_div_regs = { 0xd1, 0x7b, 0x35, 0x84, 0x03, 0x90, 0x45 },
>   	}, {
>   		.pixclk = 154000000,
> -		.pll_div_regs = { 0xd1, 0x40, 0x18, 0x83, 0x01, 0x00, 0x40 },
> +		.pll_div_regs = { 0xd1, 0x40, 0x18, 0x83, 0x01, 0x80, 0x40 },
>   	}, {
>   		.pixclk = 157000000,
>   		.pll_div_regs = { 0xd1, 0x41, 0x11, 0xa7, 0x14, 0x80, 0x40 },
> @@ -268,10 +268,10 @@ static const struct phy_config phy_pll_cfg[] = {
>   		.pixclk = 254000000,
>   		.pll_div_regs = { 0xd1, 0x69, 0x14, 0x89, 0x08, 0x80, 0x40 },
>   	}, {
> -		.pixclk = 277500000,
> +		.pixclk = 277316794,
>   		.pll_div_regs = { 0xd1, 0x73, 0x15, 0x88, 0x05, 0x90, 0x4d },
>   	},  {
> -		.pixclk = 297000000,
> +		.pixclk = 296792806,
>   		.pll_div_regs = { 0xd1, 0x7b, 0x15, 0x84, 0x03, 0x90, 0x45 },
>   	},
>   };
> -------------

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH 0/2] Extending PLL LUT for i.MX8MP Samsung HDMI PHY

[Frieder Schrempf]

Hi,

On 10.09.24 20:14, Frieder Schrempf wrote:
> This small series depends on Adam's series here [1].
> 
> The parameters have been determined using the script here [2] and validated by
> trying the modes on a Kontron BL i.MX8MP with a HDMI USB grabber.
> 
> [1] https://patchwork.kernel.org/project/linux-phy/cover/20240904233100.114611-1-aford173@gmail.com/
> [2] https://codeberg.org/fschrempf/samsung-hdmi-phy-pll-calculator/src/branch/main/pll.py
> 
> Frieder Schrempf (2):
>    phy: freescale: fsl-samsung-hdmi: Add references for calculating LUT
>      parameters to comment
>    phy: freescale: fsl-samsung-hdmi: Add PLL LUT entries for some
>      non-CEA-861 modes
> 
>   drivers/phy/freescale/phy-fsl-samsung-hdmi.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)

I want to kindly ask if there is any further feedback for this?

Thanks
Frieder

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy