GPU/DRM issue with DSI commands on msm 8916

[Archit Taneja]

On Wednesday 18 April 2018 01:58 PM, Daniel Mack wrote:
> 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.

Sorry, forgot to respond to that in your last mail. I wasn't fully
clear about how we'd do it.

Do you mean that we call clk_round_rate() on the byte and pixel
clocks in dsi_link_clk_enable_6g() after we set the rates?

Archit

> 
> 
> Thanks,
> Daniel
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>