| Message ID | 20240910181544.214797-1-frieder@fris.de |
|---|---|
| Headers | show |
| Series | Extending PLL LUT for i.MX8MP Samsung HDMI PHY | expand |
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.
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
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 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 },
},
};
-------------
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.
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.
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 }, > }, > }; > -------------
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(-)