Re: [PATCH v2] drm/amdgpu: Enable SDMA utilization for Arcturus

2020-09-11 Thread Felix Kuehling
Am 2020-09-11 um 5:33 p.m. schrieb Mukul Joshi:
> SDMA utilization calculations are enabled/disabled by
> writing to SDMAx_PUB_DUMMY_REG2 register. Currently,
> enable this only for Arcturus.
>
> Signed-off-by: Mukul Joshi 

Reviewed-by: Felix Kuehling 


> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 856c50386c86..edea8743f26e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -1063,6 +1063,15 @@ static void sdma_v4_0_ctx_switch_enable(struct 
> amdgpu_device *adev, bool enable)
>   WREG32_SDMA(i, mmSDMA0_PHASE2_QUANTUM, phase_quantum);
>   }
>   WREG32_SDMA(i, mmSDMA0_CNTL, f32_cntl);
> +
> + /*
> +  * Enable SDMA utilization. Its only supported on
> +  * Arcturus for the moment and firmware version 14
> +  * and above.
> +  */
> + if (adev->asic_type == CHIP_ARCTURUS &&
> + adev->sdma.instance[i].fw_version >= 14)
> + WREG32_SDMA(i, mmSDMA0_PUB_DUMMY_REG2, enable);
>   }
>  
>  }
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH v2 2/3] drm/amdkfd: Add process eviction counters to sysfs

2020-09-11 Thread Felix Kuehling
Am 2020-09-11 um 4:10 p.m. schrieb Philip Cox:
> Add per-process eviction counters to sysfs to keep track of
> how many eviction events have happened for each process.
>
> v2: rename the stats dir, and track all evictions per process, per device.
>
> Signed-off-by: Philip Cox 
Some more comments inline.

Amber, could you review this one as well?


> ---
>  .../drm/amd/amdkfd/kfd_device_queue_manager.c |   9 ++
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h |  15 ++-
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 109 ++
>  3 files changed, 131 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
> b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index cafbc3aa980a..5b9e0df2a90e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -653,6 +653,7 @@ static int evict_process_queues_nocpsch(struct 
> device_queue_manager *dqm,
>   pr_info_ratelimited("Evicting PASID 0x%x queues\n",
>   pdd->process->pasid);
>  
> + pdd->last_evict_timestamp = get_jiffies_64();
>   /* Mark all queues as evicted. Deactivate all active queues on
>* the qpd.
>*/
> @@ -714,6 +715,7 @@ static int evict_process_queues_cpsch(struct 
> device_queue_manager *dqm,
>   q->properties.is_active = false;
>   decrement_queue_count(dqm, q->properties.type);
>   }
> + pdd->last_evict_timestamp = get_jiffies_64();
>   retval = execute_queues_cpsch(dqm,
>   qpd->is_debug ?
>   KFD_UNMAP_QUEUES_FILTER_ALL_QUEUES :
> @@ -732,6 +734,7 @@ static int restore_process_queues_nocpsch(struct 
> device_queue_manager *dqm,
>   struct mqd_manager *mqd_mgr;
>   struct kfd_process_device *pdd;
>   uint64_t pd_base;
> + uint64_t eviction_duration;
>   int retval, ret = 0;
>  
>   pdd = qpd_to_pdd(qpd);
> @@ -799,6 +802,8 @@ static int restore_process_queues_nocpsch(struct 
> device_queue_manager *dqm,
>   ret = retval;
>   }
>   qpd->evicted = 0;
> + eviction_duration = get_jiffies_64() - pdd->last_evict_timestamp;
> + atomic64_add(eviction_duration, >evict_duration_counter);
>  out:
>   if (mm)
>   mmput(mm);
> @@ -812,6 +817,7 @@ static int restore_process_queues_cpsch(struct 
> device_queue_manager *dqm,
>   struct queue *q;
>   struct kfd_process_device *pdd;
>   uint64_t pd_base;
> + uint64_t eviction_duration;
>   int retval = 0;
>  
>   pdd = qpd_to_pdd(qpd);
> @@ -845,6 +851,9 @@ static int restore_process_queues_cpsch(struct 
> device_queue_manager *dqm,
>   retval = execute_queues_cpsch(dqm,
>   KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
>   qpd->evicted = 0;
> + eviction_duration = get_jiffies_64() - pdd->last_evict_timestamp;
> + atomic64_add(eviction_duration, >evict_duration_counter);
> +
>  out:
>   dqm_unlock(dqm);
>   return retval;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 023629f28495..468c69d22117 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -631,7 +631,7 @@ enum kfd_pdd_bound {
>   PDD_BOUND_SUSPENDED,
>  };
>  
> -#define MAX_SYSFS_FILENAME_LEN 11
> +#define MAX_SYSFS_FILENAME_LEN 15
>  
>  /*
>   * SDMA counter runs at 100MHz frequency.
> @@ -692,10 +692,20 @@ struct kfd_process_device {
>   uint64_t sdma_past_activity_counter;
>   struct attribute attr_sdma;
>   char sdma_filename[MAX_SYSFS_FILENAME_LEN];
> +
> + /* Eviction activity tracking */
> + unsigned long last_restore_timestamp;

last_restore_timestamp is not used.


> + unsigned long last_evict_timestamp;
> + atomic64_t evict_duration_counter;
> + struct attribute attr_evict;
>  };
>  
>  #define qpd_to_pdd(x) container_of(x, struct kfd_process_device, qpd)
>  
> +struct kobj_stats_list {
> + struct list_head stats_list;
> + struct kobject *kobj;
> +};

Why does this need to be a separate struct that gets dynamically
allocated with kzalloc? The kobj pointer should be in the pdd structure,
then you wouldn't need a list at all.


>  /* Process data */
>  struct kfd_process {
>   /*
> @@ -766,13 +776,14 @@ struct kfd_process {
>   /* seqno of the last scheduled eviction */
>   unsigned int last_eviction_seqno;
>   /* Approx. the last timestamp (in jiffies) when the process was
> -  * restored after an eviction
> +  * restored or evicted.

This comment should not be changed in this commit. There is no
functional change here.


>*/
>   unsigned long last_restore_timestamp;
>  
>   /* Kobj for our procfs */
>   struct kobject *kobj;
>   struct kobject *kobj_queues;
> + struct kobj_stats_list stats;

This is probably not 

[PATCH] drm/amdgpu: Workaround RCC_DEV0_EPF0_STRAP0 access issue for SRIOV

2020-09-11 Thread Rohit Khaire
Signed-off-by: Rohit Khaire 
---
 drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c 
b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
index 7429f30398b9..fdfa075e6d5a 100644
--- a/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
+++ b/drivers/gpu/drm/amd/amdgpu/nbio_v2_3.c
@@ -51,8 +51,20 @@ static void nbio_v2_3_remap_hdp_registers(struct 
amdgpu_device *adev)
 
 static u32 nbio_v2_3_get_rev_id(struct amdgpu_device *adev)
 {
-   u32 tmp = RREG32_SOC15(NBIO, 0, mmRCC_DEV0_EPF0_STRAP0);
+   u32 tmp;
 
+   /*
+* On SRIOV VF RCC_DEV0_EPF0_STRAP is blocked.
+* So we read rev_id from PCI config space.
+*/
+   if (amdgpu_sriov_vf(adev)) {
+   pci_read_config_dword(adev->pdev, PCI_REVISION_ID, );
+   /* Revision ID is the least significant 8 bits */
+   tmp &= 0xFF;
+   return tmp;
+   }
+
+   tmp = RREG32_SOC15(NBIO, 0, mmRCC_DEV0_EPF0_STRAP0);
tmp &= RCC_DEV0_EPF0_STRAP0__STRAP_ATI_REV_ID_DEV0_F0_MASK;
tmp >>= RCC_DEV0_EPF0_STRAP0__STRAP_ATI_REV_ID_DEV0_F0__SHIFT;
 
-- 
2.17.1

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


[PATCH v2] drm/amdgpu: Enable SDMA utilization for Arcturus

2020-09-11 Thread Mukul Joshi
SDMA utilization calculations are enabled/disabled by
writing to SDMAx_PUB_DUMMY_REG2 register. Currently,
enable this only for Arcturus.

Signed-off-by: Mukul Joshi 
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 856c50386c86..edea8743f26e 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1063,6 +1063,15 @@ static void sdma_v4_0_ctx_switch_enable(struct 
amdgpu_device *adev, bool enable)
WREG32_SDMA(i, mmSDMA0_PHASE2_QUANTUM, phase_quantum);
}
WREG32_SDMA(i, mmSDMA0_CNTL, f32_cntl);
+
+   /*
+* Enable SDMA utilization. Its only supported on
+* Arcturus for the moment and firmware version 14
+* and above.
+*/
+   if (adev->asic_type == CHIP_ARCTURUS &&
+   adev->sdma.instance[i].fw_version >= 14)
+   WREG32_SDMA(i, mmSDMA0_PUB_DUMMY_REG2, enable);
}
 
 }
-- 
2.17.1

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


Re: [PATCH 1/1] drm/amdgpu: Convert to using devm_drm_dev_alloc()

2020-09-11 Thread Luben Tuikov
On 2020-09-08 16:09, Luben Tuikov wrote:
> On 2020-09-07 04:07, Daniel Vetter wrote:
>> On Mon, Sep 07, 2020 at 10:06:08AM +0200, Daniel Vetter wrote:
>>> On Sat, Sep 05, 2020 at 11:50:05AM -0400, Alex Deucher wrote:
 On Thu, Sep 3, 2020 at 9:22 PM Luben Tuikov  wrote:
>
> Convert to using devm_drm_dev_alloc(),
> as drm_dev_init() is going away.
>
> Signed-off-by: Luben Tuikov 

 I think we can drop the final drm_put in the error case?  I think the
 unwinding in current devm code should take care of it.
>>>
>>> Same applies for the pci remove hook too.
>>
>> KASAN run with unload should have caught this. 
> 
> But it didn't? Why?
> Could it be that drm_dev_put() actually got
> the kref to 0 and then drm_dev_release()
> was called which did a kfree()?
> 
> Could you try that same unload KASAN run but
> with your suggestion of removing drm_dev_put() from
> amdgpu_pci_remove()? What do you get then?

Hi Daniel,

Have you had a chance to run this unload KASAN run with
your suggestion of removing drm_dev_put() from
the PCI release hook?

If it "should have caught this", but it didn't,
perhaps it did catch it when you removed the drm_dev_put()
hook from the PCI release hook, when you did a KASAN unload run?
Showing that drm_dev_put() is still necessary, since,
1) we're still using kref,
2) kref is kref-init-ed under devm_drm_dev_alloc() as I pointed
   out in my reply to Alex in this thread.

I believe KASAN (and logic) show this patch to be solid.

> 
>> I strongly recommend doing
>> that for any changes to the unload code, it's way to easy to mix up
>> something and release it in the wrong order or from the wrong callback or
>> with the wrong managed (devm_ vs drmm_) functions.
> 
> Sorry, I don't understand what you mean by "doing that"? Do
> you mean "not calling drm_dev_put()"? Sure, but what
> are we supposed to call instead?
> 
> I also don't understand what you mean by "easy to mix up something
> and release it in wrong order or from the wrong callback..." etc.
> 
> If you want things to happen in certain order,
> you can either put the correct-order-sequence
> behind the non-zero-->0 transition of kref, say in
> drm_dev_release() as it is right now,
> 
> static void drm_dev_release(struct kref *ref)
> {
> struct drm_device *dev = container_of(ref, struct drm_device, ref);
> 
> if (dev->driver->release)
> dev->driver->release(dev);
> 
> drm_managed_release(dev);
> 
> kfree(dev->managed.final_kfree);
> }
> 
> Or you can remove kref from DRM dev (which I do not
> recommend), and stipulate the release sequence
> as I asked in Message-ID: <165961bb-3b5b-cedc-2fc0-838b7999d...@amd.com>,
> "Re: [PATCH] drm/managed: Cleanup of unused functions and polishing docs".
> 
> Then we can follow that and submit patches to conform.

Eagerly awaiting your response on this so that we can conform
to the direction you're setting forth.

Are you removing kref (release() cb) from DRM and if so,
what function should we call in order to do the "final"
(although without kref, the notion of "final" is obviated)
free, OR kref stays in and this patch, which conforms
to using devm_drm_dev_alloc(), as postulated by you,
can go in.

Regards,
Luben

> 
> Regards,
> Luben
> 
> 
> 
>> -Daniel
>>
>>> -Daniel

 Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 +++
>  1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 146a85c8df1c..06d994187c24 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1142,18 +1142,13 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> if (ret)
> return ret;
>
> -   adev = kzalloc(sizeof(*adev), GFP_KERNEL);
> -   if (!adev)
> -   return -ENOMEM;
> +   adev = devm_drm_dev_alloc(>dev, _driver, typeof(*adev), 
> ddev);
> +   if (IS_ERR(adev))
> +   return PTR_ERR(adev);
>
> adev->dev  = >dev;
> adev->pdev = pdev;
> ddev = adev_to_drm(adev);
> -   ret = drm_dev_init(ddev, _driver, >dev);
> -   if (ret)
> -   goto err_free;
> -
> -   drmm_add_final_kfree(ddev, adev);
>
> if (!supports_atomic)
> ddev->driver_features &= ~DRIVER_ATOMIC;
> --
> 2.28.0.394.ge197136389
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> 

[PATCH v2 2/3] drm/amdkfd: Add process eviction counters to sysfs

2020-09-11 Thread Philip Cox
Add per-process eviction counters to sysfs to keep track of
how many eviction events have happened for each process.

v2: rename the stats dir, and track all evictions per process, per device.

Signed-off-by: Philip Cox 
---
 .../drm/amd/amdkfd/kfd_device_queue_manager.c |   9 ++
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h |  15 ++-
 drivers/gpu/drm/amd/amdkfd/kfd_process.c  | 109 ++
 3 files changed, 131 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index cafbc3aa980a..5b9e0df2a90e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -653,6 +653,7 @@ static int evict_process_queues_nocpsch(struct 
device_queue_manager *dqm,
pr_info_ratelimited("Evicting PASID 0x%x queues\n",
pdd->process->pasid);
 
+   pdd->last_evict_timestamp = get_jiffies_64();
/* Mark all queues as evicted. Deactivate all active queues on
 * the qpd.
 */
@@ -714,6 +715,7 @@ static int evict_process_queues_cpsch(struct 
device_queue_manager *dqm,
q->properties.is_active = false;
decrement_queue_count(dqm, q->properties.type);
}
+   pdd->last_evict_timestamp = get_jiffies_64();
retval = execute_queues_cpsch(dqm,
qpd->is_debug ?
KFD_UNMAP_QUEUES_FILTER_ALL_QUEUES :
@@ -732,6 +734,7 @@ static int restore_process_queues_nocpsch(struct 
device_queue_manager *dqm,
struct mqd_manager *mqd_mgr;
struct kfd_process_device *pdd;
uint64_t pd_base;
+   uint64_t eviction_duration;
int retval, ret = 0;
 
pdd = qpd_to_pdd(qpd);
@@ -799,6 +802,8 @@ static int restore_process_queues_nocpsch(struct 
device_queue_manager *dqm,
ret = retval;
}
qpd->evicted = 0;
+   eviction_duration = get_jiffies_64() - pdd->last_evict_timestamp;
+   atomic64_add(eviction_duration, >evict_duration_counter);
 out:
if (mm)
mmput(mm);
@@ -812,6 +817,7 @@ static int restore_process_queues_cpsch(struct 
device_queue_manager *dqm,
struct queue *q;
struct kfd_process_device *pdd;
uint64_t pd_base;
+   uint64_t eviction_duration;
int retval = 0;
 
pdd = qpd_to_pdd(qpd);
@@ -845,6 +851,9 @@ static int restore_process_queues_cpsch(struct 
device_queue_manager *dqm,
retval = execute_queues_cpsch(dqm,
KFD_UNMAP_QUEUES_FILTER_DYNAMIC_QUEUES, 0);
qpd->evicted = 0;
+   eviction_duration = get_jiffies_64() - pdd->last_evict_timestamp;
+   atomic64_add(eviction_duration, >evict_duration_counter);
+
 out:
dqm_unlock(dqm);
return retval;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 023629f28495..468c69d22117 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -631,7 +631,7 @@ enum kfd_pdd_bound {
PDD_BOUND_SUSPENDED,
 };
 
-#define MAX_SYSFS_FILENAME_LEN 11
+#define MAX_SYSFS_FILENAME_LEN 15
 
 /*
  * SDMA counter runs at 100MHz frequency.
@@ -692,10 +692,20 @@ struct kfd_process_device {
uint64_t sdma_past_activity_counter;
struct attribute attr_sdma;
char sdma_filename[MAX_SYSFS_FILENAME_LEN];
+
+   /* Eviction activity tracking */
+   unsigned long last_restore_timestamp;
+   unsigned long last_evict_timestamp;
+   atomic64_t evict_duration_counter;
+   struct attribute attr_evict;
 };
 
 #define qpd_to_pdd(x) container_of(x, struct kfd_process_device, qpd)
 
+struct kobj_stats_list {
+   struct list_head stats_list;
+   struct kobject *kobj;
+};
 /* Process data */
 struct kfd_process {
/*
@@ -766,13 +776,14 @@ struct kfd_process {
/* seqno of the last scheduled eviction */
unsigned int last_eviction_seqno;
/* Approx. the last timestamp (in jiffies) when the process was
-* restored after an eviction
+* restored or evicted.
 */
unsigned long last_restore_timestamp;
 
/* Kobj for our procfs */
struct kobject *kobj;
struct kobject *kobj_queues;
+   struct kobj_stats_list stats;
struct attribute attr_pasid;
 };
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 1e15aa7d8ae8..d786ba80d656 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -344,6 +344,26 @@ static ssize_t kfd_procfs_queue_show(struct kobject *kobj,
 
return 0;
 }
+static ssize_t kfd_procfs_stats_show(struct kobject *kobj,
+struct attribute *attr, char *buffer)
+{
+   if (strcmp(attr->name, "evicted_ms") == 0) 

Re: [PATCH v2 3/4] drm/amd/display: Add pipe_state tracepoint

2020-09-11 Thread Rodrigo Siqueira
On 09/11, Kazlauskas, Nicholas wrote:
> On 2020-09-11 10:59 a.m., Rodrigo Siqueira wrote:
> > This commit introduces a trace mechanism for struct pipe_ctx by adding a
> > middle layer struct in the amdgpu_dm_trace.h for capturing the most
> > important data from struct pipe_ctx and showing its data via tracepoint.
> > This tracepoint was added to dc.c and dcn10_hw_sequencer, however, it
> > can be added to other DCN architecture.
> > 
> > Co-developed-by: Nicholas Kazlauskas 
> > Signed-off-by: Nicholas Kazlauskas 
> > Signed-off-by: Rodrigo Siqueira 
> 
> 
> This patch, while very useful, unfortunately pulls in a lot of DM code into
> DC so I would prefer to hold off on this one for now.

Hi Nicholas, first of all, thanks for your feedback.

By "pulls in a lot of DM code into DC" do you mean all references to
plane_state and plane_res? Or the data inserted in the struct?
 
> It would be better if this had a proper DC interface for tracing/logging
> these states. If the API was more like how we handle tracing register
> reads/writes this would be cleaner.

Could you elaborate a little bit more about  "a proper DC interface"?
What is your view of this sort of interface?

Also, how about Patch 04? Same problems?

Best Regards
Rodrigo Siqueira

> Regards,
> Nicholas Kazlauskas
> 
> > ---
> >   .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 172 ++
> >   drivers/gpu/drm/amd/display/dc/core/dc.c  |  11 ++
> >   .../amd/display/dc/dcn10/dcn10_hw_sequencer.c |  17 +-
> >   3 files changed, 195 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> > index 5fb4c4a5c349..53f62506e17c 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> > @@ -376,6 +376,178 @@ TRACE_EVENT(amdgpu_dm_atomic_check_finish,
> >   __entry->async_update, __entry->allow_modeset)
> >   );
> > +#ifndef _AMDGPU_DM_TRACE_STRUCTS_DEFINED_
> > +#define _AMDGPU_DM_TRACE_STRUCTS_DEFINED_
> > +
> > +struct amdgpu_dm_trace_pipe_state {
> > +   int pipe_idx;
> > +   const void *stream;
> > +   int stream_w;
> > +   int stream_h;
> > +   int dst_x;
> > +   int dst_y;
> > +   int dst_w;
> > +   int dst_h;
> > +   int src_x;
> > +   int src_y;
> > +   int src_w;
> > +   int src_h;
> > +   int clip_x;
> > +   int clip_y;
> > +   int clip_w;
> > +   int clip_h;
> > +   int recout_x;
> > +   int recout_y;
> > +   int recout_w;
> > +   int recout_h;
> > +   int viewport_x;
> > +   int viewport_y;
> > +   int viewport_w;
> > +   int viewport_h;
> > +   int flip_immediate;
> > +   int surface_pitch;
> > +   int format;
> > +   int swizzle;
> > +   unsigned int update_flags;
> > +};
> > +
> > +#define fill_out_trace_pipe_state(trace_pipe_state, pipe_ctx) \
> > +   do { \
> > +   trace_pipe_state.pipe_idx   = (pipe_ctx)->pipe_idx; \
> > +   trace_pipe_state.stream = (pipe_ctx)->stream; \
> > +   trace_pipe_state.stream_w   = 
> > (pipe_ctx)->stream->timing.h_addressable; \
> > +   trace_pipe_state.stream_h   = 
> > (pipe_ctx)->stream->timing.v_addressable; \
> > +   trace_pipe_state.dst_x  = 
> > (pipe_ctx)->plane_state->dst_rect.x; \
> > +   trace_pipe_state.dst_y  = 
> > (pipe_ctx)->plane_state->dst_rect.y; \
> > +   trace_pipe_state.dst_w  = 
> > (pipe_ctx)->plane_state->dst_rect.width; \
> > +   trace_pipe_state.dst_h  = 
> > (pipe_ctx)->plane_state->dst_rect.height; \
> > +   trace_pipe_state.src_x  = 
> > (pipe_ctx)->plane_state->src_rect.x; \
> > +   trace_pipe_state.src_y  = 
> > (pipe_ctx)->plane_state->src_rect.y; \
> > +   trace_pipe_state.src_w  = 
> > (pipe_ctx)->plane_state->src_rect.width; \
> > +   trace_pipe_state.src_h  = 
> > (pipe_ctx)->plane_state->src_rect.height; \
> > +   trace_pipe_state.clip_x = 
> > (pipe_ctx)->plane_state->clip_rect.x; \
> > +   trace_pipe_state.clip_y = 
> > (pipe_ctx)->plane_state->clip_rect.y; \
> > +   trace_pipe_state.clip_w = 
> > (pipe_ctx)->plane_state->clip_rect.width; \
> > +   trace_pipe_state.clip_h = 
> > (pipe_ctx)->plane_state->clip_rect.height; \
> > +   trace_pipe_state.recout_x   = 
> > (pipe_ctx)->plane_res.scl_data.recout.x; \
> > +   trace_pipe_state.recout_y   = 
> > (pipe_ctx)->plane_res.scl_data.recout.y; \
> > +   trace_pipe_state.recout_w   = 
> > (pipe_ctx)->plane_res.scl_data.recout.width; \
> > +   trace_pipe_state.recout_h   = 
> > (pipe_ctx)->plane_res.scl_data.recout.height; \
> > +   trace_pipe_state.viewport_x = 
> > (pipe_ctx)->plane_res.scl_data.viewport.x; \
> > +   trace_pipe_state.viewport_y = 
> > 

[PATCH 2/2] drm/amdgpu: More accurate description of a function param

2020-09-11 Thread Oak Zeng
Add more accurate description of the pe parameter of function
amdgpu_vm_sdma_udpate and amdgpu_vm_cpu_update

Signed-off-by: Oak Zeng 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c  | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
index 39c704a..0786e755 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c
@@ -59,7 +59,7 @@ static int amdgpu_vm_cpu_prepare(struct 
amdgpu_vm_update_params *p,
  *
  * @p: see amdgpu_vm_update_params definition
  * @bo: PD/PT to update
- * @pe: kmap addr of the page entry
+ * @pe: byte offset of the PDE/PTE, relative to start of PDB/PTB
  * @addr: dst addr to write into pe
  * @count: number of page entries to update
  * @incr: increase next addr by incr bytes
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
index 189d46e..db79057 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c
@@ -155,7 +155,7 @@ static void amdgpu_vm_sdma_copy_ptes(struct 
amdgpu_vm_update_params *p,
  *
  * @p: see amdgpu_vm_update_params definition
  * @bo: PD/PT to update
- * @pe: addr of the page entry
+ * @pe: byte offset of the PDE/PTE, relative to start of PDB/PTB
  * @addr: dst addr to write into pe
  * @count: number of page entries to update
  * @incr: increase next addr by incr bytes
@@ -187,7 +187,7 @@ static void amdgpu_vm_sdma_set_ptes(struct 
amdgpu_vm_update_params *p,
  *
  * @p: see amdgpu_vm_update_params definition
  * @bo: PD/PT to update
- * @pe: addr of the page entry
+ * @pe: byte offset of the PDE/PTE, relative to start of PDB/PTB
  * @addr: dst addr to write into pe
  * @count: number of page entries to update
  * @incr: increase next addr by incr bytes
-- 
2.7.4

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


[PATCH 1/2] drm/amdgpu: Add comment to function amdgpu_ttm_alloc_gart

2020-09-11 Thread Oak Zeng
Add comments to refect what function does

Signed-off-by: Oak Zeng 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 63e5414..8b70445 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1210,7 +1210,12 @@ static int amdgpu_ttm_backend_bind(struct ttm_tt *ttm,
 }
 
 /**
- * amdgpu_ttm_alloc_gart - Allocate GART memory for buffer object
+ * amdgpu_ttm_alloc_gart - Make sure buffer object is accessible either
+ * through AGP or GART aperture.
+ *
+ * If bo is accessible through AGP aperture, then use AGP aperture
+ * to access bo; otherwise allocate logical space in GART aperture
+ * and map bo to GART aperture.
  */
 int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo)
 {
-- 
2.7.4

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


Re: [PATCH v2 1/4] drm/amd/display: Rework registers tracepoint

2020-09-11 Thread Rodrigo Siqueira
On 09/11, Kazlauskas, Nicholas wrote:
> On 2020-09-11 10:59 a.m., Rodrigo Siqueira wrote:
> > amdgpu_dc_rreg and amdgpu_dc_wreg are very similar, for this reason,
> > this commits abstract these two events by using DECLARE_EVENT_CLASS and
> > create an instance of it for each one of these events.
> > 
> > Signed-off-by: Rodrigo Siqueira 
> 
> This looks reasonable to me. Does this still show up as
> amdpgu_dc_rrreg/amdgpu_dc_wreg in the captured trace log?
> 
> As long as we can still tell this apart you can consider this patch:
> 
> Reviewed-by: Nicholas Kazlauskas 

Yes, this change does not change anything from the user perspective.

Thanks
 
> Regards,
> Nicholas Kazlauskas
> 
> > ---
> >   .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 55 ---
> >   1 file changed, 24 insertions(+), 31 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h 
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> > index d898981684d5..dd34e11b1079 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
> > @@ -31,40 +31,33 @@
> >   #include 
> > -TRACE_EVENT(amdgpu_dc_rreg,
> > -   TP_PROTO(unsigned long *read_count, uint32_t reg, uint32_t value),
> > -   TP_ARGS(read_count, reg, value),
> > -   TP_STRUCT__entry(
> > -   __field(uint32_t, reg)
> > -   __field(uint32_t, value)
> > -   ),
> > -   TP_fast_assign(
> > -   __entry->reg = reg;
> > -   __entry->value = value;
> > -   *read_count = *read_count + 1;
> > -   ),
> > -   TP_printk("reg=0x%08lx, value=0x%08lx",
> > -   (unsigned long)__entry->reg,
> > -   (unsigned long)__entry->value)
> > -);
> > +DECLARE_EVENT_CLASS(amdgpu_dc_reg_template,
> > +   TP_PROTO(unsigned long *count, uint32_t reg, uint32_t 
> > value),
> > +   TP_ARGS(count, reg, value),
> > -TRACE_EVENT(amdgpu_dc_wreg,
> > -   TP_PROTO(unsigned long *write_count, uint32_t reg, uint32_t value),
> > -   TP_ARGS(write_count, reg, value),
> > -   TP_STRUCT__entry(
> > -   __field(uint32_t, reg)
> > -   __field(uint32_t, value)
> > -   ),
> > -   TP_fast_assign(
> > -   __entry->reg = reg;
> > -   __entry->value = value;
> > -   *write_count = *write_count + 1;
> > -   ),
> > -   TP_printk("reg=0x%08lx, value=0x%08lx",
> > -   (unsigned long)__entry->reg,
> > -   (unsigned long)__entry->value)
> > +   TP_STRUCT__entry(
> > +__field(uint32_t, reg)
> > +__field(uint32_t, value)
> > +   ),
> > +
> > +   TP_fast_assign(
> > +  __entry->reg = reg;
> > +  __entry->value = value;
> > +  *count = *count + 1;
> > +   ),
> > +
> > +   TP_printk("reg=0x%08lx, value=0x%08lx",
> > + (unsigned long)__entry->reg,
> > + (unsigned long)__entry->value)
> >   );
> > +DEFINE_EVENT(amdgpu_dc_reg_template, amdgpu_dc_rreg,
> > +TP_PROTO(unsigned long *count, uint32_t reg, uint32_t value),
> > +TP_ARGS(count, reg, value));
> > +
> > +DEFINE_EVENT(amdgpu_dc_reg_template, amdgpu_dc_wreg,
> > +TP_PROTO(unsigned long *count, uint32_t reg, uint32_t value),
> > +TP_ARGS(count, reg, value));
> >   TRACE_EVENT(amdgpu_dc_performance,
> > TP_PROTO(unsigned long read_count, unsigned long write_count,
> > 
> 

-- 
Rodrigo Siqueira
https://siqueira.tech


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


Re: [PATCH] drm/amdgpu: Enable SDMA utilization for Arcturus

2020-09-11 Thread Felix Kuehling
Am 2020-09-11 um 12:27 p.m. schrieb Mukul Joshi:
> SDMA utilization calculations are enabled/disabled by
> writing to SDMAx_PUB_DUMMY_REG2 register. Currently,
> enable this only for Arcturus.
>
> Signed-off-by: Mukul Joshi 
> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 10 ++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
> b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 856c50386c86..c764c27ba86d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -1063,6 +1063,16 @@ static void sdma_v4_0_ctx_switch_enable(struct 
> amdgpu_device *adev, bool enable)
>   WREG32_SDMA(i, mmSDMA0_PHASE2_QUANTUM, phase_quantum);
>   }
>   WREG32_SDMA(i, mmSDMA0_CNTL, f32_cntl);
> +
> + /*
> +  * Enable SDMA utilization. Its only supported on
> +  * Arcturus for the moment and firmware version 14
> +  * and above.
> +  */
> + if ((adev->asic_type == CHIP_ARCTURUS) &&
> + (adev->sdma.instance[i].fw_version > 13)) {

There are some redundant parentheses in the condition. The curly braces
are not needed for a single statement inside the "if". Also, write >=
14, that correlates better with the statement in the comment:

if (adev->asic_type == CHIP_ARCTURUS &&
adev->sdma.instance[i].fw_version >= 14)
WREG32_SDMA(i, mmSDMA0_PUB_DUMMY_REG2, enable);

If this feature is only available on Arcturus, we should make the
creation of the sysfs entries conditional as well. You can do that in a
follow-up change.

Regards,
  Felix

> + WREG32_SDMA(i, mmSDMA0_PUB_DUMMY_REG2, enable);
> + }
>   }
>  
>  }
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH v2 1/4] drm/amd/display: Rework registers tracepoint

2020-09-11 Thread Kazlauskas, Nicholas

On 2020-09-11 10:59 a.m., Rodrigo Siqueira wrote:

amdgpu_dc_rreg and amdgpu_dc_wreg are very similar, for this reason,
this commits abstract these two events by using DECLARE_EVENT_CLASS and
create an instance of it for each one of these events.

Signed-off-by: Rodrigo Siqueira 


This looks reasonable to me. Does this still show up as 
amdpgu_dc_rrreg/amdgpu_dc_wreg in the captured trace log?


As long as we can still tell this apart you can consider this patch:

Reviewed-by: Nicholas Kazlauskas 

Regards,
Nicholas Kazlauskas


---
  .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 55 ---
  1 file changed, 24 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
index d898981684d5..dd34e11b1079 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
@@ -31,40 +31,33 @@
  
  #include 
  
-TRACE_EVENT(amdgpu_dc_rreg,

-   TP_PROTO(unsigned long *read_count, uint32_t reg, uint32_t value),
-   TP_ARGS(read_count, reg, value),
-   TP_STRUCT__entry(
-   __field(uint32_t, reg)
-   __field(uint32_t, value)
-   ),
-   TP_fast_assign(
-   __entry->reg = reg;
-   __entry->value = value;
-   *read_count = *read_count + 1;
-   ),
-   TP_printk("reg=0x%08lx, value=0x%08lx",
-   (unsigned long)__entry->reg,
-   (unsigned long)__entry->value)
-);
+DECLARE_EVENT_CLASS(amdgpu_dc_reg_template,
+   TP_PROTO(unsigned long *count, uint32_t reg, uint32_t 
value),
+   TP_ARGS(count, reg, value),
  
-TRACE_EVENT(amdgpu_dc_wreg,

-   TP_PROTO(unsigned long *write_count, uint32_t reg, uint32_t value),
-   TP_ARGS(write_count, reg, value),
-   TP_STRUCT__entry(
-   __field(uint32_t, reg)
-   __field(uint32_t, value)
-   ),
-   TP_fast_assign(
-   __entry->reg = reg;
-   __entry->value = value;
-   *write_count = *write_count + 1;
-   ),
-   TP_printk("reg=0x%08lx, value=0x%08lx",
-   (unsigned long)__entry->reg,
-   (unsigned long)__entry->value)
+   TP_STRUCT__entry(
+__field(uint32_t, reg)
+__field(uint32_t, value)
+   ),
+
+   TP_fast_assign(
+  __entry->reg = reg;
+  __entry->value = value;
+  *count = *count + 1;
+   ),
+
+   TP_printk("reg=0x%08lx, value=0x%08lx",
+ (unsigned long)__entry->reg,
+ (unsigned long)__entry->value)
  );
  
+DEFINE_EVENT(amdgpu_dc_reg_template, amdgpu_dc_rreg,

+TP_PROTO(unsigned long *count, uint32_t reg, uint32_t value),
+TP_ARGS(count, reg, value));
+
+DEFINE_EVENT(amdgpu_dc_reg_template, amdgpu_dc_wreg,
+TP_PROTO(unsigned long *count, uint32_t reg, uint32_t value),
+TP_ARGS(count, reg, value));
  
  TRACE_EVENT(amdgpu_dc_performance,

TP_PROTO(unsigned long read_count, unsigned long write_count,



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


Re: [PATCH v2 3/4] drm/amd/display: Add pipe_state tracepoint

2020-09-11 Thread Kazlauskas, Nicholas

On 2020-09-11 10:59 a.m., Rodrigo Siqueira wrote:

This commit introduces a trace mechanism for struct pipe_ctx by adding a
middle layer struct in the amdgpu_dm_trace.h for capturing the most
important data from struct pipe_ctx and showing its data via tracepoint.
This tracepoint was added to dc.c and dcn10_hw_sequencer, however, it
can be added to other DCN architecture.

Co-developed-by: Nicholas Kazlauskas 
Signed-off-by: Nicholas Kazlauskas 
Signed-off-by: Rodrigo Siqueira 



This patch, while very useful, unfortunately pulls in a lot of DM code 
into DC so I would prefer to hold off on this one for now.


It would be better if this had a proper DC interface for tracing/logging 
these states. If the API was more like how we handle tracing register 
reads/writes this would be cleaner.


Regards,
Nicholas Kazlauskas


---
  .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 172 ++
  drivers/gpu/drm/amd/display/dc/core/dc.c  |  11 ++
  .../amd/display/dc/dcn10/dcn10_hw_sequencer.c |  17 +-
  3 files changed, 195 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
index 5fb4c4a5c349..53f62506e17c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
@@ -376,6 +376,178 @@ TRACE_EVENT(amdgpu_dm_atomic_check_finish,
  __entry->async_update, __entry->allow_modeset)
  );
  
+#ifndef _AMDGPU_DM_TRACE_STRUCTS_DEFINED_

+#define _AMDGPU_DM_TRACE_STRUCTS_DEFINED_
+
+struct amdgpu_dm_trace_pipe_state {
+   int pipe_idx;
+   const void *stream;
+   int stream_w;
+   int stream_h;
+   int dst_x;
+   int dst_y;
+   int dst_w;
+   int dst_h;
+   int src_x;
+   int src_y;
+   int src_w;
+   int src_h;
+   int clip_x;
+   int clip_y;
+   int clip_w;
+   int clip_h;
+   int recout_x;
+   int recout_y;
+   int recout_w;
+   int recout_h;
+   int viewport_x;
+   int viewport_y;
+   int viewport_w;
+   int viewport_h;
+   int flip_immediate;
+   int surface_pitch;
+   int format;
+   int swizzle;
+   unsigned int update_flags;
+};
+
+#define fill_out_trace_pipe_state(trace_pipe_state, pipe_ctx) \
+   do { \
+   trace_pipe_state.pipe_idx   = (pipe_ctx)->pipe_idx; \
+   trace_pipe_state.stream = (pipe_ctx)->stream; \
+   trace_pipe_state.stream_w   = 
(pipe_ctx)->stream->timing.h_addressable; \
+   trace_pipe_state.stream_h   = 
(pipe_ctx)->stream->timing.v_addressable; \
+   trace_pipe_state.dst_x  = 
(pipe_ctx)->plane_state->dst_rect.x; \
+   trace_pipe_state.dst_y  = 
(pipe_ctx)->plane_state->dst_rect.y; \
+   trace_pipe_state.dst_w  = 
(pipe_ctx)->plane_state->dst_rect.width; \
+   trace_pipe_state.dst_h  = 
(pipe_ctx)->plane_state->dst_rect.height; \
+   trace_pipe_state.src_x  = 
(pipe_ctx)->plane_state->src_rect.x; \
+   trace_pipe_state.src_y  = 
(pipe_ctx)->plane_state->src_rect.y; \
+   trace_pipe_state.src_w  = 
(pipe_ctx)->plane_state->src_rect.width; \
+   trace_pipe_state.src_h  = 
(pipe_ctx)->plane_state->src_rect.height; \
+   trace_pipe_state.clip_x = 
(pipe_ctx)->plane_state->clip_rect.x; \
+   trace_pipe_state.clip_y = 
(pipe_ctx)->plane_state->clip_rect.y; \
+   trace_pipe_state.clip_w = 
(pipe_ctx)->plane_state->clip_rect.width; \
+   trace_pipe_state.clip_h = 
(pipe_ctx)->plane_state->clip_rect.height; \
+   trace_pipe_state.recout_x   = 
(pipe_ctx)->plane_res.scl_data.recout.x; \
+   trace_pipe_state.recout_y   = 
(pipe_ctx)->plane_res.scl_data.recout.y; \
+   trace_pipe_state.recout_w   = 
(pipe_ctx)->plane_res.scl_data.recout.width; \
+   trace_pipe_state.recout_h   = 
(pipe_ctx)->plane_res.scl_data.recout.height; \
+   trace_pipe_state.viewport_x = 
(pipe_ctx)->plane_res.scl_data.viewport.x; \
+   trace_pipe_state.viewport_y = 
(pipe_ctx)->plane_res.scl_data.viewport.y; \
+   trace_pipe_state.viewport_w = 
(pipe_ctx)->plane_res.scl_data.viewport.width; \
+   trace_pipe_state.viewport_h = 
(pipe_ctx)->plane_res.scl_data.viewport.height; \
+   trace_pipe_state.flip_immediate = 
(pipe_ctx)->plane_state->flip_immediate; \
+   trace_pipe_state.surface_pitch  = 
(pipe_ctx)->plane_state->plane_size.surface_pitch; \
+   trace_pipe_state.format = 
(pipe_ctx)->plane_state->format; \
+   trace_pipe_state.swizzle= 
(pipe_ctx)->plane_state->tiling_info.gfx9.swizzle; \
+   

Re: [PATCH v2 3/3] drm/amd/display: Move disable interrupt into commit tail

2020-09-11 Thread Kazlauskas, Nicholas

On 2020-09-11 12:27 p.m., Aurabindo Pillai wrote:

[Why]
Since there is no need for accessing crtc state in the interrupt
handler, interrupts need not be disabled well in advance, and
can be moved to commit_tail where it should be.

Signed-off-by: Aurabindo Pillai 
---
  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 41 ++-
  1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1455acf5beca..b5af1f692499 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7488,34 +7488,6 @@ static int amdgpu_dm_atomic_commit(struct drm_device 
*dev,
   struct drm_atomic_state *state,
   bool nonblock)
  {
-   struct drm_crtc *crtc;
-   struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-   struct amdgpu_device *adev = drm_to_adev(dev);
-   int i;
-
-   /*
-* We evade vblank and pflip interrupts on CRTCs that are undergoing
-* a modeset, being disabled, or have no active planes.
-*
-* It's done in atomic commit rather than commit tail for now since
-* some of these interrupt handlers access the current CRTC state and
-* potentially the stream pointer itself.
-*
-* Since the atomic state is swapped within atomic commit and not within
-* commit tail this would leave to new state (that hasn't been 
committed yet)
-* being accesssed from within the handlers.
-*
-* TODO: Fix this so we can do this in commit tail and not have to block
-* in atomic check.
-*/
-   for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
-   struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
-
-   if (old_crtc_state->active &&
-   (!new_crtc_state->active ||
-drm_atomic_crtc_needs_modeset(new_crtc_state)))
-   manage_dm_interrupts(adev, acrtc, false);
-   }
/*
 * Add check here for SoC's that support hardware cursor plane, to
 * unset legacy_cursor_update
@@ -7566,6 +7538,19 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
dc_resource_state_copy_construct_current(dm->dc, dc_state);
}
  
+	for_each_oldnew_crtc_in_state (state, crtc, old_crtc_state,

+  new_crtc_state, i) {
+   acrtc = to_amdgpu_crtc(crtc);
+   dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+
+   if (old_crtc_state->active &&
+   (!new_crtc_state->active ||
+drm_atomic_crtc_needs_modeset(new_crtc_state))) {
+   manage_dm_interrupts(adev, acrtc, false);
+   dc_stream_release(dm_old_crtc_state->stream);


Please include this dc_stream_release() in patch #2 as well to prevent 
memory leaks during bisections.


With that change, this series is Reviewed-by: Nicholas Kazlauskas 



Regards,
Nicholas Kazlauskas


+   }
+   }
+
/* update changed items */
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);



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


Re: [PATCH 2/3] drm/amd/display: Refactor to prevent crtc state access in DM IRQ handler

2020-09-11 Thread Aurabindo Pillai
On Wed, 2020-09-09 at 11:00 -0400, Kazlauskas, Nicholas wrote:
> On 2020-09-09 10:28 a.m., Aurabindo Pillai wrote:
> > [Why]Currently commit_tail holds global locks and wait for
> > dependencies which isagainst the DRM API contracts. Inorder to fix
> > this, IRQ handler should be ableto run without having to access
> > crtc state. Required parameters are copied overso that they can be
> > directly accessed from the interrupt handler
> > Signed-off-by: Aurabindo Pillai 
> > ---  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 115
> > ++  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> > |   1 -  .../display/amdgpu_dm/amdgpu_dm_irq_params.h  |   4 +  3
> > files changed, 68 insertions(+), 52 deletions(-)
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.cindex
> > 40814cdd8c92..0603436a3313 100644---
> > a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c+++
> > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c@@ -192,17
> > +192,14 @@ static u32 dm_vblank_get_counter(struct amdgpu_device
> > *adev, int crtc)return 0;   else {  str
> > uct amdgpu_crtc *acrtc = adev->mode_info.crtcs[crtc];-  
> > struct dm_crtc_state *acrtc_state = to_dm_crtc_state(-  
> > acrtc->base.state);  -- if (acrtc_state-
> > >stream == NULL) {+ if (acrtc->dm_irq_params.stream ==
> > NULL) { DRM_ERROR("dc_stream_state is NULL for
> > crtc '%d'!\n",crtc);
> > return 0;   }  -return
> > dc_stream_get_vblank_counter(acrtc_state->stream);+ return
> > dc_stream_get_vblank_counter(acrtc->dm_irq_params.stream);  
> > }  }  @@ -215,10 +212,8 @@ static int dm_crtc_get_scanoutpos(struct
> > amdgpu_device *adev, int crtc,  return -EINVAL; els
> > e { struct amdgpu_crtc *acrtc = adev-
> > >mode_info.crtcs[crtc];-struct dm_crtc_state
> > *acrtc_state = to_dm_crtc_state(-   
> > acrtc->base.state);  -  if (acrtc_state->stream
> > ==  NULL) {+if (acrtc->dm_irq_params.stream
> > ==  NULL) { DRM_ERROR("dc_stream_state is
> > NULL for crtc '%d'!\n",   crtc);
> > return 0;@@ -228,7 +223,7 @@ static int
> > dm_crtc_get_scanoutpos(struct amdgpu_device *adev, int crtc,
> >  * TODO rework base driver to use values directly.  
> >  * for now parse it back into reg-format */-
> > dc_stream_get_scanoutpos(acrtc_state->stream,+  dc_stre
> > am_get_scanoutpos(acrtc->dm_irq_params.stream,  
> >  _blank_start, 
> > _blank_end,  _position,@@
> > -287,6 +282,14 @@ get_crtc_by_otg_inst(struct amdgpu_device
> > *adev,  return NULL;  }  +static inline bool
> > amdgpu_dm_vrr_active_irq(struct amdgpu_crtc *acrtc)+{+  return
> > acrtc->dm_irq_params.freesync_config.state ==+ 
> > VRR_STATE_ACTIVE_VARIABLE ||+  acrtc-
> > >dm_irq_params.freesync_config.state ==+   VRR_STAT
> > E_ACTIVE_FIXED;+}+  static inline bool amdgpu_dm_vrr_active(struct
> > dm_crtc_state *dm_state)  { return dm_state-
> > >freesync_config.state == VRR_STATE_ACTIVE_VARIABLE ||@@ -307,7
> > +310,6 @@ static void dm_pflip_high_irq(void *interrupt_params) 
> > struct amdgpu_device *adev = irq_params->adev;  unsigned long
> > flags;  struct drm_pending_vblank_event *e;-struct
> > dm_crtc_state *acrtc_state; uint32_t vpos, hpos,
> > v_blank_start, v_blank_end; bool vrr_active;  @@ -339,12
> > +341,11 @@ static void dm_pflip_high_irq(void *interrupt_params)
> > if (!e) WARN_ON(1);  -  acrtc_state =
> > to_dm_crtc_state(amdgpu_crtc->base.state);- vrr_active =
> > amdgpu_dm_vrr_active(acrtc_state);+ vrr_active =
> > amdgpu_dm_vrr_active_irq(amdgpu_crtc);  /* Fixed refresh rate,
> > or VRR scanout position outside front-porch? */ if (!vrr_active
> > ||- !dc_stream_get_scanoutpos(acrtc_state->stream,
> > _blank_start,+!dc_stream_get_scanoutpos(amdgpu_crtc-
> > >dm_irq_params.stream, _blank_start,  
> >   _blank_end, , ) ||(vpos <
> > v_blank_start)) {   /* Update to correct count and vblank
> > timestamp if racing with@@ -405,17 +406,17 @@ static void
> > dm_vupdate_high_irq(void *interrupt_params) struct
> > common_irq_params *irq_params = interrupt_params;   struct
> > amdgpu_device *adev = irq_params->adev; struct amdgpu_crtc
> > *acrtc;-struct dm_crtc_state *acrtc_state;  unsigned
> > long flags;+int vrr_active; acrtc =
> > get_crtc_by_otg_inst(adev, 

[PATCH] drm/amdgpu: Enable SDMA utilization for Arcturus

2020-09-11 Thread Mukul Joshi
SDMA utilization calculations are enabled/disabled by
writing to SDMAx_PUB_DUMMY_REG2 register. Currently,
enable this only for Arcturus.

Signed-off-by: Mukul Joshi 
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c 
b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 856c50386c86..c764c27ba86d 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -1063,6 +1063,16 @@ static void sdma_v4_0_ctx_switch_enable(struct 
amdgpu_device *adev, bool enable)
WREG32_SDMA(i, mmSDMA0_PHASE2_QUANTUM, phase_quantum);
}
WREG32_SDMA(i, mmSDMA0_CNTL, f32_cntl);
+
+   /*
+* Enable SDMA utilization. Its only supported on
+* Arcturus for the moment and firmware version 14
+* and above.
+*/
+   if ((adev->asic_type == CHIP_ARCTURUS) &&
+   (adev->sdma.instance[i].fw_version > 13)) {
+   WREG32_SDMA(i, mmSDMA0_PUB_DUMMY_REG2, enable);
+   }
}
 
 }
-- 
2.17.1

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


[PATCH v2 0/3] Refactor DM IRQ handling

2020-09-11 Thread Aurabindo Pillai
Changes in V2:
 * fix memory leak.

Interrupts are disabled too early in DM's atomic_commit() which can
cause issues in certain situations with non blocking commits timing
out on flip_done interrupt. The early disabling of interrupts were
necessary due to interrupts accessing crtc state directly. This
refactor removes direct access of crtc state from the irq handler
so that disabling of interrupts can be done later in commit_tail()

--

Aurabindo Pillai (3):
  drm/amdgpu: Move existing pflip fields into separate struct
  drm/amd/display: Refactor to prevent crtc state access in DM IRQ
handler
  drm/amd/display: Move disable interrupt into commit tail

 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   4 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 156 +-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   1 -
 .../display/amdgpu_dm/amdgpu_dm_irq_params.h  |  37 +
 4 files changed, 115 insertions(+), 83 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq_params.h

-- 
2.25.1

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


[PATCH v2 1/3] drm/amdgpu: Move existing pflip fields into separate struct

2020-09-11 Thread Aurabindo Pillai
[Why]
To refactor DM IRQ management, all fields used by IRQ is best moved
to a separate struct so that main amdgpu_crtc struct need not be changed
Location of the new struct shall be in DM

Signed-off-by: Aurabindo Pillai 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  4 ++-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  4 +--
 .../display/amdgpu_dm/amdgpu_dm_irq_params.h  | 33 +++
 3 files changed, 38 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq_params.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index aa3e22be4f2d..345cb0464370 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -46,6 +46,7 @@
 
 #include 
 #include "modules/inc/mod_freesync.h"
+#include "amdgpu_dm_irq_params.h"
 
 struct amdgpu_bo;
 struct amdgpu_device;
@@ -410,7 +411,8 @@ struct amdgpu_crtc {
struct amdgpu_flip_work *pflip_works;
enum amdgpu_flip_status pflip_status;
int deferred_flip_completion;
-   u32 last_flip_vblank;
+   /* parameters access from DM IRQ handler */
+   struct dm_irq_params dm_irq_params;
/* pll sharing */
struct amdgpu_atom_ss ss;
bool ss_enabled;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index cb624ee70545..40814cdd8c92 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -389,7 +389,7 @@ static void dm_pflip_high_irq(void *interrupt_params)
 * of pageflip completion, so last_flip_vblank is the forbidden count
 * for queueing new pageflips if vsync + VRR is enabled.
 */
-   amdgpu_crtc->last_flip_vblank =
+   amdgpu_crtc->dm_irq_params.last_flip_vblank =
amdgpu_get_vblank_counter_kms(_crtc->base);
 
amdgpu_crtc->pflip_status = AMDGPU_FLIP_NONE;
@@ -7248,7 +7248,7 @@ static void amdgpu_dm_commit_planes(struct 
drm_atomic_state *state,
 * on late submission of flips.
 */
spin_lock_irqsave(>dev->event_lock, flags);
-   last_flip_vblank = acrtc_attach->last_flip_vblank;
+   last_flip_vblank = 
acrtc_attach->dm_irq_params.last_flip_vblank;
spin_unlock_irqrestore(>dev->event_lock, flags);
}
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq_params.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq_params.h
new file mode 100644
index ..55ef237eed8b
--- /dev/null
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq_params.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2020 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors: AMD
+ *
+ */
+
+#ifndef __AMDGPU_DM_IRQ_PARAMS_H__
+#define __AMDGPU_DM_IRQ_PARAMS_H__
+
+struct dm_irq_params {
+   u32 last_flip_vblank;
+};
+
+#endif /* __AMDGPU_DM_IRQ_PARAMS_H__ */
-- 
2.25.1

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


[PATCH v2 2/3] drm/amd/display: Refactor to prevent crtc state access in DM IRQ handler

2020-09-11 Thread Aurabindo Pillai
[Why]
Currently commit_tail holds global locks and wait for dependencies which is
against the DRM API contracts. Inorder to fix this, IRQ handler should be able
to run without having to access crtc state. Required parameters are copied over
so that they can be directly accessed from the interrupt handler

Signed-off-by: Aurabindo Pillai 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 111 ++
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   1 -
 .../display/amdgpu_dm/amdgpu_dm_irq_params.h  |   4 +
 3 files changed, 64 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 40814cdd8c92..1455acf5beca 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -192,17 +192,14 @@ static u32 dm_vblank_get_counter(struct amdgpu_device 
*adev, int crtc)
return 0;
else {
struct amdgpu_crtc *acrtc = adev->mode_info.crtcs[crtc];
-   struct dm_crtc_state *acrtc_state = to_dm_crtc_state(
-   acrtc->base.state);
 
-
-   if (acrtc_state->stream == NULL) {
+   if (acrtc->dm_irq_params.stream == NULL) {
DRM_ERROR("dc_stream_state is NULL for crtc '%d'!\n",
  crtc);
return 0;
}
 
-   return dc_stream_get_vblank_counter(acrtc_state->stream);
+   return 
dc_stream_get_vblank_counter(acrtc->dm_irq_params.stream);
}
 }
 
@@ -215,10 +212,8 @@ static int dm_crtc_get_scanoutpos(struct amdgpu_device 
*adev, int crtc,
return -EINVAL;
else {
struct amdgpu_crtc *acrtc = adev->mode_info.crtcs[crtc];
-   struct dm_crtc_state *acrtc_state = to_dm_crtc_state(
-   acrtc->base.state);
 
-   if (acrtc_state->stream ==  NULL) {
+   if (acrtc->dm_irq_params.stream ==  NULL) {
DRM_ERROR("dc_stream_state is NULL for crtc '%d'!\n",
  crtc);
return 0;
@@ -228,7 +223,7 @@ static int dm_crtc_get_scanoutpos(struct amdgpu_device 
*adev, int crtc,
 * TODO rework base driver to use values directly.
 * for now parse it back into reg-format
 */
-   dc_stream_get_scanoutpos(acrtc_state->stream,
+   dc_stream_get_scanoutpos(acrtc->dm_irq_params.stream,
 _blank_start,
 _blank_end,
 _position,
@@ -287,6 +282,14 @@ get_crtc_by_otg_inst(struct amdgpu_device *adev,
return NULL;
 }
 
+static inline bool amdgpu_dm_vrr_active_irq(struct amdgpu_crtc *acrtc)
+{
+   return acrtc->dm_irq_params.freesync_config.state ==
+  VRR_STATE_ACTIVE_VARIABLE ||
+  acrtc->dm_irq_params.freesync_config.state ==
+  VRR_STATE_ACTIVE_FIXED;
+}
+
 static inline bool amdgpu_dm_vrr_active(struct dm_crtc_state *dm_state)
 {
return dm_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE ||
@@ -307,7 +310,6 @@ static void dm_pflip_high_irq(void *interrupt_params)
struct amdgpu_device *adev = irq_params->adev;
unsigned long flags;
struct drm_pending_vblank_event *e;
-   struct dm_crtc_state *acrtc_state;
uint32_t vpos, hpos, v_blank_start, v_blank_end;
bool vrr_active;
 
@@ -339,12 +341,11 @@ static void dm_pflip_high_irq(void *interrupt_params)
if (!e)
WARN_ON(1);
 
-   acrtc_state = to_dm_crtc_state(amdgpu_crtc->base.state);
-   vrr_active = amdgpu_dm_vrr_active(acrtc_state);
+   vrr_active = amdgpu_dm_vrr_active_irq(amdgpu_crtc);
 
/* Fixed refresh rate, or VRR scanout position outside front-porch? */
if (!vrr_active ||
-   !dc_stream_get_scanoutpos(acrtc_state->stream, _blank_start,
+   !dc_stream_get_scanoutpos(amdgpu_crtc->dm_irq_params.stream, 
_blank_start,
  _blank_end, , ) ||
(vpos < v_blank_start)) {
/* Update to correct count and vblank timestamp if racing with
@@ -405,17 +406,17 @@ static void dm_vupdate_high_irq(void *interrupt_params)
struct common_irq_params *irq_params = interrupt_params;
struct amdgpu_device *adev = irq_params->adev;
struct amdgpu_crtc *acrtc;
-   struct dm_crtc_state *acrtc_state;
unsigned long flags;
+   int vrr_active;
 
acrtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - 
IRQ_TYPE_VUPDATE);
 
if (acrtc) {
-   acrtc_state = to_dm_crtc_state(acrtc->base.state);
+   vrr_active = amdgpu_dm_vrr_active_irq(acrtc);
 

[PATCH v2 3/3] drm/amd/display: Move disable interrupt into commit tail

2020-09-11 Thread Aurabindo Pillai
[Why]
Since there is no need for accessing crtc state in the interrupt
handler, interrupts need not be disabled well in advance, and
can be moved to commit_tail where it should be.

Signed-off-by: Aurabindo Pillai 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 41 ++-
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 1455acf5beca..b5af1f692499 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7488,34 +7488,6 @@ static int amdgpu_dm_atomic_commit(struct drm_device 
*dev,
   struct drm_atomic_state *state,
   bool nonblock)
 {
-   struct drm_crtc *crtc;
-   struct drm_crtc_state *old_crtc_state, *new_crtc_state;
-   struct amdgpu_device *adev = drm_to_adev(dev);
-   int i;
-
-   /*
-* We evade vblank and pflip interrupts on CRTCs that are undergoing
-* a modeset, being disabled, or have no active planes.
-*
-* It's done in atomic commit rather than commit tail for now since
-* some of these interrupt handlers access the current CRTC state and
-* potentially the stream pointer itself.
-*
-* Since the atomic state is swapped within atomic commit and not within
-* commit tail this would leave to new state (that hasn't been 
committed yet)
-* being accesssed from within the handlers.
-*
-* TODO: Fix this so we can do this in commit tail and not have to block
-* in atomic check.
-*/
-   for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
-   struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
-
-   if (old_crtc_state->active &&
-   (!new_crtc_state->active ||
-drm_atomic_crtc_needs_modeset(new_crtc_state)))
-   manage_dm_interrupts(adev, acrtc, false);
-   }
/*
 * Add check here for SoC's that support hardware cursor plane, to
 * unset legacy_cursor_update
@@ -7566,6 +7538,19 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
dc_resource_state_copy_construct_current(dm->dc, dc_state);
}
 
+   for_each_oldnew_crtc_in_state (state, crtc, old_crtc_state,
+  new_crtc_state, i) {
+   acrtc = to_amdgpu_crtc(crtc);
+   dm_old_crtc_state = to_dm_crtc_state(old_crtc_state);
+
+   if (old_crtc_state->active &&
+   (!new_crtc_state->active ||
+drm_atomic_crtc_needs_modeset(new_crtc_state))) {
+   manage_dm_interrupts(adev, acrtc, false);
+   dc_stream_release(dm_old_crtc_state->stream);
+   }
+   }
+
/* update changed items */
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
-- 
2.25.1

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


[PATCH v2 4/4] drm/amd/display: Add tracepoint for capturing clocks state

2020-09-11 Thread Rodrigo Siqueira
The clock state update is the source of many problems, and capturing
this sort of information helps debug. This commit introduces tracepoints
for capturing clock values and also add traces in DCE, DCN1, DCN2x, and
DCN3.

Co-developed-by: Nicholas Kazlauskas 
Signed-off-by: Nicholas Kazlauskas 
Signed-off-by: Rodrigo Siqueira 
---
 .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 198 ++
 .../dc/clk_mgr/dce112/dce112_clk_mgr.c|   5 +
 .../display/dc/clk_mgr/dcn10/rv1_clk_mgr.c|   4 +
 .../display/dc/clk_mgr/dcn20/dcn20_clk_mgr.c  |   4 +
 .../amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c |   4 +
 .../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c  |   4 +
 .../gpu/drm/amd/display/dc/dce/dce_clk_mgr.c  |   5 +
 7 files changed, 224 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
index 53f62506e17c..fb22b233224a 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
@@ -411,6 +411,42 @@ struct amdgpu_dm_trace_pipe_state {
unsigned int update_flags;
 };
 
+struct amdgpu_dm_trace_dc_clocks_state {
+   int dispclk_khz;
+   int dppclk_khz;
+   int disp_dpp_voltage_level_khz;
+   int dcfclk_khz;
+   int socclk_khz;
+   int dcfclk_deep_sleep_khz;
+   int fclk_khz;
+   int phyclk_khz;
+   int dramclk_khz;
+   bool p_state_change_support;
+   int pwr_state;
+   bool prev_p_state_change_support;
+   int dtm_level;
+   int max_supported_dppclk_khz;
+   int max_supported_dispclk_khz;
+   int bw_dppclk_khz;
+   int bw_dispclk_khz;
+   int safe_to_lower;
+};
+
+struct amdgpu_dm_trace_dce_clocks_state {
+   bool cpuc_state_change_enable;
+   bool cpup_state_change_enable;
+   bool stutter_mode_enable;
+   bool nbp_state_change_enable;
+   bool all_displays_in_sync;
+   int sclk_khz;
+   int sclk_deep_sleep_khz;
+   int yclk_khz;
+   int dispclk_khz;
+   int blackout_recovery_time_us;
+   int patched_disp_clk;
+   int safe_to_lower;
+};
+
 #define fill_out_trace_pipe_state(trace_pipe_state, pipe_ctx) \
do { \
trace_pipe_state.pipe_idx   = (pipe_ctx)->pipe_idx; \
@@ -444,6 +480,44 @@ struct amdgpu_dm_trace_pipe_state {
trace_pipe_state.update_flags   = (pipe_ctx)->update_flags.raw; 
\
} while (0)
 
+#define fill_out_trace_clock_state(trace_clock_state, clocks, safe_to_lower) \
+   do { \
+   trace_clock_state.dispclk_khz = (clocks)->dispclk_khz; \
+   trace_clock_state.dppclk_khz = (clocks)->dppclk_khz; \
+   trace_clock_state.disp_dpp_voltage_level_khz = 
(clocks)->disp_dpp_voltage_level_khz; \
+   trace_clock_state.dcfclk_khz = (clocks)->dcfclk_khz; \
+   trace_clock_state.socclk_khz = (clocks)->socclk_khz; \
+   trace_clock_state.dcfclk_deep_sleep_khz = 
(clocks)->dcfclk_deep_sleep_khz; \
+   trace_clock_state.fclk_khz = (clocks)->fclk_khz; \
+   trace_clock_state.phyclk_khz = (clocks)->phyclk_khz; \
+   trace_clock_state.dramclk_khz = (clocks)->dramclk_khz; \
+   trace_clock_state.p_state_change_support = 
(clocks)->p_state_change_support; \
+   trace_clock_state.pwr_state = (clocks)->pwr_state; \
+   trace_clock_state.prev_p_state_change_support = 
(clocks)->prev_p_state_change_support; \
+   trace_clock_state.dtm_level = (clocks)->dtm_level; \
+   trace_clock_state.max_supported_dppclk_khz = 
(clocks)->max_supported_dppclk_khz; \
+   trace_clock_state.max_supported_dispclk_khz = 
(clocks)->max_supported_dispclk_khz; \
+   trace_clock_state.bw_dppclk_khz = (clocks)->bw_dppclk_khz; \
+   trace_clock_state.bw_dispclk_khz = (clocks)->bw_dispclk_khz; \
+   trace_clock_state.safe_to_lower = safe_to_lower; \
+   } while (0)
+
+#define fill_out_trace_dce_clock_state(trace_clock_state, clocks, 
patched_disp_clk, safe_to_lower) \
+   do { \
+   trace_clock_state.cpuc_state_change_enable = 
(clocks)->cpuc_state_change_enable; \
+   trace_clock_state.cpup_state_change_enable = 
(clocks)->cpup_state_change_enable; \
+   trace_clock_state.stutter_mode_enable = 
(clocks)->stutter_mode_enable; \
+   trace_clock_state.nbp_state_change_enable = 
(clocks)->nbp_state_change_enable; \
+   trace_clock_state.all_displays_in_sync = 
(clocks)->all_displays_in_sync; \
+   trace_clock_state.sclk_khz = (clocks)->sclk_khz; \
+   trace_clock_state.sclk_deep_sleep_khz = 
(clocks)->sclk_deep_sleep_khz; \
+   trace_clock_state.yclk_khz = (clocks)->yclk_khz; \
+   trace_clock_state.dispclk_khz = (clocks)->dispclk_khz; \
+   

[PATCH v2 0/4] Enlarge tracepoints in the display component

2020-09-11 Thread Rodrigo Siqueira
Debug issues related to display can be a challenge due to the complexity
around this topic and different source of information might help in this
process. We already have support for tracepoints inside the display
component, i.e., we have the basic functionalities available and we just
need to expand it in order to make it more valuable for debugging. For
this reason, this patchset reworks part of the current tracepoint
options and add different sets of tracing inside amdgpu_dm, display
core, and DCN10. The first patch of this series just rework part of the
current tracepoints and the last set of patches introduces new
tracepoints.

This first patchset version is functional. Please, let me know what I
can improve in the current version but also let me know what kind of
tracepoint I can add for the next version. 

Finally, I want to highlight that this work is based on a set of patches
originally made by Nicholas Kazlauskas.

Change in V2:
- I added another patch for capturing the clock state for different display
  architecture.

Rodrigo Siqueira (4):
  drm/amd/display: Rework registers tracepoint
  drm/amd/display: Add tracepoint for amdgpu_dm
  drm/amd/display: Add pipe_state tracepoint
  drm/amd/display: Add tracepoint for capturing clocks state

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  17 +
 .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 712 +-
 .../dc/clk_mgr/dce112/dce112_clk_mgr.c|   5 +
 .../display/dc/clk_mgr/dcn10/rv1_clk_mgr.c|   4 +
 .../display/dc/clk_mgr/dcn20/dcn20_clk_mgr.c  |   4 +
 .../amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c |   4 +
 .../display/dc/clk_mgr/dcn30/dcn30_clk_mgr.c  |   4 +
 drivers/gpu/drm/amd/display/dc/core/dc.c  |  11 +
 .../gpu/drm/amd/display/dc/dce/dce_clk_mgr.c  |   5 +
 .../amd/display/dc/dcn10/dcn10_hw_sequencer.c |  17 +-
 10 files changed, 747 insertions(+), 36 deletions(-)

-- 
2.28.0

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


[PATCH v2 2/4] drm/amd/display: Add tracepoint for amdgpu_dm

2020-09-11 Thread Rodrigo Siqueira
Debug amdgpu_dm could be a complicated task, therefore, this commit adds
tracepoints in some convenient functions such as plane and connector
check inside amdgpu_dm.

Co-developed-by: Nicholas Kazlauskas 
Signed-off-by: Nicholas Kazlauskas 
Signed-off-by: Rodrigo Siqueira 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  17 ++
 .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 287 ++
 2 files changed, 304 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index cb624ee70545..552ca67c2a71 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5424,6 +5424,8 @@ amdgpu_dm_connector_atomic_check(struct drm_connector 
*conn,
struct drm_crtc_state *new_crtc_state;
int ret;
 
+   trace_amdgpu_dm_connector_atomic_check(new_con_state);
+
if (!crtc)
return 0;
 
@@ -5542,6 +5544,8 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc 
*crtc,
struct dm_crtc_state *dm_crtc_state = to_dm_crtc_state(state);
int ret = -EINVAL;
 
+   trace_amdgpu_dm_crtc_atomic_check(state);
+
dm_update_crtc_active_planes(crtc, state);
 
if (unlikely(!dm_crtc_state->stream &&
@@ -5916,6 +5920,8 @@ static int dm_plane_atomic_check(struct drm_plane *plane,
struct drm_crtc_state *new_crtc_state;
int ret;
 
+   trace_amdgpu_dm_plane_atomic_check(state);
+
dm_plane_state = to_dm_plane_state(state);
 
if (!dm_plane_state->dc_state)
@@ -5956,6 +5962,8 @@ static void dm_plane_atomic_async_update(struct drm_plane 
*plane,
struct drm_plane_state *old_state =
drm_atomic_get_old_plane_state(new_state->state, plane);
 
+   trace_amdgpu_dm_atomic_update_cursor(new_state);
+
swap(plane->state->fb, new_state->fb);
 
plane->state->src_x = new_state->src_x;
@@ -7546,6 +7554,8 @@ static void amdgpu_dm_atomic_commit_tail(struct 
drm_atomic_state *state)
int crtc_disable_count = 0;
bool mode_set_reset_required = false;
 
+   trace_amdgpu_dm_atomic_commit_tail_begin(state);
+
drm_atomic_helper_update_legacy_modeset_state(dev, state);
 
dm_state = dm_atomic_get_new_state(state);
@@ -8616,6 +8626,8 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
int ret, i;
bool lock_and_validation_needed = false;
 
+   trace_amdgpu_dm_atomic_check_begin(state);
+
ret = drm_atomic_helper_check_modeset(dev, state);
if (ret)
goto fail;
@@ -8912,6 +8924,9 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 
/* Must be success */
WARN_ON(ret);
+
+   trace_amdgpu_dm_atomic_check_finish(state, ret);
+
return ret;
 
 fail:
@@ -8922,6 +8937,8 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
else
DRM_DEBUG_DRIVER("Atomic check failed with err: %d \n", ret);
 
+   trace_amdgpu_dm_atomic_check_finish(state, ret);
+
return ret;
 }
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
index dd34e11b1079..5fb4c4a5c349 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
@@ -30,6 +30,12 @@
 #define _AMDGPU_DM_TRACE_H_
 
 #include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
 
 DECLARE_EVENT_CLASS(amdgpu_dc_reg_template,
TP_PROTO(unsigned long *count, uint32_t reg, uint32_t 
value),
@@ -89,6 +95,287 @@ TRACE_EVENT(amdgpu_dc_performance,
(unsigned long)__entry->write_delta,
(unsigned long)__entry->writes)
 );
+
+TRACE_EVENT(amdgpu_dm_connector_atomic_check,
+   TP_PROTO(const struct drm_connector_state *state),
+   TP_ARGS(state),
+
+   TP_STRUCT__entry(
+__field(uint32_t, conn_id)
+__field(const struct drm_connector_state *, 
conn_state)
+__field(const struct drm_atomic_state *, state)
+__field(const struct drm_crtc_commit *, commit)
+__field(uint32_t, crtc_id)
+__field(uint32_t, best_encoder_id)
+__field(enum drm_link_status, link_status)
+__field(bool, self_refresh_aware)
+__field(enum hdmi_picture_aspect, 
picture_aspect_ratio)
+__field(unsigned int, content_type)
+__field(unsigned int, hdcp_content_type)
+__field(unsigned int, content_protection)
+__field(unsigned int, scaling_mode)
+__field(u32, colorspace)
+   

[PATCH v2 3/4] drm/amd/display: Add pipe_state tracepoint

2020-09-11 Thread Rodrigo Siqueira
This commit introduces a trace mechanism for struct pipe_ctx by adding a
middle layer struct in the amdgpu_dm_trace.h for capturing the most
important data from struct pipe_ctx and showing its data via tracepoint.
This tracepoint was added to dc.c and dcn10_hw_sequencer, however, it
can be added to other DCN architecture.

Co-developed-by: Nicholas Kazlauskas 
Signed-off-by: Nicholas Kazlauskas 
Signed-off-by: Rodrigo Siqueira 
---
 .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 172 ++
 drivers/gpu/drm/amd/display/dc/core/dc.c  |  11 ++
 .../amd/display/dc/dcn10/dcn10_hw_sequencer.c |  17 +-
 3 files changed, 195 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
index 5fb4c4a5c349..53f62506e17c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
@@ -376,6 +376,178 @@ TRACE_EVENT(amdgpu_dm_atomic_check_finish,
  __entry->async_update, __entry->allow_modeset)
 );
 
+#ifndef _AMDGPU_DM_TRACE_STRUCTS_DEFINED_
+#define _AMDGPU_DM_TRACE_STRUCTS_DEFINED_
+
+struct amdgpu_dm_trace_pipe_state {
+   int pipe_idx;
+   const void *stream;
+   int stream_w;
+   int stream_h;
+   int dst_x;
+   int dst_y;
+   int dst_w;
+   int dst_h;
+   int src_x;
+   int src_y;
+   int src_w;
+   int src_h;
+   int clip_x;
+   int clip_y;
+   int clip_w;
+   int clip_h;
+   int recout_x;
+   int recout_y;
+   int recout_w;
+   int recout_h;
+   int viewport_x;
+   int viewport_y;
+   int viewport_w;
+   int viewport_h;
+   int flip_immediate;
+   int surface_pitch;
+   int format;
+   int swizzle;
+   unsigned int update_flags;
+};
+
+#define fill_out_trace_pipe_state(trace_pipe_state, pipe_ctx) \
+   do { \
+   trace_pipe_state.pipe_idx   = (pipe_ctx)->pipe_idx; \
+   trace_pipe_state.stream = (pipe_ctx)->stream; \
+   trace_pipe_state.stream_w   = 
(pipe_ctx)->stream->timing.h_addressable; \
+   trace_pipe_state.stream_h   = 
(pipe_ctx)->stream->timing.v_addressable; \
+   trace_pipe_state.dst_x  = 
(pipe_ctx)->plane_state->dst_rect.x; \
+   trace_pipe_state.dst_y  = 
(pipe_ctx)->plane_state->dst_rect.y; \
+   trace_pipe_state.dst_w  = 
(pipe_ctx)->plane_state->dst_rect.width; \
+   trace_pipe_state.dst_h  = 
(pipe_ctx)->plane_state->dst_rect.height; \
+   trace_pipe_state.src_x  = 
(pipe_ctx)->plane_state->src_rect.x; \
+   trace_pipe_state.src_y  = 
(pipe_ctx)->plane_state->src_rect.y; \
+   trace_pipe_state.src_w  = 
(pipe_ctx)->plane_state->src_rect.width; \
+   trace_pipe_state.src_h  = 
(pipe_ctx)->plane_state->src_rect.height; \
+   trace_pipe_state.clip_x = 
(pipe_ctx)->plane_state->clip_rect.x; \
+   trace_pipe_state.clip_y = 
(pipe_ctx)->plane_state->clip_rect.y; \
+   trace_pipe_state.clip_w = 
(pipe_ctx)->plane_state->clip_rect.width; \
+   trace_pipe_state.clip_h = 
(pipe_ctx)->plane_state->clip_rect.height; \
+   trace_pipe_state.recout_x   = 
(pipe_ctx)->plane_res.scl_data.recout.x; \
+   trace_pipe_state.recout_y   = 
(pipe_ctx)->plane_res.scl_data.recout.y; \
+   trace_pipe_state.recout_w   = 
(pipe_ctx)->plane_res.scl_data.recout.width; \
+   trace_pipe_state.recout_h   = 
(pipe_ctx)->plane_res.scl_data.recout.height; \
+   trace_pipe_state.viewport_x = 
(pipe_ctx)->plane_res.scl_data.viewport.x; \
+   trace_pipe_state.viewport_y = 
(pipe_ctx)->plane_res.scl_data.viewport.y; \
+   trace_pipe_state.viewport_w = 
(pipe_ctx)->plane_res.scl_data.viewport.width; \
+   trace_pipe_state.viewport_h = 
(pipe_ctx)->plane_res.scl_data.viewport.height; \
+   trace_pipe_state.flip_immediate = 
(pipe_ctx)->plane_state->flip_immediate; \
+   trace_pipe_state.surface_pitch  = 
(pipe_ctx)->plane_state->plane_size.surface_pitch; \
+   trace_pipe_state.format = 
(pipe_ctx)->plane_state->format; \
+   trace_pipe_state.swizzle= 
(pipe_ctx)->plane_state->tiling_info.gfx9.swizzle; \
+   trace_pipe_state.update_flags   = (pipe_ctx)->update_flags.raw; 
\
+   } while (0)
+
+#endif /* _AMDGPU_DM_TRACE_STRUCTS_DEFINED_ */
+
+TRACE_EVENT(amdgpu_dm_dc_pipe_state,
+   TP_PROTO(const struct amdgpu_dm_trace_pipe_state *pipe_state),
+   TP_ARGS(pipe_state),
+   TP_STRUCT__entry(
+__field(int, pipe_idx)
+__field(const 

[PATCH v2 1/4] drm/amd/display: Rework registers tracepoint

2020-09-11 Thread Rodrigo Siqueira
amdgpu_dc_rreg and amdgpu_dc_wreg are very similar, for this reason,
this commits abstract these two events by using DECLARE_EVENT_CLASS and
create an instance of it for each one of these events.

Signed-off-by: Rodrigo Siqueira 
---
 .../amd/display/amdgpu_dm/amdgpu_dm_trace.h   | 55 ---
 1 file changed, 24 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
index d898981684d5..dd34e11b1079 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_trace.h
@@ -31,40 +31,33 @@
 
 #include 
 
-TRACE_EVENT(amdgpu_dc_rreg,
-   TP_PROTO(unsigned long *read_count, uint32_t reg, uint32_t value),
-   TP_ARGS(read_count, reg, value),
-   TP_STRUCT__entry(
-   __field(uint32_t, reg)
-   __field(uint32_t, value)
-   ),
-   TP_fast_assign(
-   __entry->reg = reg;
-   __entry->value = value;
-   *read_count = *read_count + 1;
-   ),
-   TP_printk("reg=0x%08lx, value=0x%08lx",
-   (unsigned long)__entry->reg,
-   (unsigned long)__entry->value)
-);
+DECLARE_EVENT_CLASS(amdgpu_dc_reg_template,
+   TP_PROTO(unsigned long *count, uint32_t reg, uint32_t 
value),
+   TP_ARGS(count, reg, value),
 
-TRACE_EVENT(amdgpu_dc_wreg,
-   TP_PROTO(unsigned long *write_count, uint32_t reg, uint32_t value),
-   TP_ARGS(write_count, reg, value),
-   TP_STRUCT__entry(
-   __field(uint32_t, reg)
-   __field(uint32_t, value)
-   ),
-   TP_fast_assign(
-   __entry->reg = reg;
-   __entry->value = value;
-   *write_count = *write_count + 1;
-   ),
-   TP_printk("reg=0x%08lx, value=0x%08lx",
-   (unsigned long)__entry->reg,
-   (unsigned long)__entry->value)
+   TP_STRUCT__entry(
+__field(uint32_t, reg)
+__field(uint32_t, value)
+   ),
+
+   TP_fast_assign(
+  __entry->reg = reg;
+  __entry->value = value;
+  *count = *count + 1;
+   ),
+
+   TP_printk("reg=0x%08lx, value=0x%08lx",
+ (unsigned long)__entry->reg,
+ (unsigned long)__entry->value)
 );
 
+DEFINE_EVENT(amdgpu_dc_reg_template, amdgpu_dc_rreg,
+TP_PROTO(unsigned long *count, uint32_t reg, uint32_t value),
+TP_ARGS(count, reg, value));
+
+DEFINE_EVENT(amdgpu_dc_reg_template, amdgpu_dc_wreg,
+TP_PROTO(unsigned long *count, uint32_t reg, uint32_t value),
+TP_ARGS(count, reg, value));
 
 TRACE_EVENT(amdgpu_dc_performance,
TP_PROTO(unsigned long read_count, unsigned long write_count,
-- 
2.28.0

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


[PATCH v2 -next] drm/amdkfd: Fix -Wunused-const-variable warning

2020-09-11 Thread YueHaibing
If KFD_SUPPORT_IOMMU_V2 is not set, gcc warns:

drivers/gpu/drm/amd/amdgpu/../amdkfd/kfd_device.c:121:37: warning: 
‘raven_device_info’ defined but not used [-Wunused-const-variable=]
 static const struct kfd_device_info raven_device_info = {
 ^

As Huang Rui suggested, Raven already has the fallback path,
so it should be out of IOMMU v2 flag.

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

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
index 0e71a0543f98..e3fc6ed7b79c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device.c
@@ -503,8 +503,8 @@ static const struct kfd_device_info 
*kfd_supported_devices[][2] = {
 #ifdef KFD_SUPPORT_IOMMU_V2
[CHIP_KAVERI] = {_device_info, NULL},
[CHIP_CARRIZO] = {_device_info, NULL},
-   [CHIP_RAVEN] = {_device_info, NULL},
 #endif
+   [CHIP_RAVEN] = {_device_info, NULL},
[CHIP_HAWAII] = {_device_info, NULL},
[CHIP_TONGA] = {_device_info, NULL},
[CHIP_FIJI] = {_device_info, _vf_device_info},
-- 
2.17.1


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


[PATCH] drm/amd/display: Remove set but used 'temp'

2020-09-11 Thread Ye Bin
Addresses the following gcc warning with "make W=1":

In file included from 
drivers/gpu/drm/amd/amdgpu/../display/dmub/src/../dmub_srv.h:67:0,
from drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_dcn21.c:26:
drivers/gpu/drm/amd/amdgpu/../display/dmub/src/../inc/dmub_cmd.h: In function 
‘dmub_rb_flush_pending’:
drivers/gpu/drm/amd/amdgpu/../display/dmub/src/../inc/dmub_cmd.h:795:12: 
warning: variable ‘temp’ set but not used
 [-Wunused-but-set-variable]
uint64_t temp;
^
In file included from 
drivers/gpu/drm/amd/amdgpu/../display/dmub/src/../dmub_srv.h:67:0,
 from 
drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_dcn30.c:26:
drivers/gpu/drm/amd/amdgpu/../display/dmub/src/../inc/dmub_cmd.h: In function 
‘dmub_rb_flush_pending’:
drivers/gpu/drm/amd/amdgpu/../display/dmub/src/../inc/dmub_cmd.h:795:12: 
warning: variable ‘temp’ set but not used
[-Wunused-but-set-variable]
   uint64_t temp;

Reported-by: Hulk Robot 
Signed-off-by: Ye Bin 
---
 drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h 
b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
index d7e7f2eda92f..e32828ffc3e0 100644
--- a/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
+++ b/drivers/gpu/drm/amd/display/dmub/inc/dmub_cmd.h
@@ -791,12 +791,10 @@ static inline void dmub_rb_flush_pending(const struct 
dmub_rb *rb)
 
while (rptr != wptr) {
uint64_t volatile *data = (uint64_t volatile *)rb->base_address 
+ rptr / sizeof(uint64_t);
-   //uint64_t volatile *p = (uint64_t volatile *)data;
-   uint64_t temp;
int i;
 
for (i = 0; i < DMUB_RB_CMD_SIZE / sizeof(uint64_t); i++)
-   temp = *data++;
+   *data++;
 
rptr += DMUB_RB_CMD_SIZE;
if (rptr >= rb->capacity)
-- 
2.16.2.dirty

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


Re: [PATCH] drm/radeon: revert "Prefer lower feedback dividers"

2020-09-11 Thread Nirmoy

I don't know much about display code but this is

Acked-by: Nirmoy Das  as Sonny confirmed that the

revert fixes display corruption.


On 9/11/20 9:35 AM, Christian König wrote:

Ping, we need to revert this ASAP.

Christian.

Am 09.09.20 um 13:16 schrieb Christian König:

Turns out this breaks a lot of different hardware.

This reverts commit 522ff3a8b6d73a31084b4b087b458f7fa0ac1e14.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/radeon/radeon_display.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
b/drivers/gpu/drm/radeon/radeon_display.c

index 7b69d6dfe44a..e0ae911ef427 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -933,7 +933,7 @@ static void avivo_get_fb_ref_div(unsigned nom, 
unsigned den, unsigned post_div,

    /* get matching reference and feedback divider */
  *ref_div = min(max(den/post_div, 1u), ref_div_max);
-    *fb_div = max(nom * *ref_div * post_div / den, 1u);
+    *fb_div = DIV_ROUND_CLOSEST(nom * *ref_div * post_div, den);
    /* limit fb divider to its maximum */
  if (*fb_div > fb_div_max) {


___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=02%7C01%7Cnirmoy.das%40amd.com%7Cf783daab70b74e9eb3f908d856254f13%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637354065561865480sdata=%2FrEgUz%2FcfFkI4JHMZ9L56UHKDID2zHDRzwwBNrHYNmU%3Dreserved=0 


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


Re: [PATCH] drm/radeon: revert "Prefer lower feedback dividers"

2020-09-11 Thread Christian König

Ping, we need to revert this ASAP.

Christian.

Am 09.09.20 um 13:16 schrieb Christian König:

Turns out this breaks a lot of different hardware.

This reverts commit 522ff3a8b6d73a31084b4b087b458f7fa0ac1e14.

Signed-off-by: Christian König 
---
  drivers/gpu/drm/radeon/radeon_display.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
b/drivers/gpu/drm/radeon/radeon_display.c
index 7b69d6dfe44a..e0ae911ef427 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -933,7 +933,7 @@ static void avivo_get_fb_ref_div(unsigned nom, unsigned 
den, unsigned post_div,
  
  	/* get matching reference and feedback divider */

*ref_div = min(max(den/post_div, 1u), ref_div_max);
-   *fb_div = max(nom * *ref_div * post_div / den, 1u);
+   *fb_div = DIV_ROUND_CLOSEST(nom * *ref_div * post_div, den);
  
  	/* limit fb divider to its maximum */

if (*fb_div > fb_div_max) {


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


RE: [PATCH] drm/amdgpu: Update RAS init handling

2020-09-11 Thread Zhang, Hawking
[AMD Public Use]

+ {
+ dev_warn(psp->adev->dev, "RAS 
Init Status: 0x%X\n", ras_cmd->ras_status);
+ }
Please remove the redundant bracket.

Other than that, the patch is
Reviewed-by: Hawking Zhang 

In addition, please create another patch to move the nbio ras controller irq 
source registry to sw_init, which is the consistent as what we did for other ip 
blocks, register the irq source in IP sw_init funcs.

Regards,
Hawking
From: Clements, John 
Sent: Friday, September 11, 2020 12:03
To: amd-gfx list ; Chen, Guchun 
; Zhang, Hawking 
Subject: [PATCH] drm/amdgpu: Update RAS init handling


[AMD Official Use Only - Internal Distribution Only]

Added RAS status check and tear down RAS context if RAS init fails
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx