<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> 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 <div style="color: rgb(0, 0, 0);"> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Zhang, Jerry Sent: Tuesday, October 23, 2018 10:02 AM To: Zhu, Rex; amd-gfx@lists.freedesktop.org; Koenig, Christian Subject: Re: [PATCH] drm/amdgpu: Fix amdgpu_vm_alloc_pts failed <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">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@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); > </div> </div> </div> </div> </body> </html>