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

["Iddamsetty, Aravind" <aravind.iddamsetty@intel.com>]

On 12-06-2023 19:59, Ruhl, Michael J wrote:
>> -----Original Message-----
>> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of
>> Aravind Iddamsetty
>> Sent: Friday, June 9, 2023 2:53 PM
>> To: intel-xe@lists.freedesktop.org
>> Cc: Roper, Matthew D <matthew.d.roper@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@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?

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
>