Message-ID: <20201105162340.GC485884@ulmo>
Date:   Thu, 5 Nov 2020 17:23:40 +0100
From:   Thierry Reding <thierry.reding@...il.com>
To:     Dmitry Osipenko <digetx@...il.com>
Cc:     Robin Murphy <robin.murphy@....com>,
        Joerg Roedel <joro@...tes.org>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Will Deacon <will@...nel.org>,
        iommu@...ts.linux-foundation.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active"
 property

On Fri, Sep 25, 2020 at 04:21:17PM +0300, Dmitry Osipenko wrote:
> 25.09.2020 15:39, Robin Murphy пишет:
> ...
> >> IIRC, in the past Robin Murphy was suggesting to read out hardware state
> >> early during kernel boot in order to find what regions are in use by
> >> hardware.
> > 
> > I doubt I suggested that in general, because I've always firmly believed
> > it to be a terrible idea. I've debugged too many cases where firmware or
> > kexec has inadvertently left DMA running and corrupted kernel memory, so
> > in general we definitely *don't* want to blindly trust random hardware
> > state. Anything I may have said in relation to Qualcomm's fundamentally
> > broken hypervisor/bootloader setup should not be considered outside that
> > specific context ;)
> > 
> > Robin.
> > 
> >> I think it should be easy to do for the display controller since we
> >> could check clock and PD states in order to decide whether DC's IO could
> >> be accessed and then read out the FB pointer and size. I guess it should
> >> take about hundred lines of code.
> 
> The active DMA is indeed very dangerous, but it's a bit less dangerous
> in a case of read-only DMA.
> 
> I got another idea of how we could benefit from the active display
> hardware. Maybe we could do the following:
> 
> 1. Check whether display is active
> 
> 2. Allocate CMA that matches the FB size
> 
> 3. Create identity mapping for the CMA
> 
> 4. Switch display framebuffer to our CMA
> 
> 5. Create very early simple-framebuffer out of the CMA
> 
> 6. Once Tegra DRM driver is loaded, it will kick out the simple-fb, and
> thus, release temporal CMA and identity mapping.
> 
> This will provide us with a very early framebuffer output and it will
> work on all devices out-of-the-box!

Well that's already kind of what this is trying to achieve, only
skipping the CMA step because the memory is already there and actively
being scanned out from. The problem with your sequence above is first
that you have to allocate from CMA, which means that this has to wait
until CMA becomes available. That's fairly early, but it's not
immediately there. Until you get to that point, there's always the
potential for the display controller to read out from memory that may
now be used for something else. As you said, read-only active DMA isn't
as dangerous as write DMA, but it's not very nice either.

Furthermore, your point 5. above requires device-specific knowledge and
as I mentioned earlier that requires a small, but not necessarily
trivial, device-specific driver to work, which is very impractical for
multi-platform kernels.

There's nothing preventing these reserved-memory regions from being
reused to implement simple-framebuffer. I could in fact imagine a fairly
simple extension to the existing simple-framebuffer binding that could
look like this for Tegra:

	dc@...00000 {
		compatible = "nvidia,tegra210-display", "simple-framebuffer";
		...
		memory-region = <&framebuffer>;
		width = <1920>;
		height = <1080>;
		stride = <7680>;
		format = "r8g8b8";
		...
	};

That's not dissimilar to what you're proposing above, except that it
moves everything before step 5. into the bootloader's responsibility and
therefore avoids the need for hardware-specific early display code in
the kernel.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists