Re: [PATCH] drm/amdgpu:remove debugfs file in amdgpu_device_finish

[Christian König]

Hi Gary,

not sure what driver re-initialize feature you are talking about, but 
the last time I tried to re-initialize the driver it deadlocks in the 
modeset code because of some DC problem.


It's probably a good idea to fix that first, but in general please 
explain further what are you working on.


Regards,
Christian.

Am 07.11.2017 um 08:23 schrieb Sun, Gary:

Hi Christian,

The patch is for driver re- initialize  feature, not for driver exit or rmmod.  
When the driver initialize failed at some point, the re- initialize  feature 
will do some little clean and then try to initialize driver again, then it will 
re-register some registered debugfs , so it will fail.

Regards,
Gary


-Original Message-
From: Koenig, Christian
Sent: Monday, November 06, 2017 5:26 PM
To: Sun, Gary ; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu:remove debugfs file in amdgpu_device_finish

Am 06.11.2017 um 10:20 schrieb Gary Sun:

remove debugfs file in amdgpu_device_finish

NAK, the debugfs files are removed automatically by drm_debugfs_cleanup().

So that patch is unnecessary.

Regards,
Christian.


Signed-off-by: Gary Sun 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu.h|1 +
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   18 ++
   2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 4f919d3..6cfcb5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1250,6 +1250,7 @@ struct amdgpu_debugfs {
   int amdgpu_debugfs_add_files(struct amdgpu_device *adev,
 const struct drm_info_list *files,
 unsigned nfiles);
+int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev);
   int amdgpu_debugfs_fence_init(struct amdgpu_device *adev);
   
   #if defined(CONFIG_DEBUG_FS)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7b7439f..ee800ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2520,6 +2520,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
amdgpu_doorbell_fini(adev);
amdgpu_pm_sysfs_fini(adev);
amdgpu_debugfs_regs_cleanup(adev);
+   amdgpu_debugfs_cleanup_files(adev);
   }
   
   
@@ -3304,6 +3305,23 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev,

return 0;
   }
   
+int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev) {

+   unsigned int i;
+
+   for (i = 0; i < adev->debugfs_count; i++) { #if
+defined(CONFIG_DEBUG_FS)
+   drm_debugfs_remove_files(adev->debugfs[i].files,
+   adev->debugfs[i].num_files,
+   adev->ddev->primary);
+#endif
+   adev->debugfs[i].files = NUL;
+   adev->debugfs[i].num_files = 0;
+   }
+   adev->debugfs_count = 0;
+   return 0;
+}
+
   #if defined(CONFIG_DEBUG_FS)
   
   static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user

*buf,




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

[PATCH 1/2] drm/amdgpu: use irq-safe lock for kiq->ring_lock

[Pixel Ding]

From: pding 

This lock is used during register accessing in SRIOV guest.
The register accessing could happen both in irq enabled and
irq disabled cases. Always use irq-safe lock.

Signed-off-by: pding 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 9a73918..733c64c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -120,18 +120,19 @@ void amdgpu_virt_init_setting(struct amdgpu_device *adev)
 uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, uint32_t reg)
 {
signed long r;
+   unsigned long flags;
uint32_t val, seq;
struct amdgpu_kiq *kiq = >gfx.kiq;
struct amdgpu_ring *ring = >ring;
 
BUG_ON(!ring->funcs->emit_rreg);
 
-   spin_lock(>ring_lock);
+   spin_lock_irqsave(>ring_lock, flags);
amdgpu_ring_alloc(ring, 32);
amdgpu_ring_emit_rreg(ring, reg);
amdgpu_fence_emit_polling(ring, );
amdgpu_ring_commit(ring);
-   spin_unlock(>ring_lock);
+   spin_unlock_irqrestore(>ring_lock, flags);
 
r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
if (r < 1) {
@@ -146,18 +147,19 @@ uint32_t amdgpu_virt_kiq_rreg(struct amdgpu_device *adev, 
uint32_t reg)
 void amdgpu_virt_kiq_wreg(struct amdgpu_device *adev, uint32_t reg, uint32_t v)
 {
signed long r;
+   unsigned long flags;
uint32_t seq;
struct amdgpu_kiq *kiq = >gfx.kiq;
struct amdgpu_ring *ring = >ring;
 
BUG_ON(!ring->funcs->emit_wreg);
 
-   spin_lock(>ring_lock);
+   spin_lock_irqsave(>ring_lock, flags);
amdgpu_ring_alloc(ring, 32);
amdgpu_ring_emit_wreg(ring, reg, v);
amdgpu_fence_emit_polling(ring, );
amdgpu_ring_commit(ring);
-   spin_unlock(>ring_lock);
+   spin_unlock_irqrestore(>ring_lock, flags);
 
r = amdgpu_fence_wait_polling(ring, seq, MAX_KIQ_REG_WAIT);
if (r < 1)
-- 
2.9.5

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

[PATCH 2/2] drm/amdgpu: use irq-safe lock for adev->ring_lru_list_lock

[Pixel Ding]

From: pding 

This lock is used during register accessing in SRIOV guest
since KIQ uses general ring submission (amdgpu_ring_commit).
The register accessing could happen both in irq enabled and
irq disabled cases. Always use irq-safe lock.

Signed-off-by: pding 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
index e5ece1f..d7997b5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c
@@ -401,11 +401,12 @@ int amdgpu_ring_lru_get(struct amdgpu_device *adev, int 
type,
bool lru_pipe_order, struct amdgpu_ring **ring)
 {
struct amdgpu_ring *entry;
+   unsigned long flags;
 
/* List is sorted in LRU order, find first entry corresponding
 * to the desired HW IP */
*ring = NULL;
-   spin_lock(>ring_lru_list_lock);
+   spin_lock_irqsave(>ring_lru_list_lock, flags);
list_for_each_entry(entry, >ring_lru_list, lru_list) {
if (entry->funcs->type != type)
continue;
@@ -430,7 +431,7 @@ int amdgpu_ring_lru_get(struct amdgpu_device *adev, int 
type,
if (*ring)
amdgpu_ring_lru_touch_locked(adev, *ring);
 
-   spin_unlock(>ring_lru_list_lock);
+   spin_unlock_irqrestore(>ring_lru_list_lock, flags);
 
if (!*ring) {
DRM_ERROR("Ring LRU contains no entries for ring type:%d\n", 
type);
@@ -450,9 +451,11 @@ int amdgpu_ring_lru_get(struct amdgpu_device *adev, int 
type,
  */
 void amdgpu_ring_lru_touch(struct amdgpu_device *adev, struct amdgpu_ring 
*ring)
 {
-   spin_lock(>ring_lru_list_lock);
+   unsigned long flags;
+
+   spin_lock_irqsave(>ring_lru_list_lock, flags);
amdgpu_ring_lru_touch_locked(adev, ring);
-   spin_unlock(>ring_lru_list_lock);
+   spin_unlock_irqrestore(>ring_lru_list_lock, flags);
 }
 
 /*
-- 
2.9.5

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

Re: [PATCH 1/2] drm/amdgpu: use multipipe compute policy on non PL11 asics

[Andres Rodriguez]

Do you have any work actually going into multiple pipes? My understanding
is that opencl will only use one queue at a time (but I'm not really
certain about that).

What you can also check is if the app works correctly when it executed on
pipe0, and if it hangs on pipe 1+. I removed all the locations where pipe0
was hardcoded in the open driver, but it is possible it is still hardcoded
somewhere on the closed stack.

Regards,
Andres

On Nov 6, 2017 10:19 PM, "Zhou, David(ChunMing)" 
wrote:

> Then snychronization should have no problem, it maybe relate to multipipe
> hw setting issue.
>
>
> Regards,
>
> David Zhou
> --
> *From:* Andres Rodriguez 
> *Sent:* Tuesday, November 7, 2017 2:00:57 AM
> *To:* Zhou, David(ChunMing); amd-gfx list
> *Cc:* Deucher, Alexander
> *Subject:* Re: [PATCH 1/2] drm/amdgpu: use multipipe compute policy on
> non PL11 asics
>
> Sorry my mail client seems to have blown up. My reply got cut off,
> here is the full version:
>
>
>
> On 2017-11-06 01:49 AM, Chunming Zhou wrote:
> > Hi Andres,
> >
> Hi David,
>
> > With your this patch, OCLperf hung.
> Is this on all ASICs or just a specific one?
>
> >
> > Could you explain more?
> >
> > If I am correctly, the difference of with and without this patch is
> > setting first two queue or setting all queues of pipe0 to queue_bitmap.
> It is slightly different. With this patch we will also use the first
> two queues of all pipes, not just pipe 0;
>
> Pre-patch:
>
> |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
>        
>
> Post-patch:
>
> |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
>  1100  1100  1100  1100
>
> What this means is that we are allowing real multithreading for
> compute. Jobs on different pipes allow for parallel execution of work.
> Jobs on the same pipe (but different queues) use timeslicing to share
> the hardware.
>
>
> >
> > Then UMD can use different number queue to submit command for compute
> > selected by amdgpu_queue_mgr_map.
> >
> > I checked amdgpu_queue_mgr_map implementation,  CS_IOCTL can map user
> > ring to different hw ring depending on busy or idle, right?
> Yes, when a queue is first used, amdgpu_queue_mgr_map will decide what
> the mapping is for a usermode ring to a kernel ring id.
>
> > If yes, I see a bug in it, which will result in our sched_fence not
> > work. Our sched fence assumes the job will be executed in order, your
> > mapping queue breaks this.
>
> I think here you mean that work will execute out of order because it
> will go to different rings?
>
> That should not happen, since the id mapping is permanent on a
> per-context basis. Once a mapping is decided, it will be cached for
> this context so that we keep execution order guarantees. See the
> id-caching code in amdgpu_queue_mgr.c for reference.
>
> As long as the usermode keeps submitting work to the same ring, it
> will all be executed in order (all in the same ring). There is no
> change in this guarantee compared to pre-patch. Note that even before
> this patch amdgpu_queue_mgr_map has been using an LRU policy for a
> long time now.
>
> Regards,
> Andres
>
> On Mon, Nov 6, 2017 at 12:44 PM, Andres Rodriguez 
> wrote:
> >
> >
> > On 2017-11-06 01:49 AM, Chunming Zhou wrote:
> >>
> >> Hi Andres,
> >>
> >
> > Hi David,
> >
> >> With your this patch, OCLperf hung.
> >
> >
> > Is this on all ASICs or just a specific one?
> >
> >>
> >> Could you explain more?
> >>
> >> If I am correctly, the difference of with and without this patch is
> >> setting first two queue or setting all queues of pipe0 to queue_bitmap.
> >
> >
> > It is slightly different. With this patch we will also use the first two
> > queues of all pipes, not just pipe 0;
> >
> > Pre-patch:
> >
> > |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
> >        
> >
> > Post-patch:
> >
> > |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
> >  1100  1100  1100  1100
> >
> > What this means is that we are allowing real multithreading for compute.
> > Jobs on different pipes allow for parallel execution of work. Jobs on the
> > same pipe (but different queues) use timeslicing to share the hardware.
> >
> >
> >
> >>
> >> Then UMD can use different number queue to submit command for compute
> >> selected by amdgpu_queue_mgr_map.
> >>
> >> I checked amdgpu_queue_mgr_map implementation,  CS_IOCTL can map user
> ring
> >> to different hw ring depending on busy or idle, right?
> >>
> >> If yes, I see a bug in it, which will result in our sched_fence not
> work.
> >> Our sched fence assumes the job will be executed in order, your mapping
> >> queue breaks this.
> >>
> >>
> >> Regards,
> >>
> >> David Zhou
> >>
> >>
> >> On 2017年09月27日 00:22, Andres Rodriguez wrote:
> >>>
> >>> A performance regression for OpenCL tests on Polaris11 had this feature
> >>> disabled for all asics.
> >>>
> >>> Instead, disable it selectively on the 

[PATCH] drm/amd/display: fix static checker warning

[S, Shirish]

From: Shirish S 

This patch fixes static checker warning of
"warn: cast after binop" introduced by
4d3e00dad80a: "drm/amd/display : add high part address calculation for underlay"

Signed-off-by: Shirish S 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 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 a87e5ac..e1bdf5e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1827,7 +1827,7 @@ static int fill_plane_attributes_from_fb(struct 
amdgpu_device *adev,
= lower_32_bits(fb_location);
plane_state->address.video_progressive.luma_addr.high_part
= upper_32_bits(fb_location);
-   chroma_addr = fb_location + (u64)(awidth * fb->height);
+   chroma_addr = fb_location + (u64)awidth * fb->height;
plane_state->address.video_progressive.chroma_addr.low_part
= lower_32_bits(chroma_addr);
plane_state->address.video_progressive.chroma_addr.high_part
@@ -2959,7 +2959,7 @@ static int dm_plane_helper_prepare_fb(struct drm_plane 
*plane,
= 
lower_32_bits(afb->address);

plane_state->address.video_progressive.luma_addr.high_part
= 
upper_32_bits(afb->address);
-   chroma_addr = afb->address + (u64)(awidth * 
new_state->fb->height);
+   chroma_addr = afb->address + (u64)awidth * 
new_state->fb->height;

plane_state->address.video_progressive.chroma_addr.low_part
= 
lower_32_bits(chroma_addr);

plane_state->address.video_progressive.chroma_addr.high_part
-- 
2.7.4

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

Re: [PATCH 1/2] drm/amdgpu: use multipipe compute policy on non PL11 asics

[Zhou, David(ChunMing)]

Then snychronization should have no problem, it maybe relate to multipipe hw 
setting issue.


Regards,

David Zhou


From: Andres Rodriguez 
Sent: Tuesday, November 7, 2017 2:00:57 AM
To: Zhou, David(ChunMing); amd-gfx list
Cc: Deucher, Alexander
Subject: Re: [PATCH 1/2] drm/amdgpu: use multipipe compute policy on non PL11 
asics

Sorry my mail client seems to have blown up. My reply got cut off,
here is the full version:



On 2017-11-06 01:49 AM, Chunming Zhou wrote:
> Hi Andres,
>
Hi David,

> With your this patch, OCLperf hung.
Is this on all ASICs or just a specific one?

>
> Could you explain more?
>
> If I am correctly, the difference of with and without this patch is
> setting first two queue or setting all queues of pipe0 to queue_bitmap.
It is slightly different. With this patch we will also use the first
two queues of all pipes, not just pipe 0;

Pre-patch:

|-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
       

Post-patch:

|-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
 1100  1100  1100  1100

What this means is that we are allowing real multithreading for
compute. Jobs on different pipes allow for parallel execution of work.
Jobs on the same pipe (but different queues) use timeslicing to share
the hardware.


>
> Then UMD can use different number queue to submit command for compute
> selected by amdgpu_queue_mgr_map.
>
> I checked amdgpu_queue_mgr_map implementation,  CS_IOCTL can map user
> ring to different hw ring depending on busy or idle, right?
Yes, when a queue is first used, amdgpu_queue_mgr_map will decide what
the mapping is for a usermode ring to a kernel ring id.

> If yes, I see a bug in it, which will result in our sched_fence not
> work. Our sched fence assumes the job will be executed in order, your
> mapping queue breaks this.

I think here you mean that work will execute out of order because it
will go to different rings?

That should not happen, since the id mapping is permanent on a
per-context basis. Once a mapping is decided, it will be cached for
this context so that we keep execution order guarantees. See the
id-caching code in amdgpu_queue_mgr.c for reference.

As long as the usermode keeps submitting work to the same ring, it
will all be executed in order (all in the same ring). There is no
change in this guarantee compared to pre-patch. Note that even before
this patch amdgpu_queue_mgr_map has been using an LRU policy for a
long time now.

Regards,
Andres

On Mon, Nov 6, 2017 at 12:44 PM, Andres Rodriguez  wrote:
>
>
> On 2017-11-06 01:49 AM, Chunming Zhou wrote:
>>
>> Hi Andres,
>>
>
> Hi David,
>
>> With your this patch, OCLperf hung.
>
>
> Is this on all ASICs or just a specific one?
>
>>
>> Could you explain more?
>>
>> If I am correctly, the difference of with and without this patch is
>> setting first two queue or setting all queues of pipe0 to queue_bitmap.
>
>
> It is slightly different. With this patch we will also use the first two
> queues of all pipes, not just pipe 0;
>
> Pre-patch:
>
> |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
>        
>
> Post-patch:
>
> |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
>  1100  1100  1100  1100
>
> What this means is that we are allowing real multithreading for compute.
> Jobs on different pipes allow for parallel execution of work. Jobs on the
> same pipe (but different queues) use timeslicing to share the hardware.
>
>
>
>>
>> Then UMD can use different number queue to submit command for compute
>> selected by amdgpu_queue_mgr_map.
>>
>> I checked amdgpu_queue_mgr_map implementation,  CS_IOCTL can map user ring
>> to different hw ring depending on busy or idle, right?
>>
>> If yes, I see a bug in it, which will result in our sched_fence not work.
>> Our sched fence assumes the job will be executed in order, your mapping
>> queue breaks this.
>>
>>
>> Regards,
>>
>> David Zhou
>>
>>
>> On 2017年09月27日 00:22, Andres Rodriguez wrote:
>>>
>>> A performance regression for OpenCL tests on Polaris11 had this feature
>>> disabled for all asics.
>>>
>>> Instead, disable it selectively on the affected asics.
>>>
>>> Signed-off-by: Andres Rodriguez 
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 14 --
>>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> index 4f6c68f..3d76e76 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> @@ -109,9 +109,20 @@ void amdgpu_gfx_parse_disable_cu(unsigned *mask,
>>> unsigned max_se, unsigned max_s
>>>   }
>>>   }
>>> +static bool amdgpu_gfx_is_multipipe_capable(struct amdgpu_device *adev)
>>> +{
>>> +/* FIXME: spreading the queues across pipes causes perf regressions
>>> + * on POLARIS11 compute workloads */
>>> +if 

[PATCH] drm/amdgpu: bypass FB resizing for SRIOV VF

[Pixel Ding]

From: pding 

It introduces 900ms latency in exclusive mode which causes failure
of driver loading. Host can resize the BAR before guest staring,
so the resizing is not necessary here.

Signed-off-by: pding 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6f01dda..bf2b008 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -768,6 +768,10 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
u16 cmd;
int r;
 
+   /* Bypass for VF */
+   if (amdgpu_sriov_vf(adev))
+   return 0;
+
/* Disable memory decoding while we change the BAR addresses and size */
pci_read_config_word(adev->pdev, PCI_COMMAND, );
pci_write_config_word(adev->pdev, PCI_COMMAND,
-- 
2.9.5

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

[PATCH 1/2] drm/amd/scheduler: fix page protection of cb

[Chunming Zhou]

We must remove the fence callback.

Change-Id: I5d58a3a43b82fefd6c211c4128b0c9187c191e7f
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c 
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index af5b2c50abac..310904042dfc 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -231,6 +231,13 @@ void amd_sched_entity_fini(struct amd_gpu_scheduler *sched,
 */
kcl_kthread_park(sched->thread);
kcl_kthread_unpark(sched->thread);
+   if (entity->dependency) {
+   dma_fence_remove_callback(entity->dependency,
+ >cb);
+   dma_fence_put(entity->dependency);
+   entity->dependency = NULL;
+   }
+
while ((job = 
to_amd_sched_job(spsc_queue_pop(>job_queue {
struct amd_sched_fence *s_fence = job->s_fence;
amd_sched_fence_scheduled(s_fence);
-- 
2.14.1

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

[PATCH 2/2] drm/amd/scheduler: remove fence callback when context is killed

[Chunming Zhou]

Otherwise there could be page protection.

Change-Id: I1f6c81002fb2ba21c17cdc14fdde86579b28374e
Signed-off-by: Chunming Zhou 
---
 drivers/gpu/drm/amd/scheduler/gpu_scheduler.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c 
b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
index 310904042dfc..e38d55961a9f 100644
--- a/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/amd/scheduler/gpu_scheduler.c
@@ -243,6 +243,12 @@ void amd_sched_entity_fini(struct amd_gpu_scheduler *sched,
amd_sched_fence_scheduled(s_fence);
kcl_dma_fence_set_error(_fence->finished, -ESRCH);
amd_sched_fence_finished(s_fence);
+   if (s_fence->parent) {
+   dma_fence_remove_callback(s_fence->parent,
+ _fence->cb);
+   dma_fence_put(s_fence->parent);
+   s_fence->parent = NULL;
+   }
dma_fence_put(_fence->finished);
sched->ops->free_job(job);
}
-- 
2.14.1

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

Re: [PATCH] drm/amdgpu: resize VRAM BAR for CPU access v5

[Ding, Pixel]

It doesn’t work without that write. Meanwhile, this functionality doesn’t 
change BAR0 and BAR2 when operating on VF, just as you said it’s not necessary 
for VF.

So I think we can skip the whole stuff. Please help to review the patch coming 
later.
— 
Sincerely Yours,
Pixel







On 06/11/2017, 5:52 PM, "Christian König"  
wrote:

>> What benefits will the larger VRAM bar bring in,
>Well the obvious one that the CPU can access all of VRAM. There are some 
>games/workloads which rely quite a bit on this and it simplifies MM 
>largely when you don't need to shuffle buffers around for CPU access.
>
>> or are there new features relying on larger VRAM bar under development?
>Not that I know of.
>
>> I think it’s better to try something which can keep this change for SRIOV 
>> and don’t introduce too much latency.
>Agree on that, but reprogramming the PCI BAR is something which takes 
>quite a bunch of time.
>
>That's why I asked if it is sufficient if you comment out this one 
>pci_write_config_word() and it works?
>
>If yes than we can just skip that write, but I have doubts that this 
>will be sufficient.
>
>On the other hand in an SRIOV environment the host can resize the BAR 
>before starting the client, so that whole stuff shouldn't be necessary 
>in the first place.
>
>Regards,
>Christian.
>
>Am 06.11.2017 um 10:34 schrieb Ding, Pixel:
>> What benefits will the larger VRAM bar bring in, or are there new features 
>> relying on larger VRAM bar under development? SRIOV wants to leverage this 
>> sort of optimization too.
>>
>> I think it’s better to try something which can keep this change for SRIOV 
>> and don’t introduce too much latency.
>>
>> —
>> Sincerely Yours,
>> Pixel
>>
>>
>>
>>
>>
>>
>>
>> On 06/11/2017, 5:16 PM, "Christian König"  
>> wrote:
>>
>>> Am 06.11.2017 um 08:21 schrieb Ding, Pixel:
 Hi Christian,

 The latest driver fails to load on SRIOV VF with xen hypervisor driver.

 +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
 +{
 + u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
 + u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1;
 + u16 cmd;
 + int r;
 +
 + /* Disable memory decoding while we change the BAR addresses and 
 size */
 + pci_read_config_word(adev->pdev, PCI_COMMAND, );
 + pci_write_config_word(adev->pdev, PCI_COMMAND,

 The function above introduces 900ms latency during init in exclusive 
 accessing.


 + cmd & ~PCI_COMMAND_MEMORY);
 +


 Can we bypass disablement of response in memory space here? Any concern or 
 suggestion?
>>> Mhm, that was added to be on the extra safe side. In theory we can skip it.
>>>
>>> Does PCI BAR resize work on SRIOV if you skip this or is that just a NOP
>>> anyway?
>>>
>>> Might be a good idea to just immediately return here when SRIOV is active.
>>>
>>> Regards,
>>> Christian.
>>>

 —
 Sincerely Yours,
 Pixel







 On 02/11/2017, 9:13 PM, "amd-gfx on behalf of Alex Deucher" 
  
 wrote:

> On Thu, Nov 2, 2017 at 9:02 AM, Christian König  
> wrote:
>> From: Christian König 
>>
>> Try to resize BAR0 to let CPU access all of VRAM.
>>
>> v2: rebased, style cleanups, disable mem decode before resize,
>>   handle gmc_v9 as well, round size up to power of two.
>> v3: handle gmc_v6 as well, release and reassign all BARs in the driver.
>> v4: rename new function to amdgpu_device_resize_fb_bar,
>>   reenable mem decoding only if all resources are assigned.
>> v5: reorder resource release, return -ENODEV instead of BUG_ON().
>>
>> Signed-off-by: Christian König 
> Reviewed-by: Alex Deucher 
>
>> ---
>>drivers/gpu/drm/amd/amdgpu/amdgpu.h|  1 +
>>drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 48 
>> ++
>>drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c  | 12 ++--
>>drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  | 13 ++--
>>drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  | 13 ++--
>>drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  | 14 ++---
>>6 files changed, 88 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 2730a75..4f919d3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1854,6 +1854,7 @@ void amdgpu_ttm_placement_from_domain(struct 
>> amdgpu_bo *abo, u32 domain);
>>bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
>>   

[PATCH 2/3] amdgpu/dc: Fix potential null dereference in amdgpu_dm.c

[Ernst Sjöstrand]

Signed-off-by: Ernst Sjöstrand 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 9 +
 1 file changed, 9 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 e6bfa9f30900..2301589e4cc3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2413,6 +2413,8 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
return NULL;
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
+   if (!state)
+   return NULL;
 
__drm_atomic_helper_crtc_duplicate_state(crtc, >base);
 
@@ -3443,6 +3445,8 @@ create_i2c(struct ddc_service *ddc_service,
struct amdgpu_i2c_adapter *i2c;
 
i2c = kzalloc(sizeof(struct amdgpu_i2c_adapter), GFP_KERNEL);
+   if (!i2c)
+   return NULL;
i2c->base.owner = THIS_MODULE;
i2c->base.class = I2C_CLASS_DDC;
i2c->base.dev.parent = >pdev->dev;
@@ -3473,6 +3477,11 @@ static int amdgpu_dm_connector_init(struct 
amdgpu_display_manager *dm,
DRM_DEBUG_DRIVER("%s()\n", __func__);
 
i2c = create_i2c(link->ddc, link->link_index, );
+   if (!i2c) {
+   DRM_ERROR("Failed to create i2c adapter data\n");
+   return -1;
+   }
+
aconnector->i2c = i2c;
res = i2c_add_adapter(>base);
 
-- 
2.14.1

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

[PATCH 1/3] amdgpu/dc: fix more indentation warnings

[Ernst Sjöstrand]

More "warn: inconsistent indenting" fixes from smatch.

Signed-off-by: Ernst Sjöstrand 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 10 +-
 .../gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c   |  2 +-
 2 files changed, 6 insertions(+), 6 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 67cad46f9f15..e6bfa9f30900 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -517,7 +517,7 @@ static int detect_mst_link_for_all_connectors(struct 
drm_device *dev)
drm_modeset_lock(>mode_config.connection_mutex, NULL);
 
list_for_each_entry(connector, >mode_config.connector_list, head) {
-  aconnector = to_amdgpu_dm_connector(connector);
+   aconnector = to_amdgpu_dm_connector(connector);
if (aconnector->dc_link->type == dc_connection_mst_branch) {
DRM_DEBUG_DRIVER("DM_MST: starting TM on aconnector: %p 
[id: %d]\n",
aconnector, aconnector->base.base.id);
@@ -4754,10 +4754,10 @@ static int amdgpu_dm_atomic_check(struct drm_device 
*dev,
goto fail;
}
 
-/* Run this here since we want to validate the streams we created */
-ret = drm_atomic_helper_check_planes(dev, state);
-if (ret)
-goto fail;
+   /* Run this here since we want to validate the streams we created */
+   ret = drm_atomic_helper_check_planes(dev, state);
+   if (ret)
+   goto fail;
 
/* Check scaling and underscan changes*/
/*TODO Removed scaling changes validation due to inability to commit
diff --git a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c 
b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
index 0c4bbc10510d..81f9f3e34c10 100644
--- a/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
+++ b/drivers/gpu/drm/amd/display/dc/i2caux/dce110/aux_engine_dce110.c
@@ -318,7 +318,7 @@ static void process_channel_reply(
REG_GET(AUX_SW_DATA,
AUX_SW_DATA, _sw_data_val);
 
-reply->data[i] = aux_sw_data_val;
+   reply->data[i] = aux_sw_data_val;
++i;
}
 
-- 
2.14.1

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

[PATCH 3/3] amdgpu/dc: Fix missing null checks in amdgpu_dm.c

[Ernst Sjöstrand]

From smatch:
error: we previously assumed X could be null

Signed-off-by: Ernst Sjöstrand 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 15 +++
 1 file changed, 11 insertions(+), 4 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 2301589e4cc3..d036178c2241 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -432,8 +432,10 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
 
if (adev->dm.dc)
DRM_INFO("Display Core initialized!\n");
-   else
+   else {
DRM_INFO("Display Core failed to initialize!\n");
+   goto error;
+   }
 
INIT_WORK(>dm.mst_hotplug_work, hotplug_notify_work_func);
 
@@ -2263,7 +2265,7 @@ decide_crtc_timing_for_drm_display_mode(struct 
drm_display_mode *drm_mode,
}
 }
 
-static void create_fake_sink(struct amdgpu_dm_connector *aconnector)
+static int create_fake_sink(struct amdgpu_dm_connector *aconnector)
 {
struct dc_sink *sink = NULL;
struct dc_sink_init_data sink_init_data = { 0 };
@@ -2272,14 +2274,18 @@ static void create_fake_sink(struct amdgpu_dm_connector 
*aconnector)
sink_init_data.sink_signal = aconnector->dc_link->connector_signal;
 
sink = dc_sink_create(_init_data);
-   if (!sink)
+   if (!sink) {
DRM_ERROR("Failed to create sink!\n");
+   return -1;
+   }
 
sink->sink_signal = SIGNAL_TYPE_VIRTUAL;
aconnector->fake_enable = true;
 
aconnector->dc_sink = sink;
aconnector->dc_link->local_sink = sink;
+
+   return 0;
 }
 
 static struct dc_stream_state *
@@ -2313,7 +2319,8 @@ create_stream_for_sink(struct amdgpu_dm_connector 
*aconnector,
if (aconnector->mst_port)
goto stream_create_fail;
 
-   create_fake_sink(aconnector);
+   if (create_fake_sink(aconnector))
+   goto stream_create_fail;
}
 
stream = dc_create_stream_for_sink(aconnector->dc_sink);
-- 
2.14.1

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

More smatch fixes

[Ernst Sjöstrand]

I saw the previous smatch fixes and thought I would try to fix a few.
Since the other patches were applied to agd5f/amd-staging-drm-next
I continued there.

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

RE: [PATCH 2/2] drm/amdkfd: Use order_base_2 to get log2 of buffes sizes

[Deucher, Alexander]

> -Original Message-
> From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf
> Of Felix Kuehling
> Sent: Monday, November 06, 2017 2:52 PM
> To: amd-gfx@lists.freedesktop.org; oded.gab...@gmail.com
> Cc: Kuehling, Felix
> Subject: [PATCH 2/2] drm/amdkfd: Use order_base_2 to get log2 of buffes
> sizes
> 
> Replace (ffs(size) - 1) with order_base_2(size) as a more straight
> forward way to get log2 of buffer sizes.
> 
> Signed-off-by: Felix Kuehling 

Series is:
Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 6 +++---
>  drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c  | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> index efed6ef..7aa57ab 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
> @@ -183,7 +183,7 @@ static int update_mqd(struct mqd_manager *mm,
> void *mqd,
>* Calculating queue size which is log base 2 of actual queue size -1
>* dwords and another -1 for ffs
>*/
> - m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1;
> + m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1;
>   m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q-
> >queue_address >> 8);
>   m->cp_hqd_pq_base_hi = upper_32_bits((uint64_t)q-
> >queue_address >> 8);
>   m->cp_hqd_pq_rptr_report_addr_lo = lower_32_bits((uint64_t)q-
> >read_ptr);
> @@ -208,7 +208,7 @@ static int update_mqd_sdma(struct mqd_manager
> *mm, void *mqd,
>   struct cik_sdma_rlc_registers *m;
> 
>   m = get_sdma_mqd(mqd);
> - m->sdma_rlc_rb_cntl = (ffs(q->queue_size / 4) - 1)
> + m->sdma_rlc_rb_cntl = order_base_2(q->queue_size / 4)
>   << SDMA0_RLC0_RB_CNTL__RB_SIZE__SHIFT |
>   q->vmid <<
> SDMA0_RLC0_RB_CNTL__RB_VMID__SHIFT |
>   1 <<
> SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_ENABLE__SHIFT |
> @@ -349,7 +349,7 @@ static int update_mqd_hiq(struct mqd_manager
> *mm, void *mqd,
>* Calculating queue size which is log base 2 of actual queue
>* size -1 dwords
>*/
> - m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1;
> + m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1;
>   m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q-
> >queue_address >> 8);
>   m->cp_hqd_pq_base_hi = upper_32_bits((uint64_t)q-
> >queue_address >> 8);
>   m->cp_hqd_pq_rptr_report_addr_lo = lower_32_bits((uint64_t)q-
> >read_ptr);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> index 85e1b67..2ba7cea 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
> @@ -121,7 +121,7 @@ static int __update_mqd(struct mqd_manager *mm,
> void *mqd,
>   m->cp_hqd_pq_control = 5 <<
> CP_HQD_PQ_CONTROL__RPTR_BLOCK_SIZE__SHIFT |
>   atc_bit << CP_HQD_PQ_CONTROL__PQ_ATC__SHIFT
> |
>   mtype << CP_HQD_PQ_CONTROL__MTYPE__SHIFT;
> - m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1;
> + m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1;
>   pr_debug("cp_hqd_pq_control 0x%x\n", m->cp_hqd_pq_control);
> 
>   m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q-
> >queue_address >> 8);
> @@ -151,7 +151,7 @@ static int __update_mqd(struct mqd_manager *mm,
> void *mqd,
>* is safe, giving a maximum field value of 0xA.
>*/
>   m->cp_hqd_eop_control |= min(0xA,
> - ffs(q->eop_ring_buffer_size / 4) - 1 - 1);
> + order_base_2(q->eop_ring_buffer_size / 4) - 1);
>   m->cp_hqd_eop_base_addr_lo =
>   lower_32_bits(q->eop_ring_buffer_address >> 8);
>   m->cp_hqd_eop_base_addr_hi =
> @@ -287,7 +287,7 @@ static int update_mqd_sdma(struct mqd_manager
> *mm, void *mqd,
>   struct vi_sdma_mqd *m;
> 
>   m = get_sdma_mqd(mqd);
> - m->sdmax_rlcx_rb_cntl = (ffs(q->queue_size / 4) - 1)
> + m->sdmax_rlcx_rb_cntl = order_base_2(q->queue_size / 4)
>   << SDMA0_RLC0_RB_CNTL__RB_SIZE__SHIFT |
>   q->vmid << SDMA0_RLC0_RB_CNTL__RB_VMID__SHIFT |
>   1 <<
> SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_ENABLE__SHIFT |
> --
> 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/1] drm/amd/powerplay: initialize a variable before using it

[Alex Deucher]

On Sat, Nov 4, 2017 at 8:21 AM, Nicolas Iooss
 wrote:
> On Sun, Sep 3, 2017 at 2:00 PM, Nicolas Iooss
>  wrote:
>>
>> Function vega10_apply_state_adjust_rules() only initializes
>> stable_pstate_sclk_dpm_percentage when
>> data->registry_data.stable_pstate_sclk_dpm_percentage is not between 1
>> and 100. The variable is then used to compute stable_pstate_sclk, which
>> therefore uses an uninitialized value.
>>
>> Fix this by initializing stable_pstate_sclk_dpm_percentage to
>> data->registry_data.stable_pstate_sclk_dpm_percentage.
>>
>> This issue has been found while building the kernel with clang. The
>> compiler reported a -Wsometimes-uninitialized warning.
>>
>> Fixes: f83a9991648b ("drm/amd/powerplay: add Vega10 powerplay support (v5)")
>> Signed-off-by: Nicolas Iooss 
>> ---
>>  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
>> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
>> index 197174e562d2..c8d28f78cd47 100644
>> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
>> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
>> @@ -3043,6 +3043,8 @@ static int vega10_apply_state_adjust_rules(struct 
>> pp_hwmgr *hwmgr,
>>
>> if (phm_cap_enabled(hwmgr->platform_descriptor.platformCaps,
>> PHM_PlatformCaps_StablePState)) {
>> +   stable_pstate_sclk_dpm_percentage =
>> +   
>> data->registry_data.stable_pstate_sclk_dpm_percentage;
>> PP_ASSERT_WITH_CODE(
>> 
>> data->registry_data.stable_pstate_sclk_dpm_percentage >= 1 &&
>> 
>> data->registry_data.stable_pstate_sclk_dpm_percentage <= 100,
>> --
>> 2.14.1
>
> Hello,
> I have not received any comment on the above patch that I sent two
> months ago, and the issue which is fixed by it still exists in today's
> linux-next code [1]. Could you please review this patch?

Reviewed and applied.  Sorry for missing this.  Feel free to ping
sooner if it looks like something slipped through the cracks.

Thanks,

Alex


>
> Thanks,
> Nicolas
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c#n3137
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amd/display: remove some unneeded code

[Alex Deucher]

On Mon, Nov 6, 2017 at 6:59 AM, Christian König
 wrote:
> Am 06.11.2017 um 12:44 schrieb Dan Carpenter:
>>
>> We assign "v_init = asic_blank_start;" a few lines earlier so there is
>> no need to do it again inside the if statements.  Also "v_init" is
>> unsigned so it can't be less than zero.
>>
>> Signed-off-by: Dan Carpenter 
>
>
> Acked-by: Christian König 
>

Applied.  thanks!

Alex

>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
>> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
>> index 1994865d4351..c7333cdf1802 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
>> @@ -236,13 +236,10 @@ static void tgn10_program_timing(
>> if (tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT ||
>> tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT_MST
>> ||
>> tg->dlg_otg_param.signal == SIGNAL_TYPE_EDP) {
>> -   v_init = asic_blank_start;
>> start_point = 1;
>> if (patched_crtc_timing.flags.INTERLACE == 1)
>> field_num = 1;
>> }
>> -   if (v_init < 0)
>> -   v_init = 0;
>> v_fp2 = 0;
>> if (tg->dlg_otg_param.vstartup_start > asic_blank_end)
>> v_fp2 = tg->dlg_otg_param.vstartup_start > asic_blank_end;
>> ___
>> 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] drm/amd/display: checking for NULL instead of IS_ERR()

[Alex Deucher]

On Mon, Nov 6, 2017 at 6:51 AM, Christian König
 wrote:
> Am 06.11.2017 um 12:43 schrieb Dan Carpenter:
>>
>> backlight_device_register() never returns NULL, it returns error
>> pointers on error so the check here is wrong.
>>
>> Signed-off-by: Dan Carpenter 
>
>
> Acked-by: Christian König 

Applied.  thanks!

Alex

>
>>
>> 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 33a15a1d818c..75f9383f5b9b 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -1296,7 +1296,7 @@ amdgpu_dm_register_backlight_device(struct
>> amdgpu_display_manager *dm)
>> _dm_backlight_ops,
>> );
>>   - if (NULL == dm->backlight_dev)
>> +   if (IS_ERR(dm->backlight_dev))
>> DRM_ERROR("DM: Backlight registration failed!\n");
>> else
>> DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n",
>> bl_name);
>
>
>
> ___
> 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] drm/amd/display: small cleanup in destruct()

[Alex Deucher]

On Mon, Nov 6, 2017 at 3:44 AM, Christian König
 wrote:
> Am 06.11.2017 um 08:07 schrieb Dan Carpenter:
>>
>> Static analysis tools get annoyed that we don't indent this if
>> statement.  Actually, the if statement isn't required because kfree()
>> can handle NULL pointers just fine.  The DCE110STRENC_FROM_STRENC()
>> macro is a wrapper around container_of() but it's basically a no-op or a
>> cast.  Anyway, it's not really appropriate here so it should be removed
>> as well.
>>
>> Signed-off-by: Dan Carpenter 
>
>
> Acked-by: Christian König 

Applied.  thanks!

Alex


>
>> ---
>> v2: in v1 I just added a tab
>>
>> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> index d911590d08bc..4c4bd72d4e40 100644
>> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
>> @@ -725,10 +725,8 @@ static void destruct(struct dcn10_resource_pool
>> *pool)
>> }
>> }
>>   - for (i = 0; i < pool->base.stream_enc_count; i++) {
>> -   if (pool->base.stream_enc[i] != NULL)
>> -   kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
>> -   }
>> +   for (i = 0; i < pool->base.stream_enc_count; i++)
>> +   kfree(pool->base.stream_enc[i]);
>> for (i = 0; i < pool->base.audio_count; i++) {
>> if (pool->base.audios[i])
>
>
>
> ___
> 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 1/2] drm/amdkfd: Hardware DWORD size is 4 bytes

[Felix Kuehling]

Don't use sizeof(uint32_t) or similar types for hardware or firmware
DWORD size. The hardware and firmware don't care about Linux types.

Signed-off-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c  | 14 +-
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c|  2 +-
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 10 --
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c  |  9 -
 drivers/gpu/drm/amd/amdkfd/kfd_packet_manager.c  |  2 +-
 5 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
index c407f6b..afb26f2 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c
@@ -95,7 +95,7 @@ static int dbgdev_diq_submit_ib(struct kfd_dbgdev *dbgdev,
ib_packet->bitfields3.ib_base_hi = largep->u.high_part;
 
ib_packet->control = (1 << 23) | (1 << 31) |
-   ((size_in_bytes / sizeof(uint32_t)) & 0xf);
+   ((size_in_bytes / 4) & 0xf);
 
ib_packet->bitfields5.pasid = pasid;
 
@@ -126,8 +126,7 @@ static int dbgdev_diq_submit_ib(struct kfd_dbgdev *dbgdev,
 
rm_packet->header.opcode = IT_RELEASE_MEM;
rm_packet->header.type = PM4_TYPE_3;
-   rm_packet->header.count = sizeof(struct pm4__release_mem) /
-   sizeof(unsigned int) - 2;
+   rm_packet->header.count = sizeof(struct pm4__release_mem) / 4 - 2;
 
rm_packet->bitfields2.event_type = CACHE_FLUSH_AND_INV_TS_EVENT;
rm_packet->bitfields2.event_index =
@@ -652,8 +651,7 @@ static int dbgdev_wave_control_diq(struct kfd_dbgdev 
*dbgdev,
packets_vec[0].header.opcode = IT_SET_UCONFIG_REG;
packets_vec[0].header.type = PM4_TYPE_3;
packets_vec[0].bitfields2.reg_offset =
-   GRBM_GFX_INDEX / (sizeof(uint32_t)) -
-   USERCONFIG_REG_BASE;
+   GRBM_GFX_INDEX / 4 - USERCONFIG_REG_BASE;
 
packets_vec[0].bitfields2.insert_vmid = 0;
packets_vec[0].reg_data[0] = reg_gfx_index.u32All;
@@ -661,8 +659,7 @@ static int dbgdev_wave_control_diq(struct kfd_dbgdev 
*dbgdev,
packets_vec[1].header.count = 1;
packets_vec[1].header.opcode = IT_SET_CONFIG_REG;
packets_vec[1].header.type = PM4_TYPE_3;
-   packets_vec[1].bitfields2.reg_offset = SQ_CMD / (sizeof(uint32_t)) -
-   AMD_CONFIG_REG_BASE;
+   packets_vec[1].bitfields2.reg_offset = SQ_CMD / 4 - AMD_CONFIG_REG_BASE;
 
packets_vec[1].bitfields2.vmid_shift = SQ_CMD_VMID_OFFSET;
packets_vec[1].bitfields2.insert_vmid = 1;
@@ -678,8 +675,7 @@ static int dbgdev_wave_control_diq(struct kfd_dbgdev 
*dbgdev,
 
packets_vec[2].ordinal1 = packets_vec[0].ordinal1;
packets_vec[2].bitfields2.reg_offset =
-   GRBM_GFX_INDEX / (sizeof(uint32_t)) -
-   USERCONFIG_REG_BASE;
+   GRBM_GFX_INDEX / 4 - USERCONFIG_REG_BASE;
 
packets_vec[2].bitfields2.insert_vmid = 0;
packets_vec[2].reg_data[0] = reg_gfx_index.u32All;
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
index 8b0c064..5dc6567 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -218,7 +218,7 @@ static int acquire_packet_buffer(struct kernel_queue *kq,
rptr = *kq->rptr_kernel;
wptr = *kq->wptr_kernel;
queue_address = (unsigned int *)kq->pq_kernel_addr;
-   queue_size_dwords = kq->queue->properties.queue_size / sizeof(uint32_t);
+   queue_size_dwords = kq->queue->properties.queue_size / 4;
 
pr_debug("rptr: %d\n", rptr);
pr_debug("wptr: %d\n", wptr);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
index 9873929..efed6ef 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
@@ -154,7 +154,7 @@ static int load_mqd(struct mqd_manager *mm, void *mqd, 
uint32_t pipe_id,
 {
/* AQL write pointer counts in 64B packets, PM4/CP counts in dwords. */
uint32_t wptr_shift = (p->format == KFD_QUEUE_FORMAT_AQL ? 4 : 0);
-   uint32_t wptr_mask = (uint32_t)((p->queue_size / sizeof(uint32_t)) - 1);
+   uint32_t wptr_mask = (uint32_t)((p->queue_size / 4) - 1);
 
return mm->dev->kfd2kgd->hqd_load(mm->dev->kgd, mqd, pipe_id, queue_id,
  (uint32_t __user *)p->write_ptr,
@@ -183,8 +183,7 @@ static int update_mqd(struct mqd_manager *mm, void *mqd,
 * Calculating queue size which is log base 2 of actual queue size -1
 * dwords and another -1 for ffs
 */
-   m->cp_hqd_pq_control 

[PATCH 2/2] drm/amdkfd: Use order_base_2 to get log2 of buffes sizes

[Felix Kuehling]

Replace (ffs(size) - 1) with order_base_2(size) as a more straight
forward way to get log2 of buffer sizes.

Signed-off-by: Felix Kuehling 
---
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c | 6 +++---
 drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c  | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
index efed6ef..7aa57ab 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_cik.c
@@ -183,7 +183,7 @@ static int update_mqd(struct mqd_manager *mm, void *mqd,
 * Calculating queue size which is log base 2 of actual queue size -1
 * dwords and another -1 for ffs
 */
-   m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1;
+   m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1;
m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q->queue_address >> 8);
m->cp_hqd_pq_base_hi = upper_32_bits((uint64_t)q->queue_address >> 8);
m->cp_hqd_pq_rptr_report_addr_lo = lower_32_bits((uint64_t)q->read_ptr);
@@ -208,7 +208,7 @@ static int update_mqd_sdma(struct mqd_manager *mm, void 
*mqd,
struct cik_sdma_rlc_registers *m;
 
m = get_sdma_mqd(mqd);
-   m->sdma_rlc_rb_cntl = (ffs(q->queue_size / 4) - 1)
+   m->sdma_rlc_rb_cntl = order_base_2(q->queue_size / 4)
<< SDMA0_RLC0_RB_CNTL__RB_SIZE__SHIFT |
q->vmid << SDMA0_RLC0_RB_CNTL__RB_VMID__SHIFT |
1 << SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_ENABLE__SHIFT |
@@ -349,7 +349,7 @@ static int update_mqd_hiq(struct mqd_manager *mm, void *mqd,
 * Calculating queue size which is log base 2 of actual queue
 * size -1 dwords
 */
-   m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1;
+   m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1;
m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q->queue_address >> 8);
m->cp_hqd_pq_base_hi = upper_32_bits((uint64_t)q->queue_address >> 8);
m->cp_hqd_pq_rptr_report_addr_lo = lower_32_bits((uint64_t)q->read_ptr);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
index 85e1b67..2ba7cea 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_vi.c
@@ -121,7 +121,7 @@ static int __update_mqd(struct mqd_manager *mm, void *mqd,
m->cp_hqd_pq_control = 5 << CP_HQD_PQ_CONTROL__RPTR_BLOCK_SIZE__SHIFT |
atc_bit << CP_HQD_PQ_CONTROL__PQ_ATC__SHIFT |
mtype << CP_HQD_PQ_CONTROL__MTYPE__SHIFT;
-   m->cp_hqd_pq_control |= ffs(q->queue_size / 4) - 1 - 1;
+   m->cp_hqd_pq_control |= order_base_2(q->queue_size / 4) - 1;
pr_debug("cp_hqd_pq_control 0x%x\n", m->cp_hqd_pq_control);
 
m->cp_hqd_pq_base_lo = lower_32_bits((uint64_t)q->queue_address >> 8);
@@ -151,7 +151,7 @@ static int __update_mqd(struct mqd_manager *mm, void *mqd,
 * is safe, giving a maximum field value of 0xA.
 */
m->cp_hqd_eop_control |= min(0xA,
-   ffs(q->eop_ring_buffer_size / 4) - 1 - 1);
+   order_base_2(q->eop_ring_buffer_size / 4) - 1);
m->cp_hqd_eop_base_addr_lo =
lower_32_bits(q->eop_ring_buffer_address >> 8);
m->cp_hqd_eop_base_addr_hi =
@@ -287,7 +287,7 @@ static int update_mqd_sdma(struct mqd_manager *mm, void 
*mqd,
struct vi_sdma_mqd *m;
 
m = get_sdma_mqd(mqd);
-   m->sdmax_rlcx_rb_cntl = (ffs(q->queue_size / 4) - 1)
+   m->sdmax_rlcx_rb_cntl = order_base_2(q->queue_size / 4)
<< SDMA0_RLC0_RB_CNTL__RB_SIZE__SHIFT |
q->vmid << SDMA0_RLC0_RB_CNTL__RB_VMID__SHIFT |
1 << SDMA0_RLC0_RB_CNTL__RPTR_WRITEBACK_ENABLE__SHIFT |
-- 
2.7.4

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

Re: [PATCH] amdgpu/dc: fix non-ansi function decls.

[Christian König]

Am 06.11.2017 um 20:17 schrieb Dave Airlie:

From: Dave Airlie 

smatch reported:
drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.c:351:71:
 warning: non-ANSI function declaration of function 
'dal_cmd_tbl_helper_dce80_get_table'
drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.c:361:72:
 warning: non-ANSI function declaration of function 
'dal_cmd_tbl_helper_dce110_get_table'
drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce112/command_table_helper_dce112.c:415:72:
 warning: non-ANSI function declaration of function 
'dal_cmd_tbl_helper_dce112_get_table'
drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce112/command_table_helper2_dce112.c:415:73:
 warning: non-ANSI function declaration of function 
'dal_cmd_tbl_helper_dce112_get_table2'
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_surface.c:148:34: warning: 
non-ANSI function declaration of function 'dc_create_gamma'
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_surface.c:178:50: warning: 
non-ANSI function declaration of function 'dc_create_transfer_func'

This fixes them.

Signed-off-by: Dave Airlie 
---
  .../gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c  | 2 +-
  .../gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c | 2 +-
  .../gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c  | 2 +-
  .../gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c| 2 +-
  drivers/gpu/drm/amd/display/dc/core/dc_surface.c  | 4 ++--
  5 files changed, 6 insertions(+), 6 deletions(-)

diff --git 
a/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c 
b/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c
index 8049320..ca24154 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c
@@ -358,7 +358,7 @@ static const struct command_table_helper 
command_table_helper_funcs = {
   * const struct command_table_helper **h - [out] struct of functions
   *
   */
-const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table()
+const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void)
  {
return _table_helper_funcs;
  }


BTW may I ask what those accessors functions are good for? That looks 
just like a LOC increase to me.


Why not export the command_table_helper_funcs directly?

Regards,
Christian.


diff --git 
a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c 
b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c
index d342cde..0237ae5 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c
@@ -412,7 +412,7 @@ static const struct command_table_helper 
command_table_helper_funcs = {
   * const struct command_table_helper **h - [out] struct of functions
   *
   */
-const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table2()
+const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table2(void)
  {
return _table_helper_funcs;
  }
diff --git 
a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c 
b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c
index 48e5996..452034f 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c
@@ -412,7 +412,7 @@ static const struct command_table_helper 
command_table_helper_funcs = {
   * const struct command_table_helper **h - [out] struct of functions
   *
   */
-const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table()
+const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table(void)
  {
return _table_helper_funcs;
  }
diff --git 
a/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c 
b/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c
index 295e16e..8b30b55 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c
@@ -348,7 +348,7 @@ static const struct command_table_helper 
command_table_helper_funcs = {
dal_cmd_table_helper_encoder_mode_bp_to_atom,
  };
  
-const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table()

+const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void)
  {
return _table_helper_funcs;
  }
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
index 5aa2270..ade5b8e 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
@@ -145,7 +145,7 @@ void dc_gamma_release(struct 

[PATCH] amdgpu/dc: fix indentation warning from smatch.

[Dave Airlie]

From: Dave Airlie 

This fixes all the current smatch:
warn: inconsistent indenting

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c |  2 +-
 .../gpu/drm/amd/display/dc/bios/command_table2.c   | 18 ++---
 drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c   |  2 +-
 drivers/gpu/drm/amd/display/dc/core/dc_stream.c|  4 +--
 drivers/gpu/drm/amd/display/dc/dce/dce_audio.c | 26 +--
 drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c  | 16 ++--
 .../amd/display/dc/dce110/dce110_hw_sequencer.c| 30 +++---
 .../display/dc/dce120/dce120_timing_generator.c|  7 +++--
 .../gpu/drm/amd/display/dc/dcn10/dcn10_dpp_cm.c|  2 +-
 .../dc/i2caux/dce110/i2c_hw_engine_dce110.c| 30 +++---
 10 files changed, 68 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c 
b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
index 43e9a99..1ee1717 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/bios_parser2.c
@@ -1373,7 +1373,7 @@ static enum bp_result get_firmware_info_v3_1(
bp->cmd_tbl.get_smu_clock_info(bp) * 10;
}
 
-return BP_RESULT_OK;
+   return BP_RESULT_OK;
 }
 
 static enum bp_result bios_parser_get_encoder_cap_info(
diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table2.c 
b/drivers/gpu/drm/amd/display/dc/bios/command_table2.c
index 64eab35..ba68693 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/command_table2.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/command_table2.c
@@ -373,15 +373,15 @@ static void init_set_crtc_timing(struct bios_parser *bp)
uint32_t dtd_version =
BIOS_CMD_TABLE_PARA_REVISION(setcrtc_usingdtdtiming);
 
-   switch (dtd_version) {
-   case 3:
-   bp->cmd_tbl.set_crtc_timing =
-   set_crtc_using_dtd_timing_v3;
-   break;
-   default:
-   bp->cmd_tbl.set_crtc_timing = NULL;
-   break;
-   }
+   switch (dtd_version) {
+   case 3:
+   bp->cmd_tbl.set_crtc_timing =
+   set_crtc_using_dtd_timing_v3;
+   break;
+   default:
+   bp->cmd_tbl.set_crtc_timing = NULL;
+   break;
+   }
 }
 
 static enum bp_result set_crtc_using_dtd_timing_v3(
diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c 
b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c
index e151523..3dce35e 100644
--- a/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c
+++ b/drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c
@@ -1155,7 +1155,7 @@ static unsigned int dcn_find_normalized_clock_vdd_Level(
unsigned factor = (ddr4_dram_factor_single_Channel * 
dc->dcn_soc->number_of_channels);
 
if (clocks_in_khz > 
dc->dcn_soc->fabric_and_dram_bandwidth_vmax0p9*100/factor) {
-   vdd_level = dcn_bw_v_max0p91;
+   vdd_level = dcn_bw_v_max0p91;
BREAK_TO_DEBUGGER();
} else if (clocks_in_khz > 
dc->dcn_soc->fabric_and_dram_bandwidth_vnom0p8*100/factor) {
vdd_level = dcn_bw_v_max0p9;
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
index 572b885..de04b95 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_stream.c
@@ -182,11 +182,11 @@ bool dc_stream_set_cursor_attributes(
 
if (NULL == stream) {
dm_error("DC: dc_stream is NULL!\n");
-   return false;
+   return false;
}
if (NULL == attributes) {
dm_error("DC: attributes is NULL!\n");
-   return false;
+   return false;
}
 
if (attributes->address.quad_part == 0) {
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 526ec5c..d882adf 100644
--- a/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
+++ b/drivers/gpu/drm/amd/display/dc/dce/dce_audio.c
@@ -179,19 +179,19 @@ static void check_audio_bandwidth_hdmi(
/* Number of Audio Packets (multiplied by 10) per Line (for 8 ch number
 * of Audio samples per line multiplied by 10 - Layout 1)
 */
-samples /= 32;
-samples *= crtc_info->v_active;
-/*Number of samples multiplied by 10, per second */
-samples *= crtc_info->refresh_rate;
-/*Number of Audio samples per second */
-samples /= 10;
-
-/* @todo do it after deep color is implemented
- * 8xx - deep color bandwidth scaling
- * 

[PATCH] amdgpu/dc: handle allocation failures in dc_commit_planes_to_stream.

[Dave Airlie]

From: Dave Airlie 

Reported-by smatch:
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:966 
dc_commit_planes_to_stream() error: potential null dereference 'flip_addr'.  
(kcalloc returns null)
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:968 
dc_commit_planes_to_stream() error: potential null dereference 'plane_info'.  
(kcalloc returns null)
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:978 
dc_commit_planes_to_stream() error: potential null dereference 'scaling_info'.  
(kcalloc returns null)

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/amd/display/dc/core/dc.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c 
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index a71392f..ce3c57b 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -952,6 +952,14 @@ bool dc_commit_planes_to_stream(
scaling_info = kcalloc(MAX_SURFACES, sizeof(struct dc_scaling_info),
   GFP_KERNEL);
 
+   if (!flip_addr || !plane_info || !scaling_info) {
+   kfree(flip_addr);
+   kfree(plane_info);
+   kfree(scaling_info);
+   kfree(stream_update);
+   return false;
+   }
+
memset(updates, 0, sizeof(updates));
 
stream_update->src = dc_stream->src;
-- 
2.9.5

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

[PATCH] amdgpu/dc: fix non-ansi function decls.

[Dave Airlie]

From: Dave Airlie 

smatch reported:
drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce80/command_table_helper_dce80.c:351:71:
 warning: non-ANSI function declaration of function 
'dal_cmd_tbl_helper_dce80_get_table'
drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce110/command_table_helper_dce110.c:361:72:
 warning: non-ANSI function declaration of function 
'dal_cmd_tbl_helper_dce110_get_table'
drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce112/command_table_helper_dce112.c:415:72:
 warning: non-ANSI function declaration of function 
'dal_cmd_tbl_helper_dce112_get_table'
drivers/gpu/drm/amd/amdgpu/../display/dc/bios/dce112/command_table_helper2_dce112.c:415:73:
 warning: non-ANSI function declaration of function 
'dal_cmd_tbl_helper_dce112_get_table2'
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_surface.c:148:34: warning: 
non-ANSI function declaration of function 'dc_create_gamma'
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_surface.c:178:50: warning: 
non-ANSI function declaration of function 'dc_create_transfer_func'

This fixes them.

Signed-off-by: Dave Airlie 
---
 .../gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c  | 2 +-
 .../gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c | 2 +-
 .../gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c  | 2 +-
 .../gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c| 2 +-
 drivers/gpu/drm/amd/display/dc/core/dc_surface.c  | 4 ++--
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git 
a/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c 
b/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c
index 8049320..ca24154 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/dce110/command_table_helper_dce110.c
@@ -358,7 +358,7 @@ static const struct command_table_helper 
command_table_helper_funcs = {
  * const struct command_table_helper **h - [out] struct of functions
  *
  */
-const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table()
+const struct command_table_helper *dal_cmd_tbl_helper_dce110_get_table(void)
 {
return _table_helper_funcs;
 }
diff --git 
a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c 
b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c
index d342cde..0237ae5 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper2_dce112.c
@@ -412,7 +412,7 @@ static const struct command_table_helper 
command_table_helper_funcs = {
  * const struct command_table_helper **h - [out] struct of functions
  *
  */
-const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table2()
+const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table2(void)
 {
return _table_helper_funcs;
 }
diff --git 
a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c 
b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c
index 48e5996..452034f 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/dce112/command_table_helper_dce112.c
@@ -412,7 +412,7 @@ static const struct command_table_helper 
command_table_helper_funcs = {
  * const struct command_table_helper **h - [out] struct of functions
  *
  */
-const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table()
+const struct command_table_helper *dal_cmd_tbl_helper_dce112_get_table(void)
 {
return _table_helper_funcs;
 }
diff --git 
a/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c 
b/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c
index 295e16e..8b30b55 100644
--- a/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c
+++ b/drivers/gpu/drm/amd/display/dc/bios/dce80/command_table_helper_dce80.c
@@ -348,7 +348,7 @@ static const struct command_table_helper 
command_table_helper_funcs = {
dal_cmd_table_helper_encoder_mode_bp_to_atom,
 };
 
-const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table()
+const struct command_table_helper *dal_cmd_tbl_helper_dce80_get_table(void)
 {
return _table_helper_funcs;
 }
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
index 5aa2270..ade5b8e 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_surface.c
@@ -145,7 +145,7 @@ void dc_gamma_release(struct dc_gamma **gamma)
*gamma = NULL;
 }
 
-struct dc_gamma *dc_create_gamma()
+struct dc_gamma *dc_create_gamma(void)
 {
struct dc_gamma *gamma = kzalloc(sizeof(*gamma), GFP_KERNEL);
 
@@ -175,7 +175,7 @@ void dc_transfer_func_release(struct 

Re: [PATCH umr] Skip ahead if PDE entry is actually a PTE entry. (v2)

[Christian König]

Am 06.11.2017 um 19:39 schrieb Tom St Denis:

On 06/11/17 01:34 PM, Christian König wrote:

Am 06.11.2017 um 19:28 schrieb Tom St Denis:

On 06/11/17 05:01 AM, Christian König wrote:

Am 04.11.2017 um 18:15 schrieb Tom St Denis:

Signed-off-by: Tom St Denis 


Still not perfect, but good enough for now. Patch is Tested-by: 
Christian König .


I think you need to rework the VM walking a bit, cause we need to 
support the T bit as well in the future and your code make a few 
assumptions which doesn't allow that.


Doesn't the T bit imply V=0 which means the page isn't backed by 
memory.  Not much umr could do about that other than to print out 
the T bit.


No, the T bit means translate further. In other words it is the 
counter part of the P bit and means that a PTE should be handled as a 
PDE.


But for this to have meaning you also need to handle the fragment 
size as well (Now I have you totally confused, haven't I? :).



Yes :-)

I thought fragment size was more for hinting to the cache controller 
and not actually part of the VM decoding.


Correct, our hardware devs are unfortunately using the same name for two 
different things.


The fragment size in the PTE means what you described above and controls 
the L1 TLB settings.


The fragment size in the PDE controls how big the PTE entries will be.

So with the default fragment size of zero in the PDE we have 512 PTEs 
resulting in a 4K page table.


Also the PI docs say T => "Tiled (PRT)" and from what I gather that 
just means the page is valid but might not be backed so instead of 
raising a page fault you raise a new fault that the application (?) 
handles accordingly.


There's an 'F' bit that is labeled "translate further".

Reading section 8 of said document seems to indicate you're confusing 
bits F and T or my doc is wildly out of date (or we're talking about 
different IP revisions)


Sorry my fault. I was just confusing the F and T bits.

Christian.



Tom



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

Re: [PATCH umr] Skip ahead if PDE entry is actually a PTE entry. (v2)

[Tom St Denis]

On 06/11/17 01:34 PM, Christian König wrote:

Am 06.11.2017 um 19:28 schrieb Tom St Denis:

On 06/11/17 05:01 AM, Christian König wrote:

Am 04.11.2017 um 18:15 schrieb Tom St Denis:

Signed-off-by: Tom St Denis 


Still not perfect, but good enough for now. Patch is Tested-by: 
Christian König .


I think you need to rework the VM walking a bit, cause we need to 
support the T bit as well in the future and your code make a few 
assumptions which doesn't allow that.


Doesn't the T bit imply V=0 which means the page isn't backed by 
memory.  Not much umr could do about that other than to print out the 
T bit.


No, the T bit means translate further. In other words it is the counter 
part of the P bit and means that a PTE should be handled as a PDE.


But for this to have meaning you also need to handle the fragment size 
as well (Now I have you totally confused, haven't I? :).



Yes :-)

I thought fragment size was more for hinting to the cache controller and 
not actually part of the VM decoding.


Also the PI docs say T => "Tiled (PRT)" and from what I gather that just 
means the page is valid but might not be backed so instead of raising a 
page fault you raise a new fault that the application (?) handles 
accordingly.


There's an 'F' bit that is labeled "translate further".

Reading section 8 of said document seems to indicate you're confusing 
bits F and T or my doc is wildly out of date (or we're talking about 
different IP revisions)


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

Re: [PATCH umr] Skip ahead if PDE entry is actually a PTE entry. (v2)

[Christian König]

Am 06.11.2017 um 19:28 schrieb Tom St Denis:

On 06/11/17 05:01 AM, Christian König wrote:

Am 04.11.2017 um 18:15 schrieb Tom St Denis:

Signed-off-by: Tom St Denis 


Still not perfect, but good enough for now. Patch is Tested-by: 
Christian König .


I think you need to rework the VM walking a bit, cause we need to 
support the T bit as well in the future and your code make a few 
assumptions which doesn't allow that.


Doesn't the T bit imply V=0 which means the page isn't backed by 
memory.  Not much umr could do about that other than to print out the 
T bit.


No, the T bit means translate further. In other words it is the counter 
part of the P bit and means that a PTE should be handled as a PDE.


But for this to have meaning you also need to handle the fragment size 
as well (Now I have you totally confused, haven't I? :).


Cheers,
Christian.



Tom
___
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 umr] Skip ahead if PDE entry is actually a PTE entry. (v2)

[Tom St Denis]

On 06/11/17 05:01 AM, Christian König wrote:

Am 04.11.2017 um 18:15 schrieb Tom St Denis:

Signed-off-by: Tom St Denis 


Still not perfect, but good enough for now. Patch is Tested-by: 
Christian König .


I think you need to rework the VM walking a bit, cause we need to 
support the T bit as well in the future and your code make a few 
assumptions which doesn't allow that.


Doesn't the T bit imply V=0 which means the page isn't backed by memory. 
 Not much umr could do about that other than to print out the T bit.


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

Re: radeon_dp_aux_transfer_native: 74 callbacks suppressed

[Christian König]

Additionally, we should also just fix this ratelimit macro anyway
since it's intended purpose is not to print anything when debugging isn't
enabled. What do you think Alex?
Well, at least I think that the described behavior of the macro is a bug 
which should be fixed.


But I think that is independent of the problem that radeon is a bit 
flaky about DP aux transfers.


Regards,
Christian.

Am 06.11.2017 um 17:17 schrieb Lyude Paul:

The main reason I added this was because the radeon driver's hotplugging paths
for DP do a ton of unnessecary probing, and because the driver usually also
checks all connectors every time there's a hotplug (there isn't much of a good
reason for this, it's just an old driver) it's not at all difficult to get
well more then 70 callbacks from this that end up filling the kernel log with
useless information (all the messages mean is that for some reason or another,
a DP aux transaction fails which usually just means the port is disconnected).

Ideally: We should be printing the first error code that the i2c handlers for
radeon return in addition to the suppressed messages (and this itself should
not be suppressed iirc), since that's almost always the only piece of
information that matters.

I wouldn't drop the message entirely though unless we're printing something
from DRM. Additionally, we should also just fix this ratelimit macro anyway
since it's intended purpose is not to print anything when debugging isn't
enabled. What do you think Alex?

On Mon, 2017-11-06 at 10:21 +0100, Jean Delvare wrote:

Hi all,

commit 92c177b7947d9c889ea7b024871445015ea74221
Author: Lyude
Date:   Wed Feb 22 16:34:53 2017 -0500

 drm/radeon/dp_auxch: Ratelimit aux transfer debug messages

does more harm than good in my opinion. Since this commit, I see
several occurrences of the following message in my kernel log daily:

radeon_dp_aux_transfer_native: 74 callbacks suppressed

I never got to see the "callback" in question though, not even once, as
this is a debug message which is off by default. Before the change, I
would not get any such message in the kernel log (as I would expect
when everything works as intended.)

Does this debug message really have any practical value? If not, the
easiest solution would be to simply drop it:

--- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
@@ -168,8 +168,10 @@ radeon_dp_aux_transfer_native(struct drm
goto done;
}
if (tmp & AUX_RX_ERROR_FLAGS) {
-   DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero:
%08x\n",
- tmp);
+   /*
+* aux transfers always fail with non-zero status flags
when
+* there's nothing connected on the port
+*/
ret = -EIO;
goto done;
}

I can resend this as a formal patch if you agree with this solution.

The actual cause of the problem is that ___ratelimit() prints its
message at KERN_WARNING level regardless of the level of the message
being suppressed. This makes ratelimiting debug, info or notice
messages awkward. Looks like a design overlook to me, maybe it should
be fixed, but that's a much bigger and intrusive change than the
proposal above.


___
dri-devel mailing list
dri-de...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel



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

Re: [PATCH 1/2] drm/amdgpu: use multipipe compute policy on non PL11 asics

[Andres Rodriguez]

Sorry my mail client seems to have blown up. My reply got cut off,
here is the full version:



On 2017-11-06 01:49 AM, Chunming Zhou wrote:
> Hi Andres,
>
Hi David,

> With your this patch, OCLperf hung.
Is this on all ASICs or just a specific one?

>
> Could you explain more?
>
> If I am correctly, the difference of with and without this patch is
> setting first two queue or setting all queues of pipe0 to queue_bitmap.
It is slightly different. With this patch we will also use the first
two queues of all pipes, not just pipe 0;

Pre-patch:

|-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
       

Post-patch:

|-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
 1100  1100  1100  1100

What this means is that we are allowing real multithreading for
compute. Jobs on different pipes allow for parallel execution of work.
Jobs on the same pipe (but different queues) use timeslicing to share
the hardware.


>
> Then UMD can use different number queue to submit command for compute
> selected by amdgpu_queue_mgr_map.
>
> I checked amdgpu_queue_mgr_map implementation,  CS_IOCTL can map user
> ring to different hw ring depending on busy or idle, right?
Yes, when a queue is first used, amdgpu_queue_mgr_map will decide what
the mapping is for a usermode ring to a kernel ring id.

> If yes, I see a bug in it, which will result in our sched_fence not
> work. Our sched fence assumes the job will be executed in order, your
> mapping queue breaks this.

I think here you mean that work will execute out of order because it
will go to different rings?

That should not happen, since the id mapping is permanent on a
per-context basis. Once a mapping is decided, it will be cached for
this context so that we keep execution order guarantees. See the
id-caching code in amdgpu_queue_mgr.c for reference.

As long as the usermode keeps submitting work to the same ring, it
will all be executed in order (all in the same ring). There is no
change in this guarantee compared to pre-patch. Note that even before
this patch amdgpu_queue_mgr_map has been using an LRU policy for a
long time now.

Regards,
Andres

On Mon, Nov 6, 2017 at 12:44 PM, Andres Rodriguez  wrote:
>
>
> On 2017-11-06 01:49 AM, Chunming Zhou wrote:
>>
>> Hi Andres,
>>
>
> Hi David,
>
>> With your this patch, OCLperf hung.
>
>
> Is this on all ASICs or just a specific one?
>
>>
>> Could you explain more?
>>
>> If I am correctly, the difference of with and without this patch is
>> setting first two queue or setting all queues of pipe0 to queue_bitmap.
>
>
> It is slightly different. With this patch we will also use the first two
> queues of all pipes, not just pipe 0;
>
> Pre-patch:
>
> |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
>        
>
> Post-patch:
>
> |-Pipe 0-||-Pipe 1-||-Pipe 2-||-Pipe 3-|
>  1100  1100  1100  1100
>
> What this means is that we are allowing real multithreading for compute.
> Jobs on different pipes allow for parallel execution of work. Jobs on the
> same pipe (but different queues) use timeslicing to share the hardware.
>
>
>
>>
>> Then UMD can use different number queue to submit command for compute
>> selected by amdgpu_queue_mgr_map.
>>
>> I checked amdgpu_queue_mgr_map implementation,  CS_IOCTL can map user ring
>> to different hw ring depending on busy or idle, right?
>>
>> If yes, I see a bug in it, which will result in our sched_fence not work.
>> Our sched fence assumes the job will be executed in order, your mapping
>> queue breaks this.
>>
>>
>> Regards,
>>
>> David Zhou
>>
>>
>> On 2017年09月27日 00:22, Andres Rodriguez wrote:
>>>
>>> A performance regression for OpenCL tests on Polaris11 had this feature
>>> disabled for all asics.
>>>
>>> Instead, disable it selectively on the affected asics.
>>>
>>> Signed-off-by: Andres Rodriguez 
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 14 --
>>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> index 4f6c68f..3d76e76 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
>>> @@ -109,9 +109,20 @@ void amdgpu_gfx_parse_disable_cu(unsigned *mask,
>>> unsigned max_se, unsigned max_s
>>>   }
>>>   }
>>> +static bool amdgpu_gfx_is_multipipe_capable(struct amdgpu_device *adev)
>>> +{
>>> +/* FIXME: spreading the queues across pipes causes perf regressions
>>> + * on POLARIS11 compute workloads */
>>> +if (adev->asic_type == CHIP_POLARIS11)
>>> +return false;
>>> +
>>> +return adev->gfx.mec.num_mec > 1;
>>> +}
>>> +
>>>   void amdgpu_gfx_compute_queue_acquire(struct amdgpu_device *adev)
>>>   {
>>>   int i, queue, pipe, mec;
>>> +bool multipipe_policy = amdgpu_gfx_is_multipipe_capable(adev);
>>>   /* policy for amdgpu compute queue ownership */
>>>   for (i = 0; 

Re: linux-next: Tree for Nov 6 (amdgpu_virt.c)

[Alex Deucher]

On Mon, Nov 6, 2017 at 12:20 PM, Randy Dunlap  wrote:
> On 11/05/2017 11:30 PM, Stephen Rothwell wrote:
>> Hi all,
>>
>> Changes since 20171103:
>>
>
> on i386, when CONFIG_MODULES is not set:
>
>   CC  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o
> In file included from ../arch/x86/include/asm/atomic.h:5:0,
>  from ../include/linux/atomic.h:5,
>  from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:31,
>  from ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:24:
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c: In function 
> 'amdgpu_virt_init_data_exchange':
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing 
> pointer to incomplete type
>  if (THIS_MODULE->version != NULL)
> ^
> ../include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
>   if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
>   ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of 
> macro 'if'
>  if (THIS_MODULE->version != NULL)
>  ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing 
> pointer to incomplete type
>  if (THIS_MODULE->version != NULL)
> ^
> ../include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
>   if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
>   ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of 
> macro 'if'
>  if (THIS_MODULE->version != NULL)
>  ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing 
> pointer to incomplete type
>  if (THIS_MODULE->version != NULL)
> ^
> ../include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
>__r = !!(cond); \
> ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of 
> macro 'if'
>  if (THIS_MODULE->version != NULL)
>  ^
> ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:332:29: error: dereferencing 
> pointer to incomplete type
>   strcpy(str, THIS_MODULE->version);
>  ^
> ../scripts/Makefile.build:314: recipe for target 
> 'drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o' failed
> make[5]: *** [drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o] Error 1
>
>
>
> Reported-by: Randy Dunlap 

Thanks.  Fixed in:
https://cgit.freedesktop.org/~airlied/linux/commit/?h=drm-next=e477e940dad1836c6f6d23353e424665b9316b6e

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

Re: linux-next: Tree for Nov 6 (amdgpu_virt.c)

[Randy Dunlap]

On 11/05/2017 11:30 PM, Stephen Rothwell wrote:
> Hi all,
> 
> Changes since 20171103:
> 

on i386, when CONFIG_MODULES is not set:

  CC  drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o
In file included from ../arch/x86/include/asm/atomic.h:5:0,
 from ../include/linux/atomic.h:5,
 from ../drivers/gpu/drm/amd/amdgpu/amdgpu.h:31,
 from ../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:24:
../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c: In function 
'amdgpu_virt_init_data_exchange':
../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing 
pointer to incomplete type
 if (THIS_MODULE->version != NULL)
^
../include/linux/compiler.h:58:30: note: in definition of macro '__trace_if'
  if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
  ^
../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of macro 
'if'
 if (THIS_MODULE->version != NULL)
 ^
../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing 
pointer to incomplete type
 if (THIS_MODULE->version != NULL)
^
../include/linux/compiler.h:58:42: note: in definition of macro '__trace_if'
  if (__builtin_constant_p(!!(cond)) ? !!(cond) :   \
  ^
../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of macro 
'if'
 if (THIS_MODULE->version != NULL)
 ^
../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:20: error: dereferencing 
pointer to incomplete type
 if (THIS_MODULE->version != NULL)
^
../include/linux/compiler.h:69:16: note: in definition of macro '__trace_if'
   __r = !!(cond); \
^
../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:331:5: note: in expansion of macro 
'if'
 if (THIS_MODULE->version != NULL)
 ^
../drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c:332:29: error: dereferencing 
pointer to incomplete type
  strcpy(str, THIS_MODULE->version);
 ^
../scripts/Makefile.build:314: recipe for target 
'drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o' failed
make[5]: *** [drivers/gpu/drm/amd/amdgpu/amdgpu_virt.o] Error 1



Reported-by: Randy Dunlap 

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

Re: [PATCH 3/3] ASoC: rt5645: Wait for 400msec before concluding on value of RT5645_VENDOR_ID2

[Mark Brown]

On Fri, Nov 03, 2017 at 04:35:45PM -0400, Alex Deucher wrote:

> Minimum time required between power On of codec and read
> of RT5645_VENDOR_ID2 is 400msec. We should wait and attempt
> before erroring out.

So the description says we have to wait 400ms before attempting a
read...

> BUG=b:66978383

What does this mean?

> @@ -3786,6 +3789,15 @@ static int rt5645_i2c_probe(struct i2c_client *i2c,
>   }
>   regmap_read(regmap, RT5645_VENDOR_ID2, );
>  
> + /*
> +  * Read for 400msec, as it is the interval required between
> +  * read and power On.
> +  */
> + while (val != RT5645_DEVICE_ID && val != RT5650_DEVICE_ID && --timeout) 
> {
> + msleep(1);
> + regmap_read(regmap, RT5645_VENDOR_ID2, );
> + }
> +

...but what we actually do is try to read up to 400 times starting well
before that 400ms is up.  This directly contradicts what the commit
message said we needed, may take a lot longer if the chip misbehaves on
the I2C bus while it's not ready (which wouldn't be that much of a
surprise), might lead to us reporting before the chip is really stable
(if the read happens to work while the chip isn't yet stable) and could
cause lots of noise on the console if the I2C controller gets upset.
What are we actually waiting for here?

If we really need 400ms of dead reckoning time (which is a lot for a
modern chip, that feels more like a VMID ramp) then it's going to be
safer to just do that.


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

Re: radeon_dp_aux_transfer_native: 74 callbacks suppressed

[Lyude Paul]

The main reason I added this was because the radeon driver's hotplugging paths
for DP do a ton of unnessecary probing, and because the driver usually also
checks all connectors every time there's a hotplug (there isn't much of a good
reason for this, it's just an old driver) it's not at all difficult to get
well more then 70 callbacks from this that end up filling the kernel log with
useless information (all the messages mean is that for some reason or another,
a DP aux transaction fails which usually just means the port is disconnected).

Ideally: We should be printing the first error code that the i2c handlers for
radeon return in addition to the suppressed messages (and this itself should
not be suppressed iirc), since that's almost always the only piece of
information that matters.

I wouldn't drop the message entirely though unless we're printing something
from DRM. Additionally, we should also just fix this ratelimit macro anyway
since it's intended purpose is not to print anything when debugging isn't
enabled. What do you think Alex?

On Mon, 2017-11-06 at 10:21 +0100, Jean Delvare wrote:
> Hi all,
> 
> commit 92c177b7947d9c889ea7b024871445015ea74221
> Author: Lyude
> Date:   Wed Feb 22 16:34:53 2017 -0500
> 
> drm/radeon/dp_auxch: Ratelimit aux transfer debug messages
> 
> does more harm than good in my opinion. Since this commit, I see
> several occurrences of the following message in my kernel log daily:
> 
> radeon_dp_aux_transfer_native: 74 callbacks suppressed
> 
> I never got to see the "callback" in question though, not even once, as
> this is a debug message which is off by default. Before the change, I
> would not get any such message in the kernel log (as I would expect
> when everything works as intended.)
> 
> Does this debug message really have any practical value? If not, the
> easiest solution would be to simply drop it:
> 
> --- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
> @@ -168,8 +168,10 @@ radeon_dp_aux_transfer_native(struct drm
>   goto done;
>   }
>   if (tmp & AUX_RX_ERROR_FLAGS) {
> - DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero:
> %08x\n",
> -   tmp);
> + /*
> +  * aux transfers always fail with non-zero status flags
> when
> +  * there's nothing connected on the port
> +  */
>   ret = -EIO;
>   goto done;
>   }
> 
> I can resend this as a formal patch if you agree with this solution.
> 
> The actual cause of the problem is that ___ratelimit() prints its
> message at KERN_WARNING level regardless of the level of the message
> being suppressed. This makes ratelimiting debug, info or notice
> messages awkward. Looks like a design overlook to me, maybe it should
> be fixed, but that's a much bigger and intrusive change than the
> proposal above.
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 1/3] ASoC: amd: Report accurate hw_ptr during dma

[Mark Brown]

On Fri, Nov 03, 2017 at 04:35:43PM -0400, Alex Deucher wrote:

> Signed-off-by: Vijendar Mukunda 
> Signed-off-by: Akshu Agrawal 
> Reviewed-on: https://chromium-review.googlesource.com/659699
> Commit-Ready: Akshu Agrawal 
> Tested-by: Akshu Agrawal 
> Reviewed-by: Jason Clinton 
> Reviewed-on: https://chromium-review.googlesource.com/676627

These two URLs are different, what was being reviewed here?  What is
Commit-Ready supposed to mean?


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

Re: release ex mode after hw_init

[Felix Kuehling]

[+Oded]

Hi Pixel,

Thanks, this looks a lot cleaner. The series is Reviewed-by: Felix
Kuehling . Oded, are you OK with this as well?

Regards,
  Felix


On 2017-11-06 03:18 AM, Pixel Ding wrote:
> Hi Felix,
>
> Please review.
>
> [PATCH 1/2] drm/amdkfd: initialise kfd inside amdgpu_device_init
> As you suggested, move kfd init/fini inside amdgpu_device_init.
> Other changes for KFD interfaces are dropped.
>
> [PATCH 2/2] drm/amdgpu: release exclusive mode after hw_init
>

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

Re: Did my graphics card get damaged?

[Michel Dänzer]

On 06/11/17 09:57 AM, Vladimir Klebanov wrote:
> On Sun, Nov 5, 2017 at 11:53 PM, Bridgman, John  wrote:
> 
>> The only problems I see in the xorg log are the following lines at the end, 
>> but not sure if they are serious or not.
>>
>> [   205.542] (WW) RADEON(0): flip queue failed: Invalid argument
>> [   205.542] (WW) RADEON(0): Page flip failed: Invalid argument
>> [   205.544] (WW) RADEON(0): flip queue failed: Invalid argument
>> [   205.544] (WW) RADEON(0): Page flip failed: Invalid argument
> 
> Neither do I. I hope someone here might have an idea.

They shouldn't be serious. It's extremely unlikely that the failure to
POST is directly related to them.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: Did my graphics card get damaged?

[Vladimir Klebanov]

Hi John,

On Sun, Nov 5, 2017 at 11:53 PM, Bridgman, John  wrote:
> For clarity, are you saying that when you go back to whatever distro you had 
> installed on the machine previously it is still  not booting correctly ?

Indeed. Even worse, nothing boots correctly as the machine does not
get to/complete POST.

> The only problems I see in the xorg log are the following lines at the end, 
> but not sure if they are serious or not.
>
> [   205.542] (WW) RADEON(0): flip queue failed: Invalid argument
> [   205.542] (WW) RADEON(0): Page flip failed: Invalid argument
> [   205.544] (WW) RADEON(0): flip queue failed: Invalid argument
> [   205.544] (WW) RADEON(0): Page flip failed: Invalid argument

Neither do I. I hope someone here might have an idea.

> If it makes you feel any better I just ran into a similar problem installing 
> latest Ubuntu 17.10 on a newer laptop - no POST, no display at all. Haven't 
> had time to mess with it to figure out what the problem is but will try to 
> make time.

Sorry about that.


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

radeon_dp_aux_transfer_native: 74 callbacks suppressed

[Jean Delvare]

Hi all,

commit 92c177b7947d9c889ea7b024871445015ea74221
Author: Lyude
Date:   Wed Feb 22 16:34:53 2017 -0500

drm/radeon/dp_auxch: Ratelimit aux transfer debug messages

does more harm than good in my opinion. Since this commit, I see
several occurrences of the following message in my kernel log daily:

radeon_dp_aux_transfer_native: 74 callbacks suppressed

I never got to see the "callback" in question though, not even once, as
this is a debug message which is off by default. Before the change, I
would not get any such message in the kernel log (as I would expect
when everything works as intended.)

Does this debug message really have any practical value? If not, the
easiest solution would be to simply drop it:

--- a/drivers/gpu/drm/radeon/radeon_dp_auxch.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_auxch.c
@@ -168,8 +168,10 @@ radeon_dp_aux_transfer_native(struct drm
goto done;
}
if (tmp & AUX_RX_ERROR_FLAGS) {
-   DRM_DEBUG_KMS_RATELIMITED("dp_aux_ch flags not zero: %08x\n",
- tmp);
+   /*
+* aux transfers always fail with non-zero status flags when
+* there's nothing connected on the port
+*/
ret = -EIO;
goto done;
}

I can resend this as a formal patch if you agree with this solution.

The actual cause of the problem is that ___ratelimit() prints its
message at KERN_WARNING level regardless of the level of the message
being suppressed. This makes ratelimiting debug, info or notice
messages awkward. Looks like a design overlook to me, maybe it should
be fixed, but that's a much bigger and intrusive change than the
proposal above.

-- 
Jean Delvare
SUSE L3 Support
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH umr] Tidy up skipping PDE=>PTE mappings

[Tom St Denis]

Should fix the indentation now when a PDE entry is actually a PTE.

Signed-off-by: Tom St Denis 
---
 src/lib/read_vram.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/src/lib/read_vram.c b/src/lib/read_vram.c
index 51823d71021e..89d55ff1bef6 100644
--- a/src/lib/read_vram.c
+++ b/src/lib/read_vram.c
@@ -509,20 +509,20 @@ static int umr_access_vram_ai(struct umr_asic *asic, 
uint32_t vmid,
pde_fields.system= (pde_entry >> 1) & 1;
pde_fields.cache = (pde_entry >> 2) & 1;
pde_fields.pte   = (pde_entry >> 54) & 
1;
-   if (!pde_fields.pte && memcmp(_fields, 
_array[pde_cnt], sizeof pde_fields) && asic->options.verbose)
-   fprintf(stderr, "[VERBOSE]: %s 
PDE%d=0x%016llx, VA=0x%012llx, PBA==0x%012llx, V=%d, S=%d, C=%d, P=%d\n",
-   
[12-pde_cnt*3],
-   pde_cnt,
-   (unsigned long 
long)pde_entry,
-   (unsigned long 
long)address & va_mask,
-   (unsigned long 
long)pde_fields.pte_base_addr,
-   (int)pde_fields.valid,
-   (int)pde_fields.system,
-   (int)pde_fields.cache,
-   (int)pde_fields.pte);
-   memcpy(_array[pde_cnt++], _fields, 
sizeof pde_fields);
-
-   if (pde_fields.pte) {
+   if (!pde_fields.pte) {
+   if (memcmp(_fields, 
_array[pde_cnt], sizeof pde_fields) && asic->options.verbose)
+   fprintf(stderr, "[VERBOSE]: %s 
PDE%d=0x%016llx, VA=0x%012llx, PBA==0x%012llx, V=%d, S=%d, C=%d, P=%d\n",
+   
[12-pde_cnt*3],
+   pde_cnt,
+   (unsigned long 
long)pde_entry,
+   (unsigned long 
long)address & va_mask,
+   (unsigned long 
long)pde_fields.pte_base_addr,
+   
(int)pde_fields.valid,
+   
(int)pde_fields.system,
+   
(int)pde_fields.cache,
+   
(int)pde_fields.pte);
+   memcpy(_array[pde_cnt++], 
_fields, sizeof pde_fields);
+   } else {
pte_entry = pde_entry;
goto pde_is_pte;
}
-- 
2.12.0

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

Re: [PATCH] drm/amd/display: remove some unneeded code

[Christian König]

Am 06.11.2017 um 12:44 schrieb Dan Carpenter:

We assign "v_init = asic_blank_start;" a few lines earlier so there is
no need to do it again inside the if statements.  Also "v_init" is
unsigned so it can't be less than zero.

Signed-off-by: Dan Carpenter 


Acked-by: Christian König 



diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
index 1994865d4351..c7333cdf1802 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
@@ -236,13 +236,10 @@ static void tgn10_program_timing(
if (tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT ||
tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
tg->dlg_otg_param.signal == SIGNAL_TYPE_EDP) {
-   v_init = asic_blank_start;
start_point = 1;
if (patched_crtc_timing.flags.INTERLACE == 1)
field_num = 1;
}
-   if (v_init < 0)
-   v_init = 0;
v_fp2 = 0;
if (tg->dlg_otg_param.vstartup_start > asic_blank_end)
v_fp2 = tg->dlg_otg_param.vstartup_start > asic_blank_end;
___
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] drm/amd/display: checking for NULL instead of IS_ERR()

[Christian König]

Am 06.11.2017 um 12:43 schrieb Dan Carpenter:

backlight_device_register() never returns NULL, it returns error
pointers on error so the check here is wrong.

Signed-off-by: Dan Carpenter 


Acked-by: Christian König 



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 33a15a1d818c..75f9383f5b9b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1296,7 +1296,7 @@ amdgpu_dm_register_backlight_device(struct 
amdgpu_display_manager *dm)
_dm_backlight_ops,
);
  
-	if (NULL == dm->backlight_dev)

+   if (IS_ERR(dm->backlight_dev))
DRM_ERROR("DM: Backlight registration failed!\n");
else
DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n", 
bl_name);



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

[PATCH] drm/amd/display: remove some unneeded code

[Dan Carpenter]

We assign "v_init = asic_blank_start;" a few lines earlier so there is
no need to do it again inside the if statements.  Also "v_init" is
unsigned so it can't be less than zero.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
index 1994865d4351..c7333cdf1802 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_timing_generator.c
@@ -236,13 +236,10 @@ static void tgn10_program_timing(
if (tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT ||
tg->dlg_otg_param.signal == SIGNAL_TYPE_DISPLAY_PORT_MST ||
tg->dlg_otg_param.signal == SIGNAL_TYPE_EDP) {
-   v_init = asic_blank_start;
start_point = 1;
if (patched_crtc_timing.flags.INTERLACE == 1)
field_num = 1;
}
-   if (v_init < 0)
-   v_init = 0;
v_fp2 = 0;
if (tg->dlg_otg_param.vstartup_start > asic_blank_end)
v_fp2 = tg->dlg_otg_param.vstartup_start > asic_blank_end;
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amd/display: checking for NULL instead of IS_ERR()

[Dan Carpenter]

backlight_device_register() never returns NULL, it returns error
pointers on error so the check here is wrong.

Signed-off-by: Dan Carpenter 

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 33a15a1d818c..75f9383f5b9b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1296,7 +1296,7 @@ amdgpu_dm_register_backlight_device(struct 
amdgpu_display_manager *dm)
_dm_backlight_ops,
);
 
-   if (NULL == dm->backlight_dev)
+   if (IS_ERR(dm->backlight_dev))
DRM_ERROR("DM: Backlight registration failed!\n");
else
DRM_DEBUG_DRIVER("DM: Registered Backlight device: %s\n", 
bl_name);
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

AMD, please run Smatch on your driver

[Dan Carpenter]

Linux-next was offline for the last month and the AMD drm driver went
through major changes.  Anyway, I'm a bit overwhelmed by the number of
warnings and I'm not going to be able to go through them all so I'm just
sending them to you unfiltered.

Part of the problem is that I'm not running the released version of
Smatch myself.  That has two effects.  1) The released version is
crappier than I had imagined.  2) I get *way* more warnings than you see
which is overwhelming...  So this is mostly my fault and I will try to
do better.

Here are the current warnings from Friday's linux-next, lightly edited.
I know that everyone hates a big dump of static checker warnings...
Speaking of being ignored, I sent a fix for this one back in August but
never heard back:

  drivers/gpu/drm/amd/amdgpu/ci_dpm.c:4553 ci_set_mc_special_registers()
  error: buffer overflow 'table->mc_reg_address' 16 <= 16

https://lists.freedesktop.org/archives/amd-gfx/2017-August/012333.html

So this is partly your fault as well because if you cleaned up static
checker warnings little by little, then they wouldn't pile up like this.
Eventually, everyone is going to have to start running Smatch for
themselves because it scales better than relying on me to do it.

regards,
dan carpenter

drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:2224 amdgpu_device_init() warn: 
'adev->rio_mem' was not released on error
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:2395 amdgpu_device_init() warn: 
'adev->rio_mem' was not released on error
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3373 amdgpu_debugfs_regs_write() 
warn: 'mutex:>pm.mutex' is sometimes locked here and sometimes unlocked.
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3377 amdgpu_debugfs_regs_write() 
warn: 'mutex:>pm.mutex' is sometimes locked here and sometimes unlocked.
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:3771 amdgpu_debugfs_gpr_read() 
error: buffer overflow 'data' 1024 <= 4095
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:155 amdgpu_driver_load_kms() warn: we 
tested 'r' before and it was 'false'
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c:689 amdgpu_gem_op_ioctl() warn: should 
'robj->tbo.mem.page_alignment << 12' be a 64 bit type?
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:196 amdgpu_cs_parser_init() warn: 
'mutex:>ctx->lock' is sometimes locked here and sometimes unlocked.
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:674 amdgpu_cs_parser_bos() warn: we 
tested 'r' before and it was 'false'
drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c:755 amdgpu_cs_parser_fini() warn: 
'mutex:>ctx->lock' is sometimes locked here and sometimes unlocked.
drivers/gpu/drm/amd/amdgpu/atombios_i2c.c:72 
amdgpu_atombios_i2c_process_i2c_ch() warn: impossible condition '(num > 255) => 
(0-255 > 255)'
drivers/gpu/drm/amd/amdgpu/amdgpu_queue_mgr.c:217 amdgpu_queue_mgr_map() warn: 
variable dereferenced before check 'mgr' (see line 215)
drivers/gpu/drm/amd/amdgpu/kv_dpm.c:1618 kv_get_acp_boot_level() warn: always 
true condition '(table->entries[i]->clk >= 0) => (0-u32max >= 0)'
drivers/gpu/drm/amd/amdgpu/ci_dpm.c:4560 ci_set_mc_special_registers() error: 
buffer overflow 'table->mc_reg_address' 16 <= 16
drivers/gpu/drm/amd/amdgpu/ci_dpm.c:5065 
ci_request_link_speed_change_before_state_change() warn: missing break? 
reassigning 'pi->force_pcie_gen'
drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:5256 gfx_v7_0_get_cu_info() error: buffer 
overflow 'cu_info->bitmap' 4 <= 4
drivers/gpu/drm/amd/amdgpu/si.c:1288 si_common_early_init() warn: inconsistent 
indenting
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c:3026 dce_v6_0_pageflip_irq() warn: 
inconsistent indenting
drivers/gpu/drm/amd/amdgpu/si_dpm.c:6242 
si_request_link_speed_change_before_state_change() warn: missing break? 
reassigning 'si_pi->force_pcie_gen'
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:5222 gfx_v8_0_pre_soft_reset() warn: 
inconsistent indenting
drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:7105 gfx_v8_0_get_cu_info() error: buffer 
overflow 'cu_info->bitmap' 4 <= 4
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:3077 gfx_v9_0_soft_reset() warn: we 
tested 'grbm_soft_reset' before and it was 'true'
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:3644 gfx_v9_0_ring_emit_ib_gfx() warn: 
inconsistent indenting
drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c:4457 gfx_v9_0_get_cu_info() error: buffer 
overflow 'cu_info->bitmap' 4 <= 4
drivers/gpu/drm/amd/amdgpu/amdgpu_cgs.c:605 amdgpu_cgs_lock_grbm_idx() warn: 
'mutex:>grbm_idx_mutex' is sometimes locked here and sometimes unlocked.
drivers/gpu/drm/amd/amdgpu/../scheduler/gpu_scheduler.c:696 amd_sched_init() 
warn: call of 'kthread_create_on_node' with non-constant format argument
drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/tonga_smumgr.c:3128 
tonga_set_mc_special_registers() error: buffer overflow 'table->mc_reg_address' 
16 <= 16
drivers/gpu/drm/amd/amdgpu/../powerplay/smumgr/polaris10_smumgr.c:916 
polaris10_calculate_sclk_params() warn: should 'clock << 
table->SclkFcwRangeTable[sclk_setting->PllRange].postdiv' be a 64 bit type?

[PATCH] drm/amdgpu:remove debugfs file in amdgpu_device_finish

[Gary Sun]

remove debugfs file in amdgpu_device_finish

Signed-off-by: Gary Sun 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   18 ++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 4f919d3..6cfcb5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1250,6 +1250,7 @@ struct amdgpu_debugfs {
 int amdgpu_debugfs_add_files(struct amdgpu_device *adev,
 const struct drm_info_list *files,
 unsigned nfiles);
+int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev);
 int amdgpu_debugfs_fence_init(struct amdgpu_device *adev);
 
 #if defined(CONFIG_DEBUG_FS)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7b7439f..ee800ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2520,6 +2520,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
amdgpu_doorbell_fini(adev);
amdgpu_pm_sysfs_fini(adev);
amdgpu_debugfs_regs_cleanup(adev);
+   amdgpu_debugfs_cleanup_files(adev);
 }
 
 
@@ -3304,6 +3305,23 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev,
return 0;
 }
 
+int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev)
+{
+   unsigned int i;
+
+   for (i = 0; i < adev->debugfs_count; i++) {
+#if defined(CONFIG_DEBUG_FS)
+   drm_debugfs_remove_files(adev->debugfs[i].files,
+   adev->debugfs[i].num_files,
+   adev->ddev->primary);
+#endif
+   adev->debugfs[i].files = NUL;
+   adev->debugfs[i].num_files = 0;
+   }
+   adev->debugfs_count = 0;
+   return 0;
+}
+
 #if defined(CONFIG_DEBUG_FS)
 
 static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf,
-- 
1.7.1

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

Re: [PATCH umr] Skip ahead if PDE entry is actually a PTE entry. (v2)

[Christian König]

Am 04.11.2017 um 18:15 schrieb Tom St Denis:

Signed-off-by: Tom St Denis 


Still not perfect, but good enough for now. Patch is Tested-by: 
Christian König .


I think you need to rework the VM walking a bit, cause we need to 
support the T bit as well in the future and your code make a few 
assumptions which doesn't allow that.


Regards,
Christian.



(v2) Don't print out PDE entries with PTE bit set
---
  src/lib/read_vram.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/lib/read_vram.c b/src/lib/read_vram.c
index 0df48dadec12..51823d71021e 100644
--- a/src/lib/read_vram.c
+++ b/src/lib/read_vram.c
@@ -509,7 +509,7 @@ static int umr_access_vram_ai(struct umr_asic *asic, 
uint32_t vmid,
pde_fields.system= (pde_entry >> 1) & 1;
pde_fields.cache = (pde_entry >> 2) & 1;
pde_fields.pte   = (pde_entry >> 54) & 
1;
-   if (memcmp(_fields, _array[pde_cnt], sizeof 
pde_fields) && asic->options.verbose)
+   if (!pde_fields.pte && memcmp(_fields, 
_array[pde_cnt], sizeof pde_fields) && asic->options.verbose)
fprintf(stderr, "[VERBOSE]: %s 
PDE%d=0x%016llx, VA=0x%012llx, PBA==0x%012llx, V=%d, S=%d, C=%d, P=%d\n",

[12-pde_cnt*3],
pde_cnt,
@@ -522,6 +522,11 @@ static int umr_access_vram_ai(struct umr_asic *asic, 
uint32_t vmid,
(int)pde_fields.pte);
memcpy(_array[pde_cnt++], _fields, 
sizeof pde_fields);
  
+if (pde_fields.pte) {

+   pte_entry = pde_entry;
+   goto pde_is_pte;
+   }
+
if (!pde_fields.system)
pde_fields.pte_base_addr -= 
vm_fb_offset;
  
@@ -539,6 +544,7 @@ static int umr_access_vram_ai(struct umr_asic *asic, uint32_t vmid,

return -1;
  
  			// decode PTE values

+pde_is_pte:
pte_fields.page_base_addr = pte_entry & 
0xFF000ULL;
pte_fields.fragment   = (pte_entry >> 7)  & 0x1F;
pte_fields.system = (pte_entry >> 1) & 1;



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

Re: [PATCH] drm/amdgpu: resize VRAM BAR for CPU access v5

[Christian König]

What benefits will the larger VRAM bar bring in,
Well the obvious one that the CPU can access all of VRAM. There are some 
games/workloads which rely quite a bit on this and it simplifies MM 
largely when you don't need to shuffle buffers around for CPU access.



or are there new features relying on larger VRAM bar under development?

Not that I know of.


I think it’s better to try something which can keep this change for SRIOV and 
don’t introduce too much latency.
Agree on that, but reprogramming the PCI BAR is something which takes 
quite a bunch of time.


That's why I asked if it is sufficient if you comment out this one 
pci_write_config_word() and it works?


If yes than we can just skip that write, but I have doubts that this 
will be sufficient.


On the other hand in an SRIOV environment the host can resize the BAR 
before starting the client, so that whole stuff shouldn't be necessary 
in the first place.


Regards,
Christian.

Am 06.11.2017 um 10:34 schrieb Ding, Pixel:

What benefits will the larger VRAM bar bring in, or are there new features 
relying on larger VRAM bar under development? SRIOV wants to leverage this sort 
of optimization too.

I think it’s better to try something which can keep this change for SRIOV and 
don’t introduce too much latency.

—
Sincerely Yours,
Pixel







On 06/11/2017, 5:16 PM, "Christian König"  
wrote:


Am 06.11.2017 um 08:21 schrieb Ding, Pixel:

Hi Christian,

The latest driver fails to load on SRIOV VF with xen hypervisor driver.

+int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
+{
+ u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
+ u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1;
+ u16 cmd;
+ int r;
+
+ /* Disable memory decoding while we change the BAR addresses and size */
+ pci_read_config_word(adev->pdev, PCI_COMMAND, );
+ pci_write_config_word(adev->pdev, PCI_COMMAND,

The function above introduces 900ms latency during init in exclusive accessing.


+ cmd & ~PCI_COMMAND_MEMORY);
+


Can we bypass disablement of response in memory space here? Any concern or 
suggestion?

Mhm, that was added to be on the extra safe side. In theory we can skip it.

Does PCI BAR resize work on SRIOV if you skip this or is that just a NOP
anyway?

Might be a good idea to just immediately return here when SRIOV is active.

Regards,
Christian.



—
Sincerely Yours,
Pixel







On 02/11/2017, 9:13 PM, "amd-gfx on behalf of Alex Deucher" 
 wrote:


On Thu, Nov 2, 2017 at 9:02 AM, Christian König  wrote:

From: Christian König 

Try to resize BAR0 to let CPU access all of VRAM.

v2: rebased, style cleanups, disable mem decode before resize,
  handle gmc_v9 as well, round size up to power of two.
v3: handle gmc_v6 as well, release and reassign all BARs in the driver.
v4: rename new function to amdgpu_device_resize_fb_bar,
  reenable mem decoding only if all resources are assigned.
v5: reorder resource release, return -ENODEV instead of BUG_ON().

Signed-off-by: Christian König 

Reviewed-by: Alex Deucher 


---
   drivers/gpu/drm/amd/amdgpu/amdgpu.h|  1 +
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 48 
++
   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c  | 12 ++--
   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  | 13 ++--
   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  | 13 ++--
   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  | 14 ++---
   6 files changed, 88 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 2730a75..4f919d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1854,6 +1854,7 @@ void amdgpu_ttm_placement_from_domain(struct amdgpu_bo 
*abo, u32 domain);
   bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
   void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, 
u64 base);
   void amdgpu_gart_location(struct amdgpu_device *adev, struct amdgpu_mc *mc);
+int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev);
   void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size);
   int amdgpu_ttm_init(struct amdgpu_device *adev);
   void amdgpu_ttm_fini(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 8b33adf..cb3a0ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -410,6 +410,9 @@ static int amdgpu_doorbell_init(struct amdgpu_device *adev)
  return 0;
  }

+   if (pci_resource_flags(adev->pdev, 2) & IORESOURCE_UNSET)
+   return -EINVAL;
+
  /* doorbell bar mapping */
   

[bug report] drm/amd/display : add high part address calculation for underlay

[Dan Carpenter]

Hello Shirish S,

The patch 4d3e00dad80a: "drm/amd/display : add high part address
calculation for underlay" from Oct 19, 2017, leads to the following
static checker warning:

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:1838 
fill_plane_attributes_from_fb()
warn: cast after binop

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c
  1831  } else {
  1832  awidth = ALIGN(fb->width, 64);
  1833  plane_state->address.type = 
PLN_ADDR_TYPE_VIDEO_PROGRESSIVE;
  1834  
plane_state->address.video_progressive.luma_addr.low_part
  1835  = 
lower_32_bits(fb_location);
  1836  
plane_state->address.video_progressive.luma_addr.high_part
  1837  = 
upper_32_bits(fb_location);
  1838  chroma_addr = fb_location + (u64)(awidth * fb->height);
^
This cast is a no-op.  fb_location is a u64.  awidth and fb->height are
both unsigned int.  Perhaps you meant to do:

chroma_addr = fb_location + ((u64)awidth * fb->height);

Or maybe just remove the cast?  It doesn't help readability, because if
it did I wouldn't be so confused.

  1839  
plane_state->address.video_progressive.chroma_addr.low_part
  1840  = 
lower_32_bits(chroma_addr);
  1841  
plane_state->address.video_progressive.chroma_addr.high_part
  1842  = 
upper_32_bits(chroma_addr);
  1843  plane_state->plane_size.video.luma_size.x = 0;
  1844  plane_state->plane_size.video.luma_size.y = 0;
  1845  plane_state->plane_size.video.luma_size.width = awidth;
  1846  plane_state->plane_size.video.luma_size.height = 
fb->height;
  1847  /* TODO: unhardcode */
  1848  plane_state->plane_size.video.luma_pitch = awidth;

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

Re: [PATCH] drm/amd/powerplay: suppress KASAN out of bounds warning in vega10_populate_all_memory_levels

[Christian König]

Am 06.11.2017 um 10:33 schrieb Evan Quan:

Change-Id: I437e3e08cd48943de277c5d3eefdbaf21fd6a489
Signed-off-by: Evan Quan 


Tested-and-Acked-by: Christian König 


---
  drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index 7079e61..0364a96 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -1807,6 +1807,10 @@ static int vega10_populate_all_memory_levels(struct 
pp_hwmgr *hwmgr)
mem_channels = (cgs_read_register(hwmgr->device, reg) &
DF_CS_AON0_DramBaseAddress0__IntLvNumChan_MASK) >>
DF_CS_AON0_DramBaseAddress0__IntLvNumChan__SHIFT;
+   PP_ASSERT_WITH_CODE(mem_channels < ARRAY_SIZE(channel_number),
+   "Mem Channel Index Exceeded maximum!",
+   return -1);
+
pp_table->NumMemoryChannels = cpu_to_le16(mem_channels);
pp_table->MemoryChannelWidth =
cpu_to_le16(HBM_MEMORY_CHANNEL_WIDTH *



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

Re: [PATCH] drm/amdgpu: resize VRAM BAR for CPU access v5

[Ding, Pixel]

What benefits will the larger VRAM bar bring in, or are there new features 
relying on larger VRAM bar under development? SRIOV wants to leverage this sort 
of optimization too.

I think it’s better to try something which can keep this change for SRIOV and 
don’t introduce too much latency.

— 
Sincerely Yours,
Pixel







On 06/11/2017, 5:16 PM, "Christian König"  
wrote:

>Am 06.11.2017 um 08:21 schrieb Ding, Pixel:
>> Hi Christian,
>>
>> The latest driver fails to load on SRIOV VF with xen hypervisor driver.
>>
>> +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
>> +{
>> + u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
>> + u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1;
>> + u16 cmd;
>> + int r;
>> +
>> + /* Disable memory decoding while we change the BAR addresses and size 
>> */
>> + pci_read_config_word(adev->pdev, PCI_COMMAND, );
>> + pci_write_config_word(adev->pdev, PCI_COMMAND,
>>
>> The function above introduces 900ms latency during init in exclusive 
>> accessing.
>>
>>
>> + cmd & ~PCI_COMMAND_MEMORY);
>> +
>>
>>
>> Can we bypass disablement of response in memory space here? Any concern or 
>> suggestion?
>
>Mhm, that was added to be on the extra safe side. In theory we can skip it.
>
>Does PCI BAR resize work on SRIOV if you skip this or is that just a NOP 
>anyway?
>
>Might be a good idea to just immediately return here when SRIOV is active.
>
>Regards,
>Christian.
>
>>
>>
>> —
>> Sincerely Yours,
>> Pixel
>>
>>
>>
>>
>>
>>
>>
>> On 02/11/2017, 9:13 PM, "amd-gfx on behalf of Alex Deucher" 
>>  
>> wrote:
>>
>>> On Thu, Nov 2, 2017 at 9:02 AM, Christian König  
>>> wrote:
 From: Christian König 

 Try to resize BAR0 to let CPU access all of VRAM.

 v2: rebased, style cleanups, disable mem decode before resize,
  handle gmc_v9 as well, round size up to power of two.
 v3: handle gmc_v6 as well, release and reassign all BARs in the driver.
 v4: rename new function to amdgpu_device_resize_fb_bar,
  reenable mem decoding only if all resources are assigned.
 v5: reorder resource release, return -ENODEV instead of BUG_ON().

 Signed-off-by: Christian König 
>>> Reviewed-by: Alex Deucher 
>>>
 ---
   drivers/gpu/drm/amd/amdgpu/amdgpu.h|  1 +
   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 48 
 ++
   drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c  | 12 ++--
   drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  | 13 ++--
   drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  | 13 ++--
   drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  | 14 ++---
   6 files changed, 88 insertions(+), 13 deletions(-)

 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
 b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 index 2730a75..4f919d3 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
 @@ -1854,6 +1854,7 @@ void amdgpu_ttm_placement_from_domain(struct 
 amdgpu_bo *abo, u32 domain);
   bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
   void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc 
 *mc, u64 base);
   void amdgpu_gart_location(struct amdgpu_device *adev, struct amdgpu_mc 
 *mc);
 +int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev);
   void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 
 size);
   int amdgpu_ttm_init(struct amdgpu_device *adev);
   void amdgpu_ttm_fini(struct amdgpu_device *adev);
 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 index 8b33adf..cb3a0ac 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
 @@ -410,6 +410,9 @@ static int amdgpu_doorbell_init(struct amdgpu_device 
 *adev)
  return 0;
  }

 +   if (pci_resource_flags(adev->pdev, 2) & IORESOURCE_UNSET)
 +   return -EINVAL;
 +
  /* doorbell bar mapping */
  adev->doorbell.base = pci_resource_start(adev->pdev, 2);
  adev->doorbell.size = pci_resource_len(adev->pdev, 2);
 @@ -749,6 +752,51 @@ int amdgpu_fw_reserve_vram_init(struct amdgpu_device 
 *adev)
  return r;
   }

 +/**
 + * amdgpu_device_resize_fb_bar - try to resize FB BAR
 + *
 + * @adev: amdgpu_device pointer
 + *
 + * Try to resize FB BAR to make all VRAM CPU accessible. We try very hard 
 not
 + * to fail, but if any of the BARs is not accessible after the size we 
 abort
 + * driver loading by returning -ENODEV.

[PATCH] drm/amd/powerplay: suppress KASAN out of bounds warning in vega10_populate_all_memory_levels

[Evan Quan]

Change-Id: I437e3e08cd48943de277c5d3eefdbaf21fd6a489
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
index 7079e61..0364a96 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c
@@ -1807,6 +1807,10 @@ static int vega10_populate_all_memory_levels(struct 
pp_hwmgr *hwmgr)
mem_channels = (cgs_read_register(hwmgr->device, reg) &
DF_CS_AON0_DramBaseAddress0__IntLvNumChan_MASK) >>
DF_CS_AON0_DramBaseAddress0__IntLvNumChan__SHIFT;
+   PP_ASSERT_WITH_CODE(mem_channels < ARRAY_SIZE(channel_number),
+   "Mem Channel Index Exceeded maximum!",
+   return -1);
+
pp_table->NumMemoryChannels = cpu_to_le16(mem_channels);
pp_table->MemoryChannelWidth =
cpu_to_le16(HBM_MEMORY_CHANNEL_WIDTH *
-- 
2.7.4

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

Re: [PATCH] drm/amdgpu:remove debugfs file in amdgpu_device_finish

[Christian König]

Am 06.11.2017 um 10:20 schrieb Gary Sun:

remove debugfs file in amdgpu_device_finish


NAK, the debugfs files are removed automatically by drm_debugfs_cleanup().

So that patch is unnecessary.

Regards,
Christian.



Signed-off-by: Gary Sun 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h|1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |   18 ++
  2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 4f919d3..6cfcb5f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1250,6 +1250,7 @@ struct amdgpu_debugfs {
  int amdgpu_debugfs_add_files(struct amdgpu_device *adev,
 const struct drm_info_list *files,
 unsigned nfiles);
+int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev);
  int amdgpu_debugfs_fence_init(struct amdgpu_device *adev);
  
  #if defined(CONFIG_DEBUG_FS)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7b7439f..ee800ab 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2520,6 +2520,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
amdgpu_doorbell_fini(adev);
amdgpu_pm_sysfs_fini(adev);
amdgpu_debugfs_regs_cleanup(adev);
+   amdgpu_debugfs_cleanup_files(adev);
  }
  
  
@@ -3304,6 +3305,23 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev,

return 0;
  }
  
+int amdgpu_debugfs_cleanup_files(struct amdgpu_device *adev)

+{
+   unsigned int i;
+
+   for (i = 0; i < adev->debugfs_count; i++) {
+#if defined(CONFIG_DEBUG_FS)
+   drm_debugfs_remove_files(adev->debugfs[i].files,
+   adev->debugfs[i].num_files,
+   adev->ddev->primary);
+#endif
+   adev->debugfs[i].files = NUL;
+   adev->debugfs[i].num_files = 0;
+   }
+   adev->debugfs_count = 0;
+   return 0;
+}
+
  #if defined(CONFIG_DEBUG_FS)
  
  static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf,



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

Re: [PATCH] drm/amdgpu: resize VRAM BAR for CPU access v5

[Christian König]

Am 06.11.2017 um 08:21 schrieb Ding, Pixel:

Hi Christian,

The latest driver fails to load on SRIOV VF with xen hypervisor driver.

+int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
+{
+ u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
+ u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1;
+ u16 cmd;
+ int r;
+
+ /* Disable memory decoding while we change the BAR addresses and size */
+ pci_read_config_word(adev->pdev, PCI_COMMAND, );
+ pci_write_config_word(adev->pdev, PCI_COMMAND,

The function above introduces 900ms latency during init in exclusive accessing.


+ cmd & ~PCI_COMMAND_MEMORY);
+


Can we bypass disablement of response in memory space here? Any concern or 
suggestion?


Mhm, that was added to be on the extra safe side. In theory we can skip it.

Does PCI BAR resize work on SRIOV if you skip this or is that just a NOP 
anyway?


Might be a good idea to just immediately return here when SRIOV is active.

Regards,
Christian.




—
Sincerely Yours,
Pixel







On 02/11/2017, 9:13 PM, "amd-gfx on behalf of Alex Deucher" 
 wrote:


On Thu, Nov 2, 2017 at 9:02 AM, Christian König  wrote:

From: Christian König 

Try to resize BAR0 to let CPU access all of VRAM.

v2: rebased, style cleanups, disable mem decode before resize,
 handle gmc_v9 as well, round size up to power of two.
v3: handle gmc_v6 as well, release and reassign all BARs in the driver.
v4: rename new function to amdgpu_device_resize_fb_bar,
 reenable mem decoding only if all resources are assigned.
v5: reorder resource release, return -ENODEV instead of BUG_ON().

Signed-off-by: Christian König 

Reviewed-by: Alex Deucher 


---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h|  1 +
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 48 ++
  drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c  | 12 ++--
  drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c  | 13 ++--
  drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c  | 13 ++--
  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c  | 14 ++---
  6 files changed, 88 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 2730a75..4f919d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1854,6 +1854,7 @@ void amdgpu_ttm_placement_from_domain(struct amdgpu_bo 
*abo, u32 domain);
  bool amdgpu_ttm_bo_is_amdgpu_bo(struct ttm_buffer_object *bo);
  void amdgpu_vram_location(struct amdgpu_device *adev, struct amdgpu_mc *mc, 
u64 base);
  void amdgpu_gart_location(struct amdgpu_device *adev, struct amdgpu_mc *mc);
+int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev);
  void amdgpu_ttm_set_active_vram_size(struct amdgpu_device *adev, u64 size);
  int amdgpu_ttm_init(struct amdgpu_device *adev);
  void amdgpu_ttm_fini(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 8b33adf..cb3a0ac 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -410,6 +410,9 @@ static int amdgpu_doorbell_init(struct amdgpu_device *adev)
 return 0;
 }

+   if (pci_resource_flags(adev->pdev, 2) & IORESOURCE_UNSET)
+   return -EINVAL;
+
 /* doorbell bar mapping */
 adev->doorbell.base = pci_resource_start(adev->pdev, 2);
 adev->doorbell.size = pci_resource_len(adev->pdev, 2);
@@ -749,6 +752,51 @@ int amdgpu_fw_reserve_vram_init(struct amdgpu_device *adev)
 return r;
  }

+/**
+ * amdgpu_device_resize_fb_bar - try to resize FB BAR
+ *
+ * @adev: amdgpu_device pointer
+ *
+ * Try to resize FB BAR to make all VRAM CPU accessible. We try very hard not
+ * to fail, but if any of the BARs is not accessible after the size we abort
+ * driver loading by returning -ENODEV.
+ */
+int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
+{
+   u64 space_needed = roundup_pow_of_two(adev->mc.real_vram_size);
+   u32 rbar_size = order_base_2(((space_needed >> 20) | 1)) - 1;
+   u16 cmd;
+   int r;
+
+   /* Disable memory decoding while we change the BAR addresses and size */
+   pci_read_config_word(adev->pdev, PCI_COMMAND, );
+   pci_write_config_word(adev->pdev, PCI_COMMAND,
+ cmd & ~PCI_COMMAND_MEMORY);
+
+   /* Free the VRAM and doorbell BAR, we most likely need to move both. */
+   amdgpu_doorbell_fini(adev);
+   if (adev->asic_type >= CHIP_BONAIRE)
+   pci_release_resource(adev->pdev, 2);
+
+   pci_release_resource(adev->pdev, 0);
+
+   r = pci_resize_resource(adev->pdev, 0, rbar_size);
+   if (r == -ENOSPC)
+   DRM_INFO("Not enough PCI address 

Re: [PATCH] drm/amd/display: small cleanup in destruct()

[Christian König]

Am 06.11.2017 um 08:07 schrieb Dan Carpenter:

Static analysis tools get annoyed that we don't indent this if
statement.  Actually, the if statement isn't required because kfree()
can handle NULL pointers just fine.  The DCE110STRENC_FROM_STRENC()
macro is a wrapper around container_of() but it's basically a no-op or a
cast.  Anyway, it's not really appropriate here so it should be removed
as well.

Signed-off-by: Dan Carpenter 


Acked-by: Christian König 


---
v2: in v1 I just added a tab

diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
index d911590d08bc..4c4bd72d4e40 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c
@@ -725,10 +725,8 @@ static void destruct(struct dcn10_resource_pool *pool)
}
}
  
-	for (i = 0; i < pool->base.stream_enc_count; i++) {

-   if (pool->base.stream_enc[i] != NULL)
-   kfree(DCE110STRENC_FROM_STRENC(pool->base.stream_enc[i]));
-   }
+   for (i = 0; i < pool->base.stream_enc_count; i++)
+   kfree(pool->base.stream_enc[i]);
  
  	for (i = 0; i < pool->base.audio_count; i++) {

if (pool->base.audios[i])



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

[bug report] drm/amd/dc: Add dc display driver (v2)

[Dan Carpenter]

Hello Harry Wentland,

The patch 4562236b3bc0: "drm/amd/dc: Add dc display driver (v2)" from
Sep 12, 2017, leads to the following static checker warning:

drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:117 
get_reg_field_value_ex()
warn: mask and shift to zero

drivers/gpu/drm/amd/display/dc/dce/dce_link_encoder.c
  1357  void dce110_link_encoder_enable_hpd(struct link_encoder *enc)
  1358  {
  1359  struct dce110_link_encoder *enc110 = TO_DCE110_LINK_ENC(enc);
  1360  struct dc_context *ctx = enc110->base.ctx;
  1361  uint32_t addr = HPD_REG(DC_HPD_CONTROL);
  1362  uint32_t hpd_enable = 0;
 ^^
This is always just zero so the static checker complains:

  1363  uint32_t value = dm_read_reg(ctx, addr);
  1364  
  1365  get_reg_field_value(hpd_enable, DC_HPD_CONTROL, DC_HPD_EN);
^^
When we pass zero to here, we know that the answer is always going to
be zero.  But then we ignore the result from get_reg_field_value()
and I'm going to change Smatch so that that also generates a static
checker warning.  What's going on???

  1366  
  1367  if (hpd_enable == 0)
^^^
This is true always.

  1368  set_reg_field_value(value, 1, DC_HPD_CONTROL, 
DC_HPD_EN);

And we don't use the return for this either?  ARgh!!  This whole
function is a no-op?  My Brian is melting???

  1369  }

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

release ex mode after hw_init

[Pixel Ding]

Hi Felix,

Please review.

[PATCH 1/2] drm/amdkfd: initialise kfd inside amdgpu_device_init
As you suggested, move kfd init/fini inside amdgpu_device_init.
Other changes for KFD interfaces are dropped.

[PATCH 2/2] drm/amdgpu: release exclusive mode after hw_init

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

[PATCH 2/2] drm/amdgpu: release exclusive mode after hw_init

[Pixel Ding]

From: pding 

Signed-off-by: pding 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 3 ---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7161af2..6f01dda 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1790,6 +1790,10 @@ static int amdgpu_init(struct amdgpu_device *adev)
}
 
amdgpu_amdkfd_device_init(adev);
+
+   if (amdgpu_sriov_vf(adev))
+   amdgpu_virt_release_full_gpu(adev, true);
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index c5d2419..1d56b5b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -177,9 +177,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned 
long flags)
pm_runtime_put_autosuspend(dev->dev);
}
 
-   if (amdgpu_sriov_vf(adev))
-   amdgpu_virt_release_full_gpu(adev, true);
-
 out:
if (r) {
/* balance pm_runtime_get_sync in amdgpu_driver_unload_kms */
-- 
2.9.5

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

[PATCH 1/2] drm/amdkfd: initialise kfd inside amdgpu_device_init

[Pixel Ding]

From: pding 

Also finalize kfd inside amdgpu_device_fini. kfd device_init needs
SRIOV exclusive accessing. Try to gather exclusive accessing to
reduce time consuming.

Signed-off-by: pding 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c| 5 -
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 7b7439f..7161af2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1693,6 +1693,8 @@ static int amdgpu_early_init(struct amdgpu_device *adev)
if (r)
return r;
 
+   amdgpu_amdkfd_device_probe(adev);
+
if (amdgpu_sriov_vf(adev)) {
r = amdgpu_virt_request_full_gpu(adev, true);
if (r)
@@ -1787,6 +1789,7 @@ static int amdgpu_init(struct amdgpu_device *adev)
adev->ip_blocks[i].status.hw = true;
}
 
+   amdgpu_amdkfd_device_init(adev);
return 0;
 }
 
@@ -1854,6 +1857,7 @@ static int amdgpu_fini(struct amdgpu_device *adev)
 {
int i, r;
 
+   amdgpu_amdkfd_device_fini(adev);
/* need to disable SMC first */
for (i = 0; i < adev->num_ip_blocks; i++) {
if (!adev->ip_blocks[i].status.hw)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 3e9760d..c5d2419 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -63,8 +63,6 @@ void amdgpu_driver_unload_kms(struct drm_device *dev)
pm_runtime_forbid(dev->dev);
}
 
-   amdgpu_amdkfd_device_fini(adev);
-
amdgpu_acpi_fini(adev);
 
amdgpu_device_fini(adev);
@@ -170,9 +168,6 @@ int amdgpu_driver_load_kms(struct drm_device *dev, unsigned 
long flags)
"Error during ACPI methods call\n");
}
 
-   amdgpu_amdkfd_device_probe(adev);
-   amdgpu_amdkfd_device_init(adev);
-
if (amdgpu_device_is_px(dev)) {
pm_runtime_use_autosuspend(dev->dev);
pm_runtime_set_autosuspend_delay(dev->dev, 5000);
-- 
2.9.5

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