diff mbox

[5/7] drm/sun4i: backend: Offset layer buffer address by DRAM starting address

Message ID 20171014040252.9621-6-wens@csie.org (mailing list archive)
State New, archived
Headers show

Commit Message

Chen-Yu Tsai Oct. 14, 2017, 4:02 a.m. UTC
The display backend, as well as other peripherals that have a DRAM
clock gate and access DRAM directly, bypassing the system bus,
address the DRAM starting from 0x0, while physical addresses the
system uses starts from 0x40000000 (or 0x20000000 in A80's case).

Correct the address configured into the backend layer registers
by PHYS_OFFSET to account for this.

Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
Signed-off-by: Chen-Yu Tsai <wens@csie.org>
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Maxime Ripard Oct. 16, 2017, 8 a.m. UTC | #1
Hi,

I've applied all the other patches.

On Sat, Oct 14, 2017 at 12:02:50PM +0800, Chen-Yu Tsai wrote:
> The display backend, as well as other peripherals that have a DRAM
> clock gate and access DRAM directly, bypassing the system bus,
> address the DRAM starting from 0x0, while physical addresses the
> system uses starts from 0x40000000 (or 0x20000000 in A80's case).
> 
> Correct the address configured into the backend layer registers
> by PHYS_OFFSET to account for this.

However, I'm a bit confused at this.

The driver has been working so far, which it wouldn't have been able
to if the address was wrong. How was this problem noticed, and how can
that fix not be an issue in itself?

Thanks!
Maxime
Chen-Yu Tsai Oct. 16, 2017, 8:20 a.m. UTC | #2
Hi,

On Mon, Oct 16, 2017 at 4:00 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> I've applied all the other patches.
>
> On Sat, Oct 14, 2017 at 12:02:50PM +0800, Chen-Yu Tsai wrote:
>> The display backend, as well as other peripherals that have a DRAM
>> clock gate and access DRAM directly, bypassing the system bus,
>> address the DRAM starting from 0x0, while physical addresses the
>> system uses starts from 0x40000000 (or 0x20000000 in A80's case).
>>
>> Correct the address configured into the backend layer registers
>> by PHYS_OFFSET to account for this.
>
> However, I'm a bit confused at this.
>
> The driver has been working so far, which it wouldn't have been able
> to if the address was wrong. How was this problem noticed, and how can
> that fix not be an issue in itself?

This problem was witnessed on the Cubietruck, which has 2GB of RAM.

I believe devices with less RAM function normally due to the DRAM
address wrapping around. CMA seems to always allocate its buffer
at a very high address, close to the end of DRAM. On a 1GB RAM
device, the physical address would be something like 0x78000000.
The DRAM address 0x78000000 would access the same DRAM region as
0x38000000 on a system, as the DRAM address would only span 0x0 ~
0x3fffffff. The bit 0x40000000 is non-functional in this case.

However on the Cubietruck, the DRAM is 2GB. The physical address
is 0x40000000 ~ 0xbfffffff. The buffer would be something like
0xb8000000. But the DRAM address span 0x0 ~ 0x7fffffff, meaning
the buffer address wraps around to 0x38000000, which is wrong.
The correct DRAM address for it should be 0x78000000.

ChenYu

>
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
Icenowy Zheng Oct. 16, 2017, 9:42 a.m. UTC | #3
在 2017-10-16 16:00,Maxime Ripard 写道:
> Hi,
> 
> I've applied all the other patches.
> 
> On Sat, Oct 14, 2017 at 12:02:50PM +0800, Chen-Yu Tsai wrote:
>> The display backend, as well as other peripherals that have a DRAM
>> clock gate and access DRAM directly, bypassing the system bus,
>> address the DRAM starting from 0x0, while physical addresses the
>> system uses starts from 0x40000000 (or 0x20000000 in A80's case).
>> 
>> Correct the address configured into the backend layer registers
>> by PHYS_OFFSET to account for this.
> 
> However, I'm a bit confused at this.
> 
> The driver has been working so far, which it wouldn't have been able
> to if the address was wrong. How was this problem noticed, and how can
> that fix not be an issue in itself?

On devices with <=1GiB (0x40000000) DRAM, the DRAM access will just wrap
back and no problem would be seen.

> 
> Thanks!
> Maxime
> 
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
Maxime Ripard Oct. 16, 2017, 8:15 p.m. UTC | #4
On Mon, Oct 16, 2017 at 04:20:32PM +0800, Chen-Yu Tsai wrote:
> > On Sat, Oct 14, 2017 at 12:02:50PM +0800, Chen-Yu Tsai wrote:
> >> The display backend, as well as other peripherals that have a DRAM
> >> clock gate and access DRAM directly, bypassing the system bus,
> >> address the DRAM starting from 0x0, while physical addresses the
> >> system uses starts from 0x40000000 (or 0x20000000 in A80's case).
> >>
> >> Correct the address configured into the backend layer registers
> >> by PHYS_OFFSET to account for this.
> >
> > However, I'm a bit confused at this.
> >
> > The driver has been working so far, which it wouldn't have been able
> > to if the address was wrong. How was this problem noticed, and how can
> > that fix not be an issue in itself?
> 
> This problem was witnessed on the Cubietruck, which has 2GB of RAM.
> 
> I believe devices with less RAM function normally due to the DRAM
> address wrapping around. CMA seems to always allocate its buffer
> at a very high address, close to the end of DRAM. On a 1GB RAM
> device, the physical address would be something like 0x78000000.
> The DRAM address 0x78000000 would access the same DRAM region as
> 0x38000000 on a system, as the DRAM address would only span 0x0 ~
> 0x3fffffff. The bit 0x40000000 is non-functional in this case.
> 
> However on the Cubietruck, the DRAM is 2GB. The physical address
> is 0x40000000 ~ 0xbfffffff. The buffer would be something like
> 0xb8000000. But the DRAM address span 0x0 ~ 0x7fffffff, meaning
> the buffer address wraps around to 0x38000000, which is wrong.
> The correct DRAM address for it should be 0x78000000.

That looks great. Can you put that in the commit log?

Thanks!
Maxime
Chen-Yu Tsai Oct. 17, 2017, 3:40 a.m. UTC | #5
On Tue, Oct 17, 2017 at 4:15 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> On Mon, Oct 16, 2017 at 04:20:32PM +0800, Chen-Yu Tsai wrote:
>> > On Sat, Oct 14, 2017 at 12:02:50PM +0800, Chen-Yu Tsai wrote:
>> >> The display backend, as well as other peripherals that have a DRAM
>> >> clock gate and access DRAM directly, bypassing the system bus,
>> >> address the DRAM starting from 0x0, while physical addresses the
>> >> system uses starts from 0x40000000 (or 0x20000000 in A80's case).
>> >>
>> >> Correct the address configured into the backend layer registers
>> >> by PHYS_OFFSET to account for this.
>> >
>> > However, I'm a bit confused at this.
>> >
>> > The driver has been working so far, which it wouldn't have been able
>> > to if the address was wrong. How was this problem noticed, and how can
>> > that fix not be an issue in itself?
>>
>> This problem was witnessed on the Cubietruck, which has 2GB of RAM.
>>
>> I believe devices with less RAM function normally due to the DRAM
>> address wrapping around. CMA seems to always allocate its buffer
>> at a very high address, close to the end of DRAM. On a 1GB RAM
>> device, the physical address would be something like 0x78000000.
>> The DRAM address 0x78000000 would access the same DRAM region as
>> 0x38000000 on a system, as the DRAM address would only span 0x0 ~
>> 0x3fffffff. The bit 0x40000000 is non-functional in this case.
>>
>> However on the Cubietruck, the DRAM is 2GB. The physical address
>> is 0x40000000 ~ 0xbfffffff. The buffer would be something like
>> 0xb8000000. But the DRAM address span 0x0 ~ 0x7fffffff, meaning
>> the buffer address wraps around to 0x38000000, which is wrong.
>> The correct DRAM address for it should be 0x78000000.
>
> That looks great. Can you put that in the commit log?

Will do.

ChenYu
diff mbox

Patch

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c
index 4fefd8add714..d18d7e88d511 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -216,6 +216,13 @@  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;
+
 	/* Write the 32 lower bits of the address (in bits) */
 	lo_paddr = paddr << 3;
 	DRM_DEBUG_DRIVER("Setting address lower bits to 0x%x\n", lo_paddr);