On Tue 02 Jun 04:02 PDT 2020, Thierry Reding wrote: > On Wed, May 27, 2020 at 12:03:44PM +0100, Will Deacon wrote: > > Hi John, Bjorn, > > > > On Tue, May 26, 2020 at 01:34:45PM -0700, John Stultz wrote: > > > On Thu, May 14, 2020 at 12:34 PM <bjorn.andersson@xxxxxxxxxx> wrote: > > > > > > > > On Thu 27 Feb 18:57 PST 2020, Bjorn Andersson wrote: > > > > > > > > Rob, Will, we're reaching the point where upstream has enough > > > > functionality that this is becoming a critical issue for us. > > > > > > > > E.g. Lenovo Yoga C630 is lacking this and a single dts patch to boot > > > > mainline with display, GPU, WiFi and audio working and the story is > > > > similar on several devboards. > > > > > > > > As previously described, the only thing I want is the stream mapping > > > > related to the display controller in place, either with the CB with > > > > translation disabled or possibly with a way to specify the framebuffer > > > > region (although this turns out to mess things up in the display > > > > driver...) > > > > > > > > I did pick this up again recently and concluded that by omitting the > > > > streams for the USB controllers causes an instability issue seen on one > > > > of the controller to disappear. So I would prefer if we somehow could > > > > have a mechanism to only pick the display streams and the context > > > > allocation for this. > > > > > > > > > > > > Can you please share some pointers/insights/wishes for how we can > > > > conclude on this subject? > > > > > > Ping? I just wanted to follow up on this discussion as this small > > > series is crucial for booting mainline on the Dragonboard 845c > > > devboard. It would be really valuable to be able to get some solution > > > upstream so we can test mainline w/o adding additional patches. > > > > Sorry, it's been insanely busy recently and I haven't had a chance to think > > about this on top of everything else. We're also carrying a hack in Android > > for you :) > > > > > The rest of the db845c series has been moving forward smoothly, but > > > this set seems to be very stuck with no visible progress since Dec. > > > > > > Are there any pointers for what folks would prefer to see? > > > > I've had a chat with Robin about this. Originally, I was hoping that > > people would all work together towards an idyllic future where firmware > > would be able to describe arbitrary pre-existing mappings for devices, > > irrespective of the IOMMU through which they master and Linux could > > inherit this configuration. However, that hasn't materialised (there was > > supposed to be an IORT update, but I don't know what happened to that) > > and, in actual fact, the problem that you have on db845 is /far/ more > > restricted than the general problem. > > It doesn't sound to me like implementing platform-specific workarounds > is a good long-term solution (especially since, according to Bjorn, they > aren't as trivial to implement as it sounds). And we already have all > the infrastructure in place to implement what you describe, so I don't > see why we shouldn't do that. This patchset uses standard device tree > bindings that were designed for exactly this kind of use-case. > I think my results would imply that we would have to end up with (at least) some special case of your proposal (i.e. we need a context bank allocated). > So at least for device-tree based boot firmware can already describe > these pre-existing mappings. If something standard materializes for ACPI > eventually I'm sure we can find ways to integrate that into whatever we > come up with now for DT. > > I think between Bjorn, John, Laurentiu and myself there's pretty broad > consensus (correct me if I'm wrong, guys) that solving this via reserved > memory regions is a good solution that works. So I think what's really > missing is feedback on whether the changes proposed here or Laurentiu's > updated proposal[0] are acceptable, and if not, what the preference is > for getting something equivalent upstream. > As described in my reply to your proposal, the one problem I ran into was that I haven't figured out how to reliably "move" my display streams from one mapping entry to another. With the current scheme I see that their will either be gaps in time with no mapping for my display, or multiple mappings. The other thing I noticed in your proposal was that I have a whole bunch of DT nodes with both iommus and memory-region properties that I really don't care to set up mappings for, but I've not finalized my thoughts on this causing actual problems... > Just to highlight: the IOMMU framework already provides infrastructure > to create direct mappings (via iommu_get_resv_regions(), called from > iommu_create_device_direct_mappings()). I have patches that make use of > this on Tegra210 and earlier where a non-ARM SMMU is used and where the > IOMMU driver enables translations (and doesn't fault by default) only at > device attachment time. That works perfectly using reserved-memory > regions. Perhaps that infrastructure could be extended to cover the > kinds of early mappings that we're discussing here. On the other hand it > might be a bit premature at this point because the ARM SMMU is the only > device that currently needs this, as far as I can tell. > For Qualcomm we got patches picked up for 5.8 that will cause the display controller to be attached with direct mapping, so I think all missing now is the lack of stream mappings between arm-smmu probe and display driver probe... Regards, Bjorn > Thierry > > [0]: https://patchwork.ozlabs.org/project/linux-tegra/list/?series=164853 > > > Could you please try hacking something along the following lines and see > > how you get on? You may need my for-joerg/arm-smmu/updates branch for > > all the pieces: > > > > 1. Use the ->cfg_probe() callback to reserve the SMR/S2CRs you need > > "pinning" and configure for bypass. > > > > 2. Use the ->def_domain_type() callback to return IOMMU_DOMAIN_IDENTITY > > for the display controller > > > > I /think/ that's sufficient, but note that it differs from the current > > approach because we don't end up reserving a CB -- bypass is configured > > in the S2CR instead. Some invalidation might therefore be needed in > > ->cfg_probe() after unhooking the CB. > > > > Thanks, and please yell if you run into problems with this approach. > > > > Will
