Message-ID: <CAHCN7xKgpqfRf_2p1z5NyKiOYubbcDA7YE5ML8G56pVmcV+1xA@mail.gmail.com>
Date: Wed, 21 Aug 2024 20:59:05 -0500
From: Adam Ford <aford173@...il.com>
To: Dominique MARTINET <dominique.martinet@...ark-techno.com>
Cc: Frieder Schrempf <frieder.schrempf@...tron.de>, Lucas Stach <l.stach@...gutronix.de>, 
	linux-arm-kernel@...ts.infradead.org, marex@...x.de, 
	Laurent Pinchart <laurent.pinchart@...asonboard.com>, Andrzej Hajda <andrzej.hajda@...el.com>, 
	Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>, 
	Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, 
	Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, 
	Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam <festevam@...il.com>, 
	NXP Linux Team <linux-imx@....com>, Vinod Koul <vkoul@...nel.org>, 
	Kishon Vijay Abraham I <kishon@...nel.org>, Liu Ying <victor.liu@....com>, dri-devel@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org, 
	Makoto Sato <makoto.sato@...ark-techno.com>
Subject: Re: drm/bridge/imx8mp-hdmi-tx: Allow inexact pixel clock frequencies
 (Was: [PATCH V8 10/12] drm/bridge: imx: add bridge wrapper driver for i.MX8MP
 DWC HDMI)

On Wed, Aug 21, 2024 at 7:45 AM Adam Ford <aford173@...il.com> wrote:
>
> On Tue, Aug 20, 2024 at 10:58 PM Dominique MARTINET
> <dominique.martinet@...ark-techno.com> 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
> >
> >

Powered by blists - more mailing lists