[Intel-xe] [PATCH 1/1] drm/xe: Fix stolen memory size calculation

[Matthew Auld]

On 12/06/2023 16:04, Iddamsetty, Aravind wrote:
> 
> 
> On 12-06-2023 19:59, Ruhl, Michael J wrote:
>>> -----Original Message-----
>>> From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of
>>> Aravind Iddamsetty
>>> Sent: Friday, June 9, 2023 2:53 PM
>>> To: intel-xe at lists.freedesktop.org
>>> Cc: Roper, Matthew D <matthew.d.roper at intel.com>
>>> Subject: [Intel-xe] [PATCH 1/1] drm/xe: Fix stolen memory size calculation
>>>
>>> The stolen memory base should be derived from GSMBASE or
>>> FLAT_CCS_BASE_ADDR when flat ccs is applicable.
>>>
>>> Fixes: acf067c2f5a5 ("drm/xe: Implement stolen memory.")
>>>
>>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty at intel.com>
>>> ---
>>> drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c | 7 +++----
>>> 1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
>>> b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
>>> index 21ecc734f10a..29ce69bf8200 100644
>>> --- a/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
>>> +++ b/drivers/gpu/drm/xe/xe_ttm_stolen_mgr.c
>>> @@ -54,7 +54,6 @@ bool xe_ttm_stolen_cpu_access_needs_ggtt(struct
>>> xe_device *xe)
>>> static s64 detect_bar2_dgfx(struct xe_device *xe, struct xe_ttm_stolen_mgr
>>> *mgr)
>>> {
>>> 	struct xe_tile *tile = xe_device_get_root_tile(xe);
>>> -	struct xe_gt *mmio = xe_root_mmio_gt(xe);
>>> 	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>> 	u64 stolen_size;
>>> 	u64 tile_offset;
>>> @@ -66,12 +65,12 @@ static s64 detect_bar2_dgfx(struct xe_device *xe,
>>> struct xe_ttm_stolen_mgr *mgr)
>>> 		return 0;
>>> 	}
>>>
>>> -	/* Use DSM base address instead for stolen memory */
>>> -	mgr->stolen_base = (xe_mmio_read64(mmio, DSMBASE) & >BDSM_MASK) - tile_offset;
>>> +	/* stolen base should be at end of usable vram */
>>> +	mgr->stolen_base = vram_size;
>>
>> Hi Arvind,
>>
>> The stolen memory in i915 is not calculated like this.  It uses the DSMBASE for the offset like the
>> current code.
>>
>> The lmem size includes the CSS sizing, but that is for the LMEM memory region.
>>
>> This is calculating and creating the "stolen" region for use in kernel space.  If the offset includes the CSS space,
>> won't the DSM usage be in the wrong place?
> 
> I didn't realize until you mentioned here we wanted to carve out usable
> stolen in which case what I did is wrong, makes sense.
> 
> but why is that we are doing only on root tile even remote tile has
> stolen area right?

I think the main reason is because there is no user in the driver for it 
outside of display stuff, and display only cares about root tile atm. If 
we can actively utilise it outside of display (like some per tile 
resource needed by the driver) then maybe it makes sense to probe it for 
all tiles, assuming it's still present on such platforms.

> 
> Thanks,
> Aravind.
>>
>>> 	if (drm_WARN_ON(&xe->drm, tile_size < mgr->stolen_base))
>>> 		return 0;
>>>
>>> -	stolen_size = tile_size - mgr->stolen_base;
>>> +	stolen_size = tile_size - vram_size;
>>
>> The tile size was picked to work with devices that do not have tile information (pci_resource _len used).  vram_size
>> can be wrong in those instances.
>>
>> My original patch set used the vram_size, but that was not accurate for this usage.
>>
>> Are you seeing an issue with the current code?
>>
>> Thanks
>>
>> Mike
>>
>>> 	/* Verify usage fits in the actual resource available */
>>> 	if (mgr->stolen_base + stolen_size <= pci_resource_len(pdev, >GEN12_LMEM_BAR))
>>> --
>>> 2.25.1
>>