Re: [PATCH 2/2] drm/amdgpu: allow higher level PD invalidations

[Felix Kuehling <felix.kuehling@amd.com>]

Two style nit-picks inline. Otherwise the series is

Reviewed-by: Felix Kuehling <Felix.Kuehling@amd.com>

On 2020-01-22 9:03 a.m., Christian König wrote:
> Allow partial invalidation on unallocated PDs. This is useful when we
> need to silence faults to stop interrupt floods on Vega.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 23 ++++++++++++++++++-----
>   1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 8119f32ca94d..0f79c17118bf 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1467,9 +1467,8 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
>   			 * smaller than the address shift. Go to the next
>   			 * child entry and try again.
>   			 */
> -			if (!amdgpu_vm_pt_descendant(adev, &cursor))
> -				return -ENOENT;
> -			continue;
> +			if (amdgpu_vm_pt_descendant(adev, &cursor))
> +				continue;
>   		} else if (frag >= parent_shift) {
>   			/* If the fragment size is even larger than the parent
>   			 * shift we should go up one level and check it again.
> @@ -1480,8 +1479,19 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
>   		}
>   
>   		pt = cursor.entry->base.bo;
> -		if (!pt)
> -			return -ENOENT;
> +		if (!pt) {
> +			/* We need all PDs and PTs for mapping something, */
> +			if (flags & AMDGPU_PTE_VALID)
> +				return -ENOENT;
> +
> +			/* but unmapping something can happen at a higher
> +			 * level. */

Nit-pick: This comment would upset checkpatch.pl.


> +			if (!amdgpu_vm_pt_ancestor(&cursor))
> +				return -EINVAL;
> +
> +			pt = cursor.entry->base.bo;
> +			shift = parent_shift;
> +		}
>   
>   		/* Looks good so far, calculate parameters for the update */
>   		incr = (uint64_t)AMDGPU_GPU_PAGE_SIZE << shift;
> @@ -1495,6 +1505,9 @@ static int amdgpu_vm_update_ptes(struct amdgpu_vm_update_params *params,
>   			uint64_t upd_end = min(entry_end, frag_end);
>   			unsigned nptes = (upd_end - frag_start) >> shift;
>   
> +			/* This can happen when we set higher level PDs to
> +			 * silent to stop fault floods. */

Same as above.


> +			nptes = max(nptes, 1u);
>   			amdgpu_vm_update_flags(params, pt, cursor.level,
>   					       pe_start, dst, nptes, incr,
>   					       flags | AMDGPU_PTE_FRAG(frag));
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx