Re: [PATCH] drm/ttm: replace dma_resv object on deleted BOs v3

[Pan, Xinhui]

> 2020年2月12日 19:53，Christian König  写道：
> 
> Am 12.02.20 um 07:23 schrieb Pan, Xinhui:
>> 
>>> 2020年2月11日 23:43，Christian König  写道：
>>> 
>>> When non-imported BOs are resurrected for delayed delete we replace
>>> the dma_resv object to allow for easy reclaiming of the resources.
>>> 
>>> v2: move that to ttm_bo_individualize_resv
>>> v3: add a comment to explain what's going on
>>> 
>>> Signed-off-by: Christian König 
>>> Reviewed-by: xinhui pan 
>>> ---
>>> drivers/gpu/drm/ttm/ttm_bo.c | 14 +-
>>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>>> index bfc42a9e4fb4..8174603d390f 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>> @@ -393,6 +393,18 @@ static int ttm_bo_individualize_resv(struct 
>>> ttm_buffer_object *bo)
>>> 
>>> r = dma_resv_copy_fences(&bo->base._resv, bo->base.resv);
>>> dma_resv_unlock(&bo->base._resv);
>>> +   if (r)
>>> +   return r;
>>> +
>>> +   if (bo->type != ttm_bo_type_sg) {
>>> +   /* This works because the BO is about to be destroyed and nobody
>>> +* reference it any more. The only tricky case is the trylock on
>>> +* the resv object while holding the lru_lock.
>>> +*/
>>> +   spin_lock(&ttm_bo_glob.lru_lock);
>>> +   bo->base.resv = &bo->base._resv;
>>> +   spin_unlock(&ttm_bo_glob.lru_lock);
>>> +   }
>>> 
>> how about something like that.
>> the basic idea is to do the bo cleanup work in bo release first and avoid 
>> any race with evict.
>> As in bo dieing progress, evict also just do bo cleanup work.
>> 
>> If bo is busy, neither bo_release nor evict  can do cleanupwork  on it. For 
>> the bo release case, we just add bo back to lru list.
>> So we can clean it up  both in workqueue and shrinker as the past way  did.
>> 
>> @@ -405,8 +405,9 @@ static int ttm_bo_individualize_resv(struct 
>> ttm_buffer_object *bo)
>>   if (bo->type != ttm_bo_type_sg) {
>> spin_lock(&ttm_bo_glob.lru_lock);
>> -   bo->base.resv = &bo->base._resv;
>> +   ttm_bo_del_from_lru(bo);
>> spin_unlock(&ttm_bo_glob.lru_lock);
>> +   bo->base.resv = &bo->base._resv;
>> }
>>   return r;
>> @@ -606,10 +607,9 @@ static void ttm_bo_release(struct kref *kref)
>>  * shrinkers, now that they are queued for
>>  * destruction.
>>  */
>> -   if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) {
>> +   if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT)
>> bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT;
>> -   ttm_bo_move_to_lru_tail(bo, NULL);
>> -   }
>> +   ttm_bo_add_mem_to_lru(bo, &bo->mem);
>>   kref_init(&bo->kref);
>> list_add_tail(&bo->ddestroy, &bdev->ddestroy);
> 
> Yeah, thought about that as well. But this has the major drawback that the 
> deleted BO moves to the end of the LRU, which is something we don't want.

well, as the bo is busy, looks like it needs time to being idle. putting it to 
tail seems fair.

> I think the real solution to this problem is to go a completely different way 
> and remove the delayed delete feature from TTM altogether. Instead this 
> should be part of some DRM domain handler component.
> 

yes, completely agree. As long as we can shrink bos when OOM, the workqueue is 
not necessary, The workqueue does not  help in a heavy workload case.

Pls see my patches below, I remove the workqueue, and what’s more, we can 
clearup the bo without lru lock hold.
That would reduce the lock contention. I run kfdtest and got a good performance 
result.


> In other words it should not matter if a BO is evicted, moved or freed. 
> Whenever a piece of memory becomes available again we keep around a fence 
> which marks the end of using this piece of memory.
> 
> When then somebody asks for new memory we work through the LRU and test if 
> using a certain piece of memory makes sense or not. If we find that a BO 
> needs to be evicted for this we return a reference to the BO in question to 
> the upper level handling.
> 
> If we find that we can do the allocation but only with recently freed up 
> memory we gather the fences and say you can only use the newly allocated 
> memory after waiting for those.
> 
> HEY! Wait a second! Did I just outlined what a potential replacement to TTM 
> would look like?

yes, that is a good picture. Looks like we could do more work hen. :)

thanks
xinhui


--git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index e795d5b5f8af..ac826a09b4ec 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -405,6 +405,9 @@ static int ttm_bo_individualize_resv(struct 
ttm_buffer_object *bo)
 
if (bo->type != ttm_bo_type_sg) {
spin_lock(&ttm_bo_glob.lru_lock);
+   /* it is very likely to release bo successfully.
+* if not, just adding it back.
+*/

[PATCH] drm/amdgpu: Move to a per-IB secure flag (TMZ)

[Luben Tuikov]

Move from a per-CS secure flag (TMZ) to a per-IB
secure flag.

Signed-off-by: Luben Tuikov 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   |  2 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c   | 23 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h  |  3 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  9 -
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c   | 23 +++
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c|  3 +--
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c|  3 +--
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c|  3 +--
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c| 20 ++--
 include/uapi/drm/amdgpu_drm.h|  7 ---
 10 files changed, 44 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 80ba6dfc54e2..f9fa6e104fef 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -231,8 +231,6 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser 
*p, union drm_amdgpu_cs
if (ret)
goto free_all_kdata;
 
-   p->job->secure = cs->in.flags & AMDGPU_CS_FLAGS_SECURE;
-
if (p->ctx->vram_lost_counter != p->job->vram_lost_counter) {
ret = -ECANCELED;
goto free_all_kdata;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 6e0f97afb030..cae81914c821 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -132,6 +132,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
num_ibs,
uint64_t fence_ctx;
uint32_t status = 0, alloc_size;
unsigned fence_flags = 0;
+   bool secure;
 
unsigned i;
int r = 0;
@@ -213,9 +214,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
num_ibs,
if (job && ring->funcs->emit_cntxcntl) {
status |= job->preamble_status;
status |= job->preemption_status;
-   amdgpu_ring_emit_cntxcntl(ring, status, job->secure);
+   amdgpu_ring_emit_cntxcntl(ring, status);
}
 
+   secure = false;
for (i = 0; i < num_ibs; ++i) {
ib = &ibs[i];
 
@@ -227,12 +229,27 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
num_ibs,
!amdgpu_sriov_vf(adev)) /* for SRIOV preemption, Preamble 
CE ib must be inserted anyway */
continue;
 
+   /* If this IB is TMZ, add frame TMZ start packet,
+* else, turn off TMZ.
+*/
+   if (ib->flags & AMDGPU_IB_FLAGS_SECURE && 
ring->funcs->emit_tmz) {
+   if (!secure) {
+   secure = true;
+   amdgpu_ring_emit_tmz(ring, true);
+   }
+   } else if (secure) {
+   secure = false;
+   amdgpu_ring_emit_tmz(ring, false);
+   }
+
amdgpu_ring_emit_ib(ring, job, ib, status);
status &= ~AMDGPU_HAVE_CTX_SWITCH;
}
 
-   if (ring->funcs->emit_tmz)
-   amdgpu_ring_emit_tmz(ring, false, job ? job->secure : false);
+   if (secure) {
+   secure = false;
+   amdgpu_ring_emit_tmz(ring, false);
+   }
 
 #ifdef CONFIG_X86_64
if (!(adev->flags & AMD_IS_APU))
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 31cb0203..2e2110dddb76 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -61,9 +61,6 @@ struct amdgpu_job {
/* user fence handling */
uint64_tuf_addr;
uint64_tuf_sequence;
-
-   /* the job is due to a secure command submission */
-   boolsecure;
 };
 
 int amdgpu_job_alloc(struct amdgpu_device *adev, unsigned num_ibs,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 5134d0dd6dc2..930316e60155 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -158,8 +158,7 @@ struct amdgpu_ring_funcs {
void (*begin_use)(struct amdgpu_ring *ring);
void (*end_use)(struct amdgpu_ring *ring);
void (*emit_switch_buffer) (struct amdgpu_ring *ring);
-   void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags,
-  bool trusted);
+   void (*emit_cntxcntl) (struct amdgpu_ring *ring, uint32_t flags);
void (*emit_rreg)(struct amdgpu_ring *ring, uint32_t reg);
void (*emit_wreg)(struct amdgpu_ring *ring, uint32_t reg, uint32_t val);
void (*emit_reg_wait)(struct amdgpu_ring *ring, uint32_t reg,
@@ -167,7 +166,7 @@ struct amdgpu_ring_funcs {
void (*emit_reg_write_reg_wait)(struct amdgpu_ring *ring,
   

[pull] amdgpu 5.6 fixes

[Alex Deucher]

Hi Dave, Daniel,

Fixes for 5.6.

The following changes since commit bb6d3fb354c5ee8d6bde2d576eb7220ea09862b9:

  Linux 5.6-rc1 (2020-02-09 16:08:48 -0800)

are available in the Git repository at:

  git://people.freedesktop.org/~agd5f/linux tags/amd-drm-fixes-5.6-2020-02-12

for you to fetch changes up to e33a8cfda5198fc09554fdd77ba246de42c886bd:

  drm/amdgpu:/navi10: use the ODCAP enum to index the caps array (2020-02-11 
15:42:33 -0500)


amd-drm-fixes-5.6-2020-02-12:

amdgpu:
- Additional OD fixes for navi
- Misc display fixes
- VCN 2.5 DPG fix
- Prevent build errors on PowerPC on some configs
- GDS EDC fix


Alex Deucher (2):
  drm/amdgpu: update smu_v11_0_pptable.h
  drm/amdgpu:/navi10: use the ODCAP enum to index the caps array

Aric Cyr (1):
  drm/amd/display: Check engine is not NULL before acquiring

Daniel Kolesa (1):
  amdgpu: Prevent build errors regarding soft/hard-float FP ABI tags

Guchun Chen (2):
  drm/amdgpu: limit GDS clearing workaround in cold boot sequence
  drm/amdgpu: correct comment to clear up the confusion

Isabel Zhang (1):
  drm/amd/display: Add initialitions for PLL2 clock source

James Zhu (2):
  drm/amdgpu/vcn2.5: fix DPG mode power off issue on instance 1
  drm/amdgpu/vcn2.5: fix warning

Jonathan Kim (1):
  drm/amdgpu: fix amdgpu pmu to use hwc->config instead of hwc->conf

Nicholas Kazlauskas (1):
  drm/amd/display: Don't map ATOM_ENABLE to ATOM_INIT

Roman Li (1):
  drm/amd/display: Fix psr static frames calculation

Sung Lee (3):
  drm/amd/display: Do not set optimized_require to false after plane disable
  drm/amd/display: Use dcfclk to populate watermark ranges
  drm/amd/display: DCN2.x Do not program DPPCLK if same value

Yongqiang Sun (1):
  drm/amd/display: Limit minimum DPPCLK to 100MHz.

 drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c| 15 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h|  3 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c  | 14 +--
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c  | 14 ---
 drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c  |  6 +--
 drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c  |  8 ++--
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  5 ++-
 .../gpu/drm/amd/display/dc/bios/command_table2.c   |  4 --
 drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile|  6 +++
 .../amd/display/dc/clk_mgr/dcn20/dcn20_clk_mgr.c   |  2 +-
 .../drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c  | 20 ++
 drivers/gpu/drm/amd/display/dc/dce/dce_aux.c   |  2 +-
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c |  1 -
 .../gpu/drm/amd/display/dc/dcn21/dcn21_resource.c  |  6 +++
 .../gpu/drm/amd/powerplay/inc/smu_v11_0_pptable.h  | 46 +++---
 drivers/gpu/drm/amd/powerplay/navi10_ppt.c | 22 +--
 16 files changed, 108 insertions(+), 66 deletions(-)
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/ttm: replace dma_resv object on deleted BOs v3

[Daniel Vetter]

On Wed, Feb 12, 2020 at 12:53 PM Christian König
 wrote:
>
> Am 12.02.20 um 07:23 schrieb Pan, Xinhui:
> >
> >> 2020年2月11日 23:43，Christian König  写道：
> >>
> >> When non-imported BOs are resurrected for delayed delete we replace
> >> the dma_resv object to allow for easy reclaiming of the resources.
> >>
> >> v2: move that to ttm_bo_individualize_resv
> >> v3: add a comment to explain what's going on
> >>
> >> Signed-off-by: Christian König 
> >> Reviewed-by: xinhui pan 
> >> ---
> >> drivers/gpu/drm/ttm/ttm_bo.c | 14 +-
> >> 1 file changed, 13 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> >> index bfc42a9e4fb4..8174603d390f 100644
> >> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> >> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> >> @@ -393,6 +393,18 @@ static int ttm_bo_individualize_resv(struct 
> >> ttm_buffer_object *bo)
> >>
> >>  r = dma_resv_copy_fences(&bo->base._resv, bo->base.resv);
> >>  dma_resv_unlock(&bo->base._resv);
> >> +if (r)
> >> +return r;
> >> +
> >> +if (bo->type != ttm_bo_type_sg) {
> >> +/* This works because the BO is about to be destroyed and 
> >> nobody
> >> + * reference it any more. The only tricky case is the trylock 
> >> on
> >> + * the resv object while holding the lru_lock.
> >> + */
> >> +spin_lock(&ttm_bo_glob.lru_lock);
> >> +bo->base.resv = &bo->base._resv;
> >> +spin_unlock(&ttm_bo_glob.lru_lock);
> >> +}
> >>
> > how about something like that.
> > the basic idea is to do the bo cleanup work in bo release first and avoid 
> > any race with evict.
> > As in bo dieing progress, evict also just do bo cleanup work.
> >
> > If bo is busy, neither bo_release nor evict  can do cleanupwork  on it. For 
> > the bo release case, we just add bo back to lru list.
> > So we can clean it up  both in workqueue and shrinker as the past way  did.
> >
> > @@ -405,8 +405,9 @@ static int ttm_bo_individualize_resv(struct 
> > ttm_buffer_object *bo)
> >
> >  if (bo->type != ttm_bo_type_sg) {
> >  spin_lock(&ttm_bo_glob.lru_lock);
> > -   bo->base.resv = &bo->base._resv;
> > +   ttm_bo_del_from_lru(bo);
> >  spin_unlock(&ttm_bo_glob.lru_lock);
> > +   bo->base.resv = &bo->base._resv;
> >  }
> >
> >  return r;
> > @@ -606,10 +607,9 @@ static void ttm_bo_release(struct kref *kref)
> >   * shrinkers, now that they are queued for
> >   * destruction.
> >   */
> > -   if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) {
> > +   if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT)
> >  bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT;
> > -   ttm_bo_move_to_lru_tail(bo, NULL);
> > -   }
> > +   ttm_bo_add_mem_to_lru(bo, &bo->mem);
> >
> >  kref_init(&bo->kref);
> >  list_add_tail(&bo->ddestroy, &bdev->ddestroy);
>
> Yeah, thought about that as well. But this has the major drawback that
> the deleted BO moves to the end of the LRU, which is something we don't
> want.
>
> I think the real solution to this problem is to go a completely
> different way and remove the delayed delete feature from TTM altogether.
> Instead this should be part of some DRM domain handler component.
>
> In other words it should not matter if a BO is evicted, moved or freed.
> Whenever a piece of memory becomes available again we keep around a
> fence which marks the end of using this piece of memory.
>
> When then somebody asks for new memory we work through the LRU and test
> if using a certain piece of memory makes sense or not. If we find that a
> BO needs to be evicted for this we return a reference to the BO in
> question to the upper level handling.
>
> If we find that we can do the allocation but only with recently freed up
> memory we gather the fences and say you can only use the newly allocated
> memory after waiting for those.
>
> HEY! Wait a second! Did I just outlined what a potential replacement to
> TTM would look like?

Hm I thought that was (roughly) the idea behind the last_move fence in
the ttm allocation manager? Except it's gloabl, so you'd want to have
it slightly more fine-grained. But if your vram can essentially be
managed as pages, because (almost) no need for contig allocations,
then tracking all that gets a bit nasty ...

Anyway, I think the ghost object trickery is indeed not great, since
it mixes up fences for one thing (objects) with fences for allocated
ranges in the underlying resource. Another mismatch I'm seeing is that
ttm doesn't even track (or bothers) vma ranges. So you have that
additional fun with "the memory is gone, but the ptes are kinda still
there". I guess you could also solve that by tracking such usage at
the allocation manager level, with some fences that track the async
pte update job. I guess that's your plan, I'm not sure that fits with
all the ideas we have with fancy new submission models (stuff l

Re: [PATCH] drm/amdgpu: return -EFAULT if copy_to_user() fails

[Alex Deucher]

On Wed, Feb 12, 2020 at 7:12 AM Christian König
 wrote:
>
> Am 12.02.20 um 13:07 schrieb Dan Carpenter:
> > The copy_to_user() function returns the number of bytes remaining to be
> > copied, but we want to return a negative error code to the user.
> >
> > Fixes: 030d5b97a54b ("drm/amdgpu: use amdgpu_device_vram_access in 
> > amdgpu_ttm_vram_read")
> > Signed-off-by: Dan Carpenter 
>
> Reviewed-by: Christian König 
>
> Alex do you want to pick that up or should I do this?
>

Applied.  thanks!

Alex

> Thanks,
> Christian.
>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 ++
> >   1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 15f5451d312d..660867cf2597 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -2280,7 +2280,6 @@ static ssize_t amdgpu_ttm_vram_read(struct file *f, 
> > char __user *buf,
> >   {
> >   struct amdgpu_device *adev = file_inode(f)->i_private;
> >   ssize_t result = 0;
> > - int r;
> >
> >   if (size & 0x3 || *pos & 0x3)
> >   return -EINVAL;
> > @@ -2294,9 +2293,8 @@ static ssize_t amdgpu_ttm_vram_read(struct file *f, 
> > char __user *buf,
> >   uint32_t value[AMDGPU_TTM_VRAM_MAX_DW_READ];
> >
> >   amdgpu_device_vram_access(adev, *pos, value, bytes, false);
> > - r = copy_to_user(buf, value, bytes);
> > - if (r)
> > - return r;
> > + if (copy_to_user(buf, value, bytes))
> > + return -EFAULT;
> >
> >   result += bytes;
> >   buf += bytes;
>
> ___
> 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 2/3] drm/amdgpu/display: extend DCN guards

[Rodrigo Siqueira]

Tested-by: Rodrigo Siqueira 
On 02/11, Alex Deucher wrote:
> to cover dcn2.x related headers.
> 
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/display/dc/core/dc_resource.c  | 4 ++--
>  drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c   | 2 +-
>  drivers/gpu/drm/amd/display/dc/gpio/hw_translate.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index a65a1e7820d6..c02e5994d32b 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -46,12 +46,12 @@
>  #include "dce100/dce100_resource.h"
>  #include "dce110/dce110_resource.h"
>  #include "dce112/dce112_resource.h"
> +#include "dce120/dce120_resource.h"
>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>  #include "dcn10/dcn10_resource.h"
> -#endif
>  #include "dcn20/dcn20_resource.h"
>  #include "dcn21/dcn21_resource.h"
> -#include "dce120/dce120_resource.h"
> +#endif
>  
>  #define DC_LOGGER_INIT(logger)
>  
> diff --git a/drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c 
> b/drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c
> index d2d36d48caaa..f252af1947c3 100644
> --- a/drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c
> +++ b/drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c
> @@ -47,9 +47,9 @@
>  #include "dce120/hw_factory_dce120.h"
>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>  #include "dcn10/hw_factory_dcn10.h"
> -#endif
>  #include "dcn20/hw_factory_dcn20.h"
>  #include "dcn21/hw_factory_dcn21.h"
> +#endif
>  
>  #include "diagnostics/hw_factory_diag.h"
>  
> diff --git a/drivers/gpu/drm/amd/display/dc/gpio/hw_translate.c 
> b/drivers/gpu/drm/amd/display/dc/gpio/hw_translate.c
> index 5d396657a1ee..04e2c0f74cb0 100644
> --- a/drivers/gpu/drm/amd/display/dc/gpio/hw_translate.c
> +++ b/drivers/gpu/drm/amd/display/dc/gpio/hw_translate.c
> @@ -45,9 +45,9 @@
>  #include "dce120/hw_translate_dce120.h"
>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>  #include "dcn10/hw_translate_dcn10.h"
> -#endif
>  #include "dcn20/hw_translate_dcn20.h"
>  #include "dcn21/hw_translate_dcn21.h"
> +#endif
>  
>  #include "diagnostics/hw_translate_diag.h"
>  
> -- 
> 2.24.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7CRodrigo.Siqueira%40amd.com%7C912d22aa09094b1fb87a08d7af74b20a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637170788079229843&sdata=45YxJwNoUm5ViV3ouldnO2eCqeD4C3S%2FLyfLY4VRvs0%3D&reserved=0

-- 
Rodrigo Siqueira
Software Engineer, Advanced Micro Devices (AMD)
https://siqueira.tech


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

Re: [PATCH 1/3] drm/amdgpu/display: extend DCN guard in dal_bios_parser_init_cmd_tbl_helper2

[Rodrigo Siqueira]

Tested-by: Rodrigo Siqueira 

On 02/11, Alex Deucher wrote:
> To cover DCN 2.x.
> 
> Signed-off-by: Alex Deucher 
> ---
>  .../drm/amd/display/dc/bios/command_table_helper2.c | 13 +++--
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.c 
> b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.c
> index 7388c987c595..204d7942a6e5 100644
> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.c
> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.c
> @@ -53,25 +53,18 @@ bool dal_bios_parser_init_cmd_tbl_helper2(
>  
>   case DCE_VERSION_11_2:
>   case DCE_VERSION_11_22:
> + case DCE_VERSION_12_0:
> + case DCE_VERSION_12_1:
>   *h = dal_cmd_tbl_helper_dce112_get_table2();
>   return true;
>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>   case DCN_VERSION_1_0:
>   case DCN_VERSION_1_01:
> - *h = dal_cmd_tbl_helper_dce112_get_table2();
> - return true;
> -#endif
> -
>   case DCN_VERSION_2_0:
> - *h = dal_cmd_tbl_helper_dce112_get_table2();
> - return true;
>   case DCN_VERSION_2_1:
>   *h = dal_cmd_tbl_helper_dce112_get_table2();
>   return true;
> - case DCE_VERSION_12_0:
> - case DCE_VERSION_12_1:
> - *h = dal_cmd_tbl_helper_dce112_get_table2();
> - return true;
> +#endif
>  
>   default:
>   /* Unsupported DCE */
> -- 
> 2.24.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7CRodrigo.Siqueira%40amd.com%7Cab3f299a5c75f05c08d7af74afbd%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637170788041384449&sdata=jsO5pK4dLty0gy%2BNBQxRzJrR%2B9htMcinVWz7oR0fjS8%3D&reserved=0

-- 
Rodrigo Siqueira
Software Engineer, Advanced Micro Devices (AMD)
https://siqueira.tech


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

Re: [PATCH 3/3] drm/amdgpu/display move get_num_odm_splits() into dc_resource.c

[Rodrigo Siqueira]

Tested-by: Rodrigo Siqueira 
On 02/11, Alex Deucher wrote:
> It's used by more than just DCN2.0.  Fixes missing symbol when
> amdgpu is built without DCN support.
> 
> Signed-off-by: Alex Deucher 
> ---
>  .../gpu/drm/amd/display/dc/core/dc_resource.c| 16 
>  .../drm/amd/display/dc/dcn20/dcn20_resource.c| 16 
>  .../drm/amd/display/dc/dcn20/dcn20_resource.h|  1 -
>  drivers/gpu/drm/amd/display/dc/inc/resource.h|  3 +++
>  4 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index c02e5994d32b..572ce3842535 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -532,6 +532,22 @@ static inline void get_vp_scan_direction(
>   *flip_horz_scan_dir = !*flip_horz_scan_dir;
>  }
>  
> +int get_num_odm_splits(struct pipe_ctx *pipe)
> +{
> + int odm_split_count = 0;
> + struct pipe_ctx *next_pipe = pipe->next_odm_pipe;
> + while (next_pipe) {
> + odm_split_count++;
> + next_pipe = next_pipe->next_odm_pipe;
> + }
> + pipe = pipe->prev_odm_pipe;
> + while (pipe) {
> + odm_split_count++;
> + pipe = pipe->prev_odm_pipe;
> + }
> + return odm_split_count;
> +}
> +
>  static void calculate_split_count_and_index(struct pipe_ctx *pipe_ctx, int 
> *split_count, int *split_idx)
>  {
>   *split_count = get_num_odm_splits(pipe_ctx);
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> index 39026df56fa6..1061faccec9c 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> @@ -1861,22 +1861,6 @@ void dcn20_populate_dml_writeback_from_context(
>  
>  }
>  
> -int get_num_odm_splits(struct pipe_ctx *pipe)
> -{
> - int odm_split_count = 0;
> - struct pipe_ctx *next_pipe = pipe->next_odm_pipe;
> - while (next_pipe) {
> - odm_split_count++;
> - next_pipe = next_pipe->next_odm_pipe;
> - }
> - pipe = pipe->prev_odm_pipe;
> - while (pipe) {
> - odm_split_count++;
> - pipe = pipe->prev_odm_pipe;
> - }
> - return odm_split_count;
> -}
> -
>  int dcn20_populate_dml_pipes_from_context(
>   struct dc *dc, struct dc_state *context, 
> display_e2e_pipe_params_st *pipes)
>  {
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h 
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> index 5180088ab6bc..f5893840b79b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> @@ -49,7 +49,6 @@ unsigned int dcn20_calc_max_scaled_time(
>   unsigned int time_per_pixel,
>   enum mmhubbub_wbif_mode mode,
>   unsigned int urgent_watermark);
> -int get_num_odm_splits(struct pipe_ctx *pipe);
>  int dcn20_populate_dml_pipes_from_context(
>   struct dc *dc, struct dc_state *context, 
> display_e2e_pipe_params_st *pipes);
>  struct pipe_ctx *dcn20_acquire_idle_pipe_for_layer(
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/resource.h 
> b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> index 5ae8ada154ef..ca4c36c0c9bc 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> @@ -179,4 +179,7 @@ unsigned int resource_pixel_format_to_bpp(enum 
> surface_pixel_format format);
>  
>  void get_audio_check(struct audio_info *aud_modes,
>   struct audio_check *aud_chk);
> +
> +int get_num_odm_splits(struct pipe_ctx *pipe);
> +
>  #endif /* DRIVERS_GPU_DRM_AMD_DC_DEV_DC_INC_RESOURCE_H_ */
> -- 
> 2.24.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7CRodrigo.Siqueira%40amd.com%7Ce2697beeff3847ea057408d7af74b286%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637170788143697503&sdata=XUELq7%2FDPfX%2FifA635o5DUd09JSwzPIXZkYjB107Jk8%3D&reserved=0

-- 
Rodrigo Siqueira
Software Engineer, Advanced Micro Devices (AMD)
https://siqueira.tech


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

Re: [PATCH v4] drm/scheduler: Avoid accessing freed bad job.

[Andrey Grodzovsky]

On 2/11/20 7:53 PM, Luben Tuikov wrote:

On 2020-02-11 4:27 p.m., Andrey Grodzovsky wrote:

On 2/11/20 10:55 AM, Andrey Grodzovsky wrote:

On 2/10/20 4:50 PM, Luben Tuikov wrote:

Hi Lucas,

Thank you for bringing awareness of this issue, publicly.

As soon as this patch showed up back in November of 2019,
I objected to it, privately.


I didn't find this objection in my mail actually

Yes, I didn't send it to you.


I suggested to instead use a _list_ to store the "state" of
all jobs of the same state. Then, at any time, timeout interrupt
or whatever, we can atomically (irq spinlock) move the timeout/bad
job to the timedout/cleanup/bad job list, and wake someone up
to deal with that list asynchronously, and return from the
interrupt/etc.
immediately.


Sounds a good idea to me, i think enough for us to have 2 lists,
timeout list for jobs scheduled to HW and not yet completed
(completion fence signaled) and cleanup list for those that did
complete. This should give alternative solution to the race condition
this patch was addressing without causing the break the Lucas
reported. If no one objects I think i can try implement it.

Andrey


Thinking more i realize Luben is right about having also bad job list as
this is needed for normal job competition (by fence callback from
amdgpu_fence_process)  and you need to decide if you move it to cleanup
list from timeout list or not. If it's already in bad job list - meaning
that it's being processed by GPU recovery code you don't touch it,
otherwise you move it to cleanup list where it will be freed eventually
by invocation of drm_sched_get_cleanup_job.

Yep...

Perhaps fewer lists, than "timeout", "bad" and "cleanup" could be had.
I'd also name the "bad" list as "recovery" list, as that is what would
be done to commands on that list.

"Timeout" is a status "timed-out", so perhaps just set the timeout
flag and move it to a "done" list. (Note that the command can still
complete asynchronously while on that list and while it has status
"timed-out'.)

The idea is that,
1) it avoid contention and races when more than one context
can update the job at the same time, and
2) easy to process all jobs of a certain state and/or
move them around, etc.

Let's discuss it and come up with a plan. :-)

Regards,
Luben



Sure, let me maybe come up with a draft patch so we have more concrete 
stuff to discuss and review.


Andrey









Andrey





Then in due time, if any more interrupts or whatnot take place,
the job will either be in the timeout list or not. If it it,
then the instigator backs off as someone else (the list handler) will/is
awake and handling it (obviously a state variable may be kept as well).

This draws somewhat from my days with iSCSI, SCSI and SAS, 15 years ago,
where a device can complete a job (task) at anytime regardless
of what the SCSI layer "thinks" the task's state is: timed-out, aborted,
whatever. It is a very simple and elegant solution which generalizes
well.

Regards,
Luben

On 2020-02-10 11:55 a.m., Andrey Grodzovsky wrote:

Lucas - Ping on my question and also I attached this temporary
solution for etnaviv to clarify my point. If that something
acceptable for now at least i can do the same for v3d where it
requires a bit more code changes.

Andrey

On 2/6/20 10:49 AM, Andrey Grodzovsky wrote:

Well a revert would break our driver.

The real solution is that somebody needs to sit down, gather ALL
the requirements and then come up with a solution which is clean
and works for everyone.

Christian.

I can to take on this as indeed our general design on this becomes
more and more entangled as GPU reset scenarios grow in complexity
(at least in AMD driver). Currently I am on a high priority
internal task which should take me around a week or 2 to finish and
after that I can get to it.

Regarding temporary solution  - I looked into v3d and etnaviv use
cases and we in AMD actually face the same scenario where we decide
to skip HW reset if the guilty job did finish by the time we are
processing the timeout  (see amdgpu_device_gpu_recover and
skip_hw_reset goto) - the difference is we always call
drm_sched_stop/start irrespectively of whether we are going to
actually HW reset or not (same as extend timeout). I wonder if
something like this can be done also for ve3 and etnaviv ?

Andrey

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Candrey.grodzovsky%40amd.com%7Cef96617d23a54fe9b6ef08d7af0ac9db%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637170333200621550&sdata=wa7Eh3bdi%2BthYjjZF2yeTvTjNRipGPqVA%2FGQt0QL7R8%3D&reserved=0



___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Candre

Re: [PATCH 1/2] drm/amdgpu/vcn: fix race condition issue for vcn start

[Zhu, James]

Timeout issue are complicated. These patched can fix driver side issue. Acturus 
SPG timeout issue can be fixed with these patches. For other type of timeout 
issues are still under investigation.


Thanks & Best Regards!


James Zhu


From: Liu, Leo 
Sent: Wednesday, February 12, 2020 10:11 AM
To: Zhu, James ; amd-gfx@lists.freedesktop.org 

Subject: RE: [PATCH 1/2] drm/amdgpu/vcn: fix race condition issue for vcn start


With your patches, still seeing the hung with multiple processes of decode, 
encode, and transcode.

I think we need find the root cause of that and give a comprehensive fix either 
from driver side or firmware side or both.



Regards,

Leo



From: amd-gfx  On Behalf Of Zhu, James
Sent: Wednesday, February 12, 2020 9:28 AM
To: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/amdgpu/vcn: fix race condition issue for vcn start



[AMD Official Use Only - Internal Distribution Only]



ping





From: Zhu, James mailto:james@amd.com>>
Sent: Monday, February 10, 2020 1:06 PM
To: amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Zhu, James mailto:james@amd.com>>
Subject: [PATCH 1/2] drm/amdgpu/vcn: fix race condition issue for vcn start



Fix race condition issue when multiple vcn starts are called.

Signed-off-by: James Zhu mailto:james@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index f96464e..aa7663f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
 int i, r;

 INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
+   mutex_init(&adev->vcn.vcn_pg_lock);

 switch (adev->asic_type) {
 case CHIP_RAVEN:
@@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
 }

 release_firmware(adev->vcn.fw);
+   mutex_destroy(&adev->vcn.vcn_pg_lock);

 return 0;
 }
@@ -321,6 +323,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 struct amdgpu_device *adev = ring->adev;
 bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);

+   mutex_lock(&adev->vcn.vcn_pg_lock);
 if (set_clocks) {
 amdgpu_gfx_off_ctrl(adev, false);
 amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
@@ -345,6 +348,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)

 adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
 }
+   mutex_unlock(&adev->vcn.vcn_pg_lock);
 }

 void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 6fe0573..2ae110d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -200,6 +200,7 @@ struct amdgpu_vcn {
 struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
 uint32_t num_vcn_enc_sched;
 uint32_t num_vcn_dec_sched;
+   struct mutex vcn_pg_lock;

 unsignedharvest_config;
 int (*pause_dpg_mode)(struct amdgpu_device *adev,
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 3/3] drm/amdgpu/display move get_num_odm_splits() into dc_resource.c

[Liu, Zhan]

> -Original Message-
> From: Alex Deucher 
> Sent: 2020/February/12, Wednesday 11:05 AM
> To: Liu, Zhan 
> Cc: amd-gfx@lists.freedesktop.org; Deucher, Alexander
> 
> Subject: Re: [PATCH 3/3] drm/amdgpu/display move get_num_odm_splits()
> into dc_resource.c
> 
> On Wed, Feb 12, 2020 at 10:58 AM Liu, Zhan  wrote:
> >
> > Please find my reply inline.
> >
> > Thanks,
> > Zhan
> >
> > > -Original Message-
> > > From: amd-gfx  On Behalf Of
> > > Alex Deucher
> > > Sent: 2020/February/11, Tuesday 11:33 PM
> > > To: amd-gfx@lists.freedesktop.org
> > > Cc: Deucher, Alexander 
> > > Subject: [PATCH 3/3] drm/amdgpu/display move get_num_odm_splits()
> > > into dc_resource.c
> > >
> > > It's used by more than just DCN2.0.  Fixes missing symbol when
> > > amdgpu is built without DCN support.
> > >
> > > Signed-off-by: Alex Deucher 
> > > ---
> > >  .../gpu/drm/amd/display/dc/core/dc_resource.c| 16
> 
> > >  .../drm/amd/display/dc/dcn20/dcn20_resource.c| 16 
> > >  .../drm/amd/display/dc/dcn20/dcn20_resource.h|  1 -
> > >  drivers/gpu/drm/amd/display/dc/inc/resource.h|  3 +++
> > >  4 files changed, 19 insertions(+), 17 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> > > b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> > > index c02e5994d32b..572ce3842535 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> > > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> > > @@ -532,6 +532,22 @@ static inline void get_vp_scan_direction(
> > >   *flip_horz_scan_dir = !*flip_horz_scan_dir;  }
> > >
> > > +int get_num_odm_splits(struct pipe_ctx *pipe) {
> > > + int odm_split_count = 0;
> > > + struct pipe_ctx *next_pipe = pipe->next_odm_pipe;
> > > + while (next_pipe) {
> > > + odm_split_count++;
> > > + next_pipe = next_pipe->next_odm_pipe;
> > > + }
> > > + pipe = pipe->prev_odm_pipe;
> > > + while (pipe) {
> > > + odm_split_count++;
> > > + pipe = pipe->prev_odm_pipe;
> > > + }
> > > + return odm_split_count;
> > > +}
> > > +
> > >  static void calculate_split_count_and_index(struct pipe_ctx
> > > *pipe_ctx, int *split_count, int *split_idx)  {
> > >   *split_count = get_num_odm_splits(pipe_ctx); diff --git
> > > a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > > b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > > index 39026df56fa6..1061faccec9c 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > > +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > > @@ -1861,22 +1861,6 @@ void
> > > dcn20_populate_dml_writeback_from_context(
> > >
> > >  }
> > >
> > > -int get_num_odm_splits(struct pipe_ctx *pipe) -{
> > > - int odm_split_count = 0;
> > > - struct pipe_ctx *next_pipe = pipe->next_odm_pipe;
> > > - while (next_pipe) {
> > > - odm_split_count++;
> > > - next_pipe = next_pipe->next_odm_pipe;
> > > - }
> > > - pipe = pipe->prev_odm_pipe;
> > > - while (pipe) {
> > > - odm_split_count++;
> > > - pipe = pipe->prev_odm_pipe;
> > > - }
> > > - return odm_split_count;
> > > -}
> > > -
> > >  int dcn20_populate_dml_pipes_from_context(
> > >   struct dc *dc, struct dc_state *context,
> > > display_e2e_pipe_params_st *pipes)  { diff --git
> > > a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> > > b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> > > index 5180088ab6bc..f5893840b79b 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> > > +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> > > @@ -49,7 +49,6 @@ unsigned int dcn20_calc_max_scaled_time(
> > >   unsigned int time_per_pixel,
> > >   enum mmhubbub_wbif_mode mode,
> > >   unsigned int urgent_watermark); -int
> > > get_num_odm_splits(struct pipe_ctx *pipe);  int
> >
> > Seems like the "int" at the end of this line actually belongs to the next 
> > line.
> > I am wondering is it a typo or a format-patch glitch?
> 
> Actual patch is correct:
> https://cgit.freedesktop.org/~agd5f/linux/commit/?h=drm-next&id=c8d2c8eaa3bbcaf8e2bc20f3e3203ed444f90bcc

I see. Thx!

This patch (and this patch series) is reviewed by:
Zhan Liu 

> 
> >
> > > dcn20_populate_dml_pipes_from_context(
> > >   struct dc *dc, struct dc_state *context,
> > > display_e2e_pipe_params_st *pipes);  struct pipe_ctx
> > > *dcn20_acquire_idle_pipe_for_layer(
> > > diff --git a/drivers/gpu/drm/amd/display/dc/inc/resource.h
> > > b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> > > index 5ae8ada154ef..ca4c36c0c9bc 100644
> > > --- a/drivers/gpu/drm/amd/display/dc/inc/resource.h
> > > +++ b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> > > @@ -179,4 +179,7 @@ unsigned int
> resource_pixel_format_to_bpp(enum
> > > surface_pixel_format format);
> > >
>

Re: [PATCH 3/3] drm/amdgpu/display move get_num_odm_splits() into dc_resource.c

[Alex Deucher]

On Wed, Feb 12, 2020 at 10:58 AM Liu, Zhan  wrote:
>
> Please find my reply inline.
>
> Thanks,
> Zhan
>
> > -Original Message-
> > From: amd-gfx  On Behalf Of Alex
> > Deucher
> > Sent: 2020/February/11, Tuesday 11:33 PM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander 
> > Subject: [PATCH 3/3] drm/amdgpu/display move get_num_odm_splits() into
> > dc_resource.c
> >
> > It's used by more than just DCN2.0.  Fixes missing symbol when amdgpu is
> > built without DCN support.
> >
> > Signed-off-by: Alex Deucher 
> > ---
> >  .../gpu/drm/amd/display/dc/core/dc_resource.c| 16 
> >  .../drm/amd/display/dc/dcn20/dcn20_resource.c| 16 
> >  .../drm/amd/display/dc/dcn20/dcn20_resource.h|  1 -
> >  drivers/gpu/drm/amd/display/dc/inc/resource.h|  3 +++
> >  4 files changed, 19 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> > b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> > index c02e5994d32b..572ce3842535 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> > @@ -532,6 +532,22 @@ static inline void get_vp_scan_direction(
> >   *flip_horz_scan_dir = !*flip_horz_scan_dir;  }
> >
> > +int get_num_odm_splits(struct pipe_ctx *pipe) {
> > + int odm_split_count = 0;
> > + struct pipe_ctx *next_pipe = pipe->next_odm_pipe;
> > + while (next_pipe) {
> > + odm_split_count++;
> > + next_pipe = next_pipe->next_odm_pipe;
> > + }
> > + pipe = pipe->prev_odm_pipe;
> > + while (pipe) {
> > + odm_split_count++;
> > + pipe = pipe->prev_odm_pipe;
> > + }
> > + return odm_split_count;
> > +}
> > +
> >  static void calculate_split_count_and_index(struct pipe_ctx *pipe_ctx, int
> > *split_count, int *split_idx)  {
> >   *split_count = get_num_odm_splits(pipe_ctx); diff --git
> > a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > index 39026df56fa6..1061faccec9c 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> > @@ -1861,22 +1861,6 @@ void
> > dcn20_populate_dml_writeback_from_context(
> >
> >  }
> >
> > -int get_num_odm_splits(struct pipe_ctx *pipe) -{
> > - int odm_split_count = 0;
> > - struct pipe_ctx *next_pipe = pipe->next_odm_pipe;
> > - while (next_pipe) {
> > - odm_split_count++;
> > - next_pipe = next_pipe->next_odm_pipe;
> > - }
> > - pipe = pipe->prev_odm_pipe;
> > - while (pipe) {
> > - odm_split_count++;
> > - pipe = pipe->prev_odm_pipe;
> > - }
> > - return odm_split_count;
> > -}
> > -
> >  int dcn20_populate_dml_pipes_from_context(
> >   struct dc *dc, struct dc_state *context,
> > display_e2e_pipe_params_st *pipes)  { diff --git
> > a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> > b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> > index 5180088ab6bc..f5893840b79b 100644
> > --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> > +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> > @@ -49,7 +49,6 @@ unsigned int dcn20_calc_max_scaled_time(
> >   unsigned int time_per_pixel,
> >   enum mmhubbub_wbif_mode mode,
> >   unsigned int urgent_watermark);
> > -int get_num_odm_splits(struct pipe_ctx *pipe);  int
>
> Seems like the "int" at the end of this line actually belongs to the next 
> line.
> I am wondering is it a typo or a format-patch glitch?

Actual patch is correct:
https://cgit.freedesktop.org/~agd5f/linux/commit/?h=drm-next&id=c8d2c8eaa3bbcaf8e2bc20f3e3203ed444f90bcc

>
> > dcn20_populate_dml_pipes_from_context(
> >   struct dc *dc, struct dc_state *context,
> > display_e2e_pipe_params_st *pipes);  struct pipe_ctx
> > *dcn20_acquire_idle_pipe_for_layer(
> > diff --git a/drivers/gpu/drm/amd/display/dc/inc/resource.h
> > b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> > index 5ae8ada154ef..ca4c36c0c9bc 100644
> > --- a/drivers/gpu/drm/amd/display/dc/inc/resource.h
> > +++ b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> > @@ -179,4 +179,7 @@ unsigned int resource_pixel_format_to_bpp(enum
> > surface_pixel_format format);
> >
> >  void get_audio_check(struct audio_info *aud_modes,
> >   struct audio_check *aud_chk);
> > +
> > +int get_num_odm_splits(struct pipe_ctx *pipe);
> > +
> >  #endif /* DRIVERS_GPU_DRM_AMD_DC_DEV_DC_INC_RESOURCE_H_ */
> > --
> > 2.24.1
> >
> > ___
> > amd-gfx mailing list
> > amd-gfx@lists.freedesktop.org
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/powerplay: Ratelimit PP_ASSERT warnings

[Alex Deucher]

On Wed, Feb 12, 2020 at 9:39 AM Kent Russell  wrote:
>
> In certain situations the message could be reported dozens-to-hundreds of
> times, based on how often the function is called.
> E.g. If MCLK DPM, any calls to get/set MCLK will result in a failure
> message, potentially flooding dmesg. Ratelimit the warnings to avoid
> this flood.
>
> Change-Id: Ib817fd9227e9ffec8f1ed18c5441cbb135bc413b
> Signed-off-by: Kent Russell 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/powerplay/inc/pp_debug.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h 
> b/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h
> index 822cd8b5bf90..cea65093b6ad 100644
> --- a/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h
> +++ b/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h
> @@ -37,7 +37,7 @@
>  #define PP_ASSERT_WITH_CODE(cond, msg, code)   \
> do {\
> if (!(cond)) {  \
> -   pr_warn("%s\n", msg);   \
> +   pr_warn_ratelimited("%s\n", msg);   \
> code;   \
> }   \
> } while (0)
> @@ -45,7 +45,7 @@
>  #define PP_ASSERT(cond, msg)   \
> do {\
> if (!(cond)) {  \
> -   pr_warn("%s\n", msg);   \
> +   pr_warn_ratelimited("%s\n", msg);   \
> }   \
> } while (0)
>
> --
> 2.17.1
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 3/3] drm/amdgpu/display move get_num_odm_splits() into dc_resource.c

[Liu, Zhan]

Please find my reply inline.

Thanks,
Zhan

> -Original Message-
> From: amd-gfx  On Behalf Of Alex
> Deucher
> Sent: 2020/February/11, Tuesday 11:33 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander 
> Subject: [PATCH 3/3] drm/amdgpu/display move get_num_odm_splits() into
> dc_resource.c
> 
> It's used by more than just DCN2.0.  Fixes missing symbol when amdgpu is
> built without DCN support.
> 
> Signed-off-by: Alex Deucher 
> ---
>  .../gpu/drm/amd/display/dc/core/dc_resource.c| 16 
>  .../drm/amd/display/dc/dcn20/dcn20_resource.c| 16 
>  .../drm/amd/display/dc/dcn20/dcn20_resource.h|  1 -
>  drivers/gpu/drm/amd/display/dc/inc/resource.h|  3 +++
>  4 files changed, 19 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index c02e5994d32b..572ce3842535 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -532,6 +532,22 @@ static inline void get_vp_scan_direction(
>   *flip_horz_scan_dir = !*flip_horz_scan_dir;  }
> 
> +int get_num_odm_splits(struct pipe_ctx *pipe) {
> + int odm_split_count = 0;
> + struct pipe_ctx *next_pipe = pipe->next_odm_pipe;
> + while (next_pipe) {
> + odm_split_count++;
> + next_pipe = next_pipe->next_odm_pipe;
> + }
> + pipe = pipe->prev_odm_pipe;
> + while (pipe) {
> + odm_split_count++;
> + pipe = pipe->prev_odm_pipe;
> + }
> + return odm_split_count;
> +}
> +
>  static void calculate_split_count_and_index(struct pipe_ctx *pipe_ctx, int
> *split_count, int *split_idx)  {
>   *split_count = get_num_odm_splits(pipe_ctx); diff --git
> a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> index 39026df56fa6..1061faccec9c 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> @@ -1861,22 +1861,6 @@ void
> dcn20_populate_dml_writeback_from_context(
> 
>  }
> 
> -int get_num_odm_splits(struct pipe_ctx *pipe) -{
> - int odm_split_count = 0;
> - struct pipe_ctx *next_pipe = pipe->next_odm_pipe;
> - while (next_pipe) {
> - odm_split_count++;
> - next_pipe = next_pipe->next_odm_pipe;
> - }
> - pipe = pipe->prev_odm_pipe;
> - while (pipe) {
> - odm_split_count++;
> - pipe = pipe->prev_odm_pipe;
> - }
> - return odm_split_count;
> -}
> -
>  int dcn20_populate_dml_pipes_from_context(
>   struct dc *dc, struct dc_state *context,
> display_e2e_pipe_params_st *pipes)  { diff --git
> a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> index 5180088ab6bc..f5893840b79b 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.h
> @@ -49,7 +49,6 @@ unsigned int dcn20_calc_max_scaled_time(
>   unsigned int time_per_pixel,
>   enum mmhubbub_wbif_mode mode,
>   unsigned int urgent_watermark);
> -int get_num_odm_splits(struct pipe_ctx *pipe);  int

Seems like the "int" at the end of this line actually belongs to the next line.
I am wondering is it a typo or a format-patch glitch?

> dcn20_populate_dml_pipes_from_context(
>   struct dc *dc, struct dc_state *context,
> display_e2e_pipe_params_st *pipes);  struct pipe_ctx
> *dcn20_acquire_idle_pipe_for_layer(
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/resource.h
> b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> index 5ae8ada154ef..ca4c36c0c9bc 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/resource.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/resource.h
> @@ -179,4 +179,7 @@ unsigned int resource_pixel_format_to_bpp(enum
> surface_pixel_format format);
> 
>  void get_audio_check(struct audio_info *aud_modes,
>   struct audio_check *aud_chk);
> +
> +int get_num_odm_splits(struct pipe_ctx *pipe);
> +
>  #endif /* DRIVERS_GPU_DRM_AMD_DC_DEV_DC_INC_RESOURCE_H_ */
> --
> 2.24.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 2/3] drm/amdgpu/display: extend DCN guards

[Liu, Zhan]

> -Original Message-
> From: amd-gfx  On Behalf Of Alex
> Deucher
> Sent: 2020/February/11, Tuesday 11:33 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander 
> Subject: [PATCH 2/3] drm/amdgpu/display: extend DCN guards
> 
> to cover dcn2.x related headers.
> 
> Signed-off-by: Alex Deucher 

This patch is:
Reviewed-by: Zhan Liu 

> ---
>  drivers/gpu/drm/amd/display/dc/core/dc_resource.c  | 4 ++--
>  drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c   | 2 +-
>  drivers/gpu/drm/amd/display/dc/gpio/hw_translate.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index a65a1e7820d6..c02e5994d32b 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -46,12 +46,12 @@
>  #include "dce100/dce100_resource.h"
>  #include "dce110/dce110_resource.h"
>  #include "dce112/dce112_resource.h"
> +#include "dce120/dce120_resource.h"
>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>  #include "dcn10/dcn10_resource.h"
> -#endif
>  #include "dcn20/dcn20_resource.h"
>  #include "dcn21/dcn21_resource.h"
> -#include "dce120/dce120_resource.h"
> +#endif
> 
>  #define DC_LOGGER_INIT(logger)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c
> b/drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c
> index d2d36d48caaa..f252af1947c3 100644
> --- a/drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c
> +++ b/drivers/gpu/drm/amd/display/dc/gpio/hw_factory.c
> @@ -47,9 +47,9 @@
>  #include "dce120/hw_factory_dce120.h"
>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>  #include "dcn10/hw_factory_dcn10.h"
> -#endif
>  #include "dcn20/hw_factory_dcn20.h"
>  #include "dcn21/hw_factory_dcn21.h"
> +#endif
> 
>  #include "diagnostics/hw_factory_diag.h"
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/gpio/hw_translate.c
> b/drivers/gpu/drm/amd/display/dc/gpio/hw_translate.c
> index 5d396657a1ee..04e2c0f74cb0 100644
> --- a/drivers/gpu/drm/amd/display/dc/gpio/hw_translate.c
> +++ b/drivers/gpu/drm/amd/display/dc/gpio/hw_translate.c
> @@ -45,9 +45,9 @@
>  #include "dce120/hw_translate_dce120.h"
>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>  #include "dcn10/hw_translate_dcn10.h"
> -#endif
>  #include "dcn20/hw_translate_dcn20.h"
>  #include "dcn21/hw_translate_dcn21.h"
> +#endif
> 
>  #include "diagnostics/hw_translate_diag.h"
> 
> --
> 2.24.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/2] drm/amdgpu/gfx10: disable gfxoff when reading rlc clock

[Yuan, Xiaojie]

Series is Reviewed-by: Xiaojie Yuan 

BR,
Xiaojie

> On Feb 12, 2020, at 9:53 PM, Alex Deucher  wrote:
> 
> Otherwise we readback all ones.  Fixes rlc counter
> readback while gfxoff is active.
> 
> Signed-off-by: Alex Deucher 
> ---
> drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 4e25b39ac14f..0eff2e7d33fa 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -3924,11 +3924,13 @@ static uint64_t 
> gfx_v10_0_get_gpu_clock_counter(struct amdgpu_device *adev)
> {
>uint64_t clock;
> 
> +amdgpu_gfx_off_ctrl(adev, false);
>mutex_lock(&adev->gfx.gpu_clock_mutex);
>WREG32_SOC15(GC, 0, mmRLC_CAPTURE_GPU_CLOCK_COUNT, 1);
>clock = (uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_LSB) |
>((uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_MSB) << 32ULL);
>mutex_unlock(&adev->gfx.gpu_clock_mutex);
> +amdgpu_gfx_off_ctrl(adev, true);
>return clock;
> }
> 
> -- 
> 2.24.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7CXiaojie.Yuan%40amd.com%7C14c17abf264943e85faf08d7afc2f80e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637171124232441506&sdata=Xz4N%2FDc8ExXMIV9PK%2FMGG48vHsb6%2FrzvmrUzXer%2F5Eo%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

RE: [PATCH 1/3] drm/amdgpu/display: extend DCN guard in dal_bios_parser_init_cmd_tbl_helper2

[Liu, Zhan]

> -Original Message-
> From: amd-gfx  On Behalf Of Alex
> Deucher
> Sent: 2020/February/11, Tuesday 11:33 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander 
> Subject: [PATCH 1/3] drm/amdgpu/display: extend DCN guard in
> dal_bios_parser_init_cmd_tbl_helper2
> 
> To cover DCN 2.x.
> 
> Signed-off-by: Alex Deucher 

This patch is:
Reviewed-by: Zhan Liu 

> ---
>  .../drm/amd/display/dc/bios/command_table_helper2.c | 13 +++--
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.c
> b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.c
> index 7388c987c595..204d7942a6e5 100644
> --- a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.c
> +++ b/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.c
> @@ -53,25 +53,18 @@ bool dal_bios_parser_init_cmd_tbl_helper2(
> 
>   case DCE_VERSION_11_2:
>   case DCE_VERSION_11_22:
> + case DCE_VERSION_12_0:
> + case DCE_VERSION_12_1:
>   *h = dal_cmd_tbl_helper_dce112_get_table2();
>   return true;
>  #if defined(CONFIG_DRM_AMD_DC_DCN)
>   case DCN_VERSION_1_0:
>   case DCN_VERSION_1_01:
> - *h = dal_cmd_tbl_helper_dce112_get_table2();
> - return true;
> -#endif
> -
>   case DCN_VERSION_2_0:
> - *h = dal_cmd_tbl_helper_dce112_get_table2();
> - return true;
>   case DCN_VERSION_2_1:
>   *h = dal_cmd_tbl_helper_dce112_get_table2();
>   return true;
> - case DCE_VERSION_12_0:
> - case DCE_VERSION_12_1:
> - *h = dal_cmd_tbl_helper_dce112_get_table2();
> - return true;
> +#endif
> 
>   default:
>   /* Unsupported DCE */
> --
> 2.24.1
> 
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/amdgpu: stop allocating PDs/PTs with the eviction lock held

[Christian König]

We need to make sure to not allocate PDs/PTs while holding
the eviction lock or otherwise we will run into lock inversion
in the MM as soon as we enable the MMU notifier.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 77c400675b79..e7ab0c1e2793 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -897,27 +897,42 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
struct amdgpu_vm_pt *entry = cursor->entry;
struct amdgpu_bo_param bp;
struct amdgpu_bo *pt;
+   bool need_entries;
int r;
 
-   if (cursor->level < AMDGPU_VM_PTB && !entry->entries) {
+   need_entries = cursor->level < AMDGPU_VM_PTB && !entry->entries;
+   if (!need_entries && entry->base.bo)
+   return 0;
+
+   /* We need to make sure that we don't allocate PDs/PTs while holding the
+* eviction lock or we run into lock recursion in the MM.
+*/
+   amdgpu_vm_eviction_unlock(vm);
+
+   if (need_entries) {
unsigned num_entries;
 
num_entries = amdgpu_vm_num_entries(adev, cursor->level);
entry->entries = kvmalloc_array(num_entries,
sizeof(*entry->entries),
GFP_KERNEL | __GFP_ZERO);
-   if (!entry->entries)
-   return -ENOMEM;
+   if (!entry->entries) {
+   r = -ENOMEM;
+   goto error_lock;
+   }
}
 
-   if (entry->base.bo)
-   return 0;
+   if (entry->base.bo) {
+   r = 0;
+   goto error_lock;
+   }
 
amdgpu_vm_bo_param(adev, vm, cursor->level, direct, &bp);
 
r = amdgpu_bo_create(adev, &bp, &pt);
+   amdgpu_vm_eviction_lock(vm);
if (r)
-   return r;
+   goto error_free_pt;
 
/* Keep a reference to the root directory to avoid
 * freeing them up in the wrong order.
@@ -936,6 +951,10 @@ static int amdgpu_vm_alloc_pts(struct amdgpu_device *adev,
amdgpu_bo_unref(&pt);
entry->base.bo = NULL;
return r;
+
+error_lock:
+   amdgpu_vm_eviction_lock(vm);
+   return r;
 }
 
 /**
-- 
2.17.1

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

RE: [PATCH 1/2] drm/amdgpu/vcn: fix race condition issue for vcn start

[Liu, Leo]

With your patches, still seeing the hung with multiple processes of decode, 
encode, and transcode.
I think we need find the root cause of that and give a comprehensive fix either 
from driver side or firmware side or both.

Regards,
Leo

From: amd-gfx  On Behalf Of Zhu, James
Sent: Wednesday, February 12, 2020 9:28 AM
To: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/amdgpu/vcn: fix race condition issue for vcn start


[AMD Official Use Only - Internal Distribution Only]

ping


From: Zhu, James mailto:james@amd.com>>
Sent: Monday, February 10, 2020 1:06 PM
To: amd-gfx@lists.freedesktop.org 
mailto:amd-gfx@lists.freedesktop.org>>
Cc: Zhu, James mailto:james@amd.com>>
Subject: [PATCH 1/2] drm/amdgpu/vcn: fix race condition issue for vcn start

Fix race condition issue when multiple vcn starts are called.

Signed-off-by: James Zhu mailto:james@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index f96464e..aa7663f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
 int i, r;

 INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
+   mutex_init(&adev->vcn.vcn_pg_lock);

 switch (adev->asic_type) {
 case CHIP_RAVEN:
@@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
 }

 release_firmware(adev->vcn.fw);
+   mutex_destroy(&adev->vcn.vcn_pg_lock);

 return 0;
 }
@@ -321,6 +323,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 struct amdgpu_device *adev = ring->adev;
 bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);

+   mutex_lock(&adev->vcn.vcn_pg_lock);
 if (set_clocks) {
 amdgpu_gfx_off_ctrl(adev, false);
 amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
@@ -345,6 +348,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)

 adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
 }
+   mutex_unlock(&adev->vcn.vcn_pg_lock);
 }

 void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 6fe0573..2ae110d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -200,6 +200,7 @@ struct amdgpu_vcn {
 struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
 uint32_t num_vcn_enc_sched;
 uint32_t num_vcn_dec_sched;
+   struct mutex vcn_pg_lock;

 unsignedharvest_config;
 int (*pause_dpg_mode)(struct amdgpu_device *adev,
--
2.7.4
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/powerplay: Ratelimit PP_ASSERT warnings

[Kent Russell]

In certain situations the message could be reported dozens-to-hundreds of
times, based on how often the function is called.
E.g. If MCLK DPM, any calls to get/set MCLK will result in a failure
message, potentially flooding dmesg. Ratelimit the warnings to avoid
this flood.

Change-Id: Ib817fd9227e9ffec8f1ed18c5441cbb135bc413b
Signed-off-by: Kent Russell 
---
 drivers/gpu/drm/amd/powerplay/inc/pp_debug.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h 
b/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h
index 822cd8b5bf90..cea65093b6ad 100644
--- a/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h
+++ b/drivers/gpu/drm/amd/powerplay/inc/pp_debug.h
@@ -37,7 +37,7 @@
 #define PP_ASSERT_WITH_CODE(cond, msg, code)   \
do {\
if (!(cond)) {  \
-   pr_warn("%s\n", msg);   \
+   pr_warn_ratelimited("%s\n", msg);   \
code;   \
}   \
} while (0)
@@ -45,7 +45,7 @@
 #define PP_ASSERT(cond, msg)   \
do {\
if (!(cond)) {  \
-   pr_warn("%s\n", msg);   \
+   pr_warn_ratelimited("%s\n", msg);   \
}   \
} while (0)
 
-- 
2.17.1

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

Re: [PATCH 1/2] drm/amdgpu/vcn: fix race condition issue for vcn start

[Zhu, James]

[AMD Official Use Only - Internal Distribution Only]

ping


From: Zhu, James 
Sent: Monday, February 10, 2020 1:06 PM
To: amd-gfx@lists.freedesktop.org 
Cc: Zhu, James 
Subject: [PATCH 1/2] drm/amdgpu/vcn: fix race condition issue for vcn start

Fix race condition issue when multiple vcn starts are called.

Signed-off-by: James Zhu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 4 
 drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index f96464e..aa7663f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -63,6 +63,7 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
 int i, r;

 INIT_DELAYED_WORK(&adev->vcn.idle_work, amdgpu_vcn_idle_work_handler);
+   mutex_init(&adev->vcn.vcn_pg_lock);

 switch (adev->asic_type) {
 case CHIP_RAVEN:
@@ -210,6 +211,7 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev)
 }

 release_firmware(adev->vcn.fw);
+   mutex_destroy(&adev->vcn.vcn_pg_lock);

 return 0;
 }
@@ -321,6 +323,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)
 struct amdgpu_device *adev = ring->adev;
 bool set_clocks = !cancel_delayed_work_sync(&adev->vcn.idle_work);

+   mutex_lock(&adev->vcn.vcn_pg_lock);
 if (set_clocks) {
 amdgpu_gfx_off_ctrl(adev, false);
 amdgpu_device_ip_set_powergating_state(adev, 
AMD_IP_BLOCK_TYPE_VCN,
@@ -345,6 +348,7 @@ void amdgpu_vcn_ring_begin_use(struct amdgpu_ring *ring)

 adev->vcn.pause_dpg_mode(adev, ring->me, &new_state);
 }
+   mutex_unlock(&adev->vcn.vcn_pg_lock);
 }

 void amdgpu_vcn_ring_end_use(struct amdgpu_ring *ring)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
index 6fe0573..2ae110d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -200,6 +200,7 @@ struct amdgpu_vcn {
 struct drm_gpu_scheduler *vcn_dec_sched[AMDGPU_MAX_VCN_INSTANCES];
 uint32_t num_vcn_enc_sched;
 uint32_t num_vcn_dec_sched;
+   struct mutex vcn_pg_lock;

 unsignedharvest_config;
 int (*pause_dpg_mode)(struct amdgpu_device *adev,
--
2.7.4

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

[PATCH 2/2] drm/amdgpu/gfx10: disable gfxoff when reading rlc clock

[Alex Deucher]

Otherwise we readback all ones.  Fixes rlc counter
readback while gfxoff is active.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
index 4e25b39ac14f..0eff2e7d33fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
@@ -3924,11 +3924,13 @@ static uint64_t gfx_v10_0_get_gpu_clock_counter(struct 
amdgpu_device *adev)
 {
uint64_t clock;
 
+   amdgpu_gfx_off_ctrl(adev, false);
mutex_lock(&adev->gfx.gpu_clock_mutex);
WREG32_SOC15(GC, 0, mmRLC_CAPTURE_GPU_CLOCK_COUNT, 1);
clock = (uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_LSB) |
((uint64_t)RREG32_SOC15(GC, 0, mmRLC_GPU_CLOCK_COUNT_MSB) << 
32ULL);
mutex_unlock(&adev->gfx.gpu_clock_mutex);
+   amdgpu_gfx_off_ctrl(adev, true);
return clock;
 }
 
-- 
2.24.1

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

[PATCH 1/2] drm/amdgpu/gfx9: disable gfxoff when reading rlc clock

[Alex Deucher]

Otherwise we readback all ones.  Fixes rlc counter
readback while gfxoff is active.

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 82476f6acfad..108b9e5319f7 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -3959,6 +3959,7 @@ static uint64_t gfx_v9_0_get_gpu_clock_counter(struct 
amdgpu_device *adev)
 {
uint64_t clock;
 
+   amdgpu_gfx_off_ctrl(adev, false);
mutex_lock(&adev->gfx.gpu_clock_mutex);
if (adev->asic_type == CHIP_VEGA10 && amdgpu_sriov_runtime(adev)) {
uint32_t tmp, lsb, msb, i = 0;
@@ -3977,6 +3978,7 @@ static uint64_t gfx_v9_0_get_gpu_clock_counter(struct 
amdgpu_device *adev)
((uint64_t)RREG32_SOC15(GC, 0, 
mmRLC_GPU_CLOCK_COUNT_MSB) << 32ULL);
}
mutex_unlock(&adev->gfx.gpu_clock_mutex);
+   amdgpu_gfx_off_ctrl(adev, true);
return clock;
 }
 
-- 
2.24.1

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

Re: [PATCH] drm/amdgpu: return -EFAULT if copy_to_user() fails

[Christian König]

Am 12.02.20 um 13:07 schrieb Dan Carpenter:

The copy_to_user() function returns the number of bytes remaining to be
copied, but we want to return a negative error code to the user.

Fixes: 030d5b97a54b ("drm/amdgpu: use amdgpu_device_vram_access in 
amdgpu_ttm_vram_read")
Signed-off-by: Dan Carpenter 


Reviewed-by: Christian König 

Alex do you want to pick that up or should I do this?

Thanks,
Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 15f5451d312d..660867cf2597 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2280,7 +2280,6 @@ static ssize_t amdgpu_ttm_vram_read(struct file *f, char 
__user *buf,
  {
struct amdgpu_device *adev = file_inode(f)->i_private;
ssize_t result = 0;
-   int r;
  
  	if (size & 0x3 || *pos & 0x3)

return -EINVAL;
@@ -2294,9 +2293,8 @@ static ssize_t amdgpu_ttm_vram_read(struct file *f, char 
__user *buf,
uint32_t value[AMDGPU_TTM_VRAM_MAX_DW_READ];
  
  		amdgpu_device_vram_access(adev, *pos, value, bytes, false);

-   r = copy_to_user(buf, value, bytes);
-   if (r)
-   return r;
+   if (copy_to_user(buf, value, bytes))
+   return -EFAULT;
  
  		result += bytes;

buf += bytes;


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

[PATCH] drm/amdgpu: return -EFAULT if copy_to_user() fails

[Dan Carpenter]

The copy_to_user() function returns the number of bytes remaining to be
copied, but we want to return a negative error code to the user.

Fixes: 030d5b97a54b ("drm/amdgpu: use amdgpu_device_vram_access in 
amdgpu_ttm_vram_read")
Signed-off-by: Dan Carpenter 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 15f5451d312d..660867cf2597 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -2280,7 +2280,6 @@ static ssize_t amdgpu_ttm_vram_read(struct file *f, char 
__user *buf,
 {
struct amdgpu_device *adev = file_inode(f)->i_private;
ssize_t result = 0;
-   int r;
 
if (size & 0x3 || *pos & 0x3)
return -EINVAL;
@@ -2294,9 +2293,8 @@ static ssize_t amdgpu_ttm_vram_read(struct file *f, char 
__user *buf,
uint32_t value[AMDGPU_TTM_VRAM_MAX_DW_READ];
 
amdgpu_device_vram_access(adev, *pos, value, bytes, false);
-   r = copy_to_user(buf, value, bytes);
-   if (r)
-   return r;
+   if (copy_to_user(buf, value, bytes))
+   return -EFAULT;
 
result += bytes;
buf += bytes;
-- 
2.11.0

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

Re: [PATCH] drm/ttm: replace dma_resv object on deleted BOs v3

[Christian König]

Am 12.02.20 um 07:23 schrieb Pan, Xinhui:



2020年2月11日 23:43，Christian König  写道：

When non-imported BOs are resurrected for delayed delete we replace
the dma_resv object to allow for easy reclaiming of the resources.

v2: move that to ttm_bo_individualize_resv
v3: add a comment to explain what's going on

Signed-off-by: Christian König 
Reviewed-by: xinhui pan 
---
drivers/gpu/drm/ttm/ttm_bo.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index bfc42a9e4fb4..8174603d390f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -393,6 +393,18 @@ static int ttm_bo_individualize_resv(struct 
ttm_buffer_object *bo)

r = dma_resv_copy_fences(&bo->base._resv, bo->base.resv);
dma_resv_unlock(&bo->base._resv);
+   if (r)
+   return r;
+
+   if (bo->type != ttm_bo_type_sg) {
+   /* This works because the BO is about to be destroyed and nobody
+* reference it any more. The only tricky case is the trylock on
+* the resv object while holding the lru_lock.
+*/
+   spin_lock(&ttm_bo_glob.lru_lock);
+   bo->base.resv = &bo->base._resv;
+   spin_unlock(&ttm_bo_glob.lru_lock);
+   }


how about something like that.
the basic idea is to do the bo cleanup work in bo release first and avoid any 
race with evict.
As in bo dieing progress, evict also just do bo cleanup work.

If bo is busy, neither bo_release nor evict  can do cleanupwork  on it. For the 
bo release case, we just add bo back to lru list.
So we can clean it up  both in workqueue and shrinker as the past way  did.

@@ -405,8 +405,9 @@ static int ttm_bo_individualize_resv(struct 
ttm_buffer_object *bo)
  
 if (bo->type != ttm_bo_type_sg) {

 spin_lock(&ttm_bo_glob.lru_lock);
-   bo->base.resv = &bo->base._resv;
+   ttm_bo_del_from_lru(bo);
 spin_unlock(&ttm_bo_glob.lru_lock);
+   bo->base.resv = &bo->base._resv;
 }
  
 return r;

@@ -606,10 +607,9 @@ static void ttm_bo_release(struct kref *kref)
  * shrinkers, now that they are queued for
  * destruction.
  */
-   if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) {
+   if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT)
 bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT;
-   ttm_bo_move_to_lru_tail(bo, NULL);
-   }
+   ttm_bo_add_mem_to_lru(bo, &bo->mem);
  
 kref_init(&bo->kref);

 list_add_tail(&bo->ddestroy, &bdev->ddestroy);


Yeah, thought about that as well. But this has the major drawback that 
the deleted BO moves to the end of the LRU, which is something we don't 
want.


I think the real solution to this problem is to go a completely 
different way and remove the delayed delete feature from TTM altogether. 
Instead this should be part of some DRM domain handler component.


In other words it should not matter if a BO is evicted, moved or freed. 
Whenever a piece of memory becomes available again we keep around a 
fence which marks the end of using this piece of memory.


When then somebody asks for new memory we work through the LRU and test 
if using a certain piece of memory makes sense or not. If we find that a 
BO needs to be evicted for this we return a reference to the BO in 
question to the upper level handling.


If we find that we can do the allocation but only with recently freed up 
memory we gather the fences and say you can only use the newly allocated 
memory after waiting for those.


HEY! Wait a second! Did I just outlined what a potential replacement to 
TTM would look like?


Cheers,
Christian.



thanks
xinhui



return r;
}
@@ -724,7 +736,7 @@ static bool ttm_bo_evict_swapout_allowable(struct 
ttm_buffer_object *bo,

if (bo->base.resv == ctx->resv) {
dma_resv_assert_held(bo->base.resv);
-   if (ctx->flags & TTM_OPT_FLAG_ALLOW_RES_EVICT || bo->deleted)
+   if (ctx->flags & TTM_OPT_FLAG_ALLOW_RES_EVICT)
ret = true;
*locked = false;
if (busy)
--
2.17.1

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cxinhui.pan%40amd.com%7Cb184dff5aaf349e2210008d7af092637%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637170326204966375&sdata=KdZN1l%2FkDYodXxPQgaXaSXUvMz2RHxysSSF9krQRgpI%3D&reserved=0


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

RE: [PATCH] drm/amd/powerplay: correct the way for checking SMU_FEATURE_BACO_BIT support

[Feng, Kenneth]

[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Kenneth Feng 



-Original Message-
From: amd-gfx  On Behalf Of Evan Quan
Sent: Wednesday, February 12, 2020 3:25 PM
To: amd-gfx@lists.freedesktop.org
Cc: Quan, Evan 
Subject: [PATCH] drm/amd/powerplay: correct the way for checking 
SMU_FEATURE_BACO_BIT support

[CAUTION: External Email]

Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' will always return 
false considering the 'smu_system_features_control(smu, false)' disabled all 
SMU features.

Change-Id: I73956ffa51d6da8375c7c377895a221e13d31594
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c 
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 7c84d48c19e6..6d4c99b016f9 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -1489,7 +1489,18 @@ static int smu_disable_dpm(struct smu_context *smu)

/* For baco, need to leave BACO feature enabled */
if (use_baco) {
-   if (smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)) {
+   /*
+* Correct the way for checking whether SMU_FEATURE_BACO_BIT
+* is supported.
+*
+* Since 'smu_feature_is_enabled(smu, SMU_FEATURE_BACO_BIT)' 
will
+* always return false as the 'smu_system_features_control(smu, 
false)'
+* was just issued above which disabled all SMU features.
+*
+* Thus 'smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT)' is 
used
+* now for the checking.
+*/
+   if (smu_feature_get_index(smu, SMU_FEATURE_BACO_BIT) >= 
+ 0) {
ret = smu_feature_set_enabled(smu, 
SMU_FEATURE_BACO_BIT, true);
if (ret) {
pr_warn("set BACO feature enabled failed, 
return %d\n", ret);
--
2.25.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7CKenneth.Feng%40amd.com%7Cda38ae9058e84371c27008d7af8cb337%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637170891151776945&sdata=XSong%2FYpksq3mnwhNs6LCB%2F1zDypOppD%2FzcXKm6zvK4%3D&reserved=0
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx