Re: amd-gfx Digest, Vol 41, Issue 390

[Yin, Tianci (Rico)]

[PATCH] drm/amdgpu/gmc10: properly set BANK_SELECT and FRAGMENT_SIZE

Reviewed-by: Tianci Yin 


From: amd-gfx  on behalf of 
amd-gfx-requ...@lists.freedesktop.org 
Sent: Wednesday, October 30, 2019 6:01
To: amd-gfx@lists.freedesktop.org 
Subject: amd-gfx Digest, Vol 41, Issue 390

Send amd-gfx mailing list submissions to
amd-gfx@lists.freedesktop.org

To subscribe or unsubscribe via the World Wide Web, visit
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
or, via email, send a message with subject or body 'help' to
amd-gfx-requ...@lists.freedesktop.org

You can reach the person managing the list at
amd-gfx-ow...@lists.freedesktop.org

When replying, please edit your Subject line so it is more specific
than "Re: Contents of amd-gfx digest..."


Today's Topics:

   1. Re: [PATCH v2 14/15] drm/amdgpu: Use mmu_range_notifier
  instead of hmm_mirror (Jason Gunthorpe)
   2. [PATCH] drm/amdgpu: remove PT BOs when unmapping
  (Huang, JinHuiEric)
   3. [PATCH] drm/amdgpu/renoir: move gfxoff handling into gfx9
  module (Alex Deucher)
   4. [PATCH] drm/amdgpu/gmc10: properly set BANK_SELECT and
  FRAGMENT_SIZE (Alex Deucher)
   5. 21:9 monitor resolution incorrect since 4.14 kernel (Neil Mayhew)


--

Message: 1
Date: Tue, 29 Oct 2019 19:25:48 +
From: Jason Gunthorpe 
To: "Yang, Philip" 
Cc: "linux...@kvack.org" , Jerome Glisse
, Ralph Campbell , John
Hubbard , "Kuehling, Felix"
, Juergen Gross , "Zhou,
David(ChunMing)" , Mike Marciniszyn
, Stefano Stabellini
, Oleksandr Andrushchenko
, "linux-r...@vger.kernel.org"
, "nouv...@lists.freedesktop.org"
, Dennis Dalessandro
, "amd-gfx@lists.freedesktop.org"
, Christoph Hellwig
, "dri-de...@lists.freedesktop.org"
, "Deucher, Alexander"
, "xen-de...@lists.xenproject.org"
, Boris Ostrovsky
, Petr Cvek ,
"Koenig, Christian" , Ben Skeggs

Subject: Re: [PATCH v2 14/15] drm/amdgpu: Use mmu_range_notifier
instead of hmm_mirror
Message-ID: <20191029192544.gu22...@mellanox.com>
Content-Type: text/plain; charset="us-ascii"

On Tue, Oct 29, 2019 at 07:22:37PM +, Yang, Philip wrote:
> Hi Jason,
>
> I did quick test after merging amd-staging-drm-next with the
> mmu_notifier branch, which includes this set changes. The test result
> has different failures, app stuck intermittently, GUI no display etc. I
> am understanding the changes and will try to figure out the cause.

Thanks! I'm not surprised by this given how difficult this patch was
to make. Let me know if I can assist in any way

Please ensure to run with lockdep enabled.. Your symptops sounds sort
of like deadlocking?

Regards,
Jason


--

Message: 2
Date: Tue, 29 Oct 2019 20:06:41 +
From: "Huang, JinHuiEric" 
To: "amd-gfx@lists.freedesktop.org" 
Cc: "Huang, JinHuiEric" 
Subject: [PATCH] drm/amdgpu: remove PT BOs when unmapping
Message-ID:
<1572379585-1401-1-git-send-email-jinhuieric.hu...@amd.com>
Content-Type: text/plain; charset="iso-8859-1"

The issue is PT BOs are not freed when unmapping VA,
which causes vram usage accumulated is huge in some
memory stress test, such as kfd big buffer stress test.
Function amdgpu_vm_bo_update_mapping() is called by both
amdgpu_vm_bo_update() and amdgpu_vm_clear_freed(). The
solution is replacing amdgpu_vm_bo_update_mapping() in
amdgpu_vm_clear_freed() with removing PT BOs function
to save vram usage.

Change-Id: Ic24e35bff8ca85265b418a642373f189d972a924
Signed-off-by: Eric Huang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56 +-
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 0f4c3b2..8a480c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1930,6 +1930,51 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)
 }

 /**
+ * amdgpu_vm_remove_ptes - free PT BOs
+ *
+ * @adev: amdgpu device structure
+ * @vm: amdgpu vm structure
+ * @start: start of mapped range
+ * @end: end of mapped entry
+ *
+ * Free the page table level.
+ */
+static int amdgpu_vm_remove_ptes(struct amdgpu_device *adev,
+   struct amdgpu_vm *vm, uint64_t start, uint64_t end)
+{
+   struct amdgpu_vm_pt_cursor cursor;
+   unsigned shift, num_entries;
+
+   amdgpu_vm_pt_start(adev, vm, start, &cursor);
+   while (cursor.level < AMDGPU_VM_PTB) {
+   if (!amdgpu_vm_pt_descendant(adev, &cursor))
+   return -ENOENT;
+   }
+
+   while (cursor.pfn < end) {
+   amdgpu_vm_free_table(cursor.entry);
+   num_entries = amdgpu_vm_num_entries(adev, cursor.level - 1);
+
+ 

Re: [PATCH v2 13/15] drm/amdgpu: Use mmu_range_insert instead of hmm_mirror

[Jason Gunthorpe]

On Tue, Oct 29, 2019 at 10:14:29PM +, Kuehling, Felix wrote:

> > +static const struct mmu_range_notifier_ops amdgpu_mn_hsa_ops = {
> > +   .invalidate = amdgpu_mn_invalidate_hsa,
> > +};
> > +
> > +static int amdgpu_mn_sync_pagetables(struct hmm_mirror *mirror,
> > +const struct mmu_notifier_range *update)
> >   {
> > struct amdgpu_mn *amn = container_of(mirror, struct amdgpu_mn, mirror);
> > -   unsigned long start = update->start;
> > -   unsigned long end = update->end;
> > -   bool blockable = mmu_notifier_range_blockable(update);
> > -   struct interval_tree_node *it;
> >   
> > -   /* notification is exclusive, but interval is inclusive */
> > -   end -= 1;
> > -
> > -   if (amdgpu_mn_read_lock(amn, blockable))
> > -   return -EAGAIN;
> > -
> > -   it = interval_tree_iter_first(&amn->objects, start, end);
> > -   while (it) {
> > -   struct amdgpu_mn_node *node;
> > -   struct amdgpu_bo *bo;
> > -
> > -   if (!blockable) {
> > -   amdgpu_mn_read_unlock(amn);
> > -   return -EAGAIN;
> > -   }
> > -
> > -   node = container_of(it, struct amdgpu_mn_node, it);
> > -   it = interval_tree_iter_next(it, start, end);
> > -
> > -   list_for_each_entry(bo, &node->bos, mn_list) {
> > -   struct kgd_mem *mem = bo->kfd_bo;
> > -
> > -   if (amdgpu_ttm_tt_affect_userptr(bo->tbo.ttm,
> > -start, end))
> > -   amdgpu_amdkfd_evict_userptr(mem, amn->mm);
> > -   }
> > -   }
> > -
> > -   amdgpu_mn_read_unlock(amn);
> > +   if (!mmu_notifier_range_blockable(update))
> > +   return false;
> 
> This should return -EAGAIN. Not sure it matters much, because this whole 
> function disappears in the next commit in the series. It seems to be 
> only vestigial at this point.

Right, the only reason it is still here is that I couldn't really tell
if this:

> > +   down_read(&amn->lock);
> > +   up_read(&amn->lock);
> > return 0;
> >   }

Was serving as the 'driver lock' in the hmm scheme... If not then the
whole thing should just be deleted at this point.

I fixed the EAGAIN though

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

Re: [PATCH v2 02/15] mm/mmu_notifier: add an interval tree notifier

[Jason Gunthorpe]

On Tue, Oct 29, 2019 at 10:04:45PM +, Kuehling, Felix wrote:

> >* because mm->mm_users > 0 during mmu_notifier_register and exit_mmap
> > @@ -52,17 +286,24 @@ struct mmu_notifier_mm {
> >* can't go away from under us as exit_mmap holds an mm_count pin
> >* itself.
> >*/
> > -void __mmu_notifier_release(struct mm_struct *mm)
> > +static void mn_hlist_release(struct mmu_notifier_mm *mmn_mm,
> > +struct mm_struct *mm)
> >   {
> > struct mmu_notifier *mn;
> > int id;
> >   
> > +   if (mmn_mm->has_interval)
> > +   mn_itree_release(mmn_mm, mm);
> > +
> > +   if (hlist_empty(&mmn_mm->list))
> > +   return;
> 
> This seems to duplicate the conditions in __mmu_notifier_release. See my 
> comments below, I think one of them is wrong. I suspect this one, 
> because __mmu_notifier_release follows the same pattern as the other 
> notifiers.

Yep, this is a rebasing error from a earlier version, the above two
lines should be deleted.

I think it is harmless so it should not impact any testing.

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

[PATCH -next] drm/amd/display: Add a conversion function for transmitter and phy_id enums

[Nathan Chancellor]

Clang warns:

../drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:2520:42:
error: implicit conversion from enumeration type 'enum transmitter' to
different enumeration type 'enum physical_phy_id'
[-Werror,-Wenum-conversion]
psr_context->smuPhyId = link->link_enc->transmitter;
  ~ ^~~
1 error generated.

As the comment above this assignment states, this is intentional. To
match previous warnings of this nature, add a conversion function that
explicitly converts between the enums and warns when there is a
mismatch.

See commit 828cfa29093f ("drm/amdgpu: Fix amdgpu ras to ta enums
conversion") and commit d9ec5cfd5a2e ("drm/amd/display: Use switch table
for dc_to_smu_clock_type") for previous examples of this.

Fixes: e0d08a40a63b ("drm/amd/display: Add debugfs entry for reading psr state")
Link: https://github.com/ClangBuiltLinux/linux/issues/758
Signed-off-by: Nathan Chancellor 
---
 drivers/gpu/drm/amd/display/dc/core/dc_link.c | 38 ++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
index 7b18087be585..38dfe460e13b 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
@@ -2447,6 +2447,41 @@ bool dc_link_get_psr_state(const struct dc_link *link, 
uint32_t *psr_state)
return true;
 }
 
+static inline enum physical_phy_id
+transmitter_to_phy_id(enum transmitter transmitter_value)
+{
+   switch (transmitter_value) {
+   case TRANSMITTER_UNIPHY_A:
+   return PHYLD_0;
+   case TRANSMITTER_UNIPHY_B:
+   return PHYLD_1;
+   case TRANSMITTER_UNIPHY_C:
+   return PHYLD_2;
+   case TRANSMITTER_UNIPHY_D:
+   return PHYLD_3;
+   case TRANSMITTER_UNIPHY_E:
+   return PHYLD_4;
+   case TRANSMITTER_UNIPHY_F:
+   return PHYLD_5;
+   case TRANSMITTER_NUTMEG_CRT:
+   return PHYLD_6;
+   case TRANSMITTER_TRAVIS_CRT:
+   return PHYLD_7;
+   case TRANSMITTER_TRAVIS_LCD:
+   return PHYLD_8;
+   case TRANSMITTER_UNIPHY_G:
+   return PHYLD_9;
+   case TRANSMITTER_COUNT:
+   return PHYLD_COUNT;
+   case TRANSMITTER_UNKNOWN:
+   return PHYLD_UNKNOWN;
+   default:
+   WARN_ONCE(1, "Unknown transmitter value %d\n",
+ transmitter_value);
+   return PHYLD_0;
+   }
+}
+
 bool dc_link_setup_psr(struct dc_link *link,
const struct dc_stream_state *stream, struct psr_config 
*psr_config,
struct psr_context *psr_context)
@@ -2517,7 +2552,8 @@ bool dc_link_setup_psr(struct dc_link *link,
/* Hardcoded for now.  Can be Pcie or Uniphy (or Unknown)*/
psr_context->phyType = PHY_TYPE_UNIPHY;
/*PhyId is associated with the transmitter id*/
-   psr_context->smuPhyId = link->link_enc->transmitter;
+   psr_context->smuPhyId =
+   transmitter_to_phy_id(link->link_enc->transmitter);
 
psr_context->crtcTimingVerticalTotal = stream->timing.v_total;
psr_context->vsyncRateHz = div64_u64(div64_u64((stream->
-- 
2.24.0.rc1

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

[PATCH] drm/amd/powerplay: print the pptable provider

[Yuan, Xiaojie]

Signed-off-by: Xiaojie Yuan 
---
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c 
b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index 7e882999abad..0f7504ae2395 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -368,6 +368,7 @@ int smu_v11_0_setup_pptable(struct smu_context *smu)
version_major = le16_to_cpu(hdr->header.header_version_major);
version_minor = le16_to_cpu(hdr->header.header_version_minor);
if (version_major == 2 && smu->smu_table.boot_values.pp_table_id > 0) {
+   pr_info("use driver provided pptable %d\n", 
smu->smu_table.boot_values.pp_table_id);
switch (version_minor) {
case 0:
ret = smu_v11_0_set_pptable_v2_0(smu, &table, &size);
@@ -384,6 +385,7 @@ int smu_v11_0_setup_pptable(struct smu_context *smu)
return ret;
 
} else {
+   pr_info("use vbios provided pptable\n");
index = 
get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
powerplayinfo);
 
-- 
2.20.1

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

Re: [PATCH] drm/amd/powerplay: print the pptable provider

[Wang, Kevin(Yang)]

Reviewed-by: Kevin Wang 

also you can put this information in sysfs "amdgpu_pm_info" .

Best Regards,
Kevin


From: Yuan, Xiaojie 
Sent: Wednesday, October 30, 2019 4:19 PM
To: amd-gfx@lists.freedesktop.org 
Cc: Quan, Evan ; Wang, Kevin(Yang) ; 
Feng, Kenneth ; Yuan, Xiaojie 
Subject: [PATCH] drm/amd/powerplay: print the pptable provider

Signed-off-by: Xiaojie Yuan 
---
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c 
b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
index 7e882999abad..0f7504ae2395 100644
--- a/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
+++ b/drivers/gpu/drm/amd/powerplay/smu_v11_0.c
@@ -368,6 +368,7 @@ int smu_v11_0_setup_pptable(struct smu_context *smu)
 version_major = le16_to_cpu(hdr->header.header_version_major);
 version_minor = le16_to_cpu(hdr->header.header_version_minor);
 if (version_major == 2 && smu->smu_table.boot_values.pp_table_id > 0) {
+   pr_info("use driver provided pptable %d\n", 
smu->smu_table.boot_values.pp_table_id);
 switch (version_minor) {
 case 0:
 ret = smu_v11_0_set_pptable_v2_0(smu, &table, &size);
@@ -384,6 +385,7 @@ int smu_v11_0_setup_pptable(struct smu_context *smu)
 return ret;

 } else {
+   pr_info("use vbios provided pptable\n");
 index = 
get_index_into_master_table(atom_master_list_of_data_tables_v2_1,
 powerplayinfo);

--
2.20.1

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

Re: [PATCH] drm/amdgpu: guard ib scheduling while in reset

[S, Shirish]

On 10/25/2019 9:32 PM, Grodzovsky, Andrey wrote:


On 10/25/19 11:57 AM, Koenig, Christian wrote:
Am 25.10.19 um 17:35 schrieb Grodzovsky, Andrey:


On 10/25/19 5:26 AM, Koenig, Christian wrote:
Am 25.10.19 um 11:22 schrieb S, Shirish:


On 10/25/2019 2:23 PM, Koenig, Christian wrote:

amdgpu_do_asic_reset starting to resume blocks

...

amdgpu :03:00.0: couldn't schedule ib on ring 
[drm:amdgpu_job_run] *ERROR* Error scheduling IBs (-22)
...

amdgpu_device_ip_resume_phase2 resumed gfx_v9_0

amdgpu_device_ip_resume_phase2 resumed sdma_v4_0

amdgpu_device_ip_resume_phase2 resumed powerplay

This is what's the root of the problem.

The scheduler should never be resumed before we are done with bringing back the 
hardware into an usable state.

I dont see the scheduler being resumed when the ib is scheduled, its done way 
after the hardware is ready in reset code path.

Below are the logs:

amdgpu :03:00.0: GPU reset begin!
amdgpu_device_gpu_recover calling drm_sched_stop <==
...
amdgpu :03:00.0: couldn't schedule ib on ring 
[drm:amdgpu_job_run] *ERROR* Error scheduling IBs (-22)
...
amdgpu_device_ip_resume_phase2 resumed sdma_v4_0
amdgpu_device_ip_resume_phase2 resumed powerplay
amdgpu_device_ip_resume_phase2 resumed dm
...
[drm] recover vram bo from shadow done
amdgpu_device_gpu_recover calling  drm_sched_start <==
...

As mentioned in the call trace, drm_sched_main() is responsible for this 
job_run which seems to be called during cleanup.

Then the scheduler isn't stopped for some reason and we need to investigate why.

We used to have another kthread_park()/unpark() in drm_sched_entity_fini(), 
maybe an application is crashing while we are trying to reset the GPU?


We still have it and isn't doing kthread_park()/unpark() from 
drm_sched_entity_fini while GPU reset in progress defeats all the purpose of 
drm_sched_stop->kthread_park ? If drm_sched_entity_fini-> kthread_unpark 
happens AFTER drm_sched_stop->kthread_park nothing prevents from another 
(third) thread keep submitting job to HW which will be picked up by the 
unparked scheduler thread try to submit to HW but fail because the HW ring is 
deactivated.

If so maybe we should serialize calls to kthread_park/unpark(sched->thread) ?

Yeah, that was my thinking as well. Probably best to just grab the reset lock 
before calling drm_sched_entity_fini().


Shirish - please try locking &adev->lock_reset around calls to 
drm_sched_entity_fini as Christian suggests and see if this actually helps the 
issue.

Yes that also works.

Regards,

Shirish S

Andrey


Alternative I think we could change the kthread_park/unpark to a 
wait_event_ in drm_sched_entity_fini().

Regards,
Christian.


Andrey


Would be rather unlikely, especially that would be rather hard to reproduce but 
currently my best bet what's going wrong here.

Regards,
Christian.


Regards,

Shirish S

Regards,
Christian.

Am 25.10.19 um 10:50 schrieb S, Shirish:

Here is the call trace:

Call Trace:
 dump_stack+0x4d/0x63
 amdgpu_ib_schedule+0x86/0x4b7
 ? __mod_timer+0x21e/0x244
 amdgpu_job_run+0x108/0x178
 drm_sched_main+0x253/0x2fa
 ? remove_wait_queue+0x51/0x51
 ? drm_sched_cleanup_jobs.part.12+0xda/0xda
 kthread+0x14f/0x157
 ? kthread_park+0x86/0x86
 ret_from_fork+0x22/0x40
amdgpu :03:00.0: couldn't schedule ib on ring 
[drm:amdgpu_job_run] *ERROR* Error scheduling IBs (-22)

printed via below change:

@@ -151,6 +152,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
num_ibs,
}

if (!ring->sched.ready) {
+  dump_stack();
dev_err(adev->dev, "couldn't schedule ib on ring <%s>\n", 
ring->name);
return -EINVAL;

On 10/24/2019 10:00 PM, Christian König wrote:
Am 24.10.19 um 17:06 schrieb Grodzovsky, Andrey:


On 10/24/19 7:01 AM, Christian König wrote:
Am 24.10.19 um 12:58 schrieb S, Shirish:
[Why]
Upon GPU reset, kernel cleans up already submitted jobs
via drm_sched_cleanup_jobs.
This schedules ib's via drm_sched_main()->run_job, leading to
race condition of rings being ready or not, since during reset
rings may be suspended.

NAK, exactly that's what should not happen.

The scheduler should be suspend while a GPU reset is in progress.

So you are running into a completely different race here.

Below is the series of events when the issue occurs.

(Note that as you & Andrey mentioned the scheduler has been suspended but the 
job is scheduled nonetheless.)

amdgpu :03:00.0: GPU reset begin!

...

amdgpu_device_gpu_recover stopping ring sdma0 via drm_sched_stop

...

amdgpu :03:00.0: GPU reset succeeded, trying to resume

amdgpu_do_asic_reset starting to resume blocks

...

amdgpu :03:00.0: couldn't schedule ib on ring 
[drm:amdgpu_job_run] *ERROR* Error scheduling IBs (-22)
...

amdgpu_device_ip_resume_phase2 resumed gfx_v9_0

amdgpu_device_ip_resume_phase2 resumed sdma_v4_0

amdgpu_device_ip_resume_phase2 resumed powerplay

...

FWIW, since the job is always NULL when "

[PATCH 1/1] drm/amdgpu: fix no ACK from LDS read during stress test for Arcturus

[Le Ma]

Set mmSQ_CONFIG.DISABLE_SMEM_SOFT_CLAUSE as W/R.

Change-Id: I6225909fd62702427fbb807e0c6ba6bafcfa41d5
Signed-off-by: Le Ma 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 5e7a01c..07962ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -699,6 +699,7 @@ static const struct soc15_reg_golden 
golden_settings_gc_9_4_1_arct[] =
SOC15_REG_GOLDEN_VALUE(GC, 0, mmTCP_CHAN_STEER_3_ARCT, 0x3fff, 
0x2ebd9fe3),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmTCP_CHAN_STEER_4_ARCT, 0x3fff, 
0xb90f5b1),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmTCP_CHAN_STEER_5_ARCT, 0x3ff, 0x135),
+   SOC15_REG_GOLDEN_VALUE(GC, 0, mmSQ_CONFIG, 0x, 0x011A),
 };
 
 static const u32 GFX_RLC_SRM_INDEX_CNTL_ADDR_OFFSETS[] =
-- 
2.7.4

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

RE: [PATCH 1/1] drm/amdgpu: fix no ACK from LDS read during stress test for Arcturus

[Zhang, Hawking]

Reviewed-by: Hawking Zhang 

Regards,
Hawking
-Original Message-
From: Le Ma  
Sent: 2019年10月30日 17:02
To: amd-gfx@lists.freedesktop.org
Cc: Zhang, Hawking ; Deucher, Alexander 
; Cornwall, Jay ; Ma, Le 

Subject: [PATCH 1/1] drm/amdgpu: fix no ACK from LDS read during stress test 
for Arcturus

Set mmSQ_CONFIG.DISABLE_SMEM_SOFT_CLAUSE as W/R.

Change-Id: I6225909fd62702427fbb807e0c6ba6bafcfa41d5
Signed-off-by: Le Ma 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 5e7a01c..07962ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -699,6 +699,7 @@ static const struct soc15_reg_golden 
golden_settings_gc_9_4_1_arct[] =
SOC15_REG_GOLDEN_VALUE(GC, 0, mmTCP_CHAN_STEER_3_ARCT, 0x3fff, 
0x2ebd9fe3),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmTCP_CHAN_STEER_4_ARCT, 0x3fff, 
0xb90f5b1),
SOC15_REG_GOLDEN_VALUE(GC, 0, mmTCP_CHAN_STEER_5_ARCT, 0x3ff, 0x135),
+   SOC15_REG_GOLDEN_VALUE(GC, 0, mmSQ_CONFIG, 0x, 0x011A),
 };
 
 static const u32 GFX_RLC_SRM_INDEX_CNTL_ADDR_OFFSETS[] =
-- 
2.7.4

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

[PATCH] drm/amdgpu: dont schedule jobs while in reset

[S, Shirish]

[Why]

doing kthread_park()/unpark() from drm_sched_entity_fini
while GPU reset is in progress defeats all the purpose of
drm_sched_stop->kthread_park.
If drm_sched_entity_fini->kthread_unpark() happens AFTER
drm_sched_stop->kthread_park nothing prevents from another
(third) thread to keep submitting job to HW which will be
picked up by the unparked scheduler thread and try to submit
to HW but fail because the HW ring is deactivated.

[How]
grab the reset lock before calling drm_sched_entity_fini()

Signed-off-by: Shirish S 
Suggested-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 6614d8a..2cdaf3b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -604,8 +604,11 @@ void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr *mgr)
continue;
}
 
-   for (i = 0; i < num_entities; i++)
+   for (i = 0; i < num_entities; i++) {
+   mutex_lock(&ctx->adev->lock_reset);
drm_sched_entity_fini(&ctx->entities[0][i].entity);
+   mutex_unlock(&ctx->adev->lock_reset);
+   }
}
 }
 
-- 
2.7.4

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

Re: [PATCH] drm/amdgpu: dont schedule jobs while in reset

[Koenig, Christian]

Am 30.10.19 um 10:13 schrieb S, Shirish:
> [Why]
>
> doing kthread_park()/unpark() from drm_sched_entity_fini
> while GPU reset is in progress defeats all the purpose of
> drm_sched_stop->kthread_park.
> If drm_sched_entity_fini->kthread_unpark() happens AFTER
> drm_sched_stop->kthread_park nothing prevents from another
> (third) thread to keep submitting job to HW which will be
> picked up by the unparked scheduler thread and try to submit
> to HW but fail because the HW ring is deactivated.
>
> [How]
> grab the reset lock before calling drm_sched_entity_fini()
>
> Signed-off-by: Shirish S 
> Suggested-by: Christian König 

Patch itself is Reviewed-by: Christian König 

Does that also fix the problems you have been seeing?

Thanks,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 5 -
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 6614d8a..2cdaf3b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -604,8 +604,11 @@ void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr 
> *mgr)
>   continue;
>   }
>   
> - for (i = 0; i < num_entities; i++)
> + for (i = 0; i < num_entities; i++) {
> + mutex_lock(&ctx->adev->lock_reset);
>   drm_sched_entity_fini(&ctx->entities[0][i].entity);
> + mutex_unlock(&ctx->adev->lock_reset);
> + }
>   }
>   }
>   

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

Re: [PATCH] drm/amdgpu: dont schedule jobs while in reset

[S, Shirish]

On 10/30/2019 3:50 PM, Koenig, Christian wrote:
> Am 30.10.19 um 10:13 schrieb S, Shirish:
>> [Why]
>>
>> doing kthread_park()/unpark() from drm_sched_entity_fini
>> while GPU reset is in progress defeats all the purpose of
>> drm_sched_stop->kthread_park.
>> If drm_sched_entity_fini->kthread_unpark() happens AFTER
>> drm_sched_stop->kthread_park nothing prevents from another
>> (third) thread to keep submitting job to HW which will be
>> picked up by the unparked scheduler thread and try to submit
>> to HW but fail because the HW ring is deactivated.
>>
>> [How]
>> grab the reset lock before calling drm_sched_entity_fini()
>>
>> Signed-off-by: Shirish S 
>> Suggested-by: Christian König 
> Patch itself is Reviewed-by: Christian König 
>
> Does that also fix the problems you have been seeing?

Yes Christian.

Regards,

Shirish S

>
> Thanks,
> Christian.
>
>> ---
>>drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 5 -
>>1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> index 6614d8a..2cdaf3b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> @@ -604,8 +604,11 @@ void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr 
>> *mgr)
>>  continue;
>>  }
>>
>> -for (i = 0; i < num_entities; i++)
>> +for (i = 0; i < num_entities; i++) {
>> +mutex_lock(&ctx->adev->lock_reset);
>>  drm_sched_entity_fini(&ctx->entities[0][i].entity);
>> +mutex_unlock(&ctx->adev->lock_reset);
>> +}
>>  }
>>}
>>

-- 
Regards,
Shirish S

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

Re: [PATCH -next] drm/amd/display: Add a conversion function for transmitter and phy_id enums

[Kazlauskas, Nicholas]

On 2019-10-30 2:04 a.m., Nathan Chancellor wrote:
> Clang warns:
> 
> ../drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:2520:42:
> error: implicit conversion from enumeration type 'enum transmitter' to
> different enumeration type 'enum physical_phy_id'
> [-Werror,-Wenum-conversion]
>  psr_context->smuPhyId = link->link_enc->transmitter;
>~ ^~~
> 1 error generated.
> 
> As the comment above this assignment states, this is intentional. To
> match previous warnings of this nature, add a conversion function that
> explicitly converts between the enums and warns when there is a
> mismatch.
> 
> See commit 828cfa29093f ("drm/amdgpu: Fix amdgpu ras to ta enums
> conversion") and commit d9ec5cfd5a2e ("drm/amd/display: Use switch table
> for dc_to_smu_clock_type") for previous examples of this.
> 
> Fixes: e0d08a40a63b ("drm/amd/display: Add debugfs entry for reading psr 
> state")
> Link: https://github.com/ClangBuiltLinux/linux/issues/758
> Signed-off-by: Nathan Chancellor 

Reviewed-by: Nicholas Kazlauskas 

With the small nitpick that maybe the default case should be 
PHYLD_UNKNOWN, but well get the warning if that happens anyway.

Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/amd/display/dc/core/dc_link.c | 38 ++-
>   1 file changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> index 7b18087be585..38dfe460e13b 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> @@ -2447,6 +2447,41 @@ bool dc_link_get_psr_state(const struct dc_link *link, 
> uint32_t *psr_state)
>   return true;
>   }
>   
> +static inline enum physical_phy_id
> +transmitter_to_phy_id(enum transmitter transmitter_value)
> +{
> + switch (transmitter_value) {
> + case TRANSMITTER_UNIPHY_A:
> + return PHYLD_0;
> + case TRANSMITTER_UNIPHY_B:
> + return PHYLD_1;
> + case TRANSMITTER_UNIPHY_C:
> + return PHYLD_2;
> + case TRANSMITTER_UNIPHY_D:
> + return PHYLD_3;
> + case TRANSMITTER_UNIPHY_E:
> + return PHYLD_4;
> + case TRANSMITTER_UNIPHY_F:
> + return PHYLD_5;
> + case TRANSMITTER_NUTMEG_CRT:
> + return PHYLD_6;
> + case TRANSMITTER_TRAVIS_CRT:
> + return PHYLD_7;
> + case TRANSMITTER_TRAVIS_LCD:
> + return PHYLD_8;
> + case TRANSMITTER_UNIPHY_G:
> + return PHYLD_9;
> + case TRANSMITTER_COUNT:
> + return PHYLD_COUNT;
> + case TRANSMITTER_UNKNOWN:
> + return PHYLD_UNKNOWN;
> + default:
> + WARN_ONCE(1, "Unknown transmitter value %d\n",
> +   transmitter_value);
> + return PHYLD_0;
> + }
> +}
> +
>   bool dc_link_setup_psr(struct dc_link *link,
>   const struct dc_stream_state *stream, struct psr_config 
> *psr_config,
>   struct psr_context *psr_context)
> @@ -2517,7 +2552,8 @@ bool dc_link_setup_psr(struct dc_link *link,
>   /* Hardcoded for now.  Can be Pcie or Uniphy (or Unknown)*/
>   psr_context->phyType = PHY_TYPE_UNIPHY;
>   /*PhyId is associated with the transmitter id*/
> - psr_context->smuPhyId = link->link_enc->transmitter;
> + psr_context->smuPhyId =
> + transmitter_to_phy_id(link->link_enc->transmitter);
>   
>   psr_context->crtcTimingVerticalTotal = stream->timing.v_total;
>   psr_context->vsyncRateHz = div64_u64(div64_u64((stream->
> 

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

Re: [PATCH v3] drm/radeon: Fix EEH during kexec

[Michael Ellerman]

Hi Kyle,

KyleMahlkuch  writes:
> From: Kyle Mahlkuch 
>
> During kexec some adapters hit an EEH since they are not properly
> shut down in the radeon_pci_shutdown() function. Adding
> radeon_suspend_kms() fixes this issue.
> Enabled only on PPC because this patch causes issues on some other
> boards.

Which adapters hit the issues?

And do we know why they're not shut down correctly in
radeon_pci_shutdown()? That seems like the root cause no?


> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 9e55076..4528f4d 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -379,11 +379,25 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>  static void
>  radeon_pci_shutdown(struct pci_dev *pdev)
>  {
> +#ifdef CONFIG_PPC64
> + struct drm_device *ddev = pci_get_drvdata(pdev);
> +#endif

This local serves no real purpose and could be avoided, which would also
avoid this ifdef.

>   /* if we are running in a VM, make sure the device
>* torn down properly on reboot/shutdown
>*/
>   if (radeon_device_is_virtual())
>   radeon_pci_remove(pdev);
> +
> +#ifdef CONFIG_PPC64
> + /* Some adapters need to be suspended before a

AFAIK drm uses normal kernel comment style, so this should be:

/*
 * Some adapters need to be suspended before a
> +  * shutdown occurs in order to prevent an error
> +  * during kexec.
> +  * Make this power specific becauase it breaks
> +  * some non-power boards.
> +  */
> + radeon_suspend_kms(ddev, true, true, false);

ie, instead do:

radeon_suspend_kms(pci_get_drvdata(pdev), true, true, false);

> +#endif
>  }
>  
>  static int radeon_pmops_suspend(struct device *dev)
> -- 
> 1.8.3.1

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

Re: [PATCH] drm/amdgpu: remove PT BOs when unmapping

[Christian König]

Am 29.10.19 um 21:06 schrieb Huang, JinHuiEric:

The issue is PT BOs are not freed when unmapping VA,
which causes vram usage accumulated is huge in some
memory stress test, such as kfd big buffer stress test.
Function amdgpu_vm_bo_update_mapping() is called by both
amdgpu_vm_bo_update() and amdgpu_vm_clear_freed(). The
solution is replacing amdgpu_vm_bo_update_mapping() in
amdgpu_vm_clear_freed() with removing PT BOs function
to save vram usage.


NAK, that is intentional behavior.

Otherwise we can run into out of memory situations when page tables need 
to be allocated again under stress.


Regards,
Christian.



Change-Id: Ic24e35bff8ca85265b418a642373f189d972a924
Signed-off-by: Eric Huang 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56 +-
  1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 0f4c3b2..8a480c7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1930,6 +1930,51 @@ static void amdgpu_vm_prt_fini(struct amdgpu_device 
*adev, struct amdgpu_vm *vm)
  }
  
  /**

+ * amdgpu_vm_remove_ptes - free PT BOs
+ *
+ * @adev: amdgpu device structure
+ * @vm: amdgpu vm structure
+ * @start: start of mapped range
+ * @end: end of mapped entry
+ *
+ * Free the page table level.
+ */
+static int amdgpu_vm_remove_ptes(struct amdgpu_device *adev,
+   struct amdgpu_vm *vm, uint64_t start, uint64_t end)
+{
+   struct amdgpu_vm_pt_cursor cursor;
+   unsigned shift, num_entries;
+
+   amdgpu_vm_pt_start(adev, vm, start, &cursor);
+   while (cursor.level < AMDGPU_VM_PTB) {
+   if (!amdgpu_vm_pt_descendant(adev, &cursor))
+   return -ENOENT;
+   }
+
+   while (cursor.pfn < end) {
+   amdgpu_vm_free_table(cursor.entry);
+   num_entries = amdgpu_vm_num_entries(adev, cursor.level - 1);
+
+   if (cursor.entry != &cursor.parent->entries[num_entries - 1]) {
+   /* Next ptb entry */
+   shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
+   cursor.pfn += 1ULL << shift;
+   cursor.pfn &= ~((1ULL << shift) - 1);
+   cursor.entry++;
+   } else {
+   /* Next ptb entry in next pd0 entry */
+   amdgpu_vm_pt_ancestor(&cursor);
+   shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
+   cursor.pfn += 1ULL << shift;
+   cursor.pfn &= ~((1ULL << shift) - 1);
+   amdgpu_vm_pt_descendant(adev, &cursor);
+   }
+   }
+
+   return 0;
+}
+
+/**
   * amdgpu_vm_clear_freed - clear freed BOs in the PT
   *
   * @adev: amdgpu_device pointer
@@ -1949,7 +1994,6 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
  struct dma_fence **fence)
  {
struct amdgpu_bo_va_mapping *mapping;
-   uint64_t init_pte_value = 0;
struct dma_fence *f = NULL;
int r;
  
@@ -1958,13 +2002,10 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,

struct amdgpu_bo_va_mapping, list);
list_del(&mapping->list);
  
-		if (vm->pte_support_ats &&

-   mapping->start < AMDGPU_GMC_HOLE_START)
-   init_pte_value = AMDGPU_PTE_DEFAULT_ATC;
+   r = amdgpu_vm_remove_ptes(adev, vm,
+   (mapping->start + 0x1ff) & (~0x1ffll),
+   (mapping->last + 1) & (~0x1ffll));
  
-		r = amdgpu_vm_bo_update_mapping(adev, vm, false, NULL,

-   mapping->start, mapping->last,
-   init_pte_value, 0, NULL, &f);
amdgpu_vm_free_mapping(adev, vm, mapping, f);
if (r) {
dma_fence_put(f);
@@ -1980,7 +2021,6 @@ int amdgpu_vm_clear_freed(struct amdgpu_device *adev,
}
  
  	return 0;

-
  }
  
  /**


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

Re: [PATCH] drm/amdgpu/gmc10: properly set BANK_SELECT and FRAGMENT_SIZE

[Christian König]

Am 29.10.19 um 22:15 schrieb Alex Deucher:

These were not aligned for optimal performance for GPUVM.

Signed-off-by: Alex Deucher 


Good catch. But I haven't read the GMC10 documentation yet of everything 
is still the same as on GMC9.


So only Acked-by: Christian König 

Regards,
Christian.


---
  drivers/gpu/drm/amd/amdgpu/gfxhub_v2_0.c | 9 +
  drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c  | 9 +
  2 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_0.c
index b601c6740ef5..b4f32d853ca1 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfxhub_v2_0.c
@@ -155,6 +155,15 @@ static void gfxhub_v2_0_init_cache_regs(struct 
amdgpu_device *adev)
WREG32_SOC15(GC, 0, mmGCVM_L2_CNTL2, tmp);
  
  	tmp = mmGCVM_L2_CNTL3_DEFAULT;

+   if (adev->gmc.translate_further) {
+   tmp = REG_SET_FIELD(tmp, GCVM_L2_CNTL3, BANK_SELECT, 12);
+   tmp = REG_SET_FIELD(tmp, GCVM_L2_CNTL3,
+   L2_CACHE_BIGK_FRAGMENT_SIZE, 9);
+   } else {
+   tmp = REG_SET_FIELD(tmp, GCVM_L2_CNTL3, BANK_SELECT, 9);
+   tmp = REG_SET_FIELD(tmp, GCVM_L2_CNTL3,
+   L2_CACHE_BIGK_FRAGMENT_SIZE, 6);
+   }
WREG32_SOC15(GC, 0, mmGCVM_L2_CNTL3, tmp);
  
  	tmp = mmGCVM_L2_CNTL4_DEFAULT;

diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c 
b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
index 2eea702de8ee..945533634711 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c
@@ -142,6 +142,15 @@ static void mmhub_v2_0_init_cache_regs(struct 
amdgpu_device *adev)
WREG32_SOC15(MMHUB, 0, mmMMVM_L2_CNTL2, tmp);
  
  	tmp = mmMMVM_L2_CNTL3_DEFAULT;

+   if (adev->gmc.translate_further) {
+   tmp = REG_SET_FIELD(tmp, MMVM_L2_CNTL3, BANK_SELECT, 12);
+   tmp = REG_SET_FIELD(tmp, MMVM_L2_CNTL3,
+   L2_CACHE_BIGK_FRAGMENT_SIZE, 9);
+   } else {
+   tmp = REG_SET_FIELD(tmp, MMVM_L2_CNTL3, BANK_SELECT, 9);
+   tmp = REG_SET_FIELD(tmp, MMVM_L2_CNTL3,
+   L2_CACHE_BIGK_FRAGMENT_SIZE, 6);
+   }
WREG32_SOC15(MMHUB, 0, mmMMVM_L2_CNTL3, tmp);
  
  	tmp = mmMMVM_L2_CNTL4_DEFAULT;


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

Re: [PATCH] drm/amdgpu/renoir: move gfxoff handling into gfx9 module

[Alex Deucher]

Ping?

On Tue, Oct 29, 2019 at 4:10 PM Alex Deucher  wrote:
>
> To properly handle the option parsing ordering.
>
> Signed-off-by: Alex Deucher 
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 6 ++
>  drivers/gpu/drm/amd/amdgpu/soc15.c| 5 -
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 9fe95e7693d5..b2b3eb75c48c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -1051,6 +1051,12 @@ static void gfx_v9_0_check_if_need_gfxoff(struct 
> amdgpu_device *adev)
> !adev->gfx.rlc.is_rlc_v2_1))
> adev->pm.pp_feature &= ~PP_GFXOFF_MASK;
>
> +   if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> +   adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |
> +   AMD_PG_SUPPORT_CP |
> +   AMD_PG_SUPPORT_RLC_SMU_HS;
> +   break;
> +   case CHIP_RENOIR:
> if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |
> AMD_PG_SUPPORT_CP |
> diff --git a/drivers/gpu/drm/amd/amdgpu/soc15.c 
> b/drivers/gpu/drm/amd/amdgpu/soc15.c
> index 16c5bb75889f..25e69ea74a41 100644
> --- a/drivers/gpu/drm/amd/amdgpu/soc15.c
> +++ b/drivers/gpu/drm/amd/amdgpu/soc15.c
> @@ -1263,11 +1263,6 @@ static int soc15_common_early_init(void *handle)
>  AMD_PG_SUPPORT_VCN |
>  AMD_PG_SUPPORT_VCN_DPG;
> adev->external_rev_id = adev->rev_id + 0x91;
> -
> -   if (adev->pm.pp_feature & PP_GFXOFF_MASK)
> -   adev->pg_flags |= AMD_PG_SUPPORT_GFX_PG |
> -   AMD_PG_SUPPORT_CP |
> -   AMD_PG_SUPPORT_RLC_SMU_HS;
> break;
> default:
> /* FIXME: not supported yet */
> --
> 2.23.0
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu/gfx10: fix mqd backup/restore for gfx rings

[Alex Deucher]

On Tue, Oct 29, 2019 at 7:37 AM Yuan, Xiaojie  wrote:
>
> 1. no need to allocate an extra member for 'mqd_backup' array
> 2. backup/restore mqd to/from the correct 'mqd_backup' array slot
>
> Signed-off-by: Xiaojie Yuan 

Acked-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 +-
>  drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c  | 9 +
>  2 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index 459aa9059542..6d19e7891491 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -225,7 +225,7 @@ struct amdgpu_me {
> uint32_tnum_me;
> uint32_tnum_pipe_per_me;
> uint32_tnum_queue_per_pipe;
> -   void*mqd_backup[AMDGPU_MAX_GFX_RINGS + 1];
> +   void*mqd_backup[AMDGPU_MAX_GFX_RINGS];
>
> /* These are the resources for which amdgpu takes ownership */
> DECLARE_BITMAP(queue_bitmap, AMDGPU_MAX_GFX_QUEUES);
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index ef1975a5323a..2c5dc9b58e23 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -3075,6 +3075,7 @@ static int gfx_v10_0_gfx_init_queue(struct amdgpu_ring 
> *ring)
>  {
> struct amdgpu_device *adev = ring->adev;
> struct v10_gfx_mqd *mqd = ring->mqd_ptr;
> +   int mqd_idx = ring - &adev->gfx.gfx_ring[0];
>
> if (!adev->in_gpu_reset && !adev->in_suspend) {
> memset((void *)mqd, 0, sizeof(*mqd));
> @@ -3086,12 +3087,12 @@ static int gfx_v10_0_gfx_init_queue(struct 
> amdgpu_ring *ring)
>  #endif
> nv_grbm_select(adev, 0, 0, 0, 0);
> mutex_unlock(&adev->srbm_mutex);
> -   if (adev->gfx.me.mqd_backup[AMDGPU_MAX_GFX_RINGS])
> -   memcpy(adev->gfx.me.mqd_backup[AMDGPU_MAX_GFX_RINGS], 
> mqd, sizeof(*mqd));
> +   if (adev->gfx.me.mqd_backup[mqd_idx])
> +   memcpy(adev->gfx.me.mqd_backup[mqd_idx], mqd, 
> sizeof(*mqd));
> } else if (adev->in_gpu_reset) {
> /* reset mqd with the backup copy */
> -   if (adev->gfx.me.mqd_backup[AMDGPU_MAX_GFX_RINGS])
> -   memcpy(mqd, 
> adev->gfx.me.mqd_backup[AMDGPU_MAX_GFX_RINGS], sizeof(*mqd));
> +   if (adev->gfx.me.mqd_backup[mqd_idx])
> +   memcpy(mqd, adev->gfx.me.mqd_backup[mqd_idx], 
> sizeof(*mqd));
> /* reset the ring */
> ring->wptr = 0;
> amdgpu_ring_clear_ring(ring);
> --
> 2.20.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 -next] drm/amd/display: Add a conversion function for transmitter and phy_id enums

[Alex Deucher]

On Wed, Oct 30, 2019 at 8:33 AM Kazlauskas, Nicholas
 wrote:
>
> On 2019-10-30 2:04 a.m., Nathan Chancellor wrote:
> > Clang warns:
> >
> > ../drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:2520:42:
> > error: implicit conversion from enumeration type 'enum transmitter' to
> > different enumeration type 'enum physical_phy_id'
> > [-Werror,-Wenum-conversion]
> >  psr_context->smuPhyId = link->link_enc->transmitter;
> >~ ^~~
> > 1 error generated.
> >
> > As the comment above this assignment states, this is intentional. To
> > match previous warnings of this nature, add a conversion function that
> > explicitly converts between the enums and warns when there is a
> > mismatch.
> >
> > See commit 828cfa29093f ("drm/amdgpu: Fix amdgpu ras to ta enums
> > conversion") and commit d9ec5cfd5a2e ("drm/amd/display: Use switch table
> > for dc_to_smu_clock_type") for previous examples of this.
> >
> > Fixes: e0d08a40a63b ("drm/amd/display: Add debugfs entry for reading psr 
> > state")
> > Link: https://github.com/ClangBuiltLinux/linux/issues/758
> > Signed-off-by: Nathan Chancellor 
>
> Reviewed-by: Nicholas Kazlauskas 
>
> With the small nitpick that maybe the default case should be
> PHYLD_UNKNOWN, but well get the warning if that happens anyway.
>

Applied with that change.

Thanks!

Alex

> Nicholas Kazlauskas
>
> > ---
> >   drivers/gpu/drm/amd/display/dc/core/dc_link.c | 38 ++-
> >   1 file changed, 37 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c 
> > b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > index 7b18087be585..38dfe460e13b 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > @@ -2447,6 +2447,41 @@ bool dc_link_get_psr_state(const struct dc_link 
> > *link, uint32_t *psr_state)
> >   return true;
> >   }
> >
> > +static inline enum physical_phy_id
> > +transmitter_to_phy_id(enum transmitter transmitter_value)
> > +{
> > + switch (transmitter_value) {
> > + case TRANSMITTER_UNIPHY_A:
> > + return PHYLD_0;
> > + case TRANSMITTER_UNIPHY_B:
> > + return PHYLD_1;
> > + case TRANSMITTER_UNIPHY_C:
> > + return PHYLD_2;
> > + case TRANSMITTER_UNIPHY_D:
> > + return PHYLD_3;
> > + case TRANSMITTER_UNIPHY_E:
> > + return PHYLD_4;
> > + case TRANSMITTER_UNIPHY_F:
> > + return PHYLD_5;
> > + case TRANSMITTER_NUTMEG_CRT:
> > + return PHYLD_6;
> > + case TRANSMITTER_TRAVIS_CRT:
> > + return PHYLD_7;
> > + case TRANSMITTER_TRAVIS_LCD:
> > + return PHYLD_8;
> > + case TRANSMITTER_UNIPHY_G:
> > + return PHYLD_9;
> > + case TRANSMITTER_COUNT:
> > + return PHYLD_COUNT;
> > + case TRANSMITTER_UNKNOWN:
> > + return PHYLD_UNKNOWN;
> > + default:
> > + WARN_ONCE(1, "Unknown transmitter value %d\n",
> > +   transmitter_value);
> > + return PHYLD_0;
> > + }
> > +}
> > +
> >   bool dc_link_setup_psr(struct dc_link *link,
> >   const struct dc_stream_state *stream, struct psr_config 
> > *psr_config,
> >   struct psr_context *psr_context)
> > @@ -2517,7 +2552,8 @@ bool dc_link_setup_psr(struct dc_link *link,
> >   /* Hardcoded for now.  Can be Pcie or Uniphy (or Unknown)*/
> >   psr_context->phyType = PHY_TYPE_UNIPHY;
> >   /*PhyId is associated with the transmitter id*/
> > - psr_context->smuPhyId = link->link_enc->transmitter;
> > + psr_context->smuPhyId =
> > + transmitter_to_phy_id(link->link_enc->transmitter);
> >
> >   psr_context->crtcTimingVerticalTotal = stream->timing.v_total;
> >   psr_context->vsyncRateHz = div64_u64(div64_u64((stream->
> >
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/radeon: fix si_enable_smc_cac() failed issue

[Alex Deucher]

Need to set the dte flag on this asic.

Port the fix from amdgpu:
5cb818b861be114148e8dbeb4259698148019dd1

Signed-off-by: Alex Deucher 
---
 drivers/gpu/drm/radeon/si_dpm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/radeon/si_dpm.c b/drivers/gpu/drm/radeon/si_dpm.c
index 460fd98e40a7..a0b382a637a6 100644
--- a/drivers/gpu/drm/radeon/si_dpm.c
+++ b/drivers/gpu/drm/radeon/si_dpm.c
@@ -1958,6 +1958,7 @@ static void si_initialize_powertune_defaults(struct 
radeon_device *rdev)
case 0x682C:
si_pi->cac_weights = cac_weights_cape_verde_pro;
si_pi->dte_data = dte_data_sun_xt;
+   update_dte_from_pl2 = true;
break;
case 0x6825:
case 0x6827:
-- 
2.23.0

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

Re: [PATCH] drm/amdgpu: guard ib scheduling while in reset

[Grodzovsky, Andrey]

That good  as proof of RCA but I still think we should grab a dedicated 
lock inside scheduler since the race is internal to scheduler code so 
this better to handle it inside the scheduler code to make the fix apply 
for all drivers using it.

Andrey

On 10/30/19 4:44 AM, S, Shirish wrote:

 We still have it and isn't doing kthread_park()/unpark() from 
 drm_sched_entity_fini while GPU reset in progress defeats all the 
 purpose of drm_sched_stop->kthread_park ? If 
 drm_sched_entity_fini-> kthread_unpark happens AFTER 
 drm_sched_stop->kthread_park nothing prevents from another (third) 
 thread keep submitting job to HW which will be picked up by the 
 unparked scheduler thread try to submit to HW but fail because the 
 HW ring is deactivated.

 If so maybe we should serialize calls to 
 kthread_park/unpark(sched->thread) ?

>>>
>>> Yeah, that was my thinking as well. Probably best to just grab the 
>>> reset lock before calling drm_sched_entity_fini().
>>
>>
>> Shirish - please try locking &adev->lock_reset around calls to 
>> drm_sched_entity_fini as Christian suggests and see if this actually 
>> helps the issue.
>>
> Yes that also works.
>
> Regards,
>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: dont schedule jobs while in reset

[Grodzovsky, Andrey]

On 10/30/19 6:22 AM, S, Shirish wrote:
> On 10/30/2019 3:50 PM, Koenig, Christian wrote:
>> Am 30.10.19 um 10:13 schrieb S, Shirish:
>>> [Why]
>>>
>>> doing kthread_park()/unpark() from drm_sched_entity_fini
>>> while GPU reset is in progress defeats all the purpose of
>>> drm_sched_stop->kthread_park.
>>> If drm_sched_entity_fini->kthread_unpark() happens AFTER
>>> drm_sched_stop->kthread_park nothing prevents from another
>>> (third) thread to keep submitting job to HW which will be
>>> picked up by the unparked scheduler thread and try to submit
>>> to HW but fail because the HW ring is deactivated.
>>>
>>> [How]
>>> grab the reset lock before calling drm_sched_entity_fini()
>>>
>>> Signed-off-by: Shirish S 
>>> Suggested-by: Christian König 
>> Patch itself is Reviewed-by: Christian König 
>>
>> Does that also fix the problems you have been seeing?
> Yes Christian.
>
> Regards,
>
> Shirish S


Missed that one, why don't we fix it within scheduler code - the race is 
within scheduler ?

Andrey


>
>> Thanks,
>> Christian.
>>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 5 -
>>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> index 6614d8a..2cdaf3b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>>> @@ -604,8 +604,11 @@ void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr 
>>> *mgr)
>>> continue;
>>> }
>>> 
>>> -   for (i = 0; i < num_entities; i++)
>>> +   for (i = 0; i < num_entities; i++) {
>>> +   mutex_lock(&ctx->adev->lock_reset);
>>> 
>>> drm_sched_entity_fini(&ctx->entities[0][i].entity);
>>> +   mutex_unlock(&ctx->adev->lock_reset);
>>> +   }
>>> }
>>> }
>>> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: guard ib scheduling while in reset

[Christian König]

A lock inside the scheduler is rather tricky to implement.

What you need to do is to get rid of the park()/unpark() hack in 
drm_sched_entity_fini().


We could do this with a struct completion or convert the scheduler from 
a thread to a work item.


Regards,
Christian.

Am 30.10.19 um 15:44 schrieb Grodzovsky, Andrey:

That good  as proof of RCA but I still think we should grab a dedicated
lock inside scheduler since the race is internal to scheduler code so
this better to handle it inside the scheduler code to make the fix apply
for all drivers using it.

Andrey

On 10/30/19 4:44 AM, S, Shirish wrote:

We still have it and isn't doing kthread_park()/unpark() from
drm_sched_entity_fini while GPU reset in progress defeats all the
purpose of drm_sched_stop->kthread_park ? If
drm_sched_entity_fini-> kthread_unpark happens AFTER
drm_sched_stop->kthread_park nothing prevents from another (third)
thread keep submitting job to HW which will be picked up by the
unparked scheduler thread try to submit to HW but fail because the
HW ring is deactivated.

If so maybe we should serialize calls to
kthread_park/unpark(sched->thread) ?


Yeah, that was my thinking as well. Probably best to just grab the
reset lock before calling drm_sched_entity_fini().


Shirish - please try locking &adev->lock_reset around calls to
drm_sched_entity_fini as Christian suggests and see if this actually
helps the issue.


Yes that also works.

Regards,


___
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/amdgpu: guard ib scheduling while in reset

[Grodzovsky, Andrey]

Can you elaborate on what is the tricky part with the lock ? I assumed 
we just use per scheduler lock.

Andrey

On 10/30/19 10:50 AM, Christian König wrote:
> A lock inside the scheduler is rather tricky to implement.
>
> What you need to do is to get rid of the park()/unpark() hack in 
> drm_sched_entity_fini().
>
> We could do this with a struct completion or convert the scheduler 
> from a thread to a work item.
>
> Regards,
> Christian.
>
> Am 30.10.19 um 15:44 schrieb Grodzovsky, Andrey:
>> That good  as proof of RCA but I still think we should grab a dedicated
>> lock inside scheduler since the race is internal to scheduler code so
>> this better to handle it inside the scheduler code to make the fix apply
>> for all drivers using it.
>>
>> Andrey
>>
>> On 10/30/19 4:44 AM, S, Shirish wrote:
>> We still have it and isn't doing kthread_park()/unpark() from
>> drm_sched_entity_fini while GPU reset in progress defeats all the
>> purpose of drm_sched_stop->kthread_park ? If
>> drm_sched_entity_fini-> kthread_unpark happens AFTER
>> drm_sched_stop->kthread_park nothing prevents from another (third)
>> thread keep submitting job to HW which will be picked up by the
>> unparked scheduler thread try to submit to HW but fail because the
>> HW ring is deactivated.
>>
>> If so maybe we should serialize calls to
>> kthread_park/unpark(sched->thread) ?
>>
> Yeah, that was my thinking as well. Probably best to just grab the
> reset lock before calling drm_sched_entity_fini().

 Shirish - please try locking &adev->lock_reset around calls to
 drm_sched_entity_fini as Christian suggests and see if this actually
 helps the issue.

>>> Yes that also works.
>>>
>>> Regards,
>>>
>> ___
>> 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/amdgpu: guard ib scheduling while in reset

[Koenig, Christian]

Yeah, and exactly that's the problem :) You need a global lock covering 
all schedulers.

Otherwise you end up in hell's kitchen again with taking all those locks 
in the right order.

Christian.

Am 30.10.19 um 15:56 schrieb Grodzovsky, Andrey:
> Can you elaborate on what is the tricky part with the lock ? I assumed
> we just use per scheduler lock.
>
> Andrey
>
> On 10/30/19 10:50 AM, Christian König wrote:
>> A lock inside the scheduler is rather tricky to implement.
>>
>> What you need to do is to get rid of the park()/unpark() hack in
>> drm_sched_entity_fini().
>>
>> We could do this with a struct completion or convert the scheduler
>> from a thread to a work item.
>>
>> Regards,
>> Christian.
>>
>> Am 30.10.19 um 15:44 schrieb Grodzovsky, Andrey:
>>> That good  as proof of RCA but I still think we should grab a dedicated
>>> lock inside scheduler since the race is internal to scheduler code so
>>> this better to handle it inside the scheduler code to make the fix apply
>>> for all drivers using it.
>>>
>>> Andrey
>>>
>>> On 10/30/19 4:44 AM, S, Shirish wrote:
>>> We still have it and isn't doing kthread_park()/unpark() from
>>> drm_sched_entity_fini while GPU reset in progress defeats all the
>>> purpose of drm_sched_stop->kthread_park ? If
>>> drm_sched_entity_fini-> kthread_unpark happens AFTER
>>> drm_sched_stop->kthread_park nothing prevents from another (third)
>>> thread keep submitting job to HW which will be picked up by the
>>> unparked scheduler thread try to submit to HW but fail because the
>>> HW ring is deactivated.
>>>
>>> If so maybe we should serialize calls to
>>> kthread_park/unpark(sched->thread) ?
>>>
>> Yeah, that was my thinking as well. Probably best to just grab the
>> reset lock before calling drm_sched_entity_fini().
> Shirish - please try locking &adev->lock_reset around calls to
> drm_sched_entity_fini as Christian suggests and see if this actually
> helps the issue.
>
 Yes that also works.

 Regards,

>>> ___
>>> 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/amdgpu: guard ib scheduling while in reset

[Grodzovsky, Andrey]

I see.

OK, I will add to myself a TODO about struct completion approach.

Andrey

On 10/30/19 11:00 AM, Koenig, Christian wrote:
> Yeah, and exactly that's the problem :) You need a global lock covering
> all schedulers.
>
> Otherwise you end up in hell's kitchen again with taking all those locks
> in the right order.
>
> Christian.
>
> Am 30.10.19 um 15:56 schrieb Grodzovsky, Andrey:
>> Can you elaborate on what is the tricky part with the lock ? I assumed
>> we just use per scheduler lock.
>>
>> Andrey
>>
>> On 10/30/19 10:50 AM, Christian König wrote:
>>> A lock inside the scheduler is rather tricky to implement.
>>>
>>> What you need to do is to get rid of the park()/unpark() hack in
>>> drm_sched_entity_fini().
>>>
>>> We could do this with a struct completion or convert the scheduler
>>> from a thread to a work item.
>>>
>>> Regards,
>>> Christian.
>>>
>>> Am 30.10.19 um 15:44 schrieb Grodzovsky, Andrey:
 That good  as proof of RCA but I still think we should grab a dedicated
 lock inside scheduler since the race is internal to scheduler code so
 this better to handle it inside the scheduler code to make the fix apply
 for all drivers using it.

 Andrey

 On 10/30/19 4:44 AM, S, Shirish wrote:
 We still have it and isn't doing kthread_park()/unpark() from
 drm_sched_entity_fini while GPU reset in progress defeats all the
 purpose of drm_sched_stop->kthread_park ? If
 drm_sched_entity_fini-> kthread_unpark happens AFTER
 drm_sched_stop->kthread_park nothing prevents from another (third)
 thread keep submitting job to HW which will be picked up by the
 unparked scheduler thread try to submit to HW but fail because the
 HW ring is deactivated.

 If so maybe we should serialize calls to
 kthread_park/unpark(sched->thread) ?

>>> Yeah, that was my thinking as well. Probably best to just grab the
>>> reset lock before calling drm_sched_entity_fini().
>> Shirish - please try locking &adev->lock_reset around calls to
>> drm_sched_entity_fini as Christian suggests and see if this actually
>> helps the issue.
>>
> Yes that also works.
>
> Regards,
>
 ___
 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/amdgpu: dont schedule jobs while in reset

[Grodzovsky, Andrey]

Reviewed-by: Andrey Grodzovsky 

Andrey

On 10/30/19 6:20 AM, Koenig, Christian wrote:
> Am 30.10.19 um 10:13 schrieb S, Shirish:
>> [Why]
>>
>> doing kthread_park()/unpark() from drm_sched_entity_fini
>> while GPU reset is in progress defeats all the purpose of
>> drm_sched_stop->kthread_park.
>> If drm_sched_entity_fini->kthread_unpark() happens AFTER
>> drm_sched_stop->kthread_park nothing prevents from another
>> (third) thread to keep submitting job to HW which will be
>> picked up by the unparked scheduler thread and try to submit
>> to HW but fail because the HW ring is deactivated.
>>
>> [How]
>> grab the reset lock before calling drm_sched_entity_fini()
>>
>> Signed-off-by: Shirish S 
>> Suggested-by: Christian König 
> Patch itself is Reviewed-by: Christian König 
>
> Does that also fix the problems you have been seeing?
>
> Thanks,
> Christian.
>
>> ---
>>drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 5 -
>>1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> index 6614d8a..2cdaf3b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> @@ -604,8 +604,11 @@ void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr 
>> *mgr)
>>  continue;
>>  }
>>
>> -for (i = 0; i < num_entities; i++)
>> +for (i = 0; i < num_entities; i++) {
>> +mutex_lock(&ctx->adev->lock_reset);
>>  drm_sched_entity_fini(&ctx->entities[0][i].entity);
>> +mutex_unlock(&ctx->adev->lock_reset);
>> +}
>>  }
>>}
>>
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: remove PT BOs when unmapping

[Kuehling, Felix]

On 2019-10-30 9:52 a.m., Christian König wrote:
> Am 29.10.19 um 21:06 schrieb Huang, JinHuiEric:
>> The issue is PT BOs are not freed when unmapping VA,
>> which causes vram usage accumulated is huge in some
>> memory stress test, such as kfd big buffer stress test.
>> Function amdgpu_vm_bo_update_mapping() is called by both
>> amdgpu_vm_bo_update() and amdgpu_vm_clear_freed(). The
>> solution is replacing amdgpu_vm_bo_update_mapping() in
>> amdgpu_vm_clear_freed() with removing PT BOs function
>> to save vram usage.
>
> NAK, that is intentional behavior.
>
> Otherwise we can run into out of memory situations when page tables 
> need to be allocated again under stress.

That's a bit arbitrary and inconsistent. We are freeing page tables in 
other situations, when a mapping uses huge pages in 
amdgpu_vm_update_ptes. Why not when a mapping is destroyed completely?

I'm actually a bit surprised that the huge-page handling in 
amdgpu_vm_update_ptes isn't kicking in to free up lower-level page 
tables when a BO is unmapped.

Regards,
   Felix


>
> Regards,
> Christian.
>
>>
>> Change-Id: Ic24e35bff8ca85265b418a642373f189d972a924
>> Signed-off-by: Eric Huang 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56 
>> +-
>>   1 file changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 0f4c3b2..8a480c7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1930,6 +1930,51 @@ static void amdgpu_vm_prt_fini(struct 
>> amdgpu_device *adev, struct amdgpu_vm *vm)
>>   }
>>     /**
>> + * amdgpu_vm_remove_ptes - free PT BOs
>> + *
>> + * @adev: amdgpu device structure
>> + * @vm: amdgpu vm structure
>> + * @start: start of mapped range
>> + * @end: end of mapped entry
>> + *
>> + * Free the page table level.
>> + */
>> +static int amdgpu_vm_remove_ptes(struct amdgpu_device *adev,
>> +    struct amdgpu_vm *vm, uint64_t start, uint64_t end)
>> +{
>> +    struct amdgpu_vm_pt_cursor cursor;
>> +    unsigned shift, num_entries;
>> +
>> +    amdgpu_vm_pt_start(adev, vm, start, &cursor);
>> +    while (cursor.level < AMDGPU_VM_PTB) {
>> +    if (!amdgpu_vm_pt_descendant(adev, &cursor))
>> +    return -ENOENT;
>> +    }
>> +
>> +    while (cursor.pfn < end) {
>> +    amdgpu_vm_free_table(cursor.entry);
>> +    num_entries = amdgpu_vm_num_entries(adev, cursor.level - 1);
>> +
>> +    if (cursor.entry != &cursor.parent->entries[num_entries - 1]) {
>> +    /* Next ptb entry */
>> +    shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
>> +    cursor.pfn += 1ULL << shift;
>> +    cursor.pfn &= ~((1ULL << shift) - 1);
>> +    cursor.entry++;
>> +    } else {
>> +    /* Next ptb entry in next pd0 entry */
>> +    amdgpu_vm_pt_ancestor(&cursor);
>> +    shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
>> +    cursor.pfn += 1ULL << shift;
>> +    cursor.pfn &= ~((1ULL << shift) - 1);
>> +    amdgpu_vm_pt_descendant(adev, &cursor);
>> +    }
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +/**
>>    * amdgpu_vm_clear_freed - clear freed BOs in the PT
>>    *
>>    * @adev: amdgpu_device pointer
>> @@ -1949,7 +1994,6 @@ int amdgpu_vm_clear_freed(struct amdgpu_device 
>> *adev,
>>     struct dma_fence **fence)
>>   {
>>   struct amdgpu_bo_va_mapping *mapping;
>> -    uint64_t init_pte_value = 0;
>>   struct dma_fence *f = NULL;
>>   int r;
>>   @@ -1958,13 +2002,10 @@ int amdgpu_vm_clear_freed(struct 
>> amdgpu_device *adev,
>>   struct amdgpu_bo_va_mapping, list);
>>   list_del(&mapping->list);
>>   -    if (vm->pte_support_ats &&
>> -    mapping->start < AMDGPU_GMC_HOLE_START)
>> -    init_pte_value = AMDGPU_PTE_DEFAULT_ATC;
>> +    r = amdgpu_vm_remove_ptes(adev, vm,
>> +    (mapping->start + 0x1ff) & (~0x1ffll),
>> +    (mapping->last + 1) & (~0x1ffll));
>>   -    r = amdgpu_vm_bo_update_mapping(adev, vm, false, NULL,
>> -    mapping->start, mapping->last,
>> -    init_pte_value, 0, NULL, &f);
>>   amdgpu_vm_free_mapping(adev, vm, mapping, f);
>>   if (r) {
>>   dma_fence_put(f);
>> @@ -1980,7 +2021,6 @@ int amdgpu_vm_clear_freed(struct amdgpu_device 
>> *adev,
>>   }
>>     return 0;
>> -
>>   }
>>     /**
>
> ___
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH AUTOSEL 5.3 55/81] drm/amdgpu: fix potential VM faults

[Sasha Levin]

From: Christian König 

[ Upstream commit 3122051edc7c27cc08534be730f4c7c180919b8a ]

When we allocate new page tables under memory
pressure we should not evict old ones.

Signed-off-by: Christian König 
Acked-by: Alex Deucher 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index bea6f298dfdc5..0ff786dec8c4a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -421,7 +421,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
.interruptible = (bp->type != ttm_bo_type_kernel),
.no_wait_gpu = false,
.resv = bp->resv,
-   .flags = TTM_OPT_FLAG_ALLOW_RES_EVICT
+   .flags = bp->type != ttm_bo_type_kernel ?
+   TTM_OPT_FLAG_ALLOW_RES_EVICT : 0
};
struct amdgpu_bo *bo;
unsigned long page_align, size = bp->size;
-- 
2.20.1

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

[PATCH AUTOSEL 5.3 56/81] drm/amdgpu: fix error handling in amdgpu_bo_list_create

[Sasha Levin]

From: Christian König 

[ Upstream commit de51a5019ff32960218da8fd899fa3f361b031e9 ]

We need to drop normal and userptr BOs separately.

Signed-off-by: Christian König 
Acked-by: Huang Rui 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 7bcf86c619995..d4c4785ab5256 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -140,7 +140,12 @@ int amdgpu_bo_list_create(struct amdgpu_device *adev, 
struct drm_file *filp,
return 0;
 
 error_free:
-   while (i--) {
+   for (i = 0; i < last_entry; ++i) {
+   struct amdgpu_bo *bo = ttm_to_amdgpu_bo(array[i].tv.bo);
+
+   amdgpu_bo_unref(&bo);
+   }
+   for (i = first_userptr; i < num_entries; ++i) {
struct amdgpu_bo *bo = ttm_to_amdgpu_bo(array[i].tv.bo);
 
amdgpu_bo_unref(&bo);
-- 
2.20.1

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

[PATCH AUTOSEL 4.19 25/38] drm/amdgpu: fix potential VM faults

[Sasha Levin]

From: Christian König 

[ Upstream commit 3122051edc7c27cc08534be730f4c7c180919b8a ]

When we allocate new page tables under memory
pressure we should not evict old ones.

Signed-off-by: Christian König 
Acked-by: Alex Deucher 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index b0e14a3d54efd..b14ce112703f0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -428,7 +428,8 @@ static int amdgpu_bo_do_create(struct amdgpu_device *adev,
.interruptible = (bp->type != ttm_bo_type_kernel),
.no_wait_gpu = false,
.resv = bp->resv,
-   .flags = TTM_OPT_FLAG_ALLOW_RES_EVICT
+   .flags = bp->type != ttm_bo_type_kernel ?
+   TTM_OPT_FLAG_ALLOW_RES_EVICT : 0
};
struct amdgpu_bo *bo;
unsigned long page_align, size = bp->size;
-- 
2.20.1

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

Re: [PATCH] drm/amdgpu: remove PT BOs when unmapping

[Koenig, Christian]

Am 30.10.2019 16:47 schrieb "Kuehling, Felix" :
On 2019-10-30 9:52 a.m., Christian König wrote:
> Am 29.10.19 um 21:06 schrieb Huang, JinHuiEric:
>> The issue is PT BOs are not freed when unmapping VA,
>> which causes vram usage accumulated is huge in some
>> memory stress test, such as kfd big buffer stress test.
>> Function amdgpu_vm_bo_update_mapping() is called by both
>> amdgpu_vm_bo_update() and amdgpu_vm_clear_freed(). The
>> solution is replacing amdgpu_vm_bo_update_mapping() in
>> amdgpu_vm_clear_freed() with removing PT BOs function
>> to save vram usage.
>
> NAK, that is intentional behavior.
>
> Otherwise we can run into out of memory situations when page tables
> need to be allocated again under stress.

That's a bit arbitrary and inconsistent. We are freeing page tables in
other situations, when a mapping uses huge pages in
amdgpu_vm_update_ptes. Why not when a mapping is destroyed completely?

I'm actually a bit surprised that the huge-page handling in
amdgpu_vm_update_ptes isn't kicking in to free up lower-level page
tables when a BO is unmapped.

Well it does free the lower level, and that is already causing problems (that's 
why I added the reserved space).

What we don't do is freeing the higher levels.

E.g. when you free a 2MB BO we free the lowest level, if we free a 1GB BO we 
free the two lowest levels etc...

The problem with freeing the higher levels is that you don't know who is also 
using this. E.g. we would need to check all entries when we unmap one.

It's simply not worth it for a maximum saving of 2MB per VM.

Writing this I'm actually wondering how you ended up in this issue? There 
shouldn't be much savings from this.

Regards,
Christian.


Regards,
   Felix


>
> Regards,
> Christian.
>
>>
>> Change-Id: Ic24e35bff8ca85265b418a642373f189d972a924
>> Signed-off-by: Eric Huang 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56
>> +-
>>   1 file changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 0f4c3b2..8a480c7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1930,6 +1930,51 @@ static void amdgpu_vm_prt_fini(struct
>> amdgpu_device *adev, struct amdgpu_vm *vm)
>>   }
>> /**
>> + * amdgpu_vm_remove_ptes - free PT BOs
>> + *
>> + * @adev: amdgpu device structure
>> + * @vm: amdgpu vm structure
>> + * @start: start of mapped range
>> + * @end: end of mapped entry
>> + *
>> + * Free the page table level.
>> + */
>> +static int amdgpu_vm_remove_ptes(struct amdgpu_device *adev,
>> +struct amdgpu_vm *vm, uint64_t start, uint64_t end)
>> +{
>> +struct amdgpu_vm_pt_cursor cursor;
>> +unsigned shift, num_entries;
>> +
>> +amdgpu_vm_pt_start(adev, vm, start, &cursor);
>> +while (cursor.level < AMDGPU_VM_PTB) {
>> +if (!amdgpu_vm_pt_descendant(adev, &cursor))
>> +return -ENOENT;
>> +}
>> +
>> +while (cursor.pfn < end) {
>> +amdgpu_vm_free_table(cursor.entry);
>> +num_entries = amdgpu_vm_num_entries(adev, cursor.level - 1);
>> +
>> +if (cursor.entry != &cursor.parent->entries[num_entries - 1]) {
>> +/* Next ptb entry */
>> +shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
>> +cursor.pfn += 1ULL << shift;
>> +cursor.pfn &= ~((1ULL << shift) - 1);
>> +cursor.entry++;
>> +} else {
>> +/* Next ptb entry in next pd0 entry */
>> +amdgpu_vm_pt_ancestor(&cursor);
>> +shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
>> +cursor.pfn += 1ULL << shift;
>> +cursor.pfn &= ~((1ULL << shift) - 1);
>> +amdgpu_vm_pt_descendant(adev, &cursor);
>> +}
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +/**
>>* amdgpu_vm_clear_freed - clear freed BOs in the PT
>>*
>>* @adev: amdgpu_device pointer
>> @@ -1949,7 +1994,6 @@ int amdgpu_vm_clear_freed(struct amdgpu_device
>> *adev,
>> struct dma_fence **fence)
>>   {
>>   struct amdgpu_bo_va_mapping *mapping;
>> -uint64_t init_pte_value = 0;
>>   struct dma_fence *f = NULL;
>>   int r;
>>   @@ -1958,13 +2002,10 @@ int amdgpu_vm_clear_freed(struct
>> amdgpu_device *adev,
>>   struct amdgpu_bo_va_mapping, list);
>>   list_del(&mapping->list);
>>   -if (vm->pte_support_ats &&
>> -mapping->start < AMDGPU_GMC_HOLE_START)
>> -init_pte_value = AMDGPU_PTE_DEFAULT_ATC;
>> +r = amdgpu_vm_remove_ptes(adev, vm,
>> +(mapping->start + 0x1ff) & (~0x1ffll),
>> +(mapping->last + 1) & (~0x1ffll));
>>   -r = amdgpu_vm_bo_update_mapping(adev, vm, false, NULL,
>> -mapping->start, mapping->last,
>> -init_pte_value, 0, NULL, &f);
>>   amdgpu_vm_free_mapping(ade

Re: [PATCH] drm/amdgpu: remove PT BOs when unmapping

[Huang, JinHuiEric]

I tested it that it saves a lot of vram on KFD big buffer stress test. I think 
there are two reasons:

1. Calling amdgpu_vm_update_ptes() during unmapping will allocate unnecessary 
pts, because there is no flag to determine if the VA is mapping or unmapping in 
function
amdgpu_vm_update_ptes(). It saves the most of memory.

2. Intentionally removing those unmapping pts is logical expectation, although 
it is not removing so much pts.

Regards,

Eric

On 2019-10-30 11:57 a.m., Koenig, Christian wrote:


Am 30.10.2019 16:47 schrieb "Kuehling, Felix" 
:
On 2019-10-30 9:52 a.m., Christian König wrote:
> Am 29.10.19 um 21:06 schrieb Huang, JinHuiEric:
>> The issue is PT BOs are not freed when unmapping VA,
>> which causes vram usage accumulated is huge in some
>> memory stress test, such as kfd big buffer stress test.
>> Function amdgpu_vm_bo_update_mapping() is called by both
>> amdgpu_vm_bo_update() and amdgpu_vm_clear_freed(). The
>> solution is replacing amdgpu_vm_bo_update_mapping() in
>> amdgpu_vm_clear_freed() with removing PT BOs function
>> to save vram usage.
>
> NAK, that is intentional behavior.
>
> Otherwise we can run into out of memory situations when page tables
> need to be allocated again under stress.

That's a bit arbitrary and inconsistent. We are freeing page tables in
other situations, when a mapping uses huge pages in
amdgpu_vm_update_ptes. Why not when a mapping is destroyed completely?

I'm actually a bit surprised that the huge-page handling in
amdgpu_vm_update_ptes isn't kicking in to free up lower-level page
tables when a BO is unmapped.

Well it does free the lower level, and that is already causing problems (that's 
why I added the reserved space).

What we don't do is freeing the higher levels.

E.g. when you free a 2MB BO we free the lowest level, if we free a 1GB BO we 
free the two lowest levels etc...

The problem with freeing the higher levels is that you don't know who is also 
using this. E.g. we would need to check all entries when we unmap one.

It's simply not worth it for a maximum saving of 2MB per VM.

Writing this I'm actually wondering how you ended up in this issue? There 
shouldn't be much savings from this.

Regards,
Christian.


Regards,
   Felix


>
> Regards,
> Christian.
>
>>
>> Change-Id: Ic24e35bff8ca85265b418a642373f189d972a924
>> Signed-off-by: Eric Huang 
>> 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56
>> +-
>>   1 file changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 0f4c3b2..8a480c7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1930,6 +1930,51 @@ static void amdgpu_vm_prt_fini(struct
>> amdgpu_device *adev, struct amdgpu_vm *vm)
>>   }
>> /**
>> + * amdgpu_vm_remove_ptes - free PT BOs
>> + *
>> + * @adev: amdgpu device structure
>> + * @vm: amdgpu vm structure
>> + * @start: start of mapped range
>> + * @end: end of mapped entry
>> + *
>> + * Free the page table level.
>> + */
>> +static int amdgpu_vm_remove_ptes(struct amdgpu_device *adev,
>> +struct amdgpu_vm *vm, uint64_t start, uint64_t end)
>> +{
>> +struct amdgpu_vm_pt_cursor cursor;
>> +unsigned shift, num_entries;
>> +
>> +amdgpu_vm_pt_start(adev, vm, start, &cursor);
>> +while (cursor.level < AMDGPU_VM_PTB) {
>> +if (!amdgpu_vm_pt_descendant(adev, &cursor))
>> +return -ENOENT;
>> +}
>> +
>> +while (cursor.pfn < end) {
>> +amdgpu_vm_free_table(cursor.entry);
>> +num_entries = amdgpu_vm_num_entries(adev, cursor.level - 1);
>> +
>> +if (cursor.entry != &cursor.parent->entries[num_entries - 1]) {
>> +/* Next ptb entry */
>> +shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
>> +cursor.pfn += 1ULL << shift;
>> +cursor.pfn &= ~((1ULL << shift) - 1);
>> +cursor.entry++;
>> +} else {
>> +/* Next ptb entry in next pd0 entry */
>> +amdgpu_vm_pt_ancestor(&cursor);
>> +shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
>> +cursor.pfn += 1ULL << shift;
>> +cursor.pfn &= ~((1ULL << shift) - 1);
>> +amdgpu_vm_pt_descendant(adev, &cursor);
>> +}
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +/**
>>* amdgpu_vm_clear_freed - clear freed BOs in the PT
>>*
>>* @adev: amdgpu_device pointer
>> @@ -1949,7 +1994,6 @@ int amdgpu_vm_clear_freed(struct amdgpu_device
>> *adev,
>> struct dma_fence **fence)
>>   {
>>   struct amdgpu_bo_va_mapping *mapping;
>> -uint64_t init_pte_value = 0;
>>   struct dma_fence *f = NULL;
>>   int r;
>>   @@ -1958,13 +2002,10 @@ int amdgpu_vm_clear_freed(struct
>> amdgpu_device *adev,
>>   struct amdgpu_bo_va_mapping, list);
>> 

[pull] amdgpu, radeon, gpu scheduler drm-fixes-5.4

[Alex Deucher]

Hi Dave, Daniel,

Misc fixes for 5.4.

The following changes since commit 2a3608409f46e0bae5b6b1a77ddf4c42116698da:

  Merge tag 'drm-fixes-5.4-2019-10-23' of 
git://people.freedesktop.org/~agd5f/linux into drm-fixes (2019-10-25 14:48:53 
+1000)

are available in the Git repository at:

  git://people.freedesktop.org/~agd5f/linux tags/drm-fixes-5.4-2019-10-30

for you to fetch changes up to e8a170ff9a3576730e43c0dbdd27b7cd3dc56848:

  drm/amdgpu: enable -msse2 for GCC 7.1+ users (2019-10-30 11:56:20 -0400)


drm-fixes-5.4-2019-10-30:

amdgpu:
- clang fixes
- Updated golden settings
- GPUVM fixes for navi
- Navi sdma fix
- Navi display fixes
- Freesync fix
- Gamma fix for DCN
- DP dongle detection fix
- Fix for undervolting on vega10

radeon:
- enable kexec fix for PPC

scheduler:
- set an error on fence if hw job failed


Aidan Yang (1):
  drm/amd/display: Allow inverted gamma

Alex Deucher (1):
  drm/amdgpu/gmc10: properly set BANK_SELECT and FRAGMENT_SIZE

Andrey Grodzovsky (2):
  drm/sched: Set error to s_fence if HW job submission failed.
  drm/amdgpu: If amdgpu_ib_schedule fails return back the error.

Jun Lei (2):
  drm/amd/display: do not synchronize "drr" displays
  drm/amd/display: add 50us buffer as WA for pstate switch in active

Kyle Mahlkuch (1):
  drm/radeon: Fix EEH during kexec

Michael Strauss (1):
  drm/amd/display: Passive DP->HDMI dongle detection fix

Nick Desaulniers (3):
  drm/amdgpu: fix stack alignment ABI mismatch for Clang
  drm/amdgpu: fix stack alignment ABI mismatch for GCC 7.1+
  drm/amdgpu: enable -msse2 for GCC 7.1+ users

Pelle van Gils (1):
  drm/amdgpu/powerplay/vega10: allow undervolting in p7

Pierre-Eric Pelloux-Prayer (1):
  drm/amdgpu/sdma5: do not execute 0-sized IBs (v2)

Tianci.Yin (3):
  drm/amdgpu/gfx10: update gfx golden settings
  drm/amdgpu/gfx10: update gfx golden settings for navi14
  drm/amdgpu/gfx10: update gfx golden settings for navi12

Zhan liu (2):
  drm/amd/display: Change Navi14's DWB flag to 1
  drm/amd/display: setting the DIG_MODE to the correct value.

chen gong (1):
  drm/amdgpu: Fix SDMA hang when performing VKexample test

zhongshiqi (1):
  dc.c:use kzalloc without test

 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c|  4 +++-
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c |  6 +++---
 drivers/gpu/drm/amd/amdgpu/gfxhub_v2_0.c   |  9 
 drivers/gpu/drm/amd/amdgpu/gmc_v10_0.c |  1 +
 drivers/gpu/drm/amd/amdgpu/mmhub_v2_0.c|  9 
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c |  1 +
 drivers/gpu/drm/amd/display/dc/calcs/Makefile  | 19 ++---
 drivers/gpu/drm/amd/display/dc/core/dc.c   |  4 
 drivers/gpu/drm/amd/display/dc/core/dc_link.c  |  9 
 drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c  | 24 --
 drivers/gpu/drm/amd/display/dc/core/dc_resource.c  |  6 ++
 .../gpu/drm/amd/display/dc/dcn10/dcn10_cm_common.c | 22 
 drivers/gpu/drm/amd/display/dc/dcn20/Makefile  | 19 ++---
 .../gpu/drm/amd/display/dc/dcn20/dcn20_resource.c  |  2 +-
 drivers/gpu/drm/amd/display/dc/dcn21/Makefile  | 19 ++---
 drivers/gpu/drm/amd/display/dc/dml/Makefile| 19 ++---
 .../amd/display/dc/dml/dcn20/display_mode_vba_20.c |  3 ++-
 drivers/gpu/drm/amd/display/dc/dsc/Makefile| 19 ++---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_hwmgr.c |  4 +---
 drivers/gpu/drm/radeon/radeon_drv.c| 14 +
 drivers/gpu/drm/scheduler/sched_main.c | 19 ++---
 21 files changed, 165 insertions(+), 67 deletions(-)
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: remove PT BOs when unmapping

[Koenig, Christian]

Am 30.10.2019 17:19 schrieb "Huang, JinHuiEric" :

I tested it that it saves a lot of vram on KFD big buffer stress test. I think 
there are two reasons:

1. Calling amdgpu_vm_update_ptes() during unmapping will allocate unnecessary 
pts, because there is no flag to determine if the VA is mapping or unmapping in 
function
amdgpu_vm_update_ptes(). It saves the most of memory.

That's not correct. The valid flag is used for this.


2. Intentionally removing those unmapping pts is logical expectation, although 
it is not removing so much pts.

Well I actually don't see a change to what update_ptes is doing and have the 
strong suspicion that the patch is simply broken.

You either free page tables which are potentially still in use or update_pte 
doesn't free page tables when the valid but is not set.

Regards,
Christian.




Regards,

Eric

On 2019-10-30 11:57 a.m., Koenig, Christian wrote:


Am 30.10.2019 16:47 schrieb "Kuehling, Felix" 
:
On 2019-10-30 9:52 a.m., Christian König wrote:
> Am 29.10.19 um 21:06 schrieb Huang, JinHuiEric:
>> The issue is PT BOs are not freed when unmapping VA,
>> which causes vram usage accumulated is huge in some
>> memory stress test, such as kfd big buffer stress test.
>> Function amdgpu_vm_bo_update_mapping() is called by both
>> amdgpu_vm_bo_update() and amdgpu_vm_clear_freed(). The
>> solution is replacing amdgpu_vm_bo_update_mapping() in
>> amdgpu_vm_clear_freed() with removing PT BOs function
>> to save vram usage.
>
> NAK, that is intentional behavior.
>
> Otherwise we can run into out of memory situations when page tables
> need to be allocated again under stress.

That's a bit arbitrary and inconsistent. We are freeing page tables in
other situations, when a mapping uses huge pages in
amdgpu_vm_update_ptes. Why not when a mapping is destroyed completely?

I'm actually a bit surprised that the huge-page handling in
amdgpu_vm_update_ptes isn't kicking in to free up lower-level page
tables when a BO is unmapped.

Well it does free the lower level, and that is already causing problems (that's 
why I added the reserved space).

What we don't do is freeing the higher levels.

E.g. when you free a 2MB BO we free the lowest level, if we free a 1GB BO we 
free the two lowest levels etc...

The problem with freeing the higher levels is that you don't know who is also 
using this. E.g. we would need to check all entries when we unmap one.

It's simply not worth it for a maximum saving of 2MB per VM.

Writing this I'm actually wondering how you ended up in this issue? There 
shouldn't be much savings from this.

Regards,
Christian.


Regards,
   Felix


>
> Regards,
> Christian.
>
>>
>> Change-Id: Ic24e35bff8ca85265b418a642373f189d972a924
>> Signed-off-by: Eric Huang 
>> 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56
>> +-
>>   1 file changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 0f4c3b2..8a480c7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1930,6 +1930,51 @@ static void amdgpu_vm_prt_fini(struct
>> amdgpu_device *adev, struct amdgpu_vm *vm)
>>   }
>> /**
>> + * amdgpu_vm_remove_ptes - free PT BOs
>> + *
>> + * @adev: amdgpu device structure
>> + * @vm: amdgpu vm structure
>> + * @start: start of mapped range
>> + * @end: end of mapped entry
>> + *
>> + * Free the page table level.
>> + */
>> +static int amdgpu_vm_remove_ptes(struct amdgpu_device *adev,
>> +struct amdgpu_vm *vm, uint64_t start, uint64_t end)
>> +{
>> +struct amdgpu_vm_pt_cursor cursor;
>> +unsigned shift, num_entries;
>> +
>> +amdgpu_vm_pt_start(adev, vm, start, &cursor);
>> +while (cursor.level < AMDGPU_VM_PTB) {
>> +if (!amdgpu_vm_pt_descendant(adev, &cursor))
>> +return -ENOENT;
>> +}
>> +
>> +while (cursor.pfn < end) {
>> +amdgpu_vm_free_table(cursor.entry);
>> +num_entries = amdgpu_vm_num_entries(adev, cursor.level - 1);
>> +
>> +if (cursor.entry != &cursor.parent->entries[num_entries - 1]) {
>> +/* Next ptb entry */
>> +shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
>> +cursor.pfn += 1ULL << shift;
>> +cursor.pfn &= ~((1ULL << shift) - 1);
>> +cursor.entry++;
>> +} else {
>> +/* Next ptb entry in next pd0 entry */
>> +amdgpu_vm_pt_ancestor(&cursor);
>> +shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
>> +cursor.pfn += 1ULL << shift;
>> +cursor.pfn &= ~((1ULL << shift) - 1);
>> +amdgpu_vm_pt_descendant(adev, &cursor);
>> +}
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +/**
>>* amdgpu_vm_clear_freed - clear freed BOs in the PT
>>*
>>* @adev: amdgpu_device pointer
>> @@ -1949,7 +19

Re: [PATCH] drm/amdgpu: remove PT BOs when unmapping

[Huang, JinHuiEric]

The vaild flag doesn't take effect in this function. amdgpu_vm_alloc_pts() is 
always executed that only depended on "cursor.pfn < end". The valid flag has 
only been checked on here for asic below GMC v9:

if (adev->asic_type < CHIP_VEGA10 &&
(flags & AMDGPU_PTE_VALID))...

Regards,

Eric

On 2019-10-30 12:30 p.m., Koenig, Christian wrote:


Am 30.10.2019 17:19 schrieb "Huang, JinHuiEric" 
:

I tested it that it saves a lot of vram on KFD big buffer stress test. I think 
there are two reasons:

1. Calling amdgpu_vm_update_ptes() during unmapping will allocate unnecessary 
pts, because there is no flag to determine if the VA is mapping or unmapping in 
function
amdgpu_vm_update_ptes(). It saves the most of memory.

That's not correct. The valid flag is used for this.


2. Intentionally removing those unmapping pts is logical expectation, although 
it is not removing so much pts.

Well I actually don't see a change to what update_ptes is doing and have the 
strong suspicion that the patch is simply broken.

You either free page tables which are potentially still in use or update_pte 
doesn't free page tables when the valid but is not set.

Regards,
Christian.




Regards,

Eric

On 2019-10-30 11:57 a.m., Koenig, Christian wrote:


Am 30.10.2019 16:47 schrieb "Kuehling, Felix" 
:
On 2019-10-30 9:52 a.m., Christian König wrote:
> Am 29.10.19 um 21:06 schrieb Huang, JinHuiEric:
>> The issue is PT BOs are not freed when unmapping VA,
>> which causes vram usage accumulated is huge in some
>> memory stress test, such as kfd big buffer stress test.
>> Function amdgpu_vm_bo_update_mapping() is called by both
>> amdgpu_vm_bo_update() and amdgpu_vm_clear_freed(). The
>> solution is replacing amdgpu_vm_bo_update_mapping() in
>> amdgpu_vm_clear_freed() with removing PT BOs function
>> to save vram usage.
>
> NAK, that is intentional behavior.
>
> Otherwise we can run into out of memory situations when page tables
> need to be allocated again under stress.

That's a bit arbitrary and inconsistent. We are freeing page tables in
other situations, when a mapping uses huge pages in
amdgpu_vm_update_ptes. Why not when a mapping is destroyed completely?

I'm actually a bit surprised that the huge-page handling in
amdgpu_vm_update_ptes isn't kicking in to free up lower-level page
tables when a BO is unmapped.

Well it does free the lower level, and that is already causing problems (that's 
why I added the reserved space).

What we don't do is freeing the higher levels.

E.g. when you free a 2MB BO we free the lowest level, if we free a 1GB BO we 
free the two lowest levels etc...

The problem with freeing the higher levels is that you don't know who is also 
using this. E.g. we would need to check all entries when we unmap one.

It's simply not worth it for a maximum saving of 2MB per VM.

Writing this I'm actually wondering how you ended up in this issue? There 
shouldn't be much savings from this.

Regards,
Christian.


Regards,
   Felix


>
> Regards,
> Christian.
>
>>
>> Change-Id: Ic24e35bff8ca85265b418a642373f189d972a924
>> Signed-off-by: Eric Huang 
>> 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56
>> +-
>>   1 file changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> index 0f4c3b2..8a480c7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> @@ -1930,6 +1930,51 @@ static void amdgpu_vm_prt_fini(struct
>> amdgpu_device *adev, struct amdgpu_vm *vm)
>>   }
>> /**
>> + * amdgpu_vm_remove_ptes - free PT BOs
>> + *
>> + * @adev: amdgpu device structure
>> + * @vm: amdgpu vm structure
>> + * @start: start of mapped range
>> + * @end: end of mapped entry
>> + *
>> + * Free the page table level.
>> + */
>> +static int amdgpu_vm_remove_ptes(struct amdgpu_device *adev,
>> +struct amdgpu_vm *vm, uint64_t start, uint64_t end)
>> +{
>> +struct amdgpu_vm_pt_cursor cursor;
>> +unsigned shift, num_entries;
>> +
>> +amdgpu_vm_pt_start(adev, vm, start, &cursor);
>> +while (cursor.level < AMDGPU_VM_PTB) {
>> +if (!amdgpu_vm_pt_descendant(adev, &cursor))
>> +return -ENOENT;
>> +}
>> +
>> +while (cursor.pfn < end) {
>> +amdgpu_vm_free_table(cursor.entry);
>> +num_entries = amdgpu_vm_num_entries(adev, cursor.level - 1);
>> +
>> +if (cursor.entry != &cursor.parent->entries[num_entries - 1]) {
>> +/* Next ptb entry */
>> +shift = amdgpu_vm_level_shift(adev, cursor.level - 1);
>> +cursor.pfn += 1ULL << shift;
>> +cursor.pfn &= ~((1ULL << shift) - 1);
>> +cursor.entry++;
>> +} else {
>> +/* Next ptb entry in next pd0 entry */
>> +amdgpu_vm_pt_ancestor(&cursor);
>> +shift =

[PATCH] drm/amdgpu/arcturus: properly set BANK_SELECT and FRAGMENT_SIZE

[Alex Deucher]

These were not aligned for optimal performance for GPUVM.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c 
b/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c
index 657970f9ebfb..2c5adfe803a2 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c
@@ -219,6 +219,15 @@ static void mmhub_v9_4_init_cache_regs(struct 
amdgpu_device *adev, int hubid)
hubid * MMHUB_INSTANCE_REGISTER_OFFSET, tmp);
 
tmp = mmVML2PF0_VM_L2_CNTL3_DEFAULT;
+   if (adev->gmc.translate_further) {
+   tmp = REG_SET_FIELD(tmp, VML2PF0_VM_L2_CNTL3, BANK_SELECT, 12);
+   tmp = REG_SET_FIELD(tmp, VML2PF0_VM_L2_CNTL3,
+   L2_CACHE_BIGK_FRAGMENT_SIZE, 9);
+   } else {
+   tmp = REG_SET_FIELD(tmp, VML2PF0_VM_L2_CNTL3, BANK_SELECT, 9);
+   tmp = REG_SET_FIELD(tmp, VML2PF0_VM_L2_CNTL3,
+   L2_CACHE_BIGK_FRAGMENT_SIZE, 6);
+   }
WREG32_SOC15_OFFSET(MMHUB, 0, mmVML2PF0_VM_L2_CNTL3,
hubid * MMHUB_INSTANCE_REGISTER_OFFSET, tmp);
 
-- 
2.23.0

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

Re: [PATCH v2 09/15] xen/gntdev: use mmu_range_notifier_insert

[Boris Ostrovsky]

On 10/28/19 4:10 PM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe 
>
> gntdev simply wants to monitor a specific VMA for any notifier events,
> this can be done straightforwardly using mmu_range_notifier_insert() over
> the VMA's VA range.
>
> The notifier should be attached until the original VMA is destroyed.
>
> It is unclear if any of this is even sane, but at least a lot of duplicate
> code is removed.

I didn't have a chance to look at the patch itself yet but as a heads-up
--- it crashes dom0.

-boris


>
> Cc: Oleksandr Andrushchenko 
> Cc: Boris Ostrovsky 
> Cc: xen-de...@lists.xenproject.org
> Cc: Juergen Gross 
> Cc: Stefano Stabellini 
> Signed-off-by: Jason Gunthorpe 
> ---
>  drivers/xen/gntdev-common.h |   8 +-
>  drivers/xen/gntdev.c| 180 ++--
>  2 files changed, 49 insertions(+), 139 deletions(-)
>
> diff --git a/drivers/xen/gntdev-common.h b/drivers/xen/gntdev-common.h
> index 2f8b949c3eeb14..b201fdd20b667b 100644
> --- a/drivers/xen/gntdev-common.h
> +++ b/drivers/xen/gntdev-common.h
> @@ -21,15 +21,8 @@ struct gntdev_dmabuf_priv;
>  struct gntdev_priv {
>   /* Maps with visible offsets in the file descriptor. */
>   struct list_head maps;
> - /*
> -  * Maps that are not visible; will be freed on munmap.
> -  * Only populated if populate_freeable_maps == 1
> -  */
> - struct list_head freeable_maps;
>   /* lock protects maps and freeable_maps. */
>   struct mutex lock;
> - struct mm_struct *mm;
> - struct mmu_notifier mn;
>  
>  #ifdef CONFIG_XEN_GRANT_DMA_ALLOC
>   /* Device for which DMA memory is allocated. */
> @@ -49,6 +42,7 @@ struct gntdev_unmap_notify {
>  };
>  
>  struct gntdev_grant_map {
> + struct mmu_range_notifier notifier;
>   struct list_head next;
>   struct vm_area_struct *vma;
>   int index;
> diff --git a/drivers/xen/gntdev.c b/drivers/xen/gntdev.c
> index a446a7221e13e9..12d626670bebbc 100644
> --- a/drivers/xen/gntdev.c
> +++ b/drivers/xen/gntdev.c
> @@ -65,7 +65,6 @@ MODULE_PARM_DESC(limit, "Maximum number of grants that may 
> be mapped by "
>  static atomic_t pages_mapped = ATOMIC_INIT(0);
>  
>  static int use_ptemod;
> -#define populate_freeable_maps use_ptemod
>  
>  static int unmap_grant_pages(struct gntdev_grant_map *map,
>int offset, int pages);
> @@ -251,12 +250,6 @@ void gntdev_put_map(struct gntdev_priv *priv, struct 
> gntdev_grant_map *map)
>   evtchn_put(map->notify.event);
>   }
>  
> - if (populate_freeable_maps && priv) {
> - mutex_lock(&priv->lock);
> - list_del(&map->next);
> - mutex_unlock(&priv->lock);
> - }
> -
>   if (map->pages && !use_ptemod)
>   unmap_grant_pages(map, 0, map->count);
>   gntdev_free_map(map);
> @@ -445,17 +438,9 @@ static void gntdev_vma_close(struct vm_area_struct *vma)
>   struct gntdev_priv *priv = file->private_data;
>  
>   pr_debug("gntdev_vma_close %p\n", vma);
> - if (use_ptemod) {
> - /* It is possible that an mmu notifier could be running
> -  * concurrently, so take priv->lock to ensure that the vma won't
> -  * vanishing during the unmap_grant_pages call, since we will
> -  * spin here until that completes. Such a concurrent call will
> -  * not do any unmapping, since that has been done prior to
> -  * closing the vma, but it may still iterate the unmap_ops list.
> -  */
> - mutex_lock(&priv->lock);
> + if (use_ptemod && map->vma == vma) {
> + mmu_range_notifier_remove(&map->notifier);
>   map->vma = NULL;
> - mutex_unlock(&priv->lock);
>   }
>   vma->vm_private_data = NULL;
>   gntdev_put_map(priv, map);
> @@ -477,109 +462,44 @@ static const struct vm_operations_struct gntdev_vmops 
> = {
>  
>  /* -- */
>  
> -static bool in_range(struct gntdev_grant_map *map,
> -   unsigned long start, unsigned long end)
> -{
> - if (!map->vma)
> - return false;
> - if (map->vma->vm_start >= end)
> - return false;
> - if (map->vma->vm_end <= start)
> - return false;
> -
> - return true;
> -}
> -
> -static int unmap_if_in_range(struct gntdev_grant_map *map,
> -   unsigned long start, unsigned long end,
> -   bool blockable)
> +static bool gntdev_invalidate(struct mmu_range_notifier *mn,
> +   const struct mmu_notifier_range *range,
> +   unsigned long cur_seq)
>  {
> + struct gntdev_grant_map *map =
> + container_of(mn, struct gntdev_grant_map, notifier);
>   unsigned long mstart, mend;
>   int err;
>  
> - if (!in_range(map, start, end))
> - return 0;
> + if (!mmu_notifier_range_b

Re: [PATCH] drm/amdgpu: remove PT BOs when unmapping

[Koenig, Christian]

The vaild flag doesn't take effect in this function.
That's irrelevant.

See what amdgpu_vm_update_ptes() does is to first determine the fragment size:
amdgpu_vm_fragment(params, frag_start, end, flags, &frag, &frag_end);

Then we walk down the tree:
amdgpu_vm_pt_start(adev, params->vm, start, &cursor);
while (cursor.pfn < end) {

And make sure that the page tables covering the address range are actually 
allocated:
r = amdgpu_vm_alloc_pts(params->adev, params->vm, &cursor);

Then we update the tables with the flags and addresses and free up subsequent 
tables in the case of huge pages or freed up areas:
/* Free all child entries */
while (cursor.pfn < frag_start) {
amdgpu_vm_free_pts(adev, params->vm, &cursor);
amdgpu_vm_pt_next(adev, &cursor);
}

This is the maximum you can free, cause all other page tables are not 
completely covered by the range and so potentially still in use.

And I have the strong suspicion that this is what your patch is actually doing 
wrong. In other words you are also freeing page tables which are only partially 
covered by the range and so potentially still in use.

Since we don't have any tracking how many entries in a page table are currently 
valid and how many are invalid we actually can't implement what you are trying 
to do here. So the patch is definitely somehow broken.

Regards,
Christian.

Am 30.10.19 um 17:55 schrieb Huang, JinHuiEric:

The vaild flag doesn't take effect in this function. amdgpu_vm_alloc_pts() is 
always executed that only depended on "cursor.pfn < end". The valid flag has 
only been checked on here for asic below GMC v9:

if (adev->asic_type < CHIP_VEGA10 &&
(flags & AMDGPU_PTE_VALID))...

Regards,

Eric

On 2019-10-30 12:30 p.m., Koenig, Christian wrote:


Am 30.10.2019 17:19 schrieb "Huang, JinHuiEric" 
:

I tested it that it saves a lot of vram on KFD big buffer stress test. I think 
there are two reasons:

1. Calling amdgpu_vm_update_ptes() during unmapping will allocate unnecessary 
pts, because there is no flag to determine if the VA is mapping or unmapping in 
function
amdgpu_vm_update_ptes(). It saves the most of memory.

That's not correct. The valid flag is used for this.


2. Intentionally removing those unmapping pts is logical expectation, although 
it is not removing so much pts.

Well I actually don't see a change to what update_ptes is doing and have the 
strong suspicion that the patch is simply broken.

You either free page tables which are potentially still in use or update_pte 
doesn't free page tables when the valid but is not set.

Regards,
Christian.




Regards,

Eric

On 2019-10-30 11:57 a.m., Koenig, Christian wrote:


Am 30.10.2019 16:47 schrieb "Kuehling, Felix" 
:
On 2019-10-30 9:52 a.m., Christian König wrote:
> Am 29.10.19 um 21:06 schrieb Huang, JinHuiEric:
>> The issue is PT BOs are not freed when unmapping VA,
>> which causes vram usage accumulated is huge in some
>> memory stress test, such as kfd big buffer stress test.
>> Function amdgpu_vm_bo_update_mapping() is called by both
>> amdgpu_vm_bo_update() and amdgpu_vm_clear_freed(). The
>> solution is replacing amdgpu_vm_bo_update_mapping() in
>> amdgpu_vm_clear_freed() with removing PT BOs function
>> to save vram usage.
>
> NAK, that is intentional behavior.
>
> Otherwise we can run into out of memory situations when page tables
> need to be allocated again under stress.

That's a bit arbitrary and inconsistent. We are freeing page tables in
other situations, when a mapping uses huge pages in
amdgpu_vm_update_ptes. Why not when a mapping is destroyed completely?

I'm actually a bit surprised that the huge-page handling in
amdgpu_vm_update_ptes isn't kicking in to free up lower-level page
tables when a BO is unmapped.

Well it does free the lower level, and that is already causing problems (that's 
why I added the reserved space).

What we don't do is freeing the higher levels.

E.g. when you free a 2MB BO we free the lowest level, if we free a 1GB BO we 
free the two lowest levels etc...

The problem with freeing the higher levels is that you don't know who is also 
using this. E.g. we would need to check all entries when we unmap one.

It's simply not worth it for a maximum saving of 2MB per VM.

Writing this I'm actually wondering how you ended up in this issue? There 
shouldn't be much savings from this.

Regards,
Christian.


Regards,
   Felix


>
> Regards,
> Christian.
>
>>
>> Change-Id: Ic24e35bff8ca85265b418a642373f189d972a924
>> Signed-off-by: Eric Huang 
>> 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 56
>> +-
>>   1 file changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>> b/drivers/gpu/drm/

Re: [PATCH] drm/amdgpu/arcturus: properly set BANK_SELECT and FRAGMENT_SIZE

[Christian König]

Am 30.10.19 um 18:32 schrieb Alex Deucher:

These were not aligned for optimal performance for GPUVM.

Signed-off-by: Alex Deucher 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c 
b/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c
index 657970f9ebfb..2c5adfe803a2 100644
--- a/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c
+++ b/drivers/gpu/drm/amd/amdgpu/mmhub_v9_4.c
@@ -219,6 +219,15 @@ static void mmhub_v9_4_init_cache_regs(struct 
amdgpu_device *adev, int hubid)
hubid * MMHUB_INSTANCE_REGISTER_OFFSET, tmp);
  
  	tmp = mmVML2PF0_VM_L2_CNTL3_DEFAULT;

+   if (adev->gmc.translate_further) {
+   tmp = REG_SET_FIELD(tmp, VML2PF0_VM_L2_CNTL3, BANK_SELECT, 12);
+   tmp = REG_SET_FIELD(tmp, VML2PF0_VM_L2_CNTL3,
+   L2_CACHE_BIGK_FRAGMENT_SIZE, 9);
+   } else {
+   tmp = REG_SET_FIELD(tmp, VML2PF0_VM_L2_CNTL3, BANK_SELECT, 9);
+   tmp = REG_SET_FIELD(tmp, VML2PF0_VM_L2_CNTL3,
+   L2_CACHE_BIGK_FRAGMENT_SIZE, 6);
+   }
WREG32_SOC15_OFFSET(MMHUB, 0, mmVML2PF0_VM_L2_CNTL3,
hubid * MMHUB_INSTANCE_REGISTER_OFFSET, tmp);
  


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

Re: [PATCH] drm/amdgpu: remove PT BOs when unmapping

[Christian König]

One thing I've forgotten:

What you could maybe do to improve the situation is to join adjacent 
ranges in amdgpu_vm_clear_freed(), but I'm not sure how the chances are 
that the ranges are freed all together.


The only other alternative I can see would be to check the mappings of a 
range in amdgpu_update_ptes() and see if you could walk the tree up if 
the valid flag is not set and there are no mappings left for a page table.


Regards,
Christian.

Am 30.10.19 um 18:42 schrieb Koenig, Christian:

The vaild flag doesn't take effect in this function.

That's irrelevant.

See what amdgpu_vm_update_ptes() does is to first determine the 
fragment size:

amdgpu_vm_fragment(params, frag_start, end, flags, &frag, &frag_end);


Then we walk down the tree:

    amdgpu_vm_pt_start(adev, params->vm, start, &cursor);
    while (cursor.pfn < end) {


And make sure that the page tables covering the address range are 
actually allocated:
    r = amdgpu_vm_alloc_pts(params->adev, params->vm, 
&cursor);


Then we update the tables with the flags and addresses and free up 
subsequent tables in the case of huge pages or freed up areas:

    /* Free all child entries */
    while (cursor.pfn < frag_start) {
    amdgpu_vm_free_pts(adev, params->vm, 
&cursor);

    amdgpu_vm_pt_next(adev, &cursor);
    }


This is the maximum you can free, cause all other page tables are not 
completely covered by the range and so potentially still in use.


And I have the strong suspicion that this is what your patch is 
actually doing wrong. In other words you are also freeing page tables 
which are only partially covered by the range and so potentially still 
in use.


Since we don't have any tracking how many entries in a page table are 
currently valid and how many are invalid we actually can't implement 
what you are trying to do here. So the patch is definitely somehow broken.


Regards,
Christian.

Am 30.10.19 um 17:55 schrieb Huang, JinHuiEric:


The vaild flag doesn't take effect in this function. 
amdgpu_vm_alloc_pts() is always executed that only depended on 
"cursor.pfn < end". The valid flag has only been checked on here for 
asic below GMC v9:


if (adev->asic_type < CHIP_VEGA10 &&
            (flags & AMDGPU_PTE_VALID))...

Regards,

Eric

On 2019-10-30 12:30 p.m., Koenig, Christian wrote:



Am 30.10.2019 17:19 schrieb "Huang, JinHuiEric" 
:


I tested it that it saves a lot of vram on KFD big buffer stress
test. I think there are two reasons:

1. Calling amdgpu_vm_update_ptes() during unmapping will
allocate unnecessary pts, because there is no flag to determine
if the VA is mapping or unmapping in function
amdgpu_vm_update_ptes(). It saves the most of memory.

That's not correct. The valid flag is used for this.

2. Intentionally removing those unmapping pts is logical
expectation, although it is not removing so much pts.

Well I actually don't see a change to what update_ptes is doing and 
have the strong suspicion that the patch is simply broken.


You either free page tables which are potentially still in use or 
update_pte doesn't free page tables when the valid but is not set.


Regards,
Christian.



Regards,

Eric

On 2019-10-30 11:57 a.m., Koenig, Christian wrote:



Am 30.10.2019 16:47 schrieb "Kuehling, Felix"
 :

On 2019-10-30 9:52 a.m., Christian König wrote:
> Am 29.10.19 um 21:06 schrieb Huang, JinHuiEric:
>> The issue is PT BOs are not freed when unmapping VA,
>> which causes vram usage accumulated is huge in some
>> memory stress test, such as kfd big buffer stress test.
>> Function amdgpu_vm_bo_update_mapping() is called by both
>> amdgpu_vm_bo_update() and amdgpu_vm_clear_freed(). The
>> solution is replacing amdgpu_vm_bo_update_mapping() in
>> amdgpu_vm_clear_freed() with removing PT BOs function
>> to save vram usage.
>
> NAK, that is intentional behavior.
>
> Otherwise we can run into out of memory situations
when page tables
> need to be allocated again under stress.

That's a bit arbitrary and inconsistent. We are freeing
page tables in
other situations, when a mapping uses huge pages in
amdgpu_vm_update_ptes. Why not when a mapping is
destroyed completely?

I'm actually a bit surprised that the huge-page handling in
amdgpu_vm_update_ptes isn't kicking in to free up
lower-level page
tables when a BO is unmapped.


Well it does free the lower level, and that is already
causing problems (that's why I added the reserved space).

What we don't do is freeing the hi

Re: [PATCH] drm/amdgpu: remove PT BOs when unmapping

[Huang, JinHuiEric]

Actually I do prevent to remove in-use pts by this:

+   r = amdgpu_vm_remove_ptes(adev, vm,
+   (mapping->start + 0x1ff) & (~0x1ffll),
+   (mapping->last + 1) & (~0x1ffll));

Which is only removing aligned page table for 2M. And I have tested it at least 
on KFD tests without anything broken.

By the way, I am not familiar with memory staff. This patch is the best I can 
do for now. Could you take a look at the Jira ticket SWDEV-201443 ? and find 
the better solution. Thanks!

Regards,

Eric

On 2019-10-30 1:57 p.m., Christian König wrote:
One thing I've forgotten:

What you could maybe do to improve the situation is to join adjacent ranges in 
amdgpu_vm_clear_freed(), but I'm not sure how the chances are that the ranges 
are freed all together.

The only other alternative I can see would be to check the mappings of a range 
in amdgpu_update_ptes() and see if you could walk the tree up if the valid flag 
is not set and there are no mappings left for a page table.

Regards,
Christian.

Am 30.10.19 um 18:42 schrieb Koenig, Christian:
The vaild flag doesn't take effect in this function.
That's irrelevant.

See what amdgpu_vm_update_ptes() does is to first determine the fragment size:
amdgpu_vm_fragment(params, frag_start, end, flags, &frag, &frag_end);

Then we walk down the tree:
amdgpu_vm_pt_start(adev, params->vm, start, &cursor);
while (cursor.pfn < end) {

And make sure that the page tables covering the address range are actually 
allocated:
r = amdgpu_vm_alloc_pts(params->adev, params->vm, &cursor);

Then we update the tables with the flags and addresses and free up subsequent 
tables in the case of huge pages or freed up areas:
/* Free all child entries */
while (cursor.pfn < frag_start) {
amdgpu_vm_free_pts(adev, params->vm, &cursor);
amdgpu_vm_pt_next(adev, &cursor);
}

This is the maximum you can free, cause all other page tables are not 
completely covered by the range and so potentially still in use.

And I have the strong suspicion that this is what your patch is actually doing 
wrong. In other words you are also freeing page tables which are only partially 
covered by the range and so potentially still in use.

Since we don't have any tracking how many entries in a page table are currently 
valid and how many are invalid we actually can't implement what you are trying 
to do here. So the patch is definitely somehow broken.

Regards,
Christian.

Am 30.10.19 um 17:55 schrieb Huang, JinHuiEric:

The vaild flag doesn't take effect in this function. amdgpu_vm_alloc_pts() is 
always executed that only depended on "cursor.pfn < end". The valid flag has 
only been checked on here for asic below GMC v9:

if (adev->asic_type < CHIP_VEGA10 &&
(flags & AMDGPU_PTE_VALID))...

Regards,

Eric

On 2019-10-30 12:30 p.m., Koenig, Christian wrote:


Am 30.10.2019 17:19 schrieb "Huang, JinHuiEric" 
:

I tested it that it saves a lot of vram on KFD big buffer stress test. I think 
there are two reasons:

1. Calling amdgpu_vm_update_ptes() during unmapping will allocate unnecessary 
pts, because there is no flag to determine if the VA is mapping or unmapping in 
function
amdgpu_vm_update_ptes(). It saves the most of memory.

That's not correct. The valid flag is used for this.


2. Intentionally removing those unmapping pts is logical expectation, although 
it is not removing so much pts.

Well I actually don't see a change to what update_ptes is doing and have the 
strong suspicion that the patch is simply broken.

You either free page tables which are potentially still in use or update_pte 
doesn't free page tables when the valid but is not set.

Regards,
Christian.




Regards,

Eric

On 2019-10-30 11:57 a.m., Koenig, Christian wrote:


Am 30.10.2019 16:47 schrieb "Kuehling, Felix" 
:
On 2019-10-30 9:52 a.m., Christian König wrote:
> Am 29.10.19 um 21:06 schrieb Huang, JinHuiEric:
>> The issue is PT BOs are not freed when unmapping VA,
>> which causes vram usage accumulated is huge in some
>> memory stress test, such as kfd big buffer stress test.
>> Function amdgpu_vm_bo_update_mapping() is called by both
>> amdgpu_vm_bo_update() and amdgpu_vm_clear_freed(). The
>> solution is replacing amdgpu_vm_bo_update_mapping() in
>> amdgpu_vm_clear_freed() with removing PT BOs function
>> to save vram usage.
>
> NAK, that is intentional behavior.
>
> Otherwise we can run into out of memory situations when page tables
> need to be allocated again under stress.

That's a bit arbitrary and inconsistent. We are freeing page tables in
other situations, when a mapping uses huge pages in
amdgpu_vm_update_ptes. Why not when a mapping is destroyed completely?

I'm actually a bit surprised that the huge-page handling in
amdgpu

Re: 21:9 monitor resolution incorrect since 4.14 kernel

[Neil Mayhew]

I enabled kms debugging with drm.debug=4 on the kernel command line.
This enabled me to see that the relevant modes are being rejected with
error 11 which is DC_EXCEED_DONGLE_CAP. Sure enough, I see that there's
a dongle detected that's rated at 165MHz and my preferred modes are all
above that.

However, if I switch the monitors around everything is fine, because I
can use a lower resolution monitor with the dongle and get the 21:9
resolution on another output without a dongle.

The dongle is actually a passive DP to HDMI cable, and I need it because
one of my outputs is DP and I don't have any monitors with DP inputs. I
was told somewhere that I'd need to use the DP output to get the clock
speeds needed for 21:9, but apparently that's not true. Clearly the
cable can go above 165MHz since the 4.14 kernel can drive the display at
the preferred resolution using it, but thankfully I don't need to
concern myself with that now.
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH] drm/amdgpu: remove PT BOs when unmapping

[Koenig, Christian]

Then I don't see how this patch actually changes anything.

Could only be a bug in amdgpu_vm_update_ptes(). Going to investigate this, but 
I won't have time to look into the ticket in detail.

Regards,
Christian.

Am 30.10.19 um 19:00 schrieb Huang, JinHuiEric:

Actually I do prevent to remove in-use pts by this:

+   r = amdgpu_vm_remove_ptes(adev, vm,
+   (mapping->start + 0x1ff) & (~0x1ffll),
+   (mapping->last + 1) & (~0x1ffll));

Which is only removing aligned page table for 2M. And I have tested it at least 
on KFD tests without anything broken.

By the way, I am not familiar with memory staff. This patch is the best I can 
do for now. Could you take a look at the Jira ticket SWDEV-201443 ? and find 
the better solution. Thanks!

Regards,

Eric

On 2019-10-30 1:57 p.m., Christian König wrote:
One thing I've forgotten:

What you could maybe do to improve the situation is to join adjacent ranges in 
amdgpu_vm_clear_freed(), but I'm not sure how the chances are that the ranges 
are freed all together.

The only other alternative I can see would be to check the mappings of a range 
in amdgpu_update_ptes() and see if you could walk the tree up if the valid flag 
is not set and there are no mappings left for a page table.

Regards,
Christian.

Am 30.10.19 um 18:42 schrieb Koenig, Christian:
The vaild flag doesn't take effect in this function.
That's irrelevant.

See what amdgpu_vm_update_ptes() does is to first determine the fragment size:
amdgpu_vm_fragment(params, frag_start, end, flags, &frag, &frag_end);

Then we walk down the tree:
amdgpu_vm_pt_start(adev, params->vm, start, &cursor);
while (cursor.pfn < end) {

And make sure that the page tables covering the address range are actually 
allocated:
r = amdgpu_vm_alloc_pts(params->adev, params->vm, &cursor);

Then we update the tables with the flags and addresses and free up subsequent 
tables in the case of huge pages or freed up areas:
/* Free all child entries */
while (cursor.pfn < frag_start) {
amdgpu_vm_free_pts(adev, params->vm, &cursor);
amdgpu_vm_pt_next(adev, &cursor);
}

This is the maximum you can free, cause all other page tables are not 
completely covered by the range and so potentially still in use.

And I have the strong suspicion that this is what your patch is actually doing 
wrong. In other words you are also freeing page tables which are only partially 
covered by the range and so potentially still in use.

Since we don't have any tracking how many entries in a page table are currently 
valid and how many are invalid we actually can't implement what you are trying 
to do here. So the patch is definitely somehow broken.

Regards,
Christian.

Am 30.10.19 um 17:55 schrieb Huang, JinHuiEric:

The vaild flag doesn't take effect in this function. amdgpu_vm_alloc_pts() is 
always executed that only depended on "cursor.pfn < end". The valid flag has 
only been checked on here for asic below GMC v9:

if (adev->asic_type < CHIP_VEGA10 &&
(flags & AMDGPU_PTE_VALID))...

Regards,

Eric

On 2019-10-30 12:30 p.m., Koenig, Christian wrote:


Am 30.10.2019 17:19 schrieb "Huang, JinHuiEric" 
:

I tested it that it saves a lot of vram on KFD big buffer stress test. I think 
there are two reasons:

1. Calling amdgpu_vm_update_ptes() during unmapping will allocate unnecessary 
pts, because there is no flag to determine if the VA is mapping or unmapping in 
function
amdgpu_vm_update_ptes(). It saves the most of memory.

That's not correct. The valid flag is used for this.


2. Intentionally removing those unmapping pts is logical expectation, although 
it is not removing so much pts.

Well I actually don't see a change to what update_ptes is doing and have the 
strong suspicion that the patch is simply broken.

You either free page tables which are potentially still in use or update_pte 
doesn't free page tables when the valid but is not set.

Regards,
Christian.




Regards,

Eric

On 2019-10-30 11:57 a.m., Koenig, Christian wrote:


Am 30.10.2019 16:47 schrieb "Kuehling, Felix" 
:
On 2019-10-30 9:52 a.m., Christian König wrote:
> Am 29.10.19 um 21:06 schrieb Huang, JinHuiEric:
>> The issue is PT BOs are not freed when unmapping VA,
>> which causes vram usage accumulated is huge in some
>> memory stress test, such as kfd big buffer stress test.
>> Function amdgpu_vm_bo_update_mapping() is called by both
>> amdgpu_vm_bo_update() and amdgpu_vm_clear_freed(). The
>> solution is replacing amdgpu_vm_bo_update_mapping() in
>> amdgpu_vm_clear_freed() with removing PT BOs function
>> to save vram usage.
>
> NAK, that is intentional behavior.
>
> Otherwise we can run into out of memory situations when page tables
> need to be allocated again under stress

Re: [PATCH v7] drm/amd/display: Add MST atomic routines

[Kazlauskas, Nicholas]

On 2019-10-28 10:31 a.m., mikita.lip...@amd.com wrote:
> From: Mikita Lipski 
> 
> - Adding encoder atomic check to find vcpi slots for a connector
> - Using DRM helper functions to calculate PBN
> - Adding connector atomic check to release vcpi slots if connector
> loses CRTC
> - Calculate  PBN and VCPI slots only once during atomic
> check and store them on crtc_state to eliminate
> redundant calculation
> - Call drm_dp_mst_atomic_check to verify validity of MST topology
> during state atomic check
> 
> v2: squashed previous 3 separate patches, removed DSC PBN calculation,
> and added PBN and VCPI slots properties to amdgpu connector
> 
> v3:
> - moved vcpi_slots and pbn properties to dm_crtc_state and dc_stream_state
> - updates stream's vcpi_slots and pbn on commit
> - separated patch from the DSC MST series
> 
> v4:
> - set vcpi_slots and pbn properties to dm_connector_state
> - copy porperties from connector state on to crtc state
> 
> v5:
> - keep the pbn and vcpi values only on connnector state
> - added a void pointer to the stream state instead on two ints,
> because dc_stream_state is OS agnostic. Pointer points to the
> current dm_connector_state.
> 
> v6:
> - Remove new param from stream
> 
> v7:
> - Cleanup
> - Remove state pointer from stream
>   
> Cc: Jun Lei 
> Cc: Jerry Zuo 
> Cc: Harry Wentland 
> Cc: Nicholas Kazlauskas 
> Cc: Lyude Paul 
> Signed-off-by: Mikita Lipski 
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 ++-
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +
>   .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 51 +--
>   .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 32 
>   4 files changed, 86 insertions(+), 41 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 48f5b43e2698..28f6b93ab371 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4180,7 +4180,8 @@ void amdgpu_dm_connector_funcs_reset(struct 
> drm_connector *connector)
>   state->underscan_hborder = 0;
>   state->underscan_vborder = 0;
>   state->base.max_requested_bpc = 8;
> -
> + state->vcpi_slots = 0;
> + state->pbn = 0;
>   if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
>   state->abm_level = amdgpu_dm_abm_level;
>   
> @@ -4209,7 +4210,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct 
> drm_connector *connector)
>   new_state->underscan_enable = state->underscan_enable;
>   new_state->underscan_hborder = state->underscan_hborder;
>   new_state->underscan_vborder = state->underscan_vborder;
> -
> + new_state->vcpi_slots = state->vcpi_slots;
> + new_state->pbn = state->pbn;
>   return &new_state->base;
>   }
>   
> @@ -4610,6 +4612,37 @@ static int dm_encoder_helper_atomic_check(struct 
> drm_encoder *encoder,
> struct drm_crtc_state *crtc_state,
> struct drm_connector_state 
> *conn_state)
>   {
> + struct drm_atomic_state *state = crtc_state->state;
> + struct drm_connector *connector = conn_state->connector;
> + struct amdgpu_dm_connector *aconnector = 
> to_amdgpu_dm_connector(connector);
> + struct dm_connector_state *dm_new_connector_state = 
> to_dm_connector_state(conn_state);
> + const struct drm_display_mode *adjusted_mode = 
> &crtc_state->adjusted_mode;
> + struct drm_dp_mst_topology_mgr *mst_mgr;
> + struct drm_dp_mst_port *mst_port;
> + int clock, bpp = 0;
> +
> + if (!aconnector->port || !aconnector->dc_sink)
> + return 0;
> +
> + mst_port = aconnector->port;
> + mst_mgr = &aconnector->mst_port->mst_mgr;
> +
> + if (!crtc_state->connectors_changed && !crtc_state->mode_changed)
> + return 0;
> +
> + if (!state->duplicated) {
> + bpp = (uint8_t)connector->display_info.bpc * 3;

Is this correct? Do we always want to calculate this based on the 
display capable bpc value instead of the one actually in use?

> + clock = adjusted_mode->clock;
> + dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp);
> + }
> + dm_new_connector_state->vcpi_slots = 
> drm_dp_atomic_find_vcpi_slots(state,
> +mst_mgr,
> +mst_port,
> +
> dm_new_connector_state->pbn);
> + if (dm_new_connector_state->vcpi_slots < 0) {
> + DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n", 
> dm_new_connector_state->vcpi_slots);
> + return dm_new_connector_state->vcpi_slots;
> + }
>   return 0;
>   }
>   
> @@ -7651,6 +7684,11 @@ static int amdgpu_dm_atomic_check(struct 

[PATCH] drm/amdgpu/gpuvm: add some additional comments in amdgpu_vm_update_ptes

[Alex Deucher]

To better clarify what is happening in this function.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c8ce42200059..3c0bd6472a46 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1419,6 +1419,9 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_vm_update_params *params,
uint64_t incr, entry_end, pe_start;
struct amdgpu_bo *pt;
 
+   /* make sure that the page tables covering the address range are
+* actually allocated
+*/
r = amdgpu_vm_alloc_pts(params->adev, params->vm, &cursor,
params->direct);
if (r)
@@ -1492,7 +1495,12 @@ static int amdgpu_vm_update_ptes(struct 
amdgpu_vm_update_params *params,
} while (frag_start < entry_end);
 
if (amdgpu_vm_pt_descendant(adev, &cursor)) {
-   /* Free all child entries */
+   /* Free all child entries.
+* Update the tables with the flags and addresses and 
free up subsequent
+* tables in the case of huge pages or freed up areas.
+* This is the maximum you can free, because all other 
page tables are not
+* completely covered by the range and so potentially 
still in use.
+*/
while (cursor.pfn < frag_start) {
amdgpu_vm_free_pts(adev, params->vm, &cursor);
amdgpu_vm_pt_next(adev, &cursor);
-- 
2.23.0

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

[PATCH] drm/amdgpu: Improve RAS documentation

[Alex Deucher]

Clarify some areas, clean up formatting, add section for
unrecoverable error handling.

Signed-off-by: Alex Deucher 
---
 Documentation/gpu/amdgpu.rst| 35 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 40 -
 2 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/Documentation/gpu/amdgpu.rst b/Documentation/gpu/amdgpu.rst
index 5b9eaf23558e..1c08d64970ee 100644
--- a/Documentation/gpu/amdgpu.rst
+++ b/Documentation/gpu/amdgpu.rst
@@ -82,12 +82,21 @@ AMDGPU XGMI Support
 AMDGPU RAS Support
 ==
 
+The AMDGPU RAS interfaces are exposed via sysfs (for informational queries) and
+debugfs (for error injection).
+
 RAS debugfs/sysfs Control and Error Injection Interfaces
 
 
 .. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
:doc: AMDGPU RAS debugfs control interface
 
+RAS Reboot Behavior for Unrecoverable Errors
+
+
+.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+   :doc: AMDGPU RAS Reboot Behavior for Unrecoverable Errors
+
 RAS Error Count sysfs Interface
 ---
 
@@ -109,6 +118,32 @@ RAS VRAM Bad Pages sysfs Interface
 .. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
:internal:
 
+Sample Code
+---
+Sample code for testing error injection can be found here:
+https://cgit.freedesktop.org/mesa/drm/tree/tests/amdgpu/ras_tests.c
+
+This is part of the libdrm amdgpu unit tests which cover several areas of the 
GPU.
+There are four sets of tests:
+
+RAS Basic Test
+
+The test verifies the RAS feature enabled status and makes sure the necessary 
sysfs and debugfs files
+are present.
+
+RAS Query Test
+
+This test will check the RAS availability and enablement status for each 
supported IP block as well as
+the error counts.
+
+RAS Inject Test
+
+This test injects errors for each IP.
+
+RAS Disable Test
+
+This tests disabling of RAS features for each IP block.
+
 
 GPU Power/Thermal Controls and Monitoring
 =
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index dab90c280476..404483437bd3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -220,7 +220,7 @@ static struct ras_manager *amdgpu_ras_find_obj(struct 
amdgpu_device *adev,
  * As their names indicate, inject operation will write the
  * value to the address.
  *
- * Second member: struct ras_debug_if::op.
+ * The second member: struct ras_debug_if::op.
  * It has three kinds of operations.
  *
  * - 0: disable RAS on the block. Take ::head as its data.
@@ -228,14 +228,20 @@ static struct ras_manager *amdgpu_ras_find_obj(struct 
amdgpu_device *adev,
  * - 2: inject errors on the block. Take ::inject as its data.
  *
  * How to use the interface?
- * programs:
- * copy the struct ras_debug_if in your codes and initialize it.
- * write the struct to the control node.
+ *
+ * Programs
+ *
+ * Copy the struct ras_debug_if in your codes and initialize it.
+ * Write the struct to the control node.
+ *
+ * Shells
  *
  * .. code-block:: bash
  *
  * echo op block [error [sub_block address value]] > .../ras/ras_ctrl
  *
+ * Parameters:
+ *
  * op: disable, enable, inject
  * disable: only block is needed
  * enable: block and error are needed
@@ -265,8 +271,10 @@ static struct ras_manager *amdgpu_ras_find_obj(struct 
amdgpu_device *adev,
  * /sys/class/drm/card[0/1/2...]/device/ras/[gfx/sdma/...]_err_count
  *
  * .. note::
- * Operation is only allowed on blocks which are supported.
+ * Operations are only allowed on blocks which are supported.
  * Please check ras mask at /sys/module/amdgpu/parameters/ras_mask
+ * to see which blocks support RAS on a particular asic.
+ *
  */
 static ssize_t amdgpu_ras_debugfs_ctrl_write(struct file *f, const char __user 
*buf,
size_t size, loff_t *pos)
@@ -322,7 +330,7 @@ static ssize_t amdgpu_ras_debugfs_ctrl_write(struct file 
*f, const char __user *
  * DOC: AMDGPU RAS debugfs EEPROM table reset interface
  *
  * Some boards contain an EEPROM which is used to persistently store a list of
- * bad pages containing ECC errors detected in vram.  This interface provides
+ * bad pages which experiences ECC errors in vram.  This interface provides
  * a way to reset the EEPROM, e.g., after testing error injection.
  *
  * Usage:
@@ -362,7 +370,7 @@ static const struct file_operations 
amdgpu_ras_debugfs_eeprom_ops = {
 /**
  * DOC: AMDGPU RAS sysfs Error Count Interface
  *
- * It allows user to read the error count for each IP block on the gpu through
+ * It allows the user to read the error count for each IP block on the gpu 
through
  * /sys/class/drm/card[0/1/2...]/device/ras/[gfx/sdma/...]_err_count
  *
  * It outputs the multiple lines which report the uncorrected (ue) and 
corrected
@@ -1027,6 +103

Re: [PATCH v7] drm/amd/display: Add MST atomic routines

[Mikita Lipski]

On 30.10.2019 14:19, Kazlauskas, Nicholas wrote:
> On 2019-10-28 10:31 a.m., mikita.lip...@amd.com wrote:
>> From: Mikita Lipski 
>>
>> - Adding encoder atomic check to find vcpi slots for a connector
>> - Using DRM helper functions to calculate PBN
>> - Adding connector atomic check to release vcpi slots if connector
>> loses CRTC
>> - Calculate  PBN and VCPI slots only once during atomic
>> check and store them on crtc_state to eliminate
>> redundant calculation
>> - Call drm_dp_mst_atomic_check to verify validity of MST topology
>> during state atomic check
>>
>> v2: squashed previous 3 separate patches, removed DSC PBN calculation,
>> and added PBN and VCPI slots properties to amdgpu connector
>>
>> v3:
>> - moved vcpi_slots and pbn properties to dm_crtc_state and dc_stream_state
>> - updates stream's vcpi_slots and pbn on commit
>> - separated patch from the DSC MST series
>>
>> v4:
>> - set vcpi_slots and pbn properties to dm_connector_state
>> - copy porperties from connector state on to crtc state
>>
>> v5:
>> - keep the pbn and vcpi values only on connnector state
>> - added a void pointer to the stream state instead on two ints,
>> because dc_stream_state is OS agnostic. Pointer points to the
>> current dm_connector_state.
>>
>> v6:
>> - Remove new param from stream
>>
>> v7:
>> - Cleanup
>> - Remove state pointer from stream
>>
>> Cc: Jun Lei 
>> Cc: Jerry Zuo 
>> Cc: Harry Wentland 
>> Cc: Nicholas Kazlauskas 
>> Cc: Lyude Paul 
>> Signed-off-by: Mikita Lipski 
>> ---
>>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 ++-
>>.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +
>>.../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 51 +--
>>.../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 32 
>>4 files changed, 86 insertions(+), 41 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 48f5b43e2698..28f6b93ab371 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -4180,7 +4180,8 @@ void amdgpu_dm_connector_funcs_reset(struct 
>> drm_connector *connector)
>>  state->underscan_hborder = 0;
>>  state->underscan_vborder = 0;
>>  state->base.max_requested_bpc = 8;
>> -
>> +state->vcpi_slots = 0;
>> +state->pbn = 0;
>>  if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
>>  state->abm_level = amdgpu_dm_abm_level;
>>
>> @@ -4209,7 +4210,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct 
>> drm_connector *connector)
>>  new_state->underscan_enable = state->underscan_enable;
>>  new_state->underscan_hborder = state->underscan_hborder;
>>  new_state->underscan_vborder = state->underscan_vborder;
>> -
>> +new_state->vcpi_slots = state->vcpi_slots;
>> +new_state->pbn = state->pbn;
>>  return &new_state->base;
>>}
>>
>> @@ -4610,6 +4612,37 @@ static int dm_encoder_helper_atomic_check(struct 
>> drm_encoder *encoder,
>>struct drm_crtc_state *crtc_state,
>>struct drm_connector_state 
>> *conn_state)
>>{
>> +struct drm_atomic_state *state = crtc_state->state;
>> +struct drm_connector *connector = conn_state->connector;
>> +struct amdgpu_dm_connector *aconnector = 
>> to_amdgpu_dm_connector(connector);
>> +struct dm_connector_state *dm_new_connector_state = 
>> to_dm_connector_state(conn_state);
>> +const struct drm_display_mode *adjusted_mode = 
>> &crtc_state->adjusted_mode;
>> +struct drm_dp_mst_topology_mgr *mst_mgr;
>> +struct drm_dp_mst_port *mst_port;
>> +int clock, bpp = 0;
>> +
>> +if (!aconnector->port || !aconnector->dc_sink)
>> +return 0;
>> +
>> +mst_port = aconnector->port;
>> +mst_mgr = &aconnector->mst_port->mst_mgr;
>> +
>> +if (!crtc_state->connectors_changed && !crtc_state->mode_changed)
>> +return 0;
>> +
>> +if (!state->duplicated) {
>> +bpp = (uint8_t)connector->display_info.bpc * 3;
> Is this correct? Do we always want to calculate this based on the
> display capable bpc value instead of the one actually in use?

Wouldn't it be the same thing since we always try to use highest bpc?

Unless, we reduce the colour depth and display capable bpc will stay the 
same, which would cause PBN be higher than needed.

Ok yeah that was a false assumption. I'll update it, thanks!

>
>> +clock = adjusted_mode->clock;
>> +dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp);
>> +}
>> +dm_new_connector_state->vcpi_slots = 
>> drm_dp_atomic_find_vcpi_slots(state,
>> +   mst_mgr,
>> +   mst_port,
>> + 

Re: [PATCH v7] drm/amd/display: Add MST atomic routines

[Kazlauskas, Nicholas]

On 2019-10-30 2:42 p.m., Lipski, Mikita wrote:
> 
> On 30.10.2019 14:19, Kazlauskas, Nicholas wrote:
>> On 2019-10-28 10:31 a.m., mikita.lip...@amd.com wrote:
>>> From: Mikita Lipski 
>>>
>>> - Adding encoder atomic check to find vcpi slots for a connector
>>> - Using DRM helper functions to calculate PBN
>>> - Adding connector atomic check to release vcpi slots if connector
>>> loses CRTC
>>> - Calculate  PBN and VCPI slots only once during atomic
>>> check and store them on crtc_state to eliminate
>>> redundant calculation
>>> - Call drm_dp_mst_atomic_check to verify validity of MST topology
>>> during state atomic check
>>>
>>> v2: squashed previous 3 separate patches, removed DSC PBN calculation,
>>> and added PBN and VCPI slots properties to amdgpu connector
>>>
>>> v3:
>>> - moved vcpi_slots and pbn properties to dm_crtc_state and dc_stream_state
>>> - updates stream's vcpi_slots and pbn on commit
>>> - separated patch from the DSC MST series
>>>
>>> v4:
>>> - set vcpi_slots and pbn properties to dm_connector_state
>>> - copy porperties from connector state on to crtc state
>>>
>>> v5:
>>> - keep the pbn and vcpi values only on connnector state
>>> - added a void pointer to the stream state instead on two ints,
>>> because dc_stream_state is OS agnostic. Pointer points to the
>>> current dm_connector_state.
>>>
>>> v6:
>>> - Remove new param from stream
>>>
>>> v7:
>>> - Cleanup
>>> - Remove state pointer from stream
>>> 
>>> Cc: Jun Lei 
>>> Cc: Jerry Zuo 
>>> Cc: Harry Wentland 
>>> Cc: Nicholas Kazlauskas 
>>> Cc: Lyude Paul 
>>> Signed-off-by: Mikita Lipski 
>>> ---
>>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 42 ++-
>>> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +
>>> .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 51 +--
>>> .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 32 
>>> 4 files changed, 86 insertions(+), 41 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 48f5b43e2698..28f6b93ab371 100644
>>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>> @@ -4180,7 +4180,8 @@ void amdgpu_dm_connector_funcs_reset(struct 
>>> drm_connector *connector)
>>> state->underscan_hborder = 0;
>>> state->underscan_vborder = 0;
>>> state->base.max_requested_bpc = 8;
>>> -
>>> +   state->vcpi_slots = 0;
>>> +   state->pbn = 0;
>>> if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
>>> state->abm_level = amdgpu_dm_abm_level;
>>> 
>>> @@ -4209,7 +4210,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct 
>>> drm_connector *connector)
>>> new_state->underscan_enable = state->underscan_enable;
>>> new_state->underscan_hborder = state->underscan_hborder;
>>> new_state->underscan_vborder = state->underscan_vborder;
>>> -
>>> +   new_state->vcpi_slots = state->vcpi_slots;
>>> +   new_state->pbn = state->pbn;
>>> return &new_state->base;
>>> }
>>> 
>>> @@ -4610,6 +4612,37 @@ static int dm_encoder_helper_atomic_check(struct 
>>> drm_encoder *encoder,
>>>   struct drm_crtc_state 
>>> *crtc_state,
>>>   struct drm_connector_state 
>>> *conn_state)
>>> {
>>> +   struct drm_atomic_state *state = crtc_state->state;
>>> +   struct drm_connector *connector = conn_state->connector;
>>> +   struct amdgpu_dm_connector *aconnector = 
>>> to_amdgpu_dm_connector(connector);
>>> +   struct dm_connector_state *dm_new_connector_state = 
>>> to_dm_connector_state(conn_state);
>>> +   const struct drm_display_mode *adjusted_mode = 
>>> &crtc_state->adjusted_mode;
>>> +   struct drm_dp_mst_topology_mgr *mst_mgr;
>>> +   struct drm_dp_mst_port *mst_port;
>>> +   int clock, bpp = 0;
>>> +
>>> +   if (!aconnector->port || !aconnector->dc_sink)
>>> +   return 0;
>>> +
>>> +   mst_port = aconnector->port;
>>> +   mst_mgr = &aconnector->mst_port->mst_mgr;
>>> +
>>> +   if (!crtc_state->connectors_changed && !crtc_state->mode_changed)
>>> +   return 0;
>>> +
>>> +   if (!state->duplicated) {
>>> +   bpp = (uint8_t)connector->display_info.bpc * 3;
>> Is this correct? Do we always want to calculate this based on the
>> display capable bpc value instead of the one actually in use?
> 
> Wouldn't it be the same thing since we always try to use highest bpc?
> 
> Unless, we reduce the colour depth and display capable bpc will stay the
> same, which would cause PBN be higher than needed.
> 
> Ok yeah that was a false assumption. I'll update it, thanks!

The current policy is to always default to 8bpc.

Most userspace isn't aware or setup by default to handle greater than 
8bpc and you run out of bandwidt

Re: [PATCH] Drivers: gpu: drm: amd: display: amdgpu_dm: amdgpu_dm.h: Fixed a documentation warning

[Alex Deucher]

On Mon, Oct 28, 2019 at 4:25 AM  wrote:
>
> From: Madhuparna Bhowmik 
>
> This patch fixes the following  warning: Incorrect use of
>  kernel-doc format:  * @atomic_obj
> by adding a colon after @atomic_obj.
>
> Signed-off-by: Madhuparna Bhowmik 

Thanks for the patch.  This as already fixed by Harry.

Alex

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index c8c525a2b505..80d53d095773 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -128,7 +128,7 @@ struct amdgpu_display_manager {
> u16 display_indexes_num;
>
> /**
> -* @atomic_obj
> +* @atomic_obj:
>  *
>  * In combination with &dm_atomic_state it helps manage
>  * global atomic state that doesn't map cleanly into existing
> --
> 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

[PATCH] drm/amdgpu: Show resolution correctly in mode validation debug output

[neil]

From: Neil Mayhew 

---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 a52f0b13a2c8..f802c784e6f6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4114,8 +4114,8 @@ enum drm_mode_status 
amdgpu_dm_connector_mode_valid(struct drm_connector *connec
result = MODE_OK;
else
DRM_DEBUG_KMS("Mode %dx%d (clk %d) failed DC validation with 
error %d\n",
- mode->vdisplay,
  mode->hdisplay,
+ mode->vdisplay,
  mode->clock,
  dc_result);
 
-- 
2.23.0

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

[PATCH 07/13] drm/dp_mst: Add new quirk for Synaptics MST hubs

[mikita.lipski]

From: Mikita Lipski 

Synaptics DP1.4 hubs (BRANCH_ID 0x90CC24) do not
support virtual DPCD registers, but do support DSC.
The DSC caps can be read from the physical aux,
like in SST DSC. These hubs have many different
DEVICE_IDs.  Add a new quirk to detect this case.

Reviewed-by: Wenjing Liu 
Reviewed-by: Lyude Paul 
Signed-off-by: David Francis 
---
 drivers/gpu/drm/drm_dp_helper.c   |  2 ++
 drivers/gpu/drm/drm_dp_mst_topology.c | 27 +++
 include/drm/drm_dp_helper.h   |  7 +++
 3 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index af1cd968adfd..02fa8c3d9a82 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1271,6 +1271,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
{ OUI(0x00, 0x10, 0xfa), DEVICE_ID_ANY, false, 
BIT(DP_DPCD_QUIRK_NO_PSR) },
/* CH7511 seems to leave SINK_COUNT zeroed */
{ OUI(0x00, 0x00, 0x00), DEVICE_ID('C', 'H', '7', '5', '1', '1'), 
false, BIT(DP_DPCD_QUIRK_NO_SINK_COUNT) },
+   /* Synaptics DP1.4 MST hubs can support DSC without virtual DPCD */
+   { OUI(0x90, 0xCC, 0x24), DEVICE_ID_ANY, true, 
BIT(DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) },
 };
 
 #undef OUI
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index d8f9ba27b559..d5df02315e14 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4222,6 +4222,7 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct 
drm_dp_mst_port *port)
 {
struct drm_dp_mst_port *immediate_upstream_port;
struct drm_dp_mst_port *fec_port;
+   struct drm_dp_desc desc = { 0 };
 
if (!port)
return NULL;
@@ -4274,6 +4275,32 @@ struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct 
drm_dp_mst_port *port)
if (drm_dp_mst_is_virtual_dpcd(port))
return &port->aux;
 
+   /*
+* Synaptics quirk
+* Applies to ports for which:
+* - Physical aux has Synaptics OUI
+* - DPv1.4 or higher
+* - Port is on primary branch device
+* - Not a VGA adapter (DP_DWN_STRM_PORT_TYPE_ANALOG)
+*/
+   if (!drm_dp_read_desc(port->mgr->aux, &desc, true))
+   return NULL;
+
+   if (drm_dp_has_quirk(&desc, DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD) &&
+   port->mgr->dpcd[DP_DPCD_REV] >= DP_DPCD_REV_14 &&
+   port->parent == port->mgr->mst_primary) {
+   u8 downstreamport;
+
+   if (drm_dp_dpcd_read(&port->aux, DP_DOWNSTREAMPORT_PRESENT,
+&downstreamport, 1) < 0)
+   return NULL;
+
+   if ((downstreamport & DP_DWN_STRM_PORT_PRESENT) &&
+  ((downstreamport & DP_DWN_STRM_PORT_TYPE_MASK)
+!= DP_DWN_STRM_PORT_TYPE_ANALOG))
+   return port->mgr->aux;
+   }
+
return NULL;
 }
 EXPORT_SYMBOL(drm_dp_mst_dsc_aux_for_port);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 6e17410a0417..61ef351c5fca 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1460,6 +1460,13 @@ enum drm_dp_quirk {
 * The driver should ignore SINK_COUNT during detection.
 */
DP_DPCD_QUIRK_NO_SINK_COUNT,
+   /**
+* @DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD:
+*
+* The device supports MST DSC despite not supporting Virtual DPCD.
+* The DSC caps can be read from the physical aux instead.
+*/
+   DP_DPCD_QUIRK_DSC_WITHOUT_VIRTUAL_DPCD,
 };
 
 /**
-- 
2.17.1

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

[PATCH 08/13] drm/amd/display: Initialize DSC PPS variables to 0

[mikita.lipski]

From: David Francis 

For DSC MST, sometimes monitors would break out
in full-screen static. The issue traced back to the
PPS generation code, where these variables were being used
uninitialized and were picking up garbage.

memset to 0 to avoid this

Reviewed-by: Nicholas Kazlauskas 
Signed-off-by: David Francis 
---
 drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c | 3 +++
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c   | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c 
b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
index a519dbc5ecb6..5d6cbaebebc0 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_hwss.c
@@ -496,6 +496,9 @@ bool dp_set_dsc_pps_sdp(struct pipe_ctx *pipe_ctx, bool 
enable)
struct dsc_config dsc_cfg;
uint8_t dsc_packed_pps[128];
 
+   memset(&dsc_cfg, 0, sizeof(dsc_cfg));
+   memset(dsc_packed_pps, 0, 128);
+
/* Enable DSC hw block */
dsc_cfg.pic_width = stream->timing.h_addressable + 
stream->timing.h_border_left + stream->timing.h_border_right;
dsc_cfg.pic_height = stream->timing.v_addressable + 
stream->timing.v_border_top + stream->timing.v_border_bottom;
diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c
index 63eb377ed9c0..296eeff00296 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c
@@ -207,6 +207,9 @@ static bool dsc2_get_packed_pps(struct 
display_stream_compressor *dsc, const str
struct dsc_reg_values dsc_reg_vals;
struct dsc_optc_config dsc_optc_cfg;
 
+   memset(&dsc_reg_vals, 0, sizeof(dsc_reg_vals));
+   memset(&dsc_optc_cfg, 0, sizeof(dsc_optc_cfg));
+
DC_LOG_DSC("Getting packed DSC PPS for DSC Config:");
dsc_config_log(dsc, dsc_cfg);
DC_LOG_DSC("DSC Picture Parameter Set (PPS):");
-- 
2.17.1

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

[PATCH 10/13] drm/amd/display: Write DSC enable to MST DPCD

[mikita.lipski]

From: David Francis 

Rework the dm_helpers_write_dsc_enable callback to
handle the MST case.

Use the cached dsc_aux field.

Reviewed-by: Wenjing Liu 
Signed-off-by: David Francis 
---
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 1b2cc85b4815..2144b65f4806 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -37,6 +37,7 @@
 #include "dc.h"
 #include "amdgpu_dm.h"
 #include "amdgpu_dm_irq.h"
+#include "amdgpu_dm_mst_types.h"
 
 #include "dm_helpers.h"
 
@@ -521,8 +522,24 @@ bool dm_helpers_dp_write_dsc_enable(
 )
 {
uint8_t enable_dsc = enable ? 1 : 0;
+   struct amdgpu_dm_connector *aconnector;
+
+   if (!stream)
+   return false;
+
+   if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT_MST) {
+   aconnector = (struct amdgpu_dm_connector 
*)stream->dm_stream_context;
+
+   if (!aconnector->dsc_aux)
+   return false;
+
+   return (drm_dp_dpcd_write(aconnector->dsc_aux, DP_DSC_ENABLE, 
&enable_dsc, 1) >= 0);
+   }
+
+   if (stream->signal == SIGNAL_TYPE_DISPLAY_PORT)
+   return dm_helpers_dp_write_dpcd(ctx, stream->link, 
DP_DSC_ENABLE, &enable_dsc, 1);
 
-   return dm_helpers_dp_write_dpcd(ctx, stream->sink->link, DP_DSC_ENABLE, 
&enable_dsc, 1);
+   return false;
 }
 #endif
 
-- 
2.17.1

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

[PATCH 02/13] drm/dp_mst: Add PBN calculation for DSC modes

[mikita.lipski]

From: David Francis 

With DSC, bpp can be fractional in multiples of 1/16.

Change drm_dp_calc_pbn_mode to reflect this, adding a new
parameter bool dsc. When this parameter is true, treat the
bpp parameter as having units not of bits per pixel, but
1/16 of a bit per pixel

v2: Don't add separate function for this

Reviewed-by: Manasi Navare 
Reviewed-by: Lyude Paul 
Reviewed-by: Harry Wentland 
Signed-off-by: David Francis 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c|  2 +-
 drivers/gpu/drm/drm_dp_mst_topology.c| 16 
 drivers/gpu/drm/i915/display/intel_dp_mst.c  |  3 ++-
 drivers/gpu/drm/nouveau/dispnv50/disp.c  |  3 ++-
 drivers/gpu/drm/radeon/radeon_dp_mst.c   |  2 +-
 include/drm/drm_dp_mst_helper.h  |  3 +--
 6 files changed, 19 insertions(+), 10 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 d75726013436..1309e9cfdea3 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4656,7 +4656,7 @@ static int dm_encoder_helper_atomic_check(struct 
drm_encoder *encoder,
color_depth = convert_color_depth_from_display_info(connector, 
conn_state);
bpp = convert_dc_color_depth_into_bpc(color_depth) * 3;
clock = adjusted_mode->clock;
-   dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp);
+   dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp, 
false);
}
dm_new_connector_state->vcpi_slots = 
drm_dp_atomic_find_vcpi_slots(state,
   
mst_mgr,
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 82add736e17d..3e7b7553cf4d 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -3534,10 +3534,11 @@ EXPORT_SYMBOL(drm_dp_check_act_status);
  * drm_dp_calc_pbn_mode() - Calculate the PBN for a mode.
  * @clock: dot clock for the mode
  * @bpp: bpp for the mode.
+ * @dsc: DSC mode. If true, bpp has units of 1/16 of a bit per pixel
  *
  * This uses the formula in the spec to calculate the PBN value for a mode.
  */
-int drm_dp_calc_pbn_mode(int clock, int bpp)
+int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
 {
u64 kbps;
s64 peak_kbps;
@@ -3555,11 +3556,18 @@ int drm_dp_calc_pbn_mode(int clock, int bpp)
 * peak_kbps *= (1006/1000)
 * peak_kbps *= (64/54)
 * peak_kbps *= 8convert to bytes
+*
+* If the bpp is in units of 1/16, further divide by 16. Put this
+* factor in the numerator rather than the denominator to avoid
+* integer overflow
 */
 
numerator = 64 * 1006;
denominator = 54 * 8 * 1000 * 1000;
 
+   if (dsc)
+   numerator /= 16;
+
kbps *= numerator;
peak_kbps = drm_fixp_from_fraction(kbps, denominator);
 
@@ -3570,19 +3578,19 @@ EXPORT_SYMBOL(drm_dp_calc_pbn_mode);
 static int test_calc_pbn_mode(void)
 {
int ret;
-   ret = drm_dp_calc_pbn_mode(154000, 30);
+   ret = drm_dp_calc_pbn_mode(154000, 30, false);
if (ret != 689) {
DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, 
expected PBN %d, actual PBN %d.\n",
154000, 30, 689, ret);
return -EINVAL;
}
-   ret = drm_dp_calc_pbn_mode(234000, 30);
+   ret = drm_dp_calc_pbn_mode(234000, 30, false);
if (ret != 1047) {
DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, 
expected PBN %d, actual PBN %d.\n",
234000, 30, 1047, ret);
return -EINVAL;
}
-   ret = drm_dp_calc_pbn_mode(297000, 24);
+   ret = drm_dp_calc_pbn_mode(297000, 24, false);
if (ret != 1063) {
DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, 
expected PBN %d, actual PBN %d.\n",
297000, 24, 1063, ret);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c 
b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 2c5ac3dd647f..dfac450841df 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -61,7 +61,8 @@ static int intel_dp_mst_compute_link_config(struct 
intel_encoder *encoder,
crtc_state->pipe_bpp = bpp;
 
crtc_state->pbn = 
drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock,
-  crtc_state->pipe_bpp);
+  crtc_state->pipe_bpp,
+  false);
 
slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr,
  p

[PATCH 04/13] drm/dp_mst: Add MST support to DP DPCD R/W functions

[mikita.lipski]

From: David Francis 

Instead of having drm_dp_dpcd_read/write and
drm_dp_mst_dpcd_read/write as entry points into the
aux code, have drm_dp_dpcd_read/write handle both.

This means that DRM drivers can make MST DPCD read/writes.

v2: Fix spacing
v3: Dump dpcd access on MST read/writes
v4: Fix calling wrong function on DPCD write

Reviewed-by: Lyude Paul 
Reviewed-by: Harry Wentland 
Signed-off-by: David Francis 
Signed-off-by: Mikita Lipski 
---
 drivers/gpu/drm/drm_dp_aux_dev.c | 12 ++--
 drivers/gpu/drm/drm_dp_helper.c  | 31 +--
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c
index 0cfb386754c3..2510717d5a08 100644
--- a/drivers/gpu/drm/drm_dp_aux_dev.c
+++ b/drivers/gpu/drm/drm_dp_aux_dev.c
@@ -163,11 +163,7 @@ static ssize_t auxdev_read_iter(struct kiocb *iocb, struct 
iov_iter *to)
break;
}
 
-   if (aux_dev->aux->is_remote)
-   res = drm_dp_mst_dpcd_read(aux_dev->aux, pos, buf,
-  todo);
-   else
-   res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
+   res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
 
if (res <= 0)
break;
@@ -215,11 +211,7 @@ static ssize_t auxdev_write_iter(struct kiocb *iocb, 
struct iov_iter *from)
break;
}
 
-   if (aux_dev->aux->is_remote)
-   res = drm_dp_mst_dpcd_write(aux_dev->aux, pos, buf,
-   todo);
-   else
-   res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
+   res = drm_dp_dpcd_write(aux_dev->aux, pos, buf, todo);
 
if (res <= 0)
break;
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ffc68d305afe..af1cd968adfd 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -32,6 +32,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "drm_crtc_helper_internal.h"
 
@@ -251,7 +253,7 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 
request,
 
 /**
  * drm_dp_dpcd_read() - read a series of bytes from the DPCD
- * @aux: DisplayPort AUX channel
+ * @aux: DisplayPort AUX channel (SST or MST)
  * @offset: address of the (first) register to read
  * @buffer: buffer to store the register values
  * @size: number of bytes in @buffer
@@ -280,13 +282,18 @@ ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned 
int offset,
 * We just have to do it before any DPCD access and hope that the
 * monitor doesn't power down exactly after the throw away read.
 */
-   ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, DP_DPCD_REV, buffer,
-1);
-   if (ret != 1)
-   goto out;
+   if (!aux->is_remote) {
+   ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, DP_DPCD_REV,
+buffer, 1);
+   if (ret != 1)
+   goto out;
+   }
 
-   ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, buffer,
-size);
+   if (aux->is_remote)
+   ret = drm_dp_mst_dpcd_read(aux, offset, buffer, size);
+   else
+   ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset,
+buffer, size);
 
 out:
drm_dp_dump_access(aux, DP_AUX_NATIVE_READ, offset, buffer, ret);
@@ -296,7 +303,7 @@ EXPORT_SYMBOL(drm_dp_dpcd_read);
 
 /**
  * drm_dp_dpcd_write() - write a series of bytes to the DPCD
- * @aux: DisplayPort AUX channel
+ * @aux: DisplayPort AUX channel (SST or MST)
  * @offset: address of the (first) register to write
  * @buffer: buffer containing the values to write
  * @size: number of bytes in @buffer
@@ -313,8 +320,12 @@ ssize_t drm_dp_dpcd_write(struct drm_dp_aux *aux, unsigned 
int offset,
 {
int ret;
 
-   ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_WRITE, offset, buffer,
-size);
+   if (aux->is_remote)
+   ret = drm_dp_mst_dpcd_write(aux, offset, buffer, size);
+   else
+   ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_WRITE, offset,
+buffer, size);
+
drm_dp_dump_access(aux, DP_AUX_NATIVE_WRITE, offset, buffer, ret);
return ret;
 }
-- 
2.17.1

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

[PATCH 06/13] drm/dp_mst: Add helpers for MST DSC and virtual DPCD aux

[mikita.lipski]

From: David Francis 

Add drm_dp_mst_dsc_aux_for_port. To enable DSC, the DSC_ENABLED
register might have to be written on the leaf port's DPCD,
its parent's DPCD, or the MST manager's DPCD. This function
finds the correct aux for the job.

As part of this, add drm_dp_mst_is_virtual_dpcd. Virtual DPCD
is a DP feature new in DP v1.4, which exposes certain DPCD
registers on virtual ports.

v2: Remember to unlock mutex on all paths
v3: Refactor to match coding style and increase brevity

Reviewed-by: Lyude Paul 
Reviewed-by: Wenjing Liu 
Signed-off-by: David Francis 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 127 ++
 include/drm/drm_dp_mst_helper.h   |   2 +
 2 files changed, 129 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 502923c24450..d8f9ba27b559 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4150,3 +4150,130 @@ static void drm_dp_mst_unregister_i2c_bus(struct 
drm_dp_aux *aux)
 {
i2c_del_adapter(&aux->ddc);
 }
+
+/**
+ * drm_dp_mst_is_virtual_dpcd() - Is the given port a virtual DP Peer Device
+ * @port: The port to check
+ *
+ * A single physical MST hub object can be represented in the topology
+ * by multiple branches, with virtual ports between those branches.
+ *
+ * As of DP1.4, An MST hub with internal (virtual) ports must expose
+ * certain DPCD registers over those ports. See sections 2.6.1.1.1
+ * and 2.6.1.1.2 of Display Port specification v1.4 for details.
+ *
+ * May acquire mgr->lock
+ *
+ * Returns:
+ * true if the port is a virtual DP peer device, false otherwise
+ */
+static bool drm_dp_mst_is_virtual_dpcd(struct drm_dp_mst_port *port)
+{
+   struct drm_dp_mst_port *downstream_port;
+
+   if (!port || port->dpcd_rev < DP_DPCD_REV_14)
+   return false;
+
+   /* Virtual DP Sink (Internal Display Panel) */
+   if (port->port_num >= 8)
+   return true;
+
+   /* DP-to-HDMI Protocol Converter */
+   if (port->pdt == DP_PEER_DEVICE_DP_LEGACY_CONV &&
+   !port->mcs &&
+   port->ldps)
+   return true;
+
+   /* DP-to-DP */
+   mutex_lock(&port->mgr->lock);
+   if (port->pdt == DP_PEER_DEVICE_MST_BRANCHING &&
+   port->mstb &&
+   port->mstb->num_ports == 2) {
+   list_for_each_entry(downstream_port, &port->mstb->ports, next) {
+   if (downstream_port->pdt == DP_PEER_DEVICE_SST_SINK &&
+   !downstream_port->input) {
+   mutex_unlock(&port->mgr->lock);
+   return true;
+   }
+   }
+   }
+   mutex_unlock(&port->mgr->lock);
+
+   return false;
+}
+
+/**
+ * drm_dp_mst_dsc_aux_for_port() - Find the correct aux for DSC
+ * @port: The port to check. A leaf of the MST tree with an attached display.
+ *
+ * Depending on the situation, DSC may be enabled via the endpoint aux,
+ * the immediately upstream aux, or the connector's physical aux.
+ *
+ * This is both the correct aux to read DSC_CAPABILITY and the
+ * correct aux to write DSC_ENABLED.
+ *
+ * This operation can be expensive (up to four aux reads), so
+ * the caller should cache the return.
+ *
+ * Returns:
+ * NULL if DSC cannot be enabled on this port, otherwise the aux device
+ */
+struct drm_dp_aux *drm_dp_mst_dsc_aux_for_port(struct drm_dp_mst_port *port)
+{
+   struct drm_dp_mst_port *immediate_upstream_port;
+   struct drm_dp_mst_port *fec_port;
+
+   if (!port)
+   return NULL;
+
+   if (port->parent)
+   immediate_upstream_port = port->parent->port_parent;
+   else
+   immediate_upstream_port = NULL;
+
+   fec_port = immediate_upstream_port;
+   while (fec_port) {
+   /*
+* Each physical link (i.e. not a virtual port) between the
+* output and the primary device must support FEC
+*/
+   if (!drm_dp_mst_is_virtual_dpcd(fec_port) &&
+   !fec_port->fec_capable)
+   return NULL;
+
+   fec_port = fec_port->parent->port_parent;
+   }
+
+   /* DP-to-DP peer device */
+   if (drm_dp_mst_is_virtual_dpcd(immediate_upstream_port)) {
+   u8 upstream_dsc;
+   u8 endpoint_dsc;
+   u8 endpoint_fec;
+
+   if (drm_dp_dpcd_read(&port->aux,
+DP_DSC_SUPPORT, &endpoint_dsc, 1) < 0)
+   return NULL;
+   if (drm_dp_dpcd_read(&port->aux,
+DP_FEC_CAPABILITY, &endpoint_fec, 1) < 0)
+   return NULL;
+   if (drm_dp_dpcd_read(&immediate_upstream_port->aux,
+DP_DSC_SUPPORT, &upstream_dsc, 1) < 0)
+   return NULL;
+
+  

[PATCH 03/13] drm/dp_mst: Parse FEC capability on MST ports

[mikita.lipski]

From: David Francis 

As of DP1.4, ENUM_PATH_RESOURCES returns a bit indicating
if FEC can be supported up to that point in the MST network.

The bit is the first byte of the ENUM_PATH_RESOURCES ack reply,
bottom-most bit (refer to section 2.11.9.4 of DP standard,
v1.4)

That value is needed for FEC and DSC support

Store it on drm_dp_mst_port

Reviewed-by: Lyude Paul 
Reviewed-by: Harry Wentland 
Signed-off-by: David Francis 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++
 include/drm/drm_dp_mst_helper.h   | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 3e7b7553cf4d..9f3604355705 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -553,6 +553,7 @@ static bool 
drm_dp_sideband_parse_enum_path_resources_ack(struct drm_dp_sideband
 {
int idx = 1;
repmsg->u.path_resources.port_number = (raw->msg[idx] >> 4) & 0xf;
+   repmsg->u.path_resources.fec_capable = raw->msg[idx] & 0x1;
idx++;
if (idx > raw->curlen)
goto fail_len;
@@ -2183,6 +2184,7 @@ static int drm_dp_send_enum_path_resources(struct 
drm_dp_mst_topology_mgr *mgr,
DRM_DEBUG_KMS("enum path resources %d: %d %d\n", 
txmsg->reply.u.path_resources.port_number, 
txmsg->reply.u.path_resources.full_payload_bw_number,
   
txmsg->reply.u.path_resources.avail_payload_bw_number);
port->available_pbn = 
txmsg->reply.u.path_resources.avail_payload_bw_number;
+   port->fec_capable = 
txmsg->reply.u.path_resources.fec_capable;
}
}
 
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 9116b2c95239..f113ae04fa88 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -108,6 +108,8 @@ struct drm_dp_mst_port {
 * audio-capable.
 */
bool has_audio;
+
+   bool fec_capable;
 };
 
 /**
@@ -312,6 +314,7 @@ struct drm_dp_port_number_req {
 
 struct drm_dp_enum_path_resources_ack_reply {
u8 port_number;
+   bool fec_capable;
u16 full_payload_bw_number;
u16 avail_payload_bw_number;
 };
-- 
2.17.1

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

[PATCH 05/13] drm/dp_mst: Fill branch->num_ports

[mikita.lipski]

From: David Francis 

This field on drm_dp_mst_branch was never filled

It is initialized to zero when the port is kzallocced.
When a port is added to the list, increment num_ports,
and when a port is removed from the list, decrement num_ports.

v2: remember to decrement on port removal
v3: don't explicitly init to 0

Reviewed-by: Lyude Paul 
Reviewed-by: Harry Wentland 
Signed-off-by: David Francis 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 9f3604355705..502923c24450 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1669,6 +1669,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch 
*mstb,
mutex_lock(&mstb->mgr->lock);
drm_dp_mst_topology_get_port(port);
list_add(&port->next, &mstb->ports);
+   mstb->num_ports++;
mutex_unlock(&mstb->mgr->lock);
}
 
@@ -1703,6 +1704,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch 
*mstb,
/* remove it from the port list */
mutex_lock(&mstb->mgr->lock);
list_del(&port->next);
+   mstb->num_ports--;
mutex_unlock(&mstb->mgr->lock);
/* drop port list reference */
drm_dp_mst_topology_put_port(port);
-- 
2.17.1

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

[PATCH 01/13] drm/amd/display: Add MST atomic routines

[mikita.lipski]

From: Mikita Lipski 

- Adding encoder atomic check to find vcpi slots for a connector
- Using DRM helper functions to calculate PBN
- Adding connector atomic check to release vcpi slots if connector
loses CRTC
- Calculate  PBN and VCPI slots only once during atomic
check and store them on crtc_state to eliminate
redundant calculation
- Call drm_dp_mst_atomic_check to verify validity of MST topology
during state atomic check

v2: squashed previous 3 separate patches, removed DSC PBN calculation,
and added PBN and VCPI slots properties to amdgpu connector

v3:
- moved vcpi_slots and pbn properties to dm_crtc_state and dc_stream_state
- updates stream's vcpi_slots and pbn on commit
- separated patch from the DSC MST series

v4:
- set vcpi_slots and pbn properties to dm_connector_state
- copy porperties from connector state on to crtc state

v5:
- keep the pbn and vcpi values only on connnector state
- added a void pointer to the stream state instead on two ints,
because dc_stream_state is OS agnostic. Pointer points to the
current dm_connector_state.

v6:
- Remove new param from stream

v7:
- Fix error with using max capable bpc

Cc: Jun Lei 
Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Nicholas Kazlauskas 
Cc: Lyude Paul 
Signed-off-by: Mikita Lipski 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 65 ++-
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  2 +
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 51 ---
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 32 +
 4 files changed, 109 insertions(+), 41 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 48f5b43e2698..d75726013436 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4180,7 +4180,8 @@ void amdgpu_dm_connector_funcs_reset(struct drm_connector 
*connector)
state->underscan_hborder = 0;
state->underscan_vborder = 0;
state->base.max_requested_bpc = 8;
-
+   state->vcpi_slots = 0;
+   state->pbn = 0;
if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
state->abm_level = amdgpu_dm_abm_level;
 
@@ -4209,7 +4210,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct 
drm_connector *connector)
new_state->underscan_enable = state->underscan_enable;
new_state->underscan_hborder = state->underscan_hborder;
new_state->underscan_vborder = state->underscan_vborder;
-
+   new_state->vcpi_slots = state->vcpi_slots;
+   new_state->pbn = state->pbn;
return &new_state->base;
 }
 
@@ -4606,10 +4608,64 @@ static void dm_encoder_helper_disable(struct 
drm_encoder *encoder)
 
 }
 
+static int convert_dc_color_depth_into_bpc (enum dc_color_depth 
display_color_depth)
+{
+   switch (display_color_depth) {
+   case COLOR_DEPTH_666:
+   return 6;
+   case COLOR_DEPTH_888:
+   return 8;
+   case COLOR_DEPTH_101010:
+   return 10;
+   case COLOR_DEPTH_121212:
+   return 12;
+   case COLOR_DEPTH_141414:
+   return 14;
+   case COLOR_DEPTH_161616:
+   return 16;
+   default:
+   break;
+   }
+   return 0;
+}
+
 static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
  struct drm_crtc_state *crtc_state,
  struct drm_connector_state 
*conn_state)
 {
+   struct drm_atomic_state *state = crtc_state->state;
+   struct drm_connector *connector = conn_state->connector;
+   struct amdgpu_dm_connector *aconnector = 
to_amdgpu_dm_connector(connector);
+   struct dm_connector_state *dm_new_connector_state = 
to_dm_connector_state(conn_state);
+   const struct drm_display_mode *adjusted_mode = 
&crtc_state->adjusted_mode;
+   struct drm_dp_mst_topology_mgr *mst_mgr;
+   struct drm_dp_mst_port *mst_port;
+   enum dc_color_depth color_depth;
+   int clock, bpp = 0;
+
+   if (!aconnector->port || !aconnector->dc_sink)
+   return 0;
+
+   mst_port = aconnector->port;
+   mst_mgr = &aconnector->mst_port->mst_mgr;
+
+   if (!crtc_state->connectors_changed && !crtc_state->mode_changed)
+   return 0;
+
+   if (!state->duplicated) {
+   color_depth = convert_color_depth_from_display_info(connector, 
conn_state);
+   bpp = convert_dc_color_depth_into_bpc(color_depth) * 3;
+   clock = adjusted_mode->clock;
+   dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp);
+   }
+   dm_new_connector_state->vcpi_slots = 
drm_dp_atomic_find_vcpi_slots(state,
+   

[PATCH v4 00/13] DSC MST support for AMDGPU

[mikita.lipski]

From: Mikita Lipski 


This set of patches is a continuation of DSC enablement
patches for AMDGPU. This set enables DSC on MST. It also
contains implementation of both encoder and connector
atomic check routines.

First 10 patches have been introduced in multiple
iterations to the mailing list before. These patches were
developed by David Francis as part of his work on DSC.

Other 3 patches add atomic check functionality to
encoder and connector to allocate and release VCPI
slots on each state atomic check. These changes
utilize newly added drm_mst_helper functions for
better tracking of VCPI slots.

v2: squashed previously 3 separate atomic check patches,
separate atomic check for dsc connectors, track vcpi and
pbn on connectors.

v3: Moved modeset trigger on affected MST displays to DRM

v4: Fix warnings, use current mode's bpc rather than display's
maximum capable one

David Francis (10):
  drm/dp_mst: Add PBN calculation for DSC modes
  drm/dp_mst: Parse FEC capability on MST ports
  drm/dp_mst: Add MST support to DP DPCD R/W functions
  drm/dp_mst: Fill branch->num_ports
  drm/dp_mst: Add helpers for MST DSC and virtual DPCD aux
  drm/amd/display: Initialize DSC PPS variables to 0
  drm/amd/display: Validate DSC caps on MST endpoints
  drm/amd/display: Write DSC enable to MST DPCD
  drm/amd/display: MST DSC compute fair share
  drm/dp_mst: Add new quirk for Synaptics MST hubs

Mikita Lipski (3):
  drm/amd/display: Add MST atomic routines
  drm/dp_mst: Add DSC enablement helpers to DRM
  drm/amd/display: Recalculate VCPI slots for new DSC connectors

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 109 -
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |   5 +
 .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  70 ++-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 449 +-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.h   |   4 +
 .../drm/amd/display/dc/core/dc_link_hwss.c|   3 +
 .../gpu/drm/amd/display/dc/dcn20/dcn20_dsc.c  |   3 +
 .../drm/amd/display/dc/dcn20/dcn20_resource.c |   7 +-
 .../drm/amd/display/dc/dcn20/dcn20_resource.h |   1 +
 drivers/gpu/drm/drm_dp_aux_dev.c  |  12 +-
 drivers/gpu/drm/drm_dp_helper.c   |  33 +-
 drivers/gpu/drm/drm_dp_mst_topology.c | 277 ++-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |   3 +-
 drivers/gpu/drm/nouveau/dispnv50/disp.c   |   3 +-
 drivers/gpu/drm/radeon/radeon_dp_mst.c|   2 +-
 include/drm/drm_dp_helper.h   |   7 +
 include/drm/drm_dp_mst_helper.h   |  12 +-
 17 files changed, 925 insertions(+), 75 deletions(-)

-- 
2.17.1

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

[PATCH 09/13] drm/amd/display: Validate DSC caps on MST endpoints

[mikita.lipski]

From: David Francis 

During MST mode enumeration, if a new dc_sink is created,
populate it with dsc caps as appropriate.

Use drm_dp_mst_dsc_aux_for_port to get the raw caps,
then parse them onto dc_sink with dc_dsc_parse_dsc_dpcd.

Reviewed-by: Wenjing Liu 
Signed-off-by: David Francis 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  3 ++
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 31 ++-
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
index 910c8598faf9..37ca191a5b1c 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
@@ -279,6 +279,9 @@ struct amdgpu_dm_connector {
struct drm_dp_mst_port *port;
struct amdgpu_dm_connector *mst_port;
struct amdgpu_encoder *mst_encoder;
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+   struct drm_dp_aux *dsc_aux;
+#endif
 
/* TODO see if we can merge with ddc_bus or make a dm_connector */
struct amdgpu_i2c_adapter *i2c;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 1a17ea1b42e0..804a00082bee 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -25,6 +25,7 @@
 
 #include 
 #include 
+#include 
 #include "dm_services.h"
 #include "amdgpu.h"
 #include "amdgpu_dm.h"
@@ -188,6 +189,28 @@ static const struct drm_connector_funcs 
dm_dp_mst_connector_funcs = {
.early_unregister = amdgpu_dm_mst_connector_early_unregister,
 };
 
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+static bool validate_dsc_caps_on_connector(struct amdgpu_dm_connector 
*aconnector)
+{
+   struct dc_sink *dc_sink = aconnector->dc_sink;
+   struct drm_dp_mst_port *port = aconnector->port;
+   u8 dsc_caps[16] = { 0 };
+
+   aconnector->dsc_aux = drm_dp_mst_dsc_aux_for_port(port);
+
+   if (!aconnector->dsc_aux)
+   return false;
+
+   if (drm_dp_dpcd_read(aconnector->dsc_aux, DP_DSC_SUPPORT, dsc_caps, 16) 
< 0)
+   return false;
+
+   if (!dc_dsc_parse_dsc_dpcd(dsc_caps, NULL, 
&dc_sink->sink_dsc_caps.dsc_dec_caps))
+   return false;
+
+   return true;
+}
+#endif
+
 static int dm_dp_mst_get_modes(struct drm_connector *connector)
 {
struct amdgpu_dm_connector *aconnector = 
to_amdgpu_dm_connector(connector);
@@ -230,10 +253,16 @@ static int dm_dp_mst_get_modes(struct drm_connector 
*connector)
/* dc_link_add_remote_sink returns a new reference */
aconnector->dc_sink = dc_sink;
 
-   if (aconnector->dc_sink)
+   if (aconnector->dc_sink) {
amdgpu_dm_update_freesync_caps(
connector, aconnector->edid);
 
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+   if (!validate_dsc_caps_on_connector(aconnector))
+   memset(&aconnector->dc_sink->sink_dsc_caps,
+  0, 
sizeof(aconnector->dc_sink->sink_dsc_caps));
+#endif
+   }
}
 
drm_connector_update_edid_property(
-- 
2.17.1

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

[PATCH 13/13] drm/amd/display: Recalculate VCPI slots for new DSC connectors

[mikita.lipski]

From: Mikita Lipski 

Since for DSC MST connector's PBN is claculated differently
due to compression, we have to recalculate both PBN and
VCPI slots for that connector.

The function iterates through all the active streams to
find, which have DSC enabled, then recalculates PBN for
it and calls drm_dp_helper_update_vcpi_slots_for_dsc to
update connector's VCPI slots.

Cc: Jerry Zuo 
Cc: Harry Wentland 
Cc: Lyude Paul 
Signed-off-by: Mikita Lipski 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 50 +--
 1 file changed, 45 insertions(+), 5 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 1bbdf29f3c7d..1bc02957cd18 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4674,6 +4674,43 @@ const struct drm_encoder_helper_funcs 
amdgpu_dm_encoder_helper_funcs = {
.atomic_check = dm_encoder_helper_atomic_check
 };
 
+static int dm_update_mst_vcpi_slots_for_dsc(struct drm_atomic_state *state,
+   struct dc_state *dc_state)
+{
+   struct dc_stream_state *stream;
+   struct amdgpu_dm_connector *aconnector;
+   struct dm_connector_state *dm_conn_state;
+   int i = 0, clock = 0, bpp = 0;
+
+   for (i = 0; i < dc_state->stream_count; i++) {
+
+   stream = dc_state->streams[i];
+
+   if (!stream)
+   continue;
+
+   aconnector = (struct amdgpu_dm_connector 
*)stream->dm_stream_context;
+   dm_conn_state = to_dm_connector_state(aconnector->base.state);
+
+   if (!aconnector->port)
+   continue;
+
+   if (stream->timing.flags.DSC != 1)
+   continue;
+
+   bpp = 
convert_dc_color_depth_into_bpc(stream->timing.display_color_depth)* 3;
+   clock = stream->timing.pix_clk_100hz / 10;
+
+   dm_conn_state->pbn =  drm_dp_calc_pbn_mode(clock, bpp, true);
+
+   dm_conn_state->vcpi_slots = 
drm_dp_helper_update_vcpi_slots_for_dsc(state, aconnector->port, 
dm_conn_state->pbn);
+
+   if (dm_conn_state->vcpi_slots < 0)
+   return dm_conn_state->vcpi_slots;
+   }
+   return 0;
+}
+
 static void dm_drm_plane_reset(struct drm_plane *plane)
 {
struct dm_plane_state *amdgpu_state = NULL;
@@ -7707,11 +7744,6 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
 
-   /* Perform validation of MST topology in the state*/
-   ret = drm_dp_mst_atomic_check(state);
-   if (ret)
-   goto fail;
-
if (state->legacy_cursor_update) {
/*
 * This is a fast cursor update coming from the plane update
@@ -7783,6 +7815,10 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
 #ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
if (!compute_mst_dsc_configs_for_state(dm_state->context))
goto fail;
+
+   ret = dm_update_mst_vcpi_slots_for_dsc(state, 
dm_state->context);
+   if (ret)
+   goto fail;
 #endif
if (dc_validate_global_state(dc, dm_state->context, false) != 
DC_OK) {
ret = -EINVAL;
@@ -7812,6 +7848,10 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
dc_retain_state(old_dm_state->context);
}
}
+   /* Perform validation of MST topology in the state*/
+   ret = drm_dp_mst_atomic_check(state);
+   if (ret)
+   goto fail;
 
/* Store the overall update type for use later in atomic check. */
for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
-- 
2.17.1

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

[PATCH 12/13] drm/dp_mst: Add DSC enablement helpers to DRM

[mikita.lipski]

From: Mikita Lipski 

Adding the following elements to add MST DSC support to DRM:

- dsc_enable boolean flag to drm_dp_vcpi_allocation structure to signal,
which port got DSC enabled

- function drm_dp_helper_update_vcpi_slots_for_dsc allows reallocation
of newly recalculated VCPI slots and raises dsc_enable flag on the port.

- function drm_dp_mst_update_dsc_crtcs is called in drm_dp_mst_atomic_check,
its purpose is to iterate through all the ports in the topology and set
mode_changed flag on crtc if DSC has been enabled.

Cc: Harry Wentland 
Cc: Lyude Paul 
Signed-off-by: Mikita Lipski 
---
 drivers/gpu/drm/drm_dp_mst_topology.c | 103 +-
 include/drm/drm_dp_mst_helper.h   |   4 +
 2 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c 
b/drivers/gpu/drm/drm_dp_mst_topology.c
index d5df02315e14..4f2f09fe32f8 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -73,6 +73,7 @@ static bool drm_dp_validate_guid(struct 
drm_dp_mst_topology_mgr *mgr,
 static int drm_dp_mst_register_i2c_bus(struct drm_dp_aux *aux);
 static void drm_dp_mst_unregister_i2c_bus(struct drm_dp_aux *aux);
 static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr);
+static void drm_dp_mst_update_dsc_crtcs(struct drm_dp_mst_topology_state 
*mst_state);
 
 #define DP_STR(x) [DP_ ## x] = #x
 
@@ -3293,6 +3294,65 @@ int drm_dp_atomic_find_vcpi_slots(struct 
drm_atomic_state *state,
 }
 EXPORT_SYMBOL(drm_dp_atomic_find_vcpi_slots);
 
+/**
+ * drm_dp_helper_update_vcpi_slots_for_dsc() - Update VCPI slots with new on 
the state
+ *
+ * @state: global atomic state
+ * @port: port to find vcpi slots
+ * @pbn: updated bandwidth required for the mode in PBN
+ *
+ * Function reallocates VCPI slots to the @port by calling
+ * drm_dp_atomic_find_vcpi_slots. The assumption is that VCPI slots
+ * have already been allocated and this is second call overwritting
+ * initial values. After the VCPI is allocated dsc_enable flag is set to
+ * true for atomic check.
+ *
+ * It is driver's responsibility to call this function after it decides
+ * to enable DSC.
+ *
+ * See also:
+ * drm_dp_mst_update_dsc_crtcs()
+ *
+ * Returns:
+ * Total slots in the atomic state assigned for this port, or a negative error
+ * code if the port no longer exists or vcpi slots haven't been assigned.
+ */
+int drm_dp_helper_update_vcpi_slots_for_dsc(struct drm_atomic_state *state,
+   struct drm_dp_mst_port *port,
+   int pbn)
+{
+   struct drm_dp_mst_topology_state *topology_state;
+   struct drm_dp_vcpi_allocation *pos;
+   bool found = false;
+   int vcpi = 0;
+
+   topology_state = drm_atomic_get_mst_topology_state(state, port->mgr);
+
+   if (IS_ERR(topology_state))
+   return PTR_ERR(topology_state);
+
+   list_for_each_entry(pos, &topology_state->vcpis, next) {
+   if (pos->port == port) {
+   found = true;
+   break;
+   }
+   }
+
+   if (!found || !pos->vcpi)
+   return -EINVAL;
+
+   vcpi = drm_dp_atomic_find_vcpi_slots(state, port->mgr,
+port, pbn);
+
+   if (vcpi < 0)
+   return -EINVAL;
+
+   pos->dsc_enable = true;
+
+   return vcpi;
+}
+
+EXPORT_SYMBOL(drm_dp_helper_update_vcpi_slots_for_dsc);
 /**
  * drm_dp_atomic_release_vcpi_slots() - Release allocated vcpi slots
  * @state: global atomic state
@@ -3871,6 +3931,46 @@ drm_dp_mst_atomic_check_topology_state(struct 
drm_dp_mst_topology_mgr *mgr,
return 0;
 }
 
+/**
+ * drm_dp_mst_update_dsc_crtcs - Set mode change flag on CRTCs which
+ * just got DSC enabled
+ * @state: Pointer to the new &struct drm_dp_mst_topology_state
+ *
+ * Itearate through all the ports in MST topology to check if DSC
+ * has been enabled on any of them. Set mode_changed to true on
+ * crtc state that just got DSC enabled.
+ *
+ * See also:
+ * drm_dp_helper_update_vcpi_slots_for_dsc()
+ */
+static void
+drm_dp_mst_update_dsc_crtcs(struct drm_dp_mst_topology_state *mst_state)
+{
+   struct drm_dp_vcpi_allocation *pos;
+   struct drm_dp_mst_port *port;
+   struct drm_connector_state *conn_state;
+   struct drm_crtc *crtc;
+   struct drm_crtc_state *crtc_state;
+
+   list_for_each_entry(pos, &mst_state->vcpis, next) {
+
+   port = pos->port;
+   conn_state = 
drm_atomic_get_connector_state(mst_state->base.state,
+   port->connector);
+   crtc = conn_state->crtc;
+   if (!crtc)
+   continue;
+
+   crtc_state = drm_atomic_get_crtc_state(mst_state->base.state, 
crtc);
+   if (port->vcpi.vcpi == pos->vcpi)
+   continue;
+
+   if (pos->dsc_

[PATCH 11/13] drm/amd/display: MST DSC compute fair share

[mikita.lipski]

From: David Francis 

If there is limited link bandwidth on a MST network,
it must be divided fairly between the streams on that network

Implement an algorithm to determine the correct DSC config
for each stream

The algorithm:
This
 [   ]  ( )
represents the range of bandwidths possible for a given stream.
The [] area represents the range of DSC configs, and the ()
represents no DSC. The bandwidth used increases from left to right.

First, try disabling DSC on all streams
 [  ]  (|)
 [ ](|)
Check this against the bandwidth limits of the link and each branch
(including each endpoint). If it passes, the job is done

Second, try maximum DSC compression on all streams
that support DSC
 [| ]( )
 [|] ( )
If this does not pass, then enabling this combination of streams
is impossible

Otherwise, divide the remaining bandwidth evenly amongst the streams
 [|  ] ( )
 [|  ]( )

If one or more of the streams reach minimum compression, evenly
divide the reamining bandwidth amongst the remaining streams
 [|] ( )
 [   |]   ( )
 [ |   ]   ( )
 [ |  ]  ( )

If all streams can reach minimum compression, disable compression
greedily
 [  |]  ( )
 [|]( )
 [ ](|)

Perform this algorithm on each full update, on each MST link
with at least one DSC stream on it

After the configs are computed, call
dcn20_add_dsc_to_stream_resource on each stream with DSC enabled.
It is only after all streams are created that we can know which
of them will need DSC.

Do all of this at the end of amdgpu atomic check.  If it fails,
fail check; This combination of timings cannot be supported.

Reviewed-by: Wenjing Liu 
Signed-off-by: David Francis 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   4 +
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 386 ++
 .../display/amdgpu_dm/amdgpu_dm_mst_types.h   |   4 +
 .../drm/amd/display/dc/dcn20/dcn20_resource.c |   7 +-
 .../drm/amd/display/dc/dcn20/dcn20_resource.h |   1 +
 5 files changed, 400 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 1309e9cfdea3..1bbdf29f3c7d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -7780,6 +7780,10 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
if (ret)
goto fail;
 
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+   if (!compute_mst_dsc_configs_for_state(dm_state->context))
+   goto fail;
+#endif
if (dc_validate_global_state(dc, dm_state->context, false) != 
DC_OK) {
ret = -EINVAL;
goto fail;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 804a00082bee..c58cf41f3086 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -38,6 +38,8 @@
 
 #include "i2caux_interface.h"
 
+#include "dc/dcn20/dcn20_resource.h"
+
 /* #define TRACE_DPCD */
 
 #ifdef TRACE_DPCD
@@ -491,3 +493,387 @@ void amdgpu_dm_initialize_dp_connector(struct 
amdgpu_display_manager *dm,
aconnector->connector_id);
 }
 
+#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
+struct dsc_mst_fairness_params {
+   struct dc_crtc_timing *timing;
+   struct dc_sink *sink;
+   struct dc_dsc_bw_range bw_range;
+   bool compression_possible;
+   struct drm_dp_mst_port *port;
+};
+
+struct dsc_mst_fairness_vars {
+   int pbn;
+   bool dsc_enabled;
+   int bpp_x16;
+};
+
+static bool port_downstream_of_branch(struct drm_dp_mst_port *port,
+   struct drm_dp_mst_branch *branch)
+{
+   while (port->parent) {
+   if (port->parent == branch)
+   return true;
+
+   if (port->parent->port_parent)
+   port = port->parent->port_parent;
+   else
+   break;
+   }
+   return false;
+}
+
+static bool check_pbn_limit_on_branch(struct drm_dp_mst_branch *branch,
+   struct dsc_mst_fairness_params *params,
+   struct dsc_mst_fairness_vars *vars, int count)
+{
+   struct drm_dp_mst_port *port;
+   int i;
+   int pbn_limit = 0;
+   int pbn_used = 0;
+
+   list_for_each_entry(port, &branch->ports, next) {
+   if (port->mstb)
+   if (!check_pbn_limit_on_branch(port->mstb, params, 
vars, count))
+   return false;
+
+   if (por

[PATCH 0/2] Changes for DP 1.4 Compliance test 4.2.2.6

[Jerry (Fangzhi) Zuo]

Unlike DP 1.2 Compliance test 4.2.2.6, DP 1.4 requires to calculate real
CRC value of the last edid data block, and write it back.

Current edid CRC calculate routine adds the last CRC byte, and check if
non-zero or not. Need to return the actual CRC value when corruption is
detected.

Jerry (Fangzhi) Zuo (2):
  drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6
  drm/amd/display: Hook up drm interface for DP 1.4 edid corruption test

 .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 35 +
 drivers/gpu/drm/drm_dp_helper.c| 36 ++
 drivers/gpu/drm/drm_edid.c | 15 +++--
 include/drm/drm_connector.h|  7 +
 include/drm/drm_dp_helper.h|  3 ++
 5 files changed, 65 insertions(+), 31 deletions(-)

-- 
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/display: Hook up drm interface for DP 1.4 edid corruption test

[Jerry (Fangzhi) Zuo]

Signed-off-by: Jerry (Fangzhi) Zuo 
---
 .../drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c  | 35 +-
 1 file changed, 7 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
index 11e5784aa62a..c430890085f6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
@@ -575,6 +575,7 @@ enum dc_edid_status dm_helpers_read_local_edid(
struct dc_sink *sink)
 {
struct amdgpu_dm_connector *aconnector = link->priv;
+   struct drm_connector *connector = &aconnector->base;
struct i2c_adapter *ddc;
int retry = 3;
enum dc_edid_status edid_status;
@@ -592,6 +593,12 @@ enum dc_edid_status dm_helpers_read_local_edid(
 
edid = drm_get_edid(&aconnector->base, ddc);
 
+   if (link->aux_mode && connector->edid_corrupt)
+   
drm_dp_send_bad_edid_checksum(&aconnector->dm_dp_aux.aux, 
connector->bad_edid_checksum);
+
+   if (!edid && connector->edid_corrupt)
+   return EDID_BAD_CHECKSUM;
+
if (!edid)
return EDID_NO_RESPONSE;
 
@@ -612,34 +619,6 @@ enum dc_edid_status dm_helpers_read_local_edid(
DRM_ERROR("EDID err: %d, on connector: %s",
edid_status,
aconnector->base.name);
-   if (link->aux_mode) {
-   union test_request test_request = { {0} };
-   union test_response test_response = { {0} };
-
-   dm_helpers_dp_read_dpcd(ctx,
-   link,
-   DP_TEST_REQUEST,
-   &test_request.raw,
-   sizeof(union test_request));
-
-   if (!test_request.bits.EDID_READ)
-   return edid_status;
-
-   test_response.bits.EDID_CHECKSUM_WRITE = 1;
-
-   dm_helpers_dp_write_dpcd(ctx,
-   link,
-   DP_TEST_EDID_CHECKSUM,
-   
&sink->dc_edid.raw_edid[sink->dc_edid.length-1],
-   1);
-
-   dm_helpers_dp_write_dpcd(ctx,
-   link,
-   DP_TEST_RESPONSE,
-   &test_response.raw,
-   sizeof(test_response));
-
-   }
 
return edid_status;
 }
-- 
2.14.1

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

[PATCH 1/2] drm: Add support for DP 1.4 Compliance edid corruption test 4.2.2.6

[Jerry (Fangzhi) Zuo]

DP 1.4 edid corruption test requires source DUT to write calculated
CRC, not the corrupted CRC from reference sink.

Return the calculated CRC back, and initiate the required sequence.

Signed-off-by: Jerry (Fangzhi) Zuo 
---
 drivers/gpu/drm/drm_dp_helper.c | 36 
 drivers/gpu/drm/drm_edid.c  | 15 ---
 include/drm/drm_connector.h |  7 +++
 include/drm/drm_dp_helper.h |  3 +++
 4 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ffc68d305afe..75dbd30c62a7 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -336,6 +336,42 @@ int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 }
 EXPORT_SYMBOL(drm_dp_dpcd_read_link_status);
 
+/**
+  * drm_dp_send_bad_edid_checksum() - send back real edid checksum value
+  * @aux: DisplayPort AUX channel
+  * @bad_edid_checksum: real edid checksum for the last block
+  *
+  * Returns true on success
+  */
+bool drm_dp_send_bad_edid_checksum(struct drm_dp_aux *aux,
+u8 bad_edid_checksum)
+{
+u8 link_edid_read = 0, auto_test_req = 0;
+u8 test_resp = 0;
+
+drm_dp_dpcd_read(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 1);
+auto_test_req &= DP_AUTOMATED_TEST_REQUEST;
+
+drm_dp_dpcd_read(aux, DP_TEST_REQUEST, &link_edid_read, 1);
+link_edid_read &= DP_TEST_LINK_EDID_READ;
+
+if (!auto_test_req || !link_edid_read) {
+DRM_DEBUG_KMS("Source DUT does not support TEST_EDID_READ\n");
+return false;
+}
+
+drm_dp_dpcd_write(aux, DP_DEVICE_SERVICE_IRQ_VECTOR, &auto_test_req, 
1);
+
+/* send back checksum for the last edid extension block data */
+drm_dp_dpcd_write(aux, DP_TEST_EDID_CHECKSUM, &bad_edid_checksum, 1);
+
+test_resp |= DP_TEST_EDID_CHECKSUM_WRITE;
+drm_dp_dpcd_write(aux, DP_TEST_RESPONSE, &test_resp, 1);
+
+return true;
+}
+EXPORT_SYMBOL(drm_dp_send_bad_edid_checksum);
+
 /**
  * drm_dp_link_probe() - probe a DisplayPort link for capabilities
  * @aux: DisplayPort AUX channel
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 82a4ceed3fcf..400064dcc010 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1344,13 +1344,19 @@ static void drm_get_displayid(struct drm_connector 
*connector,
  struct edid *edid);
 static int validate_displayid(u8 *displayid, int length, int idx);
 
-static int drm_edid_block_checksum(const u8 *raw_edid)
+static int drm_edid_block_checksum(const u8 *raw_edid, bool c)
 {
int i;
u8 csum = 0;
-   for (i = 0; i < EDID_LENGTH; i++)
+   u8 len;
+
+   len = c ? EDID_LENGTH : (EDID_LENGTH - 1);
+
+   for (i = 0; i < len; i++)
csum += raw_edid[i];
 
+   csum = c ? csum : (0x100 - csum);
+
return csum;
 }
 
@@ -1408,7 +1414,7 @@ bool drm_edid_block_valid(u8 *raw_edid, int block, bool 
print_bad_edid,
}
}
 
-   csum = drm_edid_block_checksum(raw_edid);
+   csum = drm_edid_block_checksum(raw_edid, true);
if (csum) {
if (edid_corrupt)
*edid_corrupt = true;
@@ -1572,6 +1578,9 @@ static void connector_bad_edid(struct drm_connector 
*connector,
   prefix, DUMP_PREFIX_NONE, 16, 1,
   block, EDID_LENGTH, false);
}
+
+   /* Calculate real checksum for the last edid extension block data */
+   connector->bad_edid_checksum = drm_edid_block_checksum(edid + 
edid[0x7e] * EDID_LENGTH, false);
 }
 
 /* Get override or firmware EDID */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 681cb590f952..8442461542b9 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1345,6 +1345,13 @@ struct drm_connector {
 * rev1.1 4.2.2.6
 */
bool edid_corrupt;
+   /**
+ * @bad_edid_checksum: real edid checksum value for corrupted edid 
block.
+ * Required in Displayport 1.4 compliance testing
+ * rev1.1 4.2.2.6
+ */
+uint8_t bad_edid_checksum;
+
 
/** @debugfs_entry: debugfs directory for this connector */
struct dentry *debugfs_entry;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 5a795075d5da..2a7e54bebb18 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -1383,6 +1383,9 @@ static inline ssize_t drm_dp_dpcd_writeb(struct 
drm_dp_aux *aux,
 int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
 u8 status[DP_LINK_STATUS_SIZE]);
 
+bool drm_dp_send_bad_edid_checksum(struct drm_dp_aux *aux,
+   u8 bad_edid_checksum);
+
 /*
  * DisplayPort link
  */
-- 
2.14.1

[PATCH] drm/amdgpu: implement TMZ accessor

[Tuikov, Luben]

Implement an accessor of adev->tmz.enabled. Let not
code around access it as "if (adev->tmz.enabled)"
as the organization may change. Instead...

Recruit "bool amdgpu_is_tmz(adev)" to return
exactly this Boolean value. That is, this function
is now an accessor of an already initialized and
set adev and adev->tmz.

Add "void amdgpu_tmz_set(adev)" to check and set
adev->tmz.* at initialization time. After which
one uses "bool amdgpu_is_tmz(adev)" to query
whether adev supports TMZ.

Also, remove circular header file include.

Signed-off-by: Luben Tuikov 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h|  5 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c| 23 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h|  7 ++-
 4 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7d1e528cc783..23bd81a76570 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1249,5 +1249,10 @@ _name##_show(struct device *dev, 
\
\
 static struct device_attribute pmu_attr_##_name = __ATTR_RO(_name)
 
+static inline bool amdgpu_is_tmz(struct amdgpu_device *adev)
+{
+   return adev->tmz.enabled;
+}
+
 #endif
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 4eee40b9d0b0..f12b817480bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1058,7 +1058,7 @@ static int amdgpu_device_check_arguments(struct 
amdgpu_device *adev)
 
adev->firmware.load_type = amdgpu_ucode_get_load_type(adev, 
amdgpu_fw_load_type);
 
-   adev->tmz.enabled = amdgpu_is_tmz(adev);
+   amdgpu_tmz_set(adev);
 
return ret;
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
index 823527a0fa47..518b9d335550 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.c
@@ -27,26 +27,25 @@
 #include "amdgpu.h"
 #include "amdgpu_tmz.h"
 
-
 /**
- * amdgpu_is_tmz - validate trust memory zone
- *
+ * amdgpu_tmz_set -- check and set if a device supports TMZ
  * @adev: amdgpu_device pointer
  *
- * Return true if @dev supports trusted memory zones (TMZ), and return false if
- * @dev does not support TMZ.
+ * Check and set if an the device @adev supports Trusted Memory
+ * Zones (TMZ).
  */
-bool amdgpu_is_tmz(struct amdgpu_device *adev)
+void amdgpu_tmz_set(struct amdgpu_device *adev)
 {
if (!amdgpu_tmz)
-   return false;
+   return;
 
-   if (adev->asic_type < CHIP_RAVEN || adev->asic_type == CHIP_ARCTURUS) {
-   dev_warn(adev->dev, "doesn't support trusted memory zones 
(TMZ)\n");
-   return false;
+   if (adev->asic_type < CHIP_RAVEN ||
+   adev->asic_type == CHIP_ARCTURUS) {
+   dev_warn(adev->dev, "Trusted Memory Zone (TMZ) feature not 
supported\n");
+   return;
}
 
-   dev_info(adev->dev, "TMZ feature is enabled\n");
+   adev->tmz.enabled = true;
 
-   return true;
+   dev_info(adev->dev, "Trusted Memory Zone (TMZ) feature supported and 
enabled\n");
 }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
index 28e05177fb89..ad3ad8c011f9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_tmz.h
@@ -24,16 +24,13 @@
 #ifndef __AMDGPU_TMZ_H__
 #define __AMDGPU_TMZ_H__
 
-#include "amdgpu.h"
-
 /*
- * Trust memory zone stuff
+ * Trusted Memory Zone particulars
  */
 struct amdgpu_tmz {
boolenabled;
 };
 
-
-extern bool amdgpu_is_tmz(struct amdgpu_device *adev);
+extern void amdgpu_tmz_set(struct amdgpu_device *adev);
 
 #endif
-- 
2.23.0.385.gbc12974a89

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

[PATCH 2/3] drm/amdkfd: only keep release_mem function for Hawaii

[Zhao, Yong]

release_mem won't be used at all on GFX9 and GFX10, so delete it.

Change-Id: I13787a8a29b83e7516c582a7401f2e14721edf5f
Signed-off-by: Yong Zhao 
---
 .../gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c | 35 ++-
 .../gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c  | 33 ++---
 2 files changed, 4 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c
index aed32ab7102e..bfd6221acae9 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c
@@ -298,37 +298,6 @@ static int pm_query_status_v10(struct packet_manager *pm, 
uint32_t *buffer,
return 0;
 }
 
-
-static int pm_release_mem_v10(uint64_t gpu_addr, uint32_t *buffer)
-{
-   struct pm4_mec_release_mem *packet;
-
-   WARN_ON(!buffer);
-
-   packet = (struct pm4_mec_release_mem *)buffer;
-   memset(buffer, 0, sizeof(struct pm4_mec_release_mem));
-
-   packet->header.u32All = pm_build_pm4_header(IT_RELEASE_MEM,
-   sizeof(struct pm4_mec_release_mem));
-
-   packet->bitfields2.event_type = CACHE_FLUSH_AND_INV_TS_EVENT;
-   packet->bitfields2.event_index = 
event_index__mec_release_mem__end_of_pipe;
-   packet->bitfields2.tcl1_action_ena = 1;
-   packet->bitfields2.tc_action_ena = 1;
-   packet->bitfields2.cache_policy = cache_policy__mec_release_mem__lru;
-
-   packet->bitfields3.data_sel = 
data_sel__mec_release_mem__send_32_bit_low;
-   packet->bitfields3.int_sel =
-   int_sel__mec_release_mem__send_interrupt_after_write_confirm;
-
-   packet->bitfields4.address_lo_32b = (gpu_addr & 0x) >> 2;
-   packet->address_hi = upper_32_bits(gpu_addr);
-
-   packet->data_lo = 0;
-
-   return sizeof(struct pm4_mec_release_mem) / sizeof(unsigned int);
-}
-
 const struct packet_manager_funcs kfd_v10_pm_funcs = {
.map_process= pm_map_process_v10,
.runlist= pm_runlist_v10,
@@ -336,13 +305,13 @@ const struct packet_manager_funcs kfd_v10_pm_funcs = {
.map_queues = pm_map_queues_v10,
.unmap_queues   = pm_unmap_queues_v10,
.query_status   = pm_query_status_v10,
-   .release_mem= pm_release_mem_v10,
+   .release_mem= NULL,
.map_process_size   = sizeof(struct pm4_mes_map_process),
.runlist_size   = sizeof(struct pm4_mes_runlist),
.set_resources_size = sizeof(struct pm4_mes_set_resources),
.map_queues_size= sizeof(struct pm4_mes_map_queues),
.unmap_queues_size  = sizeof(struct pm4_mes_unmap_queues),
.query_status_size  = sizeof(struct pm4_mes_query_status),
-   .release_mem_size   = sizeof(struct pm4_mec_release_mem)
+   .release_mem_size   = 0,
 };
 
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
index 3b5ca2b1d7a6..f0e4910a8865 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
@@ -336,35 +336,6 @@ static int pm_query_status_v9(struct packet_manager *pm, 
uint32_t *buffer,
return 0;
 }
 
-
-static int pm_release_mem_v9(uint64_t gpu_addr, uint32_t *buffer)
-{
-   struct pm4_mec_release_mem *packet;
-
-   packet = (struct pm4_mec_release_mem *)buffer;
-   memset(buffer, 0, sizeof(struct pm4_mec_release_mem));
-
-   packet->header.u32All = pm_build_pm4_header(IT_RELEASE_MEM,
-   sizeof(struct pm4_mec_release_mem));
-
-   packet->bitfields2.event_type = CACHE_FLUSH_AND_INV_TS_EVENT;
-   packet->bitfields2.event_index = 
event_index__mec_release_mem__end_of_pipe;
-   packet->bitfields2.tcl1_action_ena = 1;
-   packet->bitfields2.tc_action_ena = 1;
-   packet->bitfields2.cache_policy = cache_policy__mec_release_mem__lru;
-
-   packet->bitfields3.data_sel = 
data_sel__mec_release_mem__send_32_bit_low;
-   packet->bitfields3.int_sel =
-   int_sel__mec_release_mem__send_interrupt_after_write_confirm;
-
-   packet->bitfields4.address_lo_32b = (gpu_addr & 0x) >> 2;
-   packet->address_hi = upper_32_bits(gpu_addr);
-
-   packet->data_lo = 0;
-
-   return 0;
-}
-
 const struct packet_manager_funcs kfd_v9_pm_funcs = {
.map_process= pm_map_process_v9,
.runlist= pm_runlist_v9,
@@ -372,12 +343,12 @@ const struct packet_manager_funcs kfd_v9_pm_funcs = {
.map_queues = pm_map_queues_v9,
.unmap_queues   = pm_unmap_queues_v9,
.query_status   = pm_query_status_v9,
-   .release_mem= pm_release_mem_v9,
+   .r

[PATCH 1/3] drm/amdkfd: Adjust function sequences to avoid unnecessary declarations

[Zhao, Yong]

This is cleaner.

Change-Id: I8cdecad387d8c547a088c6050f77385ee1135be1
Signed-off-by: Yong Zhao 
---
 .../gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c  | 19 +++
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
index 9a4bafb2e175..3b5ca2b1d7a6 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c
@@ -26,18 +26,6 @@
 #include "kfd_pm4_headers_ai.h"
 #include "kfd_pm4_opcodes.h"
 
-static bool initialize_v9(struct kernel_queue *kq, struct kfd_dev *dev,
-   enum kfd_queue_type type, unsigned int queue_size);
-static void uninitialize_v9(struct kernel_queue *kq);
-static void submit_packet_v9(struct kernel_queue *kq);
-
-void kernel_queue_init_v9(struct kernel_queue_ops *ops)
-{
-   ops->initialize = initialize_v9;
-   ops->uninitialize = uninitialize_v9;
-   ops->submit_packet = submit_packet_v9;
-}
-
 static bool initialize_v9(struct kernel_queue *kq, struct kfd_dev *dev,
enum kfd_queue_type type, unsigned int queue_size)
 {
@@ -67,6 +55,13 @@ static void submit_packet_v9(struct kernel_queue *kq)
kq->pending_wptr64);
 }
 
+void kernel_queue_init_v9(struct kernel_queue_ops *ops)
+{
+   ops->initialize = initialize_v9;
+   ops->uninitialize = uninitialize_v9;
+   ops->submit_packet = submit_packet_v9;
+}
+
 static int pm_map_process_v9(struct packet_manager *pm,
uint32_t *buffer, struct qcm_process_device *qpd)
 {
-- 
2.17.1

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

[PATCH 3/3] drm/amdkfd: Use kernel queue v9 functions for v10

[Zhao, Yong]

The kernel queue functions for v9 and v10 are the same except
pm_map_process_v* which have small difference, so they should be reused.
This eliminates the need of reapplying several patches which were
applied on v9 but not on v10, such as bigger GWS and more than 2
SDMA engine support which were introduced on Arcturus.

Change-Id: I2d385961e3c884db14e30b5afc98d0d9e4cb1802
Signed-off-by: Yong Zhao 
---
 drivers/gpu/drm/amd/amdkfd/Makefile   |   1 -
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c |   4 +-
 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h |   1 -
 .../gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c | 317 --
 .../gpu/drm/amd/amdkfd/kfd_kernel_queue_v9.c  |  49 ++-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h |   3 -
 6 files changed, 44 insertions(+), 331 deletions(-)
 delete mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c

diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile 
b/drivers/gpu/drm/amd/amdkfd/Makefile
index 48155060a57c..017a8b7156da 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -41,7 +41,6 @@ AMDKFD_FILES  := $(AMDKFD_PATH)/kfd_module.o \
$(AMDKFD_PATH)/kfd_kernel_queue_cik.o \
$(AMDKFD_PATH)/kfd_kernel_queue_vi.o \
$(AMDKFD_PATH)/kfd_kernel_queue_v9.o \
-   $(AMDKFD_PATH)/kfd_kernel_queue_v10.o \
$(AMDKFD_PATH)/kfd_packet_manager.o \
$(AMDKFD_PATH)/kfd_process_queue_manager.o \
$(AMDKFD_PATH)/kfd_device_queue_manager.o \
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
index 11d244891393..0d966408ea87 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c
@@ -332,12 +332,10 @@ struct kernel_queue *kernel_queue_init(struct kfd_dev 
*dev,
case CHIP_RAVEN:
case CHIP_RENOIR:
case CHIP_ARCTURUS:
-   kernel_queue_init_v9(&kq->ops_asic_specific);
-   break;
case CHIP_NAVI10:
case CHIP_NAVI12:
case CHIP_NAVI14:
-   kernel_queue_init_v10(&kq->ops_asic_specific);
+   kernel_queue_init_v9(&kq->ops_asic_specific);
break;
default:
WARN(1, "Unexpected ASIC family %u",
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h
index 365fc674fea4..a7116a939029 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.h
@@ -102,6 +102,5 @@ struct kernel_queue {
 void kernel_queue_init_cik(struct kernel_queue_ops *ops);
 void kernel_queue_init_vi(struct kernel_queue_ops *ops);
 void kernel_queue_init_v9(struct kernel_queue_ops *ops);
-void kernel_queue_init_v10(struct kernel_queue_ops *ops);
 
 #endif /* KFD_KERNEL_QUEUE_H_ */
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c
deleted file mode 100644
index bfd6221acae9..
--- a/drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue_v10.c
+++ /dev/null
@@ -1,317 +0,0 @@
-/*
- * Copyright 2018 Advanced Micro Devices, Inc.
- *
- * Permission is hereby granted, free of charge, to any person obtaining a
- * copy of this software and associated documentation files (the "Software"),
- * to deal in the Software without restriction, including without limitation
- * the rights to use, copy, modify, merge, publish, distribute, sublicense,
- * and/or sell copies of the Software, and to permit persons to whom the
- * Software is furnished to do so, subject to the following conditions:
- *
- * The above copyright notice and this permission notice shall be included in
- * all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
- * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
- * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
- * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
- * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
- * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
- * OTHER DEALINGS IN THE SOFTWARE.
- *
- */
-
-#include "kfd_kernel_queue.h"
-#include "kfd_device_queue_manager.h"
-#include "kfd_pm4_headers_ai.h"
-#include "kfd_pm4_opcodes.h"
-#include "gc/gc_10_1_0_sh_mask.h"
-
-static bool initialize_v10(struct kernel_queue *kq, struct kfd_dev *dev,
-   enum kfd_queue_type type, unsigned int queue_size);
-static void uninitialize_v10(struct kernel_queue *kq);
-static void submit_packet_v10(struct kernel_queue *kq);
-
-void kernel_queue_init_v10(struct kernel_queue_ops *ops)
-{
-   ops->initialize = initialize_v10;
-   ops->uninitialize = uninitialize_v10;
-   ops->submit_packet = submit_packet_v10;
-}
-
-static bool initialize_v10

[PATCH] drm/amd/powerplay: support xgmi pstate setting on powerplay routine

[Quan, Evan]

Add xgmi pstate setting on powerplay routine.

Change-Id: If1a49aa14c16f133e43ac1298c6b14eaeb44d79d
Signed-off-by: Evan Quan 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   |  5 +
 drivers/gpu/drm/amd/include/kgd_pp_interface.h |  4 
 drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 18 ++
 drivers/gpu/drm/amd/powerplay/amdgpu_smu.c |  3 +++
 .../gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 15 +++
 drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |  1 +
 drivers/gpu/drm/amd/powerplay/smu_v11_0.c  |  5 +
 7 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
index 00371713c671..167d9fbd2c4f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
@@ -285,6 +285,11 @@ int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, int 
pstate)
 
if (is_support_sw_smu_xgmi(adev))
ret = smu_set_xgmi_pstate(&adev->smu, pstate);
+   else if (adev->powerplay.pp_funcs &&
+adev->powerplay.pp_funcs->set_xgmi_pstate)
+   ret = 
adev->powerplay.pp_funcs->set_xgmi_pstate(adev->powerplay.pp_handle,
+   pstate);
+
if (ret)
dev_err(adev->dev,
"XGMI: Set pstate failure on device %llx, hive %llx, 
ret %d",
diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h 
b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
index 5902f80d1fce..a7f92d0b3a90 100644
--- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
+++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
@@ -220,6 +220,9 @@ enum pp_df_cstate {
((group) << PP_GROUP_SHIFT | (block) << PP_BLOCK_SHIFT | \
(support) << PP_STATE_SUPPORT_SHIFT | (state) << PP_STATE_SHIFT)
 
+#define XGMI_MODE_PSTATE_D3 0
+#define XGMI_MODE_PSTATE_D0 1
+
 struct seq_file;
 enum amd_pp_clock_type;
 struct amd_pp_simple_clock_info;
@@ -318,6 +321,7 @@ struct amd_pm_funcs {
int (*set_ppfeature_status)(void *handle, uint64_t ppfeature_masks);
int (*asic_reset_mode_2)(void *handle);
int (*set_df_cstate)(void *handle, enum pp_df_cstate state);
+   int (*set_xgmi_pstate)(void *handle, uint32_t pstate);
 };
 
 #endif
diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c 
b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
index f4ff15378e61..031447675203 100644
--- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
@@ -1566,6 +1566,23 @@ static int pp_set_df_cstate(void *handle, enum 
pp_df_cstate state)
return 0;
 }
 
+static int pp_set_xgmi_pstate(void *handle, uint32_t pstate)
+{
+   struct pp_hwmgr *hwmgr = handle;
+
+   if (!hwmgr)
+   return -EINVAL;
+
+   if (!hwmgr->pm_en || !hwmgr->hwmgr_func->set_xgmi_pstate)
+   return 0;
+
+   mutex_lock(&hwmgr->smu_lock);
+   hwmgr->hwmgr_func->set_xgmi_pstate(hwmgr, pstate);
+   mutex_unlock(&hwmgr->smu_lock);
+
+   return 0;
+}
+
 static const struct amd_pm_funcs pp_dpm_funcs = {
.load_firmware = pp_dpm_load_fw,
.wait_for_fw_loading_complete = pp_dpm_fw_loading_complete,
@@ -1625,4 +1642,5 @@ static const struct amd_pm_funcs pp_dpm_funcs = {
.asic_reset_mode_2 = pp_asic_reset_mode_2,
.smu_i2c_bus_access = pp_smu_i2c_bus_access,
.set_df_cstate = pp_set_df_cstate,
+   .set_xgmi_pstate = pp_set_xgmi_pstate,
 };
diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c 
b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
index 3ce01e1994fc..ca2dbef8670c 100644
--- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
@@ -529,6 +529,9 @@ bool is_support_sw_smu_xgmi(struct amdgpu_device *adev)
if (amdgpu_dpm != 1)
return false;
 
+   if (!is_support_sw_smu(adev))
+   return false;
+
if (adev->asic_type == CHIP_VEGA20)
return true;
 
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
index 9295bd90b792..5bcf0d684151 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
@@ -4176,6 +4176,20 @@ static int vega20_set_df_cstate(struct pp_hwmgr *hwmgr,
return ret;
 }
 
+static int vega20_set_xgmi_pstate(struct pp_hwmgr *hwmgr,
+ uint32_t pstate)
+{
+   int ret;
+
+   ret = smum_send_msg_to_smc_with_parameter(hwmgr,
+ PPSMC_MSG_SetXgmiMode,
+ pstate ? XGMI_MODE_PSTATE_D0 
: XGMI_MODE_PSTATE_D3);
+   if (ret)
+   pr_err("SetXgmiPstate failed!\n");
+
+   return ret;
+}
+
 static const struct pp_hwmgr_func vega20_hwmgr_funcs = {
/*

Re: [PATCH] drm/amd/powerplay: support xgmi pstate setting on powerplay routine

[Alex Deucher]

On Wed, Oct 30, 2019 at 9:50 PM Quan, Evan  wrote:
>
> Add xgmi pstate setting on powerplay routine.
>
> Change-Id: If1a49aa14c16f133e43ac1298c6b14eaeb44d79d
> Signed-off-by: Evan Quan 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c   |  5 +
>  drivers/gpu/drm/amd/include/kgd_pp_interface.h |  4 
>  drivers/gpu/drm/amd/powerplay/amd_powerplay.c  | 18 ++
>  drivers/gpu/drm/amd/powerplay/amdgpu_smu.c |  3 +++
>  .../gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c | 15 +++
>  drivers/gpu/drm/amd/powerplay/inc/hwmgr.h  |  1 +
>  drivers/gpu/drm/amd/powerplay/smu_v11_0.c  |  5 +
>  7 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 00371713c671..167d9fbd2c4f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -285,6 +285,11 @@ int amdgpu_xgmi_set_pstate(struct amdgpu_device *adev, 
> int pstate)
>
> if (is_support_sw_smu_xgmi(adev))
> ret = smu_set_xgmi_pstate(&adev->smu, pstate);
> +   else if (adev->powerplay.pp_funcs &&
> +adev->powerplay.pp_funcs->set_xgmi_pstate)
> +   ret = 
> adev->powerplay.pp_funcs->set_xgmi_pstate(adev->powerplay.pp_handle,
> +   pstate);
> +
> if (ret)
> dev_err(adev->dev,
> "XGMI: Set pstate failure on device %llx, hive %llx, 
> ret %d",
> diff --git a/drivers/gpu/drm/amd/include/kgd_pp_interface.h 
> b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> index 5902f80d1fce..a7f92d0b3a90 100644
> --- a/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_pp_interface.h
> @@ -220,6 +220,9 @@ enum pp_df_cstate {
> ((group) << PP_GROUP_SHIFT | (block) << PP_BLOCK_SHIFT | \
> (support) << PP_STATE_SUPPORT_SHIFT | (state) << 
> PP_STATE_SHIFT)
>
> +#define XGMI_MODE_PSTATE_D3 0
> +#define XGMI_MODE_PSTATE_D0 1
> +
>  struct seq_file;
>  enum amd_pp_clock_type;
>  struct amd_pp_simple_clock_info;
> @@ -318,6 +321,7 @@ struct amd_pm_funcs {
> int (*set_ppfeature_status)(void *handle, uint64_t ppfeature_masks);
> int (*asic_reset_mode_2)(void *handle);
> int (*set_df_cstate)(void *handle, enum pp_df_cstate state);
> +   int (*set_xgmi_pstate)(void *handle, uint32_t pstate);
>  };
>
>  #endif
> diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c 
> b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> index f4ff15378e61..031447675203 100644
> --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c
> @@ -1566,6 +1566,23 @@ static int pp_set_df_cstate(void *handle, enum 
> pp_df_cstate state)
> return 0;
>  }
>
> +static int pp_set_xgmi_pstate(void *handle, uint32_t pstate)
> +{
> +   struct pp_hwmgr *hwmgr = handle;
> +
> +   if (!hwmgr)
> +   return -EINVAL;
> +
> +   if (!hwmgr->pm_en || !hwmgr->hwmgr_func->set_xgmi_pstate)
> +   return 0;
> +
> +   mutex_lock(&hwmgr->smu_lock);
> +   hwmgr->hwmgr_func->set_xgmi_pstate(hwmgr, pstate);
> +   mutex_unlock(&hwmgr->smu_lock);
> +
> +   return 0;
> +}
> +
>  static const struct amd_pm_funcs pp_dpm_funcs = {
> .load_firmware = pp_dpm_load_fw,
> .wait_for_fw_loading_complete = pp_dpm_fw_loading_complete,
> @@ -1625,4 +1642,5 @@ static const struct amd_pm_funcs pp_dpm_funcs = {
> .asic_reset_mode_2 = pp_asic_reset_mode_2,
> .smu_i2c_bus_access = pp_smu_i2c_bus_access,
> .set_df_cstate = pp_set_df_cstate,
> +   .set_xgmi_pstate = pp_set_xgmi_pstate,
>  };
> diff --git a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c 
> b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> index 3ce01e1994fc..ca2dbef8670c 100644
> --- a/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/powerplay/amdgpu_smu.c
> @@ -529,6 +529,9 @@ bool is_support_sw_smu_xgmi(struct amdgpu_device *adev)
> if (amdgpu_dpm != 1)
> return false;
>
> +   if (!is_support_sw_smu(adev))
> +   return false;
> +
> if (adev->asic_type == CHIP_VEGA20)
> return true;
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c 
> b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> index 9295bd90b792..5bcf0d684151 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega20_hwmgr.c
> @@ -4176,6 +4176,20 @@ static int vega20_set_df_cstate(struct pp_hwmgr *hwmgr,
> return ret;
>  }
>
> +static int vega20_set_xgmi_pstate(struct pp_hwmgr *hwmgr,
> + uint32_t pstate)
> +{
> +   int ret;
> +
> +   ret = smum_send_msg_to_smc_with_parameter(hwmgr,
> +