On Wed, Aug 21, 2024 at 7:45 AM Adam Ford <aford173@xxxxxxxxx> wrote: > > On Tue, Aug 20, 2024 at 10:58 PM Dominique MARTINET > <dominique.martinet@xxxxxxxxxxxxxxxxx> wrote: > > > > Adam Ford wrote on Tue, Aug 20, 2024 at 09:49:03PM -0500: > > > > > However, this check is a bit overcautious in that it only allows exact > > > > > rate matches. IIRC HDMI allows a rate mismatch of +- 0.5%, so this > > > > > check could be relaxed quite a bit to allow for that. > > > > > > > > I checked with a 1080p display that reports 23 possible modes via EDID. > > > > Out of these 15 are accepted by the driver with the strict check. > > > > > > > > Two more would be accepted with a relaxed check that allows a 0.5% margin. > > > > > > > > For the remaining six modes, the pixel clock would deviate up to ~5% > > > > from what the display expects. Still, if I remove the check altogether, > > > > all 23 modes seem to work just fine. > > > > I can confirm the displays I tested also seem pretty tolerant in > > practice (up to ~3-4% at least), but I agree with Lucas that this isn't > > something we can rely on for a general purpose driver as having examples > > of things being tolerant doesn't mean all hardware will be as flexible.. > > > > > > I'd really like to be able to add more PHY PLL setpoints for displays > > > > that use non-CEA-861 modes. Unfortunately I didn't manage to figure out > > > > the fractional-n PLL parameter calculation so far. > > > > > > > > The i.MX8MP Reference Manual provides formulas to calculate the > > > > parameters based on the register values and I tried to make sense of it > > > > using the existing register values in the driver. But somehow it doesn't > > > > add up for me. > > > > > > > > Lucas, did you already work with the PLL parameters? By any chance, do > > > > you now how the math behind them works? > > > > > > I spent a little time trying to understand it a bit. I created a PMS > > > calculator similar to the one used on the DSI controller, > > > > Great! I'll admit this also flies over my head and I don't have the > > time to study this, so this is much appreciated. > > > > > except that > > > the M seems to be fixed at a value of 62 per the data sheet, so it's > > > more of a PS calculator. > > > > Hmm... PHY_REG2 in the datasheet only lists '0011_1110b - 62' as > > example(?) values, but it doesn't say other values are reserved either, > > I'm not sure what to make of it. > > In the current driver PHY_REG_02 (driver macro) is assigned the first > > field of .pll_div_regs, which goes anywhere from 0x4b to 0x7b -- pretty > > far from 62(0x3e)... > > OK. I will experiment with increasing the range of M from being fixed > at 3e to a range of 3b to 7b to see if my PMS integer calculator can > get more accurate. > > > > > Since other frequencies have been adjusting this main diviser ratio we > > actually tried copying neighboring values and adjusting only that reg 2 > > (so the M if I get this right?), but the end result ended up not > > synchronizing properly every time... We didn't have time to check with a > > scope if the generated signal was ugly or if it just didn't lock > > properly, but the display worked when we just re-used the neighboring > > values without changing anything despite being ~3-4% off, so we took the > > easy way out here and didn't dig much further. > > > > > Anyway, When I run my P-S calculator to generate the 'best rate' I get > > > a value that's usually 0.2% variance from nominal, but I only verified > > > a handful of values: > > > > > > Several which were +0.2% > > > 297600000 vs 297000000 (4k@30) > > > 148800000 vs 148500000 (1080p60) > > > 74400 vs 74200 > > > > > > One value was -0.16% > > > 24800000 vs 25200000 > > > > > > If the M value was a bit more flexible, we might be able to reduce > > > that variance more. > > > > Yes, I think the M value could be more flexible, but that'd require > > checking if it actually works, whereas having slightly off frequencies > > will most likely be OK so I don't think it's worth the effort -- happy > > to stick to what the datasheet describes. > > > > > If / when I get some time, I might play with trying to disable the > > > fractional mode and just use the PMS calculator for simplicity despite > > > the inaccuracy. Maybe we could fall back to using the PMS calculator > > > if the desired frequency isn't in the look-up-table. > > > > That'd be greatly appreciated, I don't have any strong opinion on > > whether that should completely replace the table, or only be used if > > there is no exact match in the table as these are values we know will > > work; but we can definitely test any patch you can throw at us here. > > I can't promise it'll be quick. I am not fully certain I understand > how the whole thing works, but as a rule, I don't generally like look > up tables if they can be calculated dynamically. I updated my PMS calculator, and I randomly selected 5 resolutions and they all returned an exact clock match, so I'll attempt to use the PMS integer clock instead of the fractional one. It'll be a little while longer before I can do anything with it. adam > > adam > > > > > > Cheers, > > -- > > Dominique > > > >