Re: [PATCH 04/22] arm64: dts: qcom: hamoa: Reserve low IOVA range for Iris
Vikash Garodia <[email protected]>
| Newsgroups | org.kernel.vger.linux-media,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
On 8/12/2026 6:16 PM, Dmitry Baryshkov wrote: > On Wed, Aug 12, 2026 at 06:02:58AM +0100, Bryan O'Donoghue wrote: >> On 10/08/2026 17:57, Vikash Garodia wrote: >>> >>> >>> On 8/10/2026 5:40 PM, Dmitry Baryshkov wrote: >>>> On Sat, Aug 08, 2026 at 09:18:42PM +0530, Vikash Garodia wrote: >>>>> >>>>> >>>>> On 8/7/2026 6:48 PM, Bryan O'Donoghue wrote: >>>>>> On 07/08/2026 11:22, Vikash Garodia wrote: >>>>>>>>>> I don't like the idea of this series, because it_again_ >>>>>>>>>> doesn't tell us >>>>>>>>>> the truth about the hardware. This typicall ends up with >>>>>>>>>> bigger problems >>>>>>>>> honestly...thats all the info i have about the vpu hardware that it >>>>>>>>> restricts non pixel to DMA from the 0-600MB range. The same i have been >>>>>>>>> trying for a year now >>>>>>>> You are not honest here. You also know that there are secure streams, >>>>>>>> which have to use their own IOMMU SIDs. And some of them, as far as I >>>>>>>> remember, also have memory range restrictions. >>>>>>> please read the commit description again, the answer is there. >>>>>> >>>>>> So I don't necessarily get all of the detail out of the commit log myself. >>>>>> >>>>>> Could you give some detail to address Dmitry's point. >>>>>> >>>>>> The question as I read it is - are all of the other potential SIDs >>>>>> covered by this change ? >>>>>> >>>>> >>>>> I get the query as "secure streams also have their dedicated reserve >>>>> regions, so how does this approach helps" - This patch does not reserve any >>>>> IOVA for secure streams, only the forward looking subnode can assign >>>>> specific reserve for specific streams. >>>>> The patch enforces a common IOVA across all streams, and is good enough to >>>>> fix the problem we have w.r.t device reset. >>>> >>>> No, it's not good enough. It defines that both non-secure streams use >>>> the provided memory range, it doesn't provide a natural way to later >>>> _expand_ it to support secure subnodes, etc. >>>> >>>> We know that there is a problem. We already have been bitten by not >>>> describing the hardware as is and using band-aids. Can we now learn the >>>> lesson and write a proper hardware description? >>>> >>>>> >>>>>>>> So, if we land these patches, how do extend it later to account for all >>>>>>>> of that? >>>>>>>> >>>>>>> forward looking design would be subnode, which we can land ontop of this >>>>>>> series. >>>>>> >>>>>> Yes it should be possible to branch to make subnodes work on-top of this >>>>>> - accepting that once this lands it becomes ABI and support for this >>>>>> method must be sustained, even after sub-nodes land. >>>>>> >>>>> >>>>> Thats the plan. This goes as ABI with subnode to land on top of it. >>>> >>>> So, do you actually plan to support both ABIs? This would also mean >>>> moving reserved regions to the subnode. >>> >>> How is that different from moving the reserve region when stream IDs >>> would also need to move, so be it for the associated memory regions. >>> >>>> What prevents us from landing subnodes straight away? >>>> >>> >>> You are well aware of them, but still asking the same. Reasons, >>> 1. We have been attempting the subnodes for almost a year with multiple >>> pushbacks from multiple maintainers. We are closer and working on it to >>> post for iris, followed by venus, once review is acceptable for iris. >>> 2. Current solution, in this series, is much easy to apply for all >>> kernels and can land faster so that it can fix the reset issue we have >>> for already enabled devices. >>> >>> Both from timewise and simplicity wise, this proposal is made to address >>> the reset issue, while subnode can land ontop of this, without breaking ABI. >>> >>> Regards, >>> Vikash >>> >> >> There is another solution. >> >> Restrict multiple concurrent streams in both drivers. The failure mode is as >> I understand it only triggered by _concurrent_ streams so, restrict that >> case in the .c code. > > I think this might be the best hot fix for now. Very local, very No, it's not. Restricting concurrency does not avoid the reset, it just reduces the possibility of occurrence. > presice, touching the driver pieces, letting it to continue to exist > even after the subnodes have landed to support the legacy case. >