All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Christian König" <ckoenig.leichtzumerken@gmail.com>
To: Felix Kuehling <felix.kuehling@amd.com>,
	Eric Huang <jinhuieric.huang@amd.com>,
	amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 3/4] drm/amdgpu: Add flush_tlb parameter to amdgpu_vm_bo_update
Date: Wed, 2 Jun 2021 09:02:57 +0200	[thread overview]
Message-ID: <5b4e7d78-4e1e-cd88-be0d-bd3e131cf37a@gmail.com> (raw)
In-Reply-To: <2db33600-8504-db7e-7cbe-f16247da7f8e@amd.com>

Am 02.06.21 um 04:54 schrieb Felix Kuehling:
> Am 2021-06-01 um 6:59 p.m. schrieb Eric Huang:
>> It is to pass the flag to KFD, and optimize table_freed in
>> amdgpu_vm_bo_update_mapping.
>>
>> Signed-off-by: Eric Huang <jinhuieric.huang@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  |  6 +++---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  | 10 +++++-----
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h  |  2 +-
>>   4 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> index e9f9f462a652..e3df132e53a5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>> @@ -916,7 +916,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
>>   	if (r)
>>   		return r;
>>   
>> -	r = amdgpu_vm_bo_update(adev, fpriv->prt_va, false);
>> +	r = amdgpu_vm_bo_update(adev, fpriv->prt_va, false, NULL);
>>   	if (r)
>>   		return r;
>>   
>> @@ -927,7 +927,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
>>   	if (amdgpu_mcbp || amdgpu_sriov_vf(adev)) {
>>   		bo_va = fpriv->csa_va;
>>   		BUG_ON(!bo_va);
>> -		r = amdgpu_vm_bo_update(adev, bo_va, false);
>> +		r = amdgpu_vm_bo_update(adev, bo_va, false, NULL);
>>   		if (r)
>>   			return r;
>>   
>> @@ -946,7 +946,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
>>   		if (bo_va == NULL)
>>   			continue;
>>   
>> -		r = amdgpu_vm_bo_update(adev, bo_va, false);
>> +		r = amdgpu_vm_bo_update(adev, bo_va, false, NULL);
>>   		if (r)
>>   			return r;
>>   
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 2120a87a949f..eac2fd0048cc 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -696,7 +696,7 @@ static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
>>   
>>   	if (operation == AMDGPU_VA_OP_MAP ||
>>   	    operation == AMDGPU_VA_OP_REPLACE) {
>> -		r = amdgpu_vm_bo_update(adev, bo_va, false);
>> +		r = amdgpu_vm_bo_update(adev, bo_va, false, NULL);
>>   		if (r)
>>   			goto error;
>>   	}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 2c20bba7dc1a..fed3d44b5ded 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1729,7 +1729,7 @@ int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>>   	r = vm->update_funcs->commit(&params, fence);
>>   
>>   	if (table_freed)
>> -		*table_freed = params.table_freed;
>> +		*table_freed = *table_freed || params.table_freed;
>>   
>>   error_unlock:
>>   	amdgpu_vm_eviction_unlock(vm);
>> @@ -1793,7 +1793,7 @@ void amdgpu_vm_get_memory(struct amdgpu_vm *vm, uint64_t *vram_mem,
>>    * 0 for success, -EINVAL for failure.
>>    */
>>   int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
>> -			bool clear)
>> +			bool clear, bool *flush_tlb)
> To be consistent with amdgpu_vm_bo_update_mapping I'd name this
> parameter table_freed.
>
>
>>   {
>>   	struct amdgpu_bo *bo = bo_va->base.bo;
>>   	struct amdgpu_vm *vm = bo_va->base.vm;
>> @@ -1887,7 +1887,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev, struct amdgpu_bo_va *bo_va,
>>   						resv, mapping->start,
>>   						mapping->last, update_flags,
>>   						mapping->offset, mem,
>> -						pages_addr, last_update, NULL,
>> +						pages_addr, last_update, flush_tlb,
>>   						vram_base_offset);
>>   		if (r)
>>   			return r;
>> @@ -2141,7 +2141,7 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>>   
>>   	list_for_each_entry_safe(bo_va, tmp, &vm->moved, base.vm_status) {
>>   		/* Per VM BOs never need to bo cleared in the page tables */
>> -		r = amdgpu_vm_bo_update(adev, bo_va, false);
>> +		r = amdgpu_vm_bo_update(adev, bo_va, false, NULL);
>>   		if (r)
>>   			return r;
>>   	}
>> @@ -2160,7 +2160,7 @@ int amdgpu_vm_handle_moved(struct amdgpu_device *adev,
>>   		else
>>   			clear = true;
>>   
>> -		r = amdgpu_vm_bo_update(adev, bo_va, clear);
>> +		r = amdgpu_vm_bo_update(adev, bo_va, clear, NULL);
>>   		if (r)
>>   			return r;
>>   
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> index 67bba8462e7d..24a63e284a69 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
>> @@ -419,7 +419,7 @@ int amdgpu_vm_bo_update_mapping(struct amdgpu_device *adev,
>>   
>>   int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>>   			struct amdgpu_bo_va *bo_va,
>> -			bool clear);
>> +			bool clear, bool *flush_tlb);
> Same as above. With that fixed, the patch and the series is
>
> Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>
>
> Please also give Christian a chance to review this patch in particular
> before you submit.

With the naming made consistent the patch is Reviewed-by: Christian 
König <christian.koenig@amd.com> as well.

>
> Thanks,
>    Felix
>
>
>>   bool amdgpu_vm_evictable(struct amdgpu_bo *bo);
>>   void amdgpu_vm_bo_invalidate(struct amdgpu_device *adev,
>>   			     struct amdgpu_bo *bo, bool evicted);
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

  reply	other threads:[~2021-06-02  7:03 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-01 22:59 [PATCH 1/4] drm/amdkfd: Add flush-type parameter to kfd_flush_tlb Eric Huang
2021-06-01 22:59 ` [PATCH 2/4] drm/amdkfd: Add heavy-weight TLB flush after unmapping Eric Huang
2021-06-01 22:59 ` [PATCH 3/4] drm/amdgpu: Add flush_tlb parameter to amdgpu_vm_bo_update Eric Huang
2021-06-02  2:54   ` Felix Kuehling
2021-06-02  7:02     ` Christian König [this message]
2021-06-01 22:59 ` [PATCH 4/4] drm/amdkfd: Make TLB flush conditional on mapping Eric Huang
2021-06-02  6:53 ` [PATCH 1/4] drm/amdkfd: Add flush-type parameter to kfd_flush_tlb Christian König
2021-06-02 13:29   ` philip yang
2021-06-02 13:31     ` Christian König
2021-06-02 14:53   ` Felix Kuehling

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5b4e7d78-4e1e-cd88-be0d-bd3e131cf37a@gmail.com \
    --to=ckoenig.leichtzumerken@gmail.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=felix.kuehling@amd.com \
    --cc=jinhuieric.huang@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.