Re: [PATCH v3 5/7] drm/sun4i: Rely on dma interconnect for our RAM offset

From: Maxime Ripard <maxime.ripard@bootlin.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree@vger.kernel.org, Yong Deng <yong.deng@magewell.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Daniel Vetter <daniel.vetter@ffwll.ch>,
	dri-devel@lists.freedesktop.org,
	Dave Martin <dave.martin@arm.com>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Chen-Yu Tsai <wens@csie.org>, Rob Herring <robh+dt@kernel.org>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Frank Rowand <frowand.list@gmail.com>,
	Georgi Djakov <georgi.djakov@linaro.org>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 5/7] drm/sun4i: Rely on dma interconnect for our RAM offset
Date: Wed, 13 Feb 2019 16:41:44 +0100	[thread overview]
Message-ID: <20190213154144.5m5qxbfh2qmzwatn@flea> (raw)
In-Reply-To: <16cb9f67-6b10-fbc3-6222-5364e718721b@arm.com>

[-- Attachment #1.1: Type: text/plain, Size: 2769 bytes --]

Hi Robin,

Thanks for your feedback!

On Tue, Feb 12, 2019 at 06:46:40PM +0000, Robin Murphy wrote:
> On 11/02/2019 15:02, Maxime Ripard wrote:
> > Now that we can express our DMA topology, rely on those property instead of
> > hardcoding an offset from the dma_addr_t which wasn't really great.
> > 
> > We still need to add some code to deal with the old DT that would lack that
> > property, but we move the offset to the DRM device dma_pfn_offset to be
> > able to rely on just the dma_addr_t associated to the GEM object.
> > 
> > Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >   drivers/gpu/drm/sun4i/sun4i_backend.c | 28 +++++++++++++++++++++-------
> >   1 file changed, 21 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > index 9e9255ee59cd..1846a1b30fea 100644
> > --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> > +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> > @@ -383,13 +383,6 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend *backend,
> >   	paddr = drm_fb_cma_get_gem_addr(fb, state, 0);
> >   	DRM_DEBUG_DRIVER("Setting buffer address to %pad\n", &paddr);
> > -	/*
> > -	 * backend DMA accesses DRAM directly, bypassing the system
> > -	 * bus. As such, the address range is different and the buffer
> > -	 * address needs to be corrected.
> > -	 */
> > -	paddr -= PHYS_OFFSET;
> > -
> >   	if (fb->format->is_yuv)
> >   		return sun4i_backend_update_yuv_buffer(backend, fb, paddr);
> > @@ -835,6 +828,27 @@ static int sun4i_backend_bind(struct device *dev, struct device *master,
> >   	dev_set_drvdata(dev, backend);
> >   	spin_lock_init(&backend->frontend_lock);
> > +	if (of_find_property(dev->of_node, "interconnects", NULL)) {
> > +		/*
> > +		 * This assume we have the same DMA constraints for all our the
> > +		 * devices in our pipeline (all the backends, but also the
> > +		 * frontends). This sounds bad, but it has always been the case
> > +		 * for us, and DRM doesn't do per-device allocation either, so
> > +		 * we would need to fix DRM first...
> > +		 */
> > +		ret = of_dma_configure(drm->dev, dev->of_node, true);
> 
> It would be even nicer if we could ensure that drm->dev originates from a DT
> node which has the appropriate interconnects property itself, such that we
> can assume it's already configured correctly.

The thing is drm->dev comes from a node in the DT that is a virtual
node, and therefore doesn't have any resources attached, so I'm not
sure we have any other way, unfortunately.

Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel