[PATCH] drm/etnaviv: Always init gpu->memory_base

[Marek Vasut]

On 08/26/2016 06:06 PM, Russell King - ARM Linux wrote:
> On Fri, Aug 26, 2016 at 04:25:13PM +0200, Marek Vasut wrote:
>> The content of gpu->memory_base should point to start of RAM, not zero.
>>
>> Signed-off-by: Marek Vasut <marex at denx.de>
>> Cc: Lucas Stach <l.stach at pengutronix.de>
>> Cc: Christian Gmeiner <christian.gmeiner at gmail.com>
>> Cc: Russell King <rmk+kernel at arm.linux.org.uk>
>> ---
>>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
>> index ff6aa5d..a168532 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
>> @@ -588,6 +588,8 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
>>  			gpu->memory_base = PHYS_OFFSET;
>>  		else
>>  			gpu->memory_base = dma_mask - SZ_2G + 1;
>> +	} else {
>> +		gpu->memory_base = PHYS_OFFSET;
>>  	}
> 
> The code is correct as-is.  Etnaviv GPUs are not without their own
> weirdness.

I suspected as much. I think my GPU isn't MC2.0 if I read the
minor_features0 bits right .

> For MC1.0 3D GPUs (eg, GC600 as found on Dove), there is an issue
> where _most_ GPU accesses go through a memory offsetting via the
> memory base, but accesses by the tiler do not: they bypass the
> memory base offsetting.
> 
> We have two options to work around that:
> 
> 1. Maintain two GPU address spaces, and mark relocations according
>    to their use.  This is error prone: we would have to have the
>    kernel verify the relocation type is valid for the load state
>    address to avoid obvious security issues, and the maintanence
>    of these different address spaces becomes more complex.
> 
> 2. Force the address offset to zero so all GPU accesses map through
>    the same method irrespective of which GPU block is performing the
>    access.
> 
> We've chosen (2) because it is much simpler, and less error prone,
> plus it fits with the platforms that we're aware of at the moment.
> I did push for the relocations to have an additional member which
> can be used to flag the type of the relocation if necessary in the
> future without changing the interface structure layouts, so we do
> have that option if we really need to do it - but we'd all prefer
> to avoid it.

I see, thanks for the detailed explanation.

> Also, iirc, the GPU tiler with MC1.0 is unable to access addresses
>> = 2GiB no matter what - if you do have a 3D GPU with MC1.0, and
> you have physical memory above 2GiB... there's no currently known
> solution... you lose.

Yes, i.MX6SX has GC400T and the DRAM starts at 0x8000_0000 .
Did I misunderstand something ?

-- 
Best regards,
Marek Vasut