RE: [PATCH] drm/amd/powerplay: fix message of SetHardMinByFreq failed when feature is disabled

[Quan, Evan]

Reviewed-by: Evan Quan 

> -Original Message-
> From: amd-gfx  On Behalf Of
> Wang, Kevin(Yang)
> Sent: Tuesday, August 13, 2019 10:34 AM
> To: dl.srdc_lnx_nv10 
> Cc: Wang, Kevin(Yang) ; Huang, Ray
> ; Feng, Kenneth ; amd-
> g...@lists.freedesktop.org
> Subject: [PATCH] drm/amd/powerplay: fix message of SetHardMinByFreq
> failed when feature is disabled
> 
> the direct send message to smc to set hard clokc will failed
> when smc clock dpm feature is disabled.
> so use function of smu_set_hard_freq_range to replace it.
> the function will check feature enablement.
> 
> eg: when uclk (mclk) dpm feature is disabled on navi10
> [  300.675901] amdgpu: [powerplay] failed send message:
> SetHardMinByFreq(28)
>    param: 0x00020064 response 0xfffb
> 
> Signed-off-by: Kevin Wang 
> ---
>  drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 10 +-
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> index 3a081acdf1a8..3a49cabf726f 100644
> --- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> +++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
> @@ -1306,16 +1306,8 @@ smu_v11_0_display_clock_voltage_request(struct
> smu_context *smu,
>   if (clk_select == SMU_UCLK && smu->disable_uclk_switch)
>   return 0;
> 
> - clk_id = smu_clk_get_index(smu, clk_select);
> - if (clk_id < 0) {
> - ret = -EINVAL;
> - goto failed;
> - }
> -
> -
>   mutex_lock(>mutex);
> - ret = smu_send_smc_msg_with_param(smu,
> SMU_MSG_SetHardMinByFreq,
> - (clk_id << 16) | clk_freq);
> + ret = smu_set_hard_freq_range(smu, clk_select, 0, clk_freq);
>   mutex_unlock(>mutex);
> 
>   if(clk_select == SMU_UCLK)
> --
> 2.22.0
> 
> ___
> 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/amd/powerplay: add arcturus_is_dpm_running function for arcturus

[Quan, Evan]

OK, I see. That will be fine.

Regards,
Evan

> -Original Message-
> From: Gui, Jack 
> Sent: Monday, August 12, 2019 5:51 PM
> To: Quan, Evan ; amd-gfx@lists.freedesktop.org
> Subject: RE: [PATCH 2/2] drm/amd/powerplay: add
> arcturus_is_dpm_running function for arcturus
> 
> Hi Evan,
> 
> All supported feature can be set there, Anyone of these features is running,
> we can judge dpm is running.
> 
> BR,
> Jack Gui
> 
> -Original Message-
> From: Quan, Evan 
> Sent: Monday, August 12, 2019 5:39 PM
> To: Gui, Jack ; amd-gfx@lists.freedesktop.org
> Cc: Gui, Jack 
> Subject: RE: [PATCH 2/2] drm/amd/powerplay: add
> arcturus_is_dpm_running function for arcturus
> 
> Please set FEATURE_DPM_PREFETCHER_MASK |
> FEATURE_DPM_GFXCLK_MASK only. For now, only these two are enabled
> on arcturus.
> 
> With that fixed, the patch is reviewed-by: Evan Quan 
> > -Original Message-
> > From: amd-gfx  On Behalf Of
> > Chengming Gui
> > Sent: Monday, August 12, 2019 4:22 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Gui, Jack 
> > Subject: [PATCH 2/2] drm/amd/powerplay: add arcturus_is_dpm_running
> > function for arcturus
> >
> > add arcturus_is_dpm_running function
> >
> > Signed-off-by: Chengming Gui 
> > ---
> >  drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 21
> > +
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > index 03ce871..9107beb 100644
> > --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> > @@ -51,6 +51,15 @@
> >  #define SMU_FEATURES_HIGH_MASK   0x
> >  #define SMU_FEATURES_HIGH_SHIFT  32
> >
> > +#define SMC_DPM_FEATURE ( \
> > +   FEATURE_DPM_PREFETCHER_MASK | \
> > +   FEATURE_DPM_GFXCLK_MASK | \
> > +   FEATURE_DPM_UCLK_MASK | \
> > +   FEATURE_DPM_SOCCLK_MASK | \
> > +   FEATURE_DPM_MP0CLK_MASK | \
> > +   FEATURE_DPM_FCLK_MASK | \
> > +   FEATURE_DPM_XGMI_MASK)
> > +
> >  /* possible frequency drift (1Mhz) */
> >  #define EPSILON1
> >
> > @@ -1873,6 +1882,17 @@ static void arcturus_dump_pptable(struct
> > smu_context *smu)
> >
> >  }
> >
> > +static bool arcturus_is_dpm_running(struct smu_context *smu) {
> > +   int ret = 0;
> > +   uint32_t feature_mask[2];
> > +   unsigned long feature_enabled;
> > +   ret = smu_feature_get_enabled_mask(smu, feature_mask, 2);
> > +   feature_enabled = (unsigned long)((uint64_t)feature_mask[0] |
> > +  ((uint64_t)feature_mask[1] << 32));
> > +   return !!(feature_enabled & SMC_DPM_FEATURE); }
> > +
> >  static const struct pptable_funcs arcturus_ppt_funcs = {
> > /* translate smu index into arcturus specific index */
> > .get_smu_msg_index = arcturus_get_smu_msg_index, @@ -1910,6
> > +1930,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
> > /* debug (internal used) */
> > .dump_pptable = arcturus_dump_pptable,
> > .get_power_limit = arcturus_get_power_limit,
> > +   .is_dpm_running = arcturus_is_dpm_running,
> >  };
> >
> >  void arcturus_set_ppt_funcs(struct smu_context *smu)
> > --
> > 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

[PATCH] drm/amd/powerplay: fix message of SetHardMinByFreq failed when feature is disabled

[Wang, Kevin(Yang)]

the direct send message to smc to set hard clokc will failed
when smc clock dpm feature is disabled.
so use function of smu_set_hard_freq_range to replace it.
the function will check feature enablement.

eg: when uclk (mclk) dpm feature is disabled on navi10
[  300.675901] amdgpu: [powerplay] failed send message: SetHardMinByFreq(28)
   param: 0x00020064 response 0xfffb

Signed-off-by: Kevin Wang 
---
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c 
b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index 3a081acdf1a8..3a49cabf726f 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -1306,16 +1306,8 @@ smu_v11_0_display_clock_voltage_request(struct 
smu_context *smu,
if (clk_select == SMU_UCLK && smu->disable_uclk_switch)
return 0;
 
-   clk_id = smu_clk_get_index(smu, clk_select);
-   if (clk_id < 0) {
-   ret = -EINVAL;
-   goto failed;
-   }
-
-
mutex_lock(>mutex);
-   ret = smu_send_smc_msg_with_param(smu, SMU_MSG_SetHardMinByFreq,
-   (clk_id << 16) | clk_freq);
+   ret = smu_set_hard_freq_range(smu, clk_select, 0, clk_freq);
mutex_unlock(>mutex);
 
if(clk_select == SMU_UCLK)
-- 
2.22.0

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

RE: [PATCH] SWDEV-197284 - drm/amdgpu: Only use the peek function in productor side is not correct

[Deng, Emily]

Ok, please ignore this patch.


Best wishes
Emily Deng

>-Original Message-
>From: Christian König 
>Sent: Tuesday, August 13, 2019 1:00 AM
>To: Deng, Emily ; amd-gfx@lists.freedesktop.org
>Subject: Re: [PATCH] SWDEV-197284 - drm/amdgpu: Only use the peek
>function in productor side is not correct
>
>Am 12.08.19 um 09:42 schrieb Emily Deng:
>> For spsc queue, use peek function would cause error in productor side.
>> As for the last element, when the push job is occurring during pop
>> job, the peek function will not be updated in time, and it will return NULL.
>>
>> So use queue count for double confirming the job queue is empty.
>
>For the upstream branch I'm going to push my patch which is not as invasive
>as this one.
>
>Christian.
>
>>
>> Signed-off-by: Emily Deng 
>> ---
>>   drivers/gpu/drm/scheduler/sched_entity.c | 4 ++--
>>   include/drm/spsc_queue.h | 7 +++
>>   2 files changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c
>> b/drivers/gpu/drm/scheduler/sched_entity.c
>> index 35ddbec..e74894f 100644
>> --- a/drivers/gpu/drm/scheduler/sched_entity.c
>> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
>> @@ -95,7 +95,7 @@ static bool drm_sched_entity_is_idle(struct
>drm_sched_entity *entity)
>>  rmb(); /* for list_empty to work without lock */
>>
>>  if (list_empty(>list) ||
>> -spsc_queue_peek(>job_queue) == NULL)
>> +((spsc_queue_peek(>job_queue) == NULL) &&
>> +!spsc_queue_count(>job_queue)))
>>  return true;
>>
>>  return false;
>> @@ -281,7 +281,7 @@ void drm_sched_entity_fini(struct drm_sched_entity
>*entity)
>>  /* Consumption of existing IBs wasn't completed. Forcefully
>>   * remove them here.
>>   */
>> -if (spsc_queue_peek(>job_queue)) {
>> +if ((spsc_queue_peek(>job_queue) == NULL) &&
>> +!spsc_queue_count(>job_queue)) {
>>  if (sched) {
>>  /* Park the kernel for a moment to make sure it isn't
>processing
>>   * our enity.
>> diff --git a/include/drm/spsc_queue.h b/include/drm/spsc_queue.h index
>> 125f096..78092b9 100644
>> --- a/include/drm/spsc_queue.h
>> +++ b/include/drm/spsc_queue.h
>> @@ -54,9 +54,8 @@ static inline void spsc_queue_init(struct spsc_queue
>> *queue)
>>
>>   static inline struct spsc_node *spsc_queue_peek(struct spsc_queue *queue)
>>   {
>> -return queue->head;
>> +return READ_ONCE(queue->head);
>>   }
>> -
>>   static inline int spsc_queue_count(struct spsc_queue *queue)
>>   {
>>  return atomic_read(>job_count); @@ -70,9 +69,9 @@ static
>> inline bool spsc_queue_push(struct spsc_queue *queue, struct spsc_node
>> *n
>>
>>  preempt_disable();
>>
>> +atomic_inc(>job_count);
>>  tail = (struct spsc_node **)atomic_long_xchg(>tail,
>(long)>next);
>>  WRITE_ONCE(*tail, node);
>> -atomic_inc(>job_count);
>>
>>  /*
>>   * In case of first element verify new node will be visible to the
>> consumer @@ -93,6 +92,7 @@ static inline struct spsc_node
>*spsc_queue_pop(struct spsc_queue *queue)
>>  /* Verify reading from memory and not the cache */
>>  smp_rmb();
>>
>> +atomic_dec(>job_count);
>>  node = READ_ONCE(queue->head);
>>
>>  if (!node)
>> @@ -113,7 +113,6 @@ static inline struct spsc_node
>*spsc_queue_pop(struct spsc_queue *queue)
>>  }
>>  }
>>
>> -atomic_dec(>job_count);
>>  return node;
>>   }
>>

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

Re: [PATCH] drm/amdgpu: Add more page fault info printing for GFX10

[Christian König]

Hi Yong,

that is an intentional differentiation.

The values behind the ":" are hardware values from the page faults 
interrupt vector.


All values printed without the ":" are additional things we get from the 
kernel task information based on the pasid.


Please keep that,
Christian.

Am 12.08.19 um 21:20 schrieb Zhao, Yong:

Hi Christian,

I feel with ":" it is better, because without it I found it is not easy
to interpret the printing. Moreover, it continues the format of the
former part.

It looks like this:

[  190.686668] amdgpu :03:00.0: [gfxhub0] retry page fault (src_id:0
ring:0 vmid:8 pasid:32771, for process:kfdtest pid:3273 thread:kfdtest
pid:3273)

vs without ":"

[  190.686668] amdgpu :03:00.0: [gfxhub0] retry page fault (src_id:0
ring:0 vmid:8 pasid:32771, for process kfdtest pid 3273 thread kfdtest
pid 3273)


If you are not convinced, I can change it to without ":".


Regards,

Yong

On 2019-08-12 3:12 p.m., Christian König wrote:

Am 12.08.19 um 21:05 schrieb Zhao, Yong:

The printing we did for GFX9 was not propogated to GFX10 somehow, so fix
it now.

Change-Id: Ic0b8381134340b83cd69c3fe186ac7a8a97b1bca
Signed-off-by: Yong Zhao 
---
   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 33 ++
   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  |  5 +++-
   2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 4e3ac1084a94..f23be98e9897 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -140,17 +140,40 @@ static int gmc_v10_0_process_interrupt(struct
amdgpu_device *adev,
   }
     if (printk_ratelimit()) {
+    struct amdgpu_task_info task_info;
+
+    memset(_info, 0, sizeof(struct amdgpu_task_info));
+    amdgpu_vm_get_task_info(adev, entry->pasid, _info);
+
   dev_err(adev->dev,
-    "[%s] VMC page fault (src_id:%u ring:%u vmid:%u
pasid:%u)\n",
+    "[%s] page fault (src_id:%u ring:%u vmid:%u pasid:%u, "
+    "for process:%s pid:%d thread:%s pid:%d)\n",
   entry->vmid_src ? "mmhub" : "gfxhub",
   entry->src_id, entry->ring_id, entry->vmid,
-    entry->pasid);
-    dev_err(adev->dev, "  at page 0x%016llx from %d\n",
+    entry->pasid, task_info.process_name, task_info.tgid,
+    task_info.task_name, task_info.pid);
+    dev_err(adev->dev, "  in page starting at address 0x%016llx
from client %d\n",
   addr, entry->client_id);
-    if (!amdgpu_sriov_vf(adev))
+    if (!amdgpu_sriov_vf(adev)) {
   dev_err(adev->dev,
-    "VM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
+    "GCVM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
   status);
+    dev_err(adev->dev, "\t MORE_FAULTS: 0x%lx\n",
+    REG_GET_FIELD(status,
+    GCVM_L2_PROTECTION_FAULT_STATUS, MORE_FAULTS));
+    dev_err(adev->dev, "\t WALKER_ERROR: 0x%lx\n",
+    REG_GET_FIELD(status,
+    GCVM_L2_PROTECTION_FAULT_STATUS, WALKER_ERROR));
+    dev_err(adev->dev, "\t PERMISSION_FAULTS: 0x%lx\n",
+    REG_GET_FIELD(status,
+    GCVM_L2_PROTECTION_FAULT_STATUS, PERMISSION_FAULTS));
+    dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
+    REG_GET_FIELD(status,
+    GCVM_L2_PROTECTION_FAULT_STATUS, MAPPING_ERROR));
+    dev_err(adev->dev, "\t RW: 0x%lx\n",
+    REG_GET_FIELD(status,
+    GCVM_L2_PROTECTION_FAULT_STATUS, RW));
+    }
   }
     return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 296e2d982578..34c4c2d08550 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -364,7 +364,7 @@ static int gmc_v9_0_process_interrupt(struct
amdgpu_device *adev,
     dev_err(adev->dev,
   "[%s] %s page fault (src_id:%u ring:%u vmid:%u "
-    "pasid:%u, for process %s pid %d thread %s pid %d)\n",
+    "pasid:%u, for process:%s pid:%d thread:%s pid:%d)\n",

I think the text actually looks better without the ":".


   hub_name, retry_fault ? "retry" : "no-retry",
   entry->src_id, entry->ring_id, entry->vmid,
   entry->pasid, task_info.process_name, task_info.tgid,
@@ -387,6 +387,9 @@ static int gmc_v9_0_process_interrupt(struct
amdgpu_device *adev,
   dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
   REG_GET_FIELD(status,
   VM_L2_PROTECTION_FAULT_STATUS, MAPPING_ERROR));
+    dev_err(adev->dev, "\t RW: 0x%lx\n",
+    REG_GET_FIELD(status,
+    VM_L2_PROTECTION_FAULT_STATUS, RW));

That should probably be a separate patch since it is fixing gfx9.

Apart from that the patch looks good to me,
Christian.


     

Re: [PATCH] drm/amdgpu: Add more page fault info printing for GFX10

[Zhao, Yong]

Hi Christian,

I feel with ":" it is better, because without it I found it is not easy 
to interpret the printing. Moreover, it continues the format of the 
former part.

It looks like this:

[  190.686668] amdgpu :03:00.0: [gfxhub0] retry page fault (src_id:0 
ring:0 vmid:8 pasid:32771, for process:kfdtest pid:3273 thread:kfdtest 
pid:3273)

vs without ":"

[  190.686668] amdgpu :03:00.0: [gfxhub0] retry page fault (src_id:0 
ring:0 vmid:8 pasid:32771, for process kfdtest pid 3273 thread kfdtest 
pid 3273)


If you are not convinced, I can change it to without ":".


Regards,

Yong

On 2019-08-12 3:12 p.m., Christian König wrote:
> Am 12.08.19 um 21:05 schrieb Zhao, Yong:
>> The printing we did for GFX9 was not propogated to GFX10 somehow, so fix
>> it now.
>>
>> Change-Id: Ic0b8381134340b83cd69c3fe186ac7a8a97b1bca
>> Signed-off-by: Yong Zhao 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 33 ++
>>   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  |  5 +++-
>>   2 files changed, 32 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> index 4e3ac1084a94..f23be98e9897 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
>> @@ -140,17 +140,40 @@ static int gmc_v10_0_process_interrupt(struct 
>> amdgpu_device *adev,
>>   }
>>     if (printk_ratelimit()) {
>> +    struct amdgpu_task_info task_info;
>> +
>> +    memset(_info, 0, sizeof(struct amdgpu_task_info));
>> +    amdgpu_vm_get_task_info(adev, entry->pasid, _info);
>> +
>>   dev_err(adev->dev,
>> -    "[%s] VMC page fault (src_id:%u ring:%u vmid:%u 
>> pasid:%u)\n",
>> +    "[%s] page fault (src_id:%u ring:%u vmid:%u pasid:%u, "
>> +    "for process:%s pid:%d thread:%s pid:%d)\n",
>>   entry->vmid_src ? "mmhub" : "gfxhub",
>>   entry->src_id, entry->ring_id, entry->vmid,
>> -    entry->pasid);
>> -    dev_err(adev->dev, "  at page 0x%016llx from %d\n",
>> +    entry->pasid, task_info.process_name, task_info.tgid,
>> +    task_info.task_name, task_info.pid);
>> +    dev_err(adev->dev, "  in page starting at address 0x%016llx 
>> from client %d\n",
>>   addr, entry->client_id);
>> -    if (!amdgpu_sriov_vf(adev))
>> +    if (!amdgpu_sriov_vf(adev)) {
>>   dev_err(adev->dev,
>> -    "VM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
>> +    "GCVM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
>>   status);
>> +    dev_err(adev->dev, "\t MORE_FAULTS: 0x%lx\n",
>> +    REG_GET_FIELD(status,
>> +    GCVM_L2_PROTECTION_FAULT_STATUS, MORE_FAULTS));
>> +    dev_err(adev->dev, "\t WALKER_ERROR: 0x%lx\n",
>> +    REG_GET_FIELD(status,
>> +    GCVM_L2_PROTECTION_FAULT_STATUS, WALKER_ERROR));
>> +    dev_err(adev->dev, "\t PERMISSION_FAULTS: 0x%lx\n",
>> +    REG_GET_FIELD(status,
>> +    GCVM_L2_PROTECTION_FAULT_STATUS, PERMISSION_FAULTS));
>> +    dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
>> +    REG_GET_FIELD(status,
>> +    GCVM_L2_PROTECTION_FAULT_STATUS, MAPPING_ERROR));
>> +    dev_err(adev->dev, "\t RW: 0x%lx\n",
>> +    REG_GET_FIELD(status,
>> +    GCVM_L2_PROTECTION_FAULT_STATUS, RW));
>> +    }
>>   }
>>     return 0;
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> index 296e2d982578..34c4c2d08550 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
>> @@ -364,7 +364,7 @@ static int gmc_v9_0_process_interrupt(struct 
>> amdgpu_device *adev,
>>     dev_err(adev->dev,
>>   "[%s] %s page fault (src_id:%u ring:%u vmid:%u "
>> -    "pasid:%u, for process %s pid %d thread %s pid %d)\n",
>> +    "pasid:%u, for process:%s pid:%d thread:%s pid:%d)\n",
>
> I think the text actually looks better without the ":".
>
>>   hub_name, retry_fault ? "retry" : "no-retry",
>>   entry->src_id, entry->ring_id, entry->vmid,
>>   entry->pasid, task_info.process_name, task_info.tgid,
>> @@ -387,6 +387,9 @@ static int gmc_v9_0_process_interrupt(struct 
>> amdgpu_device *adev,
>>   dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
>>   REG_GET_FIELD(status,
>>   VM_L2_PROTECTION_FAULT_STATUS, MAPPING_ERROR));
>> +    dev_err(adev->dev, "\t RW: 0x%lx\n",
>> +    REG_GET_FIELD(status,
>> +    VM_L2_PROTECTION_FAULT_STATUS, RW));
>
> That should probably be a separate patch since it is fixing gfx9.
>
> Apart from that the patch looks good to me,
> Christian.
>
>>     }
>>   }
>
___
amd-gfx 

