Re: [PATCH 04/13] drm/xe/sa: Drop useless is_iomem member

[Michal Wajdeczko <michal.wajdeczko@intel.com>]

On 12.12.2024 04:09, Matthew Brost wrote:
> On Thu, Dec 12, 2024 at 02:01:32AM +0100, Michal Wajdeczko wrote:
>> We can rely on the sa->bo->vmap.is_iomem instead as sa->bo is
>> still available at this moment.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> ---
>>  drivers/gpu/drm/xe/xe_sa.c       | 3 +--
>>  drivers/gpu/drm/xe/xe_sa_types.h | 1 -
>>  2 files changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_sa.c b/drivers/gpu/drm/xe/xe_sa.c
>> index a5e05237b646..b990bd8a49fd 100644
>> --- a/drivers/gpu/drm/xe/xe_sa.c
>> +++ b/drivers/gpu/drm/xe/xe_sa.c
>> @@ -24,7 +24,7 @@ static void sa_bo_manager_fini_bo(void *arg)
>>  {
>>  	struct xe_sa_manager *sa_manager = arg;
>>  
>> -	if (sa_manager->is_iomem)
>> +	if (sa_manager->bo->vmap.is_iomem)
> 
> If we do this anywhere else in the driver (e.g. check bo->vmap.is_iomem)
> then maybe add a BO layer helper for this check. If not, fine with this
> open coded here.

actually there is xe_bo_is_vram() so maybe no need to open code vmap

> 
> Anyways:
> Reviewed-by: Matthew Brost <matthew.brost@intel.com>

thanks, but without patch 3/13 with additional cleanup at dev removal
time, we can't drop is_iomem since we can't access bo pointer at drm
removal, as bo is already gone

> 
>>  		kvfree(sa_manager->cpu_ptr);
>>  
>>  	sa_manager->cpu_ptr = NULL;
>> @@ -55,7 +55,6 @@ struct xe_sa_manager *xe_sa_bo_manager_init(struct xe_tile *tile, u32 size, u32
>>  		return ERR_CAST(bo);
>>  	}
>>  	sa_manager->bo = bo;
>> -	sa_manager->is_iomem = bo->vmap.is_iomem;
>>  	sa_manager->gpu_addr = xe_bo_ggtt_addr(bo);
>>  
>>  	if (bo->vmap.is_iomem) {
>> diff --git a/drivers/gpu/drm/xe/xe_sa_types.h b/drivers/gpu/drm/xe/xe_sa_types.h
>> index 2b070ff1292e..2ef896aeca1d 100644
>> --- a/drivers/gpu/drm/xe/xe_sa_types.h
>> +++ b/drivers/gpu/drm/xe/xe_sa_types.h
>> @@ -14,7 +14,6 @@ struct xe_sa_manager {
>>  	struct xe_bo *bo;
>>  	u64 gpu_addr;
>>  	void *cpu_ptr;
>> -	bool is_iomem;
>>  };
>>  
>>  #endif
>> -- 
>> 2.47.1
>>