Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44014 )
Change subject: src/soc/intel/common/block: Mark only TSEG range as IO_CACHEABLE ......................................................................
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
Patch Set 2:
Please also note that this effectively reverts your own patch CB:21539 and treats things differently than other platforms (where DPR is marked cacheable for instance).
Thanks Nico for digging, if you see CB:21539, that CL was meant to add any reserve range into DRAM resource. The assumption there was that Top_of_RAM (0x7700_0000) till TSEG (0x7b00_0000) is just reserved and TSEG (0x7b00_0000) till BGSM (0x7b80_0000) is cacheable + reserved.
But CB:36216 CL breaks that assumption and making Top_of_RAM till BGSM entire range reserved and cachable.
That's both not true.
i don't understand your comments here. i have provided the value so a simple math will tell that the assumption made by CB:36216 is not true
TSEG = 0x7b000000 Assuming you don't know the value of DPR and reseved top_of_memory = 0x77000000 so the result would be 0x4000000 its not 0 right ?
Current CL is basically to restore the previous assumption where we don't really need to make such bigger memory range cacheable
Making a bigger memory range cacheable has usually better results, though. Unless something must not be cached. Which nobody said yet.
But do we have that much bigger cache either right?
my point is "TSEG - DPR - reserved - top_of_memory == 0" is not valid assumption to make as CB:36216 does
Not true.
I have given the calculation above, can you please help to understand why its still not true ?
I don't have time for this.
I don't know what to say.