[PATCH xf86-video-amdgpu] Use plain glamor_egl_create_textured_screen().

2017-05-17 Thread Michel Dänzer
From: Eric Anholt 

Since 5064ffab631 (2014), glamor's implementation of _ext just drops the
back_pixmap arg, which we were passing NULL (the default) to anyway.

Signed-off-by: Eric Anholt 
(Ported from radeon commit 2b7d77b90108911777a11ecaa63435552000c958)

Signed-off-by: Michel Dänzer 
---
 src/amdgpu_glamor.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/amdgpu_glamor.c b/src/amdgpu_glamor.c
index 1c5dfc2d1..5583cd382 100644
--- a/src/amdgpu_glamor.c
+++ b/src/amdgpu_glamor.c
@@ -66,10 +66,9 @@ Bool amdgpu_glamor_create_screen_resources(ScreenPtr screen)
 #endif
 
if (!amdgpu_bo_get_handle(info->front_buffer, &bo_handle) ||
-   !glamor_egl_create_textured_screen_ext(screen,
-  bo_handle,
-  scrn->displayWidth *
-  info->pixel_bytes, NULL)) {
+   !glamor_egl_create_textured_screen(screen, bo_handle,
+  scrn->displayWidth *
+  info->pixel_bytes)) {
return FALSE;
}
 
-- 
2.11.0

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


Re: [PATCH 4/4] drm/amdgpu: reset fpriv vram_lost_counter

2017-05-17 Thread zhoucm1



On 2017年05月17日 14:57, Michel Dänzer wrote:

On 17/05/17 01:28 PM, zhoucm1 wrote:

On 2017年05月17日 11:15, Michel Dänzer wrote:

On 17/05/17 12:04 PM, zhoucm1 wrote:

On 2017年05月17日 09:18, Michel Dänzer wrote:

On 16/05/17 06:25 PM, Chunming Zhou wrote:

Change-Id: I8eb6d7f558da05510e429d3bf1d48c8cec6c1977
Signed-off-by: Chunming Zhou 

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index bca1fb5..f3e7525 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2547,6 +2547,9 @@ int amdgpu_vm_ioctl(struct drm_device *dev,
void *data, struct drm_file *filp)
case AMDGPU_VM_OP_UNRESERVE_VMID:
amdgpu_vm_free_reserved_vmid(adev, &fpriv->vm,
AMDGPU_GFXHUB);
break;
+case AMDGPU_VM_OP_RESET:
+fpriv->vram_lost_counter =
atomic_read(&adev->vram_lost_counter);
+break;

How do you envision the UMDs using this? I can mostly think of them
calling this ioctl when a context is created or destroyed. But that
would also allow any other remaining contexts using the same DRM file
descriptor to use all ioctls again. So, I think there needs to be a
vram_lost_counter in struct amdgpu_ctx instead of in struct
amdgpu_fpriv.

struct amdgpu_fpriv for vram_lost_counter is proper place, especially
for ioctl return value.
if you need to reset ctx one by one, we can mark all contexts of that
vm, and then reset by userspace.

I'm not following. With vram_lost_counter in amdgpu_fpriv, if any
context calls this ioctl, all other contexts using the same file
descriptor will also be considered safe again, right?

Yes, but it really depends on userspace requirement, if you need to
reset ctx one by one, we can mark all contexts of that vm to guilty, and
then reset one context by userspace.

Still not sure what you mean by that.

E.g. what do you mean by "guilty"? I thought that refers to the context
which caused a hang. But it seems like you're using it to refer to any
context which hasn't reacted yet to VRAM contents being lost.

When vram is lost, we treat all contexts need to reset.

Regards,
David Zhou


Also not sure what you mean by "if you need to reset ctx one by one".




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


[PATCH 1/2] drm/amdgpu: fix re-program vm invalidate eng address range for gfxhub on resume

2017-05-17 Thread Huang Rui
Signed-off-by: Huang Rui 
---
 drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
index 005075f..41313514 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -368,7 +368,7 @@ static int gfxhub_v1_0_suspend(void *handle)
 
 static int gfxhub_v1_0_resume(void *handle)
 {
-   return 0;
+   return gfxhub_v1_0_hw_init(handle);
 }
 
 static bool gfxhub_v1_0_is_idle(void *handle)
-- 
2.7.4

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


[PATCH 2/2] drm/amdgpu: fix re-program vm invalidate eng address range for mmhub on resume

2017-05-17 Thread Huang Rui
Signed-off-by: Huang Rui 
---
 drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c 
b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
index dbfe48d..8979bc2 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
@@ -386,7 +386,7 @@ static int mmhub_v1_0_suspend(void *handle)
 
 static int mmhub_v1_0_resume(void *handle)
 {
-   return 0;
+   return mmhub_v1_0_hw_init(handle);
 }
 
 static bool mmhub_v1_0_is_idle(void *handle)
-- 
2.7.4

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


Re: [PATCH 1/2] drm/amdgpu: fix re-program vm invalidate eng address range for gfxhub on resume

2017-05-17 Thread zhoucm1
By this change, I suggest to remove mmhub/gfxhub_v1_0_ip_funcs and their 
IP block, unify them to gmc ip block, this way we cannot lost setting 
when resume back.


Regards,
David Zhou

On 2017年05月17日 15:38, Huang Rui wrote:

Signed-off-by: Huang Rui 
---
  drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
index 005075f..41313514 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -368,7 +368,7 @@ static int gfxhub_v1_0_suspend(void *handle)
  
  static int gfxhub_v1_0_resume(void *handle)

  {
-   return 0;
+   return gfxhub_v1_0_hw_init(handle);
  }
  
  static bool gfxhub_v1_0_is_idle(void *handle)


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


Re: [PATCH 1/2] drm/amdgpu: fix re-program vm invalidate eng address range for gfxhub on resume

2017-05-17 Thread Huang Rui
On Wed, May 17, 2017 at 03:43:47PM +0800, Zhou, David(ChunMing) wrote:
> By this change, I suggest to remove mmhub/gfxhub_v1_0_ip_funcs and their
> IP block, unify them to gmc ip block, this way we cannot lost setting
> when resume back.
> 

From hw side, wo won't have real gmc since this chip, mmhub and gfxhub(gc)
instead of it. Maybe we would better to align with hw desgin.

Thanks,
Rui

> Regards,
> David Zhou
> 
> On 2017年05月17日 15:38, Huang Rui wrote:
> > Signed-off-by: Huang Rui 
> > ---
> >   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c b/drivers/gpu/drm/amd/
> amdgpu/gfxhub_v1_0.c
> > index 005075f..41313514 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
> > @@ -368,7 +368,7 @@ static int gfxhub_v1_0_suspend(void *handle)
> >  
> >   static int gfxhub_v1_0_resume(void *handle)
> >   {
> > - return 0;
> > + return gfxhub_v1_0_hw_init(handle);
> >   }
> >  
> >   static bool gfxhub_v1_0_is_idle(void *handle)
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 4/4] drm/amdgpu: reset fpriv vram_lost_counter

2017-05-17 Thread Michel Dänzer
On 17/05/17 04:13 PM, zhoucm1 wrote:
> On 2017年05月17日 14:57, Michel Dänzer wrote:
>> On 17/05/17 01:28 PM, zhoucm1 wrote:
>>> On 2017年05月17日 11:15, Michel Dänzer wrote:
 On 17/05/17 12:04 PM, zhoucm1 wrote:
> On 2017年05月17日 09:18, Michel Dänzer wrote:
>> On 16/05/17 06:25 PM, Chunming Zhou wrote:
>>> Change-Id: I8eb6d7f558da05510e429d3bf1d48c8cec6c1977
>>> Signed-off-by: Chunming Zhou 
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index bca1fb5..f3e7525 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -2547,6 +2547,9 @@ int amdgpu_vm_ioctl(struct drm_device *dev,
>>> void *data, struct drm_file *filp)
>>> case AMDGPU_VM_OP_UNRESERVE_VMID:
>>> amdgpu_vm_free_reserved_vmid(adev, &fpriv->vm,
>>> AMDGPU_GFXHUB);
>>> break;
>>> +case AMDGPU_VM_OP_RESET:
>>> +fpriv->vram_lost_counter =
>>> atomic_read(&adev->vram_lost_counter);
>>> +break;
>> How do you envision the UMDs using this? I can mostly think of them
>> calling this ioctl when a context is created or destroyed. But that
>> would also allow any other remaining contexts using the same DRM file
>> descriptor to use all ioctls again. So, I think there needs to be a
>> vram_lost_counter in struct amdgpu_ctx instead of in struct
>> amdgpu_fpriv.
> struct amdgpu_fpriv for vram_lost_counter is proper place, especially
> for ioctl return value.
> if you need to reset ctx one by one, we can mark all contexts of that
> vm, and then reset by userspace.
 I'm not following. With vram_lost_counter in amdgpu_fpriv, if any
 context calls this ioctl, all other contexts using the same file
 descriptor will also be considered safe again, right?
>>> Yes, but it really depends on userspace requirement, if you need to
>>> reset ctx one by one, we can mark all contexts of that vm to guilty, and
>>> then reset one context by userspace.
>> Still not sure what you mean by that.
>>
>> E.g. what do you mean by "guilty"? I thought that refers to the context
>> which caused a hang. But it seems like you're using it to refer to any
>> context which hasn't reacted yet to VRAM contents being lost.
> When vram is lost, we treat all contexts need to reset.

Essentially, your patches only track VRAM contents being lost per file
descriptor, not per context. I'm not sure (rather skeptical) that this
is suitable for OpenGL UMDs, since state is usually tracked per context.
Marek / Nicolai?


-- 
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 1/2] drm/amdgpu: fix re-program vm invalidate eng address range for gfxhub on resume

2017-05-17 Thread zhoucm1



On 2017年05月17日 15:55, Huang Rui wrote:

On Wed, May 17, 2017 at 03:43:47PM +0800, Zhou, David(ChunMing) wrote:

By this change, I suggest to remove mmhub/gfxhub_v1_0_ip_funcs and their
IP block, unify them to gmc ip block, this way we cannot lost setting
when resume back.


 From hw side, wo won't have real gmc since this chip, mmhub and gfxhub(gc)
instead of it. Maybe we would better to align with hw desgin.
I don't see any advance, as you said, we still have gmc block in soc15, 
why not unify mmhub/gfxhub calls to gmc block?

We can keep mmhub/gfxhub_xxx.c file, but ip_funciton isn't necessary.

Regards,
David Zhou


Thanks,
Rui


Regards,
David Zhou

On 2017年05月17日 15:38, Huang Rui wrote:

Signed-off-by: Huang Rui 
---
   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c b/drivers/gpu/drm/amd/

amdgpu/gfxhub_v1_0.c

index 005075f..41313514 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -368,7 +368,7 @@ static int gfxhub_v1_0_suspend(void *handle)
  
   static int gfxhub_v1_0_resume(void *handle)

   {
- return 0;
+ return gfxhub_v1_0_hw_init(handle);
   }
  
   static bool gfxhub_v1_0_is_idle(void *handle)

___
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 2/2] drm/amdgpu: fix re-program vm invalidate eng address range for mmhub on resume

2017-05-17 Thread Wang, Ken
Reviewed-by: Ken Wang 


From: Huang Rui 
Sent: Wednesday, May 17, 2017 3:38:49 PM
To: amd-gfx@lists.freedesktop.org; Deucher, Alexander; Koenig, Christian
Cc: Zhou, David(ChunMing); Wang, Ken; Huan, Alvin; Huang, Ray
Subject: [PATCH 2/2] drm/amdgpu: fix re-program vm invalidate eng address range 
for mmhub on resume

Signed-off-by: Huang Rui 
---
 drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c 
b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
index dbfe48d..8979bc2 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v1_0.c
@@ -386,7 +386,7 @@ static int mmhub_v1_0_suspend(void *handle)

 static int mmhub_v1_0_resume(void *handle)
 {
-   return 0;
+   return mmhub_v1_0_hw_init(handle);
 }

 static bool mmhub_v1_0_is_idle(void *handle)
--
2.7.4

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


Re: [PATCH 1/2] drm/amdgpu: fix re-program vm invalidate eng address range for gfxhub on resume

2017-05-17 Thread zhoucm1

see here, I already find dis-advance:
enum amd_ip_block_type {
AMD_IP_BLOCK_TYPE_COMMON,
AMD_IP_BLOCK_TYPE_GMC,
AMD_IP_BLOCK_TYPE_IH,
AMD_IP_BLOCK_TYPE_SMC,
AMD_IP_BLOCK_TYPE_PSP,
AMD_IP_BLOCK_TYPE_DCE,
AMD_IP_BLOCK_TYPE_GFX,
AMD_IP_BLOCK_TYPE_SDMA,
AMD_IP_BLOCK_TYPE_UVD,
AMD_IP_BLOCK_TYPE_VCE,
AMD_IP_BLOCK_TYPE_ACP,
AMD_IP_BLOCK_TYPE_GFXHUB,
AMD_IP_BLOCK_TYPE_MMHUB
};
resume will follow this sequence.
but initial sequence is :
  amdgpu_ip_block_add(adev, &vega10_common_ip_block);
amdgpu_ip_block_add(adev, &gfxhub_v1_0_ip_block);
amdgpu_ip_block_add(adev, &mmhub_v1_0_ip_block);
amdgpu_ip_block_add(adev, &gmc_v9_0_ip_block);
amdgpu_ip_block_add(adev, &vega10_ih_ip_block);
if (amdgpu_fw_load_type == 2 || amdgpu_fw_load_type == -1)
amdgpu_ip_block_add(adev, &psp_v3_1_ip_block);
if (!amdgpu_sriov_vf(adev))
amdgpu_ip_block_add(adev, &amdgpu_pp_ip_block);
if (adev->enable_virtual_display || amdgpu_sriov_vf(adev))
amdgpu_ip_block_add(adev, &dce_virtual_ip_block);
#if defined(CONFIG_DRM_AMD_DC)
else if (amdgpu_device_has_dc_support(adev))
amdgpu_ip_block_add(adev, &dm_ip_block);
#else
#   warning "Enable CONFIG_DRM_AMD_DC for display support on SOC15."
#endif
amdgpu_ip_block_add(adev, &gfx_v9_0_ip_block);
amdgpu_ip_block_add(adev, &sdma_v4_0_ip_block);
amdgpu_ip_block_add(adev, &uvd_v7_0_ip_block);
amdgpu_ip_block_add(adev, &vce_v4_0_ip_block);


They are different. I remember I asked you if they are same, don't know 
why you answer 'yes'.


With s3 problem still in there, please do this improvement asap.

Regards,
David Zhou

On 2017年05月17日 16:11, zhoucm1 wrote:



On 2017年05月17日 15:55, Huang Rui wrote:

On Wed, May 17, 2017 at 03:43:47PM +0800, Zhou, David(ChunMing) wrote:
By this change, I suggest to remove mmhub/gfxhub_v1_0_ip_funcs and 
their

IP block, unify them to gmc ip block, this way we cannot lost setting
when resume back.

 From hw side, wo won't have real gmc since this chip, mmhub and 
gfxhub(gc)

instead of it. Maybe we would better to align with hw desgin.
I don't see any advance, as you said, we still have gmc block in 
soc15, why not unify mmhub/gfxhub calls to gmc block?

We can keep mmhub/gfxhub_xxx.c file, but ip_funciton isn't necessary.

Regards,
David Zhou


Thanks,
Rui


Regards,
David Zhou

On 2017年05月17日 15:38, Huang Rui wrote:

Signed-off-by: Huang Rui 
---
   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c 
b/drivers/gpu/drm/amd/

amdgpu/gfxhub_v1_0.c

index 005075f..41313514 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -368,7 +368,7 @@ static int gfxhub_v1_0_suspend(void *handle)
 static int gfxhub_v1_0_resume(void *handle)
   {
- return 0;
+ return gfxhub_v1_0_hw_init(handle);
   }
 static bool gfxhub_v1_0_is_idle(void *handle)

___
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


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


Re: [PATCH] gpu: drm: radeon: refactor code

2017-05-17 Thread Christian König

Am 17.05.2017 um 04:20 schrieb Gustavo A. R. Silva:

Local variable _color_ is assigned to a constant value and it is
never updated again. Remove this variable and refactor the code it
affects.

Addresses-Coverity-ID: 1226745
Signed-off-by: Gustavo A. R. Silva 


Mhm, on the one hand it looks like a valid cleanup. On the other that is 
legacy code we haven't touched in a while.


Feel free to put my Reviewed-by: Christian König 
 on it, but I'm not sure if Alex will pick it up.


Regards,
Christian.


---
  drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 8 +---
  1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c 
b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 222a1fa..7235d0c 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
@@ -640,7 +640,6 @@ static enum drm_connector_status 
radeon_legacy_primary_dac_detect(struct drm_enc
uint32_t vclk_ecp_cntl, crtc_ext_cntl;
uint32_t dac_ext_cntl, dac_cntl, dac_macro_cntl, tmp;
enum drm_connector_status found = connector_status_disconnected;
-   bool color = true;
  
  	/* just don't bother on RN50 those chip are often connected to remoting

 * console hw and often we get failure to load detect those. So to make
@@ -665,12 +664,7 @@ static enum drm_connector_status 
radeon_legacy_primary_dac_detect(struct drm_enc
WREG32(RADEON_CRTC_EXT_CNTL, tmp);
  
  	tmp = RADEON_DAC_FORCE_BLANK_OFF_EN |

-   RADEON_DAC_FORCE_DATA_EN;
-
-   if (color)
-   tmp |= RADEON_DAC_FORCE_DATA_SEL_RGB;
-   else
-   tmp |= RADEON_DAC_FORCE_DATA_SEL_G;
+   RADEON_DAC_FORCE_DATA_EN | RADEON_DAC_FORCE_DATA_SEL_RGB;
  
  	if (ASIC_IS_R300(rdev))

tmp |= (0x1b6 << RADEON_DAC_FORCE_DATA_SHIFT);



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


Re: [PATCH 1/2] drm/amdgpu: fix re-program vm invalidate eng address range for gfxhub on resume

2017-05-17 Thread zhoucm1



On 2017年05月17日 16:19, Christian König wrote:

Am 17.05.2017 um 10:11 schrieb zhoucm1:



On 2017年05月17日 15:55, Huang Rui wrote:

On Wed, May 17, 2017 at 03:43:47PM +0800, Zhou, David(ChunMing) wrote:
By this change, I suggest to remove mmhub/gfxhub_v1_0_ip_funcs and 
their

IP block, unify them to gmc ip block, this way we cannot lost setting
when resume back.

 From hw side, wo won't have real gmc since this chip, mmhub and 
gfxhub(gc)

instead of it. Maybe we would better to align with hw desgin.
I don't see any advance, as you said, we still have gmc block in 
soc15, why not unify mmhub/gfxhub calls to gmc block?

We can keep mmhub/gfxhub_xxx.c file, but ip_funciton isn't necessary.


Well they are two hardware block, but are mostly identical programmed 
(the MMHUB has a few more bits for guaranteed bandwith, but we ignore 
those at the moment).


So it doesn't make much sense having two separate code instances to 
handle them.


One major problem still remaining is that our generated register 
headers are crap for this. You can't for example include both headers 
at the same time.
As I said before, we can keep these two seperate files, just remove 
ip_functions, but call them from gmc file.

like gmc_v9_init()
{
gfxhub_init();
mmhub_init();
}

Regards,
David Zhou


Regards,
Christian.



Regards,
David Zhou


Thanks,
Rui


Regards,
David Zhou

On 2017年05月17日 15:38, Huang Rui wrote:

Signed-off-by: Huang Rui 
---
   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c 
b/drivers/gpu/drm/amd/

amdgpu/gfxhub_v1_0.c

index 005075f..41313514 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -368,7 +368,7 @@ static int gfxhub_v1_0_suspend(void *handle)
 static int gfxhub_v1_0_resume(void *handle)
   {
- return 0;
+ return gfxhub_v1_0_hw_init(handle);
   }
 static bool gfxhub_v1_0_is_idle(void *handle)

___
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: fix re-program vm invalidate eng address range for gfxhub on resume

2017-05-17 Thread Christian König

Am 17.05.2017 um 09:55 schrieb Huang Rui:

On Wed, May 17, 2017 at 03:43:47PM +0800, Zhou, David(ChunMing) wrote:

By this change, I suggest to remove mmhub/gfxhub_v1_0_ip_funcs and their
IP block, unify them to gmc ip block, this way we cannot lost setting
when resume back.


 From hw side, wo won't have real gmc since this chip, mmhub and gfxhub(gc)
instead of it. Maybe we would better to align with hw desgin.


Yeah, I had patches for doing exactly this but then decided dropping it.

What we should do is to have any one implementation of the code handling 
both hubs and then have both as separate IP block in the list.


Anyway for now both patches are Reviewed-by: Christian König 
.


I assume that finally fixes S3 on Vega10?

Regards,
Christian.



Thanks,
Rui


Regards,
David Zhou

On 2017年05月17日 15:38, Huang Rui wrote:

Signed-off-by: Huang Rui 
---
   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c b/drivers/gpu/drm/amd/

amdgpu/gfxhub_v1_0.c

index 005075f..41313514 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -368,7 +368,7 @@ static int gfxhub_v1_0_suspend(void *handle)
  
   static int gfxhub_v1_0_resume(void *handle)

   {
- return 0;
+ return gfxhub_v1_0_hw_init(handle);
   }
  
   static bool gfxhub_v1_0_is_idle(void *handle)



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


Re: [PATCH 0/3] Cleanup evergreen/si IRQ handling code

2017-05-17 Thread Christian König

Am 16.05.2017 um 23:11 schrieb Lyude:

This is the first part of me going through and cleaning up the IRQ handling
code for radeon, since after taking a look at it the other day while trying to
debug something I realized basically all of the code was copy pasted
everywhere, and quite difficult to actually read through.

Will come up with something for r600 and cik once I've got the chipsets on hand
to test with.


Oh, yes please. This always annoyed me as well, but never had the time 
to clean it up globally.


Just two general comments:
1. Don't modify the register headers.

They are more or less from a database and we don't want manual code like 
this in there:

+#define DISP_INTERRUPT_STATUS(i) \
+   ((i < 4) ? 0x60f4 + (0x4 * i) : 0x614c + (0x4 * (i - 4)))


Instead use a static constant array, e.g. like this:
static const uint32_t disp_interrupt_status[4] {
DISP_INTERRUPT_STATUS,
DISP_INTERRUPT_STATUS_CONTINUE,
...
};

2. Keep the order in which stuff is written to the regs exactly the same.

In other words, don't replace this:


-   rdev->irq.stat_regs.evergreen.afmt_status1 = RREG32(AFMT_STATUS + 
EVERGREEN_CRTC0_REGISTER_OFFSET);
-   rdev->irq.stat_regs.evergreen.afmt_status2 = RREG32(AFMT_STATUS + 
EVERGREEN_CRTC1_REGISTER_OFFSET);
-   rdev->irq.stat_regs.evergreen.afmt_status3 = RREG32(AFMT_STATUS + 
EVERGREEN_CRTC2_REGISTER_OFFSET);
-   rdev->irq.stat_regs.evergreen.afmt_status4 = RREG32(AFMT_STATUS + 
EVERGREEN_CRTC3_REGISTER_OFFSET);
-   rdev->irq.stat_regs.evergreen.afmt_status5 = RREG32(AFMT_STATUS + 
EVERGREEN_CRTC4_REGISTER_OFFSET);
-   rdev->irq.stat_regs.evergreen.afmt_status6 = RREG32(AFMT_STATUS + 
EVERGREEN_CRTC5_REGISTER_OFFSET);


With:


for (i = 0; i < EVERGREEN_MAX_DISP_REGISTERS; i++) {
disp_int[i] = RREG32(DISP_INTERRUPT_STATUS(i));
+   afmt_status[i] = RREG32(AFMT_STATUS + crtc_offsets[i]);
  
  		if (disp_int[i] & DC_HPDx_INTERRUPT)

WREG32_OR(DC_HPDx_INT_CONTROL(i), DC_HPDx_INT_ACK);
if (disp_int[i] & DC_HPDx_RX_INTERRUPT)
WREG32_OR(DC_HPDx_INT_CONTROL(i), DC_HPDx_RX_INT_ACK);
+   if (afmt_status[i] & AFMT_AZ_FORMAT_WTRIG)
+   WREG32_OR(AFMT_AUDIO_PACKET_CONTROL + crtc_offsets[i],
+ AFMT_AZ_FORMAT_WTRIG_ACK);


Regards,
Christian.



Lyude (3):
   drm/radeon: Cleanup display interrupt handling for evergreen, si
   drm/radeon: Cleanup HDMI audio interrupt handling for evergreen
   drm/radeon: Cleanup pageflipping IRQ handling for evergreen, si

  drivers/gpu/drm/radeon/evergreen.c  | 915 +---
  drivers/gpu/drm/radeon/evergreend.h |  74 +--
  drivers/gpu/drm/radeon/radeon.h |  27 +-
  drivers/gpu/drm/radeon/radeon_irq_kms.c |  35 ++
  drivers/gpu/drm/radeon/si.c | 627 --
  drivers/gpu/drm/radeon/sid.h|  72 +--
  6 files changed, 328 insertions(+), 1422 deletions(-)



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


Re: [PATCH 1/2] drm/amdgpu: fix re-program vm invalidate eng address range for gfxhub on resume

2017-05-17 Thread Christian König

Am 17.05.2017 um 10:11 schrieb zhoucm1:



On 2017年05月17日 15:55, Huang Rui wrote:

On Wed, May 17, 2017 at 03:43:47PM +0800, Zhou, David(ChunMing) wrote:
By this change, I suggest to remove mmhub/gfxhub_v1_0_ip_funcs and 
their

IP block, unify them to gmc ip block, this way we cannot lost setting
when resume back.

 From hw side, wo won't have real gmc since this chip, mmhub and 
gfxhub(gc)

instead of it. Maybe we would better to align with hw desgin.
I don't see any advance, as you said, we still have gmc block in 
soc15, why not unify mmhub/gfxhub calls to gmc block?

We can keep mmhub/gfxhub_xxx.c file, but ip_funciton isn't necessary.


Well they are two hardware block, but are mostly identical programmed 
(the MMHUB has a few more bits for guaranteed bandwith, but we ignore 
those at the moment).


So it doesn't make much sense having two separate code instances to 
handle them.


One major problem still remaining is that our generated register headers 
are crap for this. You can't for example include both headers at the 
same time.


Regards,
Christian.



Regards,
David Zhou


Thanks,
Rui


Regards,
David Zhou

On 2017年05月17日 15:38, Huang Rui wrote:

Signed-off-by: Huang Rui 
---
   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c 
b/drivers/gpu/drm/amd/

amdgpu/gfxhub_v1_0.c

index 005075f..41313514 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -368,7 +368,7 @@ static int gfxhub_v1_0_suspend(void *handle)
 static int gfxhub_v1_0_resume(void *handle)
   {
- return 0;
+ return gfxhub_v1_0_hw_init(handle);
   }
 static bool gfxhub_v1_0_is_idle(void *handle)

___
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 4/4] drm/amdgpu: reset fpriv vram_lost_counter

2017-05-17 Thread Christian König

Am 17.05.2017 um 10:01 schrieb Michel Dänzer:

On 17/05/17 04:13 PM, zhoucm1 wrote:

On 2017年05月17日 14:57, Michel Dänzer wrote:

On 17/05/17 01:28 PM, zhoucm1 wrote:

On 2017年05月17日 11:15, Michel Dänzer wrote:

On 17/05/17 12:04 PM, zhoucm1 wrote:

On 2017年05月17日 09:18, Michel Dänzer wrote:

On 16/05/17 06:25 PM, Chunming Zhou wrote:

Change-Id: I8eb6d7f558da05510e429d3bf1d48c8cec6c1977
Signed-off-by: Chunming Zhou 

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index bca1fb5..f3e7525 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2547,6 +2547,9 @@ int amdgpu_vm_ioctl(struct drm_device *dev,
void *data, struct drm_file *filp)
 case AMDGPU_VM_OP_UNRESERVE_VMID:
 amdgpu_vm_free_reserved_vmid(adev, &fpriv->vm,
AMDGPU_GFXHUB);
 break;
+case AMDGPU_VM_OP_RESET:
+fpriv->vram_lost_counter =
atomic_read(&adev->vram_lost_counter);
+break;

How do you envision the UMDs using this? I can mostly think of them
calling this ioctl when a context is created or destroyed. But that
would also allow any other remaining contexts using the same DRM file
descriptor to use all ioctls again. So, I think there needs to be a
vram_lost_counter in struct amdgpu_ctx instead of in struct
amdgpu_fpriv.

struct amdgpu_fpriv for vram_lost_counter is proper place, especially
for ioctl return value.
if you need to reset ctx one by one, we can mark all contexts of that
vm, and then reset by userspace.

I'm not following. With vram_lost_counter in amdgpu_fpriv, if any
context calls this ioctl, all other contexts using the same file
descriptor will also be considered safe again, right?

Yes, but it really depends on userspace requirement, if you need to
reset ctx one by one, we can mark all contexts of that vm to guilty, and
then reset one context by userspace.

Still not sure what you mean by that.

E.g. what do you mean by "guilty"? I thought that refers to the context
which caused a hang. But it seems like you're using it to refer to any
context which hasn't reacted yet to VRAM contents being lost.

When vram is lost, we treat all contexts need to reset.

Essentially, your patches only track VRAM contents being lost per file
descriptor, not per context. I'm not sure (rather skeptical) that this
is suitable for OpenGL UMDs, since state is usually tracked per context.
Marek / Nicolai?


Oh, yeah that's a good point.

The problem with tracking it per context is that Vulkan also wants the 
ENODEV on the amdgpu_gem_va_ioct() and amdgpu_info_ioctl() which are 
context less.


But thinking more about this blocking those two doesn't make much sense. 
The VM content can be restored and why should be disallow reading GPU info?


Christian.

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


Re: [PATCH 1/2] drm/amdgpu: fix re-program vm invalidate eng address range for gfxhub on resume

2017-05-17 Thread Christian König

Am 17.05.2017 um 10:22 schrieb zhoucm1:



On 2017年05月17日 16:19, Christian König wrote:

Am 17.05.2017 um 10:11 schrieb zhoucm1:



On 2017年05月17日 15:55, Huang Rui wrote:

On Wed, May 17, 2017 at 03:43:47PM +0800, Zhou, David(ChunMing) wrote:
By this change, I suggest to remove mmhub/gfxhub_v1_0_ip_funcs and 
their

IP block, unify them to gmc ip block, this way we cannot lost setting
when resume back.

 From hw side, wo won't have real gmc since this chip, mmhub and 
gfxhub(gc)

instead of it. Maybe we would better to align with hw desgin.
I don't see any advance, as you said, we still have gmc block in 
soc15, why not unify mmhub/gfxhub calls to gmc block?

We can keep mmhub/gfxhub_xxx.c file, but ip_funciton isn't necessary.


Well they are two hardware block, but are mostly identical programmed 
(the MMHUB has a few more bits for guaranteed bandwith, but we ignore 
those at the moment).


So it doesn't make much sense having two separate code instances to 
handle them.


One major problem still remaining is that our generated register 
headers are crap for this. You can't for example include both headers 
at the same time.
As I said before, we can keep these two seperate files, just remove 
ip_functions, but call them from gmc file.

like gmc_v9_init()
{
gfxhub_init();
mmhub_init();
}


The init order isn't so critical here. My concern is rather that we have 
the duplicated code.


But I agree that we sooner or later should clean up the init order on 
load/resume to be the same.


Regards,
Christian.



Regards,
David Zhou


Regards,
Christian.



Regards,
David Zhou


Thanks,
Rui


Regards,
David Zhou

On 2017年05月17日 15:38, Huang Rui wrote:

Signed-off-by: Huang Rui 
---
   drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c 
b/drivers/gpu/drm/amd/

amdgpu/gfxhub_v1_0.c

index 005075f..41313514 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v1_0.c
@@ -368,7 +368,7 @@ static int gfxhub_v1_0_suspend(void *handle)
 static int gfxhub_v1_0_resume(void *handle)
   {
- return 0;
+ return gfxhub_v1_0_hw_init(handle);
   }
 static bool gfxhub_v1_0_is_idle(void *handle)

___
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: fix re-program vm invalidate eng address range for gfxhub on resume

2017-05-17 Thread Huang Rui
On Wed, May 17, 2017 at 04:16:31PM +0800, Koenig, Christian wrote:
> Am 17.05.2017 um 09:55 schrieb Huang Rui:
> > On Wed, May 17, 2017 at 03:43:47PM +0800, Zhou, David(ChunMing) wrote:
> >> By this change, I suggest to remove mmhub/gfxhub_v1_0_ip_funcs and their
> >> IP block, unify them to gmc ip block, this way we cannot lost setting
> >> when resume back.
> >>
> >  From hw side, wo won't have real gmc since this chip, mmhub and gfxhub(gc)
> > instead of it. Maybe we would better to align with hw desgin.
> 
> Yeah, I had patches for doing exactly this but then decided dropping it.
> 
> What we should do is to have any one implementation of the code handling
> both hubs and then have both as separate IP block in the list.
> 
> Anyway for now both patches are Reviewed-by: Christian König
> .
> 
> I assume that finally fixes S3 on Vega10?
> 

No, but these fixes have improvement for the result.
We can pass a round of gfx/cp/sdma test.

But if we try to test a round again, it will encounter VM fault.
Will tell you the detailed in another thread.

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


Re: [PATCH 4/5] drm/amdgpu: Support page directory update via CPU

2017-05-17 Thread Christian König

Am 17.05.2017 um 03:54 schrieb zhoucm1:



On 2017年05月17日 05:02, Kasiviswanathan, Harish wrote:



-Original Message-
From: Zhou, David(ChunMing)
Sent: Monday, May 15, 2017 10:50 PM
To: Kasiviswanathan, Harish ; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/5] drm/amdgpu: Support page directory update 
via CPU




On 2017年05月16日 05:32, Harish Kasiviswanathan wrote:
> If amdgpu.vm_update_context param is set to use CPU, then Page
> Directories will be updated by CPU instead of SDMA
>
> Signed-off-by: Harish Kasiviswanathan 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 151 
-

>   1 file changed, 109 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

> index 9c89cb2..d72a624 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -271,6 +271,7 @@ static int amdgpu_vm_alloc_levels(struct 
amdgpu_device *adev,

>  uint64_t saddr, uint64_t eaddr,
>  unsigned level)
>   {
> + u64 flags;
>unsigned shift = (adev->vm_manager.num_level - level) *
>adev->vm_manager.block_size;
>unsigned pt_idx, from, to;
> @@ -299,6 +300,14 @@ static int amdgpu_vm_alloc_levels(struct 
amdgpu_device *adev,

>saddr = saddr & ((1 << shift) - 1);
>eaddr = eaddr & ((1 << shift) - 1);
>
> + flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
> + AMDGPU_GEM_CREATE_VRAM_CLEARED;
> + if (vm->use_cpu_for_update)
> + flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
I think shadow flag is need for CPU case as well, which is used to
backup VM bo and meaningful when gpu reset.
same comment for pd bo.

[HK]: Yes support for shadow BOs are desirable and it could be 
implemented as a separate commit. For supporting shadow BOs the 
caller should explicitly add shadow BOs into 
ttm_eu_reserve_buffer(..) to remove the BO from TTM swap list or 
ttm_bo_kmap has to be modified. This implementation for CPU update of 
VM page tables is mainly for KFD usage. Graphics will use for 
experimental and testing purpose. From KFD's view point shadow BO are 
not useful because if GPU is reset then all queue information is lost 
(since submissions are done by user space) and it is not possible to 
recover.

Either way is fine to me.


Actually I'm thinking about if we shouldn't completely drop the shadow 
handling.


When VRAM is lost we now completely drop all jobs, so for new jobs we 
can recreate the page table content from the VM structures as well.


When VRAM is not lost we don't need to restore the page tables.

What do you think?

Regards,
Christian.



David Zhou


Regards,
David Zhou
> + else
> + flags |= (AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
> + AMDGPU_GEM_CREATE_SHADOW);
> +
>/* walk over the address space and allocate the page tables */
>for (pt_idx = from; pt_idx <= to; ++pt_idx) {
>struct reservation_object *resv = vm->root.bo->tbo.resv;
> @@ -310,10 +319,7 @@ static int amdgpu_vm_alloc_levels(struct 
amdgpu_device *adev,

> amdgpu_vm_bo_size(adev, level),
> AMDGPU_GPU_PAGE_SIZE, true,
> AMDGPU_GEM_DOMAIN_VRAM,
> - AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
> - AMDGPU_GEM_CREATE_SHADOW |
> - AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
> - AMDGPU_GEM_CREATE_VRAM_CLEARED,
> +  flags,
> NULL, resv, &pt);
>if (r)
>return r;
> @@ -952,6 +958,43 @@ static uint64_t amdgpu_vm_map_gart(const 
dma_addr_t *pages_addr, uint64_t addr)

>return result;
>   }
>
> +/**
> + * amdgpu_vm_cpu_set_ptes - helper to update page tables via CPU
> + *
> + * @params: see amdgpu_pte_update_params definition
> + * @pe: kmap addr of the page entry
> + * @addr: dst addr to write into pe
> + * @count: number of page entries to update
> + * @incr: increase next addr by incr bytes
> + * @flags: hw access flags
> + */
> +static void amdgpu_vm_cpu_set_ptes(struct amdgpu_pte_update_params 
*params,

> +uint64_t pe, uint64_t addr,
> +unsigned count, uint32_t incr,
> +uint64_t flags)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < count; i++) {
> + amdgpu_gart_set_pte_pde(params->adev, (void *)pe,
> + i, addr, flags);
> + addr += incr;
> + }
> +
> + mb();
> + amdgpu_gart_flush_gpu_tlb(params->adev, 0);
> +}
> +
> +static void amdgpu_vm_bo_wait(struct amdgpu_device *adev, struct 
amdgpu_bo *bo)

> +{
> + struct amdgpu_sync sync;
> +
> + amdgpu_sync_create(&sync);
> + amdgpu_sync_resv(adev, &sync, bo->tbo.resv, 
AMDGPU_FENCE_OWNER_VM);

> + amdgpu_sync_wait(&sync);
> + amdgpu_sync_free(&sync);
> +}
> +
>   /*
>*

Re: [PATCH 4/4] drm/amdgpu: reset fpriv vram_lost_counter

2017-05-17 Thread zhoucm1



On 2017年05月17日 16:40, Christian König wrote:

Am 17.05.2017 um 10:01 schrieb Michel Dänzer:

On 17/05/17 04:13 PM, zhoucm1 wrote:

On 2017年05月17日 14:57, Michel Dänzer wrote:

On 17/05/17 01:28 PM, zhoucm1 wrote:

On 2017年05月17日 11:15, Michel Dänzer wrote:

On 17/05/17 12:04 PM, zhoucm1 wrote:

On 2017年05月17日 09:18, Michel Dänzer wrote:

On 16/05/17 06:25 PM, Chunming Zhou wrote:

Change-Id: I8eb6d7f558da05510e429d3bf1d48c8cec6c1977
Signed-off-by: Chunming Zhou 

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index bca1fb5..f3e7525 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2547,6 +2547,9 @@ int amdgpu_vm_ioctl(struct drm_device *dev,
void *data, struct drm_file *filp)
 case AMDGPU_VM_OP_UNRESERVE_VMID:
 amdgpu_vm_free_reserved_vmid(adev, &fpriv->vm,
AMDGPU_GFXHUB);
 break;
+case AMDGPU_VM_OP_RESET:
+fpriv->vram_lost_counter =
atomic_read(&adev->vram_lost_counter);
+break;
How do you envision the UMDs using this? I can mostly think of 
them
calling this ioctl when a context is created or destroyed. But 
that
would also allow any other remaining contexts using the same 
DRM file
descriptor to use all ioctls again. So, I think there needs to 
be a

vram_lost_counter in struct amdgpu_ctx instead of in struct
amdgpu_fpriv.
struct amdgpu_fpriv for vram_lost_counter is proper place, 
especially

for ioctl return value.
if you need to reset ctx one by one, we can mark all contexts of 
that

vm, and then reset by userspace.

I'm not following. With vram_lost_counter in amdgpu_fpriv, if any
context calls this ioctl, all other contexts using the same file
descriptor will also be considered safe again, right?

Yes, but it really depends on userspace requirement, if you need to
reset ctx one by one, we can mark all contexts of that vm to 
guilty, and

then reset one context by userspace.

Still not sure what you mean by that.

E.g. what do you mean by "guilty"? I thought that refers to the 
context

which caused a hang. But it seems like you're using it to refer to any
context which hasn't reacted yet to VRAM contents being lost.

When vram is lost, we treat all contexts need to reset.

Essentially, your patches only track VRAM contents being lost per file
descriptor, not per context. I'm not sure (rather skeptical) that this
is suitable for OpenGL UMDs, since state is usually tracked per context.
Marek / Nicolai?


Oh, yeah that's a good point.

The problem with tracking it per context is that Vulkan also wants the 
ENODEV on the amdgpu_gem_va_ioct() and amdgpu_info_ioctl() which are 
context less.


But thinking more about this blocking those two doesn't make much 
sense. The VM content can be restored and why should be disallow 
reading GPU info?

I can re-paste the Vulkan APIs requiring ENODEV:
"

The Vulkan APIs listed below could return VK_ERROR_DEVICE_LOST according 
to the spec.


I tries to provide a list of u/k interfaces that could be called for 
each vk API.


vkCreateDevice

-amdgpu_device_initialize.

-amdgpu_query_gpu_info

vkQueueSubmit

-amdgpu_cs_submit

vkWaitForFences

amdgpu_cs_wait_fences

vkGetEventStatus

vkQueueWaitIdle

vkDeviceWaitIdle

vkGetQueryPoolResults**

amdgpu_cs_query_Fence_status

vkQueueBindSparse**

amdgpu_bo_va_op

amdgpu_bo_va_op_raw

vkCreateSwapchainKHR**

vkAcquireNextImageKHR**

vkQueuePresentKHR

Not related with u/k interface.**

**

Besides those listed above, I think 
amdgpu_cs_signal_Sem/amdgpu_cs_wait_sem should respond to gpu reset as 
well."


Christian.



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


Re: [PATCH 4/4] drm/amdgpu: reset fpriv vram_lost_counter

2017-05-17 Thread Michel Dänzer
On 17/05/17 05:46 PM, zhoucm1 wrote:
> 
> 
> On 2017年05月17日 16:40, Christian König wrote:
>> Am 17.05.2017 um 10:01 schrieb Michel Dänzer:
>>> On 17/05/17 04:13 PM, zhoucm1 wrote:
 On 2017年05月17日 14:57, Michel Dänzer wrote:
> On 17/05/17 01:28 PM, zhoucm1 wrote:
>> On 2017年05月17日 11:15, Michel Dänzer wrote:
>>> On 17/05/17 12:04 PM, zhoucm1 wrote:
 On 2017年05月17日 09:18, Michel Dänzer wrote:
> On 16/05/17 06:25 PM, Chunming Zhou wrote:
>> Change-Id: I8eb6d7f558da05510e429d3bf1d48c8cec6c1977
>> Signed-off-by: Chunming Zhou 
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index bca1fb5..f3e7525 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -2547,6 +2547,9 @@ int amdgpu_vm_ioctl(struct drm_device *dev,
>> void *data, struct drm_file *filp)
>>  case AMDGPU_VM_OP_UNRESERVE_VMID:
>>  amdgpu_vm_free_reserved_vmid(adev, &fpriv->vm,
>> AMDGPU_GFXHUB);
>>  break;
>> +case AMDGPU_VM_OP_RESET:
>> +fpriv->vram_lost_counter =
>> atomic_read(&adev->vram_lost_counter);
>> +break;
> How do you envision the UMDs using this? I can mostly think of
> them
> calling this ioctl when a context is created or destroyed. But
> that
> would also allow any other remaining contexts using the same
> DRM file
> descriptor to use all ioctls again. So, I think there needs to
> be a
> vram_lost_counter in struct amdgpu_ctx instead of in struct
> amdgpu_fpriv.
 struct amdgpu_fpriv for vram_lost_counter is proper place,
 especially
 for ioctl return value.
 if you need to reset ctx one by one, we can mark all contexts of
 that
 vm, and then reset by userspace.
>>> I'm not following. With vram_lost_counter in amdgpu_fpriv, if any
>>> context calls this ioctl, all other contexts using the same file
>>> descriptor will also be considered safe again, right?
>> Yes, but it really depends on userspace requirement, if you need to
>> reset ctx one by one, we can mark all contexts of that vm to
>> guilty, and
>> then reset one context by userspace.
> Still not sure what you mean by that.
>
> E.g. what do you mean by "guilty"? I thought that refers to the
> context
> which caused a hang. But it seems like you're using it to refer to any
> context which hasn't reacted yet to VRAM contents being lost.
 When vram is lost, we treat all contexts need to reset.
>>> Essentially, your patches only track VRAM contents being lost per file
>>> descriptor, not per context. I'm not sure (rather skeptical) that this
>>> is suitable for OpenGL UMDs, since state is usually tracked per context.
>>> Marek / Nicolai?
>>
>> Oh, yeah that's a good point.
>>
>> The problem with tracking it per context is that Vulkan also wants the
>> ENODEV on the amdgpu_gem_va_ioct() and amdgpu_info_ioctl() which are
>> context less.
>>
>> But thinking more about this blocking those two doesn't make much
>> sense. The VM content can be restored and why should be disallow
>> reading GPU info?
> I can re-paste the Vulkan APIs requiring ENODEV:
> "
> 
> The Vulkan APIs listed below could return VK_ERROR_DEVICE_LOST according
> to the spec.
> 
> I tries to provide a list of u/k interfaces that could be called for
> each vk API.
> 
>  
> 
> vkCreateDevice
> 
> -  amdgpu_device_initialize.
> -  amdgpu_query_gpu_info

[...]

> vkQueueBindSparse**
> 
> amdgpu_bo_va_op
> amdgpu_bo_va_op_raw

So these *can* return VK_ERROR_DEVICE_LOST, but do they *have to* do
that after a reset? :)


-- 
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 4/4] drm/amdgpu: reset fpriv vram_lost_counter

2017-05-17 Thread Christian König

Am 17.05.2017 um 10:46 schrieb zhoucm1:



On 2017年05月17日 16:40, Christian König wrote:

Am 17.05.2017 um 10:01 schrieb Michel Dänzer:

On 17/05/17 04:13 PM, zhoucm1 wrote:

On 2017年05月17日 14:57, Michel Dänzer wrote:

On 17/05/17 01:28 PM, zhoucm1 wrote:

On 2017年05月17日 11:15, Michel Dänzer wrote:

On 17/05/17 12:04 PM, zhoucm1 wrote:

On 2017年05月17日 09:18, Michel Dänzer wrote:

On 16/05/17 06:25 PM, Chunming Zhou wrote:

Change-Id: I8eb6d7f558da05510e429d3bf1d48c8cec6c1977
Signed-off-by: Chunming Zhou 

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index bca1fb5..f3e7525 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2547,6 +2547,9 @@ int amdgpu_vm_ioctl(struct drm_device 
*dev,

void *data, struct drm_file *filp)
 case AMDGPU_VM_OP_UNRESERVE_VMID:
 amdgpu_vm_free_reserved_vmid(adev, &fpriv->vm,
AMDGPU_GFXHUB);
 break;
+case AMDGPU_VM_OP_RESET:
+fpriv->vram_lost_counter =
atomic_read(&adev->vram_lost_counter);
+break;
How do you envision the UMDs using this? I can mostly think of 
them
calling this ioctl when a context is created or destroyed. But 
that
would also allow any other remaining contexts using the same 
DRM file
descriptor to use all ioctls again. So, I think there needs to 
be a

vram_lost_counter in struct amdgpu_ctx instead of in struct
amdgpu_fpriv.
struct amdgpu_fpriv for vram_lost_counter is proper place, 
especially

for ioctl return value.
if you need to reset ctx one by one, we can mark all contexts 
of that

vm, and then reset by userspace.

I'm not following. With vram_lost_counter in amdgpu_fpriv, if any
context calls this ioctl, all other contexts using the same file
descriptor will also be considered safe again, right?

Yes, but it really depends on userspace requirement, if you need to
reset ctx one by one, we can mark all contexts of that vm to 
guilty, and

then reset one context by userspace.

Still not sure what you mean by that.

E.g. what do you mean by "guilty"? I thought that refers to the 
context
which caused a hang. But it seems like you're using it to refer to 
any

context which hasn't reacted yet to VRAM contents being lost.

When vram is lost, we treat all contexts need to reset.

Essentially, your patches only track VRAM contents being lost per file
descriptor, not per context. I'm not sure (rather skeptical) that this
is suitable for OpenGL UMDs, since state is usually tracked per 
context.

Marek / Nicolai?


Oh, yeah that's a good point.

The problem with tracking it per context is that Vulkan also wants 
the ENODEV on the amdgpu_gem_va_ioct() and amdgpu_info_ioctl() which 
are context less.


But thinking more about this blocking those two doesn't make much 
sense. The VM content can be restored and why should be disallow 
reading GPU info?

I can re-paste the Vulkan APIs requiring ENODEV:
"

The Vulkan APIs listed below could return VK_ERROR_DEVICE_LOST 
according to the spec.


I tries to provide a list of u/k interfaces that could be called for 
each vk API.




Well those are the Vulkan requirements, but that doesn't necessary mean 
we must follow that on the kernel side. Keep in mind that Vulkan can't 
made any requirements towards the kernel driver.


IIRC we already have a query Vulkan can use to figure out if a GPU reset 
happened or not. So they could use that instead.


Regards,
Christian.


vkCreateDevice

-amdgpu_device_initialize.

-amdgpu_query_gpu_info

vkQueueSubmit

-amdgpu_cs_submit

vkWaitForFences

amdgpu_cs_wait_fences

vkGetEventStatus

vkQueueWaitIdle

vkDeviceWaitIdle

vkGetQueryPoolResults**

amdgpu_cs_query_Fence_status

vkQueueBindSparse**

amdgpu_bo_va_op

amdgpu_bo_va_op_raw

vkCreateSwapchainKHR**

vkAcquireNextImageKHR**

vkQueuePresentKHR

Not related with u/k interface.**

**

Besides those listed above, I think 
amdgpu_cs_signal_Sem/amdgpu_cs_wait_sem should respond to gpu reset as 
well."


Christian.





___
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 4/5] drm/amdgpu: Support page directory update via CPU

2017-05-17 Thread zhoucm1



On 2017年05月17日 16:48, Christian König wrote:

Am 17.05.2017 um 03:54 schrieb zhoucm1:



On 2017年05月17日 05:02, Kasiviswanathan, Harish wrote:



-Original Message-
From: Zhou, David(ChunMing)
Sent: Monday, May 15, 2017 10:50 PM
To: Kasiviswanathan, Harish ; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/5] drm/amdgpu: Support page directory update 
via CPU




On 2017年05月16日 05:32, Harish Kasiviswanathan wrote:
> If amdgpu.vm_update_context param is set to use CPU, then Page
> Directories will be updated by CPU instead of SDMA
>
> Signed-off-by: Harish Kasiviswanathan 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 151 
-

>   1 file changed, 109 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

> index 9c89cb2..d72a624 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -271,6 +271,7 @@ static int amdgpu_vm_alloc_levels(struct 
amdgpu_device *adev,

>  uint64_t saddr, uint64_t eaddr,
>  unsigned level)
>   {
> + u64 flags;
>unsigned shift = (adev->vm_manager.num_level - level) *
>adev->vm_manager.block_size;
>unsigned pt_idx, from, to;
> @@ -299,6 +300,14 @@ static int amdgpu_vm_alloc_levels(struct 
amdgpu_device *adev,

>saddr = saddr & ((1 << shift) - 1);
>eaddr = eaddr & ((1 << shift) - 1);
>
> + flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
> + AMDGPU_GEM_CREATE_VRAM_CLEARED;
> + if (vm->use_cpu_for_update)
> + flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
I think shadow flag is need for CPU case as well, which is used to
backup VM bo and meaningful when gpu reset.
same comment for pd bo.

[HK]: Yes support for shadow BOs are desirable and it could be 
implemented as a separate commit. For supporting shadow BOs the 
caller should explicitly add shadow BOs into 
ttm_eu_reserve_buffer(..) to remove the BO from TTM swap list or 
ttm_bo_kmap has to be modified. This implementation for CPU update 
of VM page tables is mainly for KFD usage. Graphics will use for 
experimental and testing purpose. From KFD's view point shadow BO 
are not useful because if GPU is reset then all queue information is 
lost (since submissions are done by user space) and it is not 
possible to recover.

Either way is fine to me.


Actually I'm thinking about if we shouldn't completely drop the shadow 
handling.


When VRAM is lost we now completely drop all jobs, so for new jobs we 
can recreate the page table content from the VM structures as well.
For KGD, I agree. if their process is using both KGD and KFD, I still 
think shadow bo is needed.




When VRAM is not lost we don't need to restore the page tables.
In fact, our 'vram lost' detection isn't critical, I was told by other 
team, they encountered case that just some part vram is lost. So 
restoring page table seems still need for vram isn't lost.


Regards,
David Zhou


What do you think?



Regards,
Christian.



David Zhou


Regards,
David Zhou
> + else
> + flags |= (AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
> + AMDGPU_GEM_CREATE_SHADOW);
> +
>/* walk over the address space and allocate the page tables */
>for (pt_idx = from; pt_idx <= to; ++pt_idx) {
>struct reservation_object *resv = 
vm->root.bo->tbo.resv;
> @@ -310,10 +319,7 @@ static int amdgpu_vm_alloc_levels(struct 
amdgpu_device *adev,

> amdgpu_vm_bo_size(adev, level),
> AMDGPU_GPU_PAGE_SIZE, true,
> AMDGPU_GEM_DOMAIN_VRAM,
> - AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
> - AMDGPU_GEM_CREATE_SHADOW |
> - AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
> - AMDGPU_GEM_CREATE_VRAM_CLEARED,
> +  flags,
> NULL, resv, &pt);
>if (r)
>return r;
> @@ -952,6 +958,43 @@ static uint64_t amdgpu_vm_map_gart(const 
dma_addr_t *pages_addr, uint64_t addr)

>return result;
>   }
>
> +/**
> + * amdgpu_vm_cpu_set_ptes - helper to update page tables via CPU
> + *
> + * @params: see amdgpu_pte_update_params definition
> + * @pe: kmap addr of the page entry
> + * @addr: dst addr to write into pe
> + * @count: number of page entries to update
> + * @incr: increase next addr by incr bytes
> + * @flags: hw access flags
> + */
> +static void amdgpu_vm_cpu_set_ptes(struct 
amdgpu_pte_update_params *params,

> +uint64_t pe, uint64_t addr,
> +unsigned count, uint32_t incr,
> +uint64_t flags)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < count; i++) {
> + amdgpu_gart_set_pte_pde(params->adev, (void *)pe,
> + i, addr, flags);
> + addr += incr;
> + }
> +
> + mb();
> + amdgpu_gart_flush_gpu_tlb(para

Re: [PATCH 4/5] drm/amdgpu: Support page directory update via CPU

2017-05-17 Thread Christian König

Am 17.05.2017 um 10:53 schrieb zhoucm1:



On 2017年05月17日 16:48, Christian König wrote:

Am 17.05.2017 um 03:54 schrieb zhoucm1:



On 2017年05月17日 05:02, Kasiviswanathan, Harish wrote:



-Original Message-
From: Zhou, David(ChunMing)
Sent: Monday, May 15, 2017 10:50 PM
To: Kasiviswanathan, Harish ; 
amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/5] drm/amdgpu: Support page directory update 
via CPU




On 2017年05月16日 05:32, Harish Kasiviswanathan wrote:
> If amdgpu.vm_update_context param is set to use CPU, then Page
> Directories will be updated by CPU instead of SDMA
>
> Signed-off-by: Harish Kasiviswanathan 


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 151 
-

>   1 file changed, 109 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c

> index 9c89cb2..d72a624 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -271,6 +271,7 @@ static int amdgpu_vm_alloc_levels(struct 
amdgpu_device *adev,

>  uint64_t saddr, uint64_t eaddr,
>  unsigned level)
>   {
> + u64 flags;
>unsigned shift = (adev->vm_manager.num_level - level) *
>adev->vm_manager.block_size;
>unsigned pt_idx, from, to;
> @@ -299,6 +300,14 @@ static int amdgpu_vm_alloc_levels(struct 
amdgpu_device *adev,

>saddr = saddr & ((1 << shift) - 1);
>eaddr = eaddr & ((1 << shift) - 1);
>
> + flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
> + AMDGPU_GEM_CREATE_VRAM_CLEARED;
> + if (vm->use_cpu_for_update)
> + flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
I think shadow flag is need for CPU case as well, which is used to
backup VM bo and meaningful when gpu reset.
same comment for pd bo.

[HK]: Yes support for shadow BOs are desirable and it could be 
implemented as a separate commit. For supporting shadow BOs the 
caller should explicitly add shadow BOs into 
ttm_eu_reserve_buffer(..) to remove the BO from TTM swap list or 
ttm_bo_kmap has to be modified. This implementation for CPU update 
of VM page tables is mainly for KFD usage. Graphics will use for 
experimental and testing purpose. From KFD's view point shadow BO 
are not useful because if GPU is reset then all queue information 
is lost (since submissions are done by user space) and it is not 
possible to recover.

Either way is fine to me.


Actually I'm thinking about if we shouldn't completely drop the 
shadow handling.


When VRAM is lost we now completely drop all jobs, so for new jobs we 
can recreate the page table content from the VM structures as well.
For KGD, I agree. if their process is using both KGD and KFD, I still 
think shadow bo is needed.




When VRAM is not lost we don't need to restore the page tables.
In fact, our 'vram lost' detection isn't critical, I was told by other 
team, they encountered case that just some part vram is lost. So 
restoring page table seems still need for vram isn't lost.


Ok, random VRAM corruption caused by a GPU reset is a good argument. So 
we should keep this feature.


Regards,
Christian.



Regards,
David Zhou


What do you think?



Regards,
Christian.



David Zhou


Regards,
David Zhou
> + else
> + flags |= (AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
> + AMDGPU_GEM_CREATE_SHADOW);
> +
>/* walk over the address space and allocate the page tables */
>for (pt_idx = from; pt_idx <= to; ++pt_idx) {
>struct reservation_object *resv = 
vm->root.bo->tbo.resv;
> @@ -310,10 +319,7 @@ static int amdgpu_vm_alloc_levels(struct 
amdgpu_device *adev,

> amdgpu_vm_bo_size(adev, level),
> AMDGPU_GPU_PAGE_SIZE, true,
> AMDGPU_GEM_DOMAIN_VRAM,
> - AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
> - AMDGPU_GEM_CREATE_SHADOW |
> - AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
> - AMDGPU_GEM_CREATE_VRAM_CLEARED,
> +  flags,
> NULL, resv, &pt);
>if (r)
>return r;
> @@ -952,6 +958,43 @@ static uint64_t amdgpu_vm_map_gart(const 
dma_addr_t *pages_addr, uint64_t addr)

>return result;
>   }
>
> +/**
> + * amdgpu_vm_cpu_set_ptes - helper to update page tables via CPU
> + *
> + * @params: see amdgpu_pte_update_params definition
> + * @pe: kmap addr of the page entry
> + * @addr: dst addr to write into pe
> + * @count: number of page entries to update
> + * @incr: increase next addr by incr bytes
> + * @flags: hw access flags
> + */
> +static void amdgpu_vm_cpu_set_ptes(struct 
amdgpu_pte_update_params *params,

> +uint64_t pe, uint64_t addr,
> +unsigned count, uint32_t incr,
> +uint64_t flags)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < count; i++) {
> + amdgpu_gart_set_pte_pde(params->a

[PATCH] drm/amdgpu: update golden settings

2017-05-17 Thread Ken Wang
Change-Id: Ifcc39748c36273fa764cd2641d4b44405dbf59a5
Signed-off-by: Ken Wang 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 39ae97b..de35de2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -86,14 +86,27 @@ static const struct amdgpu_gds_reg_offset 
amdgpu_gds_reg_offset[] =
 
 static const u32 golden_settings_gc_9_0[] =
 {
-   SOC15_REG_OFFSET(GC, 0, mmDB_DEBUG2), 0xf00ffeff, 0x0400,
+   SOC15_REG_OFFSET(GC, 0, mmCPC_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmCPF_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmCPG_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmDB_DEBUG2), 0xf00f, 0x0420,
+   SOC15_REG_OFFSET(GC, 0, mmGB_GPU_ID), 0x000f, 0x,
+   SOC15_REG_OFFSET(GC, 0, mmIA_UTCL1_CNTL), 0x0800, 0x0880,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_BINNER_EVENT_CNTL_3), 0x0003, 
0x82400024,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_ENHANCE), 0x3fff, 0x0001,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_LINE_STIPPLE_STATE), 0xff0f, 
0x,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UTCL1_CNTL_0), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UTCL1_CNTL_1), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UTCL1_CNTL_2), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_PREWALKER_UTCL1_CNTL), 0x0800, 
0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_SPM_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmSPI_CONFIG_CNTL_1), 0x000f, 0x01000107,
SOC15_REG_OFFSET(GC, 0, mmTA_CNTL_AUX), 0xfeef, 0x010b,
SOC15_REG_OFFSET(GC, 0, mmTCP_CHAN_STEER_HI), 0x, 0x4a2c0e68,
SOC15_REG_OFFSET(GC, 0, mmTCP_CHAN_STEER_LO), 0x, 0xb5d3f197,
-   SOC15_REG_OFFSET(GC, 0, mmVGT_GS_MAX_WAVE_ID), 0x0fff, 0x03ff
+   SOC15_REG_OFFSET(GC, 0, mmVGT_CACHE_INVALIDATION), 0x3fff3af3, 
0x1920,
+   SOC15_REG_OFFSET(GC, 0, mmVGT_GS_MAX_WAVE_ID), 0x0fff, 0x03ff,
+   SOC15_REG_OFFSET(GC, 0, mmWD_UTCL1_CNTL), 0x0800, 0x0880
 };
 
 static const u32 golden_settings_gc_9_0_vg10[] =
@@ -104,8 +117,7 @@ static const u32 golden_settings_gc_9_0_vg10[] =
SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG_READ), 0x77ff, 0x2a114042,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_ENHANCE_1), 0x8000, 0x00048000,
SOC15_REG_OFFSET(GC, 0, mmRMI_UTCL1_CNTL2), 0x0003, 0x0002,
-   SOC15_REG_OFFSET(GC, 0, mmTD_CNTL), 0x1800, 0x0800,
-   SOC15_REG_OFFSET(GC, 0, mmSPI_CONFIG_CNTL_1),0x000f, 0x0007
+   SOC15_REG_OFFSET(GC, 0, mmTD_CNTL), 0x1800, 0x0800
 };
 
 #define VEGA10_GB_ADDR_CONFIG_GOLDEN 0x2a114042
-- 
2.7.4

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


[PATCH 3/7] drm/amdgpu: fix another fundamental VM bug

2017-05-17 Thread Christian König
From: Christian König 

I always wondered why this code uses the MC address. Now it came to me that
this is actually a bug and only works by coincident because we used to have
VRAM mapped at zero.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index db1542c..967786f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -884,7 +884,7 @@ static int amdgpu_cs_ib_vm_chunk(struct amdgpu_device *adev,
}
 
if (p->job->vm) {
-   p->job->vm_pd_addr = amdgpu_bo_gpu_offset(vm->root.bo);
+   p->job->vm_pd_addr = vm->root.bo->tbo.mem.start << PAGE_SHIFT;
 
r = amdgpu_bo_vm_update_pte(p);
if (r)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index b877f9f3..7256fcc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1012,7 +1012,7 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
return r;
}
 
-   pt = amdgpu_bo_gpu_offset(bo);
+   pt = bo->tbo.mem.start << PAGE_SHIFT;
if (parent->entries[pt_idx].addr == pt)
continue;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 047b1a7..63cb573 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -360,7 +360,7 @@ static uint64_t gmc_v9_0_get_vm_pte_flags(struct 
amdgpu_device *adev,
 
 static u64 gmc_v9_0_get_vm_pde(struct amdgpu_device *adev, u64 addr)
 {
-   addr = adev->vm_manager.vram_base_offset + addr - adev->mc.vram_start;
+   addr = adev->vm_manager.vram_base_offset + addr;
BUG_ON(addr & 0x003FULL);
return addr;
 }
-- 
2.7.4

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


[PATCH 6/7] drm/amdgpu: stop joining VM PTE updates

2017-05-17 Thread Christian König
From: Christian König 

This isn't beneficial any more since VRAM allocations are now split
so that they fits into a single page table.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 61 --
 1 file changed, 7 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 6a926f4..860a669 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1191,41 +1191,12 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
struct amdgpu_device *adev = params->adev;
const uint64_t mask = AMDGPU_VM_PTE_COUNT(adev) - 1;
 
-   uint64_t cur_pe_start, cur_nptes, cur_dst;
-   uint64_t addr; /* next GPU address to be updated */
+   uint64_t addr, pe_start;
struct amdgpu_bo *pt;
-   unsigned nptes; /* next number of ptes to be updated */
-   uint64_t next_pe_start;
-
-   /* initialize the variables */
-   addr = start;
-   pt = amdgpu_vm_get_pt(params, addr);
-   if (!pt) {
-   pr_err("PT not found, aborting update_ptes\n");
-   return -EINVAL;
-   }
-
-   if (params->shadow) {
-   if (!pt->shadow)
-   return 0;
-   pt = pt->shadow;
-   }
-   if ((addr & ~mask) == (end & ~mask))
-   nptes = end - addr;
-   else
-   nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);
-
-   cur_pe_start = amdgpu_bo_gpu_offset(pt);
-   cur_pe_start += (addr & mask) * 8;
-   cur_nptes = nptes;
-   cur_dst = dst;
-
-   /* for next ptb*/
-   addr += nptes;
-   dst += nptes * AMDGPU_GPU_PAGE_SIZE;
+   unsigned nptes;
 
/* walk over the address space and update the page tables */
-   while (addr < end) {
+   for (addr = start; addr < end; addr += nptes) {
pt = amdgpu_vm_get_pt(params, addr);
if (!pt) {
pr_err("PT not found, aborting update_ptes\n");
@@ -1243,33 +1214,15 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
else
nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);
 
-   next_pe_start = amdgpu_bo_gpu_offset(pt);
-   next_pe_start += (addr & mask) * 8;
-
-   if ((cur_pe_start + 8 * cur_nptes) == next_pe_start &&
-   ((cur_nptes + nptes) <= AMDGPU_VM_MAX_UPDATE_SIZE)) {
-   /* The next ptb is consecutive to current ptb.
-* Don't call the update function now.
-* Will update two ptbs together in future.
-   */
-   cur_nptes += nptes;
-   } else {
-   params->func(params, cur_pe_start, cur_dst, cur_nptes,
-AMDGPU_GPU_PAGE_SIZE, flags);
+   pe_start = amdgpu_bo_gpu_offset(pt);
+   pe_start += (addr & mask) * 8;
 
-   cur_pe_start = next_pe_start;
-   cur_nptes = nptes;
-   cur_dst = dst;
-   }
+   params->func(params, pe_start, dst, nptes,
+AMDGPU_GPU_PAGE_SIZE, flags);
 
-   /* for next ptb*/
-   addr += nptes;
dst += nptes * AMDGPU_GPU_PAGE_SIZE;
}
 
-   params->func(params, cur_pe_start, cur_dst, cur_nptes,
-AMDGPU_GPU_PAGE_SIZE, flags);
-
return 0;
 }
 
-- 
2.7.4

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


[PATCH 2/7] drm/amdgpu: add some extra VM error handling

2017-05-17 Thread Christian König
From: Christian König 

If updating the PDs fails we now invalidate all entries to try again later.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 34 +-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 344f943..b877f9f3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1105,6 +1105,32 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
 }
 
 /*
+ * amdgpu_vm_invalidate_level - mark all PD levels as invalid
+ *
+ * @parent: parent PD
+ *
+ * Mark all PD level as invalid after an error.
+ */
+static void amdgpu_vm_invalidate_level(struct amdgpu_vm_pt *parent)
+{
+   unsigned pt_idx;
+
+   /*
+* Recurse into the subdirectories. This recursion is harmless because
+* we only have a maximum of 5 layers.
+*/
+   for (pt_idx = 0; pt_idx <= parent->last_entry_used; ++pt_idx) {
+   struct amdgpu_vm_pt *entry = &parent->entries[pt_idx];
+
+   if (!entry->bo)
+   continue;
+
+   entry->addr = ~0ULL;
+   amdgpu_vm_invalidate_level(entry);
+   }
+}
+
+/*
  * amdgpu_vm_update_directories - make sure that all directories are valid
  *
  * @adev: amdgpu_device pointer
@@ -1116,7 +1142,13 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
 int amdgpu_vm_update_directories(struct amdgpu_device *adev,
 struct amdgpu_vm *vm)
 {
-   return amdgpu_vm_update_level(adev, vm, &vm->root, 0);
+   int r;
+
+   r = amdgpu_vm_update_level(adev, vm, &vm->root, 0);
+   if (r)
+   amdgpu_vm_invalidate_level(&vm->root);
+
+   return r;
 }
 
 /**
-- 
2.7.4

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


[PATCH 5/7] drm/amdgpu: cache the complete pde

2017-05-17 Thread Christian König
From: Christian König 

Makes it easier to update the PDE with huge pages.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 16 +---
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 8676a75..6a926f4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1013,6 +1013,7 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
}
 
pt = bo->tbo.mem.start << PAGE_SHIFT;
+   pt = amdgpu_gart_get_vm_pde(adev, pt);
if (parent->entries[pt_idx].addr == pt)
continue;
 
@@ -1024,18 +1025,15 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
(count == AMDGPU_VM_MAX_UPDATE_SIZE)) {
 
if (count) {
-   uint64_t entry;
-
-   entry = amdgpu_gart_get_vm_pde(adev, last_pt);
if (shadow)
amdgpu_vm_do_set_ptes(¶ms,
  last_shadow,
- entry, count,
+ last_pt, count,
  incr,
  AMDGPU_PTE_VALID);
 
amdgpu_vm_do_set_ptes(¶ms, last_pde,
- entry, count, incr,
+ last_pt, count, incr,
  AMDGPU_PTE_VALID);
}
 
@@ -1049,15 +1047,11 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
}
 
if (count) {
-   uint64_t entry;
-
-   entry = amdgpu_gart_get_vm_pde(adev, last_pt);
-
if (vm->root.bo->shadow)
-   amdgpu_vm_do_set_ptes(¶ms, last_shadow, entry,
+   amdgpu_vm_do_set_ptes(¶ms, last_shadow, last_pt,
  count, incr, AMDGPU_PTE_VALID);
 
-   amdgpu_vm_do_set_ptes(¶ms, last_pde, entry,
+   amdgpu_vm_do_set_ptes(¶ms, last_pde, last_pt,
  count, incr, AMDGPU_PTE_VALID);
}
 
-- 
2.7.4

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


[PATCH 4/7] drm/amdgpu: Return EINVAL if no PT BO

2017-05-17 Thread Christian König
From: Harish Kasiviswanathan 

This change is also useful for the upcoming changes where page tables
can be updated by CPU.

Change-Id: I07510ed60c94cf1944ee96bb4b16c40ec88ea17c
Signed-off-by: Harish Kasiviswanathan 
Reviewed-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 48 +-
 1 file changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 7256fcc..8676a75 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1188,8 +1188,9 @@ static struct amdgpu_bo *amdgpu_vm_get_pt(struct 
amdgpu_pte_update_params *p,
  * @flags: mapping flags
  *
  * Update the page tables in the range @start - @end.
+ * Returns 0 for success, -EINVAL for failure.
  */
-static void amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
+static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params *params,
  uint64_t start, uint64_t end,
  uint64_t dst, uint64_t flags)
 {
@@ -1207,12 +1208,12 @@ static void amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
pt = amdgpu_vm_get_pt(params, addr);
if (!pt) {
pr_err("PT not found, aborting update_ptes\n");
-   return;
+   return -EINVAL;
}
 
if (params->shadow) {
if (!pt->shadow)
-   return;
+   return 0;
pt = pt->shadow;
}
if ((addr & ~mask) == (end & ~mask))
@@ -1234,12 +1235,12 @@ static void amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
pt = amdgpu_vm_get_pt(params, addr);
if (!pt) {
pr_err("PT not found, aborting update_ptes\n");
-   return;
+   return -EINVAL;
}
 
if (params->shadow) {
if (!pt->shadow)
-   return;
+   return 0;
pt = pt->shadow;
}
 
@@ -1274,6 +1275,8 @@ static void amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
 
params->func(params, cur_pe_start, cur_dst, cur_nptes,
 AMDGPU_GPU_PAGE_SIZE, flags);
+
+   return 0;
 }
 
 /*
@@ -1285,11 +1288,14 @@ static void amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
  * @end: last PTE to handle
  * @dst: addr those PTEs should point to
  * @flags: hw mapping flags
+ * Returns 0 for success, -EINVAL for failure.
  */
-static void amdgpu_vm_frag_ptes(struct amdgpu_pte_update_params*params,
+static int amdgpu_vm_frag_ptes(struct amdgpu_pte_update_params *params,
uint64_t start, uint64_t end,
uint64_t dst, uint64_t flags)
 {
+   int r;
+
/**
 * The MC L1 TLB supports variable sized pages, based on a fragment
 * field in the PTE. When this field is set to a non-zero value, page
@@ -1318,28 +1324,30 @@ static void amdgpu_vm_frag_ptes(struct 
amdgpu_pte_update_params *params,
 
/* system pages are non continuously */
if (params->src || !(flags & AMDGPU_PTE_VALID) ||
-   (frag_start >= frag_end)) {
-
-   amdgpu_vm_update_ptes(params, start, end, dst, flags);
-   return;
-   }
+   (frag_start >= frag_end))
+   return amdgpu_vm_update_ptes(params, start, end, dst, flags);
 
/* handle the 4K area at the beginning */
if (start != frag_start) {
-   amdgpu_vm_update_ptes(params, start, frag_start,
- dst, flags);
+   r = amdgpu_vm_update_ptes(params, start, frag_start,
+ dst, flags);
+   if (r)
+   return r;
dst += (frag_start - start) * AMDGPU_GPU_PAGE_SIZE;
}
 
/* handle the area in the middle */
-   amdgpu_vm_update_ptes(params, frag_start, frag_end, dst,
- flags | frag_flags);
+   r = amdgpu_vm_update_ptes(params, frag_start, frag_end, dst,
+ flags | frag_flags);
+   if (r)
+   return r;
 
/* handle the 4K area at the end */
if (frag_end != end) {
dst += (frag_end - frag_start) * AMDGPU_GPU_PAGE_SIZE;
-   amdgpu_vm_update_ptes(params, frag_end, end, dst, flags);
+   r = amdgpu_vm_update_ptes(params, frag_end, end, dst, flags);
}
+   return r;
 }
 
 /**
@@ -1460,9 +1468,13 @@ static int amdgpu_vm_bo_update_mapping(struct 
amdgpu_device *adev,
goto error_free;
 
params.shadow = true;
-   amdgpu_vm_frag_ptes(¶ms, start, last + 1, addr, flags);
+   r = amdgpu_vm_frag

[PATCH 7/7] drm/amdgpu: enable huge page handling in the VM

2017-05-17 Thread Christian König
From: Christian König 

The hardware can use huge pages to map 2MB of address space with only one PDE.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 96 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 ++
 2 files changed, 76 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 860a669..8be1d7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -325,6 +325,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device 
*adev,
 
entry->bo = pt;
entry->addr = 0;
+   entry->huge_page = false;
}
 
if (level < adev->vm_manager.num_level) {
@@ -1014,7 +1015,8 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
 
pt = bo->tbo.mem.start << PAGE_SHIFT;
pt = amdgpu_gart_get_vm_pde(adev, pt);
-   if (parent->entries[pt_idx].addr == pt)
+   if (parent->entries[pt_idx].addr == pt ||
+   parent->entries[pt_idx].huge_page)
continue;
 
parent->entries[pt_idx].addr = pt;
@@ -1146,29 +1148,70 @@ int amdgpu_vm_update_directories(struct amdgpu_device 
*adev,
 }
 
 /**
- * amdgpu_vm_find_pt - find the page table for an address
+ * amdgpu_vm_handle_huge_pages - handle updating the PD with huge pages
  *
  * @p: see amdgpu_pte_update_params definition
  * @addr: virtual address in question
+ * @nptes: number of PTEs updated with this operation
+ * @dst: destination address where the PTEs should point to
+ * @flags: access flags fro the PTEs
+ * @bo: resulting page tables BO
  *
- * Find the page table BO for a virtual address, return NULL when none found.
+ * Check if we can update the PD with a huge page. Also finds the page table
+ * BO for a virtual address, returns -ENOENT when nothing found.
  */
-static struct amdgpu_bo *amdgpu_vm_get_pt(struct amdgpu_pte_update_params *p,
- uint64_t addr)
+static int amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
+  uint64_t addr, unsigned nptes,
+  uint64_t dst, uint64_t flags,
+  struct amdgpu_bo **bo)
 {
-   struct amdgpu_vm_pt *entry = &p->vm->root;
-   unsigned idx, level = p->adev->vm_manager.num_level;
+   unsigned pt_idx, level = p->adev->vm_manager.num_level;
+   struct amdgpu_vm_pt *entry = &p->vm->root, *parent;
+   uint64_t pd_addr, pde, pt;
 
-   while (entry->entries) {
-   idx = addr >> (p->adev->vm_manager.block_size * level--);
-   idx %= amdgpu_bo_size(entry->bo) / 8;
-   entry = &entry->entries[idx];
-   }
+   do {
+   pt_idx = addr >> (p->adev->vm_manager.block_size * level--);
+   pt_idx %= amdgpu_bo_size(entry->bo) / 8;
+   parent = entry;
+   entry = &entry->entries[pt_idx];
+   } while (entry->entries);
 
if (level)
-   return NULL;
+   return -ENOENT;
+
+   *bo = entry->bo;
+
+   /* In the case of a mixed PT the PDE must point to it*/
+   if (p->adev->asic_type < CHIP_VEGA10 ||
+   nptes != AMDGPU_VM_PTE_COUNT(p->adev) ||
+   p->func != amdgpu_vm_do_set_ptes ||
+   !(flags & AMDGPU_PTE_VALID)) {
+
+   pt = (*bo)->tbo.mem.start << PAGE_SHIFT;
+   pt = amdgpu_gart_get_vm_pde(p->adev, pt);
+   flags = AMDGPU_PTE_VALID;
+   } else {
+   pt = amdgpu_gart_get_vm_pde(p->adev, dst);
+   flags |= AMDGPU_PDE_PTE;
+   }
 
-   return entry->bo;
+   if (entry->addr == pt &&
+   entry->huge_page == !!(flags & AMDGPU_PDE_PTE))
+   return 0;
+
+   entry->addr = pt;
+   entry->huge_page = !!(flags & AMDGPU_PDE_PTE);
+
+   if (parent->bo->shadow) {
+   pd_addr = amdgpu_bo_gpu_offset(parent->bo->shadow);
+   pde = pd_addr + pt_idx * 8;
+   amdgpu_vm_do_set_ptes(p, pde, pt, 1, 0, flags);
+   }
+
+   pd_addr = amdgpu_bo_gpu_offset(parent->bo);
+   pde = pd_addr + pt_idx * 8;
+   amdgpu_vm_do_set_ptes(p, pde, pt, 1, 0, flags);
+   return 0;
 }
 
 /**
@@ -1194,14 +1237,20 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
uint64_t addr, pe_start;
struct amdgpu_bo *pt;
unsigned nptes;
+   int r;
 
/* walk over the address space and update the page tables */
for (addr = start; addr < end; addr += nptes) {
-   pt = amdgpu_vm_get_pt(params, addr);
-   if (!pt) {
-   pr_err("PT not found, aborting update_ptes\n");
-   return -EINVAL;
-   

[PATCH 1/7] drm/amdgpu: cleanup adjust_mc_addr handling v2

2017-05-17 Thread Christian König
From: Christian König 

Rename adjust_mc_addr to get_vm_pde and check the address bits in one place.

v2: handle vcn as well, keep setting the valid bit manually,
add a BUG_ON() for GMC v6, v7 and v8 as well.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  5 +++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 26 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  |  6 ++
 drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c  |  7 +++
 drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  |  9 -
 drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  |  9 -
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  | 10 ++
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c |  6 ++
 drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c  | 12 
 drivers/gpu/drm/amd/amdgpu/vce_v4_0.c  |  6 ++
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c  |  5 ++---
 11 files changed, 53 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ae7e775..4acbca7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -309,8 +309,8 @@ struct amdgpu_gart_funcs {
/* set pte flags based per asic */
uint64_t (*get_vm_pte_flags)(struct amdgpu_device *adev,
 uint32_t flags);
-   /* adjust mc addr in fb for APU case */
-   u64 (*adjust_mc_addr)(struct amdgpu_device *adev, u64 addr);
+   /* get the pde for a given mc addr */
+   u64 (*get_vm_pde)(struct amdgpu_device *adev, u64 addr);
uint32_t (*get_invalidate_req)(unsigned int vm_id);
 };
 
@@ -1820,6 +1820,7 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
 #define amdgpu_asic_get_config_memsize(adev) 
(adev)->asic_funcs->get_config_memsize((adev))
 #define amdgpu_gart_flush_gpu_tlb(adev, vmid) 
(adev)->gart.gart_funcs->flush_gpu_tlb((adev), (vmid))
 #define amdgpu_gart_set_pte_pde(adev, pt, idx, addr, flags) 
(adev)->gart.gart_funcs->set_pte_pde((adev), (pt), (idx), (addr), (flags))
+#define amdgpu_gart_get_vm_pde(adev, addr) 
(adev)->gart.gart_funcs->get_vm_pde((adev), (addr))
 #define amdgpu_vm_copy_pte(adev, ib, pe, src, count) 
((adev)->vm_manager.vm_pte_funcs->copy_pte((ib), (pe), (src), (count)))
 #define amdgpu_vm_write_pte(adev, ib, pe, value, count, incr) 
((adev)->vm_manager.vm_pte_funcs->write_pte((ib), (pe), (value), (count), 
(incr)))
 #define amdgpu_vm_set_pte_pde(adev, ib, pe, addr, count, incr, flags) 
((adev)->vm_manager.vm_pte_funcs->set_pte_pde((ib), (pe), (addr), (count), 
(incr), (flags)))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 88420dc..344f943 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -682,16 +682,6 @@ static bool amdgpu_vm_ring_has_compute_vm_bug(struct 
amdgpu_ring *ring)
return false;
 }
 
-static u64 amdgpu_vm_adjust_mc_addr(struct amdgpu_device *adev, u64 mc_addr)
-{
-   u64 addr = mc_addr;
-
-   if (adev->gart.gart_funcs->adjust_mc_addr)
-   addr = adev->gart.gart_funcs->adjust_mc_addr(adev, addr);
-
-   return addr;
-}
-
 bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
  struct amdgpu_job *job)
 {
@@ -1034,18 +1024,18 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
(count == AMDGPU_VM_MAX_UPDATE_SIZE)) {
 
if (count) {
-   uint64_t pt_addr =
-   amdgpu_vm_adjust_mc_addr(adev, last_pt);
+   uint64_t entry;
 
+   entry = amdgpu_gart_get_vm_pde(adev, last_pt);
if (shadow)
amdgpu_vm_do_set_ptes(¶ms,
  last_shadow,
- pt_addr, count,
+ entry, count,
  incr,
  AMDGPU_PTE_VALID);
 
amdgpu_vm_do_set_ptes(¶ms, last_pde,
- pt_addr, count, incr,
+ entry, count, incr,
  AMDGPU_PTE_VALID);
}
 
@@ -1059,13 +1049,15 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
}
 
if (count) {
-   uint64_t pt_addr = amdgpu_vm_adjust_mc_addr(adev, last_pt);
+   uint64_t entry;
+
+   entry = amdgpu_gart_get_vm_pde(adev, last_pt);
 
if (vm->root.bo->shadow)
-   amdgpu_vm_do_set_ptes(¶ms, last_shadow, pt_addr,
+   amdgpu_vm_do_set_ptes(¶ms,

Re: [PATCH 4/4] drm/amdgpu: reset fpriv vram_lost_counter

2017-05-17 Thread Marek Olšák
David,

We already have a query that returns whether a device is lost. It's
called amdgpu_cs_query_reset_state. That should return whether a hang
was caused by a certain context or whether the hang happened but the
context is innocent. You can extend it to accept no context, in which
case it will return either NO_RESET (everything is OK) or
UNKNOWN_RESET (= when a hang happened but the caller didn't specify
the context).

Marek

On Wed, May 17, 2017 at 10:56 AM, Christian König
 wrote:
> Am 17.05.2017 um 10:46 schrieb zhoucm1:
>
>
>
> On 2017年05月17日 16:40, Christian König wrote:
>
> Am 17.05.2017 um 10:01 schrieb Michel Dänzer:
>
> On 17/05/17 04:13 PM, zhoucm1 wrote:
>
> On 2017年05月17日 14:57, Michel Dänzer wrote:
>
> On 17/05/17 01:28 PM, zhoucm1 wrote:
>
> On 2017年05月17日 11:15, Michel Dänzer wrote:
>
> On 17/05/17 12:04 PM, zhoucm1 wrote:
>
> On 2017年05月17日 09:18, Michel Dänzer wrote:
>
> On 16/05/17 06:25 PM, Chunming Zhou wrote:
>
> Change-Id: I8eb6d7f558da05510e429d3bf1d48c8cec6c1977
> Signed-off-by: Chunming Zhou 
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index bca1fb5..f3e7525 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2547,6 +2547,9 @@ int amdgpu_vm_ioctl(struct drm_device *dev,
> void *data, struct drm_file *filp)
>  case AMDGPU_VM_OP_UNRESERVE_VMID:
>  amdgpu_vm_free_reserved_vmid(adev, &fpriv->vm,
> AMDGPU_GFXHUB);
>  break;
> +case AMDGPU_VM_OP_RESET:
> +fpriv->vram_lost_counter =
> atomic_read(&adev->vram_lost_counter);
> +break;
>
> How do you envision the UMDs using this? I can mostly think of them
> calling this ioctl when a context is created or destroyed. But that
> would also allow any other remaining contexts using the same DRM file
> descriptor to use all ioctls again. So, I think there needs to be a
> vram_lost_counter in struct amdgpu_ctx instead of in struct
> amdgpu_fpriv.
>
> struct amdgpu_fpriv for vram_lost_counter is proper place, especially
> for ioctl return value.
> if you need to reset ctx one by one, we can mark all contexts of that
> vm, and then reset by userspace.
>
> I'm not following. With vram_lost_counter in amdgpu_fpriv, if any
> context calls this ioctl, all other contexts using the same file
> descriptor will also be considered safe again, right?
>
> Yes, but it really depends on userspace requirement, if you need to
> reset ctx one by one, we can mark all contexts of that vm to guilty, and
> then reset one context by userspace.
>
> Still not sure what you mean by that.
>
> E.g. what do you mean by "guilty"? I thought that refers to the context
> which caused a hang. But it seems like you're using it to refer to any
> context which hasn't reacted yet to VRAM contents being lost.
>
> When vram is lost, we treat all contexts need to reset.
>
> Essentially, your patches only track VRAM contents being lost per file
> descriptor, not per context. I'm not sure (rather skeptical) that this
> is suitable for OpenGL UMDs, since state is usually tracked per context.
> Marek / Nicolai?
>
>
> Oh, yeah that's a good point.
>
> The problem with tracking it per context is that Vulkan also wants the
> ENODEV on the amdgpu_gem_va_ioct() and amdgpu_info_ioctl() which are context
> less.
>
> But thinking more about this blocking those two doesn't make much sense. The
> VM content can be restored and why should be disallow reading GPU info?
>
> I can re-paste the Vulkan APIs requiring ENODEV:
> "
>
> The Vulkan APIs listed below could return VK_ERROR_DEVICE_LOST according to
> the spec.
>
> I tries to provide a list of u/k interfaces that could be called for each vk
> API.
>
>
> Well those are the Vulkan requirements, but that doesn't necessary mean we
> must follow that on the kernel side. Keep in mind that Vulkan can't made any
> requirements towards the kernel driver.
>
> IIRC we already have a query Vulkan can use to figure out if a GPU reset
> happened or not. So they could use that instead.
>
> Regards,
> Christian.
>
>
>
> vkCreateDevice
>
> -  amdgpu_device_initialize.
>
> -  amdgpu_query_gpu_info
>
>
>
> vkQueueSubmit
>
> -  amdgpu_cs_submit
>
>
>
> vkWaitForFences
>
> amdgpu_cs_wait_fences
>
>
>
> vkGetEventStatus
>
> vkQueueWaitIdle
>
> vkDeviceWaitIdle
>
> vkGetQueryPoolResults
>
> amdgpu_cs_query_Fence_status
>
>
>
> vkQueueBindSparse
>
> amdgpu_bo_va_op
>
> amdgpu_bo_va_op_raw
>
>
>
> vkCreateSwapchainKHR
>
> vkAcquireNextImageKHR
>
> vkQueuePresentKHR
>
> Not related with u/k interface.
>
>
>
> Besides those listed above, I think amdgpu_cs_signal_Sem/amdgpu_cs_wait_sem
> should respond to gpu reset as well."
>
>
> Christian.
>
>
>
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>

RE: [PATCH 6/7] drm/amdgpu: stop joining VM PTE updates

2017-05-17 Thread Zhou, David(ChunMing)

Patch#2: Reviewed-by: Chunming Zhou 
Patch#3: RB should be from Hawking, so Acked-by: Chunming Zhou 

Patch#5 #6, are Reviewed-by: Chunming Zhou 

For patch#7, further transfer +1 page table, I need a bit time of tomorrow.

Regards,
David Zhou

-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Christian K?nig
Sent: Wednesday, May 17, 2017 5:23 PM
To: amd-gfx@lists.freedesktop.org
Subject: [PATCH 6/7] drm/amdgpu: stop joining VM PTE updates

From: Christian König 

This isn't beneficial any more since VRAM allocations are now split so that 
they fits into a single page table.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 61 --
 1 file changed, 7 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 6a926f4..860a669 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1191,41 +1191,12 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
struct amdgpu_device *adev = params->adev;
const uint64_t mask = AMDGPU_VM_PTE_COUNT(adev) - 1;
 
-   uint64_t cur_pe_start, cur_nptes, cur_dst;
-   uint64_t addr; /* next GPU address to be updated */
+   uint64_t addr, pe_start;
struct amdgpu_bo *pt;
-   unsigned nptes; /* next number of ptes to be updated */
-   uint64_t next_pe_start;
-
-   /* initialize the variables */
-   addr = start;
-   pt = amdgpu_vm_get_pt(params, addr);
-   if (!pt) {
-   pr_err("PT not found, aborting update_ptes\n");
-   return -EINVAL;
-   }
-
-   if (params->shadow) {
-   if (!pt->shadow)
-   return 0;
-   pt = pt->shadow;
-   }
-   if ((addr & ~mask) == (end & ~mask))
-   nptes = end - addr;
-   else
-   nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);
-
-   cur_pe_start = amdgpu_bo_gpu_offset(pt);
-   cur_pe_start += (addr & mask) * 8;
-   cur_nptes = nptes;
-   cur_dst = dst;
-
-   /* for next ptb*/
-   addr += nptes;
-   dst += nptes * AMDGPU_GPU_PAGE_SIZE;
+   unsigned nptes;
 
/* walk over the address space and update the page tables */
-   while (addr < end) {
+   for (addr = start; addr < end; addr += nptes) {
pt = amdgpu_vm_get_pt(params, addr);
if (!pt) {
pr_err("PT not found, aborting update_ptes\n"); @@ 
-1243,33 +1214,15 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
else
nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);
 
-   next_pe_start = amdgpu_bo_gpu_offset(pt);
-   next_pe_start += (addr & mask) * 8;
-
-   if ((cur_pe_start + 8 * cur_nptes) == next_pe_start &&
-   ((cur_nptes + nptes) <= AMDGPU_VM_MAX_UPDATE_SIZE)) {
-   /* The next ptb is consecutive to current ptb.
-* Don't call the update function now.
-* Will update two ptbs together in future.
-   */
-   cur_nptes += nptes;
-   } else {
-   params->func(params, cur_pe_start, cur_dst, cur_nptes,
-AMDGPU_GPU_PAGE_SIZE, flags);
+   pe_start = amdgpu_bo_gpu_offset(pt);
+   pe_start += (addr & mask) * 8;
 
-   cur_pe_start = next_pe_start;
-   cur_nptes = nptes;
-   cur_dst = dst;
-   }
+   params->func(params, pe_start, dst, nptes,
+AMDGPU_GPU_PAGE_SIZE, flags);
 
-   /* for next ptb*/
-   addr += nptes;
dst += nptes * AMDGPU_GPU_PAGE_SIZE;
}
 
-   params->func(params, cur_pe_start, cur_dst, cur_nptes,
-AMDGPU_GPU_PAGE_SIZE, flags);
-
return 0;
 }
 
--
2.7.4

___
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 4/4] drm/amdgpu: reset fpriv vram_lost_counter

2017-05-17 Thread Zhou, David(ChunMing)
Yes, agree, it's easy to extend it as your like.
For innocent context, which would need us to find out the context of guilty 
job, which is TODO.

Regards,
David Zhou

-Original Message-
From: Marek Olšák [mailto:mar...@gmail.com] 
Sent: Wednesday, May 17, 2017 5:50 PM
To: Christian König 
Cc: Zhou, David(ChunMing) ; Michel Dänzer 
; amd-gfx mailing list 
Subject: Re: [PATCH 4/4] drm/amdgpu: reset fpriv vram_lost_counter

David,

We already have a query that returns whether a device is lost. It's called 
amdgpu_cs_query_reset_state. That should return whether a hang was caused by a 
certain context or whether the hang happened but the context is innocent. You 
can extend it to accept no context, in which case it will return either 
NO_RESET (everything is OK) or UNKNOWN_RESET (= when a hang happened but the 
caller didn't specify the context).

Marek

On Wed, May 17, 2017 at 10:56 AM, Christian König  
wrote:
> Am 17.05.2017 um 10:46 schrieb zhoucm1:
>
>
>
> On 2017年05月17日 16:40, Christian König wrote:
>
> Am 17.05.2017 um 10:01 schrieb Michel Dänzer:
>
> On 17/05/17 04:13 PM, zhoucm1 wrote:
>
> On 2017年05月17日 14:57, Michel Dänzer wrote:
>
> On 17/05/17 01:28 PM, zhoucm1 wrote:
>
> On 2017年05月17日 11:15, Michel Dänzer wrote:
>
> On 17/05/17 12:04 PM, zhoucm1 wrote:
>
> On 2017年05月17日 09:18, Michel Dänzer wrote:
>
> On 16/05/17 06:25 PM, Chunming Zhou wrote:
>
> Change-Id: I8eb6d7f558da05510e429d3bf1d48c8cec6c1977
> Signed-off-by: Chunming Zhou 
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index bca1fb5..f3e7525 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2547,6 +2547,9 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void 
> *data, struct drm_file *filp)
>  case AMDGPU_VM_OP_UNRESERVE_VMID:
>  amdgpu_vm_free_reserved_vmid(adev, &fpriv->vm, 
> AMDGPU_GFXHUB);
>  break;
> +case AMDGPU_VM_OP_RESET:
> +fpriv->vram_lost_counter =
> atomic_read(&adev->vram_lost_counter);
> +break;
>
> How do you envision the UMDs using this? I can mostly think of them 
> calling this ioctl when a context is created or destroyed. But that 
> would also allow any other remaining contexts using the same DRM file 
> descriptor to use all ioctls again. So, I think there needs to be a 
> vram_lost_counter in struct amdgpu_ctx instead of in struct 
> amdgpu_fpriv.
>
> struct amdgpu_fpriv for vram_lost_counter is proper place, especially 
> for ioctl return value.
> if you need to reset ctx one by one, we can mark all contexts of that 
> vm, and then reset by userspace.
>
> I'm not following. With vram_lost_counter in amdgpu_fpriv, if any 
> context calls this ioctl, all other contexts using the same file 
> descriptor will also be considered safe again, right?
>
> Yes, but it really depends on userspace requirement, if you need to 
> reset ctx one by one, we can mark all contexts of that vm to guilty, 
> and then reset one context by userspace.
>
> Still not sure what you mean by that.
>
> E.g. what do you mean by "guilty"? I thought that refers to the 
> context which caused a hang. But it seems like you're using it to 
> refer to any context which hasn't reacted yet to VRAM contents being lost.
>
> When vram is lost, we treat all contexts need to reset.
>
> Essentially, your patches only track VRAM contents being lost per file 
> descriptor, not per context. I'm not sure (rather skeptical) that this 
> is suitable for OpenGL UMDs, since state is usually tracked per context.
> Marek / Nicolai?
>
>
> Oh, yeah that's a good point.
>
> The problem with tracking it per context is that Vulkan also wants the 
> ENODEV on the amdgpu_gem_va_ioct() and amdgpu_info_ioctl() which are 
> context less.
>
> But thinking more about this blocking those two doesn't make much 
> sense. The VM content can be restored and why should be disallow reading GPU 
> info?
>
> I can re-paste the Vulkan APIs requiring ENODEV:
> "
>
> The Vulkan APIs listed below could return VK_ERROR_DEVICE_LOST 
> according to the spec.
>
> I tries to provide a list of u/k interfaces that could be called for 
> each vk API.
>
>
> Well those are the Vulkan requirements, but that doesn't necessary 
> mean we must follow that on the kernel side. Keep in mind that Vulkan 
> can't made any requirements towards the kernel driver.
>
> IIRC we already have a query Vulkan can use to figure out if a GPU 
> reset happened or not. So they could use that instead.
>
> Regards,
> Christian.
>
>
>
> vkCreateDevice
>
> -  amdgpu_device_initialize.
>
> -  amdgpu_query_gpu_info
>
>
>
> vkQueueSubmit
>
> -  amdgpu_cs_submit
>
>
>
> vkWaitForFences
>
> amdgpu_cs_wait_fences
>
>
>
> vkGetEventStatus
>
> vkQueueWaitIdle
>
> vkDeviceWaitIdle
>
> vkGetQueryPoolResults
>
> amdgpu_cs_query_Fence_status
>
>
>
> vkQueueBindSparse
>
> amdgpu_bo_va_op
>
>   

[PATCH] drm/amdgpu: update golden settings

2017-05-17 Thread Ken Wang
Change-Id: Ifcc39748c36273fa764cd2641d4b44405dbf59a5
Signed-off-by: Ken Wang 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 20 
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 39ae97b..de35de2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -86,14 +86,27 @@ static const struct amdgpu_gds_reg_offset 
amdgpu_gds_reg_offset[] =
 
 static const u32 golden_settings_gc_9_0[] =
 {
-   SOC15_REG_OFFSET(GC, 0, mmDB_DEBUG2), 0xf00ffeff, 0x0400,
+   SOC15_REG_OFFSET(GC, 0, mmCPC_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmCPF_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmCPG_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmDB_DEBUG2), 0xf00f, 0x0420,
+   SOC15_REG_OFFSET(GC, 0, mmGB_GPU_ID), 0x000f, 0x,
+   SOC15_REG_OFFSET(GC, 0, mmIA_UTCL1_CNTL), 0x0800, 0x0880,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_BINNER_EVENT_CNTL_3), 0x0003, 
0x82400024,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_ENHANCE), 0x3fff, 0x0001,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_LINE_STIPPLE_STATE), 0xff0f, 
0x,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UTCL1_CNTL_0), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UTCL1_CNTL_1), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UTCL1_CNTL_2), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_PREWALKER_UTCL1_CNTL), 0x0800, 
0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_SPM_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmSPI_CONFIG_CNTL_1), 0x000f, 0x01000107,
SOC15_REG_OFFSET(GC, 0, mmTA_CNTL_AUX), 0xfeef, 0x010b,
SOC15_REG_OFFSET(GC, 0, mmTCP_CHAN_STEER_HI), 0x, 0x4a2c0e68,
SOC15_REG_OFFSET(GC, 0, mmTCP_CHAN_STEER_LO), 0x, 0xb5d3f197,
-   SOC15_REG_OFFSET(GC, 0, mmVGT_GS_MAX_WAVE_ID), 0x0fff, 0x03ff
+   SOC15_REG_OFFSET(GC, 0, mmVGT_CACHE_INVALIDATION), 0x3fff3af3, 
0x1920,
+   SOC15_REG_OFFSET(GC, 0, mmVGT_GS_MAX_WAVE_ID), 0x0fff, 0x03ff,
+   SOC15_REG_OFFSET(GC, 0, mmWD_UTCL1_CNTL), 0x0800, 0x0880
 };
 
 static const u32 golden_settings_gc_9_0_vg10[] =
@@ -104,8 +117,7 @@ static const u32 golden_settings_gc_9_0_vg10[] =
SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG_READ), 0x77ff, 0x2a114042,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_ENHANCE_1), 0x8000, 0x00048000,
SOC15_REG_OFFSET(GC, 0, mmRMI_UTCL1_CNTL2), 0x0003, 0x0002,
-   SOC15_REG_OFFSET(GC, 0, mmTD_CNTL), 0x1800, 0x0800,
-   SOC15_REG_OFFSET(GC, 0, mmSPI_CONFIG_CNTL_1),0x000f, 0x0007
+   SOC15_REG_OFFSET(GC, 0, mmTD_CNTL), 0x1800, 0x0800
 };
 
 #define VEGA10_GB_ADDR_CONFIG_GOLDEN 0x2a114042
-- 
2.7.4

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


Re: Plan: BO move throttling for visible VRAM evictions

2017-05-17 Thread Marek Olšák
On May 16, 2017 3:57 AM, "Michel Dänzer"  wrote:

On 15/05/17 07:11 PM, Marek Olšák wrote:
> On May 15, 2017 4:29 AM, "Michel Dänzer"  > wrote:
>
> I think the next step should be to make radeonsi keep track of how
much
> VRAM it's trying to use that's expected to be accessed by the CPU, and
> to use GTT instead when that exceeds a threshold (probably derived
from
> vram_vis_size).
>
> That's difficult to estimate. There are apps with 600MB of mapped VRAM
> and don't experience any performance issues. And some apps with 300MB of
> mapped VRAM do. It only depends on the CPU access pattern, not what
> radeonsi sees.

What I mean is keeping track of the total size of resources which have
RADEON_DOMAIN_VRAM and RADEON_FLAG_CPU_ACCESS set, and if it exceeds a
threshold, create new ones having those flags in GTT instead. Even
though this might not be strictly necessary with amdgpu in the long run,
it probably is for radeon anyway, and in the short term it might help
even with amdgpu.


That might hurt us more than it can help. All mappable buffers have the CPU
access flag set, but many of them are immutable.

The only place where this can be handled​ is the kernel. Even if it's as
simple as: if (bo->numcpufaults > 10) domain = GTT_WC;

Marek



--
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


[PATCH] drm/amdgpu: add an INFO query for monitoring VRAM CPU page faults

2017-05-17 Thread Marek Olšák
From: Marek Olšák 

Signed-off-by: Marek Olšák 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h| 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 3 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 1 +
 include/uapi/drm/amdgpu_drm.h  | 2 ++
 4 files changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index fadeb55..251e5de 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1525,20 +1525,21 @@ struct amdgpu_device {
 
/* memory management */
struct amdgpu_mman  mman;
struct amdgpu_vram_scratch  vram_scratch;
struct amdgpu_wbwb;
atomic64_t  vram_usage;
atomic64_t  vram_vis_usage;
atomic64_t  gtt_usage;
atomic64_t  num_bytes_moved;
atomic64_t  num_evictions;
+   atomic64_t  num_vram_cpu_page_faults;
atomic_tgpu_reset_counter;
 
/* data for buffer migration throttling */
struct {
spinlock_t  lock;
s64 last_update_us;
s64 accum_us; /* accumulated microseconds */
u32 log2_max_MBps;
} mm_stats;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index d167949..81291d8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -407,20 +407,23 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
 
return copy_to_user(out, &fw_info,
min((size_t)size, sizeof(fw_info))) ? 
-EFAULT : 0;
}
case AMDGPU_INFO_NUM_BYTES_MOVED:
ui64 = atomic64_read(&adev->num_bytes_moved);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_NUM_EVICTIONS:
ui64 = atomic64_read(&adev->num_evictions);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
+   case AMDGPU_INFO_NUM_VRAM_CPU_PAGE_FAULTS:
+   ui64 = atomic64_read(&adev->num_vram_cpu_page_faults);
+   return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_VRAM_USAGE:
ui64 = atomic64_read(&adev->vram_usage);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_VIS_VRAM_USAGE:
ui64 = atomic64_read(&adev->vram_vis_usage);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_GTT_USAGE:
ui64 = atomic64_read(&adev->gtt_usage);
return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
case AMDGPU_INFO_GDS_CONFIG: {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 6bc52cc..b6da86e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -953,20 +953,21 @@ int amdgpu_bo_fault_reserve_notify(struct 
ttm_buffer_object *bo)
offset = bo->mem.start << PAGE_SHIFT;
/* TODO: figure out how to map scattered VRAM to the CPU */
if ((offset + size) <= adev->mc.visible_vram_size)
return 0;
 
/* Can't move a pinned BO to visible VRAM */
if (abo->pin_count > 0)
return -EINVAL;
 
/* hurrah the memory is not visible ! */
+   atomic64_inc(&adev->num_vram_cpu_page_faults);
amdgpu_ttm_placement_from_domain(abo, AMDGPU_GEM_DOMAIN_VRAM);
lpfn =  adev->mc.visible_vram_size >> PAGE_SHIFT;
for (i = 0; i < abo->placement.num_placement; i++) {
/* Force into visible VRAM */
if ((abo->placements[i].flags & TTM_PL_FLAG_VRAM) &&
(!abo->placements[i].lpfn ||
 abo->placements[i].lpfn > lpfn))
abo->placements[i].lpfn = lpfn;
}
r = ttm_bo_validate(bo, &abo->placement, false, false);
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h
index c99fe63..4f34394 100644
--- a/include/uapi/drm/amdgpu_drm.h
+++ b/include/uapi/drm/amdgpu_drm.h
@@ -598,20 +598,22 @@ struct drm_amdgpu_cs_chunk_data {
/* Subquery id: Query GPU temperature */
#define AMDGPU_INFO_SENSOR_GPU_TEMP 0x3
/* Subquery id: Query GPU load */
#define AMDGPU_INFO_SENSOR_GPU_LOAD 0x4
/* Subquery id: Query average GPU power */
#define AMDGPU_INFO_SENSOR_GPU_AVG_POWER0x5
/* Subquery id: Query northbridge voltage */
#define AMDGPU_INFO_SENSOR_VDDNB0x6
/* Sub

Re: [PATCH] drm/amd/powerplay: ensure loop does not wraparound on decrement

2017-05-17 Thread Dan Carpenter
I sent a patch for this bug earlier.  There is a second bug later in the
function which my patch fixes as well.

regards,
dan carpenter

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


[REGRESSION] drm/radeon: Don't register Thunderbolt eGPU with vga_switcheroo

2017-05-17 Thread Nicolai Stange
Hi,

I'm experiencing a boot failure on next-20170515:

  BUG: unable to handle kernel NULL pointer dereference at 07cb
  IP: radeon_driver_load_kms+0xeb/0x230 [radeon]
  PGD 0 
  P4D 0 
  
  Oops:  [#1] SMP
  Modules linked in: amdkfd amd_iommu_v2 i915(+) radeon(+) i2c_algo_bit 
drm_kms_helper ttm e1000e drm sdhci_pci sdhci_acpi ptp sdhci crc32c_intel 
serio_raw mmc_core pps_core video i2c_hid hid_plantronics
  CPU: 4 PID: 389 Comm: systemd-udevd Not tainted 4.12.0-rc1-next-20170515+ #1
  Hardware name: Dell Inc. Latitude E6540/0725FP, BIOS A10 06/26/2014
  task: 97d62c8f task.stack: b96f01478000
  RIP: 0010:radeon_driver_load_kms+0xeb/0x230 [radeon]
  RSP: 0018:b96f0147b9d0 EFLAGS: 00010246
  RAX:  RBX: 97d620085000 RCX: 00610037
  RDX:  RSI: 002b RDI: 
  RBP: b96f0147b9e8 R08: 0002 R09: b96f0147b924
  R10:  R11: 97d62edd2ec0 R12: 97d628d5c000
  R13: 00610037 R14: c0698280 R15: 
  FS:  7f496363d8c0() GS:97d62eb0() knlGS:
  CS:  0010 DS:  ES:  CR0: 80050033
  CR2: 07cb CR3: 00022c14c000 CR4: 001406e0
  Call Trace:
   drm_dev_register+0x146/0x1d0 [drm]
   drm_get_pci_dev+0x9a/0x180 [drm]
   radeon_pci_probe+0xb8/0xe0 [radeon]
   local_pci_probe+0x45/0xa0
   pci_device_probe+0x14f/0x1a0
   driver_probe_device+0x29c/0x450
   __driver_attach+0xdf/0xf0
   ? driver_probe_device+0x450/0x450
   bus_for_each_dev+0x6c/0xc0
   driver_attach+0x1e/0x20
   bus_add_driver+0x170/0x270
   driver_register+0x60/0xe0
   ? 0xc0508000
   __pci_register_driver+0x4c/0x50
   drm_pci_init+0xeb/0x100 [drm]
   ? vga_switcheroo_register_handler+0x6a/0x90
   ? 0xc0508000
   radeon_init+0x98/0xb6 [radeon]
   do_one_initcall+0x52/0x1a0
   ? __vunmap+0x81/0xb0
   ? kmem_cache_alloc_trace+0x159/0x1b0
   ? do_init_module+0x27/0x1f8
   do_init_module+0x5f/0x1f8
   load_module+0x27ce/0x2be0
   SYSC_finit_module+0xdf/0x110
   ? SYSC_finit_module+0xdf/0x110
   SyS_finit_module+0xe/0x10
   do_syscall_64+0x67/0x150
   entry_SYSCALL64_slow_path+0x25/0x25
  RIP: 0033:0x7f4962295679
  RSP: 002b:7ffdd8c4f878 EFLAGS: 0246 ORIG_RAX: 0139
  RAX: ffda RBX: 55c014ed8200 RCX: 7f4962295679
  RDX:  RSI: 7f4962dd19c5 RDI: 0010
  RBP: 7f4962dd19c5 R08:  R09: 7ffdd8c4f990
  R10: 0010 R11: 0246 R12: 
  R13: 55c014ed81a0 R14: 0002 R15: 55c0149d1fca
  Code: 5d 5d c3 8b 05 a7 05 14 00 49 81 cd 00 00 08 00 85 c0 74 a3 e8 e7 c0 0e 
00 84 c0 74 9a 41 f7 c5 00 00 02 00 75 91 49 8b 44 24 10 <0f> b6 90 cb 07 00 00 
f6 c2 20 74 1e e9 7b ff ff ff 48 8b 40 38 
  RIP: radeon_driver_load_kms+0xeb/0x230 [radeon] RSP: b96f0147b9d0
  CR2: 07cb
  ---[ end trace 89cc4ba7e569c65c ]---


Bisection lead to commit 7ffb0ce31cf9 ("drm/radeon: Don't register
Thunderbolt eGPU with vga_switcheroo"). Reverting this commit on top of
next-20170515 fixes the issue for me.

My box is a Dell laptop which most certainly hasn't got any Thunderbolt
circuitry.

lspci says that my radeon card is a
  01:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] 
Mars XTX [Radeon HD 8790M] (rev ff) (prog-if ff)

or in raw form:

  01:00.0 0300: 1002:6606 (rev ff)


Thanks,

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


[PATCH] gpu: drm: radeon: refactor code

2017-05-17 Thread Gustavo A. R. Silva
Local variable _color_ is assigned to a constant value and it is
never updated again. Remove this variable and refactor the code it
affects.

Addresses-Coverity-ID: 1226745
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/gpu/drm/radeon/radeon_legacy_encoders.c | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c 
b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
index 222a1fa..7235d0c 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_encoders.c
@@ -640,7 +640,6 @@ static enum drm_connector_status 
radeon_legacy_primary_dac_detect(struct drm_enc
uint32_t vclk_ecp_cntl, crtc_ext_cntl;
uint32_t dac_ext_cntl, dac_cntl, dac_macro_cntl, tmp;
enum drm_connector_status found = connector_status_disconnected;
-   bool color = true;
 
/* just don't bother on RN50 those chip are often connected to remoting
 * console hw and often we get failure to load detect those. So to make
@@ -665,12 +664,7 @@ static enum drm_connector_status 
radeon_legacy_primary_dac_detect(struct drm_enc
WREG32(RADEON_CRTC_EXT_CNTL, tmp);
 
tmp = RADEON_DAC_FORCE_BLANK_OFF_EN |
-   RADEON_DAC_FORCE_DATA_EN;
-
-   if (color)
-   tmp |= RADEON_DAC_FORCE_DATA_SEL_RGB;
-   else
-   tmp |= RADEON_DAC_FORCE_DATA_SEL_G;
+   RADEON_DAC_FORCE_DATA_EN | RADEON_DAC_FORCE_DATA_SEL_RGB;
 
if (ASIC_IS_R300(rdev))
tmp |= (0x1b6 << RADEON_DAC_FORCE_DATA_SHIFT);
-- 
2.5.0

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


[PATCH] drm/amd/powerplay: ensure loop does not wraparound on decrement

2017-05-17 Thread Colin King
From: Colin Ian King 

The current for loop decrements i when it is zero and this causes
a wrap-around back to ~0 because i is unsigned. In the unlikely event
that mask is 0, the loop will run forever. Fix this so we can't loop
forever.

Detected by CoverityScan, CID#1435469 ("Unsigned compared against 0")

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index ad30f5d3a10d..d92c9b9b15be 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -4199,7 +4199,7 @@ static int vega10_force_clock_level(struct pp_hwmgr 
*hwmgr,
}
data->smc_state_table.gfx_boot_level = i;
 
-   for (i = 31; i >= 0; i--) {
+   for (i = 32; --i; ) {
if (mask & (1 << i))
break;
}
-- 
2.11.0

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


Re: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors

2017-05-17 Thread Bjorn Helgaas
On Tue, Apr 25, 2017 at 03:01:35PM +0200, Christian König wrote:
> Am 12.04.2017 um 18:55 schrieb Bjorn Helgaas:
> >[SNIP]
> >>>I think the specs would envision this being done via an ACPI _SRS
> >>>method on the PNP0A03 host bridge device.  That would be a more
> >>>generic path that would work on any host bridge.  Did you explore that
> >>>possibility?  I would prefer to avoid adding device-specific code if
> >>>that's possible.
> >>I've checked quite a few boards, but none of them actually
> >>implements it this way.
> >>
> >>M$ is working on a new ACPI table to enable this vendor neutral, but
> >>I guess that will still take a while.
> >>
> >>I want to support this for all AMD CPU released in the past 5 years
> >>or so, so we are going to deal with a bunch of older boards as well.
> >I've never seen _SRS for host bridges either.  I'm curious about what
> >sort of new table will be proposed.  It seems like the existing ACPI
> >resource framework could manage it, but I certainly don't know all the
> >issues.
> 
> No idea either since I'm not involved into that. My job is to get it
> working on the existing hw generations and that alone is enough work
> :)
> 
> My best guess is that MS is going to either make _SRS on the host
> bridge or a pre-configured 64bit window mandatory for the BIOS.

While researching something else, I noticed that the PCI Firmware Spec, rev
3.2, does indeed call out _PRS and _SRS as the mechanism for the OS to
configure host bridge windows.  See sections 4.1.3, 4.3.2.1, and 4.6.5.

Sec 4.6.5 also includes an implementation note that might be a clue about
the "compatibility issues" that prevent the BIOS from enabling the window
in the first place.

I'd like to incorporate some of this info into these changes, probably in a
code comment and changelog, so we can encourage a more generic approach in
the future, even if we can't use it in all existing cases.

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


Re: [PATCH] drm/amdgpu: update golden settings

2017-05-17 Thread zhoucm1

Acked-by: Chunming Zhou 

On 2017年05月17日 15:46, Ken Wang wrote:

Change-Id: Ifcc39748c36273fa764cd2641d4b44405dbf59a5
Signed-off-by: Ken Wang 
---
  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 20 
  1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 39ae97b..de35de2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -86,14 +86,27 @@ static const struct amdgpu_gds_reg_offset 
amdgpu_gds_reg_offset[] =
  
  static const u32 golden_settings_gc_9_0[] =

  {
-   SOC15_REG_OFFSET(GC, 0, mmDB_DEBUG2), 0xf00ffeff, 0x0400,
+   SOC15_REG_OFFSET(GC, 0, mmCPC_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmCPF_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmCPG_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmDB_DEBUG2), 0xf00f, 0x0420,
+   SOC15_REG_OFFSET(GC, 0, mmGB_GPU_ID), 0x000f, 0x,
+   SOC15_REG_OFFSET(GC, 0, mmIA_UTCL1_CNTL), 0x0800, 0x0880,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_BINNER_EVENT_CNTL_3), 0x0003, 
0x82400024,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_ENHANCE), 0x3fff, 0x0001,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_LINE_STIPPLE_STATE), 0xff0f, 
0x,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UTCL1_CNTL_0), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UTCL1_CNTL_1), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_GPM_UTCL1_CNTL_2), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_PREWALKER_UTCL1_CNTL), 0x0800, 
0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmRLC_SPM_UTCL1_CNTL), 0x0800, 0x0880,
+   SOC15_REG_OFFSET(GC, 0, mmSPI_CONFIG_CNTL_1), 0x000f, 0x01000107,
SOC15_REG_OFFSET(GC, 0, mmTA_CNTL_AUX), 0xfeef, 0x010b,
SOC15_REG_OFFSET(GC, 0, mmTCP_CHAN_STEER_HI), 0x, 0x4a2c0e68,
SOC15_REG_OFFSET(GC, 0, mmTCP_CHAN_STEER_LO), 0x, 0xb5d3f197,
-   SOC15_REG_OFFSET(GC, 0, mmVGT_GS_MAX_WAVE_ID), 0x0fff, 0x03ff
+   SOC15_REG_OFFSET(GC, 0, mmVGT_CACHE_INVALIDATION), 0x3fff3af3, 
0x1920,
+   SOC15_REG_OFFSET(GC, 0, mmVGT_GS_MAX_WAVE_ID), 0x0fff, 0x03ff,
+   SOC15_REG_OFFSET(GC, 0, mmWD_UTCL1_CNTL), 0x0800, 0x0880
  };
  
  static const u32 golden_settings_gc_9_0_vg10[] =

@@ -104,8 +117,7 @@ static const u32 golden_settings_gc_9_0_vg10[] =
SOC15_REG_OFFSET(GC, 0, mmGB_ADDR_CONFIG_READ), 0x77ff, 0x2a114042,
SOC15_REG_OFFSET(GC, 0, mmPA_SC_ENHANCE_1), 0x8000, 0x00048000,
SOC15_REG_OFFSET(GC, 0, mmRMI_UTCL1_CNTL2), 0x0003, 0x0002,
-   SOC15_REG_OFFSET(GC, 0, mmTD_CNTL), 0x1800, 0x0800,
-   SOC15_REG_OFFSET(GC, 0, mmSPI_CONFIG_CNTL_1),0x000f, 0x0007
+   SOC15_REG_OFFSET(GC, 0, mmTD_CNTL), 0x1800, 0x0800
  };
  
  #define VEGA10_GB_ADDR_CONFIG_GOLDEN 0x2a114042


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


[PATCH xf86-video-ati] Update URLs

2017-05-17 Thread Michel Dänzer
From: Michel Dänzer 

* Point to the amd-gfx mailing list
* Specify the component in all bugzilla URLs
* Use https:// for all HTML URLs

Signed-off-by: Michel Dänzer 
---
 README | 14 +++---
 configure.ac   |  2 +-
 man/radeon.man |  6 +++---
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/README b/README
index 4b92a1858..db982fbed 100644
--- a/README
+++ b/README
@@ -1,25 +1,25 @@
-xf86-video-ati - ATI Radeon video driver for the Xorg X server
+xf86-video-ati - ATI/AMD Radeon video driver for the Xorg X server
 
 All questions regarding this software should be directed at the
-Xorg mailing list:
+amd-gfx mailing list:
 
-http://lists.freedesktop.org/mailman/listinfo/xorg
+https://lists.freedesktop.org/mailman/listinfo/amd-gfx
 
 Please submit bug reports to the Xorg bugzilla:
 
-https://bugs.freedesktop.org/enter_bug.cgi?product=xorg
+
https://bugs.freedesktop.org/enter_bug.cgi?product=xorg&component=Driver/Radeon
 
 The master development code repository can be found at:
 
 git://anongit.freedesktop.org/git/xorg/driver/xf86-video-ati
 
-http://cgit.freedesktop.org/xorg/driver/xf86-video-ati
+https://cgit.freedesktop.org/xorg/driver/xf86-video-ati
 
 For patch submission instructions, see:
 
-   http://www.x.org/wiki/Development/Documentation/SubmittingPatches
+   https://www.x.org/wiki/Development/Documentation/SubmittingPatches
 
 For more information on the git code manager, see:
 
-http://wiki.x.org/wiki/GitPage
+https://wiki.x.org/wiki/GitPage
 
diff --git a/configure.ac b/configure.ac
index c9ccfb3dc..700e01a5a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -24,7 +24,7 @@
 AC_PREREQ([2.60])
 AC_INIT([xf86-video-ati],
 [7.9.99],
-[https://bugs.freedesktop.org/enter_bug.cgi?product=xorg],
+
[https://bugs.freedesktop.org/enter_bug.cgi?product=xorg&component=Driver/Radeon],
 [xf86-video-ati])
 
 AC_CONFIG_SRCDIR([Makefile.am])
diff --git a/man/radeon.man b/man/radeon.man
index 9334d9e86..3e1723f21 100644
--- a/man/radeon.man
+++ b/man/radeon.man
@@ -392,17 +392,17 @@ __xservername__(__appmansuffix__), 
__xconfigfile__(__filemansuffix__), Xserver(_
 .IP " 1." 4
 Wiki page:
 .RS 4
-http://www.x.org/wiki/radeon
+https://www.x.org/wiki/radeon
 .RE
 .IP " 2." 4
 Overview about radeon development code:
 .RS 4
-http://cgit.freedesktop.org/xorg/driver/xf86-video-ati/
+https://cgit.freedesktop.org/xorg/driver/xf86-video-ati/
 .RE
 .IP " 3." 4
 Mailing list:
 .RS 4
-http://lists.x.org/mailman/listinfo/xorg-driver-ati
+https://lists.freedesktop.org/mailman/listinfo/amd-gfx
 .RE
 .IP " 4." 4
 IRC channel:
-- 
2.11.0

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


Re: [PATCH 7/7] drm/amdgpu: enable huge page handling in the VM

2017-05-17 Thread zhoucm1



On 2017年05月17日 17:22, Christian König wrote:

From: Christian König 

The hardware can use huge pages to map 2MB of address space with only one PDE.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 96 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |  4 ++
  2 files changed, 76 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 860a669..8be1d7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -325,6 +325,7 @@ static int amdgpu_vm_alloc_levels(struct amdgpu_device 
*adev,
  
  			entry->bo = pt;

entry->addr = 0;
+   entry->huge_page = false;
}
  
  		if (level < adev->vm_manager.num_level) {

@@ -1014,7 +1015,8 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
  
  		pt = bo->tbo.mem.start << PAGE_SHIFT;

pt = amdgpu_gart_get_vm_pde(adev, pt);
-   if (parent->entries[pt_idx].addr == pt)
+   if (parent->entries[pt_idx].addr == pt ||
+   parent->entries[pt_idx].huge_page)
continue;
  
  		parent->entries[pt_idx].addr = pt;

@@ -1146,29 +1148,70 @@ int amdgpu_vm_update_directories(struct amdgpu_device 
*adev,
  }
  
  /**

- * amdgpu_vm_find_pt - find the page table for an address
+ * amdgpu_vm_handle_huge_pages - handle updating the PD with huge pages
   *
   * @p: see amdgpu_pte_update_params definition
   * @addr: virtual address in question
+ * @nptes: number of PTEs updated with this operation
+ * @dst: destination address where the PTEs should point to
+ * @flags: access flags fro the PTEs
+ * @bo: resulting page tables BO
   *
- * Find the page table BO for a virtual address, return NULL when none found.
+ * Check if we can update the PD with a huge page. Also finds the page table
+ * BO for a virtual address, returns -ENOENT when nothing found.
   */
-static struct amdgpu_bo *amdgpu_vm_get_pt(struct amdgpu_pte_update_params *p,
- uint64_t addr)
+static int amdgpu_vm_handle_huge_pages(struct amdgpu_pte_update_params *p,
+  uint64_t addr, unsigned nptes,
+  uint64_t dst, uint64_t flags,
+  struct amdgpu_bo **bo)
  {
-   struct amdgpu_vm_pt *entry = &p->vm->root;
-   unsigned idx, level = p->adev->vm_manager.num_level;
+   unsigned pt_idx, level = p->adev->vm_manager.num_level;
+   struct amdgpu_vm_pt *entry = &p->vm->root, *parent;
+   uint64_t pd_addr, pde, pt;
  
-	while (entry->entries) {

-   idx = addr >> (p->adev->vm_manager.block_size * level--);
-   idx %= amdgpu_bo_size(entry->bo) / 8;
-   entry = &entry->entries[idx];
-   }
+   do {
+   pt_idx = addr >> (p->adev->vm_manager.block_size * level--);
+   pt_idx %= amdgpu_bo_size(entry->bo) / 8;
+   parent = entry;
+   entry = &entry->entries[pt_idx];
+   } while (entry->entries);
  
  	if (level)

-   return NULL;
+   return -ENOENT;
+
+   *bo = entry->bo;
+
+   /* In the case of a mixed PT the PDE must point to it*/
+   if (p->adev->asic_type < CHIP_VEGA10 ||
+   nptes != AMDGPU_VM_PTE_COUNT(p->adev) ||
+   p->func != amdgpu_vm_do_set_ptes ||
+   !(flags & AMDGPU_PTE_VALID)) {
+
+   pt = (*bo)->tbo.mem.start << PAGE_SHIFT;
+   pt = amdgpu_gart_get_vm_pde(p->adev, pt);
+   flags = AMDGPU_PTE_VALID;

This case should be handled when updating levels, so return directly?

+   } else {
+   pt = amdgpu_gart_get_vm_pde(p->adev, dst);
+   flags |= AMDGPU_PDE_PTE;
+   }
  
-	return entry->bo;

+   if (entry->addr == pt &&
+   entry->huge_page == !!(flags & AMDGPU_PDE_PTE))
+   return 0;
+
+   entry->addr = pt;
+   entry->huge_page = !!(flags & AMDGPU_PDE_PTE);
+
+   if (parent->bo->shadow) {
+   pd_addr = amdgpu_bo_gpu_offset(parent->bo->shadow);
+   pde = pd_addr + pt_idx * 8;
+   amdgpu_vm_do_set_ptes(p, pde, pt, 1, 0, flags);
From the spec "any pde in the chain can itself take on the format of a 
PTE and point directly to an aligned block of logical address space by 
setting the P bit.",

So here should pass addr into PDE instead of pt.

+   }
+
+   pd_addr = amdgpu_bo_gpu_offset(parent->bo);
+   pde = pd_addr + pt_idx * 8;
+   amdgpu_vm_do_set_ptes(p, pde, pt, 1, 0, flags);

Should pass addr into PDE instead of pt as well.



+   return 0;
  }
  
  /**

@@ -1194,14 +1237,20 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
uint64_t addr, pe_start;
struct amdgpu_bo *pt;
unsigned npte

Re: [PATCH 1/7] drm/amdgpu: cleanup adjust_mc_addr handling v2

2017-05-17 Thread Zhang, Jerry (Junwei)

On 05/17/2017 05:22 PM, Christian König wrote:

From: Christian König 

Rename adjust_mc_addr to get_vm_pde and check the address bits in one place.

v2: handle vcn as well, keep setting the valid bit manually,
 add a BUG_ON() for GMC v6, v7 and v8 as well.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h|  5 +++--
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 26 +-
  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  |  6 ++
  drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c  |  7 +++
  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  |  9 -
  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  |  9 -
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  | 10 ++
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c |  6 ++
  drivers/gpu/drm/amd/amdgpu/uvd_v7_0.c  | 12 
  drivers/gpu/drm/amd/amdgpu/vce_v4_0.c  |  6 ++
  drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c  |  5 ++---
  11 files changed, 53 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index ae7e775..4acbca7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -309,8 +309,8 @@ struct amdgpu_gart_funcs {
/* set pte flags based per asic */
uint64_t (*get_vm_pte_flags)(struct amdgpu_device *adev,
 uint32_t flags);
-   /* adjust mc addr in fb for APU case */
-   u64 (*adjust_mc_addr)(struct amdgpu_device *adev, u64 addr);
+   /* get the pde for a given mc addr */
+   u64 (*get_vm_pde)(struct amdgpu_device *adev, u64 addr);
uint32_t (*get_invalidate_req)(unsigned int vm_id);
  };

@@ -1820,6 +1820,7 @@ amdgpu_get_sdma_instance(struct amdgpu_ring *ring)
  #define amdgpu_asic_get_config_memsize(adev) 
(adev)->asic_funcs->get_config_memsize((adev))
  #define amdgpu_gart_flush_gpu_tlb(adev, vmid) 
(adev)->gart.gart_funcs->flush_gpu_tlb((adev), (vmid))
  #define amdgpu_gart_set_pte_pde(adev, pt, idx, addr, flags) 
(adev)->gart.gart_funcs->set_pte_pde((adev), (pt), (idx), (addr), (flags))
+#define amdgpu_gart_get_vm_pde(adev, addr) 
(adev)->gart.gart_funcs->get_vm_pde((adev), (addr))
  #define amdgpu_vm_copy_pte(adev, ib, pe, src, count) 
((adev)->vm_manager.vm_pte_funcs->copy_pte((ib), (pe), (src), (count)))
  #define amdgpu_vm_write_pte(adev, ib, pe, value, count, incr) 
((adev)->vm_manager.vm_pte_funcs->write_pte((ib), (pe), (value), (count), 
(incr)))
  #define amdgpu_vm_set_pte_pde(adev, ib, pe, addr, count, incr, flags) 
((adev)->vm_manager.vm_pte_funcs->set_pte_pde((ib), (pe), (addr), (count), 
(incr), (flags)))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 88420dc..344f943 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -682,16 +682,6 @@ static bool amdgpu_vm_ring_has_compute_vm_bug(struct 
amdgpu_ring *ring)
return false;
  }

-static u64 amdgpu_vm_adjust_mc_addr(struct amdgpu_device *adev, u64 mc_addr)
-{
-   u64 addr = mc_addr;
-
-   if (adev->gart.gart_funcs->adjust_mc_addr)
-   addr = adev->gart.gart_funcs->adjust_mc_addr(adev, addr);
-
-   return addr;
-}
-
  bool amdgpu_vm_need_pipeline_sync(struct amdgpu_ring *ring,
  struct amdgpu_job *job)
  {
@@ -1034,18 +1024,18 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
(count == AMDGPU_VM_MAX_UPDATE_SIZE)) {

if (count) {
-   uint64_t pt_addr =
-   amdgpu_vm_adjust_mc_addr(adev, last_pt);
+   uint64_t entry;

+   entry = amdgpu_gart_get_vm_pde(adev, last_pt);
if (shadow)
amdgpu_vm_do_set_ptes(¶ms,
  last_shadow,
- pt_addr, count,
+ entry, count,
  incr,
  AMDGPU_PTE_VALID);

amdgpu_vm_do_set_ptes(¶ms, last_pde,
- pt_addr, count, incr,
+ entry, count, incr,
  AMDGPU_PTE_VALID);
}

@@ -1059,13 +1049,15 @@ static int amdgpu_vm_update_level(struct amdgpu_device 
*adev,
}

if (count) {
-   uint64_t pt_addr = amdgpu_vm_adjust_mc_addr(adev, last_pt);
+   uint64_t entry;
+
+   entry = amdgpu_gart_get_vm_pde(adev, last_pt);

if (vm->root.bo->shadow)
-   amdgpu_vm_do_set_ptes(¶ms, last_sha

Re: [PATCH 6/7] drm/amdgpu: stop joining VM PTE updates

2017-05-17 Thread Zhang, Jerry (Junwei)

On 05/17/2017 06:08 PM, Zhou, David(ChunMing) wrote:


Patch#2: Reviewed-by: Chunming Zhou 
Patch#3: RB should be from Hawking, so Acked-by: Chunming Zhou 

Patch#5 #6, are Reviewed-by: Chunming Zhou 


Feel free to add my RB about Patch #2 ~ #5

Reviewed-by: Junwei Zhang 

Jerry



For patch#7, further transfer +1 page table, I need a bit time of tomorrow.

Regards,
David Zhou

-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Christian K?nig
Sent: Wednesday, May 17, 2017 5:23 PM
To: amd-gfx@lists.freedesktop.org
Subject: [PATCH 6/7] drm/amdgpu: stop joining VM PTE updates

From: Christian König 

This isn't beneficial any more since VRAM allocations are now split so that 
they fits into a single page table.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 61 --
  1 file changed, 7 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 6a926f4..860a669 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1191,41 +1191,12 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_pte_update_params *params,
struct amdgpu_device *adev = params->adev;
const uint64_t mask = AMDGPU_VM_PTE_COUNT(adev) - 1;

-   uint64_t cur_pe_start, cur_nptes, cur_dst;
-   uint64_t addr; /* next GPU address to be updated */
+   uint64_t addr, pe_start;
struct amdgpu_bo *pt;
-   unsigned nptes; /* next number of ptes to be updated */
-   uint64_t next_pe_start;
-
-   /* initialize the variables */
-   addr = start;
-   pt = amdgpu_vm_get_pt(params, addr);
-   if (!pt) {
-   pr_err("PT not found, aborting update_ptes\n");
-   return -EINVAL;
-   }
-
-   if (params->shadow) {
-   if (!pt->shadow)
-   return 0;
-   pt = pt->shadow;
-   }
-   if ((addr & ~mask) == (end & ~mask))
-   nptes = end - addr;
-   else
-   nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);
-
-   cur_pe_start = amdgpu_bo_gpu_offset(pt);
-   cur_pe_start += (addr & mask) * 8;
-   cur_nptes = nptes;
-   cur_dst = dst;
-
-   /* for next ptb*/
-   addr += nptes;
-   dst += nptes * AMDGPU_GPU_PAGE_SIZE;
+   unsigned nptes;

/* walk over the address space and update the page tables */
-   while (addr < end) {
+   for (addr = start; addr < end; addr += nptes) {
pt = amdgpu_vm_get_pt(params, addr);
if (!pt) {
pr_err("PT not found, aborting update_ptes\n"); @@ 
-1243,33 +1214,15 @@ static int amdgpu_vm_update_ptes(struct amdgpu_pte_update_params 
*params,
else
nptes = AMDGPU_VM_PTE_COUNT(adev) - (addr & mask);

-   next_pe_start = amdgpu_bo_gpu_offset(pt);
-   next_pe_start += (addr & mask) * 8;
-
-   if ((cur_pe_start + 8 * cur_nptes) == next_pe_start &&
-   ((cur_nptes + nptes) <= AMDGPU_VM_MAX_UPDATE_SIZE)) {
-   /* The next ptb is consecutive to current ptb.
-* Don't call the update function now.
-* Will update two ptbs together in future.
-   */
-   cur_nptes += nptes;
-   } else {
-   params->func(params, cur_pe_start, cur_dst, cur_nptes,
-AMDGPU_GPU_PAGE_SIZE, flags);
+   pe_start = amdgpu_bo_gpu_offset(pt);
+   pe_start += (addr & mask) * 8;

-   cur_pe_start = next_pe_start;
-   cur_nptes = nptes;
-   cur_dst = dst;
-   }
+   params->func(params, pe_start, dst, nptes,
+AMDGPU_GPU_PAGE_SIZE, flags);

-   /* for next ptb*/
-   addr += nptes;
dst += nptes * AMDGPU_GPU_PAGE_SIZE;
}

-   params->func(params, cur_pe_start, cur_dst, cur_nptes,
-AMDGPU_GPU_PAGE_SIZE, flags);
-
return 0;
  }

--
2.7.4

___
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


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