[PATCH v3 01/11] rockchip: sdram: Support getting banks from TPL for rk3568 and rk3588

[Quentin Schulz]

Hi Jonas,

On 4/24/24 00:40, Jonas Karlman wrote:
> Hi Quentin,
> 
> On 2024-04-15 16:16, Quentin Schulz wrote:
>> From: Quentin Schulz <quentin.schulz at theobroma-systems.com>
[...]

>> +			if (!(tmp_mem_map->attrs & PTE_BLOCK_NON_SHARE)) {
> 
> This check does not seem to work because PTE_BLOCK_NON_SHARE evaluates
> to 0. Because of this the logic to split the 0-8 GiB bank reported on
> rk3568 is never split in two.
> 

Oof, that's a bit oversight, thanks for the catch.

Can you test the following please?

"""
diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c
index 5b1ff1e5495..0492f9b9f41 100644
--- a/arch/arm/mach-rockchip/sdram.c
+++ b/arch/arm/mach-rockchip/sdram.c
@@ -196,7 +196,23 @@ static int rockchip_dram_init_banksize(void)
  			const phys_size_t rsrv_size = tmp_mem_map->size;
  			const phys_addr_t rsrv_end = rsrv_start + rsrv_size;

-			if (!(tmp_mem_map->attrs & PTE_BLOCK_NON_SHARE)) {
+			/*
+			 * DRAM memories are expected by Arm to be marked as
+			 * Normal Write-back cacheable, Inner shareable[1], so
+			 * let's filter on that to put holes in non-DRAM areas.
+			 *
+			 * [1] 
https://developer.arm.com/documentation/102376/0200/Cacheability-and-shareability-attributes
+			 */
+			const u64 dram_attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
+				PTE_BLOCK_INNER_SHARE;
+			/*
+			 * (AttrIndx | SH) in Lower Attributes of Block
+			 * Descriptor[2].
+			 * [2] 
https://developer.arm.com/documentation/102376/0200/Describing-memory-in-AArch64
+			 */
+			const u64 attrs_mask = PMD_ATTRINDX_MASK | GENMASK(9, 8);
+
+			if ((tmp_mem_map->attrs & attrs_mask) == dram_attrs) {
  				tmp_mem_map++;
  				continue;
  			}
"""

The DRAM mem_map entry for Rockchip devices seems to have those 
attributes and no other non-DRAM entry seems to have 
PTE_BLOCK_MEMTYPE(MT_NORMAL).

We may have an issue in the future if we also want to mark SRAM, ROM or 
flash 
(https://developer.arm.com/documentation/102376/0200/Normal-memory) 
because it's likely those would also match the same attributes but we 
would need to put holes for those so that they aren't thought to be 
DRAM, but I guess we can tackle this the day this happens :)

Thanks again for the catch, let me know if this helps and makes sense 
and I'll send a v4 for it.

Cheers,
Quentin