Re: [PATCH v5 1/2] drm/bridge: Add Cadence DSI driver

[Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

On Tue, 30 Jan 2018 09:51:49 +0200
Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org> wrote:

> On 29/01/18 18:38, Boris Brezillon wrote:
> 
> > Note that the TX byte clk should be configured to match the DPI pixel
> > clock, which means we should refuse any config where the variation is
> > too big to be recovered. Anyway, we still don't have a way to configure
> > the PLL rate (which is driving the TX byte clk), so there's not much I
> > can do about that right now.  
> 
> We could have the code to check the allowed difference in place.

Not sure I understand what you mean. I can put a TODO/FIXME comment in
the code to say that, once we'll have the 'PLL generator' in place we'll
need to check that the pixel and TX byte clk are close enough, but I
don't see what code you want me to add? Right now, the check will
always succeed, because I assume the pixel and TX byte clk
match perfectly.

If that's a hard requirement, I'll do it, but I think it's pretty
useless given that I won't be able to test the logic in a real
situation.

> 
> >> Another thing is that the mode->crtc_clock is in kHz, I wonder if that
> >> rounding can cause miscalculations in the above code.  
> > 
> > Do we really have modes exposing pixel clks not aligned on a Khz? I  
> 
> Well, I think the clocks in the logical video modes are aligned, but the
> actual clock from the display controller or the PHY most likely is not
> aligned.

Hm, they probably will be for standard definitions.

> 
> > know the display controller can adjust the timings, but then, the
> > variation caused by the Khz approx should not be that big (999Khz /
> > 10+MHz < 1/10000), and anyway, that's what the DRM framework
> > manipulates...  
> 
> Depends on how strict the HW is about the allowed difference between the
> crtc and the DSI IP. It maybe that that 1 Hz is one too many... But that
> can be avoided by just making sure that the check rounds the values
> properly, or maybe just reduces the allowed range by 1kHz on both ends.

My point is, even the display controller can't adjust the pixel clk
rate at the sub-KHz level because mode->crtc_clock is expressed in KHz,
so why should we bother? Do you want to change the DRM API to pass the
pixel clk frequency in Hz?

> 
> So I don't think it's an issue, but it is something to keep in mind.
> 
>  Tomi
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html