Messages in this thread | | | Date | Tue, 30 Aug 2016 10:17:42 -0700 | From | Bjorn Andersson <> | Subject | Re: [PATCH 3/4] dt-binding: remoteproc: venus rproc dt binding document |
| |
On Mon 29 Aug 04:48 PDT 2016, Stanimir Varbanov wrote:
[..] > > Trying to wrap my head around how the iommu part works here. The > > downstream code seems to indicate that this is a "generic" secure iommu > > interface - used by venus, camera and kgsl; likely for dealing with DRM > > protected buffers. > > The secure iommu interface is for content protected buffers. But these > secure iommu contexts aren't used by msm DRM nor Venus in mainline. In > Venus case I use non-secure iommu context for data buffers. >
We must consider the case when DRM, VFE and Venus handles protected buffers.
> > > > As such the iommu tables are not part of the venus rproc; I believe they > > should either be tied into the msm-iommu driver or perhaps exposed as > > its own iommu(?). > > The page tables are in msm-iommu driver. >
So, just to verify your answer, the msm-iommu driver will handle both protected and unprotected?
> > > > > > But I presume from your inclusion that you've concluded that the venus > > firmware we have refuses to execute without these tables at least > > initialized, is this correct? > > Yes, the SMC call for PAS memory-setup will fail if this page table is > not initialized. >
If the msm-iommu driver will handle the protected buffers (or if there will be a separate iommu driver for protected buffers) it should issue these calls, to not be dependant on the rproc-venus driver.
With that I think we should make the rproc-venus driver depend on this being setup (even if this means creating a "dummy" driver for the protected iommu handling for now).
> > > >>> > >>>> The address is not really fixed, cause the firmware could support > >>>> relocation. In this example I just picked up the next free memory region > >>>> in memory-reserved from msm8916.dtsi. > >>>> > >>> > >>> In 8974 we do have a physical region where it's expected to be loaded. > >>> > >>> So in line with upcoming remoteproc work we should support referencing a > >>> reserved-memory node with either reg or size. > >>> > >>> In the case of spotting a "reg" we're currently better off using > >>> ioremap. We're looking at getting the remoteproc core to deal with this > >>> mess. > >> > >> You mean that remoteproc core will parse memory-region property? > >> > > > > It has to, because it's a quite common scenario for remoteproc drivers > > to either get its backing memory from a static region or be restricted > > to part of system ram - properties that reserved-memory and > > memory-region captures already. > > OK, I have no issues with that. My concern is the manual parsing of > 'memory-region' and 'reg' properties in remoteproc core. > > So that idea is to have generic binding for rproc, that would be good. >
I do share your concerns here. But it's a recurring issue with remoteproc drivers.
[..] > > But I presume we have the implementation issue of dma_alloc_coherent() > > failing in either case with the 5MB size. I think we need to look into > > I'd be good to include Marek Szyprowski? At least he will know what > design restrictions there are. >
Please do. The more I look at this the more I think we must use the existing infrastructure for allocating "dma memory". Getting dma_alloc_coherent() supporting non-power-of-2 memory regions would allow us to use the existing infrastructure, for both fixed and dynamically placed memory carveouts in remoteproc.
Regards, Bjorn
| |