Re: [Freedreno] [PATCH v6 20/21] drm/kirin: dsi: Adjust probe order

2021-10-27 Thread John Stultz
On Mon, Oct 25, 2021 at 8:16 AM Maxime Ripard  wrote:
>
> Without proper care and an agreement between how DSI hosts and devices
> drivers register their MIPI-DSI entities and potential components, we can
> end up in a situation where the drivers can never probe.
>
> Most drivers were taking evasive maneuvers to try to workaround this,
> but not all of them were following the same conventions, resulting in
> various incompatibilities between DSI hosts and devices.
>
> Now that we have a sequence agreed upon and documented, let's convert
> kirin to it.
>
> Tested-by: John Stultz 
> Signed-off-by: Maxime Ripard 

For this patch, and any others in this series folks see fit:
   Acked-by: John Stultz 

thanks
-john


Re: [Freedreno] [PATCH] drm/msm/devfreq: Restrict idle clamping to a618 for now

2021-10-18 Thread John Stultz
On Mon, Oct 18, 2021 at 8:31 AM Rob Clark  wrote:
>
> From: Rob Clark 
>
> Until we better understand the stability issues caused by frequent
> frequency changes, lets limit them to a618.
>
> Signed-off-by: Rob Clark 
> ---
> Caleb/John, I think this should help as a workaround for the power
> instability issues on a630.. could you give it a try?

While I hit it fairly often, I can't reliably reproduce the crash, but
in limited testing this seems ok to me.
I've not hit the crash so far, nor seen any other negative side
effects over 5.14.

So for what that's worth:
Tested-by: John Stultz 

Caleb has better luck tripping this issue right away, so they can
hopefully provide a more assured response.

thanks
-john


Re: [Freedreno] [PATCH v2 2/2] qcom_scm: hide Kconfig symbol

2021-10-11 Thread John Stultz
On Thu, Oct 7, 2021 at 8:10 AM Arnd Bergmann  wrote:
>
> From: Arnd Bergmann 
>
> Now that SCM can be a loadable module, we have to add another
> dependency to avoid link failures when ipa or adreno-gpu are
> built-in:
>
> aarch64-linux-ld: drivers/net/ipa/ipa_main.o: in function `ipa_probe':
> ipa_main.c:(.text+0xfc4): undefined reference to `qcom_scm_is_available'
>
> ld.lld: error: undefined symbol: qcom_scm_is_available
> >>> referenced by adreno_gpu.c
> >>>   gpu/drm/msm/adreno/adreno_gpu.o:(adreno_zap_shader_load) in 
> >>> archive drivers/built-in.a
>
> This can happen when CONFIG_ARCH_QCOM is disabled and we don't select
> QCOM_MDT_LOADER, but some other module selects QCOM_SCM. Ideally we'd
> use a similar dependency here to what we have for QCOM_RPROC_COMMON,
> but that causes dependency loops from other things selecting QCOM_SCM.
>
> This appears to be an endless problem, so try something different this
> time:
>
>  - CONFIG_QCOM_SCM becomes a hidden symbol that nothing 'depends on'
>but that is simply selected by all of its users
>
>  - All the stubs in include/linux/qcom_scm.h can go away
>
>  - arm-smccc.h needs to provide a stub for __arm_smccc_smc() to
>allow compile-testing QCOM_SCM on all architectures.
>
>  - To avoid a circular dependency chain involving RESET_CONTROLLER
>and PINCTRL_SUNXI, drop the 'select RESET_CONTROLLER' statement.
>According to my testing this still builds fine, and the QCOM
>platform selects this symbol already.
>
> Acked-by: Kalle Valo 
> Acked-by: Alex Elder 
> Signed-off-by: Arnd Bergmann 
> ---
> Changes in v2:
> - fix the iommu dependencies

Hey Arnd,
   Thanks again so much for working out these details. Also my
apologies, as Bjorn asked for me to test this patch, but I wasn't able
to get to it before it landed.  Unfortunately I've hit an issue that
is keeping the db845c from booting with this.

> diff --git a/drivers/iommu/arm/arm-smmu/Makefile 
> b/drivers/iommu/arm/arm-smmu/Makefile
> index e240a7bcf310..b0cc01aa20c9 100644
> --- a/drivers/iommu/arm/arm-smmu/Makefile
> +++ b/drivers/iommu/arm/arm-smmu/Makefile
> @@ -1,4 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o
>  obj-$(CONFIG_ARM_SMMU) += arm_smmu.o
> -arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o arm-smmu-qcom.o
> +arm_smmu-objs += arm-smmu.o arm-smmu-impl.o arm-smmu-nvidia.o
> +arm_smmu-$(CONFIG_ARM_SMMU_QCOM) += arm-smmu-qcom.o
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c 
> b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> index 9f465e146799..2c25cce38060 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> @@ -215,7 +215,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct 
> arm_smmu_device *smmu)
> of_device_is_compatible(np, "nvidia,tegra186-smmu"))
> return nvidia_smmu_impl_init(smmu);
>
> -   smmu = qcom_smmu_impl_init(smmu);
> +   if (IS_ENABLED(CONFIG_ARM_SMMU_QCOM))
> +   smmu = qcom_smmu_impl_init(smmu);
>
> if (of_device_is_compatible(np, "marvell,ap806-smmu-500"))
> smmu->impl = _mmu500_impl;


The problem with these two chunks is that there is currently no
CONFIG_ARM_SMMU_QCOM option. :)

Was that something you intended to add in the patch?

I'm working up a Kconfig patch to do so, so I'll send that out in a
second here, but let me know if you already have that somewhere (I
suspect you implemented it and just forgot to add the change to the
commit), as I'm sure your Kconfig help text will be better than mine.
:)

Again, I'm so sorry I didn't get over to testing your patch before
seeing this here!

thanks
-john


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Wed, Sep 29, 2021 at 4:20 PM Rob Clark  wrote:
> On Wed, Sep 29, 2021 at 2:51 PM John Stultz  wrote:
> > On Wed, Sep 29, 2021 at 2:32 PM John Stultz  wrote:
> > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz  
> > > wrote:
> > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> > > > > The best practice to avoid those issues is to register its functions 
> > > > > only after
> > > > > all its dependencies are live. We also shouldn't wait any longer than 
> > > > > we should
> > > > > to play nice with the other components that are waiting for us, so in 
> > > > > our case
> > > > > that would mean moving the DSI device registration to the bridge 
> > > > > probe.
> > > > >
> > > > > I also had a look at all the DSI hosts, and it seems that exynos, 
> > > > > kirin and msm
> > > > > would be affected by this and wouldn't probe anymore after those 
> > > > > changes.
> > > > > Exynos and kirin seems to be simple enough for a mechanical change 
> > > > > (that still
> > > > > requires to be tested), but the changes in msm seemed to be far more 
> > > > > important
> > > > > and I wasn't confortable doing them.
> > > >
> > > >
> > > > Hey Maxime,
> > > >   Sorry for taking so long to get to this, but now that plumbers is
> > > > over I've had a chance to check it out on kirin
> > > >
> > > > Rob Clark pointed me to his branch with some fixups here:
> > > >
> > > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > >
> > > > But trying to boot hikey with that, I see the following loop 
> > > > indefinitely:
> > > > [4.632132] adv7511 2-0039: supply avdd not found, using dummy 
> > > > regulator
> > > > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy 
> > > > regulator
> > > > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy 
> > > > regulator
> > > > [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy 
> > > > regulator
> > > > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy 
> > > > regulator
> > > > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy 
> > > > regulator
> > > > [4.681898] adv7511 2-0039: failed to find dsi host
> > >
> > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > I'll retest and let you know.
> >
> > Ok, just retested including the kirin patch and unfortunately I'm
> > still seeing the same thing.  :(
> >
> > Will dig a bit and let you know when I find more.
>
> Did you have a chance to test it on anything using drm/msm with DSI
> panels?  That would at least confirm that I didn't miss anything in
> the drm/msm patch to swap the dsi-host vs bridge ordering..

Not a dsi panel, but for what it's worth, I did just test the series
with the db845c w/ HDMI using the lt9611 bridge.
So at least that is looking ok.

thanks
-john


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Wed, Sep 29, 2021 at 2:51 PM John Stultz  wrote:
>
> On Wed, Sep 29, 2021 at 2:32 PM John Stultz  wrote:
> > On Wed, Sep 29, 2021 at 2:27 PM John Stultz  wrote:
> > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> > > > The best practice to avoid those issues is to register its functions 
> > > > only after
> > > > all its dependencies are live. We also shouldn't wait any longer than 
> > > > we should
> > > > to play nice with the other components that are waiting for us, so in 
> > > > our case
> > > > that would mean moving the DSI device registration to the bridge probe.
> > > >
> > > > I also had a look at all the DSI hosts, and it seems that exynos, kirin 
> > > > and msm
> > > > would be affected by this and wouldn't probe anymore after those 
> > > > changes.
> > > > Exynos and kirin seems to be simple enough for a mechanical change 
> > > > (that still
> > > > requires to be tested), but the changes in msm seemed to be far more 
> > > > important
> > > > and I wasn't confortable doing them.
> > >
> > >
> > > Hey Maxime,
> > >   Sorry for taking so long to get to this, but now that plumbers is
> > > over I've had a chance to check it out on kirin
> > >
> > > Rob Clark pointed me to his branch with some fixups here:
> > >
> > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > >
> > > But trying to boot hikey with that, I see the following loop indefinitely:
> > > [4.632132] adv7511 2-0039: supply avdd not found, using dummy 
> > > regulator
> > > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy 
> > > regulator
> > > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy 
> > > regulator
> > > [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy 
> > > regulator
> > > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy 
> > > regulator
> > > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy 
> > > regulator
> > > [4.681898] adv7511 2-0039: failed to find dsi host
> >
> > I just realized Rob's tree is missing the kirin patch. My apologies!
> > I'll retest and let you know.
>
> Ok, just retested including the kirin patch and unfortunately I'm
> still seeing the same thing.  :(
>
> Will dig a bit and let you know when I find more.

Hey Maxime!
  I chased down the issue. The dsi probe code was still calling
drm_of_find_panel_or_bridge() in order to succeed.

I've moved the logic that looks for the bridge into the bridge_init
and with that it seems to work.

Feel free (assuming it looks ok) to fold this change into your kirin patch:
  
https://git.linaro.org/people/john.stultz/android-dev.git/commit/?id=4a35ccc4d7a53f68d6d93da3b47e232a7c75b91d

thanks
-john


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Wed, Sep 29, 2021 at 4:20 PM Rob Clark  wrote:
> On Wed, Sep 29, 2021 at 2:51 PM John Stultz  wrote:
> > On Wed, Sep 29, 2021 at 2:32 PM John Stultz  wrote:
> > > On Wed, Sep 29, 2021 at 2:27 PM John Stultz  
> > > wrote:
> > > > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> > > > > The best practice to avoid those issues is to register its functions 
> > > > > only after
> > > > > all its dependencies are live. We also shouldn't wait any longer than 
> > > > > we should
> > > > > to play nice with the other components that are waiting for us, so in 
> > > > > our case
> > > > > that would mean moving the DSI device registration to the bridge 
> > > > > probe.
> > > > >
> > > > > I also had a look at all the DSI hosts, and it seems that exynos, 
> > > > > kirin and msm
> > > > > would be affected by this and wouldn't probe anymore after those 
> > > > > changes.
> > > > > Exynos and kirin seems to be simple enough for a mechanical change 
> > > > > (that still
> > > > > requires to be tested), but the changes in msm seemed to be far more 
> > > > > important
> > > > > and I wasn't confortable doing them.
> > > >
> > > >
> > > > Hey Maxime,
> > > >   Sorry for taking so long to get to this, but now that plumbers is
> > > > over I've had a chance to check it out on kirin
> > > >
> > > > Rob Clark pointed me to his branch with some fixups here:
> > > >
> > > > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> > > >
> > > > But trying to boot hikey with that, I see the following loop 
> > > > indefinitely:
> > > > [4.632132] adv7511 2-0039: supply avdd not found, using dummy 
> > > > regulator
> > > > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy 
> > > > regulator
> > > > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy 
> > > > regulator
> > > > [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy 
> > > > regulator
> > > > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy 
> > > > regulator
> > > > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy 
> > > > regulator
> > > > [4.681898] adv7511 2-0039: failed to find dsi host
> > >
> > > I just realized Rob's tree is missing the kirin patch. My apologies!
> > > I'll retest and let you know.
> >
> > Ok, just retested including the kirin patch and unfortunately I'm
> > still seeing the same thing.  :(
> >
> > Will dig a bit and let you know when I find more.
>
> Did you have a chance to test it on anything using drm/msm with DSI
> panels?  That would at least confirm that I didn't miss anything in
> the drm/msm patch to swap the dsi-host vs bridge ordering..

I believe Amit(cc'ed) tried to give it a run on his pocof1, but had
some troubles getting it working against a kernel that wasn't
suffering other regressions at the moment.

Amit/Caleb: Any chance one of you could try again w/ these merged to 5.15-rc3?

thanks
-john


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Wed, Sep 29, 2021 at 2:32 PM John Stultz  wrote:
> On Wed, Sep 29, 2021 at 2:27 PM John Stultz  wrote:
> > On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> > > The best practice to avoid those issues is to register its functions only 
> > > after
> > > all its dependencies are live. We also shouldn't wait any longer than we 
> > > should
> > > to play nice with the other components that are waiting for us, so in our 
> > > case
> > > that would mean moving the DSI device registration to the bridge probe.
> > >
> > > I also had a look at all the DSI hosts, and it seems that exynos, kirin 
> > > and msm
> > > would be affected by this and wouldn't probe anymore after those changes.
> > > Exynos and kirin seems to be simple enough for a mechanical change (that 
> > > still
> > > requires to be tested), but the changes in msm seemed to be far more 
> > > important
> > > and I wasn't confortable doing them.
> >
> >
> > Hey Maxime,
> >   Sorry for taking so long to get to this, but now that plumbers is
> > over I've had a chance to check it out on kirin
> >
> > Rob Clark pointed me to his branch with some fixups here:
> >
> > https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
> >
> > But trying to boot hikey with that, I see the following loop indefinitely:
> > [4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> > [4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> > [4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> > [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> > [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> > [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> > [4.681898] adv7511 2-0039: failed to find dsi host
>
> I just realized Rob's tree is missing the kirin patch. My apologies!
> I'll retest and let you know.

Ok, just retested including the kirin patch and unfortunately I'm
still seeing the same thing.  :(

Will dig a bit and let you know when I find more.

thanks
-john


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Wed, Sep 29, 2021 at 2:27 PM John Stultz  wrote:
>
> On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
> >
> > We've encountered an issue with the RaspberryPi DSI panel that prevented the
> > whole display driver from probing.
> >
> > The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
> > Only register our component once a DSI device is attached"), but the basic 
> > idea
> > is that since the panel is probed through i2c, there's no synchronization
> > between its probe and the registration of the MIPI-DSI host it's attached 
> > to.
> >
> > We initially moved the component framework registration to the MIPI-DSI Host
> > attach hook to make sure we register our component only when we have a DSI
> > device attached to our MIPI-DSI host, and then use lookup our DSI device in 
> > our
> > bind hook.
> >
> > However, all the DSI bridges controlled through i2c are only registering 
> > their
> > associated DSI device in their bridge attach hook, meaning with our change
> > above, we never got that far, and therefore ended up in the same situation 
> > than
> > the one we were trying to fix for panels.
> >
> > The best practice to avoid those issues is to register its functions only 
> > after
> > all its dependencies are live. We also shouldn't wait any longer than we 
> > should
> > to play nice with the other components that are waiting for us, so in our 
> > case
> > that would mean moving the DSI device registration to the bridge probe.
> >
> > I also had a look at all the DSI hosts, and it seems that exynos, kirin and 
> > msm
> > would be affected by this and wouldn't probe anymore after those changes.
> > Exynos and kirin seems to be simple enough for a mechanical change (that 
> > still
> > requires to be tested), but the changes in msm seemed to be far more 
> > important
> > and I wasn't confortable doing them.
>
>
> Hey Maxime,
>   Sorry for taking so long to get to this, but now that plumbers is
> over I've had a chance to check it out on kirin
>
> Rob Clark pointed me to his branch with some fixups here:
>
> https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework
>
> But trying to boot hikey with that, I see the following loop indefinitely:
> [4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
> [4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
> [4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
> [4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
> [4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
> [4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
> [4.681898] adv7511 2-0039: failed to find dsi host

I just realized Rob's tree is missing the kirin patch. My apologies!
I'll retest and let you know.

thanks
-john


Re: [Freedreno] [PATCH v4 00/24] drm/bridge: Make panel and bridge probe order consistent

2021-09-29 Thread John Stultz
On Fri, Sep 10, 2021 at 3:12 AM Maxime Ripard  wrote:
>
> We've encountered an issue with the RaspberryPi DSI panel that prevented the
> whole display driver from probing.
>
> The issue is described in detail in the commit 7213246a803f ("drm/vc4: dsi:
> Only register our component once a DSI device is attached"), but the basic 
> idea
> is that since the panel is probed through i2c, there's no synchronization
> between its probe and the registration of the MIPI-DSI host it's attached to.
>
> We initially moved the component framework registration to the MIPI-DSI Host
> attach hook to make sure we register our component only when we have a DSI
> device attached to our MIPI-DSI host, and then use lookup our DSI device in 
> our
> bind hook.
>
> However, all the DSI bridges controlled through i2c are only registering their
> associated DSI device in their bridge attach hook, meaning with our change
> above, we never got that far, and therefore ended up in the same situation 
> than
> the one we were trying to fix for panels.
>
> The best practice to avoid those issues is to register its functions only 
> after
> all its dependencies are live. We also shouldn't wait any longer than we 
> should
> to play nice with the other components that are waiting for us, so in our case
> that would mean moving the DSI device registration to the bridge probe.
>
> I also had a look at all the DSI hosts, and it seems that exynos, kirin and 
> msm
> would be affected by this and wouldn't probe anymore after those changes.
> Exynos and kirin seems to be simple enough for a mechanical change (that still
> requires to be tested), but the changes in msm seemed to be far more important
> and I wasn't confortable doing them.


Hey Maxime,
  Sorry for taking so long to get to this, but now that plumbers is
over I've had a chance to check it out on kirin

Rob Clark pointed me to his branch with some fixups here:
   
https://gitlab.freedesktop.org/robclark/msm/-/commits/for-mripard/bridge-rework

But trying to boot hikey with that, I see the following loop indefinitely:
[4.632132] adv7511 2-0039: supply avdd not found, using dummy regulator
[4.638961] adv7511 2-0039: supply dvdd not found, using dummy regulator
[4.645741] adv7511 2-0039: supply pvdd not found, using dummy regulator
[4.652483] adv7511 2-0039: supply a2vdd not found, using dummy regulator
[4.659342] adv7511 2-0039: supply v3p3 not found, using dummy regulator
[4.666086] adv7511 2-0039: supply v1p2 not found, using dummy regulator
[4.681898] adv7511 2-0039: failed to find dsi host
[4.688836] adv7511 2-0039: supply avdd not found, using dummy regulator
[4.695724] adv7511 2-0039: supply dvdd not found, using dummy regulator
[4.702583] adv7511 2-0039: supply pvdd not found, using dummy regulator
[4.709369] adv7511 2-0039: supply a2vdd not found, using dummy regulator
[4.716232] adv7511 2-0039: supply v3p3 not found, using dummy regulator
[4.722972] adv7511 2-0039: supply v1p2 not found, using dummy regulator
[4.738720] adv7511 2-0039: failed to find dsi host

I'll have to dig a bit to figure out what's going wrong, but wanted to
give you the heads up that there seems to be a problem

thanks
-john


Re: [Freedreno] [PATCH] drm/msm: Disable frequency clamping on a630

2021-09-03 Thread John Stultz
On Thu, Jul 29, 2021 at 1:49 PM Rob Clark  wrote:
> On Thu, Jul 29, 2021 at 1:28 PM Caleb Connolly
>  wrote:
> > On 29/07/2021 21:24, Rob Clark wrote:
> > > On Thu, Jul 29, 2021 at 1:06 PM Caleb Connolly
> > >  wrote:
> > >>
> > >> Hi Rob,
> > >>
> > >> I've done some more testing! It looks like before that patch ("drm/msm: 
> > >> Devfreq tuning") the GPU would never get above
> > >> the second frequency in the OPP table (342MHz) (at least, not in 
> > >> glxgears). With the patch applied it would more
> > >> aggressively jump up to the max frequency which seems to be unstable at 
> > >> the default regulator voltages.
> > >
> > > *ohh*, yeah, ok, that would explain it
> > >
> > >> Hacking the pm8005 s1 regulator (which provides VDD_GFX) up to 0.988v 
> > >> (instead of the stock 0.516v) makes the GPU stable
> > >> at the higher frequencies.
> > >>
> > >> Applying this patch reverts the behaviour, and the GPU never goes above 
> > >> 342MHz in glxgears, losing ~30% performance in
> > >> glxgear.
> > >>
> > >> I think (?) that enabling CPR support would be the proper solution to 
> > >> this - that would ensure that the regulators run
> > >> at the voltage the hardware needs to be stable.
> > >>
> > >> Is hacking the voltage higher (although ideally not quite that high) an 
> > >> acceptable short term solution until we have
> > >> CPR? Or would it be safer to just not make use of the higher frequencies 
> > >> on a630 for now?
> > >>
> > >
> > > tbh, I'm not sure about the regulator stuff and CPR.. Bjorn is already
> > > on CC and I added sboyd, maybe one of them knows better.
> > >
> > > In the short term, removing the higher problematic OPPs from dts might
> > > be a better option than this patch (which I'm dropping), since there
> > > is nothing stopping other workloads from hitting higher OPPs.
> > Oh yeah that sounds like a more sensible workaround than mine .
> > >
> > > I'm slightly curious why I didn't have problems at higher OPPs on my
> > > c630 laptop (sdm850)
> > Perhaps you won the sillicon lottery - iirc sdm850 is binned for higher 
> > clocks as is out of the factory.
> >
> > Would it be best to drop the OPPs for all devices? Or just those affected? 
> > I guess it's possible another c630 might
> > crash where yours doesn't?
>
> I've not heard any reports of similar issues from the handful of other
> folks with c630's on #aarch64-laptops.. but I can't really say if that
> is luck or not.
>
> Maybe just remove it for affected devices?  But I'll defer to Bjorn.

Just as another datapoint, I was just marveling at how suddenly smooth
the UI was performing on db845c and Caleb pointed me at the "drm/msm:
Devfreq tuning" patch as the likely cause of the improvement, and
mid-discussion my board crashed into USB crash mode:
[  146.157696][C0] adreno 500.gpu: CP | AHB bus error
[  146.163303][C0] adreno 500.gpu: CP | AHB bus error
[  146.168837][C0] adreno 500.gpu: RBBM | ATB bus overflow
[  146.174960][C0] adreno 500.gpu: CP | HW fault | status=0x
[  146.181917][C0] adreno 500.gpu: CP | AHB bus error
[  146.187547][C0] adreno 500.gpu: CP illegal instruction error
[  146.194009][C0] adreno 500.gpu: CP | AHB bus error
[  146.308909][T9] Internal error: synchronous external abort:
9610 [#1] PREEMPT SMP
[  146.317150][T9] Modules linked in:
[  146.320941][T9] CPU: 3 PID: 9 Comm: kworker/u16:1 Tainted: G
W 5.14.0-mainline-06795-g42b258c2275c #24
[  146.331974][T9] Hardware name: Thundercomm Dragonboar
Format: Log Type - Time(microsec) - Message - Optional Info
Log Type: B - Since Boot(Power On Reset),  D - Delta,  S - Statistic
S - QC_IMAGE_VERSION_STRING=BOOT.XF.2.0-00371-SDM845LZB-1
S - IMAGE_VARIANT_STRING=SDM845LA
S - OEM_IMAGE_VERSION_STRING=TSBJ-FA-PC-02170

So Caleb sent me to this thread. :)

I'm still trying to trip it again, but it does seem like db845c is
also seeing some stability issues with Linus' HEAD.

thanks
-john


Re: [Freedreno] [PATCH] drm/msm: Fix display fault handling

2021-07-26 Thread John Stultz
On Wed, Jul 7, 2021 at 10:57 AM Rob Clark  wrote:
>
> From: Rob Clark 
>
> It turns out that when the display is enabled by the bootloader, we can
> get some transient iommu faults from the display.  Which doesn't go over
> too well when we install a fault handler that is gpu specific.  To avoid
> this, defer installing the fault handler until we get around to setting
> up per-process pgtables (which is adreno_smmu specific).  The arm-smmu
> fallback error reporting is sufficient for reporting display related
> faults (and in fact was all we had prior to f8f934c180f629bb927a04fd90d)
>
> Reported-by: Dmitry Baryshkov 
> Reported-by: Yassine Oudjana 
> Fixes: 2a574cc05d38 ("drm/msm: Improve the a6xx page fault handler")
> Signed-off-by: Rob Clark 
> Tested-by: John Stultz 
> ---

Hey folks!
  Just wanted to follow up on this, as it's still missing from
5.14-rc3 and is critical for resolving a boot regression on db845c. Is
there anything keeping this from heading upstream?

thanks
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v5 0/5] iommu/arm-smmu: adreno-smmu page fault handling

2021-07-06 Thread John Stultz
On Sun, Jul 4, 2021 at 11:16 AM Rob Clark  wrote:
>
> I suspect you are getting a dpu fault, and need:
>
> https://lore.kernel.org/linux-arm-msm/CAF6AEGvTjTUQXqom-xhdh456tdLscbVFPQ+iud1H1gHc8A2=h...@mail.gmail.com/
>
> I suppose Bjorn was expecting me to send that patch

If it's helpful, I applied that and it got the db845c booting mainline
again for me (along with some reverts for a separate ext4 shrinker
crash).
Tested-by: John Stultz 

thanks
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v4 22/24] drm/msm/dsi: remove temp data from global pll structure

2021-06-11 Thread John Stultz
On Fri, Jun 11, 2021 at 2:01 AM Dmitry Baryshkov
 wrote:
>
> Hi,
>
> On Fri, 11 Jun 2021 at 10:07, John Stultz  wrote:
> >
> > On Wed, Mar 31, 2021 at 3:58 AM Dmitry Baryshkov
> >  wrote:
> > >
> > > The 7nm, 10nm and 14nm drivers would store interim data used during
> > > VCO/PLL rate setting in the global dsi_pll_Nnm structure. Move this data
> > > structures to the onstack storage. While we are at it, drop
> > > unused/static 'config' data, unused config fields, etc.
> > >
> > > Signed-off-by: Dmitry Baryshkov 
> > > Reviewed-by: Abhinav Kumar 
> > > Tested-by: Stephen Boyd  # on sc7180 lazor
> >
> > Hey Dmitry,
> >   Just wanted to give you a heads up.  Peter Collingbourne reported
> > today that his db845c wasn't booting to display for him on his 4k
> > monitor. It works fine on a 1080p screen, and while 4k isn't supported
> > (yet?),  normally the board should fall back to 1080p when connected
> > to a 4k monitor.  I was able to reproduce this myself and I see the
> > errors below[1].
>
> It looks like I made a mistake testing these patches with the splash
> screen disabled.
> Stephen Boyd has proposed a fix few days ago (will be included into
> the 5.13). Could you check that it fixes the problem for you?
>
> https://lore.kernel.org/linux-arm-msm/20210608195519.125561-1-swb...@chromium.org/

Ah! This does seem to fix it! Thank you so much for pointing it out!

thanks
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v4 22/24] drm/msm/dsi: remove temp data from global pll structure

2021-06-11 Thread John Stultz
On Wed, Mar 31, 2021 at 3:58 AM Dmitry Baryshkov
 wrote:
>
> The 7nm, 10nm and 14nm drivers would store interim data used during
> VCO/PLL rate setting in the global dsi_pll_Nnm structure. Move this data
> structures to the onstack storage. While we are at it, drop
> unused/static 'config' data, unused config fields, etc.
>
> Signed-off-by: Dmitry Baryshkov 
> Reviewed-by: Abhinav Kumar 
> Tested-by: Stephen Boyd  # on sc7180 lazor

Hey Dmitry,
  Just wanted to give you a heads up.  Peter Collingbourne reported
today that his db845c wasn't booting to display for him on his 4k
monitor. It works fine on a 1080p screen, and while 4k isn't supported
(yet?),  normally the board should fall back to 1080p when connected
to a 4k monitor.  I was able to reproduce this myself and I see the
errors below[1].

I dug back and found that things were working ok on v5.12 w/ the
recently merged commit d1a97648ae028 ("drm/bridge: lt9611: Fix
handling of 4k panels"), and started digging around.

Seeing a bunch of changes to the
drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c file, I tried reverting a
chunk of the changes since 5.12 to that, and that got it working
again. I've narrowed it down to this change -
001d8dc33875 ("drm/msm/dsi: remove temp data from global pll
structure") upstream (also reverting following 6e2ad9c3bfca and
36c5dde5fdf0 first - but its reverting this change that actually makes
it work again).

I've not managed to really look into the change to see what might be
going wrong yet (its late and I'm about to crash), but I wanted to
give you a heads up. If you have any ideas for me to try I'm happy to
give them a go.

thanks
-john

[1]:
[   19.846857] msm_dsi_phy ae94400.dsi-phy:
[drm:dsi_pll_10nm_vco_prepare] *ERROR* DSI PLL(0) lock failed,
status=0x
[   19.857925] msm_dsi_phy ae94400.dsi-phy:
[drm:dsi_pll_10nm_vco_prepare] *ERROR* PLL(0) lock failed
[   19.866978] dsi_link_clk_enable_6g: Failed to enable dsi byte clk
[   19.873124] msm_dsi_host_power_on: failed to enable link clocks. ret=-110
[   19.879987] dsi_mgr_bridge_pre_enable: power on host 0 failed, -110
[   19.886309] Turning OFF PHY while PLL is on
[   20.415019] lt9611 10-003b: video check: hactive_a=0, hactive_b=0,
vactive=0, v_total=0, h_total_sysclk=0
[   20.481062] [drm:dpu_encoder_phys_vid_wait_for_commit_done:528]
[dpu error]vblank timeout
[   20.489306] [drm:dpu_kms_wait_for_commit_done:453] [dpu error]wait
for commit done returned -110
[   20.513031] [drm:dpu_encoder_frame_done_timeout:2161] [dpu
error]enc31 frame done timeout
[   20.553059] [drm:dpu_encoder_phys_vid_wait_for_commit_done:528]
[dpu error]vblank timeout
[   20.561300] [drm:dpu_kms_wait_for_commit_done:453] [dpu error]wait
for commit done returned -110
[   20.625054] [drm:dpu_encoder_phys_vid_wait_for_commit_done:528]
[dpu error]vblank timeout
[   20.633299] [drm:dpu_kms_wait_for_commit_done:453] [dpu error]wait
for commit done returned -110
[   20.657033] [drm:dpu_encoder_frame_done_timeout:2161] [dpu
error]enc31 frame done timeout
[   20.697065] [drm:dpu_encoder_phys_vid_wait_for_commit_done:528]
[dpu error]vblank timeout
[   20.705316] [drm:dpu_kms_wait_for_commit_done:453] [dpu error]wait
for commit done returned -110
[   20.769066] [drm:dpu_encoder_phys_vid_wait_for_commit_done:528]
[dpu error]vblank timeout
[   20.777330] [drm:dpu_kms_wait_for_commit_done:453] [dpu error]wait
for commit done returned -110
[   20.801035] [drm:dpu_encoder_frame_done_timeout:2161] [dpu
error]enc31 frame done timeout
[   20.845049] [drm:dpu_encoder_phys_vid_wait_for_commit_done:528]
[dpu error]vblank timeout
...
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH] drm/msm/dpu: Delete bonkers code

2021-04-30 Thread John Stultz
On Fri, Apr 30, 2021 at 10:14 AM Rob Clark  wrote:
>
> From: Rob Clark 
>
> dpu_crtc_atomic_flush() was directly poking it's attached planes in a
> code path that ended up in dpu_plane_atomic_update(), even if the plane
> was not involved in the current atomic update.  While a bit dubious,
> this worked before because plane->state would always point to something
> valid.  But now using drm_atomic_get_new_plane_state() we could get a
> NULL state pointer instead, leading to:
>
>[   20.873273] Call trace:
>[   20.875740]  dpu_plane_atomic_update+0x5c/0xed0
>[   20.880311]  dpu_plane_restore+0x40/0x88
>[   20.884266]  dpu_crtc_atomic_flush+0xf4/0x208
>[   20.888660]  drm_atomic_helper_commit_planes+0x150/0x238
>[   20.894014]  msm_atomic_commit_tail+0x1d4/0x7a0
>[   20.898579]  commit_tail+0xa4/0x168
>[   20.902102]  drm_atomic_helper_commit+0x164/0x178
>[   20.906841]  drm_atomic_commit+0x54/0x60
>[   20.910798]  drm_atomic_connector_commit_dpms+0x10c/0x118
>[   20.916236]  drm_mode_obj_set_property_ioctl+0x1e4/0x440
>[   20.921588]  drm_connector_property_set_ioctl+0x60/0x88
>[   20.926852]  drm_ioctl_kernel+0xd0/0x120
>[   20.930807]  drm_ioctl+0x21c/0x478
>[   20.934235]  __arm64_sys_ioctl+0xa8/0xe0
>[   20.938193]  invoke_syscall+0x64/0x130
>[   20.941977]  el0_svc_common.constprop.3+0x5c/0xe0
>[   20.946716]  do_el0_svc+0x80/0xa0
>[   20.950058]  el0_svc+0x20/0x30
>[   20.953145]  el0_sync_handler+0x88/0xb0
>[   20.957014]  el0_sync+0x13c/0x140
>
> The reason for the codepath seems dubious, the atomic suspend/resume
> heplers should handle the power-collapse case.  If not, the CRTC's
> atomic_check() should be adding the planes to the atomic update.

Thanks! This patch gets things booting again!

Tested-by: John Stultz 

thanks
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH] drm/msm: Fix removal of valid error case when checking speed_bin

2021-04-05 Thread John Stultz
On Mon, Mar 29, 2021 at 6:34 PM John Stultz  wrote:
>
> Commit 7bf168c8fe8c  ("drm/msm: Fix speed-bin support not to
> access outside valid memory"), reworked the nvmem reading of
> "speed_bin", but in doing so dropped handling of the -ENOENT
> case which was previously documented as "fine".
>
> That change resulted in the db845c board display to fail to
> start, with the following error:
>
> adreno 500.gpu: [drm:a6xx_gpu_init] *ERROR* failed to read speed-bin 
> (-2). Some OPPs may not be supported by hardware
>
> Thus, this patch simply re-adds the ENOENT handling so the lack
> of the speed_bin entry isn't fatal for display, and gets things
> working on db845c.

Hey Folks,
  Just wanted to re-ping you on this, as it resolves a regression
introduced in 5.12-rc5 and I'm not yet seeing this in -next. Would be
nice to have this in place before 5.12 final.

thanks
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH] drm/msm: Fix removal of valid error case when checking speed_bin

2021-03-29 Thread John Stultz
Commit 7bf168c8fe8c  ("drm/msm: Fix speed-bin support not to
access outside valid memory"), reworked the nvmem reading of
"speed_bin", but in doing so dropped handling of the -ENOENT
case which was previously documented as "fine".

That change resulted in the db845c board display to fail to
start, with the following error:

adreno 500.gpu: [drm:a6xx_gpu_init] *ERROR* failed to read speed-bin (-2). 
Some OPPs may not be supported by hardware

Thus, this patch simply re-adds the ENOENT handling so the lack
of the speed_bin entry isn't fatal for display, and gets things
working on db845c.

Cc: Rob Clark 
Cc: Sean Paul 
Cc: Jordan Crouse 
Cc: Eric Anholt 
Cc: Douglas Anderson 
Cc: linux-arm-...@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: Bjorn Andersson 
Cc: YongQin Liu 
Reported-by: YongQin Liu 
Fixes: 7bf168c8fe8c  ("drm/msm: Fix speed-bin support not to access outside 
valid memory")
Signed-off-by: John Stultz 
---
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 690409ca8a186..cb2df8736ca85 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1406,7 +1406,13 @@ static int a6xx_set_supported_hw(struct device *dev, 
struct a6xx_gpu *a6xx_gpu,
int ret;
 
ret = nvmem_cell_read_u16(dev, "speed_bin", );
-   if (ret) {
+   /*
+* -ENOENT means that the platform doesn't support speedbin which is
+* fine
+*/
+   if (ret == -ENOENT) {
+   return 0;
+   } else if (ret) {
DRM_DEV_ERROR(dev,
  "failed to read speed-bin (%d). Some OPPs may not 
be supported by hardware",
  ret);
-- 
2.25.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH] drm/msm: Fix 0xfffflub in "Refactor address space initialization"

2020-06-12 Thread John Stultz
This week I started seeing GPU crashes on my DragonBoard 845c
which I narrowed down to being caused by commit ccac7ce373c1
("drm/msm: Refactor address space initialization").

Looking through the patch, Jordan and I couldn't find anything
obviously wrong, so I ended up breaking that change up into a
number of smaller logical steps so I could figure out which part
was causing the trouble.

Ends up, visually counting 'f's is hard, esp across a number
of lines:
  0xfff != 0x

This patch corrects the end value we pass in to
msm_gem_address_space_create() in
adreno_iommu_create_address_space() so that it matches the value
used before the problematic patch landed.

With this change, I no longer see the GPU crashes that were
affecting me.

Cc: Shawn Guo 
Cc: Rob Clark 
Cc: Sean Paul 
Cc: Jordan Crouse 
Cc: freedreno@lists.freedesktop.org
Fixes: ccac7ce373c1 ("drm/msm: Refactor address space initialization")
Signed-off-by: John Stultz 
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c 
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 89673c7ed473..3d4efe684a98 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -194,7 +194,7 @@ adreno_iommu_create_address_space(struct msm_gpu *gpu,
struct msm_gem_address_space *aspace;
 
aspace = msm_gem_address_space_create(mmu, "gpu", SZ_16M,
-   0xfff);
+   0x);
 
if (IS_ERR(aspace) && !IS_ERR(mmu))
mmu->funcs->destroy(mmu);
-- 
2.17.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [v1] drm/msm/dpu: request for display color blocks based on hw catalog entry

2020-06-11 Thread John Stultz
On Thu, Jun 11, 2020 at 5:55 AM Krishna Manikandan
 wrote:
>
> From: Kalyan Thota 
>
> Request for color processing blocks only if they are
> available in the display hw catalog and they are
> sufficient in number for the selection.
>
> Signed-off-by: Kalyan Thota 

Tested-by: John Stultz 

Thanks so much for the quick fix!
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: add support for color processing blocks in dpu driver

2020-06-08 Thread John Stultz
On Mon, Jun 8, 2020 at 3:25 PM John Stultz  wrote:
>
> On Wed, Mar 25, 2020 at 1:17 AM Kalyan Thota  wrote:
> >
> > This change adds support to configure dspp blocks in
> > the dpu driver.
> >
> > Macro description of the changes coming in this patch.
> > 1) Add dspp definitions in the hw catalog.
> > 2) Add capability to reserve dspp blocks in the display data path.
> > 3) Attach the reserved block to the encoder.
> >
> > Signed-off-by: Kalyan Thota 
>
> Hey all,
>   With this patch now merged upstream, I'm seeing a regression on
> db845c that I bisected down to it.
>
> When I boot up I see:
> [   40.976737] [drm:_dpu_rm_check_lm_and_get_connected_blks] [dpu
> error]failed to get dspp on lm 0
> [   40.985600] [drm:_dpu_rm_check_lm_and_get_connected_blks] [dpu
> error]failed to get dspp on lm 0
> [   40.994587] [drm:_dpu_rm_check_lm_and_get_connected_blks] [dpu
> error]failed to get dspp on lm 0
> [   41.003492] [drm:_dpu_rm_check_lm_and_get_connected_blks] [dpu
> error]failed to get dspp on lm 0
> [   41.012283] [drm:_dpu_rm_make_reservation] [dpu error]unable to
> find appropriate mixers
> [   41.020369] [drm:dpu_rm_reserve] [dpu error]failed to reserve hw
> resources: -119
>
> Over and over, and the display doesn't start up.
>
> I suspect we're supposed to catch the following check before the failure:
>
> +   if (!reqs->topology.num_dspp)
> +   return true;
>
> I suspect the issue is in dpu_encoder_get_topology() we don't fully
> initialize the topology structure on the stack before returning it.
>
> Does that sound plausible or is there likely some other cause?

This guess is wrong. The topology.num_dspp is 2, but lm_cfg->dspp is
coming back as zero.

I'll continue digging to see if I can understand better whats going wrong.

thanks
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH 1/2] drm/msm/dpu: add support for color processing blocks in dpu driver

2020-06-08 Thread John Stultz
On Wed, Mar 25, 2020 at 1:17 AM Kalyan Thota  wrote:
>
> This change adds support to configure dspp blocks in
> the dpu driver.
>
> Macro description of the changes coming in this patch.
> 1) Add dspp definitions in the hw catalog.
> 2) Add capability to reserve dspp blocks in the display data path.
> 3) Attach the reserved block to the encoder.
>
> Signed-off-by: Kalyan Thota 

Hey all,
  With this patch now merged upstream, I'm seeing a regression on
db845c that I bisected down to it.

When I boot up I see:
[   40.976737] [drm:_dpu_rm_check_lm_and_get_connected_blks] [dpu
error]failed to get dspp on lm 0
[   40.985600] [drm:_dpu_rm_check_lm_and_get_connected_blks] [dpu
error]failed to get dspp on lm 0
[   40.994587] [drm:_dpu_rm_check_lm_and_get_connected_blks] [dpu
error]failed to get dspp on lm 0
[   41.003492] [drm:_dpu_rm_check_lm_and_get_connected_blks] [dpu
error]failed to get dspp on lm 0
[   41.012283] [drm:_dpu_rm_make_reservation] [dpu error]unable to
find appropriate mixers
[   41.020369] [drm:dpu_rm_reserve] [dpu error]failed to reserve hw
resources: -119

Over and over, and the display doesn't start up.

I suspect we're supposed to catch the following check before the failure:

+   if (!reqs->topology.num_dspp)
+   return true;

I suspect the issue is in dpu_encoder_get_topology() we don't fully
initialize the topology structure on the stack before returning it.

Does that sound plausible or is there likely some other cause?

thanks
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v2 4/4] drm/msm/a6xx: Use the DMA API for GMU memory objects

2020-02-25 Thread John Stultz
On Thu, Feb 20, 2020 at 10:27 AM Jordan Crouse  wrote:
>
> The GMU has very few memory allocations and uses a flat memory space so
> there is no good reason to go out of our way to bypass the DMA APIs which
> were basically designed for this exact scenario.
>
> v2: Pass force_dma false to of_dma_configure to require that the DMA
> region be set up and return error from of_dma_configure to fail probe.
>
> Signed-off-by: Jordan Crouse 
> ---
>
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 112 
> +++---
>  drivers/gpu/drm/msm/adreno/a6xx_gmu.h |   5 +-
>  2 files changed, 11 insertions(+), 106 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> index 983afea..c36b38b 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
...
> -   count = bo->size >> PAGE_SHIFT;
> +   bo->virt = dma_alloc_attrs(gmu->dev, bo->size, >iova, GFP_KERNEL,
> +   bo->attrs);
>
...
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h 
> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> index 2af91ed..31bd1987 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
> @@ -13,7 +13,7 @@ struct a6xx_gmu_bo {
> void *virt;
> size_t size;
> u64 iova;
> -   struct page **pages;
> +   unsigned long attrs;
>  };

As a head up, Todd reported that this patch is causing build trouble
w/ arm32, as the iova needs to be a dma_attr_t.

I've got a patch for the android-mainline tree to fix this, but you
might want to spin a v3 to address this.
  https://android-review.googlesource.com/c/kernel/common/+/1243928

thanks
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v2 0/4] msm/gpu/a6xx: use the DMA-API for GMU memory allocations

2020-02-20 Thread John Stultz
On Thu, Feb 20, 2020 at 10:27 AM Jordan Crouse  wrote:
> When CONFIG_INIT_ON_ALLOC_DEFAULT_ON the GMU memory allocator runs afoul of
> cache coherency issues because it is mapped as write-combine without clearing
> the cache after it was zeroed.
>
> Rather than duplicate the hacky workaround we use in the GEM allocator for the
> same reason it turns out that we don't need to have a bespoke memory allocator
> for the GMU anyway. It uses a flat, global address space and there are only
> two relatively minor allocations anyway. In short, this is essentially what 
> the
> DMA API was created for so replace a bunch of memory management code with two
> calls to allocate and free DMA memory and we're fine.
>
> The only wrinkle is that the memory allocations need to be in a very specific
> location in the GMU virtual address space so in order to get the iova 
> allocator
> to do the right thing we need to specify the dma-ranges property in the device
> tree for the GMU node. Since we've not yet converted the GMU bindings over to
> YAML two patches quickly turn into four but at the end of it we have at least
> one bindings file converted to YAML and 99 less lines of code to worry about.
>
> v2: Fix the example bindings for dma-ranges - the third item is the size
> Pass false to of_dma_configure so that it fails probe if the DMA region is not
> set up.

This set still works for me as well. Thanks so much!
Tested-by: John Stultz 

thanks
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v1 0/4] msm/gpu/a6xx: use the DMA-API for GMU memory allocations

2020-02-19 Thread John Stultz
On Wed, Feb 19, 2020 at 1:33 PM Jordan Crouse  wrote:
>
> When CONFIG_INIT_ON_ALLOC_DEFAULT_ON the GMU memory allocator runs afoul of
> cache coherency issues because it is mapped as write-combine without clearing
> the cache after it was zeroed.
>
> Rather than duplicate the hacky workaround we use in the GEM allocator for the
> same reason it turns out that we don't need to have a bespoke memory allocator
> for the GMU anyway. It uses a flat, global address space and there are only
> two relatively minor allocations anyway. In short, this is essentially what 
> the
> DMA API was created for so replace a bunch of memory management code with two
> calls to allocate and free DMA memory and we're fine.
>
> The only wrinkle is that the memory allocations need to be in a very specific
> location in the GMU virtual address space so in order to get the iova 
> allocator
> to do the right thing we need to specify the dma-ranges property in the device
> tree for the GMU node. Since we've not yet converted the GMU bindings over to
> YAML two patches quickly turn into four but at the end of it we have at least
> one bindings file converted to YAML and 99 less lines of code to worry about.
>
> Jordan Crouse (4):
>   dt-bindings: display: msm: Convert GMU bindings to YAML
>   dt-bindings: display: msm: Add required dma-range property
>   arm64: dts: sdm845: Set the virtual address range for GMU allocations
>   drm/msm/a6xx: Use the DMA API for GMU memory objects

Awesome! Thanks so much for the quick turnaround on this! This set
resolves the crashes I was seeing with
CONFIG_INIT_ON_ALLOC_DEFAULT_ON.

Tested-by: John Stultz 

thanks again!
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH] drm/msm/a6xx: Remove unneeded GBIF unhalt

2020-02-05 Thread John Stultz
On Tue, Feb 4, 2020 at 9:42 AM Jordan Crouse  wrote:
>
> Commit e812744c5f95 ("drm: msm: a6xx: Add support for A618") added a
> universal GBIF un-halt into a6xx_start(). This can cause problems for
> a630 targets which do not use GBIF and might have access protection
> enabled on the region now occupied by the GBIF registers.
>
> But it turns out that we didn't need to unhalt the GBIF in this path
> since the stop function already takes care of that after executing a flush
> but before turning off the headswitch. We should be confident that the
> GBIF is open for business when we restart the hardware.
>
> Signed-off-by: Jordan Crouse 

Sorry, yesterday got busy with other things and I didn't get around to
testing your patch, but I have tested earlier with my own patch which
is identical:
  
https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/db845c-mainline-WIP=4e6a2e84dd77fe74faa1a6b797eb0ee7bf11ffd7

So, I think I can safely add:
Tested-by: John Stultz 

Thanks so much for the quick turnaround on this!
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


Re: [Freedreno] [PATCH v2 2/3] drm: msm: a6xx: Add support for A618

2020-02-03 Thread John Stultz
On Wed, Jan 22, 2020 at 11:19 PM Sharat Masetty  wrote:
>
> This patch adds support for enabling Graphics Bus Interface(GBIF)
> used in multiple A6xx series chipets. Also makes changes to the
> PDC/RSC sequencing specifically required for A618. This is needed
> for proper interfacing with RPMH.
>
> Signed-off-by: Sharat Masetty 
> ---
> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c 
> b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> index dc8ec2c..2ac9a51 100644
> --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
> @@ -378,6 +378,18 @@ static int a6xx_hw_init(struct msm_gpu *gpu)
> struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> int ret;
>
> +   /*
> +* During a previous slumber, GBIF halt is asserted to ensure
> +* no further transaction can go through GPU before GPU
> +* headswitch is turned off.
> +*
> +* This halt is deasserted once headswitch goes off but
> +* incase headswitch doesn't goes off clear GBIF halt
> +* here to ensure GPU wake-up doesn't fail because of
> +* halted GPU transactions.
> +*/
> +   gpu_write(gpu, REG_A6XX_GBIF_HALT, 0x0);
> +
> /* Make sure the GMU keeps the GPU on while we set it up */
> a6xx_gmu_set_oob(_gpu->gmu, GMU_OOB_GPU_SET);
>

So I already brought this up on #freedreno but figured I'd follow up
on the list.

With linus/master, I'm seeing hard crashes (into usb crash mode) with
the db845c, which I isolated down to this patch, and then to the chunk
above.

Dropping the gpu_write line above gets things booting again for me.

Let me know if there are any follow on patches I can help validate.

thanks
-john
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH] drm: msm: Fix return type of dsi_mgr_connector_mode_valid for kCFI

2020-01-29 Thread John Stultz
I was hitting kCFI crashes when building with clang, and after
some digging finally narrowed it down to the
dsi_mgr_connector_mode_valid() function being implemented as
returning an int, instead of an enum drm_mode_status.

This patch fixes it, and appeases the opaque word of the kCFI
gods (seriously, clang inlining everything makes the kCFI
backtraces only really rough estimates of where things went
wrong).

Thanks as always to Sami for his help narrowing this down.

Cc: Rob Clark 
Cc: Sean Paul 
Cc: Sami Tolvanen 
Cc: Todd Kjos 
Cc: Alistair Delva 
Cc: Amit Pundir 
Cc: Sumit Semwal 
Cc: freedreno@lists.freedesktop.org
Cc: clang-built-li...@googlegroups.com
Signed-off-by: John Stultz 
---
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c 
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 271aa7bbca925..355a60b4a536f 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -336,7 +336,7 @@ static int dsi_mgr_connector_get_modes(struct drm_connector 
*connector)
return num;
 }
 
-static int dsi_mgr_connector_mode_valid(struct drm_connector *connector,
+static enum drm_mode_status dsi_mgr_connector_mode_valid(struct drm_connector 
*connector,
struct drm_display_mode *mode)
 {
int id = dsi_mgr_connector_get_id(connector);
-- 
2.17.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno


[Freedreno] [PATCH] drm: msm: Quiet down plane errors in atomic_check

2020-01-07 Thread John Stultz
With the db845c running AOSP, I see the following error on every
frame on the home screen:
  [drm:dpu_plane_atomic_check:915] [dpu error]plane33 invalid src 
2880x1620+0+470 line:2560

This is due to the error paths in atomic_check using
DPU_ERROR_PLANE(), and the drm_hwcomposer using atomic_check
to decide how to composite the frame (thus it expects to see
atomic_check to fail).

In order to avoid spamming the logs, this patch converts the
DPU_ERROR_PLANE() calls to DPU_DEBUG_PLANE() calls in
atomic_check.

Cc: Todd Kjos 
Cc: Alistair Delva 
Cc: Amit Pundir 
Cc: Rob Clark 
Cc: Sean Paul 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-de...@lists.freedesktop.org
Cc: freedreno@lists.freedesktop.org
Signed-off-by: John Stultz 
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c 
b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 58d5acbcfc5c..d19ae0b51d1c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -858,7 +858,7 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
  pdpu->pipe_sblk->maxupscale << 16,
  true, true);
if (ret) {
-   DPU_ERROR_PLANE(pdpu, "Check plane state failed (%d)\n", ret);
+   DPU_DEBUG_PLANE(pdpu, "Check plane state failed (%d)\n", ret);
return ret;
}
if (!state->visible)
@@ -884,13 +884,13 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
(!(pdpu->features & DPU_SSPP_SCALER) ||
 !(pdpu->features & (BIT(DPU_SSPP_CSC)
 | BIT(DPU_SSPP_CSC_10BIT) {
-   DPU_ERROR_PLANE(pdpu,
+   DPU_DEBUG_PLANE(pdpu,
"plane doesn't have scaler/csc for yuv\n");
return -EINVAL;
 
/* check src bounds */
} else if (!dpu_plane_validate_src(, _rect, min_src_size)) {
-   DPU_ERROR_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
+   DPU_DEBUG_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
DRM_RECT_ARG());
return -E2BIG;
 
@@ -899,19 +899,19 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
   (src.x1 & 0x1 || src.y1 & 0x1 ||
drm_rect_width() & 0x1 ||
drm_rect_height() & 0x1)) {
-   DPU_ERROR_PLANE(pdpu, "invalid yuv source " DRM_RECT_FMT "\n",
+   DPU_DEBUG_PLANE(pdpu, "invalid yuv source " DRM_RECT_FMT "\n",
DRM_RECT_ARG());
return -EINVAL;
 
/* min dst support */
} else if (drm_rect_width() < 0x1 || drm_rect_height() < 0x1) {
-   DPU_ERROR_PLANE(pdpu, "invalid dest rect " DRM_RECT_FMT "\n",
+   DPU_DEBUG_PLANE(pdpu, "invalid dest rect " DRM_RECT_FMT "\n",
DRM_RECT_ARG());
return -EINVAL;
 
/* check decimated source width */
} else if (drm_rect_width() > max_linewidth) {
-   DPU_ERROR_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
+   DPU_DEBUG_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
DRM_RECT_ARG(), max_linewidth);
return -E2BIG;
}
-- 
2.17.1

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno