Re: [PATCH v2 29/34] staging: vchiq: Add 36-bit address support

From: Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
To: Phil Elwell <phil@raspberrypi.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	linux-media@vger.kernel.org
Cc: "Kieran Bingham" <kieran.bingham@ideasonboard.com>,
	"Jacopo Mondi" <jacopo@jmondi.org>,
	"Niklas Söderlund" <niklas.soderlund@ragnatech.se>,
	"Naushir Patuck" <naush@raspberrypi.com>,
	"Dave Stevenson" <dave.stevenson@raspberrypi.com>,
	"Phil Elwell" <phil@raspberrypi.org>
Subject: Re: [PATCH v2 29/34] staging: vchiq: Add 36-bit address support
Date: Tue, 05 May 2020 12:13:03 +0200	[thread overview]
Message-ID: <09a896b78b42d2c78ed3ad468de7e90163b9d6c4.camel@suse.de> (raw)
In-Reply-To: <7cbd4185-0d26-aa26-3f7a-56b5b13e5336@raspberrypi.com>

[-- Attachment #1: Type: text/plain, Size: 3027 bytes --]

On Mon, 2020-05-04 at 21:46 +0100, Phil Elwell wrote:
> Hi Nicolas,
> 
> On 04/05/2020 18:40, Nicolas Saenz Julienne wrote:
> > Hi Phil, Laurent,
> > 
> > On Mon, 2020-05-04 at 12:26 +0300, Laurent Pinchart wrote:
> > > From: Phil Elwell <phil@raspberrypi.org>
> > > 
> > > Conditional on a new compatible string, change the pagelist encoding
> > > such that the top 24 bits are the pfn, leaving 8 bits for run length
> > > (-1).
> > > 
> > > Signed-off-by: Phil Elwell <phil@raspberrypi.org>
> > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > > ---
> > >   .../interface/vchiq_arm/vchiq_2835_arm.c      | 88 ++++++++++++++-----
> > >   .../interface/vchiq_arm/vchiq_arm.c           |  6 ++
> > >   .../interface/vchiq_arm/vchiq_arm.h           |  1 +
> > >   3 files changed, 74 insertions(+), 21 deletions(-)
> > > 
> > > diff --git
> > > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> > > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> > > index 3e422a7eb3f1..ecec84ad4345 100644
> > > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> > > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> > > @@ -16,6 +16,8 @@
> > >   #include <soc/bcm2835/raspberrypi-firmware.h>
> > >     #define TOTAL_SLOTS (VCHIQ_SLOT_ZERO_SLOTS + 2 * 32)
> > > +#define VC_SAFE(x) (g_use_36bit_addrs ? ((u32)(x) | 0xc0000000) :
> > > (u32)(x))
> > > +#define IS_VC_SAFE(x) (g_use_36bit_addrs ? !((x) & ~0x3fffffffull) : 1)
> > 
> > As I commented earlier, this is a sign your dma-ranges are wrong, most of
> > the
> > code below reimplements what is already done by dma-direct (see
> > kernel/dma/direct.c). Once properly setup, you should be able to use
> > whatever
> > phys address dmam_alloc_coherent() provides and drop g_use_36bit_addrs.
> > 
> > Note that on arm32+LPAE, dma-direct/swiotlb are the default dma_ops, so this
> > also applies there.
> 
> As I explained in an offline email, the problem is that VCHIQ needs access to

We discussed this privately, but I wanted to start from scratch, specially as I
hope I won't be the only one reviewing this :).

> two
> kinds of "DMA" addresses - those suitable for the 40-bit DMA channel, and the
> 30-bit addresses that the VPU can use. Since each DT node only has access to a
> single set of DMA ranges, I can't see how to use dma-direct to calculate
> addreses
> for everything, but feel free to submit an alternative implementation showing
> how
> it should be done.

How about this):
 - Move vchiq to /soc
 - Get a handle to the relevant dma controller device pointer (so 30-bit DMA
   controller on old RPis, 40-bit on RPi4)
 - Allocate slotmem/pagelist with dma_alloc_coherent(vpu_dev, ...)
 - Map pages with dma_map_sg(dma_dev, ...)

I hope I'm not missing anything, but short of implementing it and seeing the
end result, I think you'd be free of any rpi[1-3]/rpi4 distinction in the code.

Regards,
Nicolas

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]