Re: [PATCH V6 0/6] iommu/msm: Add DT adaptation and generic bindings support

From: Rob Clark <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Sricharan <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: "devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>,
	Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>,
	linux-arm-msm
	<linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org"
	<iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org>,
	Srinivas Kandagatla
	<srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Laurent Pinchart
	<laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>,
	Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	stepanm-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org
Subject: Re: [PATCH V6 0/6] iommu/msm: Add DT adaptation and generic bindings support
Date: Fri, 12 Aug 2016 09:28:15 -0400	[thread overview]
Message-ID: <CAF6AEGuitSWt9dhXrMfBDqAkXzpZObuj6A_8XxL+TVSWnbkwaQ@mail.gmail.com> (raw)
In-Reply-To: <000001d1f499$fe2caf50$fa860df0$@codeaurora.org>

On Fri, Aug 12, 2016 at 9:03 AM, Sricharan <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> Hi Rob,
>
>>>>> btw, the current state, at least on linaro integration branch, fault
>>>>> handling doesn't work so well (ie. device never gets resumed).. which
>>>>> is a bit unfortunate for a gpu (and results in a *lot* of rebooting on
>>>>> my part when debugging userspace).  I haven't had time yet to compare
>>>>> to the ancient downstream driver, but not sure if you have any ideas?
>>>>>
>>>>> I guess probably disabling stall on fault would help.  But I'm not
>>>>> even getting the "Fault occurred in context.." prints.  Seeing the
>>>>> fault iova is pretty useful since that plus gpu cmdstream trace helps
>>>>> me figure out which texture/etc is being accessed out of bounds.
>>>>
>>>>fyi, it looks like it is not getting any fault irq..  it's *possible*
>>>>that I screwed up the irq #'s when translating from downstream, so you
>>>>might want to double check that.  I thought I had it right, I assume I
>>>>would have noticed during piglit runs if fault recovery wasn't working
>>>>(since the result is that *everything* after the faulting test would
>>>>have failed since gpu is wedged with no access to memory), but it was
>>>>long enough ago that I can't claim that definitively.
>>>>
>>>>If you need an easy way to trigger a gpu fault, msmtest is a good way,
>>>>change this line:
>>>>
>>>>  https://github.com/freedreno/msmtest/blob/master/msmtest.c#L247
>>>>
>>>>from OUT_RELOC() to OUT_RING(ring, 0x00000000) will trigger a fault.
>>>>
>>>    So for the irq to be triggered, 'non-secure' irq line has to be
>>>   populated in DT. There is a 'secure'and 'non-secure' irq lines for these iommus
>>>   and  non-secure irq number is secure + 1. I tested this by having a 'return 0'
>>>  from the msm_iommu_map (no mapping), and the faults were getting triggered.
>>>
>>>   Can you share me your dts data ?
>>>
>>
>>I think this is what you want:
>>
>>https://github.com/freedreno/kernel-msm/blob/integration-linux-qcomlt/arch/arm/boot/dts/qcom-apq8064.dtsi#L2008
>>
>>I haven't tested a display fault, so I suppose it is possible that
>>irq's are working for some iommu instances but not others?
>
> So in your DT, for gfx3d, the non-secure line is '70' and not '69' (This is secure) .
>  Infact only '70' should be populated. The driver sets the irq line based on resource 0.
>  This applies for all iommu nodes in your DT. (only the second irq line is needed).

ahh, that would explain.

Is it better to remove the extra entry, or should I just swap them
all?  Ie. might there be some point in the future where the driver
would want both?

BR,
-R