Re: GPU/DRM issue with DSI commands on msm 8916

From: Daniel Mack <daniel@zonque.org>
To: Archit Taneja <architt@codeaurora.org>,
	Rob Clark <robdclark@gmail.com>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Jordan Crouse <jcrouse@codeaurora.org>
Cc: linux-arm-msm@vger.kernel.org,
	Stephen Boyd <sboyd@codeaurora.org>,
	dri-devel@lists.freedesktop.org, swboyd@chromium.org
Subject: Re: GPU/DRM issue with DSI commands on msm 8916
Date: Wed, 18 Apr 2018 10:28:24 +0200	[thread overview]
Message-ID: <1e6954b5-92cd-c7ec-95fb-cef290a8ce0e@zonque.org> (raw)
In-Reply-To: <7f678217-53ac-d5cd-8844-8c20aa4dcea5@codeaurora.org>

On Wednesday, April 18, 2018 10:06 AM, Archit Taneja wrote:
> On Tuesday 17 April 2018 05:51 PM, Daniel Mack wrote:
> Thanks for debugging this so thoroughly.
> 
>> It shows an underlying problem in the msm driver's clock components
>> though, because without this patch, the clocks will be effectively
>> slightly off from what was requested. For instance, when
>> gcc_mdss_byte0_clk is configured to 56250000 Hz by the msm drm driver,
>> the clk core will let the DSI PLL recalculate its actual rate which is
>> 56246337 Hz. Hence, clk_set_rate() will always end up fiddling with the
>> knobs of that clock provider. That's what happens every time DSI
>> commands are sent, and that causes the image to flicker.
> 
> If I understood right, the main cause of the flicker is that we always
> end up re-locking/reconfiguring the DSI PLL every time we send a command
> (since dsi_link_clk_enable() is called in msm_dsi_host_xfer_prepare())?
> The re-configuration results in a glitch on the DSI clock, which is
> probably unacceptable for DSI Video mode transfer, especially for panels
> that don't have their own timing generators, which rely entirely on
> DSI clock lanes for scanning out the pixel data.
> 
> According to you, the reason why the reconfiguration happens is because
> the DSI PLL was never set exactly to 56.25 Mhz in the first place, and
> the core clock framework notices a difference in the requested rate and 
> the current rate (56.246 Mhz), and goes ahead to configure the PLL
> again when it's not needed. And this was averted in the downstream
> patch you mentioned as a side affect?

Yes, exactly.

>> The same problem applies to other clocks too. dsi0vco_clk for example
>> will be 449970703 rather than the requested 450000000 etc.
>>
> 
> One easy way to get around this would be to not try to set the clock
> rates every time we try to send a command. We just enable/disable them.

Yes, that could work as well, but how about rounding the rates in the
callback that exists for that purpose? We're off by a fraction of a
permille only, after all.

Thanks,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel