Re: [PATCH 04/22] arm64: dts: qcom: hamoa: Reserve low IOVA range for Iris
Dmitry Baryshkov <[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 | <zrwepjscbusdpzarfiiz2rbtej5ggjay4yqiir4pdvfvzuu7ma@5e6wdi6sck34> |
On Wed, Aug 12, 2026 at 09:15:48PM +0530, Vikash Garodia wrote: > > > 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. Ack. > > presice, touching the driver pieces, letting it to continue to exist > > even after the subnodes have landed to support the legacy case. -- With best wishes Dmitry