Re: a quetion about buffer migration for user mapped bo.

2021-04-06 Thread صالح المسعودي
771763840
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: a quetion about buffer migration for user mapped bo.

2021-04-06 Thread صالح المسعودي
771763840
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH] drm/amdgpu: add DMUB outbox event IRQ source define/complete/debug flag

2021-04-06 Thread صالح المسعودي
771763840
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 0/4] Refine GPU recovery sequence to enhance its stability

2021-04-06 Thread Andrey Grodzovsky

Hey Christian, Denis, see bellow -

On 2021-04-06 6:34 a.m., Christian König wrote:

Hi Andrey,

well good question. My job is to watch over the implementation and 
design and while I always help I can adjust anybodies schedule.


Is the patch to print a warning when the hardware is accessed without 
holding the locks merged yet? If not then that would probably be a 
good starting point.



It's merged into amd-staging-drm-next and since I work on drm-misc-next 
I will cherry-pick it into there.





Then we would need to unify this with the SRCU to make sure that we 
have both the reset lock as well as block the hotplug code from 
reusing the MMIO space.


In my understanding there is a significant difference between handling 
of GPU reset and unplug - while GPU reset use case requires any HW 
accessing code to block and wait for the reset to finish and then 
proceed, hot-unplug
is permanent and hence no need to wait and proceed but rather abort at 
once. This why I think that in any place we already check for device 
reset we should also add a check for hot-unplug but the handling would 
be different

in that for hot-unplug we would abort instead of keep waiting.

Similar to handling device reset for unplug we obviously also need to 
stop and block any MMIO accesses once device is unplugged and, as Daniel 
Vetter mentioned - we have to do it before finishing pci_remove (early 
device fini)
and not later (when last device reference is dropped from user space) in 
order to prevent reuse of MMIO space we still access by other hot 
plugging devices. As in device reset case we need to cancel all delay 
works, stop drm schedule, complete all unfinished fences(both HW and 
scheduler fences). While you stated strong objection to force signalling 
scheduler fences from GPU reset, quote:


"you can't signal the dma_fence waiting. Waiting for a dma_fence also 
means you wait for the GPU reset to finish. When we would signal the 
dma_fence during the GPU reset then we would run into memory corruption 
because the hardware jobs running after the GPU reset would access 
memory which is already freed."
To my understating this is a key difference with hot-unplug, the device 
is gone, all those concerns are irrelevant and hence we can actually 
force signal scheduler fences (setting and error to them before) to 
force completion of any

waiting clients such as possibly IOCTLs or async page flips e.t.c.

Beyond blocking all delayed works and scheduler threads we also need to 
guarantee no  IOCTL can access MMIO post device unplug OR in flight 
IOCTLs are done before we finish pci_remove (amdgpu_pci_remove for us).
For this I suggest we do something like what we worked on with Takashi 
Iwai the ALSA maintainer recently when he helped implementing PCI BARs 
move support for snd_hda_intel. Take a look at
https://cgit.freedesktop.org/~agrodzov/linux/commit/?h=yadro/pcie_hotplug/movable_bars_v9.1=cbaa324799718e2b828a8c7b5b001dd896748497 
and

https://cgit.freedesktop.org/~agrodzov/linux/commit/?h=yadro/pcie_hotplug/movable_bars_v9.1=e36365d9ab5bbc30bdc221ab4b3437de34492440
We also had same issue there, how to prevent MMIO accesses while the 
BARs are migrating. What was done there is a refcount was added to count 
all IOCTLs in flight, for any in flight  IOCTL the BAR migration handler 
would
block for the refcount to drop to 0 before it would proceed, for any 
later IOCTL it stops and wait if device is in migration state. We even 
don't need the wait part, nothing to wait for, we just return with 
-ENODEV for this case.


The above approach should allow us to wait for all the IOCTLs in flight, 
together with stopping scheduler threads and cancelling and flushing all 
in flight work items and timers i think It should give as full solution 
for the hot-unplug case

of preventing any MMIO accesses post device pci_remove.

Let me know what you think guys.

Andrey




And then testing, testing, testing to see if we have missed something.

Christian.

Am 05.04.21 um 19:58 schrieb Andrey Grodzovsky:


Denis, Christian, are there any updates in the plan on how to move on 
with this ? As you know I need very similar code for my up-streaming 
of device hot-unplug. My latest solution 
(https://lists.freedesktop.org/archives/amd-gfx/2021-January/058606.html) 
was not acceptable because of low level guards on the register 
accessors level which was hurting performance. Basically I need a way 
to prevent any MMIO write accesses from kernel driver after device is 
removed (UMD accesses are taken care of by page faulting dummy page). 
We are using now hot-unplug code for Freemont program and so 
up-streaming became more of a priority then before. This MMIO access 
issue is currently my main blocker from up-streaming. Is there any 
way I can assist in pushing this on ?


Andrey

On 2021-03-18 5:51 a.m., Christian König wrote:

Am 18.03.21 um 10:30 schrieb Li, Dennis:


>>> The GPU reset doesn't complete the fences we wait for. It only 

Re: [PATCH] drm/radeon/ttm: Fix memory leak userptr pages

2021-04-06 Thread Alex Deucher
On Mon, Mar 22, 2021 at 6:34 AM Christian König
 wrote:
>
> Hi Daniel,
>
> Am 22.03.21 um 10:38 schrieb Daniel Gomez:
> > On Fri, 19 Mar 2021 at 21:29, Felix Kuehling  wrote:
> >> This caused a regression in kfdtest in a large-buffer stress test after
> >> memory allocation for user pages fails:
> > I'm sorry to hear that. BTW, I guess you meant amdgpu leak patch and
> > not this one.
> > Just some background for the mem leak patch if helps to understand this:
> > The leak was introduce here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0b988ca1c7c4c73983b4ea96ef7c2af2263c87eb
> > where the bound status was introduced for all drm drivers including
> > radeon and amdgpu. So this patch just reverts the logic to the
> > original code but keeping the bound status. In my case, the binding
> > code allocates the user pages memory and returns without bounding (at
> > amdgpu_gtt_mgr_has_gart_addr). So,
> > when the unbinding happens, the memory needs to be cleared to prevent the 
> > leak.
>
> Ah, now I understand what's happening here. Daniel your patch is not
> really correct.
>
> The problem is rather that we don't set the tt object to bound if it
> doesn't have a GTT address.
>
> Going to provide a patch for this.

Did this patch ever land?

Alex

>
> Regards,
> Christian.
>
> >
> >> [17359.536303] amdgpu: init_user_pages: Failed to get user pages: -16
> >> [17359.543746] BUG: kernel NULL pointer dereference, address: 
> >> 
> >> [17359.551494] #PF: supervisor read access in kernel mode
> >> [17359.557375] #PF: error_code(0x) - not-present page
> >> [17359.563247] PGD 0 P4D 0
> >> [17359.566514] Oops:  [#1] SMP PTI
> >> [17359.570728] CPU: 8 PID: 5944 Comm: kfdtest Not tainted 
> >> 5.11.0-kfd-fkuehlin #193
> >> [17359.578760] Hardware name: ASUS All Series/X99-E WS/USB 3.1, BIOS 3201 
> >> 06/17/2016
> >> [17359.586971] RIP: 0010:amdgpu_ttm_backend_unbind+0x52/0x110 [amdgpu]
> >> [17359.594075] Code: 48 39 c6 74 1b 8b 53 0c 48 8d bd 80 a1 ff ff e8 24 62 
> >> 00 00 85 c0 0f 85 ab 00 00 00 c6 43 54 00 5b 5d c3 48 8b 46 10 8b 4e 50 
> >> <48> 8b 30 48 85 f6 74 ba 8b 50 0c 48 8b bf 80 a1 ff ff 83 e1 01 45
> >> [17359.614340] RSP: 0018:a4764971fc98 EFLAGS: 00010206
> >> [17359.620315] RAX:  RBX: 950e8d4edf00 RCX: 
> >> 
> >> [17359.628204] RDX:  RSI: 950e8d4edf00 RDI: 
> >> 950eadec5e80
> >> [17359.636084] RBP: 950eadec5e80 R08:  R09: 
> >> 
> >> [17359.643958] R10: 0246 R11: 0001 R12: 
> >> 950c03377800
> >> [17359.651833] R13: 950eadec5e80 R14: 950c03377858 R15: 
> >> 
> >> [17359.659701] FS:  7febb20cb740() GS:950ebfc0() 
> >> knlGS:
> >> [17359.668528] CS:  0010 DS:  ES:  CR0: 80050033
> >> [17359.675012] CR2:  CR3: 0006d700e005 CR4: 
> >> 001706e0
> >> [17359.682883] Call Trace:
> >> [17359.686063]  amdgpu_ttm_backend_destroy+0x12/0x70 [amdgpu]
> >> [17359.692349]  ttm_bo_cleanup_memtype_use+0x37/0x60 [ttm]
> >> [17359.698307]  ttm_bo_release+0x278/0x5e0 [ttm]
> >> [17359.703385]  amdgpu_bo_unref+0x1a/0x30 [amdgpu]
> >> [17359.708701]  amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu+0x7e5/0x910 
> >> [amdgpu]
> >> [17359.716307]  kfd_ioctl_alloc_memory_of_gpu+0x11a/0x220 [amdgpu]
> >> [17359.723036]  kfd_ioctl+0x223/0x400 [amdgpu]
> >> [17359.728017]  ? kfd_dev_is_large_bar+0x90/0x90 [amdgpu]
> >> [17359.734152]  __x64_sys_ioctl+0x8b/0xd0
> >> [17359.738796]  do_syscall_64+0x2d/0x40
> >> [17359.743259]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >> [17359.749205] RIP: 0033:0x7febb083b6d7
> >> [17359.753681] Code: b3 66 90 48 8b 05 b1 47 2d 00 64 c7 00 26 00 00 00 48 
> >> c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 b8 10 00 00 00 0f 05 
> >> <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 81 47 2d 00 f7 d8 64 89 01 48
> >> [17359.774340] RSP: 002b:7ffdb5522cd8 EFLAGS: 0202 ORIG_RAX: 
> >> 0010
> >> [17359.782668] RAX: ffda RBX: 0001 RCX: 
> >> 7febb083b6d7
> >> [17359.790566] RDX: 7ffdb5522d60 RSI: c0284b16 RDI: 
> >> 0003
> >> [17359.798459] RBP: 7ffdb5522d10 R08: 7ffdb5522dd0 R09: 
> >> c404
> >> [17359.806352] R10:  R11: 0202 R12: 
> >> 559416e4e2aa
> >> [17359.814251] R13:  R14: 0021 R15: 
> >> 
> >> [17359.822140] Modules linked in: ip6table_filter ip6_tables 
> >> iptable_filter amdgpu x86_pkg_temp_thermal drm_ttm_helper ttm iommu_v2 
> >> gpu_sched ip_tables x_tables
> >> [17359.837776] CR2: 
> >> [17359.841888] ---[ end trace a6f27d64475b28c8 ]---
> >> [17359.847318] RIP: 0010:amdgpu_ttm_backend_unbind+0x52/0x110 [amdgpu]
> >> [17359.854479] Code: 48 39 c6 74 1b 8b 53 0c 48 8d bd 80 a1 ff ff e8 24 62 
> >> 00 00 85 c0 0f 85 ab 00 00 00 c6 43 

Re: [pull] amdgpu, radeon, ttm, sched drm-next-5.13

2021-04-06 Thread Alex Deucher
On Fri, Apr 2, 2021 at 12:22 PM Christian König
 wrote:
>
> Hey Alex,
>
> the TTM and scheduler changes should already be in the drm-misc-next
> branch (not 100% sure about the TTM patch, need to double check next week).
>

The TTM change is not in drm-misc yet.

> Could that cause problems when both are merged into drm-next?

Dave, Daniel, how do you want to handle this?  The duplicated patch is this one:
https://cgit.freedesktop.org/drm/drm-misc/commit/?id=ac4eb83ab255de9c31184df51fd1534ba36fd212
amdgpu has changes which depend on it.  The same patch is included in this PR.

Thanks,

Alex


>
> Thanks,
> Christian.
>
> Am 02.04.21 um 00:29 schrieb Alex Deucher:
> > Hi Dave, Daniel,
> >
> > New stuff for 5.13.  There are two small patches for ttm and scheduler
> > that were dependencies for amdgpu changes.
> >
> > The following changes since commit 2cbcb78c9ee5520c8d836c7ff57d1b60ebe8e9b7:
> >
> >Merge tag 'amd-drm-next-5.13-2021-03-23' of 
> > https://gitlab.freedesktop.org/agd5f/linux into drm-next (2021-03-26 
> > 15:53:21 +0100)
> >
> > are available in the Git repository at:
> >
> >https://gitlab.freedesktop.org/agd5f/linux.git 
> > tags/amd-drm-next-5.13-2021-04-01
> >
> > for you to fetch changes up to ef95d2a98d642a537190d73c45ae3c308afee890:
> >
> >drm/amdgpu/display: fix warning on 32 bit in dmub (2021-04-01 17:32:32 
> > -0400)
> >
> > 
> > amd-drm-next-5.13-2021-04-01:
> >
> > amdgpu:
> > - Re-enable GPU reset on VanGogh
> > - Enable DPM flags for SMART_SUSPEND and MAY_SKIP_RESUME
> > - Disentangle HG from vga_switcheroo
> > - S0ix fixes
> > - W=1 fixes
> > - Resource iterator fixes
> > - DMCUB updates
> > - UBSAN fixes
> > - More PM API cleanup
> > - Aldebaran updates
> > - Modifier fixes
> > - Enable VCN load balancing with asymmetric engines
> > - Rework BO structs
> > - Aldebaran reset support
> > - Initial LTTPR display work
> > - Display MALL fixes
> > - Fall back to YCbCr420 when YCbCr444 fails
> > - SR-IOV fixes
> > - Misc cleanups and fixes
> >
> > radeon:
> > - Typo fixes
> >
> > ttm:
> > - Handle cached requests (required for Aldebaran)
> >
> > scheduler:
> > - Fix runqueue selection when changing priorities (required to fix VCN
> >load balancing)
> >
> > 
> > Alex Deucher (20):
> >drm/amdgpu/display/dm: add missing parameter documentation
> >drm/amdgpu: Add additional Sienna Cichlid PCI ID
> >drm/amdgpu: add a dev_pm_ops prepare callback (v2)
> >drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and 
> > DPM_FLAG_SMART_SUSPEND flags (v2)
> >drm/amdgpu: disentangle HG systems from vgaswitcheroo
> >drm/amdgpu: rework S3/S4/S0ix state handling
> >drm/amdgpu: don't evict vram on APUs for suspend to ram (v4)
> >drm/amdgpu: clean up non-DC suspend/resume handling
> >drm/amdgpu: move s0ix check into amdgpu_device_ip_suspend_phase2 (v3)
> >drm/amdgpu: re-enable suspend phase 2 for S0ix
> >drm/amdgpu/swsmu: skip gfx cgpg on s0ix suspend
> >drm/amdgpu: update comments about s0ix suspend/resume
> >drm/amdgpu: drop S0ix checks around CG/PG in suspend
> >drm/amdgpu: skip kfd suspend/resume for S0ix
> >drm/amdgpu/display: restore AUX_DPHY_TX_CONTROL for DCN2.x
> >drm/amdgpu/display: fix memory leak for dimgrey cavefish
> >drm/amdgpu/pm: mark pcie link/speed arrays as const
> >drm/amdgpu/pm: bail on sysfs/debugfs queries during platform suspend
> >drm/amdgpu/vangogh: don't check for dpm in is_dpm_running when in 
> > suspend
> >drm/amdgpu/display: fix warning on 32 bit in dmub
> >
> > Alex Sierra (2):
> >drm/amdgpu: replace per_device_list by array
> >drm/amdgpu: ih reroute for newer asics than vega20
> >
> > Alvin Lee (1):
> >drm/amd/display: Change input parameter for set_drr
> >
> > Anson Jacob (2):
> >drm/amd/display: Fix UBSAN: shift-out-of-bounds warning
> >drm/amd/display: Removing unused code from dmub_cmd.h
> >
> > Anthony Koo (2):
> >drm/amd/display: [FW Promotion] Release 0.0.57
> >drm/amd/display: [FW Promotion] Release 0.0.58
> >
> > Aric Cyr (2):
> >drm/amd/display: 3.2.128
> >drm/amd/display: 3.2.129
> >
> > Arnd Bergmann (3):
> >amdgpu: avoid incorrect %hu format string
> >amdgpu: fix gcc -Wrestrict warning
> >amdgpu: securedisplay: simplify i2c hexdump output
> >
> > Bhaskar Chowdhury (6):
> >drm/amdgpu: Fix a typo
> >drm/amdgpu: Fix a typo
> >drm/atomic: Couple of typo fixes
> >drm/radeon/r600_cs: Few typo fixes
> >drm/amd/amdgpu/gfx_v7_0: Trivial typo fixes
> >drm/amd: Fix a typo in two different sentences
> >
> > Bindu Ramamurthy (1):
> >drm/amd/display: Allow idle optimization based on vblank.
> >
> > Chengming Gui (1):
> >

RE: [PATCH] drm/amdgpu: move mmhub ras_func init to ip specific file

2021-04-06 Thread Lazar, Lijo
[AMD Public Use]

Reviewed-by: Lijo Lazar 

-Original Message-
From: Zhang, Hawking  
Sent: Tuesday, April 6, 2021 9:35 PM
To: amd-gfx@lists.freedesktop.org; Lazar, Lijo ; Deucher, 
Alexander ; Clements, John 
Cc: Zhang, Hawking 
Subject: [PATCH] drm/amdgpu: move mmhub ras_func init to ip specific file

mmhub ras is always owned by gpu driver. ras_funcs initialization shall be done 
at ip level, instead of putting it in common gmc interface file

Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 19 ---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 19 +++
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 013efc746821..4d32233cde92 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -30,9 +30,6 @@
 #include "amdgpu_gmc.h"
 #include "amdgpu_ras.h"
 #include "amdgpu_xgmi.h"
-#include "mmhub_v1_0.h"
-#include "mmhub_v9_4.h"
-#include "mmhub_v1_7.h"
 
 /**
  * amdgpu_gmc_pdb0_alloc - allocate vram for pdb0 @@ -401,22 +398,6 @@ int 
amdgpu_gmc_ras_late_init(struct amdgpu_device *adev)
return r;
}
 
-   /* initialize mmhub ras funcs */
-   switch (adev->asic_type) {
-   case CHIP_VEGA20:
-   adev->mmhub.ras_funcs = _v1_0_ras_funcs;
-   break;
-   case CHIP_ARCTURUS:
-   adev->mmhub.ras_funcs = _v9_4_ras_funcs;
-   break;
-   case CHIP_ALDEBARAN:
-   adev->mmhub.ras_funcs = _v1_7_ras_funcs;
-   break;
-   default:
-   /* mmhub ras is not available */
-   break;
-   }
-
if (adev->mmhub.ras_funcs &&
adev->mmhub.ras_funcs->ras_late_init) {
r = adev->mmhub.ras_funcs->ras_late_init(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 64cd08ee8290..321caf77b0a2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1186,6 +1186,24 @@ static void gmc_v9_0_set_mmhub_funcs(struct 
amdgpu_device *adev)
}
 }
 
+static void gmc_v9_0_set_mmhub_ras_funcs(struct amdgpu_device *adev) {
+   switch (adev->asic_type) {
+   case CHIP_VEGA20:
+   adev->mmhub.ras_funcs = _v1_0_ras_funcs;
+   break;
+   case CHIP_ARCTURUS:
+   adev->mmhub.ras_funcs = _v9_4_ras_funcs;
+   break;
+   case CHIP_ALDEBARAN:
+   adev->mmhub.ras_funcs = _v1_7_ras_funcs;
+   break;
+   default:
+   /* mmhub ras is not available */
+   break;
+   }
+}
+
 static void gmc_v9_0_set_gfxhub_funcs(struct amdgpu_device *adev)  {
adev->gfxhub.funcs = _v1_0_funcs; @@ -1209,6 +1227,7 @@ static 
int gmc_v9_0_early_init(void *handle)
gmc_v9_0_set_irq_funcs(adev);
gmc_v9_0_set_umc_funcs(adev);
gmc_v9_0_set_mmhub_funcs(adev);
+   gmc_v9_0_set_mmhub_ras_funcs(adev);
gmc_v9_0_set_gfxhub_funcs(adev);
 
adev->gmc.shared_aperture_start = 0x2000ULL;
--
2.17.1
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdgpu: move mmhub ras_func init to ip specific file

2021-04-06 Thread Hawking Zhang
mmhub ras is always owned by gpu driver. ras_funcs
initialization shall be done at ip level, instead of
putting it in common gmc interface file

Signed-off-by: Hawking Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 19 ---
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 19 +++
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 013efc746821..4d32233cde92 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -30,9 +30,6 @@
 #include "amdgpu_gmc.h"
 #include "amdgpu_ras.h"
 #include "amdgpu_xgmi.h"
-#include "mmhub_v1_0.h"
-#include "mmhub_v9_4.h"
-#include "mmhub_v1_7.h"
 
 /**
  * amdgpu_gmc_pdb0_alloc - allocate vram for pdb0
@@ -401,22 +398,6 @@ int amdgpu_gmc_ras_late_init(struct amdgpu_device *adev)
return r;
}
 
-   /* initialize mmhub ras funcs */
-   switch (adev->asic_type) {
-   case CHIP_VEGA20:
-   adev->mmhub.ras_funcs = _v1_0_ras_funcs;
-   break;
-   case CHIP_ARCTURUS:
-   adev->mmhub.ras_funcs = _v9_4_ras_funcs;
-   break;
-   case CHIP_ALDEBARAN:
-   adev->mmhub.ras_funcs = _v1_7_ras_funcs;
-   break;
-   default:
-   /* mmhub ras is not available */
-   break;
-   }
-
if (adev->mmhub.ras_funcs &&
adev->mmhub.ras_funcs->ras_late_init) {
r = adev->mmhub.ras_funcs->ras_late_init(adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 64cd08ee8290..321caf77b0a2 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -1186,6 +1186,24 @@ static void gmc_v9_0_set_mmhub_funcs(struct 
amdgpu_device *adev)
}
 }
 
+static void gmc_v9_0_set_mmhub_ras_funcs(struct amdgpu_device *adev)
+{
+   switch (adev->asic_type) {
+   case CHIP_VEGA20:
+   adev->mmhub.ras_funcs = _v1_0_ras_funcs;
+   break;
+   case CHIP_ARCTURUS:
+   adev->mmhub.ras_funcs = _v9_4_ras_funcs;
+   break;
+   case CHIP_ALDEBARAN:
+   adev->mmhub.ras_funcs = _v1_7_ras_funcs;
+   break;
+   default:
+   /* mmhub ras is not available */
+   break;
+   }
+}
+
 static void gmc_v9_0_set_gfxhub_funcs(struct amdgpu_device *adev)
 {
adev->gfxhub.funcs = _v1_0_funcs;
@@ -1209,6 +1227,7 @@ static int gmc_v9_0_early_init(void *handle)
gmc_v9_0_set_irq_funcs(adev);
gmc_v9_0_set_umc_funcs(adev);
gmc_v9_0_set_mmhub_funcs(adev);
+   gmc_v9_0_set_mmhub_ras_funcs(adev);
gmc_v9_0_set_gfxhub_funcs(adev);
 
adev->gmc.shared_aperture_start = 0x2000ULL;
-- 
2.17.1

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


Re: [PATCH] drm/amdgpu: Add show_fdinfo() interface

2021-04-06 Thread Felix Kuehling

Am 2021-04-06 um 7:13 a.m. schrieb Christian König:
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
>>> @@ -1042,13 +1042,15 @@ int
>>> amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, u32 pasid,
>>>     struct dma_fence **ef)
>>>   {
>>>   struct amdgpu_device *adev = get_amdgpu_device(kgd);
>>> +    struct amdgpu_fpriv *fpriv;
>>>   struct amdgpu_vm *new_vm;
>>>   int ret;
>>>   -    new_vm = kzalloc(sizeof(*new_vm), GFP_KERNEL);
>>> -    if (!new_vm)
>>> +    fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
>>> +    if (!fpriv)
>>>   return -ENOMEM;
>>>   +    new_vm = >vm;
>>>   /* Initialize AMDGPU part of the VM */
>>>   ret = amdgpu_vm_init(adev, new_vm, AMDGPU_VM_CONTEXT_COMPUTE,
>>> pasid);
>>>   if (ret) {
>>> @@ -1063,12 +1065,14 @@ int
>>> amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, u32 pasid,
>>>     *vm = (void *) new_vm;
>>>   +    amdgpu_fdinfo_init(adev, fpriv, pasid);
>>> +
>>>   return 0;
>>>     init_kfd_vm_fail:
>>>   amdgpu_vm_fini(adev, new_vm);
>>>   amdgpu_vm_init_fail:
>>> -    kfree(new_vm);
>>> +    kfree(fpriv);
>>>   return ret;
>>>   }
>>>   @@ -1142,6 +1146,8 @@ void
>>> amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm)
>>>   {
>>>   struct amdgpu_device *adev = get_amdgpu_device(kgd);
>>>   struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
>>> +    struct amdgpu_fpriv *fpriv =
>>> +    container_of(avm, struct amdgpu_fpriv, vm);
>>>     if (WARN_ON(!kgd || !vm))
>>>   return;
>>> @@ -1149,8 +1155,9 @@ void
>>> amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm)
>>>   pr_debug("Destroying process vm %p\n", vm);
>>>     /* Release the VM context */
>>> +    amdgpu_fdinfo_fini(adev, fpriv);
>>>   amdgpu_vm_fini(adev, avm);
>>> -    kfree(vm);
>>> +    kfree(fpriv);
>
> Felix needs to take a look here, but that is most likely a no-go. On
> the other hand if you drop the amdgpu_proc structure that should
> become unnecessary.

This is legacy code to support really old versions of ROCm user mode
that didn't call kfd_ioctl_acquire_vm to grab the VM from a DRM device
file descriptor. This predates upstreaming of dGPU support in KFD. We
can probably get rid of amdgpu_amdkfd_gpuvm_(create/destroy)_process_vm
and the associated code paths in KFD now.

On any ROCm less than 3 years old we should be going through
amdgpu_amdkfd_gpuvm_(acquire/release)_process_vm.

Regards,
  Felix


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


Re: [pull] amdgpu, radeon, ttm, sched drm-next-5.13

2021-04-06 Thread Alex Deucher
On Tue, Apr 6, 2021 at 11:42 AM Felix Kuehling  wrote:
>
> Am 2021-04-01 um 6:29 p.m. schrieb Alex Deucher:
> > Hi Dave, Daniel,
> >
> > New stuff for 5.13.  There are two small patches for ttm and scheduler
> > that were dependencies for amdgpu changes.
> >
> > The following changes since commit 2cbcb78c9ee5520c8d836c7ff57d1b60ebe8e9b7:
> >
> >   Merge tag 'amd-drm-next-5.13-2021-03-23' of 
> > https://gitlab.freedesktop.org/agd5f/linux into drm-next (2021-03-26 
> > 15:53:21 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://gitlab.freedesktop.org/agd5f/linux.git 
> > tags/amd-drm-next-5.13-2021-04-01
> >
> > for you to fetch changes up to ef95d2a98d642a537190d73c45ae3c308afee890:
> >
> >   drm/amdgpu/display: fix warning on 32 bit in dmub (2021-04-01 17:32:32 
> > -0400)
> >
> > 
> > amd-drm-next-5.13-2021-04-01:
> >
> > amdgpu:
> > - Re-enable GPU reset on VanGogh
> > - Enable DPM flags for SMART_SUSPEND and MAY_SKIP_RESUME
> > - Disentangle HG from vga_switcheroo
> > - S0ix fixes
> > - W=1 fixes
> > - Resource iterator fixes
> > - DMCUB updates
> > - UBSAN fixes
> > - More PM API cleanup
> > - Aldebaran updates
> > - Modifier fixes
> > - Enable VCN load balancing with asymmetric engines
> > - Rework BO structs
> > - Aldebaran reset support
> > - Initial LTTPR display work
> > - Display MALL fixes
> > - Fall back to YCbCr420 when YCbCr444 fails
> > - SR-IOV fixes
> > - Misc cleanups and fixes
> >
> > radeon:
> > - Typo fixes
> >
> > ttm:
> > - Handle cached requests (required for Aldebaran)
> >
> > scheduler:
> > - Fix runqueue selection when changing priorities (required to fix VCN
> >   load balancing)
> >
> > 
> > Alex Deucher (20):
> >   drm/amdgpu/display/dm: add missing parameter documentation
> >   drm/amdgpu: Add additional Sienna Cichlid PCI ID
> >   drm/amdgpu: add a dev_pm_ops prepare callback (v2)
> >   drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and 
> > DPM_FLAG_SMART_SUSPEND flags (v2)
> >   drm/amdgpu: disentangle HG systems from vgaswitcheroo
> >   drm/amdgpu: rework S3/S4/S0ix state handling
> >   drm/amdgpu: don't evict vram on APUs for suspend to ram (v4)
> >   drm/amdgpu: clean up non-DC suspend/resume handling
> >   drm/amdgpu: move s0ix check into amdgpu_device_ip_suspend_phase2 (v3)
> >   drm/amdgpu: re-enable suspend phase 2 for S0ix
> >   drm/amdgpu/swsmu: skip gfx cgpg on s0ix suspend
> >   drm/amdgpu: update comments about s0ix suspend/resume
> >   drm/amdgpu: drop S0ix checks around CG/PG in suspend
> >   drm/amdgpu: skip kfd suspend/resume for S0ix
> >   drm/amdgpu/display: restore AUX_DPHY_TX_CONTROL for DCN2.x
> >   drm/amdgpu/display: fix memory leak for dimgrey cavefish
> >   drm/amdgpu/pm: mark pcie link/speed arrays as const
> >   drm/amdgpu/pm: bail on sysfs/debugfs queries during platform suspend
> >   drm/amdgpu/vangogh: don't check for dpm in is_dpm_running when in 
> > suspend
> >   drm/amdgpu/display: fix warning on 32 bit in dmub
> >
> > Alex Sierra (2):
> >   drm/amdgpu: replace per_device_list by array
> >   drm/amdgpu: ih reroute for newer asics than vega20
> >
> > Alvin Lee (1):
> >   drm/amd/display: Change input parameter for set_drr
> >
> > Anson Jacob (2):
> >   drm/amd/display: Fix UBSAN: shift-out-of-bounds warning
> >   drm/amd/display: Removing unused code from dmub_cmd.h
> >
> > Anthony Koo (2):
> >   drm/amd/display: [FW Promotion] Release 0.0.57
> >   drm/amd/display: [FW Promotion] Release 0.0.58
> >
> > Aric Cyr (2):
> >   drm/amd/display: 3.2.128
> >   drm/amd/display: 3.2.129
> >
> > Arnd Bergmann (3):
> >   amdgpu: avoid incorrect %hu format string
> >   amdgpu: fix gcc -Wrestrict warning
> >   amdgpu: securedisplay: simplify i2c hexdump output
> >
> > Bhaskar Chowdhury (6):
> >   drm/amdgpu: Fix a typo
> >   drm/amdgpu: Fix a typo
> >   drm/atomic: Couple of typo fixes
> >   drm/radeon/r600_cs: Few typo fixes
> >   drm/amd/amdgpu/gfx_v7_0: Trivial typo fixes
> >   drm/amd: Fix a typo in two different sentences
> >
> > Bindu Ramamurthy (1):
> >   drm/amd/display: Allow idle optimization based on vblank.
> >
> > Chengming Gui (1):
> >   drm/amd/amdgpu: set MP1 state to UNLOAD before reload its FW for 
> > vega20/ALDEBARAN
> >
> > Chris Park (1):
> >   drm/amd/display: Disable MALL when SMU not present
> >
> > Christian König (5):
> >   drm/amdgpu: remove irq_src->data handling
> >   drm/amdgpu: add the sched_score to amdgpu_ring_init
> >   drm/amdgpu: share scheduler score on VCN3 instances
> >   drm/sched: select new rq even if there is only one v3
> >   drm/amdgpu: load balance VCN3 decode as well v8
> >
> > Daniel Gomez (2):
> >   drm/amdgpu/ttm: Fix memory leak userptr pages
>
> This introduced a 

Re: [pull] amdgpu, radeon, ttm, sched drm-next-5.13

2021-04-06 Thread Felix Kuehling
Am 2021-04-01 um 6:29 p.m. schrieb Alex Deucher:
> Hi Dave, Daniel,
>
> New stuff for 5.13.  There are two small patches for ttm and scheduler
> that were dependencies for amdgpu changes.
>
> The following changes since commit 2cbcb78c9ee5520c8d836c7ff57d1b60ebe8e9b7:
>
>   Merge tag 'amd-drm-next-5.13-2021-03-23' of 
> https://gitlab.freedesktop.org/agd5f/linux into drm-next (2021-03-26 15:53:21 
> +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.freedesktop.org/agd5f/linux.git 
> tags/amd-drm-next-5.13-2021-04-01
>
> for you to fetch changes up to ef95d2a98d642a537190d73c45ae3c308afee890:
>
>   drm/amdgpu/display: fix warning on 32 bit in dmub (2021-04-01 17:32:32 
> -0400)
>
> 
> amd-drm-next-5.13-2021-04-01:
>
> amdgpu:
> - Re-enable GPU reset on VanGogh
> - Enable DPM flags for SMART_SUSPEND and MAY_SKIP_RESUME
> - Disentangle HG from vga_switcheroo
> - S0ix fixes
> - W=1 fixes
> - Resource iterator fixes
> - DMCUB updates
> - UBSAN fixes
> - More PM API cleanup
> - Aldebaran updates
> - Modifier fixes
> - Enable VCN load balancing with asymmetric engines
> - Rework BO structs
> - Aldebaran reset support
> - Initial LTTPR display work
> - Display MALL fixes
> - Fall back to YCbCr420 when YCbCr444 fails
> - SR-IOV fixes
> - Misc cleanups and fixes
>
> radeon:
> - Typo fixes
>
> ttm:
> - Handle cached requests (required for Aldebaran)
>
> scheduler:
> - Fix runqueue selection when changing priorities (required to fix VCN
>   load balancing)
>
> 
> Alex Deucher (20):
>   drm/amdgpu/display/dm: add missing parameter documentation
>   drm/amdgpu: Add additional Sienna Cichlid PCI ID
>   drm/amdgpu: add a dev_pm_ops prepare callback (v2)
>   drm/amdgpu: enable DPM_FLAG_MAY_SKIP_RESUME and DPM_FLAG_SMART_SUSPEND 
> flags (v2)
>   drm/amdgpu: disentangle HG systems from vgaswitcheroo
>   drm/amdgpu: rework S3/S4/S0ix state handling
>   drm/amdgpu: don't evict vram on APUs for suspend to ram (v4)
>   drm/amdgpu: clean up non-DC suspend/resume handling
>   drm/amdgpu: move s0ix check into amdgpu_device_ip_suspend_phase2 (v3)
>   drm/amdgpu: re-enable suspend phase 2 for S0ix
>   drm/amdgpu/swsmu: skip gfx cgpg on s0ix suspend
>   drm/amdgpu: update comments about s0ix suspend/resume
>   drm/amdgpu: drop S0ix checks around CG/PG in suspend
>   drm/amdgpu: skip kfd suspend/resume for S0ix
>   drm/amdgpu/display: restore AUX_DPHY_TX_CONTROL for DCN2.x
>   drm/amdgpu/display: fix memory leak for dimgrey cavefish
>   drm/amdgpu/pm: mark pcie link/speed arrays as const
>   drm/amdgpu/pm: bail on sysfs/debugfs queries during platform suspend
>   drm/amdgpu/vangogh: don't check for dpm in is_dpm_running when in 
> suspend
>   drm/amdgpu/display: fix warning on 32 bit in dmub
>
> Alex Sierra (2):
>   drm/amdgpu: replace per_device_list by array
>   drm/amdgpu: ih reroute for newer asics than vega20
>
> Alvin Lee (1):
>   drm/amd/display: Change input parameter for set_drr
>
> Anson Jacob (2):
>   drm/amd/display: Fix UBSAN: shift-out-of-bounds warning
>   drm/amd/display: Removing unused code from dmub_cmd.h
>
> Anthony Koo (2):
>   drm/amd/display: [FW Promotion] Release 0.0.57
>   drm/amd/display: [FW Promotion] Release 0.0.58
>
> Aric Cyr (2):
>   drm/amd/display: 3.2.128
>   drm/amd/display: 3.2.129
>
> Arnd Bergmann (3):
>   amdgpu: avoid incorrect %hu format string
>   amdgpu: fix gcc -Wrestrict warning
>   amdgpu: securedisplay: simplify i2c hexdump output
>
> Bhaskar Chowdhury (6):
>   drm/amdgpu: Fix a typo
>   drm/amdgpu: Fix a typo
>   drm/atomic: Couple of typo fixes
>   drm/radeon/r600_cs: Few typo fixes
>   drm/amd/amdgpu/gfx_v7_0: Trivial typo fixes
>   drm/amd: Fix a typo in two different sentences
>
> Bindu Ramamurthy (1):
>   drm/amd/display: Allow idle optimization based on vblank.
>
> Chengming Gui (1):
>   drm/amd/amdgpu: set MP1 state to UNLOAD before reload its FW for 
> vega20/ALDEBARAN
>
> Chris Park (1):
>   drm/amd/display: Disable MALL when SMU not present
>
> Christian König (5):
>   drm/amdgpu: remove irq_src->data handling
>   drm/amdgpu: add the sched_score to amdgpu_ring_init
>   drm/amdgpu: share scheduler score on VCN3 instances
>   drm/sched: select new rq even if there is only one v3
>   drm/amdgpu: load balance VCN3 decode as well v8
>
> Daniel Gomez (2):
>   drm/amdgpu/ttm: Fix memory leak userptr pages

This introduced a regression for KFD, which I pointed out at the time.
Was there ever a fix for that.

Regards,
  Felix


>   drm/radeon/ttm: Fix memory leak userptr pages
>
> David Galiffi (1):
>   drm/amd/display: Fixed Clock Recovery Sequence
>
> Dennis Li (1):
>   drm/amdgpu: add codes to capture invalid hardware access when 

Re: [amdgpu] Compute kernels still run when the host process exit?

2021-04-06 Thread Felix Kuehling
Am 2021-04-01 um 2:22 p.m. schrieb Alex Deucher:
> On Thu, Apr 1, 2021 at 10:08 AM Smith John  wrote:
>> Hi, when I killed an OpenCL host process, the kernels it launched were not 
>> terminated, and still run.
>>
>> My OpenCL runtime is AMDGPU-PRO 20.20. OS Ubuntu 18.04.5 with  Linux Kernel 
>> 5.4.53
>>
>> I was wondering if it was a bug or the driver did not implement this 
>> "watchdog" mechanism.
> In general, once you issue work on the GPU it has to run to
> completion.  It is not stopped if the application that issued it goes
> away.

If this is using KFD, we destroy user mode queues and free all the
process' memory at process termination. Because we support CWSR, we can
interrupt running shaders. Is this using the legacy OpenCL?

Regards,
  Felix


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


Re: [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function

2021-04-06 Thread Felix Kuehling
Am 2021-04-06 um 6:38 a.m. schrieb Thomas Zimmermann:
> Hi
>
> Am 06.04.21 um 11:35 schrieb Christian König:
>> Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:
>>> Moving the driver-specific mmap code into a GEM object function allows
>>> for using DRM helpers for various mmap callbacks.
>>>
>>> This change resolves several inconsistencies between regular mmap and
>>> prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
>>> areas. Previously it way only set for regular mmap calls, prime-based
>>> mmap used TTM's default vm_ops. The check for kfd_bo has been taken
>>> from amdgpu_verify_access(), which is not called any longer and has
>>> been removed.
>>>
>>> As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
>>> implemented in amdgpu's GEM code.
>>>
>>> Signed-off-by: Thomas Zimmermann 
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 -
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h |  2 -
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  4 +-
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 64 +++
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 71
>>> -
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  1 -
>>>   6 files changed, 66 insertions(+), 122 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>> index e0c4f7c7f1b9..19c5ab08d9ec 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
>>> @@ -42,52 +42,6 @@
>>>   #include 
>>>   #include 
>>> -/**
>>> - * amdgpu_gem_prime_mmap - _driver.gem_prime_mmap implementation
>>> - * @obj: GEM BO
>>> - * @vma: Virtual memory area
>>> - *
>>> - * Sets up a userspace mapping of the BO's memory in the given
>>> - * virtual memory area.
>>> - *
>>> - * Returns:
>>> - * 0 on success or a negative error code on failure.
>>> - */
>>> -int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
>>> -  struct vm_area_struct *vma)
>>> -{
>>> -    struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>>> -    struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>> -    unsigned asize = amdgpu_bo_size(bo);
>>> -    int ret;
>>> -
>>> -    if (!vma->vm_file)
>>> -    return -ENODEV;
>>> -
>>> -    if (adev == NULL)
>>> -    return -ENODEV;
>>> -
>>> -    /* Check for valid size. */
>>> -    if (asize < vma->vm_end - vma->vm_start)
>>> -    return -EINVAL;
>>> -
>>> -    if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
>>> -    (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
>>> -    return -EPERM;
>>> -    }
>>> -    vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
>>> -
>>> -    /* prime mmap does not need to check access, so allow here */
>>> -    ret = drm_vma_node_allow(>vma_node,
>>> vma->vm_file->private_data);
>>> -    if (ret)
>>> -    return ret;
>>> -
>>> -    ret = ttm_bo_mmap(vma->vm_file, vma, >mman.bdev);
>>> -    drm_vma_node_revoke(>vma_node, vma->vm_file->private_data);
>>> -
>>> -    return ret;
>>> -}
>>> -
>>>   static int
>>>   __dma_resv_make_exclusive(struct dma_resv *obj)
>>>   {
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
>>> index 39b5b9616fd8..3e93b9b407a9 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
>>> @@ -31,8 +31,6 @@ struct drm_gem_object
>>> *amdgpu_gem_prime_import(struct drm_device *dev,
>>>   struct dma_buf *dma_buf);
>>>   bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
>>>     struct amdgpu_bo *bo);
>>> -int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
>>> -  struct vm_area_struct *vma);
>>>   extern const struct dma_buf_ops amdgpu_dmabuf_ops;
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index 76f48f79c70b..e96d2758f4bb 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -1656,7 +1656,7 @@ static const struct file_operations
>>> amdgpu_driver_kms_fops = {
>>>   .flush = amdgpu_flush,
>>>   .release = drm_release,
>>>   .unlocked_ioctl = amdgpu_drm_ioctl,
>>> -    .mmap = amdgpu_mmap,
>>> +    .mmap = drm_gem_mmap,
>>>   .poll = drm_poll,
>>>   .read = drm_read,
>>>   #ifdef CONFIG_COMPAT
>>> @@ -1719,7 +1719,7 @@ static const struct drm_driver
>>> amdgpu_kms_driver = {
>>>   .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
>>>   .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
>>>   .gem_prime_import = amdgpu_gem_prime_import,
>>> -    .gem_prime_mmap = amdgpu_gem_prime_mmap,
>>> +    .gem_prime_mmap = drm_gem_prime_mmap,
>>>   .name = DRIVER_NAME,
>>>   .desc = DRIVER_DESC,
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>>> 

Re: [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function

2021-04-06 Thread Felix Kuehling
Am 2021-04-06 um 9:04 a.m. schrieb Christian König:
> Am 06.04.21 um 14:52 schrieb Thomas Zimmermann:
>> Hi
>>
>> Am 06.04.21 um 14:42 schrieb Christian König:
>>> Hi Thomas,
>>>
>>> Am 06.04.21 um 13:55 schrieb Thomas Zimmermann:
 Hi

 Am 06.04.21 um 12:56 schrieb Christian König:
>>
>> In the end I went with the semantics I found in amdgpu_mmap() and
>> handled KFD specially. Let me know if this requires to be changed.
>
> Well the question is where is the call to
> drm_vma_node_verify_access() now? Cause that needs to be skipped
> for KFD BOs.

 I see. It's now drm_vma_node_is_allowed(); called by
 drm_gem_mmap(). [1] So drm_gem_mmap() cannot be used by amdgpu.

 If I understand the code at [2] correctly, KFD objects don't use
 the GEM ioctl interfaces, but they still use the internal GEM
 object that is part of the TTM BO. In this case, amdgpu could have
 its own version of drm_gem_mmap(), which calls drm_gem_mmap_obj(),
 [3] which in turn handles the mmap details via GEM object functions.
>>>
>>> Correct, well we could cleanup the KFD to use the GEM functions as
>>> well.
>>
>> The KFD code already calls amdgpu_gem_object_create(). It should have
>> the object-functions pointer set for use with mmap. Not sure what the
>> use of drm_vma_node_is_allowed() would involve.
>
> The KFD allows BOs to be mmaped with different offsets than what's
> used in the DRM node.
>
> So drm_vma_node_is_allowed() would return false as far as I know.

We used to mmap KFD BOs through the /dev/kfd file descriptor. We moved
that to using the /dev/dri/renderD* file descriptors a long time ago. If
there is some KFD special casing left in the code for BO mmap, it's
probably an oversight and we should be able to remove it.

We still have a few special mmaps in /dev/kfd, but they are for things
that don't involve GEM BOs that could be mmapped through the render
node: doorbells, MMIO pages and CWSR trap-handler mappings for APUs.

Regards,
  Felix


>
> Regards,
> Christian.
>
>>
>> Best regards
>> Thomas
>>
>>>
>>> Felix what exactly was your objections to using this?
>>>
>>> Regards,
>>> Christian.
>>>

 drm_gem_prime_mmap() doesn't do any additional verification.

 Best regards
 Thomas

 [1]
 https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/drm_gem.c#L1156

 [2]
 https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c#L1224

 [3]
 https://elixir.bootlin.com/linux/v5.12-rc6/source/drivers/gpu/drm/drm_gem.c#L1053



>
> Regards,
> Christian.
>
>>
>> Best regards
>> Thomas
>>
 -
   int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t
 src_offset,
  uint64_t dst_offset, uint32_t byte_count,
  struct dma_resv *resv,
 diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
 b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
 index dec0db8b0b13..6e51faad7371 100644
 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
 +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
 @@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
   struct dma_resv *resv,
   struct dma_fence **fence);
 -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
   int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
   int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
   uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev,
 uint32_t type);
>>>
>>> ___
>>> dri-devel mailing list
>>> dri-de...@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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


RE: [PATCH] drm/amdgpu: add DMUB outbox event IRQ source define/complete/debug flag

2021-04-06 Thread Shih, Jude
[AMD Official Use Only - Internal Distribution Only]

Hi Nicholas,

Let me summarize the suggestion. Please correct me if I misunderstood.

1. for dmub_aux_transfer_done, move it to amdgpu_display_manager in amdgpu_dm.h 
instead of amdgpu.h
2. Remove DC_ENABLE_DMUB_AUX from amd_shared.h at current stage since we don't 
have a mechanism to check the firmware yet. We just hardcore this flag when 
testing.
3. Keep the irq source change in irqsrcs_dcn_1_0.h at current stage since we 
don't have irqsrc_dcn_2_1.h

=> So I will just keep the change in irqsrcs_dcn_1_0.h and dmub_outbox_irq in 
amdgpu.h?

Thanks!

Best Regards,

Jude
-Original Message-
From: Kazlauskas, Nicholas  
Sent: Tuesday, April 6, 2021 10:30 PM
To: Shih, Jude ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Lin, Wayne 
; Hung, Cruise 
Subject: Re: [PATCH] drm/amdgpu: add DMUB outbox event IRQ source 
define/complete/debug flag

On 2021-04-06 10:22 a.m., Shih, Jude wrote:
> [AMD Official Use Only - Internal Distribution Only]
> 
> Hi Nicholas,
> 
> Does this completion need to be on the amdgpu device itself?
> 
> I would prefer if we keep this as needed within DM itself if possible.
> 
> => do you mean move it to amdgpu_display_manager in amdgpu_dm.h as global 
> variable?

There's a amdgpu_display_manager per device, but yes, it should be contained in 
there if possible since it's display code.

> 
> My problem with still leaving this as DC_ENABLE_DMUB_AUX is we shouldn't 
> require the user to have to flip this on by default later. I think I'd prefer 
> this still as a DISABLE option if we want to leave it for users to debug any 
> potential issues.
> => do you mean DC_ENABLE_DMUB_AUX = 0x10 => DC_DISABLE_DMUB_AUX = 0x10 
> and amdgpu_dc_debug_mask = 0x10 as default to turn it off?

Don't modify the default debug mask and leave it alone. We can still have 
DC_DISABLE_DMUB_AUX = 0x10 as a user debug option if they have firmware that 
supports this.

Flag or not, we need a mechanism from driver to firmware to query whether the 
firmware supports it in the first place. It's not sufficient to fully control 
this feature with just a debug flag, there needs to be a cap check regardless 
with the firmware for support.

Older firmware won't implement this check and therefore won't enable the 
feature.

Newer (or test) firmware could enable this feature and report back to driver 
that it does support it.

Driver can then decide to enable this based on 
dc->debug.dmub_aux_support or something similar to that - it can be
false or ASIC that we won't be supporting this on, but for ASIC that we do we 
can leave it off by default until it's production ready.

For developer testing we can hardcode the flag = true, I think the DC debug 
flags here in AMDGPU base driver only have value if we want general end user or 
validation to use this to debug potential issues.

Regards,
Nicholas Kazlauskas

> 
> Thanks,
> 
> Best Regards,
> 
> Jude
> 
> -Original Message-
> From: Kazlauskas, Nicholas 
> Sent: Tuesday, April 6, 2021 10:04 PM
> To: Shih, Jude ; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Lin, Wayne 
> ; Hung, Cruise 
> Subject: Re: [PATCH] drm/amdgpu: add DMUB outbox event IRQ source 
> define/complete/debug flag
> 
> On 2021-04-06 9:40 a.m., Jude Shih wrote:
>> [Why & How]
>> We use outbox interrupt that allows us to do the AUX via DMUB 
>> Therefore, we need to add some irq source related definition in the 
>> header files; Also, I added debug flag that allows us to turn it 
>> on/off for testing purpose.
>>
>> Signed-off-by: Jude Shih 
>> ---
>>drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 2 ++
>>drivers/gpu/drm/amd/include/amd_shared.h  | 3 ++-
>>drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h | 2 ++
>>3 files changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 963ecfd84347..7e64fc5e0dcd 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -923,6 +923,7 @@ struct amdgpu_device {
>>  struct amdgpu_irq_src   pageflip_irq;
>>  struct amdgpu_irq_src   hpd_irq;
>>  struct amdgpu_irq_src   dmub_trace_irq;
>> +struct amdgpu_irq_src   dmub_outbox_irq;
>>
>>  /* rings */
>>  u64 fence_context;
>> @@ -1077,6 +1078,7 @@ struct amdgpu_device {
>>
>>  boolin_pci_err_recovery;
>>  struct pci_saved_state  *pci_state;
>> +struct completion dmub_aux_transfer_done;
> 
> Does this completion need to be on the amdgpu device itself?
> 
> I would prefer if we keep this as needed within DM itself if possible.
> 
>>};
>>
>>static inline struct amdgpu_device *drm_to_adev(struct drm_device
>> *ddev) diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
>> 

[PATCH] drm/amd/pm: convert sysfs snprintf to sysfs_emit

2021-04-06 Thread Carlis
From: Xuezhi Zhang 

Fix the following coccicheck warning:
drivers/gpu/drm/amd/pm//amdgpu_pm.c:1940:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:1978:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2022:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:294:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:154:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:496:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:512:9-17: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:1740:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:1667:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2074:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2047:9-17: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2768:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2738:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2442:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:3246:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:3253:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2458:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:3047:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:3133:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:3209:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:3216:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2410:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2496:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2470:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2426:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2965:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:2972:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:3006:8-16: 
WARNING: use scnprintf or sprintf
drivers/gpu/drm/amd/pm//amdgpu_pm.c:3013:8-16: 
WARNING: use scnprintf or sprintf

Signed-off-by: Xuezhi Zhang 
---
 drivers/gpu/drm/amd/pm/amdgpu_pm.c | 58 +++---
 1 file changed, 29 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/amd/pm/amdgpu_pm.c 
b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
index 5fa65f191a37..2777966ec1ca 100644
--- a/drivers/gpu/drm/amd/pm/amdgpu_pm.c
+++ b/drivers/gpu/drm/amd/pm/amdgpu_pm.c
@@ -151,7 +151,7 @@ static ssize_t amdgpu_get_power_dpm_state(struct device 
*dev,
pm_runtime_mark_last_busy(ddev->dev);
pm_runtime_put_autosuspend(ddev->dev);
 
-   return snprintf(buf, PAGE_SIZE, "%s\n",
+   return sysfs_emit(buf, "%s\n",
(pm == POWER_STATE_TYPE_BATTERY) ? "battery" :
(pm == POWER_STATE_TYPE_BALANCED) ? "balanced" : 
"performance");
 }
@@ -291,7 +291,7 @@ static ssize_t 
amdgpu_get_power_dpm_force_performance_level(struct device *dev,
pm_runtime_mark_last_busy(ddev->dev);
pm_runtime_put_autosuspend(ddev->dev);
 
-   return snprintf(buf, PAGE_SIZE, "%s\n",
+   return sysfs_emit(buf, "%s\n",
(level == AMD_DPM_FORCED_LEVEL_AUTO) ? "auto" :
(level == AMD_DPM_FORCED_LEVEL_LOW) ? "low" :
(level == AMD_DPM_FORCED_LEVEL_HIGH) ? "high" :
@@ -493,7 +493,7 @@ static ssize_t amdgpu_get_pp_cur_state(struct device *dev,
if (i == data.nums)
i = -EINVAL;
 
-   return snprintf(buf, PAGE_SIZE, "%d\n", i);
+   return sysfs_emit(buf, "%d\n", i);
 }
 
 static ssize_t amdgpu_get_pp_force_state(struct device *dev,
@@ -509,7 +509,7 @@ static ssize_t amdgpu_get_pp_force_state(struct device *dev,
if (adev->pp_force_state_enabled)
return amdgpu_get_pp_cur_state(dev, attr, buf);
else
-   return snprintf(buf, PAGE_SIZE, "\n");
+   return sysfs_emit(buf, "\n");
 }
 
 static ssize_t amdgpu_set_pp_force_state(struct device *dev,
@@ -1664,7 +1664,7 @@ static ssize_t amdgpu_get_pp_sclk_od(struct device *dev,
pm_runtime_mark_last_busy(ddev->dev);
pm_runtime_put_autosuspend(ddev->dev);
 
-   return snprintf(buf, PAGE_SIZE, "%d\n", value);
+   return sysfs_emit(buf, "%d\n", value);
 }
 
 static ssize_t amdgpu_set_pp_sclk_od(struct device *dev,
@@ -1737,7 +1737,7 @@ static ssize_t amdgpu_get_pp_mclk_od(struct device *dev,
pm_runtime_mark_last_busy(ddev->dev);
pm_runtime_put_autosuspend(ddev->dev);
 
-   return snprintf(buf, PAGE_SIZE, "%d\n", value);
+   return sysfs_emit(buf, 

Re: [PATCH] drm/amdgpu: add DMUB outbox event IRQ source define/complete/debug flag

2021-04-06 Thread Kazlauskas, Nicholas

On 2021-04-06 10:22 a.m., Shih, Jude wrote:

[AMD Official Use Only - Internal Distribution Only]

Hi Nicholas,

Does this completion need to be on the amdgpu device itself?

I would prefer if we keep this as needed within DM itself if possible.

=> do you mean move it to amdgpu_display_manager in amdgpu_dm.h as global 
variable?


There's a amdgpu_display_manager per device, but yes, it should be 
contained in there if possible since it's display code.




My problem with still leaving this as DC_ENABLE_DMUB_AUX is we shouldn't 
require the user to have to flip this on by default later. I think I'd prefer 
this still as a DISABLE option if we want to leave it for users to debug any 
potential issues.
=> do you mean DC_ENABLE_DMUB_AUX = 0x10 => DC_DISABLE_DMUB_AUX = 0x10
and amdgpu_dc_debug_mask = 0x10 as default to turn it off?


Don't modify the default debug mask and leave it alone. We can still 
have DC_DISABLE_DMUB_AUX = 0x10 as a user debug option if they have 
firmware that supports this.


Flag or not, we need a mechanism from driver to firmware to query 
whether the firmware supports it in the first place. It's not sufficient 
to fully control this feature with just a debug flag, there needs to be 
a cap check regardless with the firmware for support.


Older firmware won't implement this check and therefore won't enable the 
feature.


Newer (or test) firmware could enable this feature and report back to 
driver that it does support it.


Driver can then decide to enable this based on 
dc->debug.dmub_aux_support or something similar to that - it can be 
false or ASIC that we won't be supporting this on, but for ASIC that we 
do we can leave it off by default until it's production ready.


For developer testing we can hardcode the flag = true, I think the DC 
debug flags here in AMDGPU base driver only have value if we want 
general end user or validation to use this to debug potential issues.


Regards,
Nicholas Kazlauskas



Thanks,

Best Regards,

Jude

-Original Message-
From: Kazlauskas, Nicholas 
Sent: Tuesday, April 6, 2021 10:04 PM
To: Shih, Jude ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Lin, Wayne ; 
Hung, Cruise 
Subject: Re: [PATCH] drm/amdgpu: add DMUB outbox event IRQ source 
define/complete/debug flag

On 2021-04-06 9:40 a.m., Jude Shih wrote:

[Why & How]
We use outbox interrupt that allows us to do the AUX via DMUB
Therefore, we need to add some irq source related definition in the
header files; Also, I added debug flag that allows us to turn it
on/off for testing purpose.

Signed-off-by: Jude Shih 
---
   drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 2 ++
   drivers/gpu/drm/amd/include/amd_shared.h  | 3 ++-
   drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h | 2 ++
   3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 963ecfd84347..7e64fc5e0dcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -923,6 +923,7 @@ struct amdgpu_device {
struct amdgpu_irq_src   pageflip_irq;
struct amdgpu_irq_src   hpd_irq;
struct amdgpu_irq_src   dmub_trace_irq;
+   struct amdgpu_irq_src   dmub_outbox_irq;
   
   	/* rings */

u64 fence_context;
@@ -1077,6 +1078,7 @@ struct amdgpu_device {
   
   	boolin_pci_err_recovery;

struct pci_saved_state  *pci_state;
+   struct completion dmub_aux_transfer_done;


Does this completion need to be on the amdgpu device itself?

I would prefer if we keep this as needed within DM itself if possible.


   };
   
   static inline struct amdgpu_device *drm_to_adev(struct drm_device

*ddev) diff --git a/drivers/gpu/drm/amd/include/amd_shared.h
b/drivers/gpu/drm/amd/include/amd_shared.h
index 43ed6291b2b8..097672cc78a1 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -227,7 +227,8 @@ enum DC_DEBUG_MASK {
DC_DISABLE_PIPE_SPLIT = 0x1,
DC_DISABLE_STUTTER = 0x2,
DC_DISABLE_DSC = 0x4,
-   DC_DISABLE_CLOCK_GATING = 0x8
+   DC_DISABLE_CLOCK_GATING = 0x8,
+   DC_ENABLE_DMUB_AUX = 0x10,


My problem with still leaving this as DC_ENABLE_DMUB_AUX is we shouldn't 
require the user to have to flip this on by default later. I think I'd prefer 
this still as a DISABLE option if we want to leave it for users to debug any 
potential issues.

If there's no value in having end users debug issues by setting this bit then we 
should keep it as a dc->debug default in DCN resource.

Regards,
Nicholas Kazlauskas


   };
   
   enum amd_dpm_forced_level;

diff --git a/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
b/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
index e2bffcae273a..754170a86ea4 100644
--- 

RE: [PATCH] drm/amdgpu: add DMUB outbox event IRQ source define/complete/debug flag

2021-04-06 Thread Shih, Jude
[AMD Official Use Only - Internal Distribution Only]

Hi Nicholas,

Does this completion need to be on the amdgpu device itself?

I would prefer if we keep this as needed within DM itself if possible.

=> do you mean move it to amdgpu_display_manager in amdgpu_dm.h as global 
variable?

My problem with still leaving this as DC_ENABLE_DMUB_AUX is we shouldn't 
require the user to have to flip this on by default later. I think I'd prefer 
this still as a DISABLE option if we want to leave it for users to debug any 
potential issues.
=> do you mean DC_ENABLE_DMUB_AUX = 0x10 => DC_DISABLE_DMUB_AUX = 0x10
and amdgpu_dc_debug_mask = 0x10 as default to turn it off?

Thanks,

Best Regards,

Jude

-Original Message-
From: Kazlauskas, Nicholas  
Sent: Tuesday, April 6, 2021 10:04 PM
To: Shih, Jude ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Lin, Wayne 
; Hung, Cruise 
Subject: Re: [PATCH] drm/amdgpu: add DMUB outbox event IRQ source 
define/complete/debug flag

On 2021-04-06 9:40 a.m., Jude Shih wrote:
> [Why & How]
> We use outbox interrupt that allows us to do the AUX via DMUB 
> Therefore, we need to add some irq source related definition in the 
> header files; Also, I added debug flag that allows us to turn it 
> on/off for testing purpose.
> 
> Signed-off-by: Jude Shih 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 2 ++
>   drivers/gpu/drm/amd/include/amd_shared.h  | 3 ++-
>   drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h | 2 ++
>   3 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 963ecfd84347..7e64fc5e0dcd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -923,6 +923,7 @@ struct amdgpu_device {
>   struct amdgpu_irq_src   pageflip_irq;
>   struct amdgpu_irq_src   hpd_irq;
>   struct amdgpu_irq_src   dmub_trace_irq;
> + struct amdgpu_irq_src   dmub_outbox_irq;
>   
>   /* rings */
>   u64 fence_context;
> @@ -1077,6 +1078,7 @@ struct amdgpu_device {
>   
>   boolin_pci_err_recovery;
>   struct pci_saved_state  *pci_state;
> + struct completion dmub_aux_transfer_done;

Does this completion need to be on the amdgpu device itself?

I would prefer if we keep this as needed within DM itself if possible.

>   };
>   
>   static inline struct amdgpu_device *drm_to_adev(struct drm_device 
> *ddev) diff --git a/drivers/gpu/drm/amd/include/amd_shared.h 
> b/drivers/gpu/drm/amd/include/amd_shared.h
> index 43ed6291b2b8..097672cc78a1 100644
> --- a/drivers/gpu/drm/amd/include/amd_shared.h
> +++ b/drivers/gpu/drm/amd/include/amd_shared.h
> @@ -227,7 +227,8 @@ enum DC_DEBUG_MASK {
>   DC_DISABLE_PIPE_SPLIT = 0x1,
>   DC_DISABLE_STUTTER = 0x2,
>   DC_DISABLE_DSC = 0x4,
> - DC_DISABLE_CLOCK_GATING = 0x8
> + DC_DISABLE_CLOCK_GATING = 0x8,
> + DC_ENABLE_DMUB_AUX = 0x10,

My problem with still leaving this as DC_ENABLE_DMUB_AUX is we shouldn't 
require the user to have to flip this on by default later. I think I'd prefer 
this still as a DISABLE option if we want to leave it for users to debug any 
potential issues.

If there's no value in having end users debug issues by setting this bit then 
we should keep it as a dc->debug default in DCN resource.

Regards,
Nicholas Kazlauskas

>   };
>   
>   enum amd_dpm_forced_level;
> diff --git a/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h 
> b/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
> index e2bffcae273a..754170a86ea4 100644
> --- a/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
> +++ b/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
> @@ -1132,5 +1132,7 @@
>   
>   #define DCN_1_0__SRCID__DMCUB_OUTBOX_HIGH_PRIORITY_READY_INT   0x68
>   #define DCN_1_0__CTXID__DMCUB_OUTBOX_HIGH_PRIORITY_READY_INT   6
> +#define DCN_1_0__SRCID__DMCUB_OUTBOX_LOW_PRIORITY_READY_INT0x68 // 
> DMCUB_IHC_outbox1_ready_int IHC_DMCUB_outbox1_ready_int_ack 
> DMCUB_OUTBOX_LOW_PRIORITY_READY_INTERRUPT DISP_INTERRUPT_STATUS_CONTINUE24 
> Level/Pulse
> +#define DCN_1_0__CTXID__DMCUB_OUTBOX_LOW_PRIORITY_READY_INT8
>   
>   #endif // __IRQSRCS_DCN_1_0_H__
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 4/8] drm/radeon: Implement mmap as GEM object function

2021-04-06 Thread Alex Deucher
On Tue, Apr 6, 2021 at 5:09 AM Thomas Zimmermann  wrote:
>
> Moving the driver-specific mmap code into a GEM object function allows
> for using DRM helpers for various mmap callbacks.
>
> This change also allows to support prime-based mmap via DRM's helper
> drm_gem_prime_mmap().
>
> Permission checks are implemented by drm_gem_mmap(), with an additional
> check for radeon_ttm_tt_has_userptr() in the GEM object function. The
> function radeon_verify_access() is now unused and has thus been removed.
>
> As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
> implemented in amdgpu's GEM code.

s/amdgpu/radeon/

Alex

>
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/radeon/radeon_drv.c |  3 +-
>  drivers/gpu/drm/radeon/radeon_gem.c | 52 +++
>  drivers/gpu/drm/radeon/radeon_ttm.c | 65 -
>  drivers/gpu/drm/radeon/radeon_ttm.h |  1 -
>  4 files changed, 54 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index efeb115ae70e..4039b6d71aa2 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -557,7 +557,7 @@ static const struct file_operations 
> radeon_driver_kms_fops = {
> .open = drm_open,
> .release = drm_release,
> .unlocked_ioctl = radeon_drm_ioctl,
> -   .mmap = radeon_mmap,
> +   .mmap = drm_gem_mmap,
> .poll = drm_poll,
> .read = drm_read,
>  #ifdef CONFIG_COMPAT
> @@ -632,6 +632,7 @@ static const struct drm_driver kms_driver = {
> .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> .gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
> +   .gem_prime_mmap = drm_gem_prime_mmap,
>
> .name = DRIVER_NAME,
> .desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
> b/drivers/gpu/drm/radeon/radeon_gem.c
> index 05ea2f39f626..71e8737bce01 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -44,6 +44,42 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj);
>
>  const struct drm_gem_object_funcs radeon_gem_object_funcs;
>
> +static vm_fault_t radeon_ttm_fault(struct vm_fault *vmf)
> +{
> +   struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
> +   struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
> +   vm_fault_t ret;
> +
> +   down_read(>pm.mclk_lock);
> +
> +   ret = ttm_bo_vm_reserve(bo, vmf);
> +   if (ret)
> +   goto unlock_mclk;
> +
> +   ret = radeon_bo_fault_reserve_notify(bo);
> +   if (ret)
> +   goto unlock_resv;
> +
> +   ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
> +  TTM_BO_VM_NUM_PREFAULT, 1);
> +   if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
> +   goto unlock_mclk;
> +
> +unlock_resv:
> +   dma_resv_unlock(bo->base.resv);
> +
> +unlock_mclk:
> +   up_read(>pm.mclk_lock);
> +   return ret;
> +}
> +
> +static const struct vm_operations_struct radeon_ttm_vm_ops = {
> +   .fault = radeon_ttm_fault,
> +   .open = ttm_bo_vm_open,
> +   .close = ttm_bo_vm_close,
> +   .access = ttm_bo_vm_access
> +};
> +
>  static void radeon_gem_object_free(struct drm_gem_object *gobj)
>  {
> struct radeon_bo *robj = gem_to_radeon_bo(gobj);
> @@ -226,6 +262,20 @@ static int radeon_gem_handle_lockup(struct radeon_device 
> *rdev, int r)
> return r;
>  }
>
> +static int radeon_gem_object_mmap(struct drm_gem_object *obj, struct 
> vm_area_struct *vma)
> +{
> +   struct radeon_bo *bo = gem_to_radeon_bo(obj);
> +   struct radeon_device *rdev = radeon_get_rdev(bo->tbo.bdev);
> +
> +   if (!rdev)
> +   return -EINVAL;
> +
> +   if (radeon_ttm_tt_has_userptr(rdev, bo->tbo.ttm))
> +   return -EPERM;
> +
> +   return drm_gem_ttm_mmap(obj, vma);
> +}
> +
>  const struct drm_gem_object_funcs radeon_gem_object_funcs = {
> .free = radeon_gem_object_free,
> .open = radeon_gem_object_open,
> @@ -236,6 +286,8 @@ const struct drm_gem_object_funcs radeon_gem_object_funcs 
> = {
> .get_sg_table = radeon_gem_prime_get_sg_table,
> .vmap = drm_gem_ttm_vmap,
> .vunmap = drm_gem_ttm_vunmap,
> +   .mmap = radeon_gem_object_mmap,
> +   .vm_ops = _ttm_vm_ops,
>  };
>
>  /*
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
> b/drivers/gpu/drm/radeon/radeon_ttm.c
> index 476ce9c24b9f..a5ce43a909a2 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -136,17 +136,6 @@ static void radeon_evict_flags(struct ttm_buffer_object 
> *bo,
> *placement = rbo->placement;
>  }
>
> -static int radeon_verify_access(struct ttm_buffer_object *bo, struct file 
> *filp)
> -{
> -   struct 

Re: [PATCH] drm/amdgpu: add DMUB outbox event IRQ source define/complete/debug flag

2021-04-06 Thread Kazlauskas, Nicholas

On 2021-04-06 9:40 a.m., Jude Shih wrote:

[Why & How]
We use outbox interrupt that allows us to do the AUX via DMUB
Therefore, we need to add some irq source related definition
in the header files;
Also, I added debug flag that allows us to turn it on/off
for testing purpose.

Signed-off-by: Jude Shih 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 2 ++
  drivers/gpu/drm/amd/include/amd_shared.h  | 3 ++-
  drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h | 2 ++
  3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 963ecfd84347..7e64fc5e0dcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -923,6 +923,7 @@ struct amdgpu_device {
struct amdgpu_irq_src   pageflip_irq;
struct amdgpu_irq_src   hpd_irq;
struct amdgpu_irq_src   dmub_trace_irq;
+   struct amdgpu_irq_src   dmub_outbox_irq;
  
  	/* rings */

u64 fence_context;
@@ -1077,6 +1078,7 @@ struct amdgpu_device {
  
  	boolin_pci_err_recovery;

struct pci_saved_state  *pci_state;
+   struct completion dmub_aux_transfer_done;


Does this completion need to be on the amdgpu device itself?

I would prefer if we keep this as needed within DM itself if possible.


  };
  
  static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev)

diff --git a/drivers/gpu/drm/amd/include/amd_shared.h 
b/drivers/gpu/drm/amd/include/amd_shared.h
index 43ed6291b2b8..097672cc78a1 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -227,7 +227,8 @@ enum DC_DEBUG_MASK {
DC_DISABLE_PIPE_SPLIT = 0x1,
DC_DISABLE_STUTTER = 0x2,
DC_DISABLE_DSC = 0x4,
-   DC_DISABLE_CLOCK_GATING = 0x8
+   DC_DISABLE_CLOCK_GATING = 0x8,
+   DC_ENABLE_DMUB_AUX = 0x10,


My problem with still leaving this as DC_ENABLE_DMUB_AUX is we shouldn't 
require the user to have to flip this on by default later. I think I'd 
prefer this still as a DISABLE option if we want to leave it for users 
to debug any potential issues.


If there's no value in having end users debug issues by setting this bit 
then we should keep it as a dc->debug default in DCN resource.


Regards,
Nicholas Kazlauskas


  };
  
  enum amd_dpm_forced_level;

diff --git a/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h 
b/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
index e2bffcae273a..754170a86ea4 100644
--- a/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
+++ b/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
@@ -1132,5 +1132,7 @@
  
  #define DCN_1_0__SRCID__DMCUB_OUTBOX_HIGH_PRIORITY_READY_INT   0x68

  #define DCN_1_0__CTXID__DMCUB_OUTBOX_HIGH_PRIORITY_READY_INT   6
+#define DCN_1_0__SRCID__DMCUB_OUTBOX_LOW_PRIORITY_READY_INT0x68 // 
DMCUB_IHC_outbox1_ready_int IHC_DMCUB_outbox1_ready_int_ack 
DMCUB_OUTBOX_LOW_PRIORITY_READY_INTERRUPT DISP_INTERRUPT_STATUS_CONTINUE24 
Level/Pulse
+#define DCN_1_0__CTXID__DMCUB_OUTBOX_LOW_PRIORITY_READY_INT8
  
  #endif // __IRQSRCS_DCN_1_0_H__




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


Re: [PATCH] drm/amdgpu: Fix a potential sdma invalid access

2021-04-06 Thread Christian König

Hi Qu,

Am 06.04.21 um 08:04 schrieb Qu Huang:

Hi Christian,

On 2021/4/3 16:49, Christian König wrote:

Hi Qu,

Am 03.04.21 um 07:08 schrieb Qu Huang:

Hi Christian,

On 2021/4/3 0:25, Christian König wrote:

Hi Qu,

Am 02.04.21 um 05:18 schrieb Qu Huang:

Before dma_resv_lock(bo->base.resv, NULL) in
amdgpu_bo_release_notify(),
the bo->base.resv lock may be held by ttm_mem_evict_first(),


That can't happen since when bo_release_notify is called the BO has 
not

more references and is therefore deleted.

And we never evict a deleted BO, we just wait for it to become idle.


Yes, the bo reference counter return to zero will enter
ttm_bo_release(),but notify bo release (call 
amdgpu_bo_release_notify())

first happen, and then test if a reservation object's fences have been
signaled, and then mark bo as deleted and remove bo from the LRU list.

When ttm_bo_release() and ttm_mem_evict_first() is concurrent,
the Bo has not been removed from the LRU list and is not marked as
deleted, this will happen.


Not sure on which code base you are, but I don't see how this can 
happen.


ttm_mem_evict_first() calls ttm_bo_get_unless_zero() and
ttm_bo_release() is only called when the BO reference count becomes 
zero.


So ttm_mem_evict_first() will see that this BO is about to be destroyed
and skips it.



Yes, you are right. My version of TTM is ROCM 3.3, so
ttm_mem_evict_first() did not call ttm_bo_get_unless_zero(), check that
ROCM 4.0 ttm doesn't have this issue. This is an oversight on my part.



As a test, when we use CPU memset instead of SDMA fill in
amdgpu_bo_release_notify(), the result is page fault:

PID: 5490   TASK: 8e8136e04100  CPU: 4   COMMAND: "gemmPerf"
  #0 [8e79eaa17970] machine_kexec at b2863784
  #1 [8e79eaa179d0] __crash_kexec at b291ce92
  #2 [8e79eaa17aa0] crash_kexec at b291cf80
  #3 [8e79eaa17ab8] oops_end at b2f6c768
  #4 [8e79eaa17ae0] no_context at b2f5aaa6
  #5 [8e79eaa17b30] __bad_area_nosemaphore at b2f5ab3d
  #6 [8e79eaa17b80] bad_area_nosemaphore at b2f5acae
  #7 [8e79eaa17b90] __do_page_fault at b2f6f6c0
  #8 [8e79eaa17c00] do_page_fault at b2f6f925
  #9 [8e79eaa17c30] page_fault at b2f6b758
 [exception RIP: memset+31]
 RIP: b2b8668f  RSP: 8e79eaa17ce8  RFLAGS: 00010a17
 RAX: bebebebebebebebe  RBX: 8e747bff10c0  RCX: 
060b0020
 RDX:   RSI: 00be  RDI: 
ab807f00
 RBP: 8e79eaa17d10   R8: 8e79eaa14000   R9: 
ab7c8000
 R10: bcba  R11: 01ba  R12: 
8e79ebaa4050
 R13: ab7c8000  R14: 00022600  R15: 
8e8136e04100

 ORIG_RAX:   CS: 0010  SS: 0018
#10 [8e79eaa17ce8] amdgpu_bo_release_notify at c092f2d1
[amdgpu]
#11 [8e79eaa17d18] ttm_bo_release at c08f39dd [amdttm]
#12 [8e79eaa17d58] amdttm_bo_put at c08f3c8c [amdttm]
#13 [8e79eaa17d68] amdttm_bo_vm_close at c08f7ac9 [amdttm]
#14 [8e79eaa17d80] remove_vma at b29ef115
#15 [8e79eaa17da0] exit_mmap at b29f2c64
#16 [8e79eaa17e58] mmput at b28940c7
#17 [8e79eaa17e78] do_exit at b289dc95
#18 [8e79eaa17f10] do_group_exit at b289e4cf
#19 [8e79eaa17f40] sys_exit_group at b289e544
#20 [8e79eaa17f50] system_call_fastpath at b2f74ddb


Well that might be perfectly expected. VRAM is not necessarily CPU
accessible.


As a test,use CPU memset instead of SDMA fill, This is my code:
void amdgpu_bo_release_notify(struct ttm_buffer_object *bo)
{
struct amdgpu_bo *abo;
uint64_t num_pages;
struct drm_mm_node *mm_node;
struct amdgpu_device *adev;
void __iomem *kaddr;

if (!amdgpu_bo_is_amdgpu_bo(bo))
    return;

abo = ttm_to_amdgpu_bo(bo);
num_pages = abo->tbo.num_pages;
mm_node = abo->tbo.mem.mm_node;
adev = amdgpu_ttm_adev(abo->tbo.bdev);
kaddr = adev->mman.aper_base_kaddr;

if (abo->kfd_bo)
    amdgpu_amdkfd_unreserve_memory_limit(abo);

if (bo->mem.mem_type != TTM_PL_VRAM || !bo->mem.mm_node ||
    !(abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE))
    return;

dma_resv_lock(amdkcl_ttm_resvp(bo), NULL);
while (num_pages && mm_node) {
    void *ptr = kaddr + (mm_node->start << PAGE_SHIFT);


That might not work as expected.

aper_base_kaddr can only point to a 256MiB window into VRAM, but VRAM 
itself is usually much larger.


So your memset_io() might end up in nirvana if the BO is allocated 
outside of the window.



memset_io(ptr, AMDGPU_POISON & 0xff, mm_node->size 

[PATCH] drm/amdgpu: add DMUB outbox event IRQ source define/complete/debug flag

2021-04-06 Thread Jude Shih
[Why & How]
We use outbox interrupt that allows us to do the AUX via DMUB
Therefore, we need to add some irq source related definition
in the header files;
Also, I added debug flag that allows us to turn it on/off
for testing purpose.

Signed-off-by: Jude Shih 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 2 ++
 drivers/gpu/drm/amd/include/amd_shared.h  | 3 ++-
 drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 963ecfd84347..7e64fc5e0dcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -923,6 +923,7 @@ struct amdgpu_device {
struct amdgpu_irq_src   pageflip_irq;
struct amdgpu_irq_src   hpd_irq;
struct amdgpu_irq_src   dmub_trace_irq;
+   struct amdgpu_irq_src   dmub_outbox_irq;
 
/* rings */
u64 fence_context;
@@ -1077,6 +1078,7 @@ struct amdgpu_device {
 
boolin_pci_err_recovery;
struct pci_saved_state  *pci_state;
+   struct completion dmub_aux_transfer_done;
 };
 
 static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev)
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h 
b/drivers/gpu/drm/amd/include/amd_shared.h
index 43ed6291b2b8..097672cc78a1 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -227,7 +227,8 @@ enum DC_DEBUG_MASK {
DC_DISABLE_PIPE_SPLIT = 0x1,
DC_DISABLE_STUTTER = 0x2,
DC_DISABLE_DSC = 0x4,
-   DC_DISABLE_CLOCK_GATING = 0x8
+   DC_DISABLE_CLOCK_GATING = 0x8,
+   DC_ENABLE_DMUB_AUX = 0x10,
 };
 
 enum amd_dpm_forced_level;
diff --git a/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h 
b/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
index e2bffcae273a..754170a86ea4 100644
--- a/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
+++ b/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
@@ -1132,5 +1132,7 @@
 
 #define DCN_1_0__SRCID__DMCUB_OUTBOX_HIGH_PRIORITY_READY_INT   0x68
 #define DCN_1_0__CTXID__DMCUB_OUTBOX_HIGH_PRIORITY_READY_INT   6
+#define DCN_1_0__SRCID__DMCUB_OUTBOX_LOW_PRIORITY_READY_INT0x68 // 
DMCUB_IHC_outbox1_ready_int IHC_DMCUB_outbox1_ready_int_ack 
DMCUB_OUTBOX_LOW_PRIORITY_READY_INTERRUPT DISP_INTERRUPT_STATUS_CONTINUE24 
Level/Pulse
+#define DCN_1_0__CTXID__DMCUB_OUTBOX_LOW_PRIORITY_READY_INT8
 
 #endif // __IRQSRCS_DCN_1_0_H__
-- 
2.25.1

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


Re: [PATCH] drm/amdgpu: Fix a potential sdma invalid access

2021-04-06 Thread Qu Huang

Hi Christian,

On 2021/4/3 16:49, Christian König wrote:

Hi Qu,

Am 03.04.21 um 07:08 schrieb Qu Huang:

Hi Christian,

On 2021/4/3 0:25, Christian König wrote:

Hi Qu,

Am 02.04.21 um 05:18 schrieb Qu Huang:

Before dma_resv_lock(bo->base.resv, NULL) in
amdgpu_bo_release_notify(),
the bo->base.resv lock may be held by ttm_mem_evict_first(),


That can't happen since when bo_release_notify is called the BO has not
more references and is therefore deleted.

And we never evict a deleted BO, we just wait for it to become idle.


Yes, the bo reference counter return to zero will enter
ttm_bo_release(),but notify bo release (call amdgpu_bo_release_notify())
first happen, and then test if a reservation object's fences have been
signaled, and then mark bo as deleted and remove bo from the LRU list.

When ttm_bo_release() and ttm_mem_evict_first() is concurrent,
the Bo has not been removed from the LRU list and is not marked as
deleted, this will happen.


Not sure on which code base you are, but I don't see how this can happen.

ttm_mem_evict_first() calls ttm_bo_get_unless_zero() and
ttm_bo_release() is only called when the BO reference count becomes zero.

So ttm_mem_evict_first() will see that this BO is about to be destroyed
and skips it.



Yes, you are right. My version of TTM is ROCM 3.3, so
ttm_mem_evict_first() did not call ttm_bo_get_unless_zero(), check that
ROCM 4.0 ttm doesn't have this issue. This is an oversight on my part.



As a test, when we use CPU memset instead of SDMA fill in
amdgpu_bo_release_notify(), the result is page fault:

PID: 5490   TASK: 8e8136e04100  CPU: 4   COMMAND: "gemmPerf"
  #0 [8e79eaa17970] machine_kexec at b2863784
  #1 [8e79eaa179d0] __crash_kexec at b291ce92
  #2 [8e79eaa17aa0] crash_kexec at b291cf80
  #3 [8e79eaa17ab8] oops_end at b2f6c768
  #4 [8e79eaa17ae0] no_context at b2f5aaa6
  #5 [8e79eaa17b30] __bad_area_nosemaphore at b2f5ab3d
  #6 [8e79eaa17b80] bad_area_nosemaphore at b2f5acae
  #7 [8e79eaa17b90] __do_page_fault at b2f6f6c0
  #8 [8e79eaa17c00] do_page_fault at b2f6f925
  #9 [8e79eaa17c30] page_fault at b2f6b758
 [exception RIP: memset+31]
 RIP: b2b8668f  RSP: 8e79eaa17ce8  RFLAGS: 00010a17
 RAX: bebebebebebebebe  RBX: 8e747bff10c0  RCX: 060b0020
 RDX:   RSI: 00be  RDI: ab807f00
 RBP: 8e79eaa17d10   R8: 8e79eaa14000   R9: ab7c8000
 R10: bcba  R11: 01ba  R12: 8e79ebaa4050
 R13: ab7c8000  R14: 00022600  R15: 8e8136e04100
 ORIG_RAX:   CS: 0010  SS: 0018
#10 [8e79eaa17ce8] amdgpu_bo_release_notify at c092f2d1
[amdgpu]
#11 [8e79eaa17d18] ttm_bo_release at c08f39dd [amdttm]
#12 [8e79eaa17d58] amdttm_bo_put at c08f3c8c [amdttm]
#13 [8e79eaa17d68] amdttm_bo_vm_close at c08f7ac9 [amdttm]
#14 [8e79eaa17d80] remove_vma at b29ef115
#15 [8e79eaa17da0] exit_mmap at b29f2c64
#16 [8e79eaa17e58] mmput at b28940c7
#17 [8e79eaa17e78] do_exit at b289dc95
#18 [8e79eaa17f10] do_group_exit at b289e4cf
#19 [8e79eaa17f40] sys_exit_group at b289e544
#20 [8e79eaa17f50] system_call_fastpath at b2f74ddb


Well that might be perfectly expected. VRAM is not necessarily CPU
accessible.


As a test,use CPU memset instead of SDMA fill, This is my code:
void amdgpu_bo_release_notify(struct ttm_buffer_object *bo)
{
struct amdgpu_bo *abo;
uint64_t num_pages;
struct drm_mm_node *mm_node;
struct amdgpu_device *adev;
void __iomem *kaddr;

if (!amdgpu_bo_is_amdgpu_bo(bo))
return;

abo = ttm_to_amdgpu_bo(bo);
num_pages = abo->tbo.num_pages;
mm_node = abo->tbo.mem.mm_node;
adev = amdgpu_ttm_adev(abo->tbo.bdev);
kaddr = adev->mman.aper_base_kaddr;

if (abo->kfd_bo)
amdgpu_amdkfd_unreserve_memory_limit(abo);

if (bo->mem.mem_type != TTM_PL_VRAM || !bo->mem.mm_node ||
!(abo->flags & AMDGPU_GEM_CREATE_VRAM_WIPE_ON_RELEASE))
return;

dma_resv_lock(amdkcl_ttm_resvp(bo), NULL);
while (num_pages && mm_node) {
void *ptr = kaddr + (mm_node->start << PAGE_SHIFT);
memset_io(ptr, AMDGPU_POISON & 0xff, mm_node->size 
base.resv lock, and SDMA will get an invalid
address in amdgpu_fill_buffer(), resulting in a 

Re: [PATCH] drm/amdgpu: add DMUB outbox event IRQ source define/complete/debug flag

2021-04-06 Thread Kazlauskas, Nicholas

On 2021-03-31 11:21 p.m., Jude Shih wrote:

[Why & How]
We use outbox interrupt that allows us to do the AUX via DMUB
Therefore, we need to add some irq source related definition
in the header files;
Also, I added debug flag that allows us to turn it on/off
for testing purpose.


Missing your signed-off-by here, please recommit with

git commit --amend --sign


---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 2 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 2 +-
  drivers/gpu/drm/amd/include/amd_shared.h  | 3 ++-
  drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h | 2 ++
  4 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 963ecfd84347..479c8a28a3a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -923,6 +923,7 @@ struct amdgpu_device {
struct amdgpu_irq_src   pageflip_irq;
struct amdgpu_irq_src   hpd_irq;
struct amdgpu_irq_src   dmub_trace_irq;
+   struct amdgpu_irq_src   outbox_irq;
  
  	/* rings */

u64 fence_context;
@@ -1077,6 +1078,7 @@ struct amdgpu_device {
  
  	boolin_pci_err_recovery;

struct pci_saved_state  *pci_state;
+   struct completion dmub_aux_transfer_done;
  };
  
  static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 6a06234dbcad..0b88e13f5a7b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -159,7 +159,7 @@ int amdgpu_smu_pptable_id = -1;
   * PSR (bit 3) disabled by default
   */
  uint amdgpu_dc_feature_mask = 2;
-uint amdgpu_dc_debug_mask;
+uint amdgpu_dc_debug_mask = 0x10;


If this is intended to be enabled by default then it shouldn't be a 
debug flag. Please either leave the default alone or fully switch over 
to DMCUB AUX support for ASIC that support it.


If you don't already have a check from driver to DMCUB firmware to 
ensure that the firmware itself supports it you'd need that as well - 
users can be running older firmware (like the firmware that originally 
released with DCN2.1/DCN3.0 support) and that wouldn't support this feature.


My recommendation:
- Add a command to check for DMUB AUX capability or add bits to the 
metadata to indicate that the firmware does support it
- Assume that the DMUB AUX implementation is solid and a complete 
replacement for existing AUX support on firmware that does support it
- Add a debug flag like DC_DISABLE_DMUB_AUX for optionally debugging 
issues if they arise



  int amdgpu_async_gfx_ring = 1;
  int amdgpu_mcbp;
  int amdgpu_discovery = -1;
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h 
b/drivers/gpu/drm/amd/include/amd_shared.h
index 43ed6291b2b8..097672cc78a1 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -227,7 +227,8 @@ enum DC_DEBUG_MASK {
DC_DISABLE_PIPE_SPLIT = 0x1,
DC_DISABLE_STUTTER = 0x2,
DC_DISABLE_DSC = 0x4,
-   DC_DISABLE_CLOCK_GATING = 0x8
+   DC_DISABLE_CLOCK_GATING = 0x8,
+   DC_ENABLE_DMUB_AUX = 0x10,
  };
  
  enum amd_dpm_forced_level;

diff --git a/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h 
b/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
index e2bffcae273a..754170a86ea4 100644
--- a/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
+++ b/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
@@ -1132,5 +1132,7 @@
  
  #define DCN_1_0__SRCID__DMCUB_OUTBOX_HIGH_PRIORITY_READY_INT   0x68

  #define DCN_1_0__CTXID__DMCUB_OUTBOX_HIGH_PRIORITY_READY_INT   6
+#define DCN_1_0__SRCID__DMCUB_OUTBOX_LOW_PRIORITY_READY_INT0x68 // 
DMCUB_IHC_outbox1_ready_int IHC_DMCUB_outbox1_ready_int_ack 
DMCUB_OUTBOX_LOW_PRIORITY_READY_INTERRUPT DISP_INTERRUPT_STATUS_CONTINUE24 
Level/Pulse
+#define DCN_1_0__CTXID__DMCUB_OUTBOX_LOW_PRIORITY_READY_INT8


This technically isn't on DCN_1_0 but I guess we've been using this file 
for all the DCNs.


I do wish this was labeled DCN_2_1 instead to make it more explicit but 
I guess this is fine for now.


Regards,
Nicholas Kazlauskas

  
  #endif // __IRQSRCS_DCN_1_0_H__




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


Re: [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function

2021-04-06 Thread Christian König

Am 06.04.21 um 14:52 schrieb Thomas Zimmermann:

Hi

Am 06.04.21 um 14:42 schrieb Christian König:

Hi Thomas,

Am 06.04.21 um 13:55 schrieb Thomas Zimmermann:

Hi

Am 06.04.21 um 12:56 schrieb Christian König:


In the end I went with the semantics I found in amdgpu_mmap() and 
handled KFD specially. Let me know if this requires to be changed.


Well the question is where is the call to 
drm_vma_node_verify_access() now? Cause that needs to be skipped 
for KFD BOs.


I see. It's now drm_vma_node_is_allowed(); called by drm_gem_mmap(). 
[1] So drm_gem_mmap() cannot be used by amdgpu.


If I understand the code at [2] correctly, KFD objects don't use the 
GEM ioctl interfaces, but they still use the internal GEM object 
that is part of the TTM BO. In this case, amdgpu could have its own 
version of drm_gem_mmap(), which calls drm_gem_mmap_obj(), [3] which 
in turn handles the mmap details via GEM object functions.


Correct, well we could cleanup the KFD to use the GEM functions as well.


The KFD code already calls amdgpu_gem_object_create(). It should have 
the object-functions pointer set for use with mmap. Not sure what the 
use of drm_vma_node_is_allowed() would involve.


The KFD allows BOs to be mmaped with different offsets than what's used 
in the DRM node.


So drm_vma_node_is_allowed() would return false as far as I know.

Regards,
Christian.



Best regards
Thomas



Felix what exactly was your objections to using this?

Regards,
Christian.



drm_gem_prime_mmap() doesn't do any additional verification.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/drm_gem.c#L1156 

[2] 
https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c#L1224 

[3] 
https://elixir.bootlin.com/linux/v5.12-rc6/source/drivers/gpu/drm/drm_gem.c#L1053 






Regards,
Christian.



Best regards
Thomas


-
  int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t 
src_offset,

 uint64_t dst_offset, uint32_t byte_count,
 struct dma_resv *resv,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h

index dec0db8b0b13..6e51faad7371 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
  struct dma_resv *resv,
  struct dma_fence **fence);
-int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
  int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
  int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
  uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, 
uint32_t type);


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




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




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




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


Re: [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function

2021-04-06 Thread Thomas Zimmermann

Hi

Am 06.04.21 um 14:42 schrieb Christian König:

Hi Thomas,

Am 06.04.21 um 13:55 schrieb Thomas Zimmermann:

Hi

Am 06.04.21 um 12:56 schrieb Christian König:


In the end I went with the semantics I found in amdgpu_mmap() and 
handled KFD specially. Let me know if this requires to be changed.


Well the question is where is the call to 
drm_vma_node_verify_access() now? Cause that needs to be skipped for 
KFD BOs.


I see. It's now drm_vma_node_is_allowed(); called by drm_gem_mmap(). 
[1] So drm_gem_mmap() cannot be used by amdgpu.


If I understand the code at [2] correctly, KFD objects don't use the 
GEM ioctl interfaces, but they still use the internal GEM object that 
is part of the TTM BO. In this case, amdgpu could have its own version 
of drm_gem_mmap(), which calls drm_gem_mmap_obj(), [3] which in turn 
handles the mmap details via GEM object functions.


Correct, well we could cleanup the KFD to use the GEM functions as well.


The KFD code already calls amdgpu_gem_object_create(). It should have 
the object-functions pointer set for use with mmap. Not sure what the 
use of drm_vma_node_is_allowed() would involve.


Best regards
Thomas



Felix what exactly was your objections to using this?

Regards,
Christian.



drm_gem_prime_mmap() doesn't do any additional verification.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/drm_gem.c#L1156 

[2] 
https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c#L1224 

[3] 
https://elixir.bootlin.com/linux/v5.12-rc6/source/drivers/gpu/drm/drm_gem.c#L1053 






Regards,
Christian.



Best regards
Thomas


-
  int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t 
src_offset,

 uint64_t dst_offset, uint32_t byte_count,
 struct dma_resv *resv,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h

index dec0db8b0b13..6e51faad7371 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
  struct dma_resv *resv,
  struct dma_fence **fence);
-int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
  int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
  int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
  uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, 
uint32_t type);


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




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




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


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



OpenPGP_signature
Description: OpenPGP digital signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function

2021-04-06 Thread Christian König

Hi Thomas,

Am 06.04.21 um 13:55 schrieb Thomas Zimmermann:

Hi

Am 06.04.21 um 12:56 schrieb Christian König:


In the end I went with the semantics I found in amdgpu_mmap() and 
handled KFD specially. Let me know if this requires to be changed.


Well the question is where is the call to 
drm_vma_node_verify_access() now? Cause that needs to be skipped for 
KFD BOs.


I see. It's now drm_vma_node_is_allowed(); called by drm_gem_mmap(). 
[1] So drm_gem_mmap() cannot be used by amdgpu.


If I understand the code at [2] correctly, KFD objects don't use the 
GEM ioctl interfaces, but they still use the internal GEM object that 
is part of the TTM BO. In this case, amdgpu could have its own version 
of drm_gem_mmap(), which calls drm_gem_mmap_obj(), [3] which in turn 
handles the mmap details via GEM object functions.


Correct, well we could cleanup the KFD to use the GEM functions as well.

Felix what exactly was your objections to using this?

Regards,
Christian.



drm_gem_prime_mmap() doesn't do any additional verification.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/drm_gem.c#L1156
[2] 
https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c#L1224
[3] 
https://elixir.bootlin.com/linux/v5.12-rc6/source/drivers/gpu/drm/drm_gem.c#L1053





Regards,
Christian.



Best regards
Thomas


-
  int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t 
src_offset,

 uint64_t dst_offset, uint32_t byte_count,
 struct dma_resv *resv,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h

index dec0db8b0b13..6e51faad7371 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
  struct dma_resv *resv,
  struct dma_fence **fence);
-int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
  int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
  int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
  uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, 
uint32_t type);


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




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




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


[PATCH] drm/amd/amdgpu: Cancel the hrtimer in sw_fini

2021-04-06 Thread Roy Sun
Move the process of cancelling hrtimer to sw_fini

Signed-off-by: Roy Sun 
---
 drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c 
b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
index 5c11144da051..33324427b555 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_virtual.c
@@ -421,6 +421,11 @@ static int dce_virtual_sw_init(void *handle)
 static int dce_virtual_sw_fini(void *handle)
 {
struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+   int i = 0;
+
+   for (i = 0; i < adev->mode_info.num_crtc; i++)
+   if (adev->mode_info.crtcs[i])
+   hrtimer_cancel(>mode_info.crtcs[i]->vblank_timer);
 
kfree(adev->mode_info.bios_hardcoded_edid);
 
@@ -480,13 +485,6 @@ static int dce_virtual_hw_init(void *handle)
 
 static int dce_virtual_hw_fini(void *handle)
 {
-   struct amdgpu_device *adev = (struct amdgpu_device *)handle;
-   int i = 0;
-
-   for (i = 0; imode_info.num_crtc; i++)
-   if (adev->mode_info.crtcs[i])
-   hrtimer_cancel(>mode_info.crtcs[i]->vblank_timer);
-
return 0;
 }
 
-- 
2.29.0

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


Re: [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function

2021-04-06 Thread Thomas Zimmermann

Hi

Am 06.04.21 um 12:56 schrieb Christian König:


In the end I went with the semantics I found in amdgpu_mmap() and 
handled KFD specially. Let me know if this requires to be changed.


Well the question is where is the call to drm_vma_node_verify_access() 
now? Cause that needs to be skipped for KFD BOs.


I see. It's now drm_vma_node_is_allowed(); called by drm_gem_mmap(). [1] 
So drm_gem_mmap() cannot be used by amdgpu.


If I understand the code at [2] correctly, KFD objects don't use the GEM 
ioctl interfaces, but they still use the internal GEM object that is 
part of the TTM BO. In this case, amdgpu could have its own version of 
drm_gem_mmap(), which calls drm_gem_mmap_obj(), [3] which in turn 
handles the mmap details via GEM object functions.


drm_gem_prime_mmap() doesn't do any additional verification.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/drm_gem.c#L1156
[2] 
https://elixir.bootlin.com/linux/v5.11.11/source/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c#L1224
[3] 
https://elixir.bootlin.com/linux/v5.12-rc6/source/drivers/gpu/drm/drm_gem.c#L1053





Regards,
Christian.



Best regards
Thomas


-
  int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset,
 uint64_t dst_offset, uint32_t byte_count,
 struct dma_resv *resv,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h

index dec0db8b0b13..6e51faad7371 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
  struct dma_resv *resv,
  struct dma_fence **fence);
-int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
  int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
  int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
  uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, 
uint32_t type);


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




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


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



OpenPGP_signature
Description: OpenPGP digital signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


Re: [PATCH 0/4] Refine GPU recovery sequence to enhance its stability

2021-04-06 Thread Christian König



Am 06.04.21 um 12:34 schrieb Christian König:

Hi Andrey,

well good question. My job is to watch over the implementation and 
design and while I always help I can adjust anybodies schedule.


That should read "I can't adjust anybodies schedule".

Christian.



Is the patch to print a warning when the hardware is accessed without 
holding the locks merged yet? If not then that would probably be a 
good starting point.


Then we would need to unify this with the SRCU to make sure that we 
have both the reset lock as well as block the hotplug code from 
reusing the MMIO space.


And then testing, testing, testing to see if we have missed something.

Christian.

Am 05.04.21 um 19:58 schrieb Andrey Grodzovsky:


Denis, Christian, are there any updates in the plan on how to move on 
with this ? As you know I need very similar code for my up-streaming 
of device hot-unplug. My latest solution 
(https://lists.freedesktop.org/archives/amd-gfx/2021-January/058606.html) 
was not acceptable because of low level guards on the register 
accessors level which was hurting performance. Basically I need a way 
to prevent any MMIO write accesses from kernel driver after device is 
removed (UMD accesses are taken care of by page faulting dummy page). 
We are using now hot-unplug code for Freemont program and so 
up-streaming became more of a priority then before. This MMIO access 
issue is currently my main blocker from up-streaming. Is there any 
way I can assist in pushing this on ?


Andrey

On 2021-03-18 5:51 a.m., Christian König wrote:

Am 18.03.21 um 10:30 schrieb Li, Dennis:


>>> The GPU reset doesn't complete the fences we wait for. It only 
completes the hardware fences as part of the reset.


>>> So waiting for a fence while holding the reset lock is illegal 
and needs to be avoided.


I understood your concern. It is more complex for DRM GFX, 
therefore I abandon adding lock protection for DRM ioctls now. 
Maybe we can try to add all kernel  dma_fence waiting in a list, 
and signal all in recovery threads. Do you have same concern for 
compute cases?




Yes, compute (KFD) is even harder to handle.

See you can't signal the dma_fence waiting. Waiting for a dma_fence 
also means you wait for the GPU reset to finish.


When we would signal the dma_fence during the GPU reset then we 
would run into memory corruption because the hardware jobs running 
after the GPU reset would access memory which is already freed.


>>> Lockdep also complains about this when it is used correctly. 
The only reason it doesn't complain here is because you use an 
atomic+wait_event instead of a locking primitive.


Agree. This approach will escape the monitor of lockdep.  Its goal 
is to block other threads when GPU recovery thread start. But I 
couldn’t find a better method to solve this problem. Do you have 
some suggestion?




Well, completely abandon those change here.

What we need to do is to identify where hardware access happens and 
then insert taking the read side of the GPU reset lock so that we 
don't wait for a dma_fence or allocate memory, but still protect the 
hardware from concurrent access and reset.


Regards,
Christian.


Best Regards

Dennis Li

*From:* Koenig, Christian 
*Sent:* Thursday, March 18, 2021 4:59 PM
*To:* Li, Dennis ; 
amd-gfx@lists.freedesktop.org; Deucher, Alexander 
; Kuehling, Felix 
; Zhang, Hawking 
*Subject:* AW: [PATCH 0/4] Refine GPU recovery sequence to enhance 
its stability


Exactly that's what you don't seem to understand.

The GPU reset doesn't complete the fences we wait for. It only 
completes the hardware fences as part of the reset.


So waiting for a fence while holding the reset lock is illegal and 
needs to be avoided.


Lockdep also complains about this when it is used correctly. The 
only reason it doesn't complain here is because you use an 
atomic+wait_event instead of a locking primitive.


Regards,

Christian.



*Von:*Li, Dennis mailto:dennis...@amd.com>>
*Gesendet:* Donnerstag, 18. März 2021 09:28
*An:* Koenig, Christian >; amd-gfx@lists.freedesktop.org 
 
>; Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>; 
Kuehling, Felix >; Zhang, Hawking 
mailto:hawking.zh...@amd.com>>
*Betreff:* RE: [PATCH 0/4] Refine GPU recovery sequence to enhance 
its stability


>>> Those two steps need to be exchanged or otherwise it is 
possible that new delayed work items etc are started before the 
lock is taken.
What about adding check for adev->in_gpu_reset in work item? If 
exchange the two steps, it maybe introduce the deadlock.  For 
example, the user thread hold the read lock and waiting for the 
fence, if recovery thread try to hold write lock and then complete 
fences, in this case, recovery thread will always be blocked.



Best Regards
Dennis Li
-Original 

Re: [PATCH] drm/amdgpu: Add show_fdinfo() interface

2021-04-06 Thread Christian König



Am 06.04.21 um 12:54 schrieb Nirmoy:


On 4/6/21 11:49 AM, Roy Sun wrote:

Tracking devices, process info and fence info using
/proc/pid/fdinfo


First of all please separate the patch into the handling for the DRM 
file descriptors and KFD file descriptors.




Signed-off-by: David M Nieto 
Signed-off-by: Roy Sun 
---
  drivers/gpu/drm/amd/amdgpu/Makefile   |   2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu.h   |   3 +
  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  15 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |   5 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c    | 282 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h    |  58 
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |   3 +
  drivers/gpu/drm/scheduler/sched_main.c    |  11 +-
  8 files changed, 371 insertions(+), 8 deletions(-)
  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile

index ee85e8aba636..f9de1acc65dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -55,7 +55,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
  amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o amdgpu_ids.o \
  amdgpu_gmc.o amdgpu_mmhub.o amdgpu_xgmi.o amdgpu_csa.o 
amdgpu_ras.o amdgpu_vm_cpu.o \
  amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o 
amdgpu_nbio.o \

-    amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
+    amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o 
amdgpu_fdinfo.o\

  amdgpu_fw_attestation.o amdgpu_securedisplay.o



Use amdgpu-$(CONFIG_PROC_FS) instead so that you can ignore some 
CONFIG_PROC_FS checks.




  amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h

index 616c85a01299..35843c8d133d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -107,6 +107,7 @@
  #include "amdgpu_gfxhub.h"
  #include "amdgpu_df.h"
  #include "amdgpu_smuio.h"
+#include "amdgpu_fdinfo.h"
    #define MAX_GPU_INSTANCE    16
  @@ -477,6 +478,8 @@ struct amdgpu_fpriv {
  struct mutex    bo_list_lock;
  struct idr    bo_list_handles;
  struct amdgpu_ctx_mgr    ctx_mgr;
+    struct drm_file    *file;
+    struct amdgpu_proc    *proc;



You should be able to avoid adding these extra members. See below:


I agree we already have the necessary information in the data structures 
here.






  };


  int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv 
**fpriv);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c

index e93850f2f3b1..702fd9054883 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1042,13 +1042,15 @@ int 
amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, u32 pasid,

    struct dma_fence **ef)
  {
  struct amdgpu_device *adev = get_amdgpu_device(kgd);
+    struct amdgpu_fpriv *fpriv;
  struct amdgpu_vm *new_vm;
  int ret;
  -    new_vm = kzalloc(sizeof(*new_vm), GFP_KERNEL);
-    if (!new_vm)
+    fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
+    if (!fpriv)
  return -ENOMEM;
  +    new_vm = >vm;
  /* Initialize AMDGPU part of the VM */
  ret = amdgpu_vm_init(adev, new_vm, AMDGPU_VM_CONTEXT_COMPUTE, 
pasid);

  if (ret) {
@@ -1063,12 +1065,14 @@ int 
amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, u32 pasid,

    *vm = (void *) new_vm;
  +    amdgpu_fdinfo_init(adev, fpriv, pasid);
+
  return 0;
    init_kfd_vm_fail:
  amdgpu_vm_fini(adev, new_vm);
  amdgpu_vm_init_fail:
-    kfree(new_vm);
+    kfree(fpriv);
  return ret;
  }
  @@ -1142,6 +1146,8 @@ void 
amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm)

  {
  struct amdgpu_device *adev = get_amdgpu_device(kgd);
  struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
+    struct amdgpu_fpriv *fpriv =
+    container_of(avm, struct amdgpu_fpriv, vm);
    if (WARN_ON(!kgd || !vm))
  return;
@@ -1149,8 +1155,9 @@ void 
amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm)

  pr_debug("Destroying process vm %p\n", vm);
    /* Release the VM context */
+    amdgpu_fdinfo_fini(adev, fpriv);
  amdgpu_vm_fini(adev, avm);
-    kfree(vm);
+    kfree(fpriv);


Felix needs to take a look here, but that is most likely a no-go. On the 
other hand if you drop the amdgpu_proc structure that should become 
unnecessary.



  }
    void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, 
void *vm)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c

index 4bcc03c4c6c5..07aed377dec8 100644
--- 

Re: [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function

2021-04-06 Thread Christian König

Hi Thomas,

Am 06.04.21 um 12:38 schrieb Thomas Zimmermann:

Hi

Am 06.04.21 um 11:35 schrieb Christian König:

Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:

Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

This change resolves several inconsistencies between regular mmap and
prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
areas. Previously it way only set for regular mmap calls, prime-based
mmap used TTM's default vm_ops. The check for kfd_bo has been taken
from amdgpu_verify_access(), which is not called any longer and has
been removed.

As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.

Signed-off-by: Thomas Zimmermann 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h |  2 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  4 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 64 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 71 
-

  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  1 -
  6 files changed, 66 insertions(+), 122 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c

index e0c4f7c7f1b9..19c5ab08d9ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -42,52 +42,6 @@
  #include 
  #include 
-/**
- * amdgpu_gem_prime_mmap - _driver.gem_prime_mmap implementation
- * @obj: GEM BO
- * @vma: Virtual memory area
- *
- * Sets up a userspace mapping of the BO's memory in the given
- * virtual memory area.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
- */
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
-  struct vm_area_struct *vma)
-{
-    struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
-    struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
-    unsigned asize = amdgpu_bo_size(bo);
-    int ret;
-
-    if (!vma->vm_file)
-    return -ENODEV;
-
-    if (adev == NULL)
-    return -ENODEV;
-
-    /* Check for valid size. */
-    if (asize < vma->vm_end - vma->vm_start)
-    return -EINVAL;
-
-    if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
-    (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
-    return -EPERM;
-    }
-    vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
-
-    /* prime mmap does not need to check access, so allow here */
-    ret = drm_vma_node_allow(>vma_node, 
vma->vm_file->private_data);

-    if (ret)
-    return ret;
-
-    ret = ttm_bo_mmap(vma->vm_file, vma, >mman.bdev);
-    drm_vma_node_revoke(>vma_node, vma->vm_file->private_data);
-
-    return ret;
-}
-
  static int
  __dma_resv_make_exclusive(struct dma_resv *obj)
  {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h

index 39b5b9616fd8..3e93b9b407a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -31,8 +31,6 @@ struct drm_gem_object 
*amdgpu_gem_prime_import(struct drm_device *dev,

  struct dma_buf *dma_buf);
  bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
    struct amdgpu_bo *bo);
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
-  struct vm_area_struct *vma);
  extern const struct dma_buf_ops amdgpu_dmabuf_ops;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c

index 76f48f79c70b..e96d2758f4bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1656,7 +1656,7 @@ static const struct file_operations 
amdgpu_driver_kms_fops = {

  .flush = amdgpu_flush,
  .release = drm_release,
  .unlocked_ioctl = amdgpu_drm_ioctl,
-    .mmap = amdgpu_mmap,
+    .mmap = drm_gem_mmap,
  .poll = drm_poll,
  .read = drm_read,
  #ifdef CONFIG_COMPAT
@@ -1719,7 +1719,7 @@ static const struct drm_driver 
amdgpu_kms_driver = {

  .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
  .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
  .gem_prime_import = amdgpu_gem_prime_import,
-    .gem_prime_mmap = amdgpu_gem_prime_mmap,
+    .gem_prime_mmap = drm_gem_prime_mmap,
  .name = DRIVER_NAME,
  .desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c

index fb7171e5507c..fe93faad05f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -41,6 +41,36 @@
  static const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
+static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)


Please name that function amdgpu_gem_fault or amdgpu_gem_object_fault


+{
+    struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+    vm_fault_t ret;
+
+    ret = 

Re: [PATCH] drm/amdgpu: Add show_fdinfo() interface

2021-04-06 Thread Nirmoy



On 4/6/21 11:49 AM, Roy Sun wrote:

Tracking devices, process info and fence info using
/proc/pid/fdinfo

Signed-off-by: David M Nieto 
Signed-off-by: Roy Sun 
---
  drivers/gpu/drm/amd/amdgpu/Makefile   |   2 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu.h   |   3 +
  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  15 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |   5 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c| 282 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h|  58 
  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |   3 +
  drivers/gpu/drm/scheduler/sched_main.c|  11 +-
  8 files changed, 371 insertions(+), 8 deletions(-)
  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index ee85e8aba636..f9de1acc65dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -55,7 +55,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o amdgpu_ids.o \
amdgpu_gmc.o amdgpu_mmhub.o amdgpu_xgmi.o amdgpu_csa.o amdgpu_ras.o 
amdgpu_vm_cpu.o \
amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \
-   amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
+   amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o 
amdgpu_fdinfo.o\
amdgpu_fw_attestation.o amdgpu_securedisplay.o
  



Use amdgpu-$(CONFIG_PROC_FS) instead so that you can ignore some 
CONFIG_PROC_FS checks.




  amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 616c85a01299..35843c8d133d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -107,6 +107,7 @@
  #include "amdgpu_gfxhub.h"
  #include "amdgpu_df.h"
  #include "amdgpu_smuio.h"
+#include "amdgpu_fdinfo.h"
  
  #define MAX_GPU_INSTANCE		16
  
@@ -477,6 +478,8 @@ struct amdgpu_fpriv {

struct mutexbo_list_lock;
struct idr  bo_list_handles;
struct amdgpu_ctx_mgr   ctx_mgr;
+   struct drm_file *file;
+   struct amdgpu_proc  *proc;



You should be able to avoid adding these extra members. See below:



  };
  



  int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e93850f2f3b1..702fd9054883 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1042,13 +1042,15 @@ int amdgpu_amdkfd_gpuvm_create_process_vm(struct 
kgd_dev *kgd, u32 pasid,
  struct dma_fence **ef)
  {
struct amdgpu_device *adev = get_amdgpu_device(kgd);
+   struct amdgpu_fpriv *fpriv;
struct amdgpu_vm *new_vm;
int ret;
  
-	new_vm = kzalloc(sizeof(*new_vm), GFP_KERNEL);

-   if (!new_vm)
+   fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
+   if (!fpriv)
return -ENOMEM;
  
+	new_vm = >vm;

/* Initialize AMDGPU part of the VM */
ret = amdgpu_vm_init(adev, new_vm, AMDGPU_VM_CONTEXT_COMPUTE, pasid);
if (ret) {
@@ -1063,12 +1065,14 @@ int amdgpu_amdkfd_gpuvm_create_process_vm(struct 
kgd_dev *kgd, u32 pasid,
  
  	*vm = (void *) new_vm;
  
+	amdgpu_fdinfo_init(adev, fpriv, pasid);

+
return 0;
  
  init_kfd_vm_fail:

amdgpu_vm_fini(adev, new_vm);
  amdgpu_vm_init_fail:
-   kfree(new_vm);
+   kfree(fpriv);
return ret;
  }
  
@@ -1142,6 +1146,8 @@ void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm)

  {
struct amdgpu_device *adev = get_amdgpu_device(kgd);
struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
+   struct amdgpu_fpriv *fpriv =
+   container_of(avm, struct amdgpu_fpriv, vm);
  
  	if (WARN_ON(!kgd || !vm))

return;
@@ -1149,8 +1155,9 @@ void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct 
kgd_dev *kgd, void *vm)
pr_debug("Destroying process vm %p\n", vm);
  
  	/* Release the VM context */

+   amdgpu_fdinfo_fini(adev, fpriv);
amdgpu_vm_fini(adev, avm);
-   kfree(vm);
+   kfree(fpriv);
  }
  
  void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 4bcc03c4c6c5..07aed377dec8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -42,7 +42,7 @@
  #include "amdgpu_irq.h"
  #include "amdgpu_dma_buf.h"
  #include "amdgpu_sched.h"
-
+#include "amdgpu_fdinfo.h"
  #include "amdgpu_amdkfd.h"
  
  #include "amdgpu_ras.h"

@@ -1691,6 +1691,9 @@ static const struct 

Re: [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function

2021-04-06 Thread Thomas Zimmermann

Hi

Am 06.04.21 um 11:35 schrieb Christian König:

Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:

Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

This change resolves several inconsistencies between regular mmap and
prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
areas. Previously it way only set for regular mmap calls, prime-based
mmap used TTM's default vm_ops. The check for kfd_bo has been taken
from amdgpu_verify_access(), which is not called any longer and has
been removed.

As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.

Signed-off-by: Thomas Zimmermann 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h |  2 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  4 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 64 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 71 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  1 -
  6 files changed, 66 insertions(+), 122 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c

index e0c4f7c7f1b9..19c5ab08d9ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -42,52 +42,6 @@
  #include 
  #include 
-/**
- * amdgpu_gem_prime_mmap - _driver.gem_prime_mmap implementation
- * @obj: GEM BO
- * @vma: Virtual memory area
- *
- * Sets up a userspace mapping of the BO's memory in the given
- * virtual memory area.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
- */
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
-  struct vm_area_struct *vma)
-{
-    struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
-    struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
-    unsigned asize = amdgpu_bo_size(bo);
-    int ret;
-
-    if (!vma->vm_file)
-    return -ENODEV;
-
-    if (adev == NULL)
-    return -ENODEV;
-
-    /* Check for valid size. */
-    if (asize < vma->vm_end - vma->vm_start)
-    return -EINVAL;
-
-    if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
-    (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
-    return -EPERM;
-    }
-    vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
-
-    /* prime mmap does not need to check access, so allow here */
-    ret = drm_vma_node_allow(>vma_node, 
vma->vm_file->private_data);

-    if (ret)
-    return ret;
-
-    ret = ttm_bo_mmap(vma->vm_file, vma, >mman.bdev);
-    drm_vma_node_revoke(>vma_node, vma->vm_file->private_data);
-
-    return ret;
-}
-
  static int
  __dma_resv_make_exclusive(struct dma_resv *obj)
  {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h

index 39b5b9616fd8..3e93b9b407a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -31,8 +31,6 @@ struct drm_gem_object 
*amdgpu_gem_prime_import(struct drm_device *dev,

  struct dma_buf *dma_buf);
  bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
    struct amdgpu_bo *bo);
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
-  struct vm_area_struct *vma);
  extern const struct dma_buf_ops amdgpu_dmabuf_ops;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c

index 76f48f79c70b..e96d2758f4bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1656,7 +1656,7 @@ static const struct file_operations 
amdgpu_driver_kms_fops = {

  .flush = amdgpu_flush,
  .release = drm_release,
  .unlocked_ioctl = amdgpu_drm_ioctl,
-    .mmap = amdgpu_mmap,
+    .mmap = drm_gem_mmap,
  .poll = drm_poll,
  .read = drm_read,
  #ifdef CONFIG_COMPAT
@@ -1719,7 +1719,7 @@ static const struct drm_driver amdgpu_kms_driver 
= {

  .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
  .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
  .gem_prime_import = amdgpu_gem_prime_import,
-    .gem_prime_mmap = amdgpu_gem_prime_mmap,
+    .gem_prime_mmap = drm_gem_prime_mmap,
  .name = DRIVER_NAME,
  .desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c

index fb7171e5507c..fe93faad05f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -41,6 +41,36 @@
  static const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
+static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)


Please name that function amdgpu_gem_fault or amdgpu_gem_object_fault


+{
+    struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+    vm_fault_t ret;
+
+    ret = ttm_bo_vm_reserve(bo, vmf);
+    if (ret)
+    return ret;
+
+    ret = 

Re: [PATCH 0/4] Refine GPU recovery sequence to enhance its stability

2021-04-06 Thread Christian König

Hi Andrey,

well good question. My job is to watch over the implementation and 
design and while I always help I can adjust anybodies schedule.


Is the patch to print a warning when the hardware is accessed without 
holding the locks merged yet? If not then that would probably be a good 
starting point.


Then we would need to unify this with the SRCU to make sure that we have 
both the reset lock as well as block the hotplug code from reusing the 
MMIO space.


And then testing, testing, testing to see if we have missed something.

Christian.

Am 05.04.21 um 19:58 schrieb Andrey Grodzovsky:


Denis, Christian, are there any updates in the plan on how to move on 
with this ? As you know I need very similar code for my up-streaming 
of device hot-unplug. My latest solution 
(https://lists.freedesktop.org/archives/amd-gfx/2021-January/058606.html) 
was not acceptable because of low level guards on the register 
accessors level which was hurting performance. Basically I need a way 
to prevent any MMIO write accesses from kernel driver after device is 
removed (UMD accesses are taken care of by page faulting dummy page). 
We are using now hot-unplug code for Freemont program and so 
up-streaming became more of a priority then before. This MMIO access 
issue is currently my main blocker from up-streaming. Is there any way 
I can assist in pushing this on ?


Andrey

On 2021-03-18 5:51 a.m., Christian König wrote:

Am 18.03.21 um 10:30 schrieb Li, Dennis:


>>> The GPU reset doesn't complete the fences we wait for. It only 
completes the hardware fences as part of the reset.


>>> So waiting for a fence while holding the reset lock is illegal 
and needs to be avoided.


I understood your concern. It is more complex for DRM GFX, therefore 
I abandon adding lock protection for DRM ioctls now. Maybe we can 
try to add all kernel  dma_fence waiting in a list, and signal all 
in recovery threads. Do you have same concern for compute cases?




Yes, compute (KFD) is even harder to handle.

See you can't signal the dma_fence waiting. Waiting for a dma_fence 
also means you wait for the GPU reset to finish.


When we would signal the dma_fence during the GPU reset then we would 
run into memory corruption because the hardware jobs running after 
the GPU reset would access memory which is already freed.


>>> Lockdep also complains about this when it is used correctly. The 
only reason it doesn't complain here is because you use an 
atomic+wait_event instead of a locking primitive.


Agree. This approach will escape the monitor of lockdep.  Its goal 
is to block other threads when GPU recovery thread start. But I 
couldn’t find a better method to solve this problem. Do you have 
some suggestion?




Well, completely abandon those change here.

What we need to do is to identify where hardware access happens and 
then insert taking the read side of the GPU reset lock so that we 
don't wait for a dma_fence or allocate memory, but still protect the 
hardware from concurrent access and reset.


Regards,
Christian.


Best Regards

Dennis Li

*From:* Koenig, Christian 
*Sent:* Thursday, March 18, 2021 4:59 PM
*To:* Li, Dennis ; amd-gfx@lists.freedesktop.org; 
Deucher, Alexander ; Kuehling, Felix 
; Zhang, Hawking 
*Subject:* AW: [PATCH 0/4] Refine GPU recovery sequence to enhance 
its stability


Exactly that's what you don't seem to understand.

The GPU reset doesn't complete the fences we wait for. It only 
completes the hardware fences as part of the reset.


So waiting for a fence while holding the reset lock is illegal and 
needs to be avoided.


Lockdep also complains about this when it is used correctly. The 
only reason it doesn't complain here is because you use an 
atomic+wait_event instead of a locking primitive.


Regards,

Christian.



*Von:*Li, Dennis mailto:dennis...@amd.com>>
*Gesendet:* Donnerstag, 18. März 2021 09:28
*An:* Koenig, Christian >; amd-gfx@lists.freedesktop.org 
 
>; Deucher, Alexander 
mailto:alexander.deuc...@amd.com>>; 
Kuehling, Felix >; Zhang, Hawking 
mailto:hawking.zh...@amd.com>>
*Betreff:* RE: [PATCH 0/4] Refine GPU recovery sequence to enhance 
its stability


>>> Those two steps need to be exchanged or otherwise it is possible 
that new delayed work items etc are started before the lock is taken.
What about adding check for adev->in_gpu_reset in work item? If 
exchange the two steps, it maybe introduce the deadlock.  For 
example, the user thread hold the read lock and waiting for the 
fence, if recovery thread try to hold write lock and then complete 
fences, in this case, recovery thread will always be blocked.



Best Regards
Dennis Li
-Original Message-
From: Koenig, Christian >

Sent: Thursday, March 18, 2021 3:54 PM
To: Li, 

Re: [PATCH 2/8] drm/amdgpu: Remove unused function amdgpu_bo_fbdev_mmap()

2021-04-06 Thread Thomas Zimmermann

Hi

Am 06.04.21 um 11:43 schrieb Christian König:

Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:

Remove an unused function. Mapping the fbdev framebuffer is apparently
not supported.

Signed-off-by: Thomas Zimmermann 


Reviewed-by: Christian König 

Should I just upstream this through our internal branches?


I guess you can pick up this patch into your branches if that's easier 
for you. For the other patches, I'd like to merge them via drm-misc-next.


Best regards
Thomas



Thanks,
Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 19 ---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 --
  2 files changed, 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c

index b99e9d8736c2..cfc89164dee8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1092,25 +1092,6 @@ void amdgpu_bo_fini(struct amdgpu_device *adev)
  }
  }
-/**
- * amdgpu_bo_fbdev_mmap - mmap fbdev memory
- * @bo: _bo buffer object
- * @vma: vma as input from the fbdev mmap method
- *
- * Calls ttm_fbdev_mmap() to mmap fbdev memory if it is backed by a bo.
- *
- * Returns:
- * 0 for success or a negative error code on failure.
- */
-int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
- struct vm_area_struct *vma)
-{
-    if (vma->vm_pgoff != 0)
-    return -EACCES;
-
-    return ttm_bo_mmap_obj(vma, >tbo);
-}
-
  /**
   * amdgpu_bo_set_tiling_flags - set tiling flags
   * @bo: _bo buffer object
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h

index 54ceb065e546..46e94d413c5c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -268,8 +268,6 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo);
  int amdgpu_bo_evict_vram(struct amdgpu_device *adev);
  int amdgpu_bo_init(struct amdgpu_device *adev);
  void amdgpu_bo_fini(struct amdgpu_device *adev);
-int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
-    struct vm_area_struct *vma);
  int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags);
  void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, u64 
*tiling_flags);

  int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,




--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



OpenPGP_signature
Description: OpenPGP digital signature
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH] drm/amdgpu: Add show_fdinfo() interface

2021-04-06 Thread Roy Sun
Tracking devices, process info and fence info using
/proc/pid/fdinfo

Signed-off-by: David M Nieto 
Signed-off-by: Roy Sun 
---
 drivers/gpu/drm/amd/amdgpu/Makefile   |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   |   3 +
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  15 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |   5 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c| 282 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h|  58 
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |   3 +
 drivers/gpu/drm/scheduler/sched_main.c|  11 +-
 8 files changed, 371 insertions(+), 8 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
b/drivers/gpu/drm/amd/amdgpu/Makefile
index ee85e8aba636..f9de1acc65dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -55,7 +55,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o amdgpu_ids.o \
amdgpu_gmc.o amdgpu_mmhub.o amdgpu_xgmi.o amdgpu_csa.o amdgpu_ras.o 
amdgpu_vm_cpu.o \
amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \
-   amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
+   amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o 
amdgpu_fdinfo.o\
amdgpu_fw_attestation.o amdgpu_securedisplay.o
 
 amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 616c85a01299..35843c8d133d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -107,6 +107,7 @@
 #include "amdgpu_gfxhub.h"
 #include "amdgpu_df.h"
 #include "amdgpu_smuio.h"
+#include "amdgpu_fdinfo.h"
 
 #define MAX_GPU_INSTANCE   16
 
@@ -477,6 +478,8 @@ struct amdgpu_fpriv {
struct mutexbo_list_lock;
struct idr  bo_list_handles;
struct amdgpu_ctx_mgr   ctx_mgr;
+   struct drm_file *file;
+   struct amdgpu_proc  *proc;
 };
 
 int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index e93850f2f3b1..702fd9054883 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1042,13 +1042,15 @@ int amdgpu_amdkfd_gpuvm_create_process_vm(struct 
kgd_dev *kgd, u32 pasid,
  struct dma_fence **ef)
 {
struct amdgpu_device *adev = get_amdgpu_device(kgd);
+   struct amdgpu_fpriv *fpriv;
struct amdgpu_vm *new_vm;
int ret;
 
-   new_vm = kzalloc(sizeof(*new_vm), GFP_KERNEL);
-   if (!new_vm)
+   fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
+   if (!fpriv)
return -ENOMEM;
 
+   new_vm = >vm;
/* Initialize AMDGPU part of the VM */
ret = amdgpu_vm_init(adev, new_vm, AMDGPU_VM_CONTEXT_COMPUTE, pasid);
if (ret) {
@@ -1063,12 +1065,14 @@ int amdgpu_amdkfd_gpuvm_create_process_vm(struct 
kgd_dev *kgd, u32 pasid,
 
*vm = (void *) new_vm;
 
+   amdgpu_fdinfo_init(adev, fpriv, pasid);
+
return 0;
 
 init_kfd_vm_fail:
amdgpu_vm_fini(adev, new_vm);
 amdgpu_vm_init_fail:
-   kfree(new_vm);
+   kfree(fpriv);
return ret;
 }
 
@@ -1142,6 +1146,8 @@ void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct 
kgd_dev *kgd, void *vm)
 {
struct amdgpu_device *adev = get_amdgpu_device(kgd);
struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
+   struct amdgpu_fpriv *fpriv =
+   container_of(avm, struct amdgpu_fpriv, vm);
 
if (WARN_ON(!kgd || !vm))
return;
@@ -1149,8 +1155,9 @@ void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct 
kgd_dev *kgd, void *vm)
pr_debug("Destroying process vm %p\n", vm);
 
/* Release the VM context */
+   amdgpu_fdinfo_fini(adev, fpriv);
amdgpu_vm_fini(adev, avm);
-   kfree(vm);
+   kfree(fpriv);
 }
 
 void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 4bcc03c4c6c5..07aed377dec8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -42,7 +42,7 @@
 #include "amdgpu_irq.h"
 #include "amdgpu_dma_buf.h"
 #include "amdgpu_sched.h"
-
+#include "amdgpu_fdinfo.h"
 #include "amdgpu_amdkfd.h"
 
 #include "amdgpu_ras.h"
@@ -1691,6 +1691,9 @@ static const struct file_operations 
amdgpu_driver_kms_fops = {
 #ifdef CONFIG_COMPAT
.compat_ioctl = amdgpu_kms_compat_ioctl,
 #endif
+#ifdef CONFIG_PROC_FS
+   .show_fdinfo = amdgpu_show_fdinfo
+#endif
 };
 
 int 

Re: [PATCH 2/8] drm/amdgpu: Remove unused function amdgpu_bo_fbdev_mmap()

2021-04-06 Thread Christian König

Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:

Remove an unused function. Mapping the fbdev framebuffer is apparently
not supported.

Signed-off-by: Thomas Zimmermann 


Reviewed-by: Christian König 

Should I just upstream this through our internal branches?

Thanks,
Christian.


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 19 ---
  drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 --
  2 files changed, 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index b99e9d8736c2..cfc89164dee8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1092,25 +1092,6 @@ void amdgpu_bo_fini(struct amdgpu_device *adev)
}
  }
  
-/**

- * amdgpu_bo_fbdev_mmap - mmap fbdev memory
- * @bo: _bo buffer object
- * @vma: vma as input from the fbdev mmap method
- *
- * Calls ttm_fbdev_mmap() to mmap fbdev memory if it is backed by a bo.
- *
- * Returns:
- * 0 for success or a negative error code on failure.
- */
-int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
-struct vm_area_struct *vma)
-{
-   if (vma->vm_pgoff != 0)
-   return -EACCES;
-
-   return ttm_bo_mmap_obj(vma, >tbo);
-}
-
  /**
   * amdgpu_bo_set_tiling_flags - set tiling flags
   * @bo: _bo buffer object
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 54ceb065e546..46e94d413c5c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -268,8 +268,6 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo);
  int amdgpu_bo_evict_vram(struct amdgpu_device *adev);
  int amdgpu_bo_init(struct amdgpu_device *adev);
  void amdgpu_bo_fini(struct amdgpu_device *adev);
-int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
-   struct vm_area_struct *vma);
  int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags);
  void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, u64 *tiling_flags);
  int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,


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


Re: [PATCH 8/8] drm/ttm: Remove ttm_bo_mmap() and friends

2021-04-06 Thread Christian König

Am 06.04.21 um 11:09 schrieb Thomas Zimmermann:

The function ttm_bo_mmap is unused. Remove it and it's helpers; including
the verify_access callback in struct ttm_device_funcs.

Signed-off-by: Thomas Zimmermann 


Reviewed-by: Christian König 


---
  drivers/gpu/drm/ttm/ttm_bo_vm.c | 53 -
  include/drm/ttm/ttm_bo_api.h| 13 
  include/drm/ttm/ttm_device.h| 15 --
  3 files changed, 81 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index bf4a213bc66c..6cd352399941 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -508,30 +508,6 @@ static const struct vm_operations_struct ttm_bo_vm_ops = {
.access = ttm_bo_vm_access,
  };
  
-static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_device *bdev,

- unsigned long offset,
- unsigned long pages)
-{
-   struct drm_vma_offset_node *node;
-   struct ttm_buffer_object *bo = NULL;
-
-   drm_vma_offset_lock_lookup(bdev->vma_manager);
-
-   node = drm_vma_offset_lookup_locked(bdev->vma_manager, offset, pages);
-   if (likely(node)) {
-   bo = container_of(node, struct ttm_buffer_object,
- base.vma_node);
-   bo = ttm_bo_get_unless_zero(bo);
-   }
-
-   drm_vma_offset_unlock_lookup(bdev->vma_manager);
-
-   if (!bo)
-   pr_err("Could not find buffer object to map\n");
-
-   return bo;
-}
-
  static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct 
vm_area_struct *vma)
  {
/*
@@ -559,35 +535,6 @@ static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object 
*bo, struct vm_area_s
vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
  }
  
-int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,

-   struct ttm_device *bdev)
-{
-   struct ttm_buffer_object *bo;
-   int ret;
-
-   if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
-   return -EINVAL;
-
-   bo = ttm_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
-   if (unlikely(!bo))
-   return -EINVAL;
-
-   if (unlikely(!bo->bdev->funcs->verify_access)) {
-   ret = -EPERM;
-   goto out_unref;
-   }
-   ret = bo->bdev->funcs->verify_access(bo, filp);
-   if (unlikely(ret != 0))
-   goto out_unref;
-
-   ttm_bo_mmap_vma_setup(bo, vma);
-   return 0;
-out_unref:
-   ttm_bo_put(bo);
-   return ret;
-}
-EXPORT_SYMBOL(ttm_bo_mmap);
-
  int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
  {
ttm_bo_get(bo);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 2155e2e38aec..6e35680ac01b 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -522,19 +522,6 @@ void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct 
dma_buf_map *map);
   */
  int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo);
  
-/**

- * ttm_bo_mmap - mmap out of the ttm device address space.
- *
- * @filp:  filp as input from the mmap method.
- * @vma:   vma as input from the mmap method.
- * @bdev:  Pointer to the ttm_device with the address space manager.
- *
- * This function is intended to be called by the device mmap method.
- * if the device address space is to be backed by the bo manager.
- */
-int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
-   struct ttm_device *bdev);
-
  /**
   * ttm_bo_io
   *
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index 7c8f87bd52d3..cd592f8e941b 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -161,21 +161,6 @@ struct ttm_device_funcs {
struct ttm_resource *new_mem,
struct ttm_place *hop);
  
-	/**

-* struct ttm_bo_driver_member verify_access
-*
-* @bo: Pointer to a buffer object.
-* @filp: Pointer to a struct file trying to access the object.
-*
-* Called from the map / write / read methods to verify that the
-* caller is permitted to access the buffer object.
-* This member may be set to NULL, which will refuse this kind of
-* access for all buffer objects.
-* This function should return 0 if access is granted, -EPERM otherwise.
-*/
-   int (*verify_access)(struct ttm_buffer_object *bo,
-struct file *filp);
-
/**
 * Hook to notify driver about a resource delete.
 */


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


Re: [PATCH 4/8] drm/radeon: Implement mmap as GEM object function

2021-04-06 Thread Christian König

Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:

Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

This change also allows to support prime-based mmap via DRM's helper
drm_gem_prime_mmap().

Permission checks are implemented by drm_gem_mmap(), with an additional
check for radeon_ttm_tt_has_userptr() in the GEM object function. The
function radeon_verify_access() is now unused and has thus been removed.

As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.

Signed-off-by: Thomas Zimmermann 
---
  drivers/gpu/drm/radeon/radeon_drv.c |  3 +-
  drivers/gpu/drm/radeon/radeon_gem.c | 52 +++
  drivers/gpu/drm/radeon/radeon_ttm.c | 65 -
  drivers/gpu/drm/radeon/radeon_ttm.h |  1 -
  4 files changed, 54 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index efeb115ae70e..4039b6d71aa2 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -557,7 +557,7 @@ static const struct file_operations radeon_driver_kms_fops 
= {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = radeon_drm_ioctl,
-   .mmap = radeon_mmap,
+   .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
  #ifdef CONFIG_COMPAT
@@ -632,6 +632,7 @@ static const struct drm_driver kms_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
+   .gem_prime_mmap = drm_gem_prime_mmap,
  
  	.name = DRIVER_NAME,

.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
b/drivers/gpu/drm/radeon/radeon_gem.c
index 05ea2f39f626..71e8737bce01 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -44,6 +44,42 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj);
  
  const struct drm_gem_object_funcs radeon_gem_object_funcs;
  
+static vm_fault_t radeon_ttm_fault(struct vm_fault *vmf)


Please name this radeon_gem_fault or radeon_gem_object_fault.

Apart from that looks good to me.

Christian.


+{
+   struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+   struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
+   vm_fault_t ret;
+
+   down_read(>pm.mclk_lock);
+
+   ret = ttm_bo_vm_reserve(bo, vmf);
+   if (ret)
+   goto unlock_mclk;
+
+   ret = radeon_bo_fault_reserve_notify(bo);
+   if (ret)
+   goto unlock_resv;
+
+   ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
+  TTM_BO_VM_NUM_PREFAULT, 1);
+   if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+   goto unlock_mclk;
+
+unlock_resv:
+   dma_resv_unlock(bo->base.resv);
+
+unlock_mclk:
+   up_read(>pm.mclk_lock);
+   return ret;
+}
+
+static const struct vm_operations_struct radeon_ttm_vm_ops = {
+   .fault = radeon_ttm_fault,
+   .open = ttm_bo_vm_open,
+   .close = ttm_bo_vm_close,
+   .access = ttm_bo_vm_access
+};
+
  static void radeon_gem_object_free(struct drm_gem_object *gobj)
  {
struct radeon_bo *robj = gem_to_radeon_bo(gobj);
@@ -226,6 +262,20 @@ static int radeon_gem_handle_lockup(struct radeon_device 
*rdev, int r)
return r;
  }
  
+static int radeon_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)

+{
+   struct radeon_bo *bo = gem_to_radeon_bo(obj);
+   struct radeon_device *rdev = radeon_get_rdev(bo->tbo.bdev);
+
+   if (!rdev)
+   return -EINVAL;
+
+   if (radeon_ttm_tt_has_userptr(rdev, bo->tbo.ttm))
+   return -EPERM;
+
+   return drm_gem_ttm_mmap(obj, vma);
+}
+
  const struct drm_gem_object_funcs radeon_gem_object_funcs = {
.free = radeon_gem_object_free,
.open = radeon_gem_object_open,
@@ -236,6 +286,8 @@ const struct drm_gem_object_funcs radeon_gem_object_funcs = 
{
.get_sg_table = radeon_gem_prime_get_sg_table,
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
+   .mmap = radeon_gem_object_mmap,
+   .vm_ops = _ttm_vm_ops,
  };
  
  /*

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 476ce9c24b9f..a5ce43a909a2 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -136,17 +136,6 @@ static void radeon_evict_flags(struct ttm_buffer_object 
*bo,
*placement = rbo->placement;
  }
  
-static int radeon_verify_access(struct ttm_buffer_object *bo, struct file *filp)

-{
-   struct radeon_bo *rbo = container_of(bo, struct radeon_bo, tbo);
-   struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
-
-   if (radeon_ttm_tt_has_userptr(rdev, 

Re: [PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function

2021-04-06 Thread Christian König

Am 06.04.21 um 11:08 schrieb Thomas Zimmermann:

Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

This change resolves several inconsistencies between regular mmap and
prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
areas. Previously it way only set for regular mmap calls, prime-based
mmap used TTM's default vm_ops. The check for kfd_bo has been taken
from amdgpu_verify_access(), which is not called any longer and has
been removed.

As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.

Signed-off-by: Thomas Zimmermann 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h |  2 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  4 +-
  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 64 +++
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 71 -
  drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  1 -
  6 files changed, 66 insertions(+), 122 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index e0c4f7c7f1b9..19c5ab08d9ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -42,52 +42,6 @@
  #include 
  #include 
  
-/**

- * amdgpu_gem_prime_mmap - _driver.gem_prime_mmap implementation
- * @obj: GEM BO
- * @vma: Virtual memory area
- *
- * Sets up a userspace mapping of the BO's memory in the given
- * virtual memory area.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
- */
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma)
-{
-   struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
-   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
-   unsigned asize = amdgpu_bo_size(bo);
-   int ret;
-
-   if (!vma->vm_file)
-   return -ENODEV;
-
-   if (adev == NULL)
-   return -ENODEV;
-
-   /* Check for valid size. */
-   if (asize < vma->vm_end - vma->vm_start)
-   return -EINVAL;
-
-   if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
-   (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
-   return -EPERM;
-   }
-   vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
-
-   /* prime mmap does not need to check access, so allow here */
-   ret = drm_vma_node_allow(>vma_node, vma->vm_file->private_data);
-   if (ret)
-   return ret;
-
-   ret = ttm_bo_mmap(vma->vm_file, vma, >mman.bdev);
-   drm_vma_node_revoke(>vma_node, vma->vm_file->private_data);
-
-   return ret;
-}
-
  static int
  __dma_resv_make_exclusive(struct dma_resv *obj)
  {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
index 39b5b9616fd8..3e93b9b407a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -31,8 +31,6 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct 
drm_device *dev,
struct dma_buf *dma_buf);
  bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
  struct amdgpu_bo *bo);
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma);
  
  extern const struct dma_buf_ops amdgpu_dmabuf_ops;
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c

index 76f48f79c70b..e96d2758f4bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1656,7 +1656,7 @@ static const struct file_operations 
amdgpu_driver_kms_fops = {
.flush = amdgpu_flush,
.release = drm_release,
.unlocked_ioctl = amdgpu_drm_ioctl,
-   .mmap = amdgpu_mmap,
+   .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
  #ifdef CONFIG_COMPAT
@@ -1719,7 +1719,7 @@ static const struct drm_driver amdgpu_kms_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import = amdgpu_gem_prime_import,
-   .gem_prime_mmap = amdgpu_gem_prime_mmap,
+   .gem_prime_mmap = drm_gem_prime_mmap,
  
  	.name = DRIVER_NAME,

.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index fb7171e5507c..fe93faad05f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -41,6 +41,36 @@
  
  static const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
  
+static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)


Please name that function amdgpu_gem_fault or amdgpu_gem_object_fault


+{
+   struct ttm_buffer_object *bo = 

Re: [PATCH] drm/amdgpu: fix gfx9 rlc modprobe rlcg program timeout issue

2021-04-06 Thread Huang Rui
On Tue, Apr 06, 2021 at 03:29:48PM +0800, Zhu, Changfeng wrote:
> From: changzhu 
> 
> From: Changfeng 
> 
> It needs to add amdgpu_sriov_fullaccess judgement as gfx_v10_rlcg_wreg
> when doing gfx_v9_0_rlcg_wreg.
> Or it will cause modprobe issue as below:
> kernel: [   59.992843] amdgpu: timeout: rlcg program reg:0x02984 failed!
> 
> Fix for patch:
> drm/amdgpu: indirect register access for nv12 sriov
> 
> Change-Id: I971804e4e8dbd83e4179beefa8ae8a06bd52913b
> Signed-off-by: Changfeng 

Acked-by: Huang Rui 

> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 2111e4c46a52..06811a1f4625 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -734,7 +734,7 @@ static const u32 GFX_RLC_SRM_INDEX_CNTL_DATA_OFFSETS[] =
>   mmRLC_SRM_INDEX_CNTL_DATA_7 - mmRLC_SRM_INDEX_CNTL_DATA_0,
>  };
>  
> -void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v, u32 
> flag)
> +static void gfx_v9_0_rlcg_rw(struct amdgpu_device *adev, u32 offset, u32 v, 
> u32 flag)
>  {
>   static void *scratch_reg0;
>   static void *scratch_reg1;
> @@ -787,6 +787,20 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 
> offset, u32 v, u32 flag)
>  
>  }
>  
> +static void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 
> v, u32 flag)
> +{
> + if (amdgpu_sriov_fullaccess(adev)) {
> + gfx_v9_0_rlcg_rw(adev, offset, v, flag);
> +
> + return;
> + }
> +
> + if (flag & AMDGPU_REGS_NO_KIQ)
> + WREG32_NO_KIQ(offset, v);
> + else
> + WREG32(offset, v);
> +}
> +
>  #define VEGA10_GB_ADDR_CONFIG_GOLDEN 0x2a114042
>  #define VEGA12_GB_ADDR_CONFIG_GOLDEN 0x24104041
>  #define RAVEN_GB_ADDR_CONFIG_GOLDEN 0x2442
> -- 
> 2.17.1
> 
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx


[PATCH 8/8] drm/ttm: Remove ttm_bo_mmap() and friends

2021-04-06 Thread Thomas Zimmermann
The function ttm_bo_mmap is unused. Remove it and it's helpers; including
the verify_access callback in struct ttm_device_funcs.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 53 -
 include/drm/ttm/ttm_bo_api.h| 13 
 include/drm/ttm/ttm_device.h| 15 --
 3 files changed, 81 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index bf4a213bc66c..6cd352399941 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -508,30 +508,6 @@ static const struct vm_operations_struct ttm_bo_vm_ops = {
.access = ttm_bo_vm_access,
 };
 
-static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_device *bdev,
- unsigned long offset,
- unsigned long pages)
-{
-   struct drm_vma_offset_node *node;
-   struct ttm_buffer_object *bo = NULL;
-
-   drm_vma_offset_lock_lookup(bdev->vma_manager);
-
-   node = drm_vma_offset_lookup_locked(bdev->vma_manager, offset, pages);
-   if (likely(node)) {
-   bo = container_of(node, struct ttm_buffer_object,
- base.vma_node);
-   bo = ttm_bo_get_unless_zero(bo);
-   }
-
-   drm_vma_offset_unlock_lookup(bdev->vma_manager);
-
-   if (!bo)
-   pr_err("Could not find buffer object to map\n");
-
-   return bo;
-}
-
 static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct 
vm_area_struct *vma)
 {
/*
@@ -559,35 +535,6 @@ static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object 
*bo, struct vm_area_s
vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
 }
 
-int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
-   struct ttm_device *bdev)
-{
-   struct ttm_buffer_object *bo;
-   int ret;
-
-   if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
-   return -EINVAL;
-
-   bo = ttm_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
-   if (unlikely(!bo))
-   return -EINVAL;
-
-   if (unlikely(!bo->bdev->funcs->verify_access)) {
-   ret = -EPERM;
-   goto out_unref;
-   }
-   ret = bo->bdev->funcs->verify_access(bo, filp);
-   if (unlikely(ret != 0))
-   goto out_unref;
-
-   ttm_bo_mmap_vma_setup(bo, vma);
-   return 0;
-out_unref:
-   ttm_bo_put(bo);
-   return ret;
-}
-EXPORT_SYMBOL(ttm_bo_mmap);
-
 int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
 {
ttm_bo_get(bo);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 2155e2e38aec..6e35680ac01b 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -522,19 +522,6 @@ void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct 
dma_buf_map *map);
  */
 int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo);
 
-/**
- * ttm_bo_mmap - mmap out of the ttm device address space.
- *
- * @filp:  filp as input from the mmap method.
- * @vma:   vma as input from the mmap method.
- * @bdev:  Pointer to the ttm_device with the address space manager.
- *
- * This function is intended to be called by the device mmap method.
- * if the device address space is to be backed by the bo manager.
- */
-int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
-   struct ttm_device *bdev);
-
 /**
  * ttm_bo_io
  *
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index 7c8f87bd52d3..cd592f8e941b 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -161,21 +161,6 @@ struct ttm_device_funcs {
struct ttm_resource *new_mem,
struct ttm_place *hop);
 
-   /**
-* struct ttm_bo_driver_member verify_access
-*
-* @bo: Pointer to a buffer object.
-* @filp: Pointer to a struct file trying to access the object.
-*
-* Called from the map / write / read methods to verify that the
-* caller is permitted to access the buffer object.
-* This member may be set to NULL, which will refuse this kind of
-* access for all buffer objects.
-* This function should return 0 if access is granted, -EPERM otherwise.
-*/
-   int (*verify_access)(struct ttm_buffer_object *bo,
-struct file *filp);
-
/**
 * Hook to notify driver about a resource delete.
 */
-- 
2.30.2

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


[PATCH 7/8] drm/vmwgfx: Inline vmw_verify_access()

2021-04-06 Thread Thomas Zimmermann
Vmwgfx is the only user of the TTM's verify_access callback. Inline
the call and avoid the indirection through the function pointer.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 -
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c   | 7 ++-
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 2dc031fe4a90..a079734f9d68 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -658,14 +658,6 @@ static void vmw_evict_flags(struct ttm_buffer_object *bo,
*placement = vmw_sys_placement;
 }
 
-static int vmw_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-   struct ttm_object_file *tfile =
-   vmw_fpriv((struct drm_file *)filp->private_data)->tfile;
-
-   return vmw_user_bo_verify_access(bo, tfile);
-}
-
 static int vmw_ttm_io_mem_reserve(struct ttm_device *bdev, struct ttm_resource 
*mem)
 {
struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, 
bdev);
@@ -768,7 +760,6 @@ struct ttm_device_funcs vmw_bo_driver = {
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = vmw_evict_flags,
.move = vmw_move,
-   .verify_access = vmw_verify_access,
.swap_notify = vmw_swap_notify,
.io_mem_reserve = _ttm_io_mem_reserve,
 };
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
index 3eaad00668f2..2574d4707407 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
@@ -65,6 +65,7 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
};
struct drm_file *file_priv = filp->private_data;
struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
+   struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
struct ttm_device *bdev = _priv->bdev;
struct ttm_buffer_object *bo;
int ret;
@@ -76,11 +77,7 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
if (unlikely(!bo))
return -EINVAL;
 
-   if (unlikely(!bo->bdev->funcs->verify_access)) {
-   ret = -EPERM;
-   goto out_unref;
-   }
-   ret = bo->bdev->funcs->verify_access(bo, filp);
+   ret = vmw_user_bo_verify_access(bo, tfile);
if (unlikely(ret != 0))
goto out_unref;
 
-- 
2.30.2

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


[PATCH 6/8] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver

2021-04-06 Thread Thomas Zimmermann
The vmwgfx driver is the only remaining user of ttm_bo_mmap(). Inline
the code. The internal helper ttm_bo_vm_lookup() is now also part of
vmwgfx as vmw_bo_vm_lookup().

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 54 ++--
 1 file changed, 51 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
index cb9975889e2f..3eaad00668f2 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
@@ -27,6 +27,30 @@
 
 #include "vmwgfx_drv.h"
 
+static struct ttm_buffer_object *vmw_bo_vm_lookup(struct ttm_device *bdev,
+ unsigned long offset,
+ unsigned long pages)
+{
+   struct drm_vma_offset_node *node;
+   struct ttm_buffer_object *bo = NULL;
+
+   drm_vma_offset_lock_lookup(bdev->vma_manager);
+
+   node = drm_vma_offset_lookup_locked(bdev->vma_manager, offset, pages);
+   if (likely(node)) {
+   bo = container_of(node, struct ttm_buffer_object,
+ base.vma_node);
+   bo = ttm_bo_get_unless_zero(bo);
+   }
+
+   drm_vma_offset_unlock_lookup(bdev->vma_manager);
+
+   if (!bo)
+   pr_err("Could not find buffer object to map\n");
+
+   return bo;
+}
+
 int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
 {
static const struct vm_operations_struct vmw_vm_ops = {
@@ -41,10 +65,28 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
};
struct drm_file *file_priv = filp->private_data;
struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
-   int ret = ttm_bo_mmap(filp, vma, _priv->bdev);
+   struct ttm_device *bdev = _priv->bdev;
+   struct ttm_buffer_object *bo;
+   int ret;
+
+   if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
+   return -EINVAL;
+
+   bo = vmw_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
+   if (unlikely(!bo))
+   return -EINVAL;
 
-   if (ret)
-   return ret;
+   if (unlikely(!bo->bdev->funcs->verify_access)) {
+   ret = -EPERM;
+   goto out_unref;
+   }
+   ret = bo->bdev->funcs->verify_access(bo, filp);
+   if (unlikely(ret != 0))
+   goto out_unref;
+
+   ret = ttm_bo_mmap_obj(vma, bo);
+   if (unlikely(ret != 0))
+   goto out_unref;
 
vma->vm_ops = _vm_ops;
 
@@ -52,7 +94,13 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
if (!is_cow_mapping(vma->vm_flags))
vma->vm_flags = (vma->vm_flags & ~VM_MIXEDMAP) | VM_PFNMAP;
 
+   ttm_bo_put(bo); /* release extra ref taken by ttm_bo_mmap_obj() */
+
return 0;
+
+out_unref:
+   ttm_bo_put(bo);
+   return ret;
 }
 
 /* struct vmw_validation_mem callback */
-- 
2.30.2

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


[PATCH 4/8] drm/radeon: Implement mmap as GEM object function

2021-04-06 Thread Thomas Zimmermann
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

This change also allows to support prime-based mmap via DRM's helper
drm_gem_prime_mmap().

Permission checks are implemented by drm_gem_mmap(), with an additional
check for radeon_ttm_tt_has_userptr() in the GEM object function. The
function radeon_verify_access() is now unused and has thus been removed.

As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/radeon/radeon_drv.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c | 52 +++
 drivers/gpu/drm/radeon/radeon_ttm.c | 65 -
 drivers/gpu/drm/radeon/radeon_ttm.h |  1 -
 4 files changed, 54 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index efeb115ae70e..4039b6d71aa2 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -557,7 +557,7 @@ static const struct file_operations radeon_driver_kms_fops 
= {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = radeon_drm_ioctl,
-   .mmap = radeon_mmap,
+   .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
 #ifdef CONFIG_COMPAT
@@ -632,6 +632,7 @@ static const struct drm_driver kms_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
+   .gem_prime_mmap = drm_gem_prime_mmap,
 
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
b/drivers/gpu/drm/radeon/radeon_gem.c
index 05ea2f39f626..71e8737bce01 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -44,6 +44,42 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj);
 
 const struct drm_gem_object_funcs radeon_gem_object_funcs;
 
+static vm_fault_t radeon_ttm_fault(struct vm_fault *vmf)
+{
+   struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+   struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
+   vm_fault_t ret;
+
+   down_read(>pm.mclk_lock);
+
+   ret = ttm_bo_vm_reserve(bo, vmf);
+   if (ret)
+   goto unlock_mclk;
+
+   ret = radeon_bo_fault_reserve_notify(bo);
+   if (ret)
+   goto unlock_resv;
+
+   ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
+  TTM_BO_VM_NUM_PREFAULT, 1);
+   if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+   goto unlock_mclk;
+
+unlock_resv:
+   dma_resv_unlock(bo->base.resv);
+
+unlock_mclk:
+   up_read(>pm.mclk_lock);
+   return ret;
+}
+
+static const struct vm_operations_struct radeon_ttm_vm_ops = {
+   .fault = radeon_ttm_fault,
+   .open = ttm_bo_vm_open,
+   .close = ttm_bo_vm_close,
+   .access = ttm_bo_vm_access
+};
+
 static void radeon_gem_object_free(struct drm_gem_object *gobj)
 {
struct radeon_bo *robj = gem_to_radeon_bo(gobj);
@@ -226,6 +262,20 @@ static int radeon_gem_handle_lockup(struct radeon_device 
*rdev, int r)
return r;
 }
 
+static int radeon_gem_object_mmap(struct drm_gem_object *obj, struct 
vm_area_struct *vma)
+{
+   struct radeon_bo *bo = gem_to_radeon_bo(obj);
+   struct radeon_device *rdev = radeon_get_rdev(bo->tbo.bdev);
+
+   if (!rdev)
+   return -EINVAL;
+
+   if (radeon_ttm_tt_has_userptr(rdev, bo->tbo.ttm))
+   return -EPERM;
+
+   return drm_gem_ttm_mmap(obj, vma);
+}
+
 const struct drm_gem_object_funcs radeon_gem_object_funcs = {
.free = radeon_gem_object_free,
.open = radeon_gem_object_open,
@@ -236,6 +286,8 @@ const struct drm_gem_object_funcs radeon_gem_object_funcs = 
{
.get_sg_table = radeon_gem_prime_get_sg_table,
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
+   .mmap = radeon_gem_object_mmap,
+   .vm_ops = _ttm_vm_ops,
 };
 
 /*
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 476ce9c24b9f..a5ce43a909a2 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -136,17 +136,6 @@ static void radeon_evict_flags(struct ttm_buffer_object 
*bo,
*placement = rbo->placement;
 }
 
-static int radeon_verify_access(struct ttm_buffer_object *bo, struct file 
*filp)
-{
-   struct radeon_bo *rbo = container_of(bo, struct radeon_bo, tbo);
-   struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
-
-   if (radeon_ttm_tt_has_userptr(rdev, bo->ttm))
-   return -EPERM;
-   return drm_vma_node_verify_access(>tbo.base.vma_node,
- filp->private_data);
-}
-
 static 

[PATCH 5/8] drm/nouveau: Implement mmap as GEM object function

2021-04-06 Thread Thomas Zimmermann
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

The GEM object function is provided by GEM TTM helpers. Nouveau's
implementation of verify_access is unused and has been removed. Access
permissions are validated by the DRM helpers.

As a side effect, nouveau_ttm_vm_ops and nouveau_ttm_fault() are now
implemented in nouveau's GEM code.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/nouveau/nouveau_bo.c  | 10 --
 drivers/gpu/drm/nouveau/nouveau_drm.c |  3 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c | 36 
 drivers/gpu/drm/nouveau/nouveau_ttm.c | 49 ---
 drivers/gpu/drm/nouveau/nouveau_ttm.h |  1 -
 5 files changed, 38 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 3e09df0472ce..bc67cbccc83b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1051,15 +1051,6 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
return ret;
 }
 
-static int
-nouveau_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-   struct nouveau_bo *nvbo = nouveau_bo(bo);
-
-   return drm_vma_node_verify_access(>bo.base.vma_node,
- filp->private_data);
-}
-
 static void
 nouveau_ttm_io_mem_free_locked(struct nouveau_drm *drm,
   struct ttm_resource *reg)
@@ -1332,7 +1323,6 @@ struct ttm_device_funcs nouveau_bo_driver = {
.evict_flags = nouveau_bo_evict_flags,
.delete_mem_notify = nouveau_bo_delete_mem_notify,
.move = nouveau_bo_move,
-   .verify_access = nouveau_bo_verify_access,
.io_mem_reserve = _ttm_io_mem_reserve,
.io_mem_free = _ttm_io_mem_free,
 };
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 885815ea917f..7586328c1de5 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1177,7 +1177,7 @@ nouveau_driver_fops = {
.open = drm_open,
.release = drm_release,
.unlocked_ioctl = nouveau_drm_ioctl,
-   .mmap = nouveau_ttm_mmap,
+   .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
 #if defined(CONFIG_COMPAT)
@@ -1210,6 +1210,7 @@ driver_stub = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import_sg_table = nouveau_gem_prime_import_sg_table,
+   .gem_prime_mmap = drm_gem_prime_mmap,
 
.dumb_create = nouveau_display_dumb_create,
.dumb_map_offset = nouveau_display_dumb_map_offset,
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c 
b/drivers/gpu/drm/nouveau/nouveau_gem.c
index c88cbb85f101..71dfac820c4d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -39,6 +39,40 @@
 #include 
 #include 
 
+static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
+{
+   struct vm_area_struct *vma = vmf->vma;
+   struct ttm_buffer_object *bo = vma->vm_private_data;
+   pgprot_t prot;
+   vm_fault_t ret;
+
+   ret = ttm_bo_vm_reserve(bo, vmf);
+   if (ret)
+   return ret;
+
+   ret = nouveau_ttm_fault_reserve_notify(bo);
+   if (ret)
+   goto error_unlock;
+
+   nouveau_bo_del_io_reserve_lru(bo);
+   prot = vm_get_page_prot(vma->vm_flags);
+   ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1);
+   nouveau_bo_add_io_reserve_lru(bo);
+   if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+   return ret;
+
+error_unlock:
+   dma_resv_unlock(bo->base.resv);
+   return ret;
+}
+
+static const struct vm_operations_struct nouveau_ttm_vm_ops = {
+   .fault = nouveau_ttm_fault,
+   .open = ttm_bo_vm_open,
+   .close = ttm_bo_vm_close,
+   .access = ttm_bo_vm_access
+};
+
 void
 nouveau_gem_object_del(struct drm_gem_object *gem)
 {
@@ -180,6 +214,8 @@ const struct drm_gem_object_funcs nouveau_gem_object_funcs 
= {
.get_sg_table = nouveau_gem_prime_get_sg_table,
.vmap = drm_gem_ttm_vmap,
.vunmap = drm_gem_ttm_vunmap,
+   .mmap = drm_gem_ttm_mmap,
+   .vm_ops = _ttm_vm_ops,
 };
 
 int
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c 
b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index b81ae90b8449..e511a26379da 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -127,55 +127,6 @@ const struct ttm_resource_manager_func nv04_gart_manager = 
{
.free = nouveau_manager_del,
 };
 
-static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
-{
-   struct vm_area_struct *vma = vmf->vma;
-   struct ttm_buffer_object *bo = vma->vm_private_data;
-   pgprot_t prot;
-   vm_fault_t ret;
-
-   ret = ttm_bo_vm_reserve(bo, vmf);
- 

[PATCH 3/8] drm/amdgpu: Implement mmap as GEM object function

2021-04-06 Thread Thomas Zimmermann
Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

This change resolves several inconsistencies between regular mmap and
prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
areas. Previously it way only set for regular mmap calls, prime-based
mmap used TTM's default vm_ops. The check for kfd_bo has been taken
from amdgpu_verify_access(), which is not called any longer and has
been removed.

As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h |  2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 64 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 71 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  1 -
 6 files changed, 66 insertions(+), 122 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index e0c4f7c7f1b9..19c5ab08d9ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -42,52 +42,6 @@
 #include 
 #include 
 
-/**
- * amdgpu_gem_prime_mmap - _driver.gem_prime_mmap implementation
- * @obj: GEM BO
- * @vma: Virtual memory area
- *
- * Sets up a userspace mapping of the BO's memory in the given
- * virtual memory area.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
- */
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma)
-{
-   struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
-   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
-   unsigned asize = amdgpu_bo_size(bo);
-   int ret;
-
-   if (!vma->vm_file)
-   return -ENODEV;
-
-   if (adev == NULL)
-   return -ENODEV;
-
-   /* Check for valid size. */
-   if (asize < vma->vm_end - vma->vm_start)
-   return -EINVAL;
-
-   if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
-   (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
-   return -EPERM;
-   }
-   vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
-
-   /* prime mmap does not need to check access, so allow here */
-   ret = drm_vma_node_allow(>vma_node, vma->vm_file->private_data);
-   if (ret)
-   return ret;
-
-   ret = ttm_bo_mmap(vma->vm_file, vma, >mman.bdev);
-   drm_vma_node_revoke(>vma_node, vma->vm_file->private_data);
-
-   return ret;
-}
-
 static int
 __dma_resv_make_exclusive(struct dma_resv *obj)
 {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
index 39b5b9616fd8..3e93b9b407a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -31,8 +31,6 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct 
drm_device *dev,
struct dma_buf *dma_buf);
 bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
  struct amdgpu_bo *bo);
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
- struct vm_area_struct *vma);
 
 extern const struct dma_buf_ops amdgpu_dmabuf_ops;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 76f48f79c70b..e96d2758f4bb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1656,7 +1656,7 @@ static const struct file_operations 
amdgpu_driver_kms_fops = {
.flush = amdgpu_flush,
.release = drm_release,
.unlocked_ioctl = amdgpu_drm_ioctl,
-   .mmap = amdgpu_mmap,
+   .mmap = drm_gem_mmap,
.poll = drm_poll,
.read = drm_read,
 #ifdef CONFIG_COMPAT
@@ -1719,7 +1719,7 @@ static const struct drm_driver amdgpu_kms_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import = amdgpu_gem_prime_import,
-   .gem_prime_mmap = amdgpu_gem_prime_mmap,
+   .gem_prime_mmap = drm_gem_prime_mmap,
 
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index fb7171e5507c..fe93faad05f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -41,6 +41,36 @@
 
 static const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
 
+static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)
+{
+   struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+   vm_fault_t ret;
+
+   ret = ttm_bo_vm_reserve(bo, vmf);
+   if (ret)
+   return ret;
+
+  

[PATCH 1/8] drm/ttm: Don't override vm_ops callbacks, if set

2021-04-06 Thread Thomas Zimmermann
Drivers may want to set their own callbacks for a VM area. Only set
TTM's callbacks if the vm_ops field is clear.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index b31b18058965..bf4a213bc66c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -534,7 +534,12 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct 
ttm_device *bdev,
 
 static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct 
vm_area_struct *vma)
 {
-   vma->vm_ops = _bo_vm_ops;
+   /*
+* Drivers may want to override the vm_ops field. Otherwise we
+* use TTM's default callbacks.
+*/
+   if (!vma->vm_ops)
+   vma->vm_ops = _bo_vm_ops;
 
/*
 * Note: We're transferring the bo reference to
-- 
2.30.2

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


[PATCH 2/8] drm/amdgpu: Remove unused function amdgpu_bo_fbdev_mmap()

2021-04-06 Thread Thomas Zimmermann
Remove an unused function. Mapping the fbdev framebuffer is apparently
not supported.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 19 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 --
 2 files changed, 21 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index b99e9d8736c2..cfc89164dee8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -1092,25 +1092,6 @@ void amdgpu_bo_fini(struct amdgpu_device *adev)
}
 }
 
-/**
- * amdgpu_bo_fbdev_mmap - mmap fbdev memory
- * @bo: _bo buffer object
- * @vma: vma as input from the fbdev mmap method
- *
- * Calls ttm_fbdev_mmap() to mmap fbdev memory if it is backed by a bo.
- *
- * Returns:
- * 0 for success or a negative error code on failure.
- */
-int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
-struct vm_area_struct *vma)
-{
-   if (vma->vm_pgoff != 0)
-   return -EACCES;
-
-   return ttm_bo_mmap_obj(vma, >tbo);
-}
-
 /**
  * amdgpu_bo_set_tiling_flags - set tiling flags
  * @bo: _bo buffer object
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 54ceb065e546..46e94d413c5c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -268,8 +268,6 @@ void amdgpu_bo_unpin(struct amdgpu_bo *bo);
 int amdgpu_bo_evict_vram(struct amdgpu_device *adev);
 int amdgpu_bo_init(struct amdgpu_device *adev);
 void amdgpu_bo_fini(struct amdgpu_device *adev);
-int amdgpu_bo_fbdev_mmap(struct amdgpu_bo *bo,
-   struct vm_area_struct *vma);
 int amdgpu_bo_set_tiling_flags(struct amdgpu_bo *bo, u64 tiling_flags);
 void amdgpu_bo_get_tiling_flags(struct amdgpu_bo *bo, u64 *tiling_flags);
 int amdgpu_bo_set_metadata (struct amdgpu_bo *bo, void *metadata,
-- 
2.30.2

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


[PATCH 0/8] drm: Clean up mmap for TTM-based GEM drivers

2021-04-06 Thread Thomas Zimmermann
Implement mmap via struct drm_gem_object_functions.mmap for amdgpu,
radeon and nouveau. This allows for using common DRM helpers for
the mmap-related callbacks in struct file_operations and struct
drm_driver. The drivers have their own vm_ops, which are now set
automatically by the DRM core functions. The code in each driver's
verify_access becomes part of the driver's new mmap implementation.

With the GEM drivers converted, vmwgfx is the only user of
ttm_bo_mmap() and related infrastructure. So move everything into
vmwgfx and delete the rsp code from TTM.

This touches several drivers. Preferably everything would be merged
at once via drm-misc-next.

Thomas Zimmermann (8):
  drm/ttm: Don't override vm_ops callbacks, if set
  drm/amdgpu: Remove unused function amdgpu_bo_fbdev_mmap()
  drm/amdgpu: Implement mmap as GEM object function
  drm/radeon: Implement mmap as GEM object function
  drm/nouveau: Implement mmap as GEM object function
  drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
  drm/vmwgfx: Inline vmw_verify_access()
  drm/ttm: Remove ttm_bo_mmap() and friends

 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h |  2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 64 +++
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 19 --
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 71 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h |  1 -
 drivers/gpu/drm/nouveau/nouveau_bo.c| 10 ---
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  3 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c   | 36 +++
 drivers/gpu/drm/nouveau/nouveau_ttm.c   | 49 --
 drivers/gpu/drm/nouveau/nouveau_ttm.h   |  1 -
 drivers/gpu/drm/radeon/radeon_drv.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c | 52 +++
 drivers/gpu/drm/radeon/radeon_ttm.c | 65 ---
 drivers/gpu/drm/radeon/radeon_ttm.h |  1 -
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 60 ++---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c  |  9 ---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c| 51 ++-
 include/drm/ttm/ttm_bo_api.h| 13 
 include/drm/ttm/ttm_device.h| 15 -
 22 files changed, 212 insertions(+), 365 deletions(-)

--
2.30.2

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


RE: [PATCH] drm/amdgpu: fix gfx9 rlc modprobe rlcg program timeout issue

2021-04-06 Thread Deng, Emily
[AMD Official Use Only - Internal Distribution Only]

Reviewed-by: Emily Deng 

>-Original Message-
>From: Zhu, Changfeng 
>Sent: Tuesday, April 6, 2021 3:30 PM
>To: amd-gfx@lists.freedesktop.org; Huang, Ray ;
>Zhou, Peng Ju ; Deng, Emily 
>Cc: Zhu, Changfeng 
>Subject: [PATCH] drm/amdgpu: fix gfx9 rlc modprobe rlcg program timeout
>issue
>
>From: changzhu 
>
>From: Changfeng 
>
>It needs to add amdgpu_sriov_fullaccess judgement as gfx_v10_rlcg_wreg
>when doing gfx_v9_0_rlcg_wreg.
>Or it will cause modprobe issue as below:
>kernel: [   59.992843] amdgpu: timeout: rlcg program reg:0x02984 failed!
>
>Fix for patch:
>drm/amdgpu: indirect register access for nv12 sriov
>
>Change-Id: I971804e4e8dbd83e4179beefa8ae8a06bd52913b
>Signed-off-by: Changfeng 
>---
> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 16 +++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>index 2111e4c46a52..06811a1f4625 100644
>--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>@@ -734,7 +734,7 @@ static const u32
>GFX_RLC_SRM_INDEX_CNTL_DATA_OFFSETS[] =
> mmRLC_SRM_INDEX_CNTL_DATA_7 -
>mmRLC_SRM_INDEX_CNTL_DATA_0,  };
>
>-void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v, u32
>flag)
>+static void gfx_v9_0_rlcg_rw(struct amdgpu_device *adev, u32 offset,
>+u32 v, u32 flag)
> {
> static void *scratch_reg0;
> static void *scratch_reg1;
>@@ -787,6 +787,20 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device
>*adev, u32 offset, u32 v, u32 flag)
>
> }
>
>+static void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset,
>+u32 v, u32 flag) {
>+if (amdgpu_sriov_fullaccess(adev)) {
>+gfx_v9_0_rlcg_rw(adev, offset, v, flag);
>+
>+return;
>+}
>+
>+if (flag & AMDGPU_REGS_NO_KIQ)
>+WREG32_NO_KIQ(offset, v);
>+else
>+WREG32(offset, v);
>+}
>+
> #define VEGA10_GB_ADDR_CONFIG_GOLDEN 0x2a114042  #define
>VEGA12_GB_ADDR_CONFIG_GOLDEN 0x24104041  #define
>RAVEN_GB_ADDR_CONFIG_GOLDEN 0x2442
>--
>2.17.1

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


[PATCH] drm/amdgpu: fix gfx9 rlc modprobe rlcg program timeout issue

2021-04-06 Thread Changfeng
From: changzhu 

From: Changfeng 

It needs to add amdgpu_sriov_fullaccess judgement as gfx_v10_rlcg_wreg
when doing gfx_v9_0_rlcg_wreg.
Or it will cause modprobe issue as below:
kernel: [   59.992843] amdgpu: timeout: rlcg program reg:0x02984 failed!

Fix for patch:
drm/amdgpu: indirect register access for nv12 sriov

Change-Id: I971804e4e8dbd83e4179beefa8ae8a06bd52913b
Signed-off-by: Changfeng 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 2111e4c46a52..06811a1f4625 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -734,7 +734,7 @@ static const u32 GFX_RLC_SRM_INDEX_CNTL_DATA_OFFSETS[] =
mmRLC_SRM_INDEX_CNTL_DATA_7 - mmRLC_SRM_INDEX_CNTL_DATA_0,
 };
 
-void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v, u32 
flag)
+static void gfx_v9_0_rlcg_rw(struct amdgpu_device *adev, u32 offset, u32 v, 
u32 flag)
 {
static void *scratch_reg0;
static void *scratch_reg1;
@@ -787,6 +787,20 @@ void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 
offset, u32 v, u32 flag)
 
 }
 
+static void gfx_v9_0_rlcg_wreg(struct amdgpu_device *adev, u32 offset, u32 v, 
u32 flag)
+{
+   if (amdgpu_sriov_fullaccess(adev)) {
+   gfx_v9_0_rlcg_rw(adev, offset, v, flag);
+
+   return;
+   }
+
+   if (flag & AMDGPU_REGS_NO_KIQ)
+   WREG32_NO_KIQ(offset, v);
+   else
+   WREG32(offset, v);
+}
+
 #define VEGA10_GB_ADDR_CONFIG_GOLDEN 0x2a114042
 #define VEGA12_GB_ADDR_CONFIG_GOLDEN 0x24104041
 #define RAVEN_GB_ADDR_CONFIG_GOLDEN 0x2442
-- 
2.17.1

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


Re: a quetion about buffer migration for user mapped bo.

2021-04-06 Thread Christian König

Yes, Andrey is right.

A top level explanation is that we don't prevent moving the buffer but 
rather we prevent user space from accessing it.


Regards,
Christian.

Am 05.04.21 um 18:34 schrieb Andrey Grodzovsky:


From my understanding and looking at the code I think we don't prevent 
but rather invalidate current user mappings and use subsequent page 
faults to map into user space process the pages from the new location. 
Check what this function is doing during move - 
https://elixir.bootlin.com/linux/v5.12-rc5/source/drivers/gpu/drm/ttm/ttm_bo.c#L238


Andrey

On 2021-04-05 12:01 p.m., 258454946 wrote:

Hi Guys,

I am a newbee of gfx development. Recently, I am researching amdgpu 
open source driver, and encounter a problem, but do not find the answer.


We know the user maybe map a gem backing buffer for reading/writing 
and hold the mapping for a long term. while, kernel driver will also 
moves the user mapped bo to other memory region. vram ->gtt, 
gtt->vram, even it may be swaped out under OOM case.


So, my question is how driver prevents kernel ttm from moving the 
user mapped bo while user is accessing it?


Thanks for your attention!

Lizhi.

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


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


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


[PATCH] drm/amdgpu: add DMUB outbox event IRQ source define/complete/debug flag

2021-04-06 Thread Jude Shih
[Why & How]
We use outbox interrupt that allows us to do the AUX via DMUB
Therefore, we need to add some irq source related definition
in the header files;
Also, I added debug flag that allows us to turn it on/off
for testing purpose.
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h   | 2 ++
 drivers/gpu/drm/amd/include/amd_shared.h  | 3 ++-
 drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h | 2 ++
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 963ecfd84347..7e64fc5e0dcd 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -923,6 +923,7 @@ struct amdgpu_device {
struct amdgpu_irq_src   pageflip_irq;
struct amdgpu_irq_src   hpd_irq;
struct amdgpu_irq_src   dmub_trace_irq;
+   struct amdgpu_irq_src   dmub_outbox_irq;
 
/* rings */
u64 fence_context;
@@ -1077,6 +1078,7 @@ struct amdgpu_device {
 
boolin_pci_err_recovery;
struct pci_saved_state  *pci_state;
+   struct completion dmub_aux_transfer_done;
 };
 
 static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev)
diff --git a/drivers/gpu/drm/amd/include/amd_shared.h 
b/drivers/gpu/drm/amd/include/amd_shared.h
index 43ed6291b2b8..097672cc78a1 100644
--- a/drivers/gpu/drm/amd/include/amd_shared.h
+++ b/drivers/gpu/drm/amd/include/amd_shared.h
@@ -227,7 +227,8 @@ enum DC_DEBUG_MASK {
DC_DISABLE_PIPE_SPLIT = 0x1,
DC_DISABLE_STUTTER = 0x2,
DC_DISABLE_DSC = 0x4,
-   DC_DISABLE_CLOCK_GATING = 0x8
+   DC_DISABLE_CLOCK_GATING = 0x8,
+   DC_ENABLE_DMUB_AUX = 0x10,
 };
 
 enum amd_dpm_forced_level;
diff --git a/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h 
b/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
index e2bffcae273a..754170a86ea4 100644
--- a/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
+++ b/drivers/gpu/drm/amd/include/ivsrcid/dcn/irqsrcs_dcn_1_0.h
@@ -1132,5 +1132,7 @@
 
 #define DCN_1_0__SRCID__DMCUB_OUTBOX_HIGH_PRIORITY_READY_INT   0x68
 #define DCN_1_0__CTXID__DMCUB_OUTBOX_HIGH_PRIORITY_READY_INT   6
+#define DCN_1_0__SRCID__DMCUB_OUTBOX_LOW_PRIORITY_READY_INT0x68 // 
DMCUB_IHC_outbox1_ready_int IHC_DMCUB_outbox1_ready_int_ack 
DMCUB_OUTBOX_LOW_PRIORITY_READY_INTERRUPT DISP_INTERRUPT_STATUS_CONTINUE24 
Level/Pulse
+#define DCN_1_0__CTXID__DMCUB_OUTBOX_LOW_PRIORITY_READY_INT8
 
 #endif // __IRQSRCS_DCN_1_0_H__
-- 
2.25.1

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