Re: [PATCH] drm/amdgpu: Add more page fault info printing for GFX10

[Christian König]

Am 12.08.19 um 21:05 schrieb Zhao, Yong:

The printing we did for GFX9 was not propogated to GFX10 somehow, so fix
it now.

Change-Id: Ic0b8381134340b83cd69c3fe186ac7a8a97b1bca
Signed-off-by: Yong Zhao 
---
  drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 33 ++
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  |  5 +++-
  2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 4e3ac1084a94..f23be98e9897 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -140,17 +140,40 @@ static int gmc_v10_0_process_interrupt(struct 
amdgpu_device *adev,
}
  
  	if (printk_ratelimit()) {

+   struct amdgpu_task_info task_info;
+
+   memset(_info, 0, sizeof(struct amdgpu_task_info));
+   amdgpu_vm_get_task_info(adev, entry->pasid, _info);
+
dev_err(adev->dev,
-   "[%s] VMC page fault (src_id:%u ring:%u vmid:%u 
pasid:%u)\n",
+   "[%s] page fault (src_id:%u ring:%u vmid:%u pasid:%u, "
+   "for process:%s pid:%d thread:%s pid:%d)\n",
entry->vmid_src ? "mmhub" : "gfxhub",
entry->src_id, entry->ring_id, entry->vmid,
-   entry->pasid);
-   dev_err(adev->dev, "  at page 0x%016llx from %d\n",
+   entry->pasid, task_info.process_name, task_info.tgid,
+   task_info.task_name, task_info.pid);
+   dev_err(adev->dev, "  in page starting at address 0x%016llx from 
client %d\n",
addr, entry->client_id);
-   if (!amdgpu_sriov_vf(adev))
+   if (!amdgpu_sriov_vf(adev)) {
dev_err(adev->dev,
-   "VM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
+   "GCVM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
status);
+   dev_err(adev->dev, "\t MORE_FAULTS: 0x%lx\n",
+   REG_GET_FIELD(status,
+   GCVM_L2_PROTECTION_FAULT_STATUS, MORE_FAULTS));
+   dev_err(adev->dev, "\t WALKER_ERROR: 0x%lx\n",
+   REG_GET_FIELD(status,
+   GCVM_L2_PROTECTION_FAULT_STATUS, WALKER_ERROR));
+   dev_err(adev->dev, "\t PERMISSION_FAULTS: 0x%lx\n",
+   REG_GET_FIELD(status,
+   GCVM_L2_PROTECTION_FAULT_STATUS, 
PERMISSION_FAULTS));
+   dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
+   REG_GET_FIELD(status,
+   GCVM_L2_PROTECTION_FAULT_STATUS, 
MAPPING_ERROR));
+   dev_err(adev->dev, "\t RW: 0x%lx\n",
+   REG_GET_FIELD(status,
+   GCVM_L2_PROTECTION_FAULT_STATUS, RW));
+   }
}
  
  	return 0;

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 296e2d982578..34c4c2d08550 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -364,7 +364,7 @@ static int gmc_v9_0_process_interrupt(struct amdgpu_device 
*adev,
  
  		dev_err(adev->dev,

"[%s] %s page fault (src_id:%u ring:%u vmid:%u "
-   "pasid:%u, for process %s pid %d thread %s pid %d)\n",
+   "pasid:%u, for process:%s pid:%d thread:%s pid:%d)\n",


I think the text actually looks better without the ":".


hub_name, retry_fault ? "retry" : "no-retry",
entry->src_id, entry->ring_id, entry->vmid,
entry->pasid, task_info.process_name, task_info.tgid,
@@ -387,6 +387,9 @@ static int gmc_v9_0_process_interrupt(struct amdgpu_device 
*adev,
dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
REG_GET_FIELD(status,
VM_L2_PROTECTION_FAULT_STATUS, MAPPING_ERROR));
+   dev_err(adev->dev, "\t RW: 0x%lx\n",
+   REG_GET_FIELD(status,
+   VM_L2_PROTECTION_FAULT_STATUS, RW));


That should probably be a separate patch since it is fixing gfx9.

Apart from that the patch looks good to me,
Christian.

  
  		}

}


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

[PATCH] drm/amdgpu: Add more page fault info printing for GFX10

[Zhao, Yong]

The printing we did for GFX9 was not propogated to GFX10 somehow, so fix
it now.

Change-Id: Ic0b8381134340b83cd69c3fe186ac7a8a97b1bca
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c | 33 ++
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  |  5 +++-
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
index 4e3ac1084a94..f23be98e9897 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c
@@ -140,17 +140,40 @@ static int gmc_v10_0_process_interrupt(struct 
amdgpu_device *adev,
}
 
if (printk_ratelimit()) {
+   struct amdgpu_task_info task_info;
+
+   memset(_info, 0, sizeof(struct amdgpu_task_info));
+   amdgpu_vm_get_task_info(adev, entry->pasid, _info);
+
dev_err(adev->dev,
-   "[%s] VMC page fault (src_id:%u ring:%u vmid:%u 
pasid:%u)\n",
+   "[%s] page fault (src_id:%u ring:%u vmid:%u pasid:%u, "
+   "for process:%s pid:%d thread:%s pid:%d)\n",
entry->vmid_src ? "mmhub" : "gfxhub",
entry->src_id, entry->ring_id, entry->vmid,
-   entry->pasid);
-   dev_err(adev->dev, "  at page 0x%016llx from %d\n",
+   entry->pasid, task_info.process_name, task_info.tgid,
+   task_info.task_name, task_info.pid);
+   dev_err(adev->dev, "  in page starting at address 0x%016llx 
from client %d\n",
addr, entry->client_id);
-   if (!amdgpu_sriov_vf(adev))
+   if (!amdgpu_sriov_vf(adev)) {
dev_err(adev->dev,
-   "VM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
+   "GCVM_L2_PROTECTION_FAULT_STATUS:0x%08X\n",
status);
+   dev_err(adev->dev, "\t MORE_FAULTS: 0x%lx\n",
+   REG_GET_FIELD(status,
+   GCVM_L2_PROTECTION_FAULT_STATUS, MORE_FAULTS));
+   dev_err(adev->dev, "\t WALKER_ERROR: 0x%lx\n",
+   REG_GET_FIELD(status,
+   GCVM_L2_PROTECTION_FAULT_STATUS, WALKER_ERROR));
+   dev_err(adev->dev, "\t PERMISSION_FAULTS: 0x%lx\n",
+   REG_GET_FIELD(status,
+   GCVM_L2_PROTECTION_FAULT_STATUS, 
PERMISSION_FAULTS));
+   dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
+   REG_GET_FIELD(status,
+   GCVM_L2_PROTECTION_FAULT_STATUS, 
MAPPING_ERROR));
+   dev_err(adev->dev, "\t RW: 0x%lx\n",
+   REG_GET_FIELD(status,
+   GCVM_L2_PROTECTION_FAULT_STATUS, RW));
+   }
}
 
return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 296e2d982578..34c4c2d08550 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -364,7 +364,7 @@ static int gmc_v9_0_process_interrupt(struct amdgpu_device 
*adev,
 
dev_err(adev->dev,
"[%s] %s page fault (src_id:%u ring:%u vmid:%u "
-   "pasid:%u, for process %s pid %d thread %s pid %d)\n",
+   "pasid:%u, for process:%s pid:%d thread:%s pid:%d)\n",
hub_name, retry_fault ? "retry" : "no-retry",
entry->src_id, entry->ring_id, entry->vmid,
entry->pasid, task_info.process_name, task_info.tgid,
@@ -387,6 +387,9 @@ static int gmc_v9_0_process_interrupt(struct amdgpu_device 
*adev,
dev_err(adev->dev, "\t MAPPING_ERROR: 0x%lx\n",
REG_GET_FIELD(status,
VM_L2_PROTECTION_FAULT_STATUS, MAPPING_ERROR));
+   dev_err(adev->dev, "\t RW: 0x%lx\n",
+   REG_GET_FIELD(status,
+   VM_L2_PROTECTION_FAULT_STATUS, RW));
 
}
}
-- 
2.17.1

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

Re: [PATCH] SWDEV-197284 - drm/amdgpu: Only use the peek function in productor side is not correct

[Christian König]

Am 12.08.19 um 09:42 schrieb Emily Deng:

For spsc queue, use peek function would cause error in productor side.
As for the last element, when the push job is occurring during pop job, the 
peek function
will not be updated in time, and it will return NULL.

So use queue count for double confirming the job queue is empty.


For the upstream branch I'm going to push my patch which is not as 
invasive as this one.


Christian.



Signed-off-by: Emily Deng 
---
  drivers/gpu/drm/scheduler/sched_entity.c | 4 ++--
  include/drm/spsc_queue.h | 7 +++
  2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_entity.c 
b/drivers/gpu/drm/scheduler/sched_entity.c
index 35ddbec..e74894f 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -95,7 +95,7 @@ static bool drm_sched_entity_is_idle(struct drm_sched_entity 
*entity)
rmb(); /* for list_empty to work without lock */
  
  	if (list_empty(>list) ||

-   spsc_queue_peek(>job_queue) == NULL)
+   ((spsc_queue_peek(>job_queue) == NULL) && 
!spsc_queue_count(>job_queue)))
return true;
  
  	return false;

@@ -281,7 +281,7 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
/* Consumption of existing IBs wasn't completed. Forcefully
 * remove them here.
 */
-   if (spsc_queue_peek(>job_queue)) {
+   if ((spsc_queue_peek(>job_queue) == NULL) && 
!spsc_queue_count(>job_queue)) {
if (sched) {
/* Park the kernel for a moment to make sure it isn't 
processing
 * our enity.
diff --git a/include/drm/spsc_queue.h b/include/drm/spsc_queue.h
index 125f096..78092b9 100644
--- a/include/drm/spsc_queue.h
+++ b/include/drm/spsc_queue.h
@@ -54,9 +54,8 @@ static inline void spsc_queue_init(struct spsc_queue *queue)
  
  static inline struct spsc_node *spsc_queue_peek(struct spsc_queue *queue)

  {
-   return queue->head;
+   return READ_ONCE(queue->head);
  }
-
  static inline int spsc_queue_count(struct spsc_queue *queue)
  {
return atomic_read(>job_count);
@@ -70,9 +69,9 @@ static inline bool spsc_queue_push(struct spsc_queue *queue, 
struct spsc_node *n
  
  	preempt_disable();
  
+	atomic_inc(>job_count);

tail = (struct spsc_node **)atomic_long_xchg(>tail, 
(long)>next);
WRITE_ONCE(*tail, node);
-   atomic_inc(>job_count);
  
  	/*

 * In case of first element verify new node will be visible to the 
consumer
@@ -93,6 +92,7 @@ static inline struct spsc_node *spsc_queue_pop(struct 
spsc_queue *queue)
/* Verify reading from memory and not the cache */
smp_rmb();
  
+	atomic_dec(>job_count);

node = READ_ONCE(queue->head);
  
  	if (!node)

@@ -113,7 +113,6 @@ static inline struct spsc_node *spsc_queue_pop(struct 
spsc_queue *queue)
}
}
  
-	atomic_dec(>job_count);

return node;
  }
  


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

Re: [PATCH][drm-next] drm/amd/powerplay: remove redundant duplicated return check

[Alex Deucher]

Applied.  thanks!

Alex

On Mon, Aug 5, 2019 at 6:29 AM Colin King  wrote:
>
> From: Colin Ian King 
>
> The check on ret is duplicated in two places, it is redundant code.
> Remove it.
>
> Addresses-Coverity: ("Logically dead code")
> Fixes: b94afb61cdae ("drm/amd/powerplay: honor hw limit on fetching metrics 
> data for navi10")
> Signed-off-by: Colin Ian King 
> ---
>  drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 4 
>  1 file changed, 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c 
> b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> index d62c2784b102..b272c8dc8f79 100644
> --- a/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/navi10_ppt.c
> @@ -941,8 +941,6 @@ static int navi10_get_gpu_power(struct smu_context *smu, 
> uint32_t *value)
> ret = navi10_get_metrics_table(smu, );
> if (ret)
> return ret;
> -   if (ret)
> -   return ret;
>
> *value = metrics.AverageSocketPower << 8;
>
> @@ -1001,8 +999,6 @@ static int navi10_get_fan_speed_rpm(struct smu_context 
> *smu,
> ret = navi10_get_metrics_table(smu, );
> if (ret)
> return ret;
> -   if (ret)
> -   return ret;
>
> *speed = metrics.CurrFanSpeed;
>
> --
> 2.20.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amd/display: Fix a typo - dce_aduio_mask --> dce_audio_mask

[Alex Deucher]

Applied.  thanks!

Alex

On Sat, Aug 10, 2019 at 9:55 AM Christophe JAILLET
 wrote:
>
> This should be 'dce_audio_mask', not 'dce_aduio_mask'.
>
> Signed-off-by: Christophe JAILLET 
> ---
>  drivers/gpu/drm/amd/display/dc/dce/dce_audio.c  | 2 +-
>  drivers/gpu/drm/amd/display/dc/dce/dce_audio.h  | 6 +++---
>  drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c | 2 +-
>  drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c | 2 +-
>  drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 2 +-
>  drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c | 2 +-
>  drivers/gpu/drm/amd/display/dc/dce80/dce80_resource.c   | 2 +-
>  drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c   | 2 +-
>  8 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c 
> b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
> index 549704998f84..1e88c5f46be7 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
> @@ -937,7 +937,7 @@ struct audio *dce_audio_create(
> unsigned int inst,
> const struct dce_audio_registers *reg,
> const struct dce_audio_shift *shifts,
> -   const struct dce_aduio_mask *masks
> +   const struct dce_audio_mask *masks
> )
>  {
> struct dce_audio *audio = kzalloc(sizeof(*audio), GFP_KERNEL);
> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.h 
> b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.h
> index a0d5724aab31..1392fab0860b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.h
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.h
> @@ -101,7 +101,7 @@ struct dce_audio_shift {
> uint32_t DCCG_AUDIO_DTO1_USE_512FBR_DTO;
>  };
>
> -struct dce_aduio_mask {
> +struct dce_audio_mask {
> uint32_t AZALIA_ENDPOINT_REG_INDEX;
> uint32_t AZALIA_ENDPOINT_REG_DATA;
>
> @@ -125,7 +125,7 @@ struct dce_audio {
> struct audio base;
> const struct dce_audio_registers *regs;
> const struct dce_audio_shift *shifts;
> -   const struct dce_aduio_mask *masks;
> +   const struct dce_audio_mask *masks;
>  };
>
>  struct audio *dce_audio_create(
> @@ -133,7 +133,7 @@ struct audio *dce_audio_create(
> unsigned int inst,
> const struct dce_audio_registers *reg,
> const struct dce_audio_shift *shifts,
> -   const struct dce_aduio_mask *masks);
> +   const struct dce_audio_mask *masks);
>
>  void dce_aud_destroy(struct audio **audio);
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c 
> b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> index 6248c8455314..81116286b15b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_resource.c
> @@ -304,7 +304,7 @@ static const struct dce_audio_shift audio_shift = {
> AUD_COMMON_MASK_SH_LIST(__SHIFT)
>  };
>
> -static const struct dce_aduio_mask audio_mask = {
> +static const struct dce_audio_mask audio_mask = {
> AUD_COMMON_MASK_SH_LIST(_MASK)
>  };
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c 
> b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> index 764329264c3b..765e26454a18 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce110/dce110_resource.c
> @@ -331,7 +331,7 @@ static const struct dce_audio_shift audio_shift = {
> AUD_COMMON_MASK_SH_LIST(__SHIFT)
>  };
>
> -static const struct dce_aduio_mask audio_mask = {
> +static const struct dce_audio_mask audio_mask = {
> AUD_COMMON_MASK_SH_LIST(_MASK)
>  };
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c 
> b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
> index c6136e0ed1a4..3ac4c7e73050 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c
> @@ -337,7 +337,7 @@ static const struct dce_audio_shift audio_shift = {
> AUD_COMMON_MASK_SH_LIST(__SHIFT)
>  };
>
> -static const struct dce_aduio_mask audio_mask = {
> +static const struct dce_audio_mask audio_mask = {
> AUD_COMMON_MASK_SH_LIST(_MASK)
>  };
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c 
> b/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
> index 54be7ab370df..9a922cd39cf2 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce120/dce120_resource.c
> @@ -352,7 +352,7 @@ static const struct dce_audio_shift audio_shift = {
> DCE120_AUD_COMMON_MASK_SH_LIST(__SHIFT)
>  };
>
> -static const struct dce_aduio_mask audio_mask = {
> +static const struct dce_audio_mask audio_mask = {
> 

Re: [PATCH 2/2] drm/amd/powerplay: add arcturus_is_dpm_running function for arcturus

[Wang, Kevin(Yang)]

if any DPM feature is enabled, then DPM is running, return true.
if all DPM feature is disabled, the DPM is not running, return false.
so the macro of  "SMC_DPM_FEATURE" should be include all dpm feature, whether 
is enabled or supported.
i think the @Gui, Jack's patch is right.

Best Regards,
Kevin


From: amd-gfx  on behalf of Quan, Evan 

Sent: Monday, August 12, 2019 5:38 PM
To: Gui, Jack ; amd-gfx@lists.freedesktop.org 

Cc: Gui, Jack 
Subject: RE: [PATCH 2/2] drm/amd/powerplay: add arcturus_is_dpm_running 
function for arcturus

Please set FEATURE_DPM_PREFETCHER_MASK | FEATURE_DPM_GFXCLK_MASK only. For now, 
only these two are enabled on arcturus.

With that fixed, the patch is reviewed-by: Evan Quan 
> -Original Message-
> From: amd-gfx  On Behalf Of
> Chengming Gui
> Sent: Monday, August 12, 2019 4:22 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Gui, Jack 
> Subject: [PATCH 2/2] drm/amd/powerplay: add arcturus_is_dpm_running
> function for arcturus
>
> add arcturus_is_dpm_running function
>
> Signed-off-by: Chengming Gui 
> ---
>  drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 21
> +
>  1 file changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 03ce871..9107beb 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -51,6 +51,15 @@
>  #define SMU_FEATURES_HIGH_MASK   0x
>  #define SMU_FEATURES_HIGH_SHIFT  32
>
> +#define SMC_DPM_FEATURE ( \
> + FEATURE_DPM_PREFETCHER_MASK | \
> + FEATURE_DPM_GFXCLK_MASK | \
> + FEATURE_DPM_UCLK_MASK | \
> + FEATURE_DPM_SOCCLK_MASK | \
> + FEATURE_DPM_MP0CLK_MASK | \
> + FEATURE_DPM_FCLK_MASK | \
> + FEATURE_DPM_XGMI_MASK)
> +
>  /* possible frequency drift (1Mhz) */
>  #define EPSILON  1
>
> @@ -1873,6 +1882,17 @@ static void arcturus_dump_pptable(struct
> smu_context *smu)
>
>  }
>
> +static bool arcturus_is_dpm_running(struct smu_context *smu) {
> + int ret = 0;
> + uint32_t feature_mask[2];
> + unsigned long feature_enabled;
> + ret = smu_feature_get_enabled_mask(smu, feature_mask, 2);
> + feature_enabled = (unsigned long)((uint64_t)feature_mask[0] |
> +((uint64_t)feature_mask[1] << 32));
> + return !!(feature_enabled & SMC_DPM_FEATURE); }
> +
>  static const struct pptable_funcs arcturus_ppt_funcs = {
>/* translate smu index into arcturus specific index */
>.get_smu_msg_index = arcturus_get_smu_msg_index, @@ -1910,6
> +1930,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
>/* debug (internal used) */
>.dump_pptable = arcturus_dump_pptable,
>.get_power_limit = arcturus_get_power_limit,
> + .is_dpm_running = arcturus_is_dpm_running,
>  };
>
>  void arcturus_set_ppt_funcs(struct smu_context *smu)
> --
> 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

RE: [PATCH 2/2] drm/amd/powerplay: add arcturus_is_dpm_running function for arcturus

[Gui, Jack]

Hi Evan,

All supported feature can be set there, 
Anyone of these features is running, we can judge dpm is running.  

BR,
Jack Gui

-Original Message-
From: Quan, Evan  
Sent: Monday, August 12, 2019 5:39 PM
To: Gui, Jack ; amd-gfx@lists.freedesktop.org
Cc: Gui, Jack 
Subject: RE: [PATCH 2/2] drm/amd/powerplay: add arcturus_is_dpm_running 
function for arcturus

Please set FEATURE_DPM_PREFETCHER_MASK | FEATURE_DPM_GFXCLK_MASK only. For now, 
only these two are enabled on arcturus.

With that fixed, the patch is reviewed-by: Evan Quan 
> -Original Message-
> From: amd-gfx  On Behalf Of 
> Chengming Gui
> Sent: Monday, August 12, 2019 4:22 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Gui, Jack 
> Subject: [PATCH 2/2] drm/amd/powerplay: add arcturus_is_dpm_running 
> function for arcturus
> 
> add arcturus_is_dpm_running function
> 
> Signed-off-by: Chengming Gui 
> ---
>  drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 21
> +
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 03ce871..9107beb 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -51,6 +51,15 @@
>  #define SMU_FEATURES_HIGH_MASK   0x
>  #define SMU_FEATURES_HIGH_SHIFT  32
> 
> +#define SMC_DPM_FEATURE ( \
> + FEATURE_DPM_PREFETCHER_MASK | \
> + FEATURE_DPM_GFXCLK_MASK | \
> + FEATURE_DPM_UCLK_MASK | \
> + FEATURE_DPM_SOCCLK_MASK | \
> + FEATURE_DPM_MP0CLK_MASK | \
> + FEATURE_DPM_FCLK_MASK | \
> + FEATURE_DPM_XGMI_MASK)
> +
>  /* possible frequency drift (1Mhz) */
>  #define EPSILON  1
> 
> @@ -1873,6 +1882,17 @@ static void arcturus_dump_pptable(struct 
> smu_context *smu)
> 
>  }
> 
> +static bool arcturus_is_dpm_running(struct smu_context *smu) {
> + int ret = 0;
> + uint32_t feature_mask[2];
> + unsigned long feature_enabled;
> + ret = smu_feature_get_enabled_mask(smu, feature_mask, 2);
> + feature_enabled = (unsigned long)((uint64_t)feature_mask[0] |
> +((uint64_t)feature_mask[1] << 32));
> + return !!(feature_enabled & SMC_DPM_FEATURE); }
> +
>  static const struct pptable_funcs arcturus_ppt_funcs = {
>   /* translate smu index into arcturus specific index */
>   .get_smu_msg_index = arcturus_get_smu_msg_index, @@ -1910,6
> +1930,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
>   /* debug (internal used) */
>   .dump_pptable = arcturus_dump_pptable,
>   .get_power_limit = arcturus_get_power_limit,
> + .is_dpm_running = arcturus_is_dpm_running,
>  };
> 
>  void arcturus_set_ppt_funcs(struct smu_context *smu)
> --
> 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 2/2] drm/amd/powerplay: add arcturus_is_dpm_running function for arcturus

[Quan, Evan]

Please set FEATURE_DPM_PREFETCHER_MASK | FEATURE_DPM_GFXCLK_MASK only. For now, 
only these two are enabled on arcturus.

With that fixed, the patch is reviewed-by: Evan Quan 
> -Original Message-
> From: amd-gfx  On Behalf Of
> Chengming Gui
> Sent: Monday, August 12, 2019 4:22 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Gui, Jack 
> Subject: [PATCH 2/2] drm/amd/powerplay: add arcturus_is_dpm_running
> function for arcturus
> 
> add arcturus_is_dpm_running function
> 
> Signed-off-by: Chengming Gui 
> ---
>  drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 21
> +
>  1 file changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 03ce871..9107beb 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -51,6 +51,15 @@
>  #define SMU_FEATURES_HIGH_MASK   0x
>  #define SMU_FEATURES_HIGH_SHIFT  32
> 
> +#define SMC_DPM_FEATURE ( \
> + FEATURE_DPM_PREFETCHER_MASK | \
> + FEATURE_DPM_GFXCLK_MASK | \
> + FEATURE_DPM_UCLK_MASK | \
> + FEATURE_DPM_SOCCLK_MASK | \
> + FEATURE_DPM_MP0CLK_MASK | \
> + FEATURE_DPM_FCLK_MASK | \
> + FEATURE_DPM_XGMI_MASK)
> +
>  /* possible frequency drift (1Mhz) */
>  #define EPSILON  1
> 
> @@ -1873,6 +1882,17 @@ static void arcturus_dump_pptable(struct
> smu_context *smu)
> 
>  }
> 
> +static bool arcturus_is_dpm_running(struct smu_context *smu) {
> + int ret = 0;
> + uint32_t feature_mask[2];
> + unsigned long feature_enabled;
> + ret = smu_feature_get_enabled_mask(smu, feature_mask, 2);
> + feature_enabled = (unsigned long)((uint64_t)feature_mask[0] |
> +((uint64_t)feature_mask[1] << 32));
> + return !!(feature_enabled & SMC_DPM_FEATURE); }
> +
>  static const struct pptable_funcs arcturus_ppt_funcs = {
>   /* translate smu index into arcturus specific index */
>   .get_smu_msg_index = arcturus_get_smu_msg_index, @@ -1910,6
> +1930,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
>   /* debug (internal used) */
>   .dump_pptable = arcturus_dump_pptable,
>   .get_power_limit = arcturus_get_power_limit,
> + .is_dpm_running = arcturus_is_dpm_running,
>  };
> 
>  void arcturus_set_ppt_funcs(struct smu_context *smu)
> --
> 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 1/2] drm/amd/powerplay: remove redundancy debug log about smu unsupported features

[Kevin Wang]

Reviewed-by: Kevin Wang 

On 8/12/19 4:22 PM, Chengming Gui wrote:
> remove redundancy debug log about smu unsupported features
>
> Signed-off-by: Chengming Gui 
> ---
>   drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 1 -
>   1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c 
> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index e6fcbdf..03ce871 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -215,7 +215,6 @@ static int arcturus_get_smu_feature_index(struct 
> smu_context *smc, uint32_t inde
>   
>   mapping = arcturus_feature_mask_map[index];
>   if (!(mapping.valid_mapping)) {
> - pr_warn("Unsupported SMU feature: %d\n", index);
>   return -EINVAL;
>   }
>   
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amd/powerplay: add arcturus_is_dpm_running function for arcturus

[Kevin Wang]

Reviewed-by: Kevin Wang 

On 8/12/19 4:22 PM, Chengming Gui wrote:
> add arcturus_is_dpm_running function
>
> Signed-off-by: Chengming Gui 
> ---
>   drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 21 +
>   1 file changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c 
> b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> index 03ce871..9107beb 100644
> --- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
> @@ -51,6 +51,15 @@
>   #define SMU_FEATURES_HIGH_MASK   0x
>   #define SMU_FEATURES_HIGH_SHIFT  32
>   
> +#define SMC_DPM_FEATURE ( \
> + FEATURE_DPM_PREFETCHER_MASK | \
> + FEATURE_DPM_GFXCLK_MASK | \
> + FEATURE_DPM_UCLK_MASK | \
> + FEATURE_DPM_SOCCLK_MASK | \
> + FEATURE_DPM_MP0CLK_MASK | \
> + FEATURE_DPM_FCLK_MASK | \
> + FEATURE_DPM_XGMI_MASK)
> +
>   /* possible frequency drift (1Mhz) */
>   #define EPSILON 1
>   
> @@ -1873,6 +1882,17 @@ static void arcturus_dump_pptable(struct smu_context 
> *smu)
>   
>   }
>   
> +static bool arcturus_is_dpm_running(struct smu_context *smu)
> +{
> + int ret = 0;
> + uint32_t feature_mask[2];
> + unsigned long feature_enabled;
> + ret = smu_feature_get_enabled_mask(smu, feature_mask, 2);
> + feature_enabled = (unsigned long)((uint64_t)feature_mask[0] |
> +((uint64_t)feature_mask[1] << 32));
> + return !!(feature_enabled & SMC_DPM_FEATURE);
> +}
> +
>   static const struct pptable_funcs arcturus_ppt_funcs = {
>   /* translate smu index into arcturus specific index */
>   .get_smu_msg_index = arcturus_get_smu_msg_index,
> @@ -1910,6 +1930,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
>   /* debug (internal used) */
>   .dump_pptable = arcturus_dump_pptable,
>   .get_power_limit = arcturus_get_power_limit,
> + .is_dpm_running = arcturus_is_dpm_running,
>   };
>   
>   void arcturus_set_ppt_funcs(struct smu_context *smu)
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 1/2] drm/amd/powerplay: remove redundancy debug log about smu unsupported features

[Chengming Gui]

remove redundancy debug log about smu unsupported features

Signed-off-by: Chengming Gui 
---
 drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c 
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index e6fcbdf..03ce871 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -215,7 +215,6 @@ static int arcturus_get_smu_feature_index(struct 
smu_context *smc, uint32_t inde
 
mapping = arcturus_feature_mask_map[index];
if (!(mapping.valid_mapping)) {
-   pr_warn("Unsupported SMU feature: %d\n", index);
return -EINVAL;
}
 
-- 
2.7.4

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

[PATCH 2/2] drm/amd/powerplay: add arcturus_is_dpm_running function for arcturus

[Chengming Gui]

add arcturus_is_dpm_running function

Signed-off-by: Chengming Gui 
---
 drivers/gpu/drm/amd/powerplay/arcturus_ppt.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c 
b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
index 03ce871..9107beb 100644
--- a/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/powerplay/arcturus_ppt.c
@@ -51,6 +51,15 @@
 #define SMU_FEATURES_HIGH_MASK   0x
 #define SMU_FEATURES_HIGH_SHIFT  32
 
+#define SMC_DPM_FEATURE ( \
+   FEATURE_DPM_PREFETCHER_MASK | \
+   FEATURE_DPM_GFXCLK_MASK | \
+   FEATURE_DPM_UCLK_MASK | \
+   FEATURE_DPM_SOCCLK_MASK | \
+   FEATURE_DPM_MP0CLK_MASK | \
+   FEATURE_DPM_FCLK_MASK | \
+   FEATURE_DPM_XGMI_MASK)
+
 /* possible frequency drift (1Mhz) */
 #define EPSILON1
 
@@ -1873,6 +1882,17 @@ static void arcturus_dump_pptable(struct smu_context 
*smu)
 
 }
 
+static bool arcturus_is_dpm_running(struct smu_context *smu)
+{
+   int ret = 0;
+   uint32_t feature_mask[2];
+   unsigned long feature_enabled;
+   ret = smu_feature_get_enabled_mask(smu, feature_mask, 2);
+   feature_enabled = (unsigned long)((uint64_t)feature_mask[0] |
+  ((uint64_t)feature_mask[1] << 32));
+   return !!(feature_enabled & SMC_DPM_FEATURE);
+}
+
 static const struct pptable_funcs arcturus_ppt_funcs = {
/* translate smu index into arcturus specific index */
.get_smu_msg_index = arcturus_get_smu_msg_index,
@@ -1910,6 +1930,7 @@ static const struct pptable_funcs arcturus_ppt_funcs = {
/* debug (internal used) */
.dump_pptable = arcturus_dump_pptable,
.get_power_limit = arcturus_get_power_limit,
+   .is_dpm_running = arcturus_is_dpm_running,
 };
 
 void arcturus_set_ppt_funcs(struct smu_context *smu)
-- 
2.7.4

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

[PATCH] SWDEV-197284 - drm/amdgpu: Only use the peek function in productor side is not correct

[Emily Deng]

For spsc queue, use peek function would cause error in productor side.
As for the last element, when the push job is occurring during pop job, the 
peek function
will not be updated in time, and it will return NULL.

So use queue count for double confirming the job queue is empty.

Signed-off-by: Emily Deng 
---
 drivers/gpu/drm/scheduler/sched_entity.c | 4 ++--
 include/drm/spsc_queue.h | 7 +++
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/scheduler/sched_entity.c 
b/drivers/gpu/drm/scheduler/sched_entity.c
index 35ddbec..e74894f 100644
--- a/drivers/gpu/drm/scheduler/sched_entity.c
+++ b/drivers/gpu/drm/scheduler/sched_entity.c
@@ -95,7 +95,7 @@ static bool drm_sched_entity_is_idle(struct drm_sched_entity 
*entity)
rmb(); /* for list_empty to work without lock */
 
if (list_empty(>list) ||
-   spsc_queue_peek(>job_queue) == NULL)
+   ((spsc_queue_peek(>job_queue) == NULL) && 
!spsc_queue_count(>job_queue)))
return true;
 
return false;
@@ -281,7 +281,7 @@ void drm_sched_entity_fini(struct drm_sched_entity *entity)
/* Consumption of existing IBs wasn't completed. Forcefully
 * remove them here.
 */
-   if (spsc_queue_peek(>job_queue)) {
+   if ((spsc_queue_peek(>job_queue) == NULL) && 
!spsc_queue_count(>job_queue)) {
if (sched) {
/* Park the kernel for a moment to make sure it isn't 
processing
 * our enity.
diff --git a/include/drm/spsc_queue.h b/include/drm/spsc_queue.h
index 125f096..78092b9 100644
--- a/include/drm/spsc_queue.h
+++ b/include/drm/spsc_queue.h
@@ -54,9 +54,8 @@ static inline void spsc_queue_init(struct spsc_queue *queue)
 
 static inline struct spsc_node *spsc_queue_peek(struct spsc_queue *queue)
 {
-   return queue->head;
+   return READ_ONCE(queue->head);
 }
-
 static inline int spsc_queue_count(struct spsc_queue *queue)
 {
return atomic_read(>job_count);
@@ -70,9 +69,9 @@ static inline bool spsc_queue_push(struct spsc_queue *queue, 
struct spsc_node *n
 
preempt_disable();
 
+   atomic_inc(>job_count);
tail = (struct spsc_node **)atomic_long_xchg(>tail, 
(long)>next);
WRITE_ONCE(*tail, node);
-   atomic_inc(>job_count);
 
/*
 * In case of first element verify new node will be visible to the 
consumer
@@ -93,6 +92,7 @@ static inline struct spsc_node *spsc_queue_pop(struct 
spsc_queue *queue)
/* Verify reading from memory and not the cache */
smp_rmb();
 
+   atomic_dec(>job_count);
node = READ_ONCE(queue->head);
 
if (!node)
@@ -113,7 +113,6 @@ static inline struct spsc_node *spsc_queue_pop(struct 
spsc_queue *queue)
}
}
 
-   atomic_dec(>job_count);
return node;
 }
 
-- 
2.7.4

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