Re: [PATCH 1/2] drm/amdgpu: add amdgpu_vm_entries_mask
On Mon, Sep 17, 2018 at 05:00:42PM +0800, Christian König wrote:
> Am 17.09.2018 um 10:52 schrieb Huang Rui:
> > On Sat, Sep 15, 2018 at 10:05:40AM +0200, Christian König wrote:
> >> We can't get the mask for the root directory from the number of entries.
> >>
> >> So add a new function to avoid that problem.
> >>
> >> Signed-off-by: Christian König
> >> ---
> >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34
> >> +++---
> >> 1 file changed, 27 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> index a7f9aaa47c49..aaf54fc8cafe 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> @@ -190,6 +190,26 @@ static unsigned amdgpu_vm_num_entries(struct
> >> amdgpu_device *adev,
> >>return AMDGPU_VM_PTE_COUNT(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 0x;
> >> + else if (level != AMDGPU_VM_PTB)
> >> + return 0x1f;
> > I think the mask should be 0x1ff (9 bits) while the level is on PDB. The
> > purpose here is to figure out the PTE entry id with the mask, am I right.
> > So the mask should bit width of one entry.
>
> Oh, yes of course that is a typo.
>
> Thanks for catching,
> Christian.
No problem~
With that fixed, feel free to add my RB:
Reviewed-by: Huang Rui
>
> >
> > Thanks,
> > Ray
> >
> >> + else
> >> + return AMDGPU_VM_PTE_COUNT(adev) - 1;
> >> +}
> >> +
> >> /**
> >>* amdgpu_vm_bo_size - returns the size of the BOs in bytes
> >>*
> >> @@ -399,17 +419,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 num_entries, shift, idx;
> >> + unsigned mask, shift, idx;
> >>
> >>if (!cursor->entry->entries)
> >>return false;
> >>
> >>BUG_ON(!cursor->entry->base.bo);
> >> - num_entries = amdgpu_vm_num_entries(adev, cursor->level);
> >> + mask = amdgpu_vm_entries_mask(adev, cursor->level);
> >>shift = amdgpu_vm_level_shift(adev, cursor->level);
> >>
> >>++cursor->level;
> >> - idx = (cursor->pfn >> shift) % num_entries;
> >> + idx = (cursor->pfn >> shift) & mask;
> >>cursor->parent = cursor->entry;
> >>cursor->entry = >entry->entries[idx];
> >>return true;
> >> @@ -1599,7 +1619,7 @@ static int amdgpu_vm_update_ptes(struct
> >> amdgpu_pte_update_params *params,
> >>amdgpu_vm_pt_start(adev, params->vm, start, );
> >>while (cursor.pfn < end) {
> >>struct amdgpu_bo *pt = cursor.entry->base.bo;
> >> - unsigned shift, parent_shift, num_entries;
> >> + unsigned shift, parent_shift, mask;
> >>uint64_t incr, entry_end, pe_start;
> >>
> >>if (!pt)
> >> @@ -1654,9 +1674,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;
> >> - num_entries = amdgpu_vm_num_entries(adev, cursor.level);
> >> - pe_start = ((cursor.pfn >> shift) & (num_entries - 1)) * 8;
> >> - entry_end = num_entries << shift;
> >> + mask = amdgpu_vm_entries_mask(adev, cursor.level);
> >> + pe_start = ((cursor.pfn >> shift) & mask) * 8;
> >> + entry_end = (mask + 1) << shift;
> >>entry_end += cursor.pfn & ~(entry_end - 1);
> >>entry_end = min(entry_end, end);
> >>
> >> --
> >> 2.14.1
> >>
> >> ___
> >> 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
Re: [PATCH 1/2] drm/amdgpu: add amdgpu_vm_entries_mask
Am 17.09.2018 um 10:52 schrieb Huang Rui:
On Sat, Sep 15, 2018 at 10:05:40AM +0200, Christian König wrote:
We can't get the mask for the root directory from the number of entries.
So add a new function to avoid that problem.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34 +++---
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a7f9aaa47c49..aaf54fc8cafe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -190,6 +190,26 @@ static unsigned amdgpu_vm_num_entries(struct amdgpu_device
*adev,
return AMDGPU_VM_PTE_COUNT(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 0x;
+ else if (level != AMDGPU_VM_PTB)
+ return 0x1f;
I think the mask should be 0x1ff (9 bits) while the level is on PDB. The
purpose here is to figure out the PTE entry id with the mask, am I right.
So the mask should bit width of one entry.
Oh, yes of course that is a typo.
Thanks for catching,
Christian.
Thanks,
Ray
+ else
+ return AMDGPU_VM_PTE_COUNT(adev) - 1;
+}
+
/**
* amdgpu_vm_bo_size - returns the size of the BOs in bytes
*
@@ -399,17 +419,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 num_entries, shift, idx;
+ unsigned mask, shift, idx;
if (!cursor->entry->entries)
return false;
BUG_ON(!cursor->entry->base.bo);
- num_entries = amdgpu_vm_num_entries(adev, cursor->level);
+ mask = amdgpu_vm_entries_mask(adev, cursor->level);
shift = amdgpu_vm_level_shift(adev, cursor->level);
++cursor->level;
- idx = (cursor->pfn >> shift) % num_entries;
+ idx = (cursor->pfn >> shift) & mask;
cursor->parent = cursor->entry;
cursor->entry = >entry->entries[idx];
return true;
@@ -1599,7 +1619,7 @@ static int amdgpu_vm_update_ptes(struct
amdgpu_pte_update_params *params,
amdgpu_vm_pt_start(adev, params->vm, start, );
while (cursor.pfn < end) {
struct amdgpu_bo *pt = cursor.entry->base.bo;
- unsigned shift, parent_shift, num_entries;
+ unsigned shift, parent_shift, mask;
uint64_t incr, entry_end, pe_start;
if (!pt)
@@ -1654,9 +1674,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;
- num_entries = amdgpu_vm_num_entries(adev, cursor.level);
- pe_start = ((cursor.pfn >> shift) & (num_entries - 1)) * 8;
- entry_end = num_entries << shift;
+ mask = amdgpu_vm_entries_mask(adev, cursor.level);
+ pe_start = ((cursor.pfn >> shift) & mask) * 8;
+ entry_end = (mask + 1) << shift;
entry_end += cursor.pfn & ~(entry_end - 1);
entry_end = min(entry_end, end);
--
2.14.1
___
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
Re: [PATCH 1/2] drm/amdgpu: add amdgpu_vm_entries_mask
On Sat, Sep 15, 2018 at 10:05:40AM +0200, Christian König wrote:
> We can't get the mask for the root directory from the number of entries.
>
> So add a new function to avoid that problem.
>
> Signed-off-by: Christian König
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34
> +++---
> 1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index a7f9aaa47c49..aaf54fc8cafe 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -190,6 +190,26 @@ static unsigned amdgpu_vm_num_entries(struct
> amdgpu_device *adev,
> return AMDGPU_VM_PTE_COUNT(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 0x;
> + else if (level != AMDGPU_VM_PTB)
> + return 0x1f;
I think the mask should be 0x1ff (9 bits) while the level is on PDB. The
purpose here is to figure out the PTE entry id with the mask, am I right.
So the mask should bit width of one entry.
Thanks,
Ray
> + else
> + return AMDGPU_VM_PTE_COUNT(adev) - 1;
> +}
> +
> /**
> * amdgpu_vm_bo_size - returns the size of the BOs in bytes
> *
> @@ -399,17 +419,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 num_entries, shift, idx;
> + unsigned mask, shift, idx;
>
> if (!cursor->entry->entries)
> return false;
>
> BUG_ON(!cursor->entry->base.bo);
> - num_entries = amdgpu_vm_num_entries(adev, cursor->level);
> + mask = amdgpu_vm_entries_mask(adev, cursor->level);
> shift = amdgpu_vm_level_shift(adev, cursor->level);
>
> ++cursor->level;
> - idx = (cursor->pfn >> shift) % num_entries;
> + idx = (cursor->pfn >> shift) & mask;
> cursor->parent = cursor->entry;
> cursor->entry = >entry->entries[idx];
> return true;
> @@ -1599,7 +1619,7 @@ static int amdgpu_vm_update_ptes(struct
> amdgpu_pte_update_params *params,
> amdgpu_vm_pt_start(adev, params->vm, start, );
> while (cursor.pfn < end) {
> struct amdgpu_bo *pt = cursor.entry->base.bo;
> - unsigned shift, parent_shift, num_entries;
> + unsigned shift, parent_shift, mask;
> uint64_t incr, entry_end, pe_start;
>
> if (!pt)
> @@ -1654,9 +1674,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;
> - num_entries = amdgpu_vm_num_entries(adev, cursor.level);
> - pe_start = ((cursor.pfn >> shift) & (num_entries - 1)) * 8;
> - entry_end = num_entries << shift;
> + mask = amdgpu_vm_entries_mask(adev, cursor.level);
> + pe_start = ((cursor.pfn >> shift) & mask) * 8;
> + entry_end = (mask + 1) << shift;
> entry_end += cursor.pfn & ~(entry_end - 1);
> entry_end = min(entry_end, end);
>
> --
> 2.14.1
>
> ___
> 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
[PATCH 1/2] drm/amdgpu: add amdgpu_vm_entries_mask
We can't get the mask for the root directory from the number of entries.
So add a new function to avoid that problem.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34 +++---
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a7f9aaa47c49..aaf54fc8cafe 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -190,6 +190,26 @@ static unsigned amdgpu_vm_num_entries(struct amdgpu_device
*adev,
return AMDGPU_VM_PTE_COUNT(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 0x;
+ else if (level != AMDGPU_VM_PTB)
+ return 0x1f;
+ else
+ return AMDGPU_VM_PTE_COUNT(adev) - 1;
+}
+
/**
* amdgpu_vm_bo_size - returns the size of the BOs in bytes
*
@@ -399,17 +419,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 num_entries, shift, idx;
+ unsigned mask, shift, idx;
if (!cursor->entry->entries)
return false;
BUG_ON(!cursor->entry->base.bo);
- num_entries = amdgpu_vm_num_entries(adev, cursor->level);
+ mask = amdgpu_vm_entries_mask(adev, cursor->level);
shift = amdgpu_vm_level_shift(adev, cursor->level);
++cursor->level;
- idx = (cursor->pfn >> shift) % num_entries;
+ idx = (cursor->pfn >> shift) & mask;
cursor->parent = cursor->entry;
cursor->entry = >entry->entries[idx];
return true;
@@ -1599,7 +1619,7 @@ static int amdgpu_vm_update_ptes(struct
amdgpu_pte_update_params *params,
amdgpu_vm_pt_start(adev, params->vm, start, );
while (cursor.pfn < end) {
struct amdgpu_bo *pt = cursor.entry->base.bo;
- unsigned shift, parent_shift, num_entries;
+ unsigned shift, parent_shift, mask;
uint64_t incr, entry_end, pe_start;
if (!pt)
@@ -1654,9 +1674,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;
- num_entries = amdgpu_vm_num_entries(adev, cursor.level);
- pe_start = ((cursor.pfn >> shift) & (num_entries - 1)) * 8;
- entry_end = num_entries << shift;
+ mask = amdgpu_vm_entries_mask(adev, cursor.level);
+ pe_start = ((cursor.pfn >> shift) & mask) * 8;
+ entry_end = (mask + 1) << shift;
entry_end += cursor.pfn & ~(entry_end - 1);
entry_end = min(entry_end, end);
--
2.14.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
