[PATCH 1/3] gpu: mali: Use real framebuffer location instead of magic constants

[Siarhei Siamashka]

For security reasons Mali400 driver does not allow userspace code
to map any arbitrary physical memory buffers via MALI_IOC_MEM_MAP_EXT
ioctl. So the address of the framebuffer needs to be initialized
in the driver.

Instead of hardcoding framebuffer address/size constants in
sunxi specific config.h, use sunxi platform file to register
this information.

Signed-off-by: Siarhei Siamashka <siarhei....@gmail.com>
---
.../mali/mali/arch-ca8-virtex820-m400-1/config.h | 7 -------
.../mali/mali/platform/mali400-pmu/mali_platform.c | 24 ++++++++++++++++++++++
2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/mali/mali/arch-ca8-virtex820-m400-1/config.h b/drivers/gpu/mali/mali/arch-ca8-virtex820-m400-1/config.h
index 6cc6a47..baab12e 100644
--- a/drivers/gpu/mali/mali/arch-ca8-virtex820-m400-1/config.h
+++ b/drivers/gpu/mali/mali/arch-ca8-virtex820-m400-1/config.h
@@ -59,13 +59,6 @@ static _mali_osk_resource_t arch_configuration [] =
.flags = _MALI_CPU_WRITEABLE | _MALI_CPU_READABLE | _MALI_MMU_READABLE | _MALI_MMU_WRITEABLE
},
{
- .type = MEM_VALIDATION,
- .description = "Framebuffer",
- .base = 0x5A000000,
- .size = 32 * 1024 * 1024, /*32M*/
- .flags = _MALI_CPU_WRITEABLE | _MALI_CPU_READABLE | _MALI_MMU_READABLE | _MALI_MMU_WRITEABLE
- },
- {
.type = OS_MEMORY,
.description = "OS Memory",
.alloc_order = 1, /* Lowest preference for this memory */
diff --git a/drivers/gpu/mali/mali/platform/mali400-pmu/mali_platform.c b/drivers/gpu/mali/mali/platform/mali400-pmu/mali_platform.c
index 5161faa..d482720 100644
--- a/drivers/gpu/mali/mali/platform/mali400-pmu/mali_platform.c
+++ b/drivers/gpu/mali/mali/platform/mali400-pmu/mali_platform.c
@@ -15,6 +15,7 @@
#include "mali_kernel_common.h"
#include "mali_osk.h"
#include "mali_platform.h"
+#include "mali_mem_validation.h"

#include <linux/module.h>
#include <linux/clk.h>
@@ -22,6 +23,11 @@
#include <mach/clock.h>
#include <plat/sys_config.h>

+#ifdef CONFIG_FB_SUNXI_RESERVED_MEM
+extern unsigned long fb_start;
+extern unsigned long fb_size;
+static int fb_validation_range_added;
+#endif

int mali_clk_div = 3;
module_param(mali_clk_div, int, S_IRUSR | S_IWUSR | S_IWGRP | S_IRGRP | S_IROTH);
@@ -37,6 +43,24 @@ _mali_osk_errcode_t mali_platform_init(void)
int clk_div;
int mali_used = 0;

+#ifdef CONFIG_FB_SUNXI_RESERVED_MEM
+ /* mali_platform_init() may be called multiple times,
+ but we only need to set the validation range once */
+ if (!fb_validation_range_added) {
+ _mali_osk_resource_t fb_resource = {
+ .type = MEM_VALIDATION,
+ .description = "Framebuffer",
+ .base = fb_start,
+ .size = fb_size,
+ .flags = 0
+ };
+ mali_mem_validation_add_range(&fb_resource);
+ MALI_PRINT(("permit MALI_IOC_MEM_MAP_EXT ioctl for framebuffer"
+ " (paddr=0x%08X, size=%d)\n", fb_start, fb_size));
+ fb_validation_range_added = 1;
+ }
+#endif
+
//get mali ahb clock
h_ahb_mali = clk_get(NULL, "ahb_mali");
if(!h_ahb_mali){
--
1.8.1.5

[Siarhei Siamashka]

Registering a chunk of reserved memory at a hardcoded location in the
Mali400 GPU resources table is wrong when CONFIG_SUNXI_MALI_RESERVED_MEM
option is not enabled.

After this fix, Mali400 GPU is configured to use up to 256 MiB of
normal memory when CONFIG_SUNXI_MALI_RESERVED_MEM is not enabled.
When CONFIG_SUNXI_MALI_RESERVED_MEM is enabled, it uses 64 MiB of
reserved memory and up to 192 MiB of normal memory (same as before).

Signed-off-by: Siarhei Siamashka <siarhei....@gmail.com>
---

drivers/gpu/mali/mali/arch-ca8-virtex820-m400-1/config.h | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/mali/mali/arch-ca8-virtex820-m400-1/config.h b/drivers/gpu/mali/mali/arch-ca8-virtex820-m400-1/config.h
index baab12e..ef89b77 100644
--- a/drivers/gpu/mali/mali/arch-ca8-virtex820-m400-1/config.h
+++ b/drivers/gpu/mali/mali/arch-ca8-virtex820-m400-1/config.h
@@ -50,6 +50,7 @@ static _mali_osk_resource_t arch_configuration [] =
.description = "Mali-400 MMU for PP",
.mmu_id = 2
},
+#ifdef CONFIG_SUNXI_MALI_RESERVED_MEM
{
.type = MEMORY,
.description = "Mali Sdram",
@@ -65,6 +66,15 @@ static _mali_osk_resource_t arch_configuration [] =
.size = 192 * 1024 * 1024, /* 64 MB */

.flags = _MALI_CPU_WRITEABLE | _MALI_CPU_READABLE | _MALI_MMU_READABLE | _MALI_MMU_WRITEABLE
},

+#else
+ {
+ .type = OS_MEMORY,
+ .description = "OS Memory",
+ .alloc_order = 0, /* Highest preference for this memory */
+ .size = 256 * 1024 * 1024, /* 256 MB */
+ .flags = _MALI_CPU_WRITEABLE | _MALI_CPU_READABLE | _MALI_MMU_READABLE | _MALI_MMU_WRITEABLE
+ },
+#endif
{
.type = MALI400L2,
.base = 0x01C41000,
--
1.8.1.5

[Siarhei Siamashka]

Mali has MMU and does not strictly need any physically contiguous
memory reservation. Most users will likely prefer more flexible
memory allocation instead of always wasting 64 MiB (even when Mali
is not used or needs much less).

CONFIG_SUNXI_MALI_RESERVED_MEM option is still available and can
be enabled if needed (for performance or some other reasons).

Signed-off-by: Siarhei Siamashka <siarhei....@gmail.com>
---

arch/arm/plat-sunxi/Kconfig | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/plat-sunxi/Kconfig b/arch/arm/plat-sunxi/Kconfig
index 7c46724..b0bc392 100644
--- a/arch/arm/plat-sunxi/Kconfig
+++ b/arch/arm/plat-sunxi/Kconfig
@@ -9,9 +9,14 @@ config SW_DEBUG_UART
output.

config SUNXI_MALI_RESERVED_MEM
- bool
+ bool "Reserve 64 MiB of physically contiguous memory for Mali GPU"
depends on MALI
- default y
+ default n
+ help
+ If enabled, use 64 MiB of dedicated memory and 192 MiB of os memory
+ (instead of 256 MiB of os memory alone) for Mali. Reservation of
+ dedicated memory may or may not improve Mali performance in some
+ cases, but makes this memory unavailable for the rest of the system.

config MACH_SUN4I
bool
--
1.8.1.5

[Hans de Goede]

Hi,

Hmm, can this be made dynamic like in patch 1/1? Currently
with the sunxi_no_mali_mem_reserve kernel cmdline option,
the mali reserved mem can be disabled runtime (When compiled
in).

Also what about the userspace code, last time I checked some
if these addresses are also hardcoded in the blob (IIRC).

Regards,

Hans

[Hans de Goede]

Hi,

Maybe completely get rid of the Kconfig option and only, rely
on the kernel cmdline for this?

Regards,

Hans

On 04/26/2013 05:36 AM, Siarhei Siamashka wrote:

[Siarhei Siamashka]

On Fri, 26 Apr 2013 09:45:40 +0200
Hans de Goede <hdeg...@redhat.com> wrote:

> Hi,
>
> Hmm, can this be made dynamic like in patch 1/1? Currently
> with the sunxi_no_mali_mem_reserve kernel cmdline option,
> the mali reserved mem can be disabled runtime (When compiled
> in).

Right now I don't see any good reason for CONFIG_SUNXI_MALI_RESERVED_MEM
to exist in the first place, but just don't feel like chopping it off
completely yet. At least my limited set of benchmarks did not reveal
any performance regressions so far.

CONFIG_FB_SUNXI_RESERVED_MEM is a different case, because it is
actually needed.

> Also what about the userspace code, last time I checked some
> if these addresses are also hardcoded in the blob (IIRC).

If this is true, I would be really surprised. IMHO it makes no sense
at all. Do you have any link confirming this information?

A reply from Mali people in this forum thread seems to imply
that the reservation exists to prevent the GPU running out of
memory when it is all used up by the rest of the system:

http://forums.arm.com/index.php?/topic/16191-mali-400-video-memory-available/

Maybe we need to run some tests in Linux and in Android to see how it
really behaves under memory pressure?

The following link also seems to provide a hint that the magic 64 MiB
reservation size might be specific to some Android requirements:

https://bugs.launchpad.net/igloocommunity/+bug/925913

--
Best regards,
Siarhei Siamashka

[Hans de Goede]

Hi,

On 04/26/2013 11:10 AM, Siarhei Siamashka wrote:
> On Fri, 26 Apr 2013 09:45:40 +0200
> Hans de Goede <hdeg...@redhat.com> wrote:
>
>> Hi,
>>
>> Hmm, can this be made dynamic like in patch 1/1? Currently
>> with the sunxi_no_mali_mem_reserve kernel cmdline option,
>> the mali reserved mem can be disabled runtime (When compiled
>> in).
>
> Right now I don't see any good reason for CONFIG_SUNXI_MALI_RESERVED_MEM
> to exist in the first place, but just don't feel like chopping it off
> completely yet. At least my limited set of benchmarks did not reveal
> any performance regressions so far.

Right, as I said we should probably make it fully dynamic (from the
kernel cmdline) and make it default to 0, still as you indicate
below it may be needed in some cases.

>
> CONFIG_FB_SUNXI_RESERVED_MEM is a different case, because it is
> actually needed.
>

Right.

>> Also what about the userspace code, last time I checked some
>> if these addresses are also hardcoded in the blob (IIRC).
>
> If this is true, I would be really surprised. IMHO it makes no sense
> at all. Do you have any link confirming this information?

I'm probably wrong on this, when doing the memory reservation from
kernel cmdline patches, mainly commit 9be6628d titled:
"sunxi: Add sunxi_*_mem_reserve kernel cmdline options"

I had to add some magic to lock the fb to a certain address, but that
may very well have been due to the kernel stuff you're now changing
and not due to the userspace parts. I do remember seeing sources
which had the address hardcoded, as the kernel bits do, so that is
probably why I had to do that.

Regards,

Hans

[Siarhei Siamashka]

On Fri, 26 Apr 2013 09:46:17 +0200

Hans de Goede <hdeg...@redhat.com> wrote:

> Hi,
>
> Maybe completely get rid of the Kconfig option and only, rely
> on the kernel cmdline for this?

Yes, this might be possible. But I would like to get some feedback
from the other people first.

I'm currently checking how to best upgrade to r3p2 version of the
kernel driver for mali. Maybe we will end up having both r3p0 and r3p2
mali drivers in the kernel just like ODROID devices.

The X11 mali userspace blobs from the currently used r3p0 version have
some serious problems related to resizing windows. And the userspace
blobs only work with matching kernel driver versions.

The initialization of hardware resources has been changed in r3p2 mali
kernel driver. That's why I looked at this part of code when trying
to adapt to the changes and was quite puzzled about the purpose of
reserving a block of dedicated memory the way it is currently done.

[alex allss]

Main question what mali driver do in no-mem cases?

I think some one need do test no-mem cases( kernel stability)

Also this can produce regressions

26.04.2013 15:41 пользователь "Hans de Goede" <hdeg...@redhat.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/groups/opt_out.

[Siarhei Siamashka]

Hi,

Thanks for providing your feedback.

On Fri, 26 Apr 2013 22:44:45 +0600
alex allss <wing...@gmail.com> wrote:

> Main question what mali driver do in no-mem cases?

There is no no-mem case. As the commit message tries to explain, Mali
driver can work in one of these two configurations depending on whether
CONFIG_SUNXI_MALI_RESERVED_MEM option is enabled or not:

1) 64 MB reserved + 192 MB dynamically allocated (current setup)
2) 256 MB dynamically allocated (new proposed setup)

In both cases Mali can use up to 256 MB memory total and works fine.

> I think some one need do test no-mem cases( kernel stability)

I think it goes without saying that every patch contributor is
responsible for testing his own patches prior to submitting ;-)

But additional testing by more people is always welcome.

> Also this can produce regressions

Any patch potentially can. That's why we are supposed to have
patch review process and a stage branch.

Please let me know if you find any real problem when using it.

[Michal Suchanek]

On 26 April 2013 11:45, Siarhei Siamashka <siarhei....@gmail.com> wrote:
> On Fri, 26 Apr 2013 09:46:17 +0200
> Hans de Goede <hdeg...@redhat.com> wrote:
>
>> Hi,
>>
>> Maybe completely get rid of the Kconfig option and only, rely
>> on the kernel cmdline for this?
>
> Yes, this might be possible. But I would like to get some feedback
> from the other people first.
>

Gnome appears to run reasonably with
# CONFIG_SUNXI_MALI_RESERVED_MEM is not set

Thanks

Michal

[Jari Helaakoski]

2013/4/26 Michal Suchanek <hram...@gmail.com>

Is there possibility that GPU memory goes to swap? This could cause serious performance regression..

Anyway these patches sounds great for devices with 512MB and are really welcome.

-Jari

[Siarhei Siamashka]

On Fri, 26 Apr 2013 20:49:08 +0300
Jari Helaakoski <tek...@gmail.com> wrote:

> 2013/4/26 Michal Suchanek <hram...@gmail.com>
>
> > On 26 April 2013 11:45, Siarhei Siamashka <siarhei....@gmail.com>
> > wrote:
> > > On Fri, 26 Apr 2013 09:46:17 +0200
> > > Hans de Goede <hdeg...@redhat.com> wrote:
> > >
> > >> Hi,
> > >>
> > >> Maybe completely get rid of the Kconfig option and only, rely
> > >> on the kernel cmdline for this?
> > >
> > > Yes, this might be possible. But I would like to get some feedback
> > > from the other people first.
> > >
> >
> > Gnome appears to run reasonably with
> > # CONFIG_SUNXI_MALI_RESERVED_MEM is not set
> >
>
> Is there possibility that GPU memory goes to swap? This could cause serious
> performance regression..

Unless I'm misunderstanding something, this is unlikely. The GPU memory
gets allocated as individual physical pages here:

https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-v3.4.29-r1/drivers/gpu/mali/mali/linux/mali_osk_low_level_mem.c#L123

On the other hand, GPU memory allocations can push other applications
into swap when it is necessary and if swap exists.

And a reasonable limit for GPU memory allocations is also in force
(so that the GPU abusers can't eat all the memory and take down the
whole system). If GPU exceeds this limit, the functions like
glBufferData() start to fail and glGetError() returns GL_OUT_OF_MEMORY.
We also get the following messages in dmesg log:

[ 117.740000] Mali: Out of memory. Mali memory allocated: 261120 kB Configured maximum OS memory usage: 262144 kB
[ 117.850000] Mali: Memory allocate failed, could not allocate size 1280 kB.

> Anyway these patches sounds great for devices with 512MB and are really
> welcome.

I'm an owner of one of these 512MB devices :)