All of lore.kernel.org
 help / color / mirror / Atom feed
* GPU/DRM issue with DSI commands on msm 8916
@ 2018-04-05 14:58 Daniel Mack
  2018-04-06 11:25 ` Archit Taneja
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Mack @ 2018-04-05 14:58 UTC (permalink / raw)
  To: Rob Clark, Archit Taneja, Bjorn Andersson, Jordan Crouse
  Cc: linux-arm-msm, dri-devel

Hi,

I'm having issues with the GPU/DRM drivers on a msm8916 based platform
which is very similar to the DragonBoard 410c. In my setup, a DSI
display is directly connected to the SoC, and the video link is stable.

However, when the host sends DCS commands down to the DSI panel (for
instance to set the backlight brightness), the image drops out, most of
the time only in terms of a short flicker, but sometimes it will
completely kill the image. In the latter case, only restarting the
Wayland compositor in userspace helps. This is also quite reproducible;
sending a NOP command once a second would give a visual flicker in 90%
of the cases, and it needs at most a minute to make the screen turn black.

The interesting thing is that this used to work in a v4.9 based version,
but it broke somewhere on the way to v4.14. Unfortunately, the platform
does not boot a vanilla kernel, so I can't really bisect this. We
currently depend on the Linaro downstream patches which can be found here:


http://git.linaro.org/landing-teams/working/qualcomm/kernel.git/log/?h=release/qcomlt-4.9

http://git.linaro.org/landing-teams/working/qualcomm/kernel.git/log/?h=release/qcomlt-4.14

I've looked at the development that has happened in the area for some
time now, but I can't really pin-point any specific commit. Also, I
cherry-picked most of the patches to these drivers that came in after
v4.14, but that didn't help either.

Has this has been observed before? A pointer what to investigate on
would be very much appreciated. If there is any more information I can
provide, please let me know.


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: GPU/DRM issue with DSI commands on msm 8916
  2018-04-05 14:58 GPU/DRM issue with DSI commands on msm 8916 Daniel Mack
@ 2018-04-06 11:25 ` Archit Taneja
  2018-04-09 10:58   ` Daniel Mack
  0 siblings, 1 reply; 10+ messages in thread
From: Archit Taneja @ 2018-04-06 11:25 UTC (permalink / raw)
  To: Daniel Mack, Rob Clark, Bjorn Andersson, Jordan Crouse
  Cc: linux-arm-msm, dri-devel

Hi,

On Thursday 05 April 2018 08:28 PM, Daniel Mack wrote:
> Hi,
> 
> I'm having issues with the GPU/DRM drivers on a msm8916 based platform
> which is very similar to the DragonBoard 410c. In my setup, a DSI
> display is directly connected to the SoC, and the video link is stable.
> 
> However, when the host sends DCS commands down to the DSI panel (for
> instance to set the backlight brightness), the image drops out, most of
> the time only in terms of a short flicker, but sometimes it will
> completely kill the image. In the latter case, only restarting the
> Wayland compositor in userspace helps. This is also quite reproducible;
> sending a NOP command once a second would give a visual flicker in 90%
> of the cases, and it needs at most a minute to make the screen turn black.
> 
> The interesting thing is that this used to work in a v4.9 based version,
> but it broke somewhere on the way to v4.14. Unfortunately, the platform
> does not boot a vanilla kernel, so I can't really bisect this. We
> currently depend on the Linaro downstream patches which can be found here:

The major change that happened between qcomlt-4.9 and qcomlt-4.14 from a 
DSI point of view was probably the addition of runtime PM support.

The register configurations that are responsible for interleaving DCS
commands while video mode is still on should be the same.

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().

Does your device initialize a splash screen in the bootloader?

You could also compare the reg dumps between 4.9 and 4.14 by enabling
the config CONFIG_DRM_MSM_REGISTER_LOGGING and check if there are
any register configuration differences between the two.

One (rather unlikely) possibility I can think of is if somehow the
buffers used to send/receive DCS commands aren't mapped/unmapped
correctly. There have been some msm_gem changes, and the IOMMU driver
is new. That's the main reason why I'm wondering if the contents of the
DCS buffers somehow got corrupt. Is the panel initialized using DCS
commands too?

Thanks,
Archit

> 
> 
> http://git.linaro.org/landing-teams/working/qualcomm/kernel.git/log/?h=release/qcomlt-4.9
> 
> http://git.linaro.org/landing-teams/working/qualcomm/kernel.git/log/?h=release/qcomlt-4.14
> 
> I've looked at the development that has happened in the area for some
> time now, but I can't really pin-point any specific commit. Also, I
> cherry-picked most of the patches to these drivers that came in after
> v4.14, but that didn't help either.
> 
> Has this has been observed before? A pointer what to investigate on
> would be very much appreciated. If there is any more information I can
> provide, please let me know.
> 
> 
> Thanks,
> Daniel
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: GPU/DRM issue with DSI commands on msm 8916
  2018-04-06 11:25 ` Archit Taneja
@ 2018-04-09 10:58   ` Daniel Mack
  2018-04-09 13:08     ` Archit Taneja
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Mack @ 2018-04-09 10:58 UTC (permalink / raw)
  To: Archit Taneja, Rob Clark, Bjorn Andersson, Jordan Crouse
  Cc: linux-arm-msm, dri-devel

Hi Archit,

Thanks a lot for your reply.

On Friday, April 06, 2018 01:25 PM, Archit Taneja wrote:
> On Thursday 05 April 2018 08:28 PM, Daniel Mack wrote:
>> I'm having issues with the GPU/DRM drivers on a msm8916 based platform
>> which is very similar to the DragonBoard 410c. In my setup, a DSI
>> display is directly connected to the SoC, and the video link is stable.
>>
>> However, when the host sends DCS commands down to the DSI panel (for
>> instance to set the backlight brightness), the image drops out, most of
>> the time only in terms of a short flicker, but sometimes it will
>> completely kill the image. In the latter case, only restarting the
>> Wayland compositor in userspace helps. This is also quite reproducible;
>> sending a NOP command once a second would give a visual flicker in 90%
>> of the cases, and it needs at most a minute to make the screen turn black.
>>
>> The interesting thing is that this used to work in a v4.9 based version,
>> but it broke somewhere on the way to v4.14. Unfortunately, the platform
>> does not boot a vanilla kernel, so I can't really bisect this. We
>> currently depend on the Linaro downstream patches which can be found here:
> 
> The major change that happened between qcomlt-4.9 and qcomlt-4.14 from a 
> DSI point of view was probably the addition of runtime PM support.
> 
> The register configurations that are responsible for interleaving DCS
> commands while video mode is still on should be the same.

Yeah, I think so too. I compared a lot of code but couldn't really find
anything either. At least, the command buffer contents and lengths are
identical.

> 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.

> Does your device initialize a splash screen in the bootloader?

It does, but that's the case for either of the two kernels. Do you think
that matters? And as you mention it - I'm building the driver as module,
because when built into the kernel, the msm driver fails to initialize
the hardware, and the console is flooded with the following message:

[   63.356837] dsi_err_worker: status=4

> You could also compare the reg dumps between 4.9 and 4.14 by enabling
> the config CONFIG_DRM_MSM_REGISTER_LOGGING and check if there are
> any register configuration differences between the two.

I did that, and there a quite a number of changes, mostly because the
KMS bits have changed a lot. Given that I'm not too familiar with this
driver stack, I'm not sure what exactly to look at.

> One (rather unlikely) possibility I can think of is if somehow the
> buffers used to send/receive DCS commands aren't mapped/unmapped
> correctly. There have been some msm_gem changes, and the IOMMU driver
> is new. That's the main reason why I'm wondering if the contents of the
> DCS buffers somehow got corrupt.

That may well be, but I can't really see what's wrong in that area.
Which iommu driver are you referring to, exactly?

> Is the panel initialized using DCS
> commands too?

Yes, and that works. The thing is that the commands do in fact reach the
panel and cause the desired effect, it's just that as a side effect, the
display very likely drops out when a command is sent. The registers that
are modified through msm_writel() by and between
msm_dsi_host_xfer_prepare() and msm_dsi_host_xfer_restore() are exactly
the same though. So it must be that some other part (the GPU or the
KMS?) doesn't like the fact that the DSI core mangles with the hardware
state in some way.

What hardware are all these changes developed and tested on, btw? I
guess it might be worth looking into differences between these platforms
and my own.


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: GPU/DRM issue with DSI commands on msm 8916
  2018-04-09 10:58   ` Daniel Mack
@ 2018-04-09 13:08     ` Archit Taneja
  2018-04-16 17:06       ` Daniel Mack
  0 siblings, 1 reply; 10+ messages in thread
From: Archit Taneja @ 2018-04-09 13:08 UTC (permalink / raw)
  To: Daniel Mack, Rob Clark, Bjorn Andersson, Jordan Crouse
  Cc: linux-arm-msm, dri-devel



On Monday 09 April 2018 04:28 PM, Daniel Mack wrote:
> Hi Archit,
> 
> Thanks a lot for your reply.
> 
> On Friday, April 06, 2018 01:25 PM, Archit Taneja wrote:
>> On Thursday 05 April 2018 08:28 PM, Daniel Mack wrote:
>>> I'm having issues with the GPU/DRM drivers on a msm8916 based platform
>>> which is very similar to the DragonBoard 410c. In my setup, a DSI
>>> display is directly connected to the SoC, and the video link is stable.
>>>
>>> However, when the host sends DCS commands down to the DSI panel (for
>>> instance to set the backlight brightness), the image drops out, most of
>>> the time only in terms of a short flicker, but sometimes it will
>>> completely kill the image. In the latter case, only restarting the
>>> Wayland compositor in userspace helps. This is also quite reproducible;
>>> sending a NOP command once a second would give a visual flicker in 90%
>>> of the cases, and it needs at most a minute to make the screen turn black.
>>>
>>> The interesting thing is that this used to work in a v4.9 based version,
>>> but it broke somewhere on the way to v4.14. Unfortunately, the platform
>>> does not boot a vanilla kernel, so I can't really bisect this. We
>>> currently depend on the Linaro downstream patches which can be found here:
>>
>> The major change that happened between qcomlt-4.9 and qcomlt-4.14 from a
>> DSI point of view was probably the addition of runtime PM support.
>>
>> The register configurations that are responsible for interleaving DCS
>> commands while video mode is still on should be the same.
> 
> Yeah, I think so too. I compared a lot of code but couldn't really find
> anything either. At least, the command buffer contents and lengths are
> identical.
> 
>> 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.

> 
>> Does your device initialize a splash screen in the bootloader?
> 
> It does, but that's the case for either of the two kernels. Do you think
> that matters? And as you mention it - I'm building the driver as module,
> because when built into the kernel, the msm driver fails to initialize
> the hardware, and the console is flooded with the following message:

The bootloader may have configured some additional registers that the
DSI driver in the kernel doesn't touch. On 4.9, these registers would
have retained their state since we don't do any PM stuff. On 4.14, these
registers would probably lose their state, which results in a difference
in behavior. This can be avoided by commenting all the put_sync() calls
as mentioned above.

> 
> [   63.356837] dsi_err_worker: status=4
> 
>> You could also compare the reg dumps between 4.9 and 4.14 by enabling
>> the config CONFIG_DRM_MSM_REGISTER_LOGGING and check if there are
>> any register configuration differences between the two.
> 
> I did that, and there a quite a number of changes, mostly because the
> KMS bits have changed a lot. Given that I'm not too familiar with this
> driver stack, I'm not sure what exactly to look at.

Mostly to check if there were any changes in the DSI register writes,
and maybe writes to the MDP5 INTF blocks.

> 
>> One (rather unlikely) possibility I can think of is if somehow the
>> buffers used to send/receive DCS commands aren't mapped/unmapped
>> correctly. There have been some msm_gem changes, and the IOMMU driver
>> is new. That's the main reason why I'm wondering if the contents of the
>> DCS buffers somehow got corrupt.
> 
> That may well be, but I can't really see what's wrong in that area.
> Which iommu driver are you referring to, exactly?

On 4.9, this driver is used:
drivers/iommu/qcom

On 4.14 we use:
drivers/iommu/qcom_iommu.c
> 
>> Is the panel initialized using DCS
>> commands too?
> 
> Yes, and that works. The thing is that the commands do in fact reach the
> panel and cause the desired effect, it's just that as a side effect, the
> display very likely drops out when a command is sent. The registers that
> are modified through msm_writel() by and between
> msm_dsi_host_xfer_prepare() and msm_dsi_host_xfer_restore() are exactly
> the same though. So it must be that some other part (the GPU or the
> KMS?) doesn't like the fact that the DSI core mangles with the hardware
> state in some way.

It seems unlikely that sending a DCS command via DSI core would affect
MDP or the GPU. It feels like sending the DCS commands is causing the
DSI lanes to be temporarily in an invalid state, resulting in the
flicker.

> 
> What hardware are all these changes developed and tested on, btw? I
> guess it might be worth looking into differences between these platforms
> and my own.

The releases are mostly tested on DB600c, DB410c and DB820c. On DB410c,
DSI is mostly tested using the hdmi bridge output, and the splash screen
isn't enabled in the bootloader (LK),

Thanks,
Archit

> 
> 
> Again, thanks!
> Daniel
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: GPU/DRM issue with DSI commands on msm 8916
  2018-04-09 13:08     ` Archit Taneja
@ 2018-04-16 17:06       ` Daniel Mack
  2018-04-17 12:21         ` Daniel Mack
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Mack @ 2018-04-16 17:06 UTC (permalink / raw)
  To: Archit Taneja, Rob Clark, Bjorn Andersson, Jordan Crouse
  Cc: linux-arm-msm, dri-devel

Hi Archit,

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.

That rules out all other components such as GPU and IOMMU, but I still
don't grok what's going on, because I can't see a big difference in the
relevant clock functions in the dsi driver and the clock drivers between
4.9 and 4.14.

Any idea? I'll do some more debugging tomorrow.


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: GPU/DRM issue with DSI commands on msm 8916
  2018-04-16 17:06       ` Daniel Mack
@ 2018-04-17 12:21         ` Daniel Mack
  2018-04-18  8:06           ` Archit Taneja
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Mack @ 2018-04-17 12:21 UTC (permalink / raw)
  To: Archit Taneja, Rob Clark, Bjorn Andersson, Jordan Crouse
  Cc: linux-arm-msm, Stephen Boyd, dri-devel

(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.

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.

The same problem applies to other clocks too. dsi0vco_clk for example
will be 449970703 rather than the requested 450000000 etc.

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.


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: GPU/DRM issue with DSI commands on msm 8916
  2018-04-17 12:21         ` Daniel Mack
@ 2018-04-18  8:06           ` Archit Taneja
  2018-04-18  8:28             ` Daniel Mack
  0 siblings, 1 reply; 10+ messages in thread
From: Archit Taneja @ 2018-04-18  8:06 UTC (permalink / raw)
  To: Daniel Mack, Rob Clark, Bjorn Andersson, Jordan Crouse
  Cc: linux-arm-msm, Stephen Boyd, dri-devel, swboyd

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
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: GPU/DRM issue with DSI commands on msm 8916
  2018-04-18  8:06           ` Archit Taneja
@ 2018-04-18  8:28             ` Daniel Mack
  2018-04-18  8:53               ` Archit Taneja
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Mack @ 2018-04-18  8:28 UTC (permalink / raw)
  To: Archit Taneja, Rob Clark, Bjorn Andersson, Jordan Crouse
  Cc: linux-arm-msm, Stephen Boyd, dri-devel, swboyd

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: GPU/DRM issue with DSI commands on msm 8916
  2018-04-18  8:28             ` Daniel Mack
@ 2018-04-18  8:53               ` Archit Taneja
  2018-04-18  9:05                 ` Daniel Mack
  0 siblings, 1 reply; 10+ messages in thread
From: Archit Taneja @ 2018-04-18  8:53 UTC (permalink / raw)
  To: Daniel Mack, Rob Clark, Bjorn Andersson, Jordan Crouse
  Cc: linux-arm-msm, Stephen Boyd, dri-devel, swboyd



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@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: GPU/DRM issue with DSI commands on msm 8916
  2018-04-18  8:53               ` Archit Taneja
@ 2018-04-18  9:05                 ` Daniel Mack
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Mack @ 2018-04-18  9:05 UTC (permalink / raw)
  To: Archit Taneja, Rob Clark, Bjorn Andersson, Jordan Crouse
  Cc: linux-arm-msm, Stephen Boyd, dri-devel, swboyd

On Wednesday, April 18, 2018 10:53 AM, Archit Taneja wrote:
> On Wednesday 18 April 2018 01:58 PM, Daniel Mack wrote:
>> On Wednesday, April 18, 2018 10:06 AM, Archit Taneja wrote:

>>> 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?

No, before. AFAIU, the clk core calls into the clock provider's
.round_rate() callback if it exists to determine the exact rate that it
is about to set. With this, the driver can return a rate that it
actually supports.

The MSM PLL driver currently only clamps the values in that callback,
but it could be smarter than that and return the closest rate to the
desired rate they can actually generate. The calculations based on the
various registers in dsi_pll_{14,28}nm_clk_recalc_rate() beat me though,
so it's not immediately clear how to get the math right to implement
that properly.


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-04-18  9:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-05 14:58 GPU/DRM issue with DSI commands on msm 8916 Daniel Mack
2018-04-06 11:25 ` Archit Taneja
2018-04-09 10:58   ` Daniel Mack
2018-04-09 13:08     ` Archit Taneja
2018-04-16 17:06       ` Daniel Mack
2018-04-17 12:21         ` Daniel Mack
2018-04-18  8:06           ` Archit Taneja
2018-04-18  8:28             ` Daniel Mack
2018-04-18  8:53               ` Archit Taneja
2018-04-18  9:05                 ` Daniel Mack

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.