Re: [PATCH v2] drm/amdgpu: Correct the irq types' num of sdma

[Christian König]

Am 28.03.19 um 06:51 schrieb Emily Deng:

Fix the issue about TDR-2 will have "fallback timer expired on ring sdma1".
It is because the wrong number of irq types setting.

Signed-off-by: Emily Deng 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h |  7 ++-
  drivers/gpu/drm/amd/amdgpu/cik_sdma.c|  8 
  drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c   |  8 
  drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c   |  8 
  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c   | 25 -
  drivers/gpu/drm/amd/amdgpu/si_dma.c  |  8 
  6 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
index c17af30..1ba9ba3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sdma.h
@@ -28,11 +28,8 @@
  #define AMDGPU_MAX_SDMA_INSTANCES 2
  
  enum amdgpu_sdma_irq {

-   AMDGPU_SDMA_IRQ_TRAP0 = 0,
-   AMDGPU_SDMA_IRQ_TRAP1,
-   AMDGPU_SDMA_IRQ_ECC0,
-   AMDGPU_SDMA_IRQ_ECC1,
-
+   AMDGPU_SDMA_IRQ_INSTANCE0  = 0,
+   AMDGPU_SDMA_IRQ_INSTANCE1,
AMDGPU_SDMA_IRQ_LAST
  };
  
diff --git a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c

index 189599b..d42808b 100644
--- a/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/cik_sdma.c
@@ -977,8 +977,8 @@ static int cik_sdma_sw_init(void *handle)
r = amdgpu_ring_init(adev, ring, 1024,
 &adev->sdma.trap_irq,
 (i == 0) ?
-AMDGPU_SDMA_IRQ_TRAP0 :
-AMDGPU_SDMA_IRQ_TRAP1);
+AMDGPU_SDMA_IRQ_INSTANCE0 :
+AMDGPU_SDMA_IRQ_INSTANCE1);
if (r)
return r;
}
@@ -1114,7 +1114,7 @@ static int cik_sdma_set_trap_irq_state(struct 
amdgpu_device *adev,
u32 sdma_cntl;
  
  	switch (type) {

-   case AMDGPU_SDMA_IRQ_TRAP0:
+   case AMDGPU_SDMA_IRQ_INSTANCE0:
switch (state) {
case AMDGPU_IRQ_STATE_DISABLE:
sdma_cntl = RREG32(mmSDMA0_CNTL + 
SDMA0_REGISTER_OFFSET);
@@ -1130,7 +1130,7 @@ static int cik_sdma_set_trap_irq_state(struct 
amdgpu_device *adev,
break;
}
break;
-   case AMDGPU_SDMA_IRQ_TRAP1:
+   case AMDGPU_SDMA_IRQ_INSTANCE1:
switch (state) {
case AMDGPU_IRQ_STATE_DISABLE:
sdma_cntl = RREG32(mmSDMA0_CNTL + 
SDMA1_REGISTER_OFFSET);
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
index cca3552..3619637 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v2_4.c
@@ -870,8 +870,8 @@ static int sdma_v2_4_sw_init(void *handle)
r = amdgpu_ring_init(adev, ring, 1024,
 &adev->sdma.trap_irq,
 (i == 0) ?
-AMDGPU_SDMA_IRQ_TRAP0 :
-AMDGPU_SDMA_IRQ_TRAP1);
+AMDGPU_SDMA_IRQ_INSTANCE0 :
+AMDGPU_SDMA_IRQ_INSTANCE1);
if (r)
return r;
}
@@ -1006,7 +1006,7 @@ static int sdma_v2_4_set_trap_irq_state(struct 
amdgpu_device *adev,
u32 sdma_cntl;
  
  	switch (type) {

-   case AMDGPU_SDMA_IRQ_TRAP0:
+   case AMDGPU_SDMA_IRQ_INSTANCE0:
switch (state) {
case AMDGPU_IRQ_STATE_DISABLE:
sdma_cntl = RREG32(mmSDMA0_CNTL + 
SDMA0_REGISTER_OFFSET);
@@ -1022,7 +1022,7 @@ static int sdma_v2_4_set_trap_irq_state(struct 
amdgpu_device *adev,
break;
}
break;
-   case AMDGPU_SDMA_IRQ_TRAP1:
+   case AMDGPU_SDMA_IRQ_INSTANCE1:
switch (state) {
case AMDGPU_IRQ_STATE_DISABLE:
sdma_cntl = RREG32(mmSDMA0_CNTL + 
SDMA1_REGISTER_OFFSET);
diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
index 0ce8331..6d39544 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
@@ -1154,8 +1154,8 @@ static int sdma_v3_0_sw_init(void *handle)
r = amdgpu_ring_init(adev, ring, 1024,
 &adev->sdma.trap_irq,
 (i == 0) ?
-AMDGPU_SDMA_IRQ_TRAP0 :
-AMDGPU_SDMA_IRQ_TRAP1);
+AMDGPU_SDMA_IRQ_INSTANCE0 :
+AMDGPU_SDMA_IRQ_INSTANCE1);
if (r)

Re: [PATCH 2/2] drm/amdgpu: Allow switching to CUSTOM profile on smu7 v2

[Russell, Kent]

Hi Evan,

I set tmp=smu7_profiling[PP_SMC_PROFILE_CUSTOM] before checking the bupdate 
parameters (see below). That way I don't have ugly 100-character-long checks in 
the if size==0 check. Then if its invalid I just return -EINVAL, or if 
it'svalid then we use it to writeto the SMU. If parameters are passed in, tmp 
is overwritten with the passed-in values and we use it as before.

Kent

KENT RUSSELL
Sr. Software Engineer | Linux Compute Kernel
1 Commerce Valley Drive East
Markham, ON L3T 7X6
O +(1) 289-695-2122 | Ext 72122

From: Quan, Evan
Sent: Wednesday, March 27, 2019 9:26:41 PM
To: Russell, Kent; amd-gfx@lists.freedesktop.org
Cc: Russell, Kent
Subject: RE: [PATCH 2/2] drm/amdgpu: Allow switching to CUSTOM profile on smu7 
v2

I think you should use 
smu7_profiling[PP_SMC_POWER_PROFILE_CUSTOM].bupdate_sclk/bupdate_mclk to judge 
whether there is saved custom profile.
That's where previous custom profile settings store in.
'tmp' is a temporary structures and you should not rely on that.

Regards,
Evan
> -Original Message-
> From: amd-gfx  On Behalf Of
> Russell, Kent
> Sent: Wednesday, March 27, 2019 9:59 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Russell, Kent 
> Subject: [PATCH 2/2] drm/amdgpu: Allow switching to CUSTOM profile on
> smu7 v2
>
> Allow changing to the CUSTOM profile without requiring the parameters
> being passed in each time. Store the values in the smu7_profiling table since
> it's defined here anyways
>
> v2: Add check that CUSTOM was previously set
>
> Change-Id: I6c5e3a1487e12410a6a7670a5cf1a6599253344d
> Signed-off-by: Kent Russell 
> ---
>  .../gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c  | 32 --
> -
>  1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index 83d3d935f3ac..048757e8f494 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -77,7 +77,7 @@
>  #define PCIE_BUS_CLK1
>  #define TCLK(PCIE_BUS_CLK / 10)
>
> -static const struct profile_mode_setting smu7_profiling[7] =
> +static struct profile_mode_setting smu7_profiling[7] =
>{{0, 0, 0, 0, 0, 0, 0, 0},
> {1, 0, 100, 30, 1, 0, 100, 10},
> {1, 10, 0, 30, 0, 0, 0, 0},
> @@ -4984,17 +4984,27 @@ static int smu7_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, uint
>mode = input[size];
>switch (mode) {
>case PP_SMC_POWER_PROFILE_CUSTOM:
> - if (size < 8)
> + if (size < 8 && size != 0)
>return -EINVAL;
> -
> - tmp.bupdate_sclk = input[0];
> - tmp.sclk_up_hyst = input[1];
> - tmp.sclk_down_hyst = input[2];
> - tmp.sclk_activity = input[3];
> - tmp.bupdate_mclk = input[4];
> - tmp.mclk_up_hyst = input[5];
> - tmp.mclk_down_hyst = input[6];
> - tmp.mclk_activity = input[7];
> + /* If only CUSTOM is passed in, use the saved values. Check
> +  * that we actually have a CUSTOM profile by ensuring that
> +  * the "use sclk" or the "use mclk" bits are set
> +  */
> + tmp = smu7_profiling[PP_SMC_POWER_PROFILE_CUSTOM];
> + if (size == 0) {
> + if (tmp.bupdate_sclk == 0 && tmp.bupdate_mclk ==
> 0)
> + return -EINVAL;
> + } else {
> + tmp.bupdate_sclk = input[0];
> + tmp.sclk_up_hyst = input[1];
> + tmp.sclk_down_hyst = input[2];
> + tmp.sclk_activity = input[3];
> + tmp.bupdate_mclk = input[4];
> + tmp.mclk_up_hyst = input[5];
> + tmp.mclk_down_hyst = input[6];
> + tmp.mclk_activity = input[7];
> + smu7_profiling[PP_SMC_POWER_PROFILE_CUSTOM]
> = tmp;
> + }
>if (!smum_update_dpm_settings(hwmgr, &tmp)) {
>memcpy(&data->current_profile_setting, &tmp,
> sizeof(struct profile_mode_setting));
>hwmgr->power_profile_mode = mode;
> --
> 2.17.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/2] drm/amdgpu: Allow switching to CUSTOM profile on Vega10/20 v2

[Russell, Kent]

Am I mistaken in thinking that activity monitor gets the existing CUSTOM 
profile from the SMU? When I tested it, the CUSTOM profile was saved and could 
be retrieved through this method. I specifically set the profile to something 
ridiculous (1 2 3 4 5 6 7 8 9 10) and these values were retained when switching 
to VR and back to CUSTOM, so I was under the impression that it was saved to 
the SMU when we submit it to the SMU.

Kent

KENT RUSSELL
Sr. Software Engineer | Linux Compute Kernel
1 Commerce Valley Drive East
Markham, ON L3T 7X6
O +(1) 289-695-2122 | Ext 72122

From: Quan, Evan
Sent: Wednesday, March 27, 2019 9:36:02 PM
To: Russell, Kent; amd-gfx@lists.freedesktop.org
Cc: Russell, Kent
Subject: RE: [PATCH 1/2] drm/amdgpu: Allow switching to CUSTOM profile on 
Vega10/20 v2

For vega20, activity_monitor is also a temporary structure and you should not 
rely on that for judging existence of custom profile.

Regards
Evan
> -Original Message-
> From: amd-gfx  On Behalf Of
> Russell, Kent
> Sent: Wednesday, March 27, 2019 9:59 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Russell, Kent 
> Subject: [PATCH 1/2] drm/amdgpu: Allow switching to CUSTOM profile on
> Vega10/20 v2
>
> Don't return an error if the CUSTOM profile is selected, just apply it with 
> the
> values saved to the GPU
>
> v2: Remove reference to fixed bug, check that CUSTOM profile was set
>
> Change-Id: I114cc9783226ee9ebb146863897e951527a85e20
> Signed-off-by: Kent Russell 
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 14
> +-
> drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 14
> +-
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> index 85a536924571..7e9e7e254a0d 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> @@ -4910,9 +4910,20 @@ static int vega10_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>1 << power_profile_mode);
>
>if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) {
> - if (size == 0 || size > 4)
> + if (size != 0 && size != 4)
>return -EINVAL;
>
> + /* If size = 0 and the CUSTOM profile has been set already
> +  * (check BUSY_SET_POINT since that is always >0 for a valid
> +  * profile) then just apply the profile.
> +  */
> + if (size == 0) {
> + if (data->custom_profile_mode[0] != 0)
> + goto out;
> + else
> + return -EINVAL;
> + }
> +
>data->custom_profile_mode[0] = busy_set_point = input[0];
>data->custom_profile_mode[1] = FPS = input[1];
>data->custom_profile_mode[2] = use_rlc_busy = input[2];
> @@ -4923,6 +4934,7 @@ static int vega10_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>use_rlc_busy << 16 |
> min_active_level<<24);
>}
>
> +out:
>hwmgr->power_profile_mode = power_profile_mode;
>
>return 0;
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> index 70dc641bf94d..6dc16a9e766b 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> @@ -3843,7 +3843,7 @@ static int vega20_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>}
>
>if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) {
> - if (size < 10)
> + if (size < 10 && size != 0)
>return -EINVAL;
>
>result = vega20_get_activity_monitor_coeff(hwmgr,
> @@ -3853,6 +3853,17 @@ static int vega20_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>"[SetPowerProfile] Failed to get activity
> monitor!",
>return result);
>
> + /* If size = 0 and the CUSTOM profile has been set already
> +  * (check BoosterFreqType since that is always >0 for a valid
> +  * profile) then just apply the profile.
> +  */
> + if (size == 0) {
> + if (activity_monitor.Soc_BoosterFreqType != 0)
> + goto out;
> + else
> + return -EINVAL;
> + }
> +
>switch (input[0]) {
>case 0: /* Gfxclk */
>activity_monitor.Gfx_FPS = input[1]; @@ -3908,6
> +3919,7 @@ static int vega20_set_power_profile_mode(struct pp_hwmgr
> *hwmgr, long *input, ui
>

Re: [PATCH 8/9] drm/syncobj: add timeline signal ioctl for syncobj v5

[Lionel Landwerlin]

On 25/03/2019 08:32, Chunming Zhou wrote:

v2: individually allocate chain array, since chain node is free independently.
v3: all existing points must be already signaled before cpu perform signal 
operation,
 so add check condition for that.
v4: remove v3 change and add checking to prevent out-of-order
v5: unify binary and timeline

Signed-off-by: Chunming Zhou 
Cc: Tobias Hector 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: Chris Wilson 
Cc: Lionel Landwerlin 
---
  drivers/gpu/drm/drm_internal.h |  2 +
  drivers/gpu/drm/drm_ioctl.c|  2 +
  drivers/gpu/drm/drm_syncobj.c  | 73 ++
  include/uapi/drm/drm.h |  1 +
  4 files changed, 78 insertions(+)

diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index dd11ae5f1eef..d9a483a5fce0 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -190,6 +190,8 @@ int drm_syncobj_reset_ioctl(struct drm_device *dev, void 
*data,
struct drm_file *file_private);
  int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
 struct drm_file *file_private);
+int drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
  int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_private);
  
diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c

index 92b3b7b2fd81..d337f161909c 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -696,6 +696,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
+   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL, 
drm_syncobj_timeline_signal_ioctl,
+ DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl,
  DRM_UNLOCKED|DRM_RENDER_ALLOW),
DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, 
DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index ee2d66e047e7..099596190845 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1183,6 +1183,79 @@ drm_syncobj_signal_ioctl(struct drm_device *dev, void 
*data,
return ret;
  }
  
+int

+drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private)
+{
+   struct drm_syncobj_timeline_array *args = data;
+   struct drm_syncobj **syncobjs;
+   struct dma_fence_chain **chains;
+   uint64_t *points;
+   uint32_t i, j;
+   int ret;
+
+   if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+   return -EOPNOTSUPP;
+
+   if (args->pad != 0)
+   return -EINVAL;
+
+   if (args->count_handles == 0)
+   return -EINVAL;
+
+   ret = drm_syncobj_array_find(file_private,
+u64_to_user_ptr(args->handles),
+args->count_handles,
+&syncobjs);
+   if (ret < 0)
+   return ret;
+
+   points = kmalloc_array(args->count_handles, sizeof(*points),
+  GFP_KERNEL);
+   if (!points) {
+   ret = -ENOMEM;
+   goto out;
+   }
+   if (!u64_to_user_ptr(args->points)) {
+   memset(points, 0, args->count_handles * sizeof(uint64_t));
+   } else if (copy_from_user(points, u64_to_user_ptr(args->points),
+ sizeof(uint64_t) * args->count_handles)) {
+   ret = -EFAULT;
+   goto err_points;
+   }
+
+   chains = kmalloc_array(args->count_handles, sizeof(void *), GFP_KERNEL);
+   if (!chains) {
+   ret = -ENOMEM;
+   goto err_points;
+   }
+   for (i = 0; i < args->count_handles; i++) {
+   chains[i] = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL);
+   if (!chains[i]) {
+   for (j = 0; j < i; j++)
+   kfree(chains[j]);
+   ret = -ENOMEM;
+   goto err_chains;
+   }
+   }
+
+   for (i = 0; i < args->count_handles; i++) {
+   struct dma_fence *fence = dma_fence_get_stub();
+
+   drm_syncobj_add_point(syncobjs[i], chains[i],
+ fence, points[i]);
+   dma_fence_put(fence);
+   }



I think I found an issue where the implementation doesn't match what the 
spec requires on host signaling.


Consider the following :


Timeline semaphore has a value of 4.

A device submits a commands to sign

Re: [PATCH 8/9] drm/syncobj: add timeline signal ioctl for syncobj v5

[Chunming Zhou]

在 2019/3/28 20:53, Lionel Landwerlin 写道:
> On 25/03/2019 08:32, Chunming Zhou wrote:
>> v2: individually allocate chain array, since chain node is free 
>> independently.
>> v3: all existing points must be already signaled before cpu perform 
>> signal operation,
>>  so add check condition for that.
>> v4: remove v3 change and add checking to prevent out-of-order
>> v5: unify binary and timeline
>>
>> Signed-off-by: Chunming Zhou 
>> Cc: Tobias Hector 
>> Cc: Jason Ekstrand 
>> Cc: Dave Airlie 
>> Cc: Chris Wilson 
>> Cc: Lionel Landwerlin 
>> ---
>>   drivers/gpu/drm/drm_internal.h |  2 +
>>   drivers/gpu/drm/drm_ioctl.c    |  2 +
>>   drivers/gpu/drm/drm_syncobj.c  | 73 ++
>>   include/uapi/drm/drm.h |  1 +
>>   4 files changed, 78 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_internal.h 
>> b/drivers/gpu/drm/drm_internal.h
>> index dd11ae5f1eef..d9a483a5fce0 100644
>> --- a/drivers/gpu/drm/drm_internal.h
>> +++ b/drivers/gpu/drm/drm_internal.h
>> @@ -190,6 +190,8 @@ int drm_syncobj_reset_ioctl(struct drm_device 
>> *dev, void *data,
>>   struct drm_file *file_private);
>>   int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
>>    struct drm_file *file_private);
>> +int drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void 
>> *data,
>> +  struct drm_file *file_private);
>>   int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
>>   struct drm_file *file_private);
>>   diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
>> index 92b3b7b2fd81..d337f161909c 100644
>> --- a/drivers/gpu/drm/drm_ioctl.c
>> +++ b/drivers/gpu/drm/drm_ioctl.c
>> @@ -696,6 +696,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
>>     DRM_UNLOCKED|DRM_RENDER_ALLOW),
>>   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
>>     DRM_UNLOCKED|DRM_RENDER_ALLOW),
>> +    DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL, 
>> drm_syncobj_timeline_signal_ioctl,
>> +  DRM_UNLOCKED|DRM_RENDER_ALLOW),
>>   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl,
>>     DRM_UNLOCKED|DRM_RENDER_ALLOW),
>>   DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, 
>> drm_crtc_get_sequence_ioctl, DRM_UNLOCKED),
>> diff --git a/drivers/gpu/drm/drm_syncobj.c 
>> b/drivers/gpu/drm/drm_syncobj.c
>> index ee2d66e047e7..099596190845 100644
>> --- a/drivers/gpu/drm/drm_syncobj.c
>> +++ b/drivers/gpu/drm/drm_syncobj.c
>> @@ -1183,6 +1183,79 @@ drm_syncobj_signal_ioctl(struct drm_device 
>> *dev, void *data,
>>   return ret;
>>   }
>>   +int
>> +drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data,
>> +  struct drm_file *file_private)
>> +{
>> +    struct drm_syncobj_timeline_array *args = data;
>> +    struct drm_syncobj **syncobjs;
>> +    struct dma_fence_chain **chains;
>> +    uint64_t *points;
>> +    uint32_t i, j;
>> +    int ret;
>> +
>> +    if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
>> +    return -EOPNOTSUPP;
>> +
>> +    if (args->pad != 0)
>> +    return -EINVAL;
>> +
>> +    if (args->count_handles == 0)
>> +    return -EINVAL;
>> +
>> +    ret = drm_syncobj_array_find(file_private,
>> + u64_to_user_ptr(args->handles),
>> + args->count_handles,
>> + &syncobjs);
>> +    if (ret < 0)
>> +    return ret;
>> +
>> +    points = kmalloc_array(args->count_handles, sizeof(*points),
>> +   GFP_KERNEL);
>> +    if (!points) {
>> +    ret = -ENOMEM;
>> +    goto out;
>> +    }
>> +    if (!u64_to_user_ptr(args->points)) {
>> +    memset(points, 0, args->count_handles * sizeof(uint64_t));
>> +    } else if (copy_from_user(points, u64_to_user_ptr(args->points),
>> +  sizeof(uint64_t) * args->count_handles)) {
>> +    ret = -EFAULT;
>> +    goto err_points;
>> +    }
>> +
>> +    chains = kmalloc_array(args->count_handles, sizeof(void *), 
>> GFP_KERNEL);
>> +    if (!chains) {
>> +    ret = -ENOMEM;
>> +    goto err_points;
>> +    }
>> +    for (i = 0; i < args->count_handles; i++) {
>> +    chains[i] = kzalloc(sizeof(struct dma_fence_chain), 
>> GFP_KERNEL);
>> +    if (!chains[i]) {
>> +    for (j = 0; j < i; j++)
>> +    kfree(chains[j]);
>> +    ret = -ENOMEM;
>> +    goto err_chains;
>> +    }
>> +    }
>> +
>> +    for (i = 0; i < args->count_handles; i++) {
>> +    struct dma_fence *fence = dma_fence_get_stub();
>> +
>> +    drm_syncobj_add_point(syncobjs[i], chains[i],
>> +  fence, points[i]);
>> +    dma_fence_put(fence);
>> +    }
>
>
> I think I found an issue where the implementation doesn't match what 
> the spec requires on host signaling.
>
> Consider the following :
>
>
> Timeline semaphore has a value of 4.
>
> A device submits a comman

Re: [PATCH AUTOSEL 5.0 070/262] drm/amd/display: Pass app_tf by value rather than by reference

[Pavel Machek]

> From: Nathan Chancellor 
> 
> [ Upstream commit 672e78cab819ebe31e3b9b8abac367be8a110472 ]
> 
> Clang warns when an expression that equals zero is used as a null
> pointer constant (in lieu of NULL):

Fixes warning, with unsupported compiler; not a serious bug. Plus, not
a minimal fix.
Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 8/9] drm/syncobj: add timeline signal ioctl for syncobj v5

[Lionel Landwerlin]

On 28/03/2019 13:08, Chunming Zhou wrote:

在 2019/3/28 20:53, Lionel Landwerlin 写道:

On 25/03/2019 08:32, Chunming Zhou wrote:

v2: individually allocate chain array, since chain node is free
independently.
v3: all existing points must be already signaled before cpu perform
signal operation,
  so add check condition for that.
v4: remove v3 change and add checking to prevent out-of-order
v5: unify binary and timeline

Signed-off-by: Chunming Zhou 
Cc: Tobias Hector 
Cc: Jason Ekstrand 
Cc: Dave Airlie 
Cc: Chris Wilson 
Cc: Lionel Landwerlin 
---
   drivers/gpu/drm/drm_internal.h |  2 +
   drivers/gpu/drm/drm_ioctl.c    |  2 +
   drivers/gpu/drm/drm_syncobj.c  | 73 ++
   include/uapi/drm/drm.h |  1 +
   4 files changed, 78 insertions(+)

diff --git a/drivers/gpu/drm/drm_internal.h
b/drivers/gpu/drm/drm_internal.h
index dd11ae5f1eef..d9a483a5fce0 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -190,6 +190,8 @@ int drm_syncobj_reset_ioctl(struct drm_device
*dev, void *data,
   struct drm_file *file_private);
   int drm_syncobj_signal_ioctl(struct drm_device *dev, void *data,
    struct drm_file *file_private);
+int drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void
*data,
+  struct drm_file *file_private);
   int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file_private);
   diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 92b3b7b2fd81..d337f161909c 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -696,6 +696,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
     DRM_UNLOCKED|DRM_RENDER_ALLOW),
   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_SIGNAL, drm_syncobj_signal_ioctl,
     DRM_UNLOCKED|DRM_RENDER_ALLOW),
+    DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_TIMELINE_SIGNAL,
drm_syncobj_timeline_signal_ioctl,
+  DRM_UNLOCKED|DRM_RENDER_ALLOW),
   DRM_IOCTL_DEF(DRM_IOCTL_SYNCOBJ_QUERY, drm_syncobj_query_ioctl,
     DRM_UNLOCKED|DRM_RENDER_ALLOW),
   DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE,
drm_crtc_get_sequence_ioctl, DRM_UNLOCKED),
diff --git a/drivers/gpu/drm/drm_syncobj.c
b/drivers/gpu/drm/drm_syncobj.c
index ee2d66e047e7..099596190845 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1183,6 +1183,79 @@ drm_syncobj_signal_ioctl(struct drm_device
*dev, void *data,
   return ret;
   }
   +int
+drm_syncobj_timeline_signal_ioctl(struct drm_device *dev, void *data,
+  struct drm_file *file_private)
+{
+    struct drm_syncobj_timeline_array *args = data;
+    struct drm_syncobj **syncobjs;
+    struct dma_fence_chain **chains;
+    uint64_t *points;
+    uint32_t i, j;
+    int ret;
+
+    if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+    return -EOPNOTSUPP;
+
+    if (args->pad != 0)
+    return -EINVAL;
+
+    if (args->count_handles == 0)
+    return -EINVAL;
+
+    ret = drm_syncobj_array_find(file_private,
+ u64_to_user_ptr(args->handles),
+ args->count_handles,
+ &syncobjs);
+    if (ret < 0)
+    return ret;
+
+    points = kmalloc_array(args->count_handles, sizeof(*points),
+   GFP_KERNEL);
+    if (!points) {
+    ret = -ENOMEM;
+    goto out;
+    }
+    if (!u64_to_user_ptr(args->points)) {
+    memset(points, 0, args->count_handles * sizeof(uint64_t));
+    } else if (copy_from_user(points, u64_to_user_ptr(args->points),
+  sizeof(uint64_t) * args->count_handles)) {
+    ret = -EFAULT;
+    goto err_points;
+    }
+
+    chains = kmalloc_array(args->count_handles, sizeof(void *),
GFP_KERNEL);
+    if (!chains) {
+    ret = -ENOMEM;
+    goto err_points;
+    }
+    for (i = 0; i < args->count_handles; i++) {
+    chains[i] = kzalloc(sizeof(struct dma_fence_chain),
GFP_KERNEL);
+    if (!chains[i]) {
+    for (j = 0; j < i; j++)
+    kfree(chains[j]);
+    ret = -ENOMEM;
+    goto err_chains;
+    }
+    }
+
+    for (i = 0; i < args->count_handles; i++) {
+    struct dma_fence *fence = dma_fence_get_stub();
+
+    drm_syncobj_add_point(syncobjs[i], chains[i],
+  fence, points[i]);
+    dma_fence_put(fence);
+    }


I think I found an issue where the implementation doesn't match what
the spec requires on host signaling.

Consider the following :


Timeline semaphore has a value of 4.

A device submits a commands to signal point 5.

The host signals point 6.


The value of the timeline will remain 4 until the device commands
complete, at which point it will change to 6.

But the specification seems to say that the timeline value should be 6
once the host signaling completes.


The only option that I see working would be to add point 6 in the

Re: [PATCH 2/9] drm/syncobj: add new drm_syncobj_add_point interface v4

[Lionel Landwerlin]

On 25/03/2019 08:32, Chunming Zhou wrote:

From: Christian König 

Use the dma_fence_chain object to create a timeline of fence objects
instead of just replacing the existing fence.

v2: rebase and cleanup
v3: fix garbage collection parameters
v4: add unorder point check, print a warn calltrace

Signed-off-by: Christian König 
Cc: Lionel Landwerlin 
---
  drivers/gpu/drm/drm_syncobj.c | 39 +++
  include/drm/drm_syncobj.h |  5 +
  2 files changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 5329e66598c6..19a9ce638119 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -122,6 +122,45 @@ static void drm_syncobj_remove_wait(struct drm_syncobj 
*syncobj,
spin_unlock(&syncobj->lock);
  }
  
+/**

+ * drm_syncobj_add_point - add new timeline point to the syncobj
+ * @syncobj: sync object to add timeline point do
+ * @chain: chain node to use to add the point
+ * @fence: fence to encapsulate in the chain node
+ * @point: sequence number to use for the point
+ *
+ * Add the chain node as new timeline point to the syncobj.
+ */
+void drm_syncobj_add_point(struct drm_syncobj *syncobj,
+  struct dma_fence_chain *chain,
+  struct dma_fence *fence,
+  uint64_t point)
+{
+   struct syncobj_wait_entry *cur, *tmp;
+   struct dma_fence *prev;
+
+   dma_fence_get(fence);
+
+   spin_lock(&syncobj->lock);
+
+   prev = drm_syncobj_fence_get(syncobj);
+   /* You are adding an unorder point to timeline, which could cause 
payload returned from query_ioctl is 0! */
+   WARN_ON_ONCE(prev && prev->seqno >= point);



I think the WARN/BUG macros should only fire when there is an issue with 
programming from within the kernel.


But this particular warning can be triggered by an application.


Probably best to just remove it?


-Lionel



+   dma_fence_chain_init(chain, prev, fence, point);
+   rcu_assign_pointer(syncobj->fence, &chain->base);
+
+   list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) {
+   list_del_init(&cur->node);
+   syncobj_wait_syncobj_func(syncobj, cur);
+   }
+   spin_unlock(&syncobj->lock);
+
+   /* Walk the chain once to trigger garbage collection */
+   dma_fence_chain_for_each(fence, prev);
+   dma_fence_put(prev);
+}
+EXPORT_SYMBOL(drm_syncobj_add_point);
+
  /**
   * drm_syncobj_replace_fence - replace fence in a sync object.
   * @syncobj: Sync object to replace fence in
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index 0311c9fdbd2f..6cf7243a1dc5 100644
--- a/include/drm/drm_syncobj.h
+++ b/include/drm/drm_syncobj.h
@@ -27,6 +27,7 @@
  #define __DRM_SYNCOBJ_H__
  
  #include 

+#include 
  
  struct drm_file;
  
@@ -112,6 +113,10 @@ drm_syncobj_fence_get(struct drm_syncobj *syncobj)
  
  struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private,

 u32 handle);
+void drm_syncobj_add_point(struct drm_syncobj *syncobj,
+  struct dma_fence_chain *chain,
+  struct dma_fence *fence,
+  uint64_t point);
  void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
   struct dma_fence *fence);
  int drm_syncobj_find_fence(struct drm_file *file_private,



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

RE: [PATCH 1/2] drm/amdgpu: Allow switching to CUSTOM profile on Vega10/20 v2

[Quan, Evan]

OK, I see. Yep, the new settings will be stored in SMU.
But as I know, the SMU comes with some default settings for CUSTOM profile.
I suspect "activity_monitor.Soc_BoosterFreqType != 0" will be also true even 
without your first CUSTOM profile settings(with input parameters).
So, I would prefer to have a local copy of previous custom settigns(as what we 
do on vega10 and smu7).

Evan
From: Russell, Kent 
Sent: Thursday, March 28, 2019 5:29 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/amdgpu: Allow switching to CUSTOM profile on 
Vega10/20 v2

Am I mistaken in thinking that activity monitor gets the existing CUSTOM 
profile from the SMU? When I tested it, the CUSTOM profile was saved and could 
be retrieved through this method. I specifically set the profile to something 
ridiculous (1 2 3 4 5 6 7 8 9 10) and these values were retained when switching 
to VR and back to CUSTOM, so I was under the impression that it was saved to 
the SMU when we submit it to the SMU.

Kent

KENT RUSSELL
Sr. Software Engineer | Linux Compute Kernel
1 Commerce Valley Drive East
Markham, ON L3T 7X6
O +(1) 289-695-2122 | Ext 72122

From: Quan, Evan
Sent: Wednesday, March 27, 2019 9:36:02 PM
To: Russell, Kent; 
amd-gfx@lists.freedesktop.org
Cc: Russell, Kent
Subject: RE: [PATCH 1/2] drm/amdgpu: Allow switching to CUSTOM profile on 
Vega10/20 v2

For vega20, activity_monitor is also a temporary structure and you should not 
rely on that for judging existence of custom profile.

Regards
Evan
> -Original Message-
> From: amd-gfx 
> mailto:amd-gfx-boun...@lists.freedesktop.org>>
>  On Behalf Of
> Russell, Kent
> Sent: Wednesday, March 27, 2019 9:59 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Russell, Kent mailto:kent.russ...@amd.com>>
> Subject: [PATCH 1/2] drm/amdgpu: Allow switching to CUSTOM profile on
> Vega10/20 v2
>
> Don't return an error if the CUSTOM profile is selected, just apply it with 
> the
> values saved to the GPU
>
> v2: Remove reference to fixed bug, check that CUSTOM profile was set
>
> Change-Id: I114cc9783226ee9ebb146863897e951527a85e20
> Signed-off-by: Kent Russell 
> mailto:kent.russ...@amd.com>>
> ---
>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 14
> +-
> drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 14
> +-
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> index 85a536924571..7e9e7e254a0d 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
> @@ -4910,9 +4910,20 @@ static int vega10_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>1 << power_profile_mode);
>
>if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) {
> - if (size == 0 || size > 4)
> + if (size != 0 && size != 4)
>return -EINVAL;
>
> + /* If size = 0 and the CUSTOM profile has been set already
> +  * (check BUSY_SET_POINT since that is always >0 for a valid
> +  * profile) then just apply the profile.
> +  */
> + if (size == 0) {
> + if (data->custom_profile_mode[0] != 0)
> + goto out;
> + else
> + return -EINVAL;
> + }
> +
>data->custom_profile_mode[0] = busy_set_point = input[0];
>data->custom_profile_mode[1] = FPS = input[1];
>data->custom_profile_mode[2] = use_rlc_busy = input[2];
> @@ -4923,6 +4934,7 @@ static int vega10_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>use_rlc_busy << 16 |
> min_active_level<<24);
>}
>
> +out:
>hwmgr->power_profile_mode = power_profile_mode;
>
>return 0;
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> index 70dc641bf94d..6dc16a9e766b 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> @@ -3843,7 +3843,7 @@ static int vega20_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>}
>
>if (power_profile_mode == PP_SMC_POWER_PROFILE_CUSTOM) {
> - if (size < 10)
> + if (size < 10 && size != 0)
>return -EINVAL;
>
>result = vega20_get_activity_monitor_coeff(hwmgr,
> @@ -3853,6 +3853,17 @@ static int vega20_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, ui
>"[SetPowerProfile] Failed to get activity
> monitor!",
> 

RE: [PATCH 2/2] drm/amdgpu: Allow switching to CUSTOM profile on smu7 v2

[Quan, Evan]

Oops, sorry, missed that. That will be fine.
Reviewed-by: Evan Quan 

Evan

From: amd-gfx  On Behalf Of Russell, Kent
Sent: Thursday, March 28, 2019 5:25 PM
To: Quan, Evan ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: Allow switching to CUSTOM profile on smu7 
v2

Hi Evan,

I set tmp=smu7_profiling[PP_SMC_PROFILE_CUSTOM] before checking the bupdate 
parameters (see below). That way I don't have ugly 100-character-long checks in 
the if size==0 check. Then if its invalid I just return -EINVAL, or if 
it'svalid then we use it to writeto the SMU. If parameters are passed in, tmp 
is overwritten with the passed-in values and we use it as before.

Kent

KENT RUSSELL
Sr. Software Engineer | Linux Compute Kernel
1 Commerce Valley Drive East
Markham, ON L3T 7X6
O +(1) 289-695-2122 | Ext 72122

From: Quan, Evan
Sent: Wednesday, March 27, 2019 9:26:41 PM
To: Russell, Kent; 
amd-gfx@lists.freedesktop.org
Cc: Russell, Kent
Subject: RE: [PATCH 2/2] drm/amdgpu: Allow switching to CUSTOM profile on smu7 
v2

I think you should use 
smu7_profiling[PP_SMC_POWER_PROFILE_CUSTOM].bupdate_sclk/bupdate_mclk to judge 
whether there is saved custom profile.
That's where previous custom profile settings store in.
'tmp' is a temporary structures and you should not rely on that.

Regards,
Evan
> -Original Message-
> From: amd-gfx 
> mailto:amd-gfx-boun...@lists.freedesktop.org>>
>  On Behalf Of
> Russell, Kent
> Sent: Wednesday, March 27, 2019 9:59 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Russell, Kent mailto:kent.russ...@amd.com>>
> Subject: [PATCH 2/2] drm/amdgpu: Allow switching to CUSTOM profile on
> smu7 v2
>
> Allow changing to the CUSTOM profile without requiring the parameters
> being passed in each time. Store the values in the smu7_profiling table since
> it's defined here anyways
>
> v2: Add check that CUSTOM was previously set
>
> Change-Id: I6c5e3a1487e12410a6a7670a5cf1a6599253344d
> Signed-off-by: Kent Russell 
> mailto:kent.russ...@amd.com>>
> ---
>  .../gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c  | 32 --
> -
>  1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> index 83d3d935f3ac..048757e8f494 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
> @@ -77,7 +77,7 @@
>  #define PCIE_BUS_CLK1
>  #define TCLK(PCIE_BUS_CLK / 10)
>
> -static const struct profile_mode_setting smu7_profiling[7] =
> +static struct profile_mode_setting smu7_profiling[7] =
>{{0, 0, 0, 0, 0, 0, 0, 0},
> {1, 0, 100, 30, 1, 0, 100, 10},
> {1, 10, 0, 30, 0, 0, 0, 0},
> @@ -4984,17 +4984,27 @@ static int smu7_set_power_profile_mode(struct
> pp_hwmgr *hwmgr, long *input, uint
>mode = input[size];
>switch (mode) {
>case PP_SMC_POWER_PROFILE_CUSTOM:
> - if (size < 8)
> + if (size < 8 && size != 0)
>return -EINVAL;
> -
> - tmp.bupdate_sclk = input[0];
> - tmp.sclk_up_hyst = input[1];
> - tmp.sclk_down_hyst = input[2];
> - tmp.sclk_activity = input[3];
> - tmp.bupdate_mclk = input[4];
> - tmp.mclk_up_hyst = input[5];
> - tmp.mclk_down_hyst = input[6];
> - tmp.mclk_activity = input[7];
> + /* If only CUSTOM is passed in, use the saved values. Check
> +  * that we actually have a CUSTOM profile by ensuring that
> +  * the "use sclk" or the "use mclk" bits are set
> +  */
> + tmp = smu7_profiling[PP_SMC_POWER_PROFILE_CUSTOM];
> + if (size == 0) {
> + if (tmp.bupdate_sclk == 0 && tmp.bupdate_mclk ==
> 0)
> + return -EINVAL;
> + } else {
> + tmp.bupdate_sclk = input[0];
> + tmp.sclk_up_hyst = input[1];
> + tmp.sclk_down_hyst = input[2];
> + tmp.sclk_activity = input[3];
> + tmp.bupdate_mclk = input[4];
> + tmp.mclk_up_hyst = input[5];
> + tmp.mclk_down_hyst = input[6];
> + tmp.mclk_activity = input[7];
> + smu7_profiling[PP_SMC_POWER_PROFILE_CUSTOM]
> = tmp;
> + }
>if (!smum_update_dpm_settings(hwmgr, &tmp)) {
>memcpy(&data->current_profile_setting, &tmp,
> sizeof(struct profile_mode_setting));
>hwmgr->power_profile_mode = mode;
> --
> 2.17.1
>
> ___
> amd-gfx mailing lis

[PATCH] drm/radeon: Finish GART and AGP resources after TTM

[Thomas Zimmermann]

When called during unloading, the function radeon_gart_unbind() may
complain about the GART not being initialized.

TTM calls the function radeon_gart_unbind() as part of its finish
procedure and expects the GART structure to be initialized. As the radeon
driver calls radeon_gart_fini() before finishing TTM, radeon_gart_unbind()
complains with

trying to unbind memory from uninitialized GART !

This patch reorders the shutdown such that GART and AGP resources are
always finished after TTM. The problem has been observed on SI hardware.
The code for other hardware is being fixed accordingly.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/radeon/cik.c   | 2 +-
 drivers/gpu/drm/radeon/evergreen.c | 2 +-
 drivers/gpu/drm/radeon/ni.c| 2 +-
 drivers/gpu/drm/radeon/r100.c  | 6 +++---
 drivers/gpu/drm/radeon/r300.c  | 6 +++---
 drivers/gpu/drm/radeon/r600.c  | 4 ++--
 drivers/gpu/drm/radeon/rs400.c | 2 +-
 drivers/gpu/drm/radeon/rs600.c | 2 +-
 drivers/gpu/drm/radeon/rv770.c | 4 ++--
 drivers/gpu/drm/radeon/si.c| 2 +-
 10 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
index ab7b4e2ffcd2..0795b4f1d1d7 100644
--- a/drivers/gpu/drm/radeon/cik.c
+++ b/drivers/gpu/drm/radeon/cik.c
@@ -8727,11 +8727,11 @@ void cik_fini(struct radeon_device *rdev)
uvd_v1_0_fini(rdev);
radeon_uvd_fini(rdev);
radeon_vce_fini(rdev);
-   cik_pcie_gart_fini(rdev);
r600_vram_scratch_fini(rdev);
radeon_gem_fini(rdev);
radeon_fence_driver_fini(rdev);
radeon_bo_fini(rdev);
+   cik_pcie_gart_fini(rdev);
radeon_atombios_fini(rdev);
kfree(rdev->bios);
rdev->bios = NULL;
diff --git a/drivers/gpu/drm/radeon/evergreen.c 
b/drivers/gpu/drm/radeon/evergreen.c
index 42025b6fd858..9c61ca7e52ae 100644
--- a/drivers/gpu/drm/radeon/evergreen.c
+++ b/drivers/gpu/drm/radeon/evergreen.c
@@ -5307,12 +5307,12 @@ void evergreen_fini(struct radeon_device *rdev)
radeon_irq_kms_fini(rdev);
uvd_v1_0_fini(rdev);
radeon_uvd_fini(rdev);
-   evergreen_pcie_gart_fini(rdev);
r600_vram_scratch_fini(rdev);
radeon_gem_fini(rdev);
radeon_fence_driver_fini(rdev);
radeon_agp_fini(rdev);
radeon_bo_fini(rdev);
+   evergreen_pcie_gart_fini(rdev);
radeon_atombios_fini(rdev);
kfree(rdev->bios);
rdev->bios = NULL;
diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c
index 37c3a33c4a81..cd00ffcb2028 100644
--- a/drivers/gpu/drm/radeon/ni.c
+++ b/drivers/gpu/drm/radeon/ni.c
@@ -2487,11 +2487,11 @@ void cayman_fini(struct radeon_device *rdev)
radeon_uvd_fini(rdev);
if (rdev->has_vce)
radeon_vce_fini(rdev);
-   cayman_pcie_gart_fini(rdev);
r600_vram_scratch_fini(rdev);
radeon_gem_fini(rdev);
radeon_fence_driver_fini(rdev);
radeon_bo_fini(rdev);
+   cayman_pcie_gart_fini(rdev);
radeon_atombios_fini(rdev);
kfree(rdev->bios);
rdev->bios = NULL;
diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
index c12b4fa7f850..54a5cc20511d 100644
--- a/drivers/gpu/drm/radeon/r100.c
+++ b/drivers/gpu/drm/radeon/r100.c
@@ -3984,12 +3984,12 @@ void r100_fini(struct radeon_device *rdev)
radeon_wb_fini(rdev);
radeon_ib_pool_fini(rdev);
radeon_gem_fini(rdev);
-   if (rdev->flags & RADEON_IS_PCI)
-   r100_pci_gart_fini(rdev);
-   radeon_agp_fini(rdev);
radeon_irq_kms_fini(rdev);
radeon_fence_driver_fini(rdev);
radeon_bo_fini(rdev);
+   if (rdev->flags & RADEON_IS_PCI)
+   r100_pci_gart_fini(rdev);
+   radeon_agp_fini(rdev);
radeon_atombios_fini(rdev);
kfree(rdev->bios);
rdev->bios = NULL;
diff --git a/drivers/gpu/drm/radeon/r300.c b/drivers/gpu/drm/radeon/r300.c
index 652126fd6dd4..3fd838a847ca 100644
--- a/drivers/gpu/drm/radeon/r300.c
+++ b/drivers/gpu/drm/radeon/r300.c
@@ -1488,14 +1488,14 @@ void r300_fini(struct radeon_device *rdev)
radeon_wb_fini(rdev);
radeon_ib_pool_fini(rdev);
radeon_gem_fini(rdev);
+   radeon_irq_kms_fini(rdev);
+   radeon_fence_driver_fini(rdev);
+   radeon_bo_fini(rdev);
if (rdev->flags & RADEON_IS_PCIE)
rv370_pcie_gart_fini(rdev);
if (rdev->flags & RADEON_IS_PCI)
r100_pci_gart_fini(rdev);
radeon_agp_fini(rdev);
-   radeon_irq_kms_fini(rdev);
-   radeon_fence_driver_fini(rdev);
-   radeon_bo_fini(rdev);
radeon_atombios_fini(rdev);
kfree(rdev->bios);
rdev->bios = NULL;
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index e06e2d8feab3..d208fbdefc86 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -3345,12 +3345,12 @@ void r600_fini(struct radeon_

Re: [PATCH 2/9] drm/syncobj: add new drm_syncobj_add_point interface v4

[Christian König]

Am 28.03.19 um 14:50 schrieb Lionel Landwerlin:

On 25/03/2019 08:32, Chunming Zhou wrote:

From: Christian König 

Use the dma_fence_chain object to create a timeline of fence objects
instead of just replacing the existing fence.

v2: rebase and cleanup
v3: fix garbage collection parameters
v4: add unorder point check, print a warn calltrace

Signed-off-by: Christian König 
Cc: Lionel Landwerlin 
---
  drivers/gpu/drm/drm_syncobj.c | 39 +++
  include/drm/drm_syncobj.h |  5 +
  2 files changed, 44 insertions(+)

diff --git a/drivers/gpu/drm/drm_syncobj.c 
b/drivers/gpu/drm/drm_syncobj.c

index 5329e66598c6..19a9ce638119 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -122,6 +122,45 @@ static void drm_syncobj_remove_wait(struct 
drm_syncobj *syncobj,

  spin_unlock(&syncobj->lock);
  }
  +/**
+ * drm_syncobj_add_point - add new timeline point to the syncobj
+ * @syncobj: sync object to add timeline point do
+ * @chain: chain node to use to add the point
+ * @fence: fence to encapsulate in the chain node
+ * @point: sequence number to use for the point
+ *
+ * Add the chain node as new timeline point to the syncobj.
+ */
+void drm_syncobj_add_point(struct drm_syncobj *syncobj,
+   struct dma_fence_chain *chain,
+   struct dma_fence *fence,
+   uint64_t point)
+{
+    struct syncobj_wait_entry *cur, *tmp;
+    struct dma_fence *prev;
+
+    dma_fence_get(fence);
+
+    spin_lock(&syncobj->lock);
+
+    prev = drm_syncobj_fence_get(syncobj);
+    /* You are adding an unorder point to timeline, which could 
cause payload returned from query_ioctl is 0! */

+    WARN_ON_ONCE(prev && prev->seqno >= point);



I think the WARN/BUG macros should only fire when there is an issue 
with programming from within the kernel.


But this particular warning can be triggered by an application.


Probably best to just remove it?


Yeah, that was also my argument against it.

Key point here is that we still want to note somehow that userspace did 
something wrong and returning an error is not an option.


Maybe just use DRM_ERROR with a static variable to print the message 
only once.


Christian.




-Lionel



+    dma_fence_chain_init(chain, prev, fence, point);
+    rcu_assign_pointer(syncobj->fence, &chain->base);
+
+    list_for_each_entry_safe(cur, tmp, &syncobj->cb_list, node) {
+    list_del_init(&cur->node);
+    syncobj_wait_syncobj_func(syncobj, cur);
+    }
+    spin_unlock(&syncobj->lock);
+
+    /* Walk the chain once to trigger garbage collection */
+    dma_fence_chain_for_each(fence, prev);
+    dma_fence_put(prev);
+}
+EXPORT_SYMBOL(drm_syncobj_add_point);
+
  /**
   * drm_syncobj_replace_fence - replace fence in a sync object.
   * @syncobj: Sync object to replace fence in
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index 0311c9fdbd2f..6cf7243a1dc5 100644
--- a/include/drm/drm_syncobj.h
+++ b/include/drm/drm_syncobj.h
@@ -27,6 +27,7 @@
  #define __DRM_SYNCOBJ_H__
    #include 
+#include 
    struct drm_file;
  @@ -112,6 +113,10 @@ drm_syncobj_fence_get(struct drm_syncobj 
*syncobj)

    struct drm_syncobj *drm_syncobj_find(struct drm_file *file_private,
   u32 handle);
+void drm_syncobj_add_point(struct drm_syncobj *syncobj,
+   struct dma_fence_chain *chain,
+   struct dma_fence *fence,
+   uint64_t point);
  void drm_syncobj_replace_fence(struct drm_syncobj *syncobj,
 struct dma_fence *fence);
  int drm_syncobj_find_fence(struct drm_file *file_private,





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

Re: [PATCH 2/9] drm/syncobj: add new drm_syncobj_add_point interface v4

[Michel Dänzer]

On 2019-03-28 4:18 p.m., Christian König wrote:
> Am 28.03.19 um 14:50 schrieb Lionel Landwerlin:
>> On 25/03/2019 08:32, Chunming Zhou wrote:
>>> From: Christian König 
>>>
>>> Use the dma_fence_chain object to create a timeline of fence objects
>>> instead of just replacing the existing fence.
>>>
>>> v2: rebase and cleanup
>>> v3: fix garbage collection parameters
>>> v4: add unorder point check, print a warn calltrace
>>>
>>> Signed-off-by: Christian König 
>>> Cc: Lionel Landwerlin 
>>> ---
>>>   drivers/gpu/drm/drm_syncobj.c | 39 +++
>>>   include/drm/drm_syncobj.h |  5 +
>>>   2 files changed, 44 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/drm_syncobj.c
>>> b/drivers/gpu/drm/drm_syncobj.c
>>> index 5329e66598c6..19a9ce638119 100644
>>> --- a/drivers/gpu/drm/drm_syncobj.c
>>> +++ b/drivers/gpu/drm/drm_syncobj.c
>>> @@ -122,6 +122,45 @@ static void drm_syncobj_remove_wait(struct
>>> drm_syncobj *syncobj,
>>>   spin_unlock(&syncobj->lock);
>>>   }
>>>   +/**
>>> + * drm_syncobj_add_point - add new timeline point to the syncobj
>>> + * @syncobj: sync object to add timeline point do
>>> + * @chain: chain node to use to add the point
>>> + * @fence: fence to encapsulate in the chain node
>>> + * @point: sequence number to use for the point
>>> + *
>>> + * Add the chain node as new timeline point to the syncobj.
>>> + */
>>> +void drm_syncobj_add_point(struct drm_syncobj *syncobj,
>>> +   struct dma_fence_chain *chain,
>>> +   struct dma_fence *fence,
>>> +   uint64_t point)
>>> +{
>>> +    struct syncobj_wait_entry *cur, *tmp;
>>> +    struct dma_fence *prev;
>>> +
>>> +    dma_fence_get(fence);
>>> +
>>> +    spin_lock(&syncobj->lock);
>>> +
>>> +    prev = drm_syncobj_fence_get(syncobj);
>>> +    /* You are adding an unorder point to timeline, which could
>>> cause payload returned from query_ioctl is 0! */
>>> +    WARN_ON_ONCE(prev && prev->seqno >= point);
>>
>>
>> I think the WARN/BUG macros should only fire when there is an issue
>> with programming from within the kernel.
>>
>> But this particular warning can be triggered by an application.
>>
>>
>> Probably best to just remove it?
> 
> Yeah, that was also my argument against it.
> 
> Key point here is that we still want to note somehow that userspace did
> something wrong and returning an error is not an option.
> 
> Maybe just use DRM_ERROR with a static variable to print the message
> only once.

How about DRM_WARN_ONCE ?


-- 
Earthling Michel Dänzer   |  https://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

[pull] amdgpu, amdkfd, ttm drm-next-5.2

[Alex Deucher]

Hi Dave, Daniel,

New stuff for 5.2:

amdgpu:
- Switch to HMM for userptr (reverted until HMM fixes land)
- New experimental SMU 11 replacement for powerplay for vega20 (not enabled by 
default)
- Initial RAS support for vega20
- BACO support for vega12
- BACO fixes for vega20
- Rework IH handling for page fault and retry interrupts
- Cleanly split CPU and GPU paths for GPUVM updates
- Powerplay fixes
- XGMI fixes
- Rework how DC interacts with atomic for planes
- Clean up and simplify DC/Powerplay interfaces
- Misc cleanups and bug fixes

amdkfd:
- Switch to HMM for userptr (reverted until HMM fixes land)
- Add initial RAS support
- MQD fixes

ttm:
- Unify DRM_FILE_PAGE_OFFSET handling
- Account for kernel allocations in kernel zone only
- Misc cleanups

The following changes since commit fbac3c48fa6b4cfa43eaae39d5a53269bff7ec5f:

  Merge branch 'drm-next-5.1' of git://people.freedesktop.org/~agd5f/linux into 
drm-next (2019-02-22 15:56:42 +1000)

are available in the Git repository at:

  git://people.freedesktop.org/~agd5f/linux drm-next-5.2

for you to fetch changes up to 296bb163e2d549c0fcae307e6aff1e407bb1a998:

  Revert "drm/amdgpu: use HMM callback to replace mmu notifier" (2019-03-28 
10:16:12 -0500)


Alex Deucher (12):
  drm/amdgpu/powerplay: add missing breaks in polaris10_smumgr
  drm/amdgpu/powerplay: add BACO support for vega12
  drm/amdgpu/powerplay: split out common smu9 BACO code
  drm/amdgpu: use BACO on vega12 if platform supports it
  drm/amdgpu/display: fix build when DCN KCONFIG is not set
  Revert "drm/amdgpu: more descriptive message if HMM not enabled"
  Revert "drm/amdgpu: support userptr cross VMAs case with HMM"
  Revert "drm/amdkfd: support concurrent userptr update for HMM"
  Revert "drm/amdgpu: fix HMM config dependency issue"
  Revert "drm/amdgpu: replace get_user_pages with HMM mirror helpers"
  Revert "drm/amdkfd: avoid HMM change cause circular lock"
  Revert "drm/amdgpu: use HMM callback to replace mmu notifier"

Andrey Grodzovsky (2):
  drm/amdgpu: Add sysfs entries  for xgmi hive v2.
  drm/amdgpu: Move IB pool init and fini v2

Anthony Koo (5):
  drm/amd/display: Fix issue with link_active state not correct for MST
  drm/amd/display: make seamless boot work generically
  drm/amd/display: Fix exception from AUX acquire failure
  drm/amd/display: Keep clocks high before seamless boot done
  drm/amd/display: Fix soft hang issue when some DPCD data invalid

Aric Cyr (6):
  drm/amd/display: 3.2.20
  drm/amd/display: 3.2.21
  drm/amd/display: Add a hysteresis to BTR frame multiplier
  drm/amd/display: 3.2.22
  drm/amd/display: 3.2.23
  drm/amd/display: 3.2.24

Candice Li (1):
  Revert "drm/amdgpu: use BACO reset on vega20 if platform support"

Charlene Liu (6):
  drm/amd/display: Add disable triple buffering DC debug option
  drm/amd/display: dcn add check surface in_use
  Revert "drm/amd/display: dcn add check surface in_use"
  drm/amd/display: Add pp_smu null pointer check
  drm/amd/display: add HW i2c arbitration with dmcu
  drm/amd/display: fix DP 422 VID_M half the rate issue.

Chengming Gui (21):
  drm/amd/powerplay: implement power_dpm_state sys interface for SMU11
  drm/amd/powerplay: add watermarks related data structs and function for 
SMU11.
  drm/amd/powerplay: implement pp_power_profile_mode sys inerface for SMU11
  drm/amd/powerplay: add display_config to handle display config for SMU11.
  drm/amd/powerplay: add mclk_latency_table struct and smu_clocks struct 
for SMU11
  drm/amd/powerplay: add enable_umd_pstate functions for SMU11
  drm/amd/powerplay: add get_profiling_clk_mask functions for SMU11
  drm/amd/powerplay: add set_uclk_to_highest_level for SMU11
  drm/amd/powerplay: add display_config_changed for SMU11.
  drm/amd/powerplay: add apply_clock_adjust_rules for SMU11.
  drm/amd/powerplay: add vega20_notify_smc_display_config functions for 
SMU11
  drm/amd/powerplay: add vega20_find/force_higest/lowest_dpm for SMU11 (v2)
  drm/amd/powerplay: add vega20_unforce_dpm_levels for SMU11.
  drm/amd/powerplay: implement power_dpm_force_performance_level for SMU11
  drm/amd/powerplay: implement power1_cap and power1_cap_max interface for 
SMU11 (v2)
  drm/amd/powerplay: add STABLE_PSTATE_SCLK and STABLE_PSTATE_MCLK when 
read sensor for SMU11
  drm/amd/powerplay: implement pwm1 hwmon interface for SMU11 (v2)
  drm/amd/powerplay: implement pwm1_enable hwmon interface for SMU11 (v2)
  drm/amd/powerplay: implement fan1_enable hwmon interface for SMU11 (v2)
  drm/amd/powerplay: add smu_late_init for SMU11.
  drm/amd/powerplay: add is_dpm_running for SMU11

Christian König (33):
  drm/amdgpu: clear PDs/PTs only after initializing them
  drm/amdgpu: reroute VMC and UMD to IH ring 1
  d

[PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Liu, Shaoyun]

Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for 
peer device

Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ee8d7a..82dc2b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -31,6 +31,7 @@
 #include 
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_xgmi.h"
 
 void amdgpu_gem_object_free(struct drm_gem_object *gobj)
 {
@@ -666,6 +667,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
struct amdgpu_device *adev = dev->dev_private;
struct drm_amdgpu_gem_op *args = data;
struct drm_gem_object *gobj;
+   struct amdgpu_vm_bo_base *base;
+   struct amdgpu_bo_va *bo_va;
struct amdgpu_bo *robj;
int r;
 
@@ -704,6 +707,16 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
amdgpu_bo_unreserve(robj);
break;
}
+   for (base = robj->vm_bo; base; base = base->next) {
+   bo_va = container_of(base, struct amdgpu_bo_va, base);
+   if (bo_va &&
+   
amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev))) {
+   r = -EINVAL;
+   amdgpu_bo_unreserve(robj);
+   goto out;
+   }
+   }
+
robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
|
AMDGPU_GEM_DOMAIN_GTT |
AMDGPU_GEM_DOMAIN_CPU);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 3d221f0..eb242a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
amdgpu_device *adev,
INIT_LIST_HEAD(&bo_va->valids);
INIT_LIST_HEAD(&bo_va->invalids);
 
-   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
+   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
+   (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)) {
bo_va->is_xgmi = true;
mutex_lock(&adev->vm_manager.lock_pstate);
/* Power up XGMI if it can be potentially used */
-- 
2.7.4

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

Re: [PATCH v13 04/20] mm, arm64: untag user pointers passed to memory syscalls

[Steven Rostedt]

On Thu, 28 Mar 2019 19:10:07 +0100
Andrey Konovalov  wrote:

> > > Signed-off-by: Andrey Konovalov 
> > > ---
> > >  ipc/shm.c  | 2 ++
> > >  mm/madvise.c   | 2 ++
> > >  mm/mempolicy.c | 5 +
> > >  mm/migrate.c   | 1 +
> > >  mm/mincore.c   | 2 ++
> > >  mm/mlock.c | 5 +
> > >  mm/mmap.c  | 7 +++
> > >  mm/mprotect.c  | 1 +
> > >  mm/mremap.c| 2 ++
> > >  mm/msync.c | 2 ++
> > >  10 files changed, 29 insertions(+)  
> >
> > I wonder whether it's better to keep these as wrappers in the arm64
> > code.  
> 
> I don't think I understand what you propose, could you elaborate?

I believe Catalin is saying that instead of placing things like:

@@ -1593,6 +1593,7 @@ SYSCALL_DEFINE3(shmat, int, shmid, char __user *, 
shmaddr, int, shmflg)
unsigned long ret;
long err;
 
+   shmaddr = untagged_addr(shmaddr);

To instead have the shmaddr set to the untagged_addr() before calling
the system call, and passing the untagged addr to the system call, as
that goes through the arm64 architecture specific code first.

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

Re: [PATCH v13 04/20] mm, arm64: untag user pointers passed to memory syscalls

[Andrey Konovalov]

On Fri, Mar 22, 2019 at 12:44 PM Catalin Marinas
 wrote:
>
> On Wed, Mar 20, 2019 at 03:51:18PM +0100, Andrey Konovalov wrote:
> > This patch is a part of a series that extends arm64 kernel ABI to allow to
> > pass tagged user pointers (with the top byte set to something else other
> > than 0x00) as syscall arguments.
> >
> > This patch allows tagged pointers to be passed to the following memory
> > syscalls: madvise, mbind, get_mempolicy, mincore, mlock, mlock2, brk,
> > mmap_pgoff, old_mmap, munmap, remap_file_pages, mprotect, pkey_mprotect,
> > mremap, msync and shmdt.
> >
> > This is done by untagging pointers passed to these syscalls in the
> > prologues of their handlers.
> >
> > Signed-off-by: Andrey Konovalov 
> > ---
> >  ipc/shm.c  | 2 ++
> >  mm/madvise.c   | 2 ++
> >  mm/mempolicy.c | 5 +
> >  mm/migrate.c   | 1 +
> >  mm/mincore.c   | 2 ++
> >  mm/mlock.c | 5 +
> >  mm/mmap.c  | 7 +++
> >  mm/mprotect.c  | 1 +
> >  mm/mremap.c| 2 ++
> >  mm/msync.c | 2 ++
> >  10 files changed, 29 insertions(+)
>
> I wonder whether it's better to keep these as wrappers in the arm64
> code.

I don't think I understand what you propose, could you elaborate?
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Kuehling, Felix]

On 2019-03-28 1:55 p.m., Liu, Shaoyun wrote:
> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for 
> peer device
>
> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
> Signed-off-by: shaoyunl 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
>   2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 9ee8d7a..82dc2b6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -31,6 +31,7 @@
>   #include 
>   #include "amdgpu.h"
>   #include "amdgpu_display.h"
> +#include "amdgpu_xgmi.h"
>   
>   void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>   {
> @@ -666,6 +667,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void 
> *data,
>   struct amdgpu_device *adev = dev->dev_private;
>   struct drm_amdgpu_gem_op *args = data;
>   struct drm_gem_object *gobj;
> + struct amdgpu_vm_bo_base *base;
> + struct amdgpu_bo_va *bo_va;
>   struct amdgpu_bo *robj;
>   int r;
>   
> @@ -704,6 +707,16 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void 
> *data,
>   amdgpu_bo_unreserve(robj);
>   break;
>   }
> + for (base = robj->vm_bo; base; base = base->next) {
> + bo_va = container_of(base, struct amdgpu_bo_va, base);
> + if (bo_va &&
> + 
> amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev))) {

adev and robj->tbo.bdev are the same in each loop iteration. Shouldn't 
we get one of the devices from the bo_va or amdgpu_vm_bo_base?

Regards,
   Felix


> + r = -EINVAL;
> + amdgpu_bo_unreserve(robj);
> + goto out;
> + }
> + }
> +
>   robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
> |
>   AMDGPU_GEM_DOMAIN_GTT |
>   AMDGPU_GEM_DOMAIN_CPU);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 3d221f0..eb242a1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
> amdgpu_device *adev,
>   INIT_LIST_HEAD(&bo_va->valids);
>   INIT_LIST_HEAD(&bo_va->invalids);
>   
> - if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
> + if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
> + (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)) {
>   bo_va->is_xgmi = true;
>   mutex_lock(&adev->vm_manager.lock_pstate);
>   /* Power up XGMI if it can be potentially used */
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: don't put the root PD into the relocated list

[Kuehling, Felix]

The change looks reasonable to me. Acked-by: Felix Kuehling 


I just don't understand why the root PD is special and handled 
differently from other PDs and PTs.

Regards,
   Felix

On 2019-03-27 6:39 a.m., Christian König wrote:
> Instead of skipping the root PD while processing the relocated list just never
> put it on the list in the first place.
>
> This avoids walking the list all together when the root PD is the only entry
> and so also avoids trying to submit a zero sized IB to the SDMA.
>
> Signed-off-by: Christian König 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 18 --
>   1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index af1a7020c3ab..5f615d63e2e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -305,7 +305,7 @@ static void amdgpu_vm_bo_base_init(struct 
> amdgpu_vm_bo_base *base,
>   return;
>   
>   vm->bulk_moveable = false;
> - if (bo->tbo.type == ttm_bo_type_kernel)
> + if (bo->tbo.type == ttm_bo_type_kernel && bo->parent)
>   amdgpu_vm_bo_relocated(base);
>   else
>   amdgpu_vm_bo_idle(base);
> @@ -671,7 +671,10 @@ int amdgpu_vm_validate_pt_bos(struct amdgpu_device 
> *adev, struct amdgpu_vm *vm,
>   if (r)
>   break;
>   }
> - amdgpu_vm_bo_relocated(bo_base);
> + if (bo->parent)
> + amdgpu_vm_bo_relocated(bo_base);
> + else
> + amdgpu_vm_bo_idle(bo_base);
>   }
>   }
>   
> @@ -1184,16 +1187,15 @@ uint64_t amdgpu_vm_map_gart(const dma_addr_t 
> *pages_addr, uint64_t addr)
>*
>* @param: parameters for the update
>* @vm: requested vm
> - * @parent: parent directory
>* @entry: entry to update
>*
>* Makes sure the requested entry in parent is up to date.
>*/
>   static int amdgpu_vm_update_pde(struct amdgpu_vm_update_params *params,
>   struct amdgpu_vm *vm,
> - struct amdgpu_vm_pt *parent,
>   struct amdgpu_vm_pt *entry)
>   {
> + struct amdgpu_vm_pt *parent = amdgpu_vm_pt_parent(entry);
>   struct amdgpu_bo *bo = parent->base.bo, *pbo;
>   uint64_t pde, pt, flags;
>   unsigned level;
> @@ -1255,17 +1257,13 @@ int amdgpu_vm_update_directories(struct amdgpu_device 
> *adev,
>   return r;
>   
>   while (!list_empty(&vm->relocated)) {
> - struct amdgpu_vm_pt *pt, *entry;
> + struct amdgpu_vm_pt *entry;
>   
>   entry = list_first_entry(&vm->relocated, struct amdgpu_vm_pt,
>base.vm_status);
>   amdgpu_vm_bo_idle(&entry->base);
>   
> - pt = amdgpu_vm_pt_parent(entry);
> - if (!pt)
> - continue;
> -
> - r = amdgpu_vm_update_pde(¶ms, vm, pt, entry);
> + r = amdgpu_vm_update_pde(¶ms, vm, entry);
>   if (r)
>   goto error;
>   }
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Liu, Shaoyun]

I think we only care about the context device (adev) and the real device 
this bo been allocated(robj->tbo.bdev).  The bo_va or base  don't have  
the device pointer directly, it have  a pointer to bo which should be 
the  same  as robj here.  We  can move the same_hive  check out of the 
loop .

Regards

shaoyun.liu


On 2019-03-28 3:18 p.m., Kuehling, Felix wrote:
> On 2019-03-28 1:55 p.m., Liu, Shaoyun wrote:
>> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory 
>> for peer device
>>
>> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
>> Signed-off-by: shaoyunl 
>> ---
>>drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +
>>drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
>>2 files changed, 15 insertions(+), 1 deletion(-)
>> I think we only care about the context device (adev)  and  the real device 
>> this bo been allocated ,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 9ee8d7a..82dc2b6 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -31,6 +31,7 @@
>>#include 
>>#include "amdgpu.h"
>>#include "amdgpu_display.h"
>> +#include "amdgpu_xgmi.h"
>>
>>void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>>{
>> @@ -666,6 +667,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void 
>> *data,
>>  struct amdgpu_device *adev = dev->dev_private;
>>  struct drm_amdgpu_gem_op *args = data;
>>  struct drm_gem_object *gobj;
>> +struct amdgpu_vm_bo_base *base;
>> +struct amdgpu_bo_va *bo_va;
>>  struct amdgpu_bo *robj;
>>  int r;
>>
>> @@ -704,6 +707,16 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void 
>> *data,
>>  amdgpu_bo_unreserve(robj);
>>  break;
>>  }
>> +for (base = robj->vm_bo; base; base = base->next) {
>> +bo_va = container_of(base, struct amdgpu_bo_va, base);
>> +if (bo_va &&
>> +
>> amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev))) {
> adev and robj->tbo.bdev are the same in each loop iteration. Shouldn't
> we get one of the devices from the bo_va or amdgpu_vm_bo_base?
>
> Regards,
>     Felix
>
>
>> +r = -EINVAL;
>> +amdgpu_bo_unreserve(robj);
>> +goto out;
>> +}
>> +}
>> +
>>  robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
>> |
>>  AMDGPU_GEM_DOMAIN_GTT |
>>  AMDGPU_GEM_DOMAIN_CPU);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 3d221f0..eb242a1 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
>> amdgpu_device *adev,
>>  INIT_LIST_HEAD(&bo_va->valids);
>>  INIT_LIST_HEAD(&bo_va->invalids);
>>
>> -if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
>> +if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
>> +(bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)) {
>>  bo_va->is_xgmi = true;
>>  mutex_lock(&adev->vm_manager.lock_pstate);
>>  /* Power up XGMI if it can be potentially used */
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Liu, Shaoyun]

Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for 
peer device

Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ee8d7a..7716ada 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -31,6 +31,7 @@
 #include 
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_xgmi.h"
 
 void amdgpu_gem_object_free(struct drm_gem_object *gobj)
 {
@@ -666,6 +667,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
struct amdgpu_device *adev = dev->dev_private;
struct drm_amdgpu_gem_op *args = data;
struct drm_gem_object *gobj;
+   struct amdgpu_vm_bo_base *base;
+   struct amdgpu_bo_va *bo_va;
struct amdgpu_bo *robj;
int r;
 
@@ -704,6 +707,16 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
amdgpu_bo_unreserve(robj);
break;
}
+   if (amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev)))
+   for (base = robj->vm_bo; base; base = base->next) {
+   bo_va = container_of(base, struct amdgpu_bo_va, 
base);
+   if (bo_va) {
+   r = -EINVAL;
+   amdgpu_bo_unreserve(robj);
+   goto out;
+   }
+   }
+
robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
|
AMDGPU_GEM_DOMAIN_GTT |
AMDGPU_GEM_DOMAIN_CPU);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 3d221f0..eb242a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
amdgpu_device *adev,
INIT_LIST_HEAD(&bo_va->valids);
INIT_LIST_HEAD(&bo_va->invalids);
 
-   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
+   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
+   (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)) {
bo_va->is_xgmi = true;
mutex_lock(&adev->vm_manager.lock_pstate);
/* Power up XGMI if it can be potentially used */
-- 
2.7.4

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

Re: [PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Kuehling, Felix]

On 2019-03-28 3:47 p.m., Liu, Shaoyun wrote:
> I think we only care about the context device (adev) and the real device
> this bo been allocated(robj->tbo.bdev).  The bo_va or base  don't have
> the device pointer directly, it have  a pointer to bo which should be
> the  same  as robj here.  We  can move the same_hive  check out of the
> loop .

That doesn't make sense. The "same hive" check only makes sense if one 
of the devices is the one where the memory is physically located 
(robj->tbo.bdev), and the other one is where the memory is accessed 
from. That only makes sense inside the loop. The amdgpu_vm_bo_base 
should tell you the device that's mapping and potentially accessing the 
memory over XGMI. You could get it like this:

     mapping_adev = base->vm->root.base.bo->tbo.bdev;

Regards,
   Felix


>
> Regards
>
> shaoyun.liu
>
>
> On 2019-03-28 3:18 p.m., Kuehling, Felix wrote:
>> On 2019-03-28 1:55 p.m., Liu, Shaoyun wrote:
>>> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory 
>>> for peer device
>>>
>>> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
>>> Signed-off-by: shaoyunl 
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
>>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>> I think we only care about the context device (adev)  and  the real device 
>>> this bo been allocated ,
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> index 9ee8d7a..82dc2b6 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> @@ -31,6 +31,7 @@
>>> #include 
>>> #include "amdgpu.h"
>>> #include "amdgpu_display.h"
>>> +#include "amdgpu_xgmi.h"
>>> 
>>> void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>>> {
>>> @@ -666,6 +667,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void 
>>> *data,
>>> struct amdgpu_device *adev = dev->dev_private;
>>> struct drm_amdgpu_gem_op *args = data;
>>> struct drm_gem_object *gobj;
>>> +   struct amdgpu_vm_bo_base *base;
>>> +   struct amdgpu_bo_va *bo_va;
>>> struct amdgpu_bo *robj;
>>> int r;
>>> 
>>> @@ -704,6 +707,16 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void 
>>> *data,
>>> amdgpu_bo_unreserve(robj);
>>> break;
>>> }
>>> +   for (base = robj->vm_bo; base; base = base->next) {
>>> +   bo_va = container_of(base, struct amdgpu_bo_va, base);
>>> +   if (bo_va &&
>>> +   
>>> amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev))) {
>> adev and robj->tbo.bdev are the same in each loop iteration. Shouldn't
>> we get one of the devices from the bo_va or amdgpu_vm_bo_base?
>>
>> Regards,
>>  Felix
>>
>>
>>> +   r = -EINVAL;
>>> +   amdgpu_bo_unreserve(robj);
>>> +   goto out;
>>> +   }
>>> +   }
>>> +
>>> robj->preferred_domains = args->value & 
>>> (AMDGPU_GEM_DOMAIN_VRAM |
>>> 
>>> AMDGPU_GEM_DOMAIN_GTT |
>>> 
>>> AMDGPU_GEM_DOMAIN_CPU);
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 3d221f0..eb242a1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
>>> amdgpu_device *adev,
>>> INIT_LIST_HEAD(&bo_va->valids);
>>> INIT_LIST_HEAD(&bo_va->invalids);
>>> 
>>> -   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
>>> +   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
>>> +   (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)) {
>>> bo_va->is_xgmi = true;
>>> mutex_lock(&adev->vm_manager.lock_pstate);
>>> /* Power up XGMI if it can be potentially used */
>> ___
>> amd-gfx mailing list
>> amd-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Liu, Shaoyun]

Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for 
peer device

Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ee8d7a..7716ada 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -31,6 +31,7 @@
 #include 
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_xgmi.h"
 
 void amdgpu_gem_object_free(struct drm_gem_object *gobj)
 {
@@ -666,6 +667,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
struct amdgpu_device *adev = dev->dev_private;
struct drm_amdgpu_gem_op *args = data;
struct drm_gem_object *gobj;
+   struct amdgpu_vm_bo_base *base;
+   struct amdgpu_bo_va *bo_va;
struct amdgpu_bo *robj;
int r;
 
@@ -704,6 +707,16 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
amdgpu_bo_unreserve(robj);
break;
}
+   if (amdgpu_xgmi_same_hive(adev,amdgpu_ttm_adev(robj->tbo.bdev)))
+   for (base = robj->vm_bo; base; base = base->next) {
+   bo_va = container_of(base, struct amdgpu_bo_va, 
base);
+   if (bo_va) {
+   r = -EINVAL;
+   amdgpu_bo_unreserve(robj);
+   goto out;
+   }
+   }
+
robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
|
AMDGPU_GEM_DOMAIN_GTT |
AMDGPU_GEM_DOMAIN_CPU);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 3d221f0..eb242a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
amdgpu_device *adev,
INIT_LIST_HEAD(&bo_va->valids);
INIT_LIST_HEAD(&bo_va->invalids);
 
-   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
+   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
+   (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)) {
bo_va->is_xgmi = true;
mutex_lock(&adev->vm_manager.lock_pstate);
/* Power up XGMI if it can be potentially used */
-- 
2.7.4

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

[PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Liu, Shaoyun]

Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for 
peer device

Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 14 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ee8d7a..6a076d7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -31,6 +31,7 @@
 #include 
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_xgmi.h"
 
 void amdgpu_gem_object_free(struct drm_gem_object *gobj)
 {
@@ -666,6 +667,8 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
struct amdgpu_device *adev = dev->dev_private;
struct drm_amdgpu_gem_op *args = data;
struct drm_gem_object *gobj;
+   struct amdgpu_vm_bo_base *base;
+   struct amdgpu_bo_va *bo_va;
struct amdgpu_bo *robj;
int r;
 
@@ -704,6 +707,17 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
amdgpu_bo_unreserve(robj);
break;
}
+   for (base = robj->vm_bo; base; base = base->next) {
+   bo_va = container_of(base, struct amdgpu_bo_va, base);
+   if (bo_va &&
+   
amdgpu_xgmi_same_hive(amdgpu_ttm_adev(robj->tbo.bdev),
+   
amdgpu_ttm_adev(bo_va->base.vm->root.base.bo->tbo.bdev))) {
+   r = -EINVAL;
+   amdgpu_bo_unreserve(robj);
+   goto out;
+   }
+   }
+
robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
|
AMDGPU_GEM_DOMAIN_GTT |
AMDGPU_GEM_DOMAIN_CPU);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 3d221f0..eb242a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
amdgpu_device *adev,
INIT_LIST_HEAD(&bo_va->valids);
INIT_LIST_HEAD(&bo_va->invalids);
 
-   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
+   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
+   (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)) {
bo_va->is_xgmi = true;
mutex_lock(&adev->vm_manager.lock_pstate);
/* Power up XGMI if it can be potentially used */
-- 
2.7.4

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

[PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Liu, Shaoyun]

Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for 
peer device

Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
Signed-off-by: shaoyunl 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 11 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 9ee8d7a..7b84036 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -31,6 +31,7 @@
 #include 
 #include "amdgpu.h"
 #include "amdgpu_display.h"
+#include "amdgpu_xgmi.h"
 
 void amdgpu_gem_object_free(struct drm_gem_object *gobj)
 {
@@ -666,6 +667,7 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
struct amdgpu_device *adev = dev->dev_private;
struct drm_amdgpu_gem_op *args = data;
struct drm_gem_object *gobj;
+   struct amdgpu_vm_bo_base *base;
struct amdgpu_bo *robj;
int r;
 
@@ -704,6 +706,15 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void *data,
amdgpu_bo_unreserve(robj);
break;
}
+   for (base = robj->vm_bo; base; base = base->next)
+   if 
(amdgpu_xgmi_same_hive(amdgpu_ttm_adev(robj->tbo.bdev),
+   
amdgpu_ttm_adev(base->vm->root.base.bo->tbo.bdev))) {
+   r = -EINVAL;
+   amdgpu_bo_unreserve(robj);
+   goto out;
+   }
+
+
robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
|
AMDGPU_GEM_DOMAIN_GTT |
AMDGPU_GEM_DOMAIN_CPU);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 3d221f0..eb242a1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
amdgpu_device *adev,
INIT_LIST_HEAD(&bo_va->valids);
INIT_LIST_HEAD(&bo_va->invalids);
 
-   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
+   if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
+   (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)) {
bo_va->is_xgmi = true;
mutex_lock(&adev->vm_manager.lock_pstate);
/* Power up XGMI if it can be potentially used */
-- 
2.7.4

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

Re: [PATCH] drm/amdgpu: Add preferred_domain check when determine XGMI state

[Kuehling, Felix]

On 2019-03-28 4:38 p.m., Liu, Shaoyun wrote:
> Avoid unnecessary XGMI hight pstate trigger when mapping none-vram memory for 
> peer device
>
> Change-Id: I1881deff3da19f1f4b58d5765db03a590092a5b2
> Signed-off-by: shaoyunl 

This patch is Reviewed-by: Felix Kuehling 

Please also give Christian a chance to review this one before you submit.

Thanks,
   Felix


> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 11 +++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c  |  3 ++-
>   2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index 9ee8d7a..7b84036 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -31,6 +31,7 @@
>   #include 
>   #include "amdgpu.h"
>   #include "amdgpu_display.h"
> +#include "amdgpu_xgmi.h"
>   
>   void amdgpu_gem_object_free(struct drm_gem_object *gobj)
>   {
> @@ -666,6 +667,7 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void 
> *data,
>   struct amdgpu_device *adev = dev->dev_private;
>   struct drm_amdgpu_gem_op *args = data;
>   struct drm_gem_object *gobj;
> + struct amdgpu_vm_bo_base *base;
>   struct amdgpu_bo *robj;
>   int r;
>   
> @@ -704,6 +706,15 @@ int amdgpu_gem_op_ioctl(struct drm_device *dev, void 
> *data,
>   amdgpu_bo_unreserve(robj);
>   break;
>   }
> + for (base = robj->vm_bo; base; base = base->next)
> + if 
> (amdgpu_xgmi_same_hive(amdgpu_ttm_adev(robj->tbo.bdev),
> + 
> amdgpu_ttm_adev(base->vm->root.base.bo->tbo.bdev))) {
> + r = -EINVAL;
> + amdgpu_bo_unreserve(robj);
> + goto out;
> + }
> +
> +
>   robj->preferred_domains = args->value & (AMDGPU_GEM_DOMAIN_VRAM 
> |
>   AMDGPU_GEM_DOMAIN_GTT |
>   AMDGPU_GEM_DOMAIN_CPU);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index 3d221f0..eb242a1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2025,7 +2025,8 @@ struct amdgpu_bo_va *amdgpu_vm_bo_add(struct 
> amdgpu_device *adev,
>   INIT_LIST_HEAD(&bo_va->valids);
>   INIT_LIST_HEAD(&bo_va->invalids);
>   
> - if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev))) {
> + if (bo && amdgpu_xgmi_same_hive(adev, amdgpu_ttm_adev(bo->tbo.bdev)) &&
> + (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM)) {
>   bo_va->is_xgmi = true;
>   mutex_lock(&adev->vm_manager.lock_pstate);
>   /* Power up XGMI if it can be potentially used */
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [pull] amdgpu, amdkfd, ttm drm-next-5.2

[Dave Airlie]

On Fri, 29 Mar 2019 at 03:44, Alex Deucher  wrote:
>
> Hi Dave, Daniel,
>
> New stuff for 5.2:

32-bit arm build:

/home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:
In function ‘smu_v11_0_notify_memory_pool_location’:
/home/airlied/devel/kernel/dim/src/drivers/gpu/drm/amd/amdgpu/../powerplay/smu_v11_0.c:530:12:
warning: cast from pointer to integer of different size
[-Wpointer-to-int-cast]

Please fix and resend.

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