Hi Daniel,
On Tuesday 17 April 2018 05:51 PM, Daniel Mack wrote:
(cc Stephen)
Hi Archit,
On Monday, April 16, 2018 07:06 PM, Daniel Mack wrote:
On Monday, April 09, 2018 03:08 PM, Archit Taneja wrote:
You could comment out the pm_runtime_put_sync() calls in
drivers/gpu/drm/msm/dsi/dsi_host.c, in case some registers got
reset during put_sync and weren't restored correctly after get_sync().
That was my first suspicion too, but unfortunately, that's not the culprit.
I think it would be good to comment out the put_sync() calls in
drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c and
drivers/gpu/drm/msm/msm_drv.c too, since there is a parent-child
hierarchy between DSI
and the top level MDSS block. Commenting out the put_syncs() just
in put_sync() might still result in the MDSS GDSC to switch off.
I spent some more time debugging this today and it turns out that
calling into dsi_link_clk_enable() from msm_dsi_host_xfer_prepare() is
already causing the drop-outs, even when no command buffers, DMA
transfers etc. are involved. I then drilled further down and it showed
that at least
clk_set_rate(msm_host->byte_clk, msm_host->byte_clk_rate);
in dsi_link_clk_enable_6g() one of the culprits. If I don't touch the
clocks anymore after the initialization is done, everything is fine.
Okay, I finally found the bits between the two trees that make the
difference. It's a downstream patch that is included in the Linaro 4.9
branch but that's missing upstream and in their 4.14 branch:
https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=release/qcomlt-4.9&id=4ce2d6108d
What this patch does as a side-effect is that is sets the clock's actual
rate to the requested rate (core->rate = core->new_rate), and that fixes
the problem I'm seeing.
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?
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.
It would hide the real problem, though.
I guess a way to fix this properly would be to use
msm_dsi_pll_helper_clk_round_rate() to actually round the rates, but I'm
not sure.
Stephen,
Do you have any suggestions on how we can manage this? I.e we set a
clock rate, it goes through, but it's very slightly different than what
we asked for. The next time we try to set the same rate, the clock
provider driver goes through the whole ordeal of reconfiguring the
HW to reach to the same configuration.
Thanks,
Archit
Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
