[PATCH] drm/amdgpu: GPU vs CPU page size fixes in amdgpu_vm_bo_split_mapping

[PATCH] drm/amdgpu: GPU vs CPU page size fixes in amdgpu_vm_bo_split_mapping

[Michel Dänzer]

From: Michel Dänzer <michel.daenzer@amd.com>

start / last / max_entries are numbers of GPU pages, pfn / count are
numbers of CPU pages. Convert between them accordingly.

Fixes badness on systems with > 4K page size.

Cc: stable@vger.kernel.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106258
Reported-by: Matt Corallo <freedesktop@bluematt.me>
Tested-by: foxbat@ruin.net
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 7c30451ba897..590db78b8c72 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1577,7 +1577,9 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 			uint64_t count;
 
 			max_entries = min(max_entries, 16ull * 1024ull);
-			for (count = 1; count < max_entries; ++count) {
+			for (count = 1;
+			     count < max_entries / (PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE);
+			     ++count) {
 				uint64_t idx = pfn + count;
 
 				if (pages_addr[idx] !=
@@ -1590,7 +1592,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 				dma_addr = pages_addr;
 			} else {
 				addr = pages_addr[pfn];
-				max_entries = count;
+				max_entries = count * (PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE);
 			}
 
 		} else if (flags & AMDGPU_PTE_VALID) {
@@ -1605,7 +1607,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
 		if (r)
 			return r;
 
-		pfn += last - start + 1;
+		pfn += (last - start + 1) / (PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE);
 		if (nodes && nodes->size == pfn) {
 			pfn = 0;
 			++nodes;
-- 
2.17.1

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

Re: [PATCH] drm/amdgpu: GPU vs CPU page size fixes in amdgpu_vm_bo_split_mapping

[Christian König]

Am 22.06.2018 um 11:10 schrieb Michel Dänzer:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> start / last / max_entries are numbers of GPU pages, pfn / count are
> numbers of CPU pages. Convert between them accordingly.
>
> Fixes badness on systems with > 4K page size.
>
> Cc: stable@vger.kernel.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106258
> Reported-by: Matt Corallo <freedesktop@bluematt.me>
> Tested-by: foxbat@ruin.net
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Nice work! Just one comment: Can we somewhere add a define for 
"(PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE)"?

IIRC that term was used in the GART code as well and just adding it 
somewhere could makes things cleaner if you ask me.

Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 7c30451ba897..590db78b8c72 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1577,7 +1577,9 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>   			uint64_t count;
>   
>   			max_entries = min(max_entries, 16ull * 1024ull);
> -			for (count = 1; count < max_entries; ++count) {
> +			for (count = 1;
> +			     count < max_entries / (PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE);
> +			     ++count) {
>   				uint64_t idx = pfn + count;
>   
>   				if (pages_addr[idx] !=
> @@ -1590,7 +1592,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>   				dma_addr = pages_addr;
>   			} else {
>   				addr = pages_addr[pfn];
> -				max_entries = count;
> +				max_entries = count * (PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE);
>   			}
>   
>   		} else if (flags & AMDGPU_PTE_VALID) {
> @@ -1605,7 +1607,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>   		if (r)
>   			return r;
>   
> -		pfn += last - start + 1;
> +		pfn += (last - start + 1) / (PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE);
>   		if (nodes && nodes->size == pfn) {
>   			pfn = 0;
>   			++nodes;

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

Re: [PATCH] drm/amdgpu: GPU vs CPU page size fixes in amdgpu_vm_bo_split_mapping

[Michel Dänzer]

On 2018-06-22 12:34 PM, Christian König wrote:
> Am 22.06.2018 um 11:10 schrieb Michel Dänzer:
>> From: Michel Dänzer <michel.daenzer@amd.com>
>>
>> start / last / max_entries are numbers of GPU pages, pfn / count are
>> numbers of CPU pages. Convert between them accordingly.
>>
>> Fixes badness on systems with > 4K page size.
>>
>> Cc: stable@vger.kernel.org
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106258
>> Reported-by: Matt Corallo <freedesktop@bluematt.me>
>> Tested-by: foxbat@ruin.net
>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
> 
> Nice work!

Thanks.


> Just one comment: Can we somewhere add a define for
> "(PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE)"?
> 
> IIRC that term was used in the GART code as well and just adding it
> somewhere could makes things cleaner if you ask me.

Sure, but that can be a follow-up patch, right?

Can I get your R-b for this one?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: GPU vs CPU page size fixes in amdgpu_vm_bo_split_mapping

[Alex Deucher]

On Fri, Jun 22, 2018 at 5:10 AM, Michel Dänzer <michel@daenzer.net> wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> start / last / max_entries are numbers of GPU pages, pfn / count are
> numbers of CPU pages. Convert between them accordingly.
>
> Fixes badness on systems with > 4K page size.
>
> Cc: stable@vger.kernel.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106258
> Reported-by: Matt Corallo <freedesktop@bluematt.me>
> Tested-by: foxbat@ruin.net
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 7c30451ba897..590db78b8c72 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1577,7 +1577,9 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>                         uint64_t count;
>
>                         max_entries = min(max_entries, 16ull * 1024ull);
> -                       for (count = 1; count < max_entries; ++count) {
> +                       for (count = 1;
> +                            count < max_entries / (PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE);
> +                            ++count) {
>                                 uint64_t idx = pfn + count;
>
>                                 if (pages_addr[idx] !=
> @@ -1590,7 +1592,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>                                 dma_addr = pages_addr;
>                         } else {
>                                 addr = pages_addr[pfn];
> -                               max_entries = count;
> +                               max_entries = count * (PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE);
>                         }
>
>                 } else if (flags & AMDGPU_PTE_VALID) {
> @@ -1605,7 +1607,7 @@ static int amdgpu_vm_bo_split_mapping(struct amdgpu_device *adev,
>                 if (r)
>                         return r;
>
> -               pfn += last - start + 1;
> +               pfn += (last - start + 1) / (PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE);
>                 if (nodes && nodes->size == pfn) {
>                         pfn = 0;
>                         ++nodes;
> --
> 2.17.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] drm/amdgpu: GPU vs CPU page size fixes in amdgpu_vm_bo_split_mapping

[Christian König]

Am 22.06.2018 um 16:12 schrieb Michel Dänzer:
> On 2018-06-22 12:34 PM, Christian König wrote:
>> Am 22.06.2018 um 11:10 schrieb Michel Dänzer:
>>> From: Michel Dänzer <michel.daenzer@amd.com>
>>>
>>> start / last / max_entries are numbers of GPU pages, pfn / count are
>>> numbers of CPU pages. Convert between them accordingly.
>>>
>>> Fixes badness on systems with > 4K page size.
>>>
>>> Cc: stable@vger.kernel.org
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106258
>>> Reported-by: Matt Corallo <freedesktop@bluematt.me>
>>> Tested-by: foxbat@ruin.net
>>> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
>> Nice work!
> Thanks.
>
>
>> Just one comment: Can we somewhere add a define for
>> "(PAGE_SIZE / AMDGPU_GPU_PAGE_SIZE)"?
>>
>> IIRC that term was used in the GART code as well and just adding it
>> somewhere could makes things cleaner if you ask me.
> Sure, but that can be a follow-up patch, right?
>
> Can I get your R-b for this one?

Certainly, patch is Reviewed-by: Christian König <christian.koenig@amd.com>.

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