[2/2] drm/amdgpu: calc addr with domain's gpu_offset

Submitted by Cui, Flora on Sept. 9, 2016, 6:30 a.m.

Details

Message ID 1473402651-2395-2-git-send-email-Flora.Cui@amd.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in AMD X.Org drivers

Not browsing as part of any series.

Commit Message

Cui, Flora Sept. 9, 2016, 6:30 a.m.
Change-Id: I8112e9d85866104559ecef7449f50fbb94167382
Signed-off-by: Flora Cui <Flora.Cui@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index a549abd..3d7a3ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1151,7 +1151,7 @@  int amdgpu_vm_bo_update(struct amdgpu_device *adev,
 			break;
 
 		case TTM_PL_VRAM:
-			addr += adev->vm_manager.vram_base_offset;
+			addr += bo_va->bo->tbo.bdev->man[mem->mem_type].gpu_offset;
 			break;
 
 		default:

Comments

NAK, that is clearly incorrect. The gpu_offset are MC addresses while 
the vram_base_offset is a bud address.

Most of the time they are both zero, except for APUs which would break 
with this change.

Regards,
Christian.

Am 09.09.2016 um 08:30 schrieb Flora Cui:
> Change-Id: I8112e9d85866104559ecef7449f50fbb94167382
> Signed-off-by: Flora Cui <Flora.Cui@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index a549abd..3d7a3ab 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1151,7 +1151,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>   			break;
>   
>   		case TTM_PL_VRAM:
> -			addr += adev->vm_manager.vram_base_offset;
> +			addr += bo_va->bo->tbo.bdev->man[mem->mem_type].gpu_offset;
>   			break;
>   
>   		default:
what about commit e6eb9e1cdb740f778fbbacc5f62edd0ce97c2d52?
should the start addr get updated either?

On Fri, Sep 09, 2016 at 09:18:47AM +0200, Christian König wrote:
> NAK, that is clearly incorrect. The gpu_offset are MC addresses while the
> vram_base_offset is a bud address.
> 
> Most of the time they are both zero, except for APUs which would break with
> this change.
> 
> Regards,
> Christian.
> 
> Am 09.09.2016 um 08:30 schrieb Flora Cui:
> >Change-Id: I8112e9d85866104559ecef7449f50fbb94167382
> >Signed-off-by: Flora Cui <Flora.Cui@amd.com>
> >---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >index a549abd..3d7a3ab 100644
> >--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >@@ -1151,7 +1151,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
> >  			break;
> >  		case TTM_PL_VRAM:
> >-			addr += adev->vm_manager.vram_base_offset;
> >+			addr += bo_va->bo->tbo.bdev->man[mem->mem_type].gpu_offset;
> >  			break;
> >  		default:
> 
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
No that patch is correct. See we are dealing with different address 
spaces here.

The TTM code works with MC addresses. E.g. the internal address space as 
seen by everything which runs on system VM (copy operations, ring 
buffers, UVD, VCE etc...).

The MC in turn then works with bus addresses which it either send to the 
PCIE bus or to the VRAM. Those are just a completely different address 
space.

Regards,
Christian.

Am 09.09.2016 um 09:54 schrieb Flora Cui:
> what about commit e6eb9e1cdb740f778fbbacc5f62edd0ce97c2d52?
> should the start addr get updated either?
>
> On Fri, Sep 09, 2016 at 09:18:47AM +0200, Christian König wrote:
>> NAK, that is clearly incorrect. The gpu_offset are MC addresses while the
>> vram_base_offset is a bud address.
>>
>> Most of the time they are both zero, except for APUs which would break with
>> this change.
>>
>> Regards,
>> Christian.
>>
>> Am 09.09.2016 um 08:30 schrieb Flora Cui:
>>> Change-Id: I8112e9d85866104559ecef7449f50fbb94167382
>>> Signed-off-by: Flora Cui <Flora.Cui@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index a549abd..3d7a3ab 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -1151,7 +1151,7 @@ int amdgpu_vm_bo_update(struct amdgpu_device *adev,
>>>   			break;
>>>   		case TTM_PL_VRAM:
>>> -			addr += adev->vm_manager.vram_base_offset;
>>> +			addr += bo_va->bo->tbo.bdev->man[mem->mem_type].gpu_offset;
>>>   			break;
>>>   		default:
>>
>> _______________________________________________
>> 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