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. adam > > > Cheers, > -- > Dominique > >
