[PATCH] drm/amdgpu: Fix amdgpu_vm_alloc_pts failed

[Zhu, Rex]

Thanks Jerry.

Good suggestion.

Use the right mask for PD instand of hardcode.

so don't need to revert the whole patch.


Best Regards

Rex


________________________________
From: Zhang, Jerry
Sent: Tuesday, October 23, 2018 10:02 AM
To: Zhu, Rex; amd-gfx at lists.freedesktop.org; Koenig, Christian
Subject: Re: [PATCH] drm/amdgpu: Fix amdgpu_vm_alloc_pts failed

On 10/23/2018 12:09 AM, Rex Zhu wrote:
> When the va address located in the last pd entry,

Do you mean the root PD?
maybe we need roundup root PD in amdgpu_vm_entries_mask() like
amdgpu_vm_num_entries().

BTW, looks amdgpu_vm_entries_mask() is going to replace the
amdgpu_vm_num_entries()

Jerry
> the alloc_pts will failed.
> caused by
> "drm/amdgpu: add amdgpu_vm_entries_mask v2"
> commit 72af632549b97ead9251bb155f08fefd1fb6f5c3.
>
> Signed-off-by: Rex Zhu <Rex.Zhu at amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34 +++++++---------------------------
>   1 file changed, 7 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 054633b..1a3af72 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -191,26 +191,6 @@ static unsigned amdgpu_vm_num_entries(struct amdgpu_device *adev,
>   }
>
>   /**
> - * amdgpu_vm_entries_mask - the mask to get the entry number of a PD/PT
> - *
> - * @adev: amdgpu_device pointer
> - * @level: VMPT level
> - *
> - * Returns:
> - * The mask to extract the entry number of a PD/PT from an address.
> - */
> -static uint32_t amdgpu_vm_entries_mask(struct amdgpu_device *adev,
> -                                    unsigned int level)
> -{
> -     if (level <= adev->vm_manager.root_level)
> -             return 0xffffffff;
> -     else if (level != AMDGPU_VM_PTB)
> -             return 0x1ff;
> -     else
> -             return AMDGPU_VM_PTE_COUNT(adev) - 1;
> -}
> -
> -/**
>    * amdgpu_vm_bo_size - returns the size of the BOs in bytes
>    *
>    * @adev: amdgpu_device pointer
> @@ -419,17 +399,17 @@ static void amdgpu_vm_pt_start(struct amdgpu_device *adev,
>   static bool amdgpu_vm_pt_descendant(struct amdgpu_device *adev,
>                                    struct amdgpu_vm_pt_cursor *cursor)
>   {
> -     unsigned mask, shift, idx;
> +     unsigned num_entries, shift, idx;
>
>        if (!cursor->entry->entries)
>                return false;
>
>        BUG_ON(!cursor->entry->base.bo);
> -     mask = amdgpu_vm_entries_mask(adev, cursor->level);
> +     num_entries = amdgpu_vm_num_entries(adev, cursor->level);
>        shift = amdgpu_vm_level_shift(adev, cursor->level);
>
>        ++cursor->level;
> -     idx = (cursor->pfn >> shift) & mask;
> +     idx = (cursor->pfn >> shift) % num_entries;
>        cursor->parent = cursor->entry;
>        cursor->entry = &cursor->entry->entries[idx];
>        return true;
> @@ -1618,7 +1598,7 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
>        amdgpu_vm_pt_start(adev, params->vm, start, &cursor);
>        while (cursor.pfn < end) {
>                struct amdgpu_bo *pt = cursor.entry->base.bo;
> -             unsigned shift, parent_shift, mask;
> +             unsigned shift, parent_shift, num_entries;
>                uint64_t incr, entry_end, pe_start;
>
>                if (!pt)
> @@ -1673,9 +1653,9 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
>
>                /* Looks good so far, calculate parameters for the update */
>                incr = AMDGPU_GPU_PAGE_SIZE << shift;
> -             mask = amdgpu_vm_entries_mask(adev, cursor.level);
> -             pe_start = ((cursor.pfn >> shift) & mask) * 8;
> -             entry_end = (mask + 1) << shift;
> +             num_entries = amdgpu_vm_num_entries(adev, cursor.level);
> +             pe_start = ((cursor.pfn >> shift) & (num_entries - 1)) * 8;
> +             entry_end = num_entries << shift;
>                entry_end += cursor.pfn & ~(entry_end - 1);
>                entry_end = min(entry_end, end);
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20181023/20a3c3bf/attachment.html>