Message-ID: <CAHCN7xJnjfjr7HfKF+4pwbENP+p2=vvMXWW1AQShNy87vfQ=-A@mail.gmail.com>
Date: Wed, 21 Aug 2024 07:45:58 -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 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.

adam
>
>
> Cheers,
> --
> Dominique
>
>

Powered by blists - more mailing lists