[PATCH xf86-video-amdgpu 1/2] Add m4 directory
From: Michel Dänzer Although normally it only warns about it, under some circumstances, aclocal can error out if this directory doesn't exist. Reported-by: John Lumby (Cherry picked from radeon commit 7b01c10137aba24c8f61dd9b2a19ea257ad24371) Signed-off-by: Michel Dänzer --- .gitignore| 5 - m4/.gitignore | 5 + 2 files changed, 5 insertions(+), 5 deletions(-) create mode 100644 m4/.gitignore diff --git a/.gitignore b/.gitignore index 38da47d63..0397812e7 100644 --- a/.gitignore +++ b/.gitignore @@ -26,12 +26,7 @@ INSTALL install-sh .libs/ libtool -libtool.m4 ltmain.sh -lt~obsolete.m4 -ltoptions.m4 -ltsugar.m4 -ltversion.m4 Makefile Makefile.in mdate-sh diff --git a/m4/.gitignore b/m4/.gitignore new file mode 100644 index 0..464ba5caa --- /dev/null +++ b/m4/.gitignore @@ -0,0 +1,5 @@ +libtool.m4 +lt~obsolete.m4 +ltoptions.m4 +ltsugar.m4 +ltversion.m4 -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu 2/2] Use AC_CONFIG_MACRO_DIR instead of AC_CONFIG_MACRO_DIRS
From: Michel Dänzer Older versions of autoconf only supported the former. (Cherry picked from radeon commit cba8fe4d64819aaa8ba516aa68dbe6d2aa153046) Signed-off-by: Michel Dänzer --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 1ab5a7ae2..8671cdf9c 100644 --- a/configure.ac +++ b/configure.ac @@ -29,7 +29,7 @@ AC_INIT([xf86-video-amdgpu], AC_CONFIG_SRCDIR([Makefile.am]) AC_CONFIG_HEADERS([config.h]) -AC_CONFIG_MACRO_DIRS([m4]) +AC_CONFIG_MACRO_DIR([m4]) AC_CONFIG_AUX_DIR(.) -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v5 5/5] drm/amdgpu: move PD/PT bos on LRU again
Hi Ray, On 2018-08-22 9:52 a.m., Huang Rui wrote: > The new bulk moving functionality is ready, the overhead of moving PD/PT bos > to > LRU is fixed. So move them on LRU again. > > Signed-off-by: Huang Rui > Tested-by: Mike Lothian > Tested-by: Dieter Nützel > Acked-by: Chunming Zhou > Reviewed-by: Junwei Zhang > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index db1f28a..d195a3d 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -1107,7 +1107,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device > *adev, > struct amdgpu_vm_bo_base, > vm_status); > bo_base->moved = false; > - list_del_init(_base->vm_status); > + list_move(_base->vm_status, >idle); > > bo = bo_base->bo->parent; > if (!bo) > Since this change, I'm getting various badness when running piglit using radeonsi on Bonaire, see the attached dmesg excerpt. Reverting just this change on top of current amd-staging-drm-next avoids the problem. Looks like some list manipulation isn't sufficiently protected against concurrent execution? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer Aug 27 17:34:59 kaveri kernel: [ 567.429026] WARNING: CPU: 7 PID: 12214 at drivers/gpu/drm//ttm/ttm_bo.c:228 ttm_bo_move_to_lru_tail+0x28b/0x3d0 [ttm] Aug 27 17:34:59 kaveri kernel: [ 567.429029] Modules linked in: fuse(E) lz4(E) lz4_compress(E) amdkfd(OE) amdgpu(OE) cpufreq_powersave(E) cpufreq_userspace(E) cpufreq_conservative(E) chash(OE) gpu_sched(OE) binfmt_misc(E) nls_ascii(E) nls_cp437(E) vfat(E) edac_mce_amd(E) fat(E) kvm(E) irqbypass(E) crct10dif_pclmul(E) crc32_pclmul(E) ghash_clmulni_intel(E) pcbc(E) radeon(OE) snd_hda_codec_realtek(E) snd_hda_codec_generic(E) snd_hda_codec_hdmi(E) ttm(OE) snd_hda_intel(E) wmi_bmof(E) aesni_intel(E) efi_pstore(E) aes_x86_64(E) crypto_simd(E) drm_kms_helper(OE) cryptd(E) glue_helper(E) pcspkr(E) efivars(E) snd_hda_codec(E) k10temp(E) drm(OE) snd_hda_core(E) snd_hwdep(E) snd_pcm(E) snd_timer(E) i2c_algo_bit(E) fb_sys_fops(E) r8169(E) sp5100_tco(E) syscopyarea(E) snd(E) sysfillrect(E) ccp(E) sg(E) mii(E) sysimgblt(E) soundcore(E) i2c_piix4(E) Aug 27 17:34:59 kaveri kernel: [ 567.429118] rng_core(E) wmi(E) button(E) acpi_cpufreq(E) tcp_bbr(E) sch_fq(E) sunrpc(E) nct6775(E) hwmon_vid(E) efivarfs(E) ip_tables(E) x_tables(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) fscrypto(E) dm_mod(E) raid10(E) raid1(E) raid0(E) multipath(E) linear(E) md_mod(E) sd_mod(E) evdev(E) hid_generic(E) usbhid(E) hid(E) ahci(E) libahci(E) xhci_pci(E) libata(E) xhci_hcd(E) crc32c_intel(E) scsi_mod(E) usbcore(E) gpio_amdpt(E) gpio_generic(E) Aug 27 17:34:59 kaveri kernel: [ 567.429182] CPU: 7 PID: 12214 Comm: shader_run:cs0 Tainted: GW OE 4.18.0-rc1+ #111 Aug 27 17:34:59 kaveri kernel: [ 567.429184] Hardware name: Micro-Star International Co., Ltd. MS-7A34/B350 TOMAHAWK (MS-7A34), BIOS 1.80 09/13/2017 Aug 27 17:34:59 kaveri kernel: [ 567.429191] RIP: 0010:ttm_bo_move_to_lru_tail+0x28b/0x3d0 [ttm] Aug 27 17:34:59 kaveri kernel: [ 567.429192] Code: c1 ea 03 80 3c 02 00 0f 85 e6 00 00 00 48 8b 83 e8 01 00 00 be ff ff ff ff 48 8d 78 60 e8 1d 08 1e c3 85 c0 0f 85 c3 fd ff ff <0f> 0b e9 bc fd ff ff 48 8d bb d0 01 00 00 48 b8 00 00 00 00 00 fc Aug 27 17:34:59 kaveri kernel: [ 567.429285] RSP: 0018:8803e6aa76a8 EFLAGS: 00010246 Aug 27 17:34:59 kaveri kernel: [ 567.429290] RAX: RBX: 8803d7f4aad0 RCX: 11007afe95f1 Aug 27 17:34:59 kaveri kernel: [ 567.429292] RDX: RSI: 8803c61b57a0 RDI: 0246 Aug 27 17:34:59 kaveri kernel: [ 567.429295] RBP: 8803d4c58638 R08: ed007cd54ebe R09: ed007cd54ebe Aug 27 17:34:59 kaveri kernel: [ 567.429297] R10: 0001 R11: ed007cd54ebe R12: 8803d4c58078 Aug 27 17:34:59 kaveri kernel: [ 567.429299] R13: 8803d4c58000 R14: 8803d7f4aa80 R15: dc00 Aug 27 17:34:59 kaveri kernel: [ 567.429301] FS: 7ff030b2d700() GS:8803ee1c() knlGS: Aug 27 17:34:59 kaveri kernel: [ 567.429304] CS: 0010 DS: ES: CR0: 80050033 Aug 27 17:34:59 kaveri kernel: [ 567.429306] CR2: 7ff00cee6a90 CR3: 0003471e6000 CR4: 003406e0 Aug 27 17:34:59 kaveri kernel: [ 567.429307] Call Trace: Aug 27 17:34:59 kaveri kernel: [ 567.429363] amdgpu_vm_move_to_lru_tail+0x128/0x240 [amdgpu] Aug 27 17:34:59 kaveri kernel: [ 567.429420] amdgpu_cs_ioctl+0x967/0x4ba0 [amdgpu] Aug 27
[PATCH] drm/amdgpu: Only retrieve GPU address of GART table after pinning it
From: Michel Dänzer Doing it earlier hits a WARN_ON_ONCE in amdgpu_bo_gpu_offset. Fixes: "drm/amdgpu: remove gart.table_addr" Signed-off-by: Michel Dänzer --- drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c | 5 - drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c | 5 - drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c | 5 - 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c index 543287e5d67b..9c45ea318bd6 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v6_0.c @@ -494,7 +494,7 @@ static void gmc_v6_0_set_prt(struct amdgpu_device *adev, bool enable) static int gmc_v6_0_gart_enable(struct amdgpu_device *adev) { - uint64_t table_addr = amdgpu_bo_gpu_offset(adev->gart.bo); + uint64_t table_addr; int r, i; u32 field; @@ -505,6 +505,9 @@ static int gmc_v6_0_gart_enable(struct amdgpu_device *adev) r = amdgpu_gart_table_vram_pin(adev); if (r) return r; + + table_addr = amdgpu_bo_gpu_offset(adev->gart.bo); + /* Setup TLB control */ WREG32(mmMC_VM_MX_L1_TLB_CNTL, (0xA << 7) | diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c index 060c79afef80..fc5fe187b614 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v7_0.c @@ -604,7 +604,7 @@ static void gmc_v7_0_set_prt(struct amdgpu_device *adev, bool enable) */ static int gmc_v7_0_gart_enable(struct amdgpu_device *adev) { - uint64_t table_addr = amdgpu_bo_gpu_offset(adev->gart.bo); + uint64_t table_addr; int r, i; u32 tmp, field; @@ -615,6 +615,9 @@ static int gmc_v7_0_gart_enable(struct amdgpu_device *adev) r = amdgpu_gart_table_vram_pin(adev); if (r) return r; + + table_addr = amdgpu_bo_gpu_offset(adev->gart.bo); + /* Setup TLB control */ tmp = RREG32(mmMC_VM_MX_L1_TLB_CNTL); tmp = REG_SET_FIELD(tmp, MC_VM_MX_L1_TLB_CNTL, ENABLE_L1_TLB, 1); diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c index 3fe9b9755cf7..91216cdf4d1c 100644 --- a/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v8_0.c @@ -806,7 +806,7 @@ static void gmc_v8_0_set_prt(struct amdgpu_device *adev, bool enable) */ static int gmc_v8_0_gart_enable(struct amdgpu_device *adev) { - uint64_t table_addr = amdgpu_bo_gpu_offset(adev->gart.bo); + uint64_t table_addr; int r, i; u32 tmp, field; @@ -817,6 +817,9 @@ static int gmc_v8_0_gart_enable(struct amdgpu_device *adev) r = amdgpu_gart_table_vram_pin(adev); if (r) return r; + + table_addr = amdgpu_bo_gpu_offset(adev->gart.bo); + /* Setup TLB control */ tmp = RREG32(mmMC_VM_MX_L1_TLB_CNTL); tmp = REG_SET_FIELD(tmp, MC_VM_MX_L1_TLB_CNTL, ENABLE_L1_TLB, 1); -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: remove amdgpu_bo_gpu_accessible
On 2018-08-28 2:20 p.m., Christian König wrote: > Not used any more. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.h | 13 - > 1 file changed, 13 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h > index 18945dd6982d..907fdf46d895 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h > @@ -193,19 +193,6 @@ static inline u64 amdgpu_bo_mmap_offset(struct amdgpu_bo > *bo) > return drm_vma_node_offset_addr(>tbo.vma_node); > } > > -/** > - * amdgpu_bo_gpu_accessible - return whether the bo is currently in memory > that > - * is accessible to the GPU. > - */ > -static inline bool amdgpu_bo_gpu_accessible(struct amdgpu_bo *bo) > -{ > - switch (bo->tbo.mem.mem_type) { > - case TTM_PL_TT: return amdgpu_gtt_mgr_has_gart_addr(>tbo.mem); > - case TTM_PL_VRAM: return true; > - default: return false; > - } > -} > - > /** > * amdgpu_bo_in_cpu_visible_vram - check if BO is (partly) in visible VRAM > */ > Reviewed-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v5 5/5] drm/amdgpu: move PD/PT bos on LRU again
On 2018-08-28 11:14 a.m., Michel Dänzer wrote: > > Hi Ray, > > > On 2018-08-22 9:52 a.m., Huang Rui wrote: >> The new bulk moving functionality is ready, the overhead of moving PD/PT bos >> to >> LRU is fixed. So move them on LRU again. >> >> Signed-off-by: Huang Rui >> Tested-by: Mike Lothian >> Tested-by: Dieter Nützel >> Acked-by: Chunming Zhou >> Reviewed-by: Junwei Zhang >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> index db1f28a..d195a3d 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >> @@ -1107,7 +1107,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device >> *adev, >> struct amdgpu_vm_bo_base, >> vm_status); >> bo_base->moved = false; >> -list_del_init(_base->vm_status); >> +list_move(_base->vm_status, >idle); >> >> bo = bo_base->bo->parent; >> if (!bo) >> > > Since this change, I'm getting various badness when running piglit using > radeonsi on Bonaire, see the attached dmesg excerpt. > > Reverting just this change on top of current amd-staging-drm-next avoids > the problem. > > Looks like some list manipulation isn't sufficiently protected against > concurrent execution? KASAN pointed me to one issue: https://patchwork.freedesktop.org/patch/246212/ However, this doesn't fully fix the problem. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v5 5/5] drm/amdgpu: move PD/PT bos on LRU again
On 2018-08-29 10:57 a.m., Christian König wrote: > Am 29.08.2018 um 09:52 schrieb Michel Dänzer: >> On 2018-08-28 7:03 p.m., Michel Dänzer wrote: >>> On 2018-08-28 11:14 a.m., Michel Dänzer wrote: >>>> On 2018-08-22 9:52 a.m., Huang Rui wrote: >>>>> The new bulk moving functionality is ready, the overhead of moving >>>>> PD/PT bos to >>>>> LRU is fixed. So move them on LRU again. >>>>> >>>>> Signed-off-by: Huang Rui >>>>> Tested-by: Mike Lothian >>>>> Tested-by: Dieter Nützel >>>>> Acked-by: Chunming Zhou >>>>> Reviewed-by: Junwei Zhang >>>>> --- >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> index db1f28a..d195a3d 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> @@ -1107,7 +1107,7 @@ int amdgpu_vm_update_directories(struct >>>>> amdgpu_device *adev, >>>>> struct amdgpu_vm_bo_base, >>>>> vm_status); >>>>> bo_base->moved = false; >>>>> - list_del_init(_base->vm_status); >>>>> + list_move(_base->vm_status, >idle); >>>>> bo = bo_base->bo->parent; >>>>> if (!bo) >>>>> >>>> Since this change, I'm getting various badness when running piglit >>>> using >>>> radeonsi on Bonaire, see the attached dmesg excerpt. >>>> >>>> Reverting just this change on top of current amd-staging-drm-next >>>> avoids >>>> the problem. >>>> >>>> Looks like some list manipulation isn't sufficiently protected against >>>> concurrent execution? >>> KASAN pointed me to one issue: >>> https://patchwork.freedesktop.org/patch/246212/ >>> >>> However, this doesn't fully fix the problem. >> Ray, any ideas yet for solving this? If not, let's revert this change >> for now. > > I've gone over this multiple times now as well, but can't find anything > obvious wrong either. Thanks for looking into it. > If we don't have any more ideas I would say revert it for now and try to > debug it further. Yep. > BTW: Any idea how to force the issue? Not specifically. It happens reliably and pretty quickly for me when running the piglit gpu profile. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: add missing CHIP_HAINAN in amdgpu_ucode_get_load_type
On 2018-08-28 9:19 p.m., Alex Deucher wrote: > This caused a confusing error message, but there is functionally > no problem since the default method is DIRECT. > > Signed-off-by: Alex Deucher > Cc: sta...@vger.kernel.org > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > index b419d6e33b3a..a942fd28dae8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c > @@ -277,6 +277,7 @@ amdgpu_ucode_get_load_type(struct amdgpu_device *adev, > int load_type) > case CHIP_PITCAIRN: > case CHIP_VERDE: > case CHIP_OLAND: > + case CHIP_HAINAN: > return AMDGPU_FW_LOAD_DIRECT; > #endif > #ifdef CONFIG_DRM_AMDGPU_CIK > Reviewed-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v5 5/5] drm/amdgpu: move PD/PT bos on LRU again
On 2018-08-28 7:03 p.m., Michel Dänzer wrote: > On 2018-08-28 11:14 a.m., Michel Dänzer wrote: >> On 2018-08-22 9:52 a.m., Huang Rui wrote: >>> The new bulk moving functionality is ready, the overhead of moving PD/PT >>> bos to >>> LRU is fixed. So move them on LRU again. >>> >>> Signed-off-by: Huang Rui >>> Tested-by: Mike Lothian >>> Tested-by: Dieter Nützel >>> Acked-by: Chunming Zhou >>> Reviewed-by: Junwei Zhang >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> index db1f28a..d195a3d 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>> @@ -1107,7 +1107,7 @@ int amdgpu_vm_update_directories(struct amdgpu_device >>> *adev, >>>struct amdgpu_vm_bo_base, >>>vm_status); >>> bo_base->moved = false; >>> - list_del_init(_base->vm_status); >>> + list_move(_base->vm_status, >idle); >>> >>> bo = bo_base->bo->parent; >>> if (!bo) >>> >> >> Since this change, I'm getting various badness when running piglit using >> radeonsi on Bonaire, see the attached dmesg excerpt. >> >> Reverting just this change on top of current amd-staging-drm-next avoids >> the problem. >> >> Looks like some list manipulation isn't sufficiently protected against >> concurrent execution? > > KASAN pointed me to one issue: > https://patchwork.freedesktop.org/patch/246212/ > > However, this doesn't fully fix the problem. Ray, any ideas yet for solving this? If not, let's revert this change for now. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Regression: Kaveri VCE ring test fails
Hi Rex, I bisected a regression to your "drm/amd/pp: Unify powergate_uvd/vce/mmhub to set_powergating_by_smu" change. Since this change, the VCE ring test fails on this Kaveri laptop, which prevents the amdgpu driver from initializing at all, see the the attached kern.log. If I disable VCE via the ip_block_mask module parameter, I get an oops in kv_dpm_get_sclk called from amdkfd, see the attached no-vce.log. Since the change above landed for 4.19-rc1, the regression should be fixed before the final 4.19 release if at all possible. Note that before this change, the VCE IB test has always (as far as I remember) failed on this machine, see the attached vce-ib-fail.log. But I don't care about that too much, as I don't need VCE, and I haven't noticed any other issues related to that. P.S. Somebody else bisected a Mullins issue to the same commit: https://bugs.freedesktop.org/show_bug.cgi?id=107595 -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer Aug 21 10:20:37 thor kernel: [4.429726] [drm] amdgpu kernel modesetting enabled. Aug 21 10:20:37 thor kernel: [4.434379] thermal LNXTHERM:04: registered as thermal_zone4 Aug 21 10:20:37 thor kernel: [4.435586] ACPI: Thermal Zone [BATZ] (27 C) Aug 21 10:20:37 thor kernel: [4.438045] ohci-pci: OHCI PCI platform driver Aug 21 10:20:37 thor kernel: [4.438785] Parsing CRAT table with 1 nodes Aug 21 10:20:37 thor kernel: [4.440120] ohci-pci :00:12.0: OHCI PCI host controller Aug 21 10:20:37 thor kernel: [4.440522] Creating topology SYSFS entries Aug 21 10:20:37 thor kernel: [4.441676] ohci-pci :00:12.0: new USB bus registered, assigned bus number 5 Aug 21 10:20:37 thor kernel: [4.442936] Topology: Add APU node [0x0:0x0] Aug 21 10:20:37 thor kernel: [4.444233] ohci-pci :00:12.0: irq 18, io mem 0xd684e000 Aug 21 10:20:37 thor kernel: [4.445142] Finished initializing topology Aug 21 10:20:37 thor kernel: [4.445268] kfd kfd: Initialized module Aug 21 10:20:37 thor kernel: [4.449415] checking generic (c000 30) vs hw (c000 1000) Aug 21 10:20:37 thor kernel: [4.449420] fb: switching to amdgpudrmfb from EFI VGA Aug 21 10:20:37 thor kernel: [4.450767] Console: switching to colour dummy device 80x25 Aug 21 10:20:37 thor kernel: [4.455227] [drm] initializing kernel modesetting (KAVERI 0x1002:0x130A 0x103C:0x2234 0x00). Aug 21 10:20:37 thor kernel: [4.455359] [drm] register mmio base: 0xD680 Aug 21 10:20:37 thor kernel: [4.455380] [drm] register mmio size: 262144 Aug 21 10:20:37 thor kernel: [4.455415] [drm] add ip block number 0 Aug 21 10:20:37 thor kernel: [4.455437] [drm] add ip block number 1 Aug 21 10:20:37 thor kernel: [4.455458] [drm] add ip block number 2 Aug 21 10:20:37 thor kernel: [4.455479] [drm] add ip block number 3 Aug 21 10:20:37 thor kernel: [4.455499] [drm] add ip block number 4 Aug 21 10:20:37 thor kernel: [4.455521] [drm] add ip block number 5 Aug 21 10:20:37 thor kernel: [4.455542] [drm] add ip block number 6 Aug 21 10:20:37 thor kernel: [4.455562] [drm] add ip block number 7 Aug 21 10:20:37 thor kernel: [4.455582] [drm] add ip block number 8 Aug 21 10:20:37 thor kernel: [4.480652] [drm] BIOS signature incorrect 0 0 Aug 21 10:20:37 thor kernel: [4.480716] resource sanity check: requesting [mem 0x000c-0x000d], which spans more than PCI Bus :00 [mem 0x000c-0x000c3fff window] Aug 21 10:20:37 thor kernel: [4.480825] caller pci_map_rom+0x58/0xe0 mapping multiple BARs Aug 21 10:20:37 thor kernel: [4.482188] ATOM BIOS: BR45464.001 Aug 21 10:20:37 thor kernel: [4.483717] [drm] vm size is 64 GB, 2 levels, block size is 10-bit, fragment size is 9-bit Aug 21 10:20:37 thor kernel: [4.483764] amdgpu :00:01.0: VRAM: 1024M 0x00F4 - 0x00F43FFF (1024M used) Aug 21 10:20:37 thor kernel: [4.483804] amdgpu :00:01.0: GART: 1024M 0x - 0x3FFF Aug 21 10:20:37 thor kernel: [4.483850] [drm] Detected VRAM RAM=1024M, BAR=1024M Aug 21 10:20:37 thor kernel: [4.483875] [drm] RAM width 128bits UNKNOWN Aug 21 10:20:37 thor kernel: [4.484472] [TTM] Zone kernel: Available graphics memory: 3568746 kiB Aug 21 10:20:37 thor kernel: [4.484537] [TTM] Zone dma32: Available graphics memory: 2097152 kiB Aug 21 10:20:37 thor kernel: [4.484563] [TTM] Initializing pool allocator Aug 21 10:20:37 thor kernel: [4.484620] [TTM] Initializing DMA pool allocator Aug 21 10:20:37 thor kernel: [4.485188] [drm] amdgpu: 1024M of VRAM memory ready Aug 21 10:20:37 thor kernel: [4.485217] [drm] amdgpu: 3072M of GTT memory ready. Aug 21 10:20:37 thor kernel: [4.485316] [drm] GART: num cpu pages 262144, num gpu pages 262144 Aug 21 10:20:37 thor kernel: [4.486477] [drm] PCIE GART of 1024M enabled (table at 0x00F40030
Re: [PATCH 2/3] drm/amdgpu: Power up uvd block when hw_fini
On 2018-08-23 11:24 a.m., Rex Zhu wrote: > when hw_fini/suspend, smu only need to power up uvd block > if uvd pg is supported, don't need to call vce to do hw_init. Do you really mean VCE here, not UVD? > diff --git a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c > b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c > index a713c8b..8f625d6 100644 > --- a/drivers/gpu/drm/amd/amdgpu/kv_dpm.c > +++ b/drivers/gpu/drm/amd/amdgpu/kv_dpm.c > @@ -65,7 +65,6 @@ static int kv_set_thermal_temperature_range(struct > amdgpu_device *adev, > int min_temp, int max_temp); > static int kv_init_fps_limits(struct amdgpu_device *adev); > > -static void kv_dpm_powergate_uvd(void *handle, bool gate); > static void kv_dpm_powergate_samu(struct amdgpu_device *adev, bool gate); > static void kv_dpm_powergate_acp(struct amdgpu_device *adev, bool gate); > > @@ -1390,7 +1389,8 @@ static void kv_dpm_disable(struct amdgpu_device *adev) > kv_dpm_powergate_samu(adev, false); > if (pi->caps_vce_pg) /* power on the VCE block */ > amdgpu_kv_notify_message_to_smu(adev, PPSMC_MSG_VCEPowerON); > - kv_dpm_powergate_uvd(adev, false); > + if (pi->caps_uvd_pg) /* power off the UVD block */ > + amdgpu_kv_notify_message_to_smu(adev, PPSMC_MSG_UVDPowerON); The comment should say "power on", shouldn't it? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/3] drm/amdgpu: Fix vce initialize failed on Kaveri/Mullins
On 2018-08-23 11:24 a.m., Rex Zhu wrote: > Forgot to add vce pg support via smu for Kaveri/Mullins. > > Regresstion issue caused by > 'commit 561a5c83eadd ("drm/amd/pp: Unify powergate_uvd/vce/mmhub > to set_powergating_by_smu")' You can replace this paragraph with Fixes: 561a5c83eadd ("drm/amd/pp: Unify powergate_uvd/vce/mmhub to set_powergating_by_smu") This patch fixes the VCE ring (and also IB) test on this laptop, thanks! Unfortunately though, there's still an oops if I let the amdkfd driver load together with amdgpu (no issue when loading amdkfd manually later), see the attached kernel.log excerpt. This is also a regression in the 4.19 drm tree changes. It might be a separate issue, but TBH I don't feel like another day or two bisecting right now. :) Anyway, this series is Tested-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer Aug 23 12:25:30 thor kernel: [ 200.456163] [drm] amdgpu kernel modesetting enabled. Aug 23 12:25:30 thor kernel: [ 200.465731] Parsing CRAT table with 1 nodes Aug 23 12:25:30 thor kernel: [ 200.465741] Creating topology SYSFS entries Aug 23 12:25:30 thor kernel: [ 200.465786] Topology: Add APU node [0x0:0x0] Aug 23 12:25:30 thor kernel: [ 200.465789] Finished initializing topology Aug 23 12:25:30 thor kernel: [ 200.465853] kfd kfd: Initialized module Aug 23 12:25:30 thor kernel: [ 200.466288] checking generic (c000 30) vs hw (c000 1000) Aug 23 12:25:30 thor kernel: [ 200.466296] fb: switching to amdgpudrmfb from EFI VGA Aug 23 12:25:30 thor kernel: [ 200.466418] Console: switching to colour dummy device 80x25 Aug 23 12:25:30 thor kernel: [ 200.467646] [drm] initializing kernel modesetting (KAVERI 0x1002:0x130A 0x103C:0x2234 0x00). Aug 23 12:25:30 thor kernel: [ 200.468031] [drm] register mmio base: 0xD680 Aug 23 12:25:30 thor kernel: [ 200.468035] [drm] register mmio size: 262144 Aug 23 12:25:30 thor kernel: [ 200.468058] [drm] add ip block number 0 Aug 23 12:25:30 thor kernel: [ 200.468062] [drm] add ip block number 1 Aug 23 12:25:30 thor kernel: [ 200.468064] [drm] add ip block number 2 Aug 23 12:25:30 thor kernel: [ 200.468067] [drm] add ip block number 3 Aug 23 12:25:30 thor kernel: [ 200.468071] [drm] add ip block number 4 Aug 23 12:25:30 thor kernel: [ 200.468074] [drm] add ip block number 5 Aug 23 12:25:30 thor kernel: [ 200.468077] [drm] add ip block number 6 Aug 23 12:25:30 thor kernel: [ 200.468080] [drm] add ip block number 7 Aug 23 12:25:30 thor kernel: [ 200.468082] [drm] add ip block number 8 Aug 23 12:25:30 thor kernel: [ 200.501755] [drm] BIOS signature incorrect 0 0 Aug 23 12:25:30 thor kernel: [ 200.501804] resource sanity check: requesting [mem 0x000c-0x000d], which spans more than PCI Bus :00 [mem 0x000c-0x000c3fff window] Aug 23 12:25:30 thor kernel: [ 200.501812] caller pci_map_rom+0x58/0xe0 mapping multiple BARs Aug 23 12:25:30 thor kernel: [ 200.503187] ATOM BIOS: BR45464.001 Aug 23 12:25:30 thor kernel: [ 200.503219] [drm] GPU posting now... Aug 23 12:25:31 thor kernel: [ 200.966309] [drm] vm size is 64 GB, 2 levels, block size is 10-bit, fragment size is 9-bit Aug 23 12:25:31 thor kernel: [ 200.966329] amdgpu :00:01.0: VRAM: 1024M 0x00F4 - 0x00F43FFF (1024M used) Aug 23 12:25:31 thor kernel: [ 200.966333] amdgpu :00:01.0: GART: 1024M 0x - 0x3FFF Aug 23 12:25:31 thor kernel: [ 200.966352] [drm] Detected VRAM RAM=1024M, BAR=1024M Aug 23 12:25:31 thor kernel: [ 200.966354] [drm] RAM width 128bits UNKNOWN Aug 23 12:25:31 thor kernel: [ 200.966695] [TTM] Zone kernel: Available graphics memory: 3568742 kiB Aug 23 12:25:31 thor kernel: [ 200.966702] [TTM] Zone dma32: Available graphics memory: 2097152 kiB Aug 23 12:25:31 thor kernel: [ 200.966705] [TTM] Initializing pool allocator Aug 23 12:25:31 thor kernel: [ 200.966714] [TTM] Initializing DMA pool allocator Aug 23 12:25:31 thor kernel: [ 200.966799] [drm] amdgpu: 1024M of VRAM memory ready Aug 23 12:25:31 thor kernel: [ 200.966803] [drm] amdgpu: 3072M of GTT memory ready. Aug 23 12:25:31 thor kernel: [ 200.966842] [drm] GART: num cpu pages 262144, num gpu pages 262144 Aug 23 12:25:31 thor kernel: [ 200.967622] [drm] PCIE GART of 1024M enabled (table at 0x00F4). Aug 23 12:25:31 thor kernel: [ 200.967771] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). Aug 23 12:25:31 thor kernel: [ 200.967774] [drm] Driver supports precise vblank timestamp query. Aug 23 12:25:31 thor kernel: [ 200.967803] [drm] Internal thermal controller without fan control Aug 23 12:25:31 thor kernel: [ 200.967806] [drm] amdgpu: dpm initialized Aug 23 12:25:31 thor kernel: [ 200.969641] [drm] amdgpu atom DIG backlight initialized Aug 23 12:25:31 thor kernel: [ 200.969644] [drm] AMD
Re: [PATCH] drm/amdgpu: lock and unlock console only for amdgpu_fbdev_set_suspend [V2]
On 2018-07-20 09:58 AM, S, Shirish wrote: > Sure Michel, but not immediately. > Is that fine? Sure, thanks. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: lock and unlock console only for amdgpu_fbdev_set_suspend [V2]
On 2018-07-19 11:20 AM, Michel Dänzer wrote: > > Possible follow-up work: > > * Move the console_(un)lock calls into amdgpu_fbdev_set_suspend, or > maybe use drm_fb_helper_set_suspend_unlocked instead of locking ourselves > > * Move the amdgpu_fbdev_set_suspend call in amdgpu_device_suspend > further up, at least before the pci_set_power_state call. Shirish, would you like to look into these? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: use drm_fb helper for console_(un)lock
On 2018-07-20 02:30 PM, Shirish S wrote: > This patch removes the usage of console_(un)lock > by replacing drm_fb_helper_set_suspend() to > drm_fb_helper_set_suspend_unlocked() which locks and > unlocks the console instead of locking ourselves. > > Signed-off-by: Shirish S > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 4 ++-- > 2 files changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 022c136..a759b74 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -2691,11 +2691,9 @@ int amdgpu_device_suspend(struct drm_device *dev, bool > suspend, bool fbcon) > DRM_ERROR("amdgpu asic reset failed\n"); > } > > - if (fbcon) { > - console_lock(); > + if (fbcon) > amdgpu_fbdev_set_suspend(adev, 1); > - console_unlock(); > - } > + > return 0; > } > > @@ -2784,9 +2782,7 @@ int amdgpu_device_resume(struct drm_device *dev, bool > resume, bool fbcon) > } > drm_modeset_unlock_all(dev); > } > - console_lock(); > amdgpu_fbdev_set_suspend(adev, 0); > - console_unlock(); > } > > drm_kms_helper_poll_enable(dev); > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c > index d44b764..69c5d22 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c > @@ -373,8 +373,8 @@ void amdgpu_fbdev_fini(struct amdgpu_device *adev) > void amdgpu_fbdev_set_suspend(struct amdgpu_device *adev, int state) > { > if (adev->mode_info.rfbdev) > - drm_fb_helper_set_suspend(>mode_info.rfbdev->helper, > - state); > + > drm_fb_helper_set_suspend_unlocked(>mode_info.rfbdev->helper, > + state); > } > > int amdgpu_fbdev_total_size(struct amdgpu_device *adev) > Thanks for doing this, looks good. Reviewed-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Fix RLC safe mode test in gfx_v9_0_enter_rlc_safe_mode
From: Michel Dänzer We were testing the register offset, instead of the value stored in the register, therefore always timing out the loop. This reduces suspend time of the system in the bug report below by ~600 ms. Bugzilla: https://bugs.freedesktop.org/107277 Tested-by: Paul Menzel Signed-off-by: Michel Dänzer --- drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 2 +- 1 file changed, 1 insertion(+), 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 9ab39117cc4e..ef00d14f8645 100644 --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c @@ -3490,7 +3490,7 @@ static void gfx_v9_0_enter_rlc_safe_mode(struct amdgpu_device *adev) /* wait for RLC_SAFE_MODE */ for (i = 0; i < adev->usec_timeout; i++) { - if (!REG_GET_FIELD(SOC15_REG_OFFSET(GC, 0, mmRLC_SAFE_MODE), RLC_SAFE_MODE, CMD)) + if (!REG_GET_FIELD(RREG32_SOC15(GC, 0, mmRLC_SAFE_MODE), RLC_SAFE_MODE, CMD)) break; udelay(1); } -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/7] drm/amd/display: Remove unnecessary warning
On 2018-07-17 03:29 PM, sunpeng...@amd.com wrote: > From: Mikita Lipski > > [why] > The warning message floods the dmesg log on Tonga even > though it is expected to have a pix_clk set to zero, > when there is no display connected. > [how] > remove the assert > > Change-Id: I4ca1e42439369b2305694b403457b5de60fc4ab1 > Signed-off-by: Mikita Lipski > Reviewed-by: Harry Wentland > Acked-by: Leo Li > --- > drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c | 4 > 1 file changed, 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c > b/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c > index ec32213..74c05e8 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c > +++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c > @@ -149,10 +149,6 @@ static uint32_t get_max_pixel_clock_for_all_paths( > max_pix_clk = > > pipe_ctx->stream_res.pix_clk_params.requested_pix_clk; > } > - > - if (max_pix_clk == 0) > - ASSERT(0); > - > return max_pix_clk; > } > > On my development system, max_pix_clk == 0 even though there's a display connected via HDMI. Is that expected as well? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/7] drm/amd/display: Remove unnecessary warning
On 2018-07-17 05:10 PM, Harry Wentland wrote: > On 2018-07-17 10:17 AM, Michel Dänzer wrote: >> On 2018-07-17 03:29 PM, sunpeng...@amd.com wrote: >>> From: Mikita Lipski >>> >>> [why] >>> The warning message floods the dmesg log on Tonga even >>> though it is expected to have a pix_clk set to zero, >>> when there is no display connected. >>> [how] >>> remove the assert >>> >>> Change-Id: I4ca1e42439369b2305694b403457b5de60fc4ab1 >>> Signed-off-by: Mikita Lipski >>> Reviewed-by: Harry Wentland >>> Acked-by: Leo Li >>> --- >>> drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c | 4 >>> 1 file changed, 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c >>> b/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c >>> index ec32213..74c05e8 100644 >>> --- a/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c >>> +++ b/drivers/gpu/drm/amd/display/dc/dce100/dce100_hw_sequencer.c >>> @@ -149,10 +149,6 @@ static uint32_t get_max_pixel_clock_for_all_paths( >>> max_pix_clk = >>> >>> pipe_ctx->stream_res.pix_clk_params.requested_pix_clk; >>> } >>> - >>> - if (max_pix_clk == 0) >>> - ASSERT(0); >>> - >>> return max_pix_clk; >>> } >>> >>> >> >> On my development system, max_pix_clk == 0 even though there's a display >> connected via HDMI. Is that expected as well? >> > > It's not really expected. Does it happen on mode set, hotplug, randomly? Seems to happen when the display is turned off, e.g. via DPMS. > Do you have a stack trace? Yep, attached. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer [ 2367.329074] WARNING: CPU: 8 PID: 16019 at drivers/gpu/drm//amd/amdgpu/../display/dc/dce100/dce100_hw_sequencer.c:154 dce100_set_bandwidth+0x154/0x300 [amdgpu] [ 2367.329086] Modules linked in: lz4(E) lz4_compress(E) cpufreq_powersave(E) cpufreq_userspace(E) cpufreq_conservative(E) binfmt_misc(E) nls_ascii(E) nls_cp437(E) vfat(E) edac_mce_amd(E) fat(E) amdkfd(OE) kvm(E) irqbypass(E) crct10dif_pclmul(E) crc32_pclmul(E) ghash_clmulni_intel(E) pcbc(E) radeon(OE) wmi_bmof(E) amdgpu(OE) snd_hda_codec_realtek(E) snd_hda_codec_generic(E) chash(OE) gpu_sched(OE) snd_hda_codec_hdmi(E) ttm(OE) aesni_intel(E) aes_x86_64(E) crypto_simd(E) drm_kms_helper(OE) cryptd(E) snd_hda_intel(E) efi_pstore(E) glue_helper(E) r8169(E) pcspkr(E) snd_hda_codec(E) drm(OE) mii(E) snd_hda_core(E) efivars(E) sg(E) i2c_algo_bit(E) snd_hwdep(E) fb_sys_fops(E) syscopyarea(E) snd_pcm(E) sysfillrect(E) sp5100_tco(E) snd_timer(E) sysimgblt(E) i2c_piix4(E) k10temp(E) wmi(E) ccp(E) snd(E) soundcore(E) [ 2367.329232] rng_core(E) button(E) acpi_cpufreq(E) tcp_bbr(E) sch_fq(E) nct6775(E) hwmon_vid(E) sunrpc(E) efivarfs(E) ip_tables(E) x_tables(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) fscrypto(E) dm_mod(E) raid10(E) raid1(E) raid0(E) multipath(E) linear(E) md_mod(E) sd_mod(E) evdev(E) hid_generic(E) usbhid(E) hid(E) ahci(E) xhci_pci(E) libahci(E) xhci_hcd(E) libata(E) crc32c_intel(E) usbcore(E) scsi_mod(E) gpio_amdpt(E) gpio_generic(E) [ 2367.329337] CPU: 8 PID: 16019 Comm: Xorg Tainted: GW OE 4.18.0-rc1+ #110 [ 2367.329344] Hardware name: Micro-Star International Co., Ltd. MS-7A34/B350 TOMAHAWK (MS-7A34), BIOS 1.80 09/13/2017 [ 2367.329532] RIP: 0010:dce100_set_bandwidth+0x154/0x300 [amdgpu] [ 2367.329538] Code: 84 c0 74 08 3c 03 0f 8e 7d 01 00 00 8b 83 10 01 00 00 41 39 c6 44 0f 42 f0 48 81 c3 30 04 00 00 48 39 d3 75 90 45 85 f6 75 02 <0f> 0b 49 8d 9d 48 01 00 00 48 b8 00 00 00 00 00 fc ff df 44 89 74 [ 2367.329663] RSP: 0018:88028cc2f338 EFLAGS: 00010246 [ 2367.329672] RAX: 11007b1652d0 RBX: 8803d8b29ab0 RCX: [ 2367.329678] RDX: 8803d8b29ab0 RSI: RDI: 8803d8b29c5c [ 2367.329684] RBP: dc00 R08: ed007a55d632 R09: ed007a55d632 [ 2367.329690] R10: 88028cc2f140 R11: ed007a55d631 R12: 110051985e69 [ 2367.329695] R13: 8803c8bee600 R14: R15: 8803d8b28000 [ 2367.329703] FS: 7f3ad605f940() GS:8803ee20() knlGS: [ 2367.329709] CS: 0010 DS: ES: CR0: 80050033 [ 2367.329714] CR2: 55720e2bb560 CR3: 0003c1418000 CR4: 003406e0 [ 2367.329720] Call Trace: [ 2367.329882] ? dce100_pplib_apply_display_requirements+0x1b0/0x1b0 [amdgpu] [ 2367.330041] ? bios_is_accelerated_mode+0xdb/0x140 [amdgpu] [ 2367.330201] dc
[PATCH xf86-video-amdgpu] Support gamma correction & colormaps at depth 30 as well
From: Michel Dänzer Only supported with the advanced colour management properties available with DC as of kernel 4.17. Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 47 +++ 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index f6cafccdc..f5ab7955e 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -,14 +,14 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp) info->drmmode_crtc_funcs.shadow_destroy = NULL; } + drmmode_cm_init(pAMDGPUEnt->fd, drmmode, mode_res); + /* Hw gamma lut's are currently bypassed by the hw at color depth 30, * so spare the server the effort to compute and update the cluts. */ - if (pScrn->depth == 30) + if (pScrn->depth == 30 && !drmmode_cm_enabled(drmmode)) info->drmmode_crtc_funcs.gamma_set = NULL; - drmmode_cm_init(pAMDGPUEnt->fd, drmmode, mode_res); - for (i = 0; i < mode_res->count_crtcs; i++) if (!xf86IsEntityShared(pScrn->entityList[0]) || (crtcs_needed && !(pAMDGPUEnt->assigned_crtcs & (1 << i @@ -3636,29 +3636,50 @@ Bool drmmode_set_desired_modes(ScrnInfoPtr pScrn, drmmode_ptr drmmode, Bool drmmode_setup_colormap(ScreenPtr pScreen, ScrnInfoPtr pScrn) { xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn); + AMDGPUInfoPtr info = AMDGPUPTR(pScrn); int i; if (xf86_config->num_crtc) { xf86DrvMsgVerb(pScrn->scrnIndex, X_INFO, AMDGPU_LOGLEVEL_DEBUG, "Initializing kms color map\n"); + if (!miCreateDefColormap(pScreen)) return FALSE; - /* All radeons support 10 bit CLUTs. They get bypassed at depth 30. */ - if (pScrn->depth != 30) { - if (!xf86HandleColormaps(pScreen, 256, 10, NULL, NULL, -CMAP_PALETTED_TRUECOLOR -| CMAP_RELOAD_ON_MODE_SWITCH)) - return FALSE; + + if (pScrn->depth == 30) { + if (!drmmode_cm_enabled(>drmmode)) + return TRUE; for (i = 0; i < xf86_config->num_crtc; i++) { xf86CrtcPtr crtc = xf86_config->crtc[i]; + void *gamma = malloc(1024 * 3 * sizeof(CARD16)); + + if (!gamma) { + ErrorF("Failed to allocate gamma LUT memory\n"); + return FALSE; + } - drmmode_crtc_gamma_do_set(crtc, crtc->gamma_red, - crtc->gamma_green, - crtc->gamma_blue, - crtc->gamma_size); + crtc->gamma_size = 1024; + crtc->gamma_red = gamma; + crtc->gamma_green = crtc->gamma_red + crtc->gamma_size; + crtc->gamma_blue = crtc->gamma_green + crtc->gamma_size; } } + + /* All Radeons support 10 bit CLUTs. */ + if (!xf86HandleColormaps(pScreen, 1 << pScrn->rgbBits, 10, +NULL, NULL, CMAP_PALETTED_TRUECOLOR +| CMAP_RELOAD_ON_MODE_SWITCH)) + return FALSE; + + for (i = 0; i < xf86_config->num_crtc; i++) { + xf86CrtcPtr crtc = xf86_config->crtc[i]; + + drmmode_crtc_gamma_do_set(crtc, crtc->gamma_red, + crtc->gamma_green, + crtc->gamma_blue, + crtc->gamma_size); + } } return TRUE; -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: reciprocate amdgpu.._resume sequence to match amdgpu.._suspend
On 2018-07-18 11:40 AM, Shirish S wrote: > [Why] > 1. To ensure that resume path reciprocates the sequence followed during > suspend. > 2. While the console_lock is held, console output will be buffered, till > its unlocked it wont be emitted, hence its ideal to unlock sooner to enable > debugging/detecting/fixing of any issue in the remaining sequence of events > in resume path. > > [How] > This patch restructures the console_lock, console_unlock around > amdgpu_fbdev_set_suspend() and moves this new block to the very beginning > of the resume sequence. > > Signed-off-by: Shirish S > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 12 > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index 709e4a3..fc4c517 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -2720,8 +2720,11 @@ int amdgpu_device_resume(struct drm_device *dev, bool > resume, bool fbcon) > if (dev->switch_power_state == DRM_SWITCH_POWER_OFF) > return 0; > > - if (fbcon) > + if (fbcon) { > console_lock(); > + amdgpu_fbdev_set_suspend(adev, 0); > + console_unlock(); > + } > > if (resume) { > pci_set_power_state(dev->pdev, PCI_D0); I don't think the amdgpu_fbdev_set_suspend call can be moved before the pci_set_power_state call, because fbcon may presumably try writing to the device's VRAM at any point after amdgpu_fbdev_set_suspend. There might be other things that need to happen before amdgpu_fbdev_set_suspend. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu] Remove drmmode_terminate_leases
From: Michel Dänzer The RandR screen private is already freed when our CloseScreen runs, so this can't do anything useful. This cleanup has to be done by the X server itself. Signed-off-by: Michel Dänzer --- src/amdgpu_kms.c | 1 - src/drmmode_display.c | 20 src/drmmode_display.h | 2 -- 3 files changed, 23 deletions(-) diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c index 39e047e29..d2ab7ee2b 100644 --- a/src/amdgpu_kms.c +++ b/src/amdgpu_kms.c @@ -1705,7 +1705,6 @@ static Bool AMDGPUCloseScreen_KMS(ScreenPtr pScreen) /* Clear mask of assigned crtc's in this generation */ pAMDGPUEnt->assigned_crtcs = 0; - drmmode_terminate_leases(pScrn); drmmode_uevent_fini(pScrn, >drmmode); amdgpu_drm_queue_close(pScrn); diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 0223e..1aefd199b 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -3063,26 +3063,6 @@ drmmode_terminate_lease(RRLeasePtr lease) #endif // XF86_LEASE_VERSION -void -drmmode_terminate_leases(ScrnInfoPtr pScrn) -{ -#ifdef XF86_LEASE_VERSION - ScreenPtr screen = xf86ScrnToScreen(pScrn); - AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn); - rrScrPrivPtr scr_priv = rrGetScrPriv(screen); - RRLeasePtr lease, next; - - xorg_list_for_each_entry_safe(lease, next, _priv->leases, list) { - drmmode_lease_private_ptr lease_private = lease->devPrivate; - drmModeRevokeLease(pAMDGPUEnt->fd, lease_private->lessee_id); - free(lease_private); - lease->devPrivate = NULL; - RRLeaseTerminated(lease); - RRLeaseFree(lease); - } -#endif -} - static const xf86CrtcConfigFuncsRec drmmode_xf86crtc_config_funcs = { .resize = drmmode_xf86crtc_resize, #ifdef XF86_LEASE_VERSION diff --git a/src/drmmode_display.h b/src/drmmode_display.h index 0646752c2..8b949f79d 100644 --- a/src/drmmode_display.h +++ b/src/drmmode_display.h @@ -245,8 +245,6 @@ PixmapPtr drmmode_crtc_scanout_create(xf86CrtcPtr crtc, extern void drmmode_uevent_init(ScrnInfoPtr scrn, drmmode_ptr drmmode); extern void drmmode_uevent_fini(ScrnInfoPtr scrn, drmmode_ptr drmmode); -extern void drmmode_terminate_leases(ScrnInfoPtr scrn); - Bool drmmode_set_mode(xf86CrtcPtr crtc, struct drmmode_fb *fb, DisplayModePtr mode, int x, int y); -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: new KASAN running piglit
On 2018-07-18 06:17 PM, Michel Dänzer wrote: > On 2018-07-18 06:05 PM, Tom St Denis wrote: >> Hi Christian, >> >> This patch: >> >> [root@raven linux]# git bisect bad >> 90f362bdf0d0d06a126a5fd35b084436dd8250ad is the first bad commit >> commit 90f362bdf0d0d06a126a5fd35b084436dd8250ad >> Author: Christian König >> Date: Mon Jul 16 14:58:48 2018 +0200 >> >> drm/amdgpu: change ring priority after pushing the job >> >> Pushing a job can change the ring assignment of an entity. >> >> Signed-off-by: Christian König >> Reviewed-by: Chunming Zhou >> >> :04 04 9a09d3e9d055e4f5024019861c334ee9cc0bd11b >> 522540c31c3d1a4f4a6fbac75e985e9a7f7e93c9 M drivers >> >> causes a KASAN while running piglit. It's reproduceable 100% of the >> time. The commit before this doesn't not cause a KASAN. > > I got the attached KASAN use-after-free report while running piglit > today, could be the same? Sorry, that was for a different issue. Here's the KASAN report. Tom, what does the file command say about the amdgpu.ko file loaded on your system? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer Jul 18 18:02:46 kaveri kernel: [18273.337881] BUG: KASAN: use-after-free in amdgpu_cs_ioctl+0x48b2/0x4eb0 [amdgpu] Jul 18 18:02:46 kaveri kernel: [18273.337889] Read of size 4 at addr 8801004b7aa4 by task shader_run:cs0/19401 Jul 18 18:02:46 kaveri kernel: [18273.337892] Jul 18 18:02:46 kaveri kernel: [18273.337900] CPU: 9 PID: 19401 Comm: shader_run:cs0 Tainted: GW OE 4.18.0-rc1+ #110 Jul 18 18:02:46 kaveri kernel: [18273.337905] Hardware name: Micro-Star International Co., Ltd. MS-7A34/B350 TOMAHAWK (MS-7A34), BIOS 1.80 09/13/2017 Jul 18 18:02:46 kaveri kernel: [18273.337909] Call Trace: Jul 18 18:02:46 kaveri kernel: [18273.337920] dump_stack+0x9a/0xeb Jul 18 18:02:46 kaveri kernel: [18273.337928] print_address_description+0x6a/0x270 Jul 18 18:02:46 kaveri kernel: [18273.337935] kasan_report+0x258/0x380 Jul 18 18:02:46 kaveri kernel: [18273.338018] ? amdgpu_cs_ioctl+0x48b2/0x4eb0 [amdgpu] Jul 18 18:02:46 kaveri kernel: [18273.338098] amdgpu_cs_ioctl+0x48b2/0x4eb0 [amdgpu] Jul 18 18:02:46 kaveri kernel: [18273.338187] ? amdgpu_bo_list_ioctl+0x2d1/0x3e0 [amdgpu] Jul 18 18:02:46 kaveri kernel: [18273.338257] ? amdgpu_cs_find_mapping+0x3c0/0x3c0 [amdgpu] Jul 18 18:02:46 kaveri kernel: [18273.338263] ? do_vfs_ioctl+0x192/0xf30 Jul 18 18:02:46 kaveri kernel: [18273.338267] ? ksys_ioctl+0x70/0x80 Jul 18 18:02:46 kaveri kernel: [18273.338272] ? __x64_sys_ioctl+0x6f/0xb0 Jul 18 18:02:46 kaveri kernel: [18273.338278] ? do_syscall_64+0xa5/0x3f0 Jul 18 18:02:46 kaveri kernel: [18273.338283] ? entry_SYSCALL_64_after_hwframe+0x49/0xbe Jul 18 18:02:46 kaveri kernel: [18273.338290] ? idr_get_free+0x4bc/0x980 Jul 18 18:02:46 kaveri kernel: [18273.338297] ? __radix_tree_replace+0xa7/0x160 Jul 18 18:02:46 kaveri kernel: [18273.338325] ? drm_dev_enter+0x5/0xe0 [drm] Jul 18 18:02:46 kaveri kernel: [18273.338332] ? debug_check_no_locks_freed+0x2c0/0x2c0 Jul 18 18:02:46 kaveri kernel: [18273.338337] ? __fprop_inc_percpu_max+0x1d0/0x1d0 Jul 18 18:02:46 kaveri kernel: [18273.338438] ? amdgpu_cs_find_mapping+0x3c0/0x3c0 [amdgpu] Jul 18 18:02:46 kaveri kernel: [18273.338463] drm_ioctl_kernel+0x197/0x220 [drm] Jul 18 18:02:46 kaveri kernel: [18273.338484] ? drm_setversion+0x7d0/0x7d0 [drm] Jul 18 18:02:46 kaveri kernel: [18273.338493] ? __check_object_size+0x149/0x360 Jul 18 18:02:46 kaveri kernel: [18273.338515] drm_ioctl+0x60a/0x970 [drm] Jul 18 18:02:46 kaveri kernel: [18273.338597] ? amdgpu_cs_find_mapping+0x3c0/0x3c0 [amdgpu] Jul 18 18:02:46 kaveri kernel: [18273.338623] ? drm_ioctl_kernel+0x220/0x220 [drm] Jul 18 18:02:46 kaveri kernel: [18273.338633] ? lock_downgrade+0x5e0/0x5e0 Jul 18 18:02:46 kaveri kernel: [18273.338638] ? __pm_runtime_resume+0x79/0x100 Jul 18 18:02:46 kaveri kernel: [18273.338644] ? debug_check_no_locks_freed+0x2c0/0x2c0 Jul 18 18:02:46 kaveri kernel: [18273.338650] ? do_raw_spin_unlock+0x54/0x220 Jul 18 18:02:46 kaveri kernel: [18273.338731] amdgpu_drm_ioctl+0xcc/0x1a0 [amdgpu] Jul 18 18:02:46 kaveri kernel: [18273.338739] do_vfs_ioctl+0x192/0xf30 Jul 18 18:02:46 kaveri kernel: [18273.338744] ? lock_acquire+0x10b/0x330 Jul 18 18:02:46 kaveri kernel: [18273.338749] ? finish_task_switch+0xf1/0x670 Jul 18 18:02:46 kaveri kernel: [18273.338756] ? ioctl_preallocate+0x1b0/0x1b0 Jul 18 18:02:46 kaveri kernel: [18273.338762] ? __fget+0x1c8/0x300 Jul 18 18:02:46 kaveri kernel: [18273.338768] ? lock_downgrade+0x5e0/0x5e0 Jul 18 18:02:46 kaveri kernel: [18273.338771] ? __fget+0x49/0x300 Jul 18 18:02:46 kaveri kernel: [18273.338780] ? __fget+0x1e0/0x300 Jul 18 18:02:46 kaveri kernel: [18273.338790] ksys_ioctl+0x70
Re: new KASAN running piglit
On 2018-07-18 06:46 PM, Christian König wrote: > Am 18.07.2018 um 18:44 schrieb Michel Dänzer: >> On 2018-07-18 06:17 PM, Michel Dänzer wrote: >>> On 2018-07-18 06:05 PM, Tom St Denis wrote: >>>> Hi Christian, >>>> >>>> This patch: >>>> >>>> [root@raven linux]# git bisect bad >>>> 90f362bdf0d0d06a126a5fd35b084436dd8250ad is the first bad commit >>>> commit 90f362bdf0d0d06a126a5fd35b084436dd8250ad >>>> Author: Christian König >>>> Date: Mon Jul 16 14:58:48 2018 +0200 >>>> >>>> drm/amdgpu: change ring priority after pushing the job >>>> >>>> Pushing a job can change the ring assignment of an entity. >>>> >>>> Signed-off-by: Christian König >>>> Reviewed-by: Chunming Zhou >>>> >>>> :04 04 9a09d3e9d055e4f5024019861c334ee9cc0bd11b >>>> 522540c31c3d1a4f4a6fbac75e985e9a7f7e93c9 M drivers >>>> >>>> causes a KASAN while running piglit. It's reproduceable 100% of the >>>> time. The commit before this doesn't not cause a KASAN. >>> I got the attached KASAN use-after-free report while running piglit >>> today, could be the same? >> Sorry, that was for a different issue. Here's the KASAN report. > > No, problem. I've already know what's going wrong here. > > Accessing job after it is pushed to the scheduler is a bad idea, need to > cache the priority field locally. > > Give me an hour to get the kids into bed and I can fix it, Thanks, but no rush from my side, I won't be able to test it before tomorrow anyway (and it doesn't seem to cause any problems other than the KASAN report). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm] amdgpu: add amdgpu_bo_handle_type_kms_noimport
On 2018-07-17 08:14 PM, Marek Olšák wrote: > Michel, I think you are wasting your time. This change can be misused > as easily as any other API. It's not more dangerous that any other > amdgpu libdrm function. That's trivially false. > You won't achieve anything by optimizing the hash table (= losing time), > [...] I think you're focusing too much on your immediate desire instead of the big(ger) picture. E.g. I see amdgpu_bo_export getting called from surprising places (in Xorg), performing a hash table lookup each time. Fixing that would achieve something, though probably not much. Anyway, adding dangerous API (keep in mind that we don't control all libdrm_amdgpu users, or even know how they're using it) for something that can also be achieved without is just a bad idea. Avoiding that is achievement enough. This is my opinion. I don't have veto power over libdrm_amdgpu. At least Christian seems in favour of this change. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: new KASAN running piglit
On 2018-07-18 06:05 PM, Tom St Denis wrote: > Hi Christian, > > This patch: > > [root@raven linux]# git bisect bad > 90f362bdf0d0d06a126a5fd35b084436dd8250ad is the first bad commit > commit 90f362bdf0d0d06a126a5fd35b084436dd8250ad > Author: Christian König > Date: Mon Jul 16 14:58:48 2018 +0200 > > drm/amdgpu: change ring priority after pushing the job > > Pushing a job can change the ring assignment of an entity. > > Signed-off-by: Christian König > Reviewed-by: Chunming Zhou > > :04 04 9a09d3e9d055e4f5024019861c334ee9cc0bd11b > 522540c31c3d1a4f4a6fbac75e985e9a7f7e93c9 M drivers > > causes a KASAN while running piglit. It's reproduceable 100% of the > time. The commit before this doesn't not cause a KASAN. I got the attached KASAN use-after-free report while running piglit today, could be the same? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer [ 2367.329074] WARNING: CPU: 8 PID: 16019 at drivers/gpu/drm//amd/amdgpu/../display/dc/dce100/dce100_hw_sequencer.c:154 dce100_set_bandwidth+0x154/0x300 [amdgpu] [ 2367.329086] Modules linked in: lz4(E) lz4_compress(E) cpufreq_powersave(E) cpufreq_userspace(E) cpufreq_conservative(E) binfmt_misc(E) nls_ascii(E) nls_cp437(E) vfat(E) edac_mce_amd(E) fat(E) amdkfd(OE) kvm(E) irqbypass(E) crct10dif_pclmul(E) crc32_pclmul(E) ghash_clmulni_intel(E) pcbc(E) radeon(OE) wmi_bmof(E) amdgpu(OE) snd_hda_codec_realtek(E) snd_hda_codec_generic(E) chash(OE) gpu_sched(OE) snd_hda_codec_hdmi(E) ttm(OE) aesni_intel(E) aes_x86_64(E) crypto_simd(E) drm_kms_helper(OE) cryptd(E) snd_hda_intel(E) efi_pstore(E) glue_helper(E) r8169(E) pcspkr(E) snd_hda_codec(E) drm(OE) mii(E) snd_hda_core(E) efivars(E) sg(E) i2c_algo_bit(E) snd_hwdep(E) fb_sys_fops(E) syscopyarea(E) snd_pcm(E) sysfillrect(E) sp5100_tco(E) snd_timer(E) sysimgblt(E) i2c_piix4(E) k10temp(E) wmi(E) ccp(E) snd(E) soundcore(E) [ 2367.329232] rng_core(E) button(E) acpi_cpufreq(E) tcp_bbr(E) sch_fq(E) nct6775(E) hwmon_vid(E) sunrpc(E) efivarfs(E) ip_tables(E) x_tables(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) fscrypto(E) dm_mod(E) raid10(E) raid1(E) raid0(E) multipath(E) linear(E) md_mod(E) sd_mod(E) evdev(E) hid_generic(E) usbhid(E) hid(E) ahci(E) xhci_pci(E) libahci(E) xhci_hcd(E) libata(E) crc32c_intel(E) usbcore(E) scsi_mod(E) gpio_amdpt(E) gpio_generic(E) [ 2367.329337] CPU: 8 PID: 16019 Comm: Xorg Tainted: GW OE 4.18.0-rc1+ #110 [ 2367.329344] Hardware name: Micro-Star International Co., Ltd. MS-7A34/B350 TOMAHAWK (MS-7A34), BIOS 1.80 09/13/2017 [ 2367.329532] RIP: 0010:dce100_set_bandwidth+0x154/0x300 [amdgpu] [ 2367.329538] Code: 84 c0 74 08 3c 03 0f 8e 7d 01 00 00 8b 83 10 01 00 00 41 39 c6 44 0f 42 f0 48 81 c3 30 04 00 00 48 39 d3 75 90 45 85 f6 75 02 <0f> 0b 49 8d 9d 48 01 00 00 48 b8 00 00 00 00 00 fc ff df 44 89 74 [ 2367.329663] RSP: 0018:88028cc2f338 EFLAGS: 00010246 [ 2367.329672] RAX: 11007b1652d0 RBX: 8803d8b29ab0 RCX: [ 2367.329678] RDX: 8803d8b29ab0 RSI: RDI: 8803d8b29c5c [ 2367.329684] RBP: dc00 R08: ed007a55d632 R09: ed007a55d632 [ 2367.329690] R10: 88028cc2f140 R11: ed007a55d631 R12: 110051985e69 [ 2367.329695] R13: 8803c8bee600 R14: R15: 8803d8b28000 [ 2367.329703] FS: 7f3ad605f940() GS:8803ee20() knlGS: [ 2367.329709] CS: 0010 DS: ES: CR0: 80050033 [ 2367.329714] CR2: 55720e2bb560 CR3: 0003c1418000 CR4: 003406e0 [ 2367.329720] Call Trace: [ 2367.329882] ? dce100_pplib_apply_display_requirements+0x1b0/0x1b0 [amdgpu] [ 2367.330041] ? bios_is_accelerated_mode+0xdb/0x140 [amdgpu] [ 2367.330201] dc_commit_state+0xd1e/0x1520 [amdgpu] [ 2367.330364] ? dc_destroy+0x90/0x90 [amdgpu] [ 2367.330403] ? drm_dev_dbg+0x1a0/0x1a0 [drm] [ 2367.330571] amdgpu_dm_atomic_commit_tail+0x968/0x3fa0 [amdgpu] [ 2367.330592] ? do_raw_spin_unlock+0x54/0x220 [ 2367.330602] ? _raw_spin_unlock_irq+0x29/0x40 [ 2367.330751] ? amdgpu_dm_do_flip+0xab0/0xab0 [amdgpu] [ 2367.330774] ? drm_atomic_helper_swap_state+0x7a2/0x15b0 [drm_kms_helper] [ 2367.330784] ? wait_for_completion_io_timeout+0x390/0x390 [ 2367.330793] ? lock_downgrade+0x5e0/0x5e0 [ 2367.330815] ? drm_atomic_helper_swap_state+0x6b5/0x15b0 [drm_kms_helper] [ 2367.330964] ? dm_plane_helper_prepare_fb+0x291/0xb00 [amdgpu] [ 2367.330990] ? drm_atomic_helper_wait_for_dependencies+0x3ee/0x7d0 [drm_kms_helper] [ 2367.331017] commit_tail+0x9a/0xf0 [drm_kms_helper] [ 2367.331041] drm_atomic_helper_commit+0x179/0x240 [drm_kms_helper] [ 2367.331086] drm_atomic_connector_commit_dpms+0x311/0x490 [drm] [ 2367.331097] ? ww_mutex_lock+0x34/0xa0 [ 2367.331138] ? drm_modeset_backoff+0x1b1/0x4f0 [
Re: dmesg warning during init
On 2018-07-16 05:55 PM, Tom St Denis wrote: > I have the same now and get the attached dmesg. Still all question marks. :( Anyway, looks like it's probably from ttm_bo_delayed_delete, but that still doesn't tell us where the BO is really freed from. Can you try the attached patch and see if the warning message triggers? If yes, what error does it report? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index b12526ce1a9d..89669544ffa8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -361,16 +361,21 @@ int amdgpu_bo_create_kernel(struct amdgpu_device *adev, void amdgpu_bo_free_kernel(struct amdgpu_bo **bo, u64 *gpu_addr, void **cpu_addr) { + int r; + if (*bo == NULL) return; - if (likely(amdgpu_bo_reserve(*bo, true) == 0)) { + r = amdgpu_bo_reserve(*bo, true); + if (likely(r == 0)) { if (cpu_addr) amdgpu_bo_kunmap(*bo); amdgpu_bo_unpin(*bo); amdgpu_bo_unreserve(*bo); - } + } else + DRM_WARN_ONCE("amdgpu_bo_reserve returned %d, can't unpin kernel BO\n", r); + amdgpu_bo_unref(bo); if (gpu_addr) ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: dmesg warning during init
On 2018-07-16 06:36 PM, Tom St Denis wrote: > Will in a bit. I'm tracking down a KASAN oops that the tip of drm-next > causes with unigine-heaven. Is it different from the one I reported an hour ago? https://lists.freedesktop.org/archives/amd-gfx/2018-July/024207.html -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/scheduler: add NULL pointer check for run queue (v2)
On 2018-07-16 11:23 AM, Junwei Zhang wrote: > To check rq pointer before adding entity into it. > That avoids NULL pointer access in some case. > > v2: move the check to caller > > Suggested-by: Christian König > Signed-off-by: Junwei Zhang > --- > drivers/gpu/drm/scheduler/gpu_scheduler.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c > b/drivers/gpu/drm/scheduler/gpu_scheduler.c > index 16bf446..dac71e3 100644 > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c > @@ -547,6 +547,11 @@ void drm_sched_entity_push_job(struct drm_sched_job > *sched_job, > if (first) { > /* Add the entity to the run queue */ > spin_lock(>rq_lock); > + if (!entity->rq) { > + DRM_ERROR("Trying to push to a killed entity\n"); This could result in spamming dmesg with this error message. I suggest if (WARN_ON_ONCE(!entity->rq)) { instead, no DRM_ERROR. That will produce a single warning with a backtrace. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm] amdgpu: add amdgpu_bo_handle_type_kms_noimport
On 2018-07-17 09:59 AM, Christian König wrote: > Am 17.07.2018 um 09:46 schrieb Michel Dänzer: >> On 2018-07-17 09:33 AM, Christian König wrote: >>> Am 17.07.2018 um 09:26 schrieb Michel Dänzer: >>>> On 2018-07-17 08:50 AM, Christian König wrote: >>>>> Am 16.07.2018 um 18:05 schrieb Michel Dänzer: >>>>>> On 2018-07-13 08:47 PM, Marek Olšák wrote: >>>>>> [SNIP] >>>>>> Other opinions? >>>>> I understand the reason why Marek wants to do this, but I agree that >>>>> this is a little bit dangerous if used incorrectly. >>>>> >>>>> On the other hand I don't see any other way to sanely handle it >>>>> either. >>>> Sanely handle what exactly? :) I still haven't seen any description of >>>> an actual problem, other than "the handle is stored in the hash table". >>> Well the problem is that it's not "the handle" but rather "all handles" >>> which are now stored in the hash table. >>> >>> To begin with that is quite a bunch of wasted memory, not talking about >>> the extra CPU cycles. >> All that should be needed is one struct list_head per BO, 16 bytes on >> 64-bit. > > +malloc overhead and that for *every* BO the application/driver > allocated. The struct list_head can be stored in struct amdgpu_bo, no additional malloc necessary. > The last time I looked we could easily have a few thousands of that > (but not in the same CS). > > So I would guess that the wasted memory can easily be in the lower kb > range, compared to adding just a flag that we never going to import the > handle again. I wouldn't call the memory "wasted", as it serves a clear purpose. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm] amdgpu: add amdgpu_bo_handle_type_kms_noimport
On 2018-07-17 10:19 AM, Christian König wrote: > Am 17.07.2018 um 10:03 schrieb Michel Dänzer: >> On 2018-07-17 09:59 AM, Christian König wrote: >>> Am 17.07.2018 um 09:46 schrieb Michel Dänzer: >>>> On 2018-07-17 09:33 AM, Christian König wrote: >>>>> Am 17.07.2018 um 09:26 schrieb Michel Dänzer: >>>>> [SNIP] >>>> All that should be needed is one struct list_head per BO, 16 bytes on >>>> 64-bit. >>> +malloc overhead and that for *every* BO the application/driver >>> allocated. >> The struct list_head can be stored in struct amdgpu_bo, no additional >> malloc necessary. > > Well that sounds we are not talking about the same code, do we? > > IIRC the hashtable implementation in libdrm is using an ever growing > array for the BOs and *NOT* a linked list. So let's use something more suitable, e.g.: An array of 2^n struct list_head in struct amdgpu_device for the hash buckets. The BO's handle is hashed to the bucket number handle & (2^n - 1) and linked in there via struct list_head in struct amdgpu_bo. amdgpu_bo_alloc and amdgpu_create_bo_from_user_mem add the handle at the end of the list, amdgpu_bo_import adds it at or moves it to the beginning. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm] amdgpu: add amdgpu_bo_handle_type_kms_noimport
On 2018-07-16 08:51 PM, Marek Olšák wrote: > On Mon, Jul 16, 2018 at 12:05 PM, Michel Dänzer wrote: >> On 2018-07-13 08:47 PM, Marek Olšák wrote: >>> On Fri, Jul 13, 2018 at 4:28 AM, Michel Dänzer wrote: >> >>>> I'd rather add the handle to the hash table in amdgpu_bo_alloc, >>>> amdgpu_create_bo_from_user_mem and amdgpu_bo_import instead of in >>>> amdgpu_bo_export, making amdgpu_bo_export(bo, amdgpu_bo_handle_type_kms, >>>> ...) essentially free. In the unlikely (since allocating a BO from the >>>> kernel is expensive) case that the hash table shows up on profiles, we >>>> can optimize it. >>> >>> The hash table isn't very good for high BO counts. The time complexity >>> of a lookup is O(n). >> >> A lookup is only needed in amdgpu_bo_import. amdgpu_bo_alloc and >> amdgpu_create_bo_from_user_mem can just add the handle to the hash >> bucket directly. >> >> Do you know of, or can you imagine, any workload where amdgpu_bo_import >> is called often enough for this to be a concern? > > Fullscreen DRI2 or DRI3 re-imports buffers every frame. DRI3 doesn't. The X server only imports each DRI3 buffer once, after that it's referred to via the pixmap XID. With DRI2 page flipping (ignoring that basically nobody's using that anymore with radeonsi :), it's always the same set of buffers, so the lookup can be made fast as discussed in the sub-thread with Christian. (Also, DRI2 can only use page flipping with sync-to-vblank enabled, so this happens on the order of hundreds of times per second max) -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm] amdgpu: add amdgpu_bo_handle_type_kms_noimport
On 2018-07-17 08:50 AM, Christian König wrote: > Am 16.07.2018 um 18:05 schrieb Michel Dänzer: >> On 2018-07-13 08:47 PM, Marek Olšák wrote: >>> On Fri, Jul 13, 2018 at 4:28 AM, Michel Dänzer >>> wrote: >>>> On 2018-07-12 07:03 PM, Marek Olšák wrote: >>>>> On Thu, Jul 12, 2018, 3:31 AM Michel Dänzer >>>>> wrote: >>>>>> What is the rationale for this? I.e. why do you want to not store >>>>>> some >>>>>> handles in the hash table? >>>>> >>>>> Because I have the option. >>>> Seems like you're expecting this patch to be accepted without providing >>>> any real justification for it (here or in the corresponding Mesa >>>> patch). >>>> NAK from me if so. >>> The real justification is implied by the patch. See: >>> amdgpu_add_handle_to_table >>> Like I said: There is no risk of regression and it simplifies one >>> simple case trivially. We shouldn't have to even talk about it. >> IMO you haven't provided enough justification for adding API which is >> prone to breakage if used incorrectly. >> >> Other opinions? > > I understand the reason why Marek wants to do this, but I agree that > this is a little bit dangerous if used incorrectly. > > On the other hand I don't see any other way to sanely handle it either. Sanely handle what exactly? :) I still haven't seen any description of an actual problem, other than "the handle is stored in the hash table". >>>> I'd rather add the handle to the hash table in amdgpu_bo_alloc, >>>> amdgpu_create_bo_from_user_mem and amdgpu_bo_import instead of in >>>> amdgpu_bo_export, making amdgpu_bo_export(bo, >>>> amdgpu_bo_handle_type_kms, >>>> ...) essentially free. In the unlikely (since allocating a BO from the >>>> kernel is expensive) case that the hash table shows up on profiles, we >>>> can optimize it. >>> The hash table isn't very good for high BO counts. The time complexity >>> of a lookup is O(n). >> A lookup is only needed in amdgpu_bo_import. amdgpu_bo_alloc and >> amdgpu_create_bo_from_user_mem can just add the handle to the hash >> bucket directly. >> >> Do you know of, or can you imagine, any workload where amdgpu_bo_import >> is called often enough for this to be a concern? > > MM interop with GFX usually imports BOs on each frame, so that can hurt > here. That's always the same set of BOs in the steady state, right? So it's easy to make the repeated lookups fast, by moving them to the start of their hash buckets. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm] amdgpu: add amdgpu_bo_handle_type_kms_noimport
On 2018-07-17 09:33 AM, Christian König wrote: > Am 17.07.2018 um 09:26 schrieb Michel Dänzer: >> On 2018-07-17 08:50 AM, Christian König wrote: >>> Am 16.07.2018 um 18:05 schrieb Michel Dänzer: >>>> On 2018-07-13 08:47 PM, Marek Olšák wrote: >>>> [SNIP] >>>> Other opinions? >>> I understand the reason why Marek wants to do this, but I agree that >>> this is a little bit dangerous if used incorrectly. >>> >>> On the other hand I don't see any other way to sanely handle it either. >> Sanely handle what exactly? :) I still haven't seen any description of >> an actual problem, other than "the handle is stored in the hash table". > > Well the problem is that it's not "the handle" but rather "all handles" > which are now stored in the hash table. > > To begin with that is quite a bunch of wasted memory, not talking about > the extra CPU cycles. All that should be needed is one struct list_head per BO, 16 bytes on 64-bit. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu] Use strcpy for RandR output property names
From: Michel Dänzer Instead of strncpy with the string length. Avoids new warnings with GCC 8: ../../src/drmmode_display.c: In function ‘drmmode_output_create_resources’: ../../src/drmmode_display.c:2240:2: warning: ‘strncpy’ output truncated before terminating nul copying 8 bytes from a string of the same length [-Wstringop-truncation] strncpy(tearfree_prop->name, "TearFree", 8); ^~~ ../../src/drmmode_display.c:2244:2: warning: ‘strncpy’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation] strncpy(tearfree_prop->enums[0].name, "off", 3); ^~~ ../../src/drmmode_display.c:2245:2: warning: ‘strncpy’ output truncated before terminating nul copying 2 bytes from a string of the same length [-Wstringop-truncation] strncpy(tearfree_prop->enums[1].name, "on", 2); ^~ ../../src/drmmode_display.c:2247:2: warning: ‘strncpy’ output truncated before terminating nul copying 4 bytes from a string of the same length [-Wstringop-truncation] strncpy(tearfree_prop->enums[2].name, "auto", 4); ^~~~~~~~ Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index b3e754005..92f58c157 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -2211,14 +2211,14 @@ static void drmmode_output_create_resources(xf86OutputPtr output) /* Userspace-only property for TearFree */ tearfree_prop = calloc(1, sizeof(*tearfree_prop)); tearfree_prop->flags = DRM_MODE_PROP_ENUM; - strncpy(tearfree_prop->name, "TearFree", 8); + strcpy(tearfree_prop->name, "TearFree"); tearfree_prop->count_enums = 3; tearfree_prop->enums = calloc(tearfree_prop->count_enums, sizeof(*tearfree_prop->enums)); - strncpy(tearfree_prop->enums[0].name, "off", 3); - strncpy(tearfree_prop->enums[1].name, "on", 2); + strcpy(tearfree_prop->enums[0].name, "off"); + strcpy(tearfree_prop->enums[1].name, "on"); tearfree_prop->enums[1].value = 1; - strncpy(tearfree_prop->enums[2].name, "auto", 4); + strcpy(tearfree_prop->enums[2].name, "auto"); tearfree_prop->enums[2].value = 2; drmmode_output->props[j].mode_prop = tearfree_prop; drmmode_output->props[j].value = info->tear_free; -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati] Remove drmmode_terminate_leases
From: Michel Dänzer The RandR screen private is already freed when our CloseScreen runs, so this can't do anything useful. This cleanup has to be done by the X server itself. (Ported from amdgpu commit 5f06d6b8ba570b500956ad26fee711d5ac427818) Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 20 src/drmmode_display.h | 2 -- src/radeon_kms.c | 1 - 3 files changed, 23 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index ff098975f..60c1cdc18 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -2522,26 +2522,6 @@ drmmode_terminate_lease(RRLeasePtr lease) #endif // XF86_LEASE_VERSION -void -drmmode_terminate_leases(ScrnInfoPtr pScrn) -{ -#ifdef XF86_LEASE_VERSION - ScreenPtr screen = xf86ScrnToScreen(pScrn); - RADEONEntPtr pRADEONEnt = RADEONEntPriv(pScrn); - rrScrPrivPtr scr_priv = rrGetScrPriv(screen); - RRLeasePtr lease, next; - - xorg_list_for_each_entry_safe(lease, next, _priv->leases, list) { - drmmode_lease_private_ptr lease_private = lease->devPrivate; - drmModeRevokeLease(pRADEONEnt->fd, lease_private->lessee_id); - free(lease_private); - lease->devPrivate = NULL; - RRLeaseTerminated(lease); - RRLeaseFree(lease); - } -#endif -} - static const xf86CrtcConfigFuncsRec drmmode_xf86crtc_config_funcs = { .resize = drmmode_xf86crtc_resize, #ifdef XF86_LEASE_VERSION diff --git a/src/drmmode_display.h b/src/drmmode_display.h index 4551e0c77..c5a55891a 100644 --- a/src/drmmode_display.h +++ b/src/drmmode_display.h @@ -227,8 +227,6 @@ PixmapPtr drmmode_crtc_scanout_create(xf86CrtcPtr crtc, extern void drmmode_uevent_init(ScrnInfoPtr scrn, drmmode_ptr drmmode); extern void drmmode_uevent_fini(ScrnInfoPtr scrn, drmmode_ptr drmmode); -extern void drmmode_terminate_leases(ScrnInfoPtr scrn); - Bool drmmode_set_mode(xf86CrtcPtr crtc, struct drmmode_fb *fb, DisplayModePtr mode, int x, int y); diff --git a/src/radeon_kms.c b/src/radeon_kms.c index c8a5726ad..36840ad36 100644 --- a/src/radeon_kms.c +++ b/src/radeon_kms.c @@ -2154,7 +2154,6 @@ static Bool RADEONCloseScreen_KMS(ScreenPtr pScreen) /* Clear mask of assigned crtc's in this generation */ pRADEONEnt->assigned_crtcs = 0; -drmmode_terminate_leases(pScrn); drmmode_uevent_fini(pScrn, >drmmode); radeon_drm_queue_close(pScrn); radeon_cs_flush_indirect(pScrn); -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Don't warn on destroying a pinned BO
On 2018-07-19 05:39 PM, Michel Dänzer wrote: > From: Michel Dänzer > > The warning turned out to be not so useful, as BO destruction tends to > be deferred to a workqueue. > > Also, we should be preventing any damage from this now, so not really > important anymore to fix code doing this. > > Signed-off-by: Michel Dänzer > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index b12526ce1a9d..3010f0136de9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -89,7 +89,7 @@ static void amdgpu_ttm_bo_destroy(struct ttm_buffer_object > *tbo) > struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev); > struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); > > - if (WARN_ON_ONCE(bo->pin_count > 0)) > + if (bo->pin_count > 0) > amdgpu_bo_subtract_pin_size(bo); > > if (bo->kfd_bo) > Any feedback? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati] Hardcode "non-desktop" RandR property name
From: Michel Dänzer It's a bit silly to require current randrproto just for this definition, which can't really change anyway. Suggested-by: Qiang Yu (Ported from amdgpu commit ae8e02c6fc4ef5d5340b8cd4739e66b19b9e3386) Signed-off-by: Michel Dänzer --- configure.ac | 2 +- src/drmmode_display.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 15c7b2128..bea205c34 100644 --- a/configure.ac +++ b/configure.ac @@ -64,7 +64,7 @@ AC_ARG_WITH(xorg-module-dir, [moduledir="$libdir/xorg/modules"]) # Store the list of server defined optional extensions in REQUIRED_MODULES -XORG_DRIVER_CHECK_EXT(RANDR, [randrproto >= 1.6.0]) +XORG_DRIVER_CHECK_EXT(RANDR, randrproto) XORG_DRIVER_CHECK_EXT(RENDER, renderproto) XORG_DRIVER_CHECK_EXT(XV, videoproto) XORG_DRIVER_CHECK_EXT(DPMSExtension, xextproto) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 000c7fc62..ff098975f 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -2012,7 +2012,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_r #if XF86_CRTC_VERSION >= 8 i = koutput_get_prop_idx(pRADEONEnt->fd, koutput, DRM_MODE_PROP_RANGE, -RR_PROPERTY_NON_DESKTOP); +"non-desktop"); if (i >= 0) nonDesktop = koutput->prop_values[i] != 0; #endif -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: move the amdgpu_fbdev_set_suspend() further up
On 2018-07-23 12:06 PM, Shirish S wrote: > This patch moves amdgpu_fbdev_set_suspend() to the beginning > of suspend sequence. Would be nice to have some rationale, e.g.: This is to avoid fbcon trying to write to the GPU's memory when the GPU is already powered down. Other than that looks great, thanks. With a rationale added to the commit log, Reviewed-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu] Don't use DRM_IOCTL_GEM_FLINK in create_pixmap_for_fbcon
From: Michel Dänzer We don't need it. Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 39 +++ 1 file changed, 11 insertions(+), 28 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index e947ca979..bf8b1a8b6 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -355,13 +355,11 @@ static PixmapPtr create_pixmap_for_fbcon(drmmode_ptr drmmode, ScrnInfoPtr pScrn, int fbcon_id) { + ScreenPtr pScreen = pScrn->pScreen; AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn); AMDGPUInfoPtr info = AMDGPUPTR(pScrn); PixmapPtr pixmap = info->fbcon_pixmap; - struct amdgpu_buffer *bo; drmModeFBPtr fbcon; - struct drm_gem_flink flink; - struct amdgpu_bo_import_result import = {0}; if (pixmap) return pixmap; @@ -375,36 +373,21 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode, fbcon->height != pScrn->virtualY) goto out_free_fb; - flink.handle = fbcon->handle; - if (ioctl(pAMDGPUEnt->fd, DRM_IOCTL_GEM_FLINK, ) < 0) { - xf86DrvMsg(pScrn->scrnIndex, X_ERROR, - "Couldn't flink fbcon handle\n"); + pixmap = fbCreatePixmap(pScreen, 0, 0, fbcon->depth, 0); + if (!pixmap) goto out_free_fb; - } - bo = calloc(1, sizeof(struct amdgpu_buffer)); - if (!bo) { - xf86DrvMsg(pScrn->scrnIndex, X_ERROR, - "Couldn't allocate bo for fbcon handle\n"); - goto out_free_fb; - } - bo->ref_count = 1; + pScreen->ModifyPixmapHeader(pixmap, fbcon->width, fbcon->height, 0, 0, + fbcon->pitch, NULL); + pixmap->devPrivate.ptr = NULL; - if (amdgpu_bo_import(pAMDGPUEnt->pDev, -amdgpu_bo_handle_type_gem_flink_name, flink.name, -) != 0) { - xf86DrvMsg(pScrn->scrnIndex, X_ERROR, - "Couldn't import BO for fbcon handle\n"); - goto out_free_bo; + if (!glamor_egl_create_textured_pixmap(pixmap, fbcon->handle, + pixmap->devKind)) { + pScreen->DestroyPixmap(pixmap); + pixmap = NULL; } - bo->bo.amdgpu = import.buf_handle; - - pixmap = drmmode_create_bo_pixmap(pScrn, fbcon->width, fbcon->height, - fbcon->depth, fbcon->bpp, - fbcon->pitch, bo); + info->fbcon_pixmap = pixmap; -out_free_bo: - amdgpu_bo_unref(); out_free_fb: drmModeFreeFB(fbcon); return pixmap; -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: lock and unlock console only for amdgpu_fbdev_set_suspend [V2]
On 2018-07-18 12:24 PM, Shirish S wrote: > [Why] > While the console_lock is held, console output will be buffered, till > its unlocked it wont be emitted, hence its ideal to unlock sooner to enable > debugging/detecting/fixing of any issue in the remaining sequence of events > in resume path. Maybe this could be clarified that the concern is about consoles other than fbcon on this device, e.g. a serial console. > @@ -2746,7 +2743,6 @@ int amdgpu_device_resume(struct drm_device *dev, bool > resume, bool fbcon) > > amdgpu_fence_driver_resume(adev); > > - > r = amdgpu_device_ip_late_init(adev); > if (r) > goto unlock; Drop this hunk. With the above fixed, Reviewed-by: Michel Dänzer Possible follow-up work: * Move the console_(un)lock calls into amdgpu_fbdev_set_suspend, or maybe use drm_fb_helper_set_suspend_unlocked instead of locking ourselves * Move the amdgpu_fbdev_set_suspend call in amdgpu_device_suspend further up, at least before the pci_set_power_state call. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: lock and unlock console only for amdgpu_fbdev_set_suspend [V3]
On 2018-07-19 11:29 AM, Shirish S wrote: > [Why] > While the console_lock is held, console output will be buffered, till > its unlocked it wont be emitted, hence its ideal to unlock sooner to enable > debugging/detecting/fixing of any issue in the remaining sequence of events > in resume path. > The concern here is about consoles other than fbcon on the device, > e.g. a serial console > > [How] > This patch restructures the console_lock, console_unlock around > amdgpu_fbdev_set_suspend() and moves this new block appropriately. > > V2: Kept amdgpu_fbdev_set_suspend after pci_set_power_state > V3: Updated the commit message to clarify the real concern that this patch > addresses. > Signed-off-by: Shirish S > Reviewed-by: Michel Dänzer > > [...] > > drm_kms_helper_poll_enable(dev); > @@ -2808,13 +2808,7 @@ int amdgpu_device_resume(struct drm_device *dev, bool > resume, bool fbcon) > dev->dev->power.disable_depth--; > #endif > > - if (fbcon) > - amdgpu_fbdev_set_suspend(adev, 0); > - > unlock: > - if (fbcon) > - console_unlock(); > - > return r; > } > > Oh, please remove the now useless unlock label as well. Sorry I missed that before. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu] Free previous xf86CrtcRec gamma LUT memory
From: Michel Dänzer We were leaking it. Also, don't bother allocating new memory if it's already the expected size. Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 1aefd199b..e947ca979 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -3629,13 +3629,18 @@ Bool drmmode_setup_colormap(ScreenPtr pScreen, ScrnInfoPtr pScrn) for (i = 0; i < xf86_config->num_crtc; i++) { xf86CrtcPtr crtc = xf86_config->crtc[i]; - void *gamma = malloc(1024 * 3 * sizeof(CARD16)); + void *gamma; + if (crtc->gamma_size == 1024) + continue; + + gamma = malloc(1024 * 3 * sizeof(CARD16)); if (!gamma) { ErrorF("Failed to allocate gamma LUT memory\n"); return FALSE; } + free(crtc->gamma_red); crtc->gamma_size = 1024; crtc->gamma_red = gamma; crtc->gamma_green = crtc->gamma_red + crtc->gamma_size; -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu: Don't warn on destroying a pinned BO
From: Michel Dänzer The warning turned out to be not so useful, as BO destruction tends to be deferred to a workqueue. Also, we should be preventing any damage from this now, so not really important anymore to fix code doing this. Signed-off-by: Michel Dänzer --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index b12526ce1a9d..3010f0136de9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -89,7 +89,7 @@ static void amdgpu_ttm_bo_destroy(struct ttm_buffer_object *tbo) struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev); struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); - if (WARN_ON_ONCE(bo->pin_count > 0)) + if (bo->pin_count > 0) amdgpu_bo_subtract_pin_size(bo); if (bo->kfd_bo) -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Fix S3 resume failre.
On 2018-07-19 06:33 PM, Andrey Grodzovsky wrote: > On 07/19/2018 11:39 AM, Ville Syrjälä wrote: >> On Thu, Jul 19, 2018 at 11:19:56AM -0400, Andrey Grodzovsky wrote: >>> Problem: >>> FB is still not unpinned during the first run of amdgpu_bo_evict_vram >>> and so it's left for the second run, but during second run the SDMA for >>> moving buffer around already disabled and you have to do >>> it with CPU, but FB is not in visible VRAM and hence the eviction >>> failure >>> leading later to resume failure. >>> >>> Fix: >>> When DAL in use get a pointer to FB from crtc->primary->state rather >>> then from crtc->primary which is not set for DAL since it supports >>> atomic KMS. >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107065 >>> Fixes e00fb85 drm: Stop updating plane->crtc/fb/old_fb on atomic drivers >>> Signed-off-by: Andrey Grodzovsky >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> index 709e4a3..dd9ebf7 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>> @@ -2642,7 +2642,8 @@ int amdgpu_device_suspend(struct drm_device >>> *dev, bool suspend, bool fbcon) >>> /* unpin the front buffers and cursors */ >>> list_for_each_entry(crtc, >mode_config.crtc_list, head) { >>> struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc); >>> - struct drm_framebuffer *fb = crtc->primary->fb; >>> + struct drm_framebuffer *fb = >>> amdgpu_device_has_dc_support(adev) ? >>> + crtc->primary->state->fb : crtc->primary->fb; >> So apparently you haven't yet turned off the planes here. If I'm >> reading things right amdgpu_device_ip_suspend() should end up doing >> that through drm_atomic_helper_suspend(). So it looks like like now >> you'll end up unpinning the same bos twice. Doesn't that mess up >> some kind of refcount or something? > amdgpu_bo_unpin has a guard against that, amdgpu_bo_unreserve is less > clear. BO reservation shouldn't an issue here, BOs are only reserved for a short time around (un)pinning them. >> To me it would seem better to susped the display before trying >> to evict the bos. > > Yea, i was aware of that and indeed DAL shouldn't rely on the code in > amdgpu_device_suspend to unpin > front buffer and cursor since the atomic helper should do it. Problem is > that during amdgpu_device_ip_suspend > the SDMA engine gets suspended too, so you have to embed another > eviction in between, after display is suspended but before > SDMA and this forces ordering between them which kind of already in > place (amd_ip_block_type) but still it's an extra constrain. Ville's point (which I basically agree with) is that the display hardware should be turned off before evicting VRAM the first time, in which case no second eviction should be necessary (for this purpose). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: lock and unlock console only for amdgpu_fbdev_set_suspend [V5]
On 2018-07-19 12:02 PM, Shirish S wrote: > [Why] > While the console_lock is held, console output will be buffered, till > its unlocked it wont be emitted, hence its ideal to unlock sooner to enable > debugging/detecting/fixing of any issue in the remaining sequence of events > in resume path. > The concern here is about consoles other than fbcon on the device, > e.g. a serial console > > [How] > This patch restructures the console_lock, console_unlock around > amdgpu_fbdev_set_suspend() and moves this new block appropriately. > > V2: Kept amdgpu_fbdev_set_suspend after pci_set_power_state > V3: Updated the commit message to clarify the real concern that this patch > addresses. > V4: code clean-up. > V5: fixed return value > > Signed-off-by: Shirish S > Reviewed-by: Michel Dänzer In the future, please mark which revision a review was given for when making significant changes in later revisions. However, v5 looks good, so in this case, you can leave it as is. :) (BTW, this is also a bug fix: we would leave the console locked if amdgpu_amdkfd_resume returned an error) -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] MAINTAINERS: Add separate section for DC
On 2018-07-19 04:36 PM, Christian König wrote: > Note that Harry and Leo Li are maintainers for that stuff. > > Signed-off-by: Christian König > --- > MAINTAINERS | 8 > 1 file changed, 8 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index e613df455ae0..2e9111e65d64 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -728,6 +728,14 @@ S: Supported > F: drivers/crypto/ccp/ > F: include/linux/ccp.h > > +AMD DISPLAY CORE > +M: Harry Wentland > +M: Leo Li > +L: amd-gfx@lists.freedesktop.org > +T: git git://people.freedesktop.org/~agd5f/linux > +S: Supported > +F: drivers/gpu/drm/amd/display/ > + > AMD FAM15H PROCESSOR POWER MONITORING DRIVER > M: Huang Rui > L: linux-hw...@vger.kernel.org > Reviewed-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu] Remove AMDGPUInfoRec::fbcon_pixmap
From: Michel Dänzer We always destroy the fbcon pixmap in drmmode_copy_fb anyway. Signed-off-by: Michel Dänzer --- src/amdgpu_drv.h | 1 - src/amdgpu_kms.c | 3 --- src/drmmode_display.c | 13 ++--- 3 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/amdgpu_drv.h b/src/amdgpu_drv.h index 8a77b05fd..45bc394aa 100644 --- a/src/amdgpu_drv.h +++ b/src/amdgpu_drv.h @@ -264,7 +264,6 @@ typedef struct { struct amdgpu_dri2 dri2; /* accel */ - PixmapPtr fbcon_pixmap; int callback_event_type; uint_fast32_t gpu_flushed; uint_fast32_t gpu_synced; diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c index 39e047e29..c22b7975c 100644 --- a/src/amdgpu_kms.c +++ b/src/amdgpu_kms.c @@ -125,9 +125,6 @@ static void AMDGPUFreeRec(ScrnInfoPtr pScrn) info = AMDGPUPTR(pScrn); if (info) { - if (info->fbcon_pixmap) - pScrn->pScreen->DestroyPixmap(info->fbcon_pixmap); - pEnt = info->pEnt; free(pScrn->driverPrivate); pScrn->driverPrivate = NULL; diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 8407a4577..aa4915933 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -357,13 +357,9 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode, { ScreenPtr pScreen = pScrn->pScreen; AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn); - AMDGPUInfoPtr info = AMDGPUPTR(pScrn); - PixmapPtr pixmap = info->fbcon_pixmap; + PixmapPtr pixmap = NULL; drmModeFBPtr fbcon; - if (pixmap) - return pixmap; - fbcon = drmModeGetFB(pAMDGPUEnt->fd, fbcon_id); if (!fbcon) return NULL; @@ -387,7 +383,6 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode, pixmap = NULL; } - info->fbcon_pixmap = pixmap; out_free_fb: drmModeFreeFB(fbcon); return pixmap; @@ -396,7 +391,6 @@ out_free_fb: void drmmode_copy_fb(ScrnInfoPtr pScrn, drmmode_ptr drmmode) { xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn); - AMDGPUInfoPtr info = AMDGPUPTR(pScrn); ScreenPtr pScreen = pScrn->pScreen; PixmapPtr src, dst = pScreen->GetScreenPixmap(pScreen); struct drmmode_fb *fb = amdgpu_pixmap_get_fb(dst); @@ -436,10 +430,7 @@ void drmmode_copy_fb(ScrnInfoPtr pScrn, drmmode_ptr drmmode) FreeScratchGC(gc); pScreen->canDoBGNoneRoot = TRUE; - - if (info->fbcon_pixmap) - pScrn->pScreen->DestroyPixmap(info->fbcon_pixmap); - info->fbcon_pixmap = NULL; + pScreen->DestroyPixmap(src); return; } -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Fix S3 resume failre.
On 2018-07-19 06:53 PM, Andrey Grodzovsky wrote: > > > On 07/19/2018 12:47 PM, Michel Dänzer wrote: >> On 2018-07-19 06:33 PM, Andrey Grodzovsky wrote: >>> On 07/19/2018 11:39 AM, Ville Syrjälä wrote: >>>> On Thu, Jul 19, 2018 at 11:19:56AM -0400, Andrey Grodzovsky wrote: >>>>> Problem: >>>>> FB is still not unpinned during the first run of amdgpu_bo_evict_vram >>>>> and so it's left for the second run, but during second run the SDMA >>>>> for >>>>> moving buffer around already disabled and you have to do >>>>> it with CPU, but FB is not in visible VRAM and hence the eviction >>>>> failure >>>>> leading later to resume failure. >>>>> >>>>> Fix: >>>>> When DAL in use get a pointer to FB from crtc->primary->state rather >>>>> then from crtc->primary which is not set for DAL since it supports >>>>> atomic KMS. >>>>> >>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107065 >>>>> Fixes e00fb85 drm: Stop updating plane->crtc/fb/old_fb on atomic >>>>> drivers >>>>> Signed-off-by: Andrey Grodzovsky >>>>> --- >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>>>> index 709e4a3..dd9ebf7 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >>>>> @@ -2642,7 +2642,8 @@ int amdgpu_device_suspend(struct drm_device >>>>> *dev, bool suspend, bool fbcon) >>>>> /* unpin the front buffers and cursors */ >>>>> list_for_each_entry(crtc, >mode_config.crtc_list, head) { >>>>> struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc); >>>>> - struct drm_framebuffer *fb = crtc->primary->fb; >>>>> + struct drm_framebuffer *fb = >>>>> amdgpu_device_has_dc_support(adev) ? >>>>> + crtc->primary->state->fb : crtc->primary->fb; >>>> So apparently you haven't yet turned off the planes here. If I'm >>>> reading things right amdgpu_device_ip_suspend() should end up doing >>>> that through drm_atomic_helper_suspend(). So it looks like like now >>>> you'll end up unpinning the same bos twice. Doesn't that mess up >>>> some kind of refcount or something? >>> amdgpu_bo_unpin has a guard against that, amdgpu_bo_unreserve is less >>> clear. >> BO reservation shouldn't an issue here, BOs are only reserved for a >> short time around (un)pinning them. >> >> >>>> To me it would seem better to susped the display before trying >>>> to evict the bos. >>> Yea, i was aware of that and indeed DAL shouldn't rely on the code in >>> amdgpu_device_suspend to unpin >>> front buffer and cursor since the atomic helper should do it. Problem is >>> that during amdgpu_device_ip_suspend >>> the SDMA engine gets suspended too, so you have to embed another >>> eviction in between, after display is suspended but before >>> SDMA and this forces ordering between them which kind of already in >>> place (amd_ip_block_type) but still it's an extra constrain. >> Ville's point (which I basically agree with) is that the display >> hardware should be turned off before evicting VRAM the first time, in >> which case no second eviction should be necessary (for this purpose). > > Display HW is turned off as part of all IPs in a loop inside > amdgpu_device_ip_suspend. > Are you suggesting to extract the display HW turn off from inside > amdgpu_device_ip_suspend and place it > before the first call to amdgpu_bo_evict_vram ? In a nutshell, yes. Or maybe it would be easier to move the amdgpu_bo_evict_vram call down to somewhere called from amdgpu_device_ip_suspend? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: dmesg warning during init
On 2018-07-16 02:10 PM, Tom St Denis wrote: > Hi all, > > Back from vacation, booting up the latest drm-next kernel and noticed > this warning on boot. Attached. > > I like the comment in the commit 5c200f56506a142451f17dbea7befa76dd74a942: > > "This shouldn't happen, but if it does, we'll get a backtrace of the > caller, and update the pin_size values as needed." Once again, turns out it is better to be safe than sorry. :) Unfortunately, the backtrace isn't really usable, all entries have a question mark. What does grep -e UNWINDER -e DEBUG_INFO -e FRAME_POINTER .config say in your build tree? FWIW, it's the below for me, and in my backtraces some entries have no question mark, and those seem reliable. CONFIG_SCHED_OMIT_FRAME_POINTER=y CONFIG_DEBUG_INFO=y CONFIG_DEBUG_INFO_REDUCED=y # CONFIG_DEBUG_INFO_SPLIT is not set # CONFIG_DEBUG_INFO_DWARF4 is not set CONFIG_UNWINDER_ORC=y # CONFIG_UNWINDER_FRAME_POINTER is not set # CONFIG_UNWINDER_GUESS is not set -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 6/7] drm/amdgpu: remove job->adev
On 2018-07-13 05:19 PM, Christian König wrote: > We can get that from the ring. > > Signed-off-by: Christian König This change introduced the attached oops when running the piglit max-texture-size test, after which the test process hangs. Note that the test always triggers the out of memory condition in amdgpu_cs_ioctl, but before this change that was handled gracefully. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer Jul 16 17:35:32 kaveri kernel: [ 97.203039] [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* amdgpu_cs_list_validate(validated) failed. Jul 16 17:35:32 kaveri kernel: [ 97.203299] [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Not enough memory for command submission! Jul 16 17:35:32 kaveri kernel: [ 97.203388] BUG: unable to handle kernel paging request at ff30 Jul 16 17:35:32 kaveri kernel: [ 97.203398] PGD 1a6e16067 P4D 1a6e16067 PUD 1a6e18067 PMD 0 Jul 16 17:35:32 kaveri kernel: [ 97.203414] Oops: [#1] SMP KASAN NOPTI Jul 16 17:35:32 kaveri kernel: [ 97.203423] CPU: 10 PID: 2009 Comm: max-textur:cs0 Tainted: GW OE 4.18.0-rc1+ #110 Jul 16 17:35:32 kaveri kernel: [ 97.203428] Hardware name: Micro-Star International Co., Ltd. MS-7A34/B350 TOMAHAWK (MS-7A34), BIOS 1.80 09/13/2017 Jul 16 17:35:32 kaveri kernel: [ 97.203555] RIP: 0010:amdgpu_job_free_resources+0x122/0x1f0 [amdgpu] Jul 16 17:35:32 kaveri kernel: [ 97.203559] Code: 85 dc 00 00 00 48 8b 44 24 08 89 ea 48 8d 34 92 49 8b 96 48 02 00 00 42 80 3c 38 00 48 8d 34 f2 0f 85 a2 00 00 00 48 8b 14 24 <49> 8b bc 24 30 ff ff ff 83 c5 01 e8 2e 45 d9 ff 48 89 da 48 c1 ea Jul 16 17:35:32 kaveri kernel: [ 97.203664] RSP: 0018:8803de2e7698 EFLAGS: 00010246 Jul 16 17:35:32 kaveri kernel: [ 97.203671] RAX: 1fe6 RBX: 8803465969dc RCX: 1fe6 Jul 16 17:35:32 kaveri kernel: [ 97.203677] RDX: RSI: 880346596a30 RDI: 8803465969d0 Jul 16 17:35:32 kaveri kernel: [ 97.203682] RBP: R08: 8803de2e74b0 R09: Jul 16 17:35:32 kaveri kernel: [ 97.203686] R10: R11: R12: Jul 16 17:35:32 kaveri kernel: [ 97.203691] R13: ed0068cb2d39 R14: 880346596780 R15: dc00 Jul 16 17:35:32 kaveri kernel: [ 97.203698] FS: 7fca13089700() GS:8803ee28() knlGS: Jul 16 17:35:32 kaveri kernel: [ 97.203703] CS: 0010 DS: ES: CR0: 80050033 Jul 16 17:35:32 kaveri kernel: [ 97.203708] CR2: ff30 CR3: 0003cf8f4000 CR4: 003406e0 Jul 16 17:35:32 kaveri kernel: [ 97.203712] Call Trace: Jul 16 17:35:32 kaveri kernel: [ 97.203843] amdgpu_job_free+0xf/0x80 [amdgpu] Jul 16 17:35:32 kaveri kernel: [ 97.203947] amdgpu_cs_ioctl+0x8dd/0x4e60 [amdgpu] Jul 16 17:35:32 kaveri kernel: [ 97.204058] ? amdgpu_bo_list_ioctl+0x2d1/0x3e0 [amdgpu] Jul 16 17:35:32 kaveri kernel: [ 97.204160] ? amdgpu_cs_find_mapping+0x3c0/0x3c0 [amdgpu] Jul 16 17:35:32 kaveri kernel: [ 97.204170] ? do_vfs_ioctl+0x192/0xf30 Jul 16 17:35:32 kaveri kernel: [ 97.204176] ? ksys_ioctl+0x70/0x80 Jul 16 17:35:32 kaveri kernel: [ 97.204182] ? __x64_sys_ioctl+0x6f/0xb0 Jul 16 17:35:32 kaveri kernel: [ 97.204189] ? do_syscall_64+0xa5/0x3f0 Jul 16 17:35:32 kaveri kernel: [ 97.204199] ? __lock_acquire+0x605/0x3670 Jul 16 17:35:32 kaveri kernel: [ 97.204207] ? idr_get_free+0x4bc/0x980 Jul 16 17:35:32 kaveri kernel: [ 97.204215] ? find_held_lock+0x32/0x1c0 Jul 16 17:35:32 kaveri kernel: [ 97.204228] ? debug_check_no_locks_freed+0x2c0/0x2c0 Jul 16 17:35:32 kaveri kernel: [ 97.204235] ? __fprop_inc_percpu_max+0x1d0/0x1d0 Jul 16 17:35:32 kaveri kernel: [ 97.204361] ? amdgpu_cs_find_mapping+0x3c0/0x3c0 [amdgpu] Jul 16 17:35:32 kaveri kernel: [ 97.204391] drm_ioctl_kernel+0x197/0x220 [drm] Jul 16 17:35:32 kaveri kernel: [ 97.204422] ? drm_setversion+0x7d0/0x7d0 [drm] Jul 16 17:35:32 kaveri kernel: [ 97.204432] ? __check_object_size+0x149/0x360 Jul 16 17:35:32 kaveri kernel: [ 97.204464] drm_ioctl+0x60a/0x970 [drm] Jul 16 17:35:32 kaveri kernel: [ 97.204568] ? amdgpu_cs_find_mapping+0x3c0/0x3c0 [amdgpu] Jul 16 17:35:32 kaveri kernel: [ 97.204599] ? drm_ioctl_kernel+0x220/0x220 [drm] Jul 16 17:35:32 kaveri kernel: [ 97.204612] ? lock_downgrade+0x5e0/0x5e0 Jul 16 17:35:32 kaveri kernel: [ 97.204621] ? _raw_spin_unlock_irqrestore+0x32/0x60 Jul 16 17:35:32 kaveri kernel: [ 97.204629] ? trace_hardirqs_on_caller+0x381/0x570 Jul 16 17:35:32 kaveri kernel: [ 97.204729] amdgpu_drm_ioctl+0xcc/0x1a0 [amdgpu] Jul 16 17:35:32 kaveri kernel: [ 97.204739] do_vfs_ioctl+0x192/0xf30 Jul 16 17:35:32 kaveri kernel: [ 97.204747] ? find_held_lock+0x32/0x1c0 Jul 16 17:35:32 kaveri kernel: [ 97.204754] ? ioctl_preallocate+0x1b0/0x1b0 Jul 16 17:35:32 kaveri kernel: [ 97.204763] ? __fget+
Re: [PATCH libdrm] amdgpu: add amdgpu_bo_handle_type_kms_noimport
On 2018-07-13 08:47 PM, Marek Olšák wrote: > On Fri, Jul 13, 2018 at 4:28 AM, Michel Dänzer wrote: >> On 2018-07-12 07:03 PM, Marek Olšák wrote: >>> On Thu, Jul 12, 2018, 3:31 AM Michel Dänzer wrote: >>>> >>>> What is the rationale for this? I.e. why do you want to not store some >>>> handles in the hash table? >>> >>> >>> Because I have the option. >> >> Seems like you're expecting this patch to be accepted without providing >> any real justification for it (here or in the corresponding Mesa patch). >> NAK from me if so. > > The real justification is implied by the patch. See: > amdgpu_add_handle_to_table > Like I said: There is no risk of regression and it simplifies one > simple case trivially. We shouldn't have to even talk about it. IMO you haven't provided enough justification for adding API which is prone to breakage if used incorrectly. Other opinions? >> I'd rather add the handle to the hash table in amdgpu_bo_alloc, >> amdgpu_create_bo_from_user_mem and amdgpu_bo_import instead of in >> amdgpu_bo_export, making amdgpu_bo_export(bo, amdgpu_bo_handle_type_kms, >> ...) essentially free. In the unlikely (since allocating a BO from the >> kernel is expensive) case that the hash table shows up on profiles, we >> can optimize it. > > The hash table isn't very good for high BO counts. The time complexity > of a lookup is O(n). A lookup is only needed in amdgpu_bo_import. amdgpu_bo_alloc and amdgpu_create_bo_from_user_mem can just add the handle to the hash bucket directly. Do you know of, or can you imagine, any workload where amdgpu_bo_import is called often enough for this to be a concern? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu] glamor: Set AMDGPU_CREATE_PIXMAP_DRI2 for DRI3 pixmaps
From: Michel Dänzer Not doing this resulted in falling back to software for DRI3 client presentation operations with ShadowPrimary. Signed-off-by: Michel Dänzer --- src/amdgpu_dri3.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/amdgpu_dri3.c b/src/amdgpu_dri3.c index 87e3d85c1..84a03dc36 100644 --- a/src/amdgpu_dri3.c +++ b/src/amdgpu_dri3.c @@ -168,6 +168,7 @@ static PixmapPtr amdgpu_dri3_pixmap_from_fd(ScreenPtr screen, if (priv) { amdgpu_set_pixmap_private(pixmap, priv); + pixmap->usage_hint |= AMDGPU_CREATE_PIXMAP_DRI2; return pixmap; } -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Don't warn on destroying a pinned BO
On 2018-07-24 02:47 PM, Christian König wrote: > Am 23.07.2018 um 18:50 schrieb Michel Dänzer: >> On 2018-07-19 05:39 PM, Michel Dänzer wrote: >>> From: Michel Dänzer >>> >>> The warning turned out to be not so useful, as BO destruction tends to >>> be deferred to a workqueue. >>> >>> Also, we should be preventing any damage from this now, so not really >>> important anymore to fix code doing this. >>> >>> Signed-off-by: Michel Dänzer >>> --- >>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>> index b12526ce1a9d..3010f0136de9 100644 >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c >>> @@ -89,7 +89,7 @@ static void amdgpu_ttm_bo_destroy(struct >>> ttm_buffer_object *tbo) >>> struct amdgpu_device *adev = amdgpu_ttm_adev(tbo->bdev); >>> struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo); >>> - if (WARN_ON_ONCE(bo->pin_count > 0)) >>> + if (bo->pin_count > 0) >>> amdgpu_bo_subtract_pin_size(bo); >>> if (bo->kfd_bo) >>> >> Any feedback? > > I'm a bit torn on that. On the one hand the backtrace at this point is > not very useful, but on the other hand it would still be nice to have a > warning. > > Maybe reduce it to a DRM_ERROR()? I don't see that being useful; we'd still get reports about the errors, but couldn't do anything about them. Anyway, since the patch was on the list since last Thursday, I pushed it this morning with Alex's review (thanks!). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati] Use strcpy for RandR output property names
From: Michel Dänzer Instead of strncpy with the string length. Avoids new warnings with GCC 8: ../../src/drmmode_display.c: In function ‘drmmode_output_create_resources’: ../../src/drmmode_display.c:2240:2: warning: ‘strncpy’ output truncated before terminating nul copying 8 bytes from a string of the same length [-Wstringop-truncation] strncpy(tearfree_prop->name, "TearFree", 8); ^~~ ../../src/drmmode_display.c:2244:2: warning: ‘strncpy’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation] strncpy(tearfree_prop->enums[0].name, "off", 3); ^~~ ../../src/drmmode_display.c:2245:2: warning: ‘strncpy’ output truncated before terminating nul copying 2 bytes from a string of the same length [-Wstringop-truncation] strncpy(tearfree_prop->enums[1].name, "on", 2); ^~ ../../src/drmmode_display.c:2247:2: warning: ‘strncpy’ output truncated before terminating nul copying 4 bytes from a string of the same length [-Wstringop-truncation] strncpy(tearfree_prop->enums[2].name, "auto", 4); ^~~~ (Ported from amdgpu commit f3b2ed37d683f8616a0a31ff63133ddb8fe1a4a3) Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 60c1cdc18..6a0dba2c8 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -1686,14 +1686,14 @@ drmmode_output_create_resources(xf86OutputPtr output) /* Userspace-only property for TearFree */ tearfree_prop = calloc(1, sizeof(*tearfree_prop)); tearfree_prop->flags = DRM_MODE_PROP_ENUM; -strncpy(tearfree_prop->name, "TearFree", 8); +strcpy(tearfree_prop->name, "TearFree"); tearfree_prop->count_enums = 3; tearfree_prop->enums = calloc(tearfree_prop->count_enums, sizeof(*tearfree_prop->enums)); -strncpy(tearfree_prop->enums[0].name, "off", 3); -strncpy(tearfree_prop->enums[1].name, "on", 2); +strcpy(tearfree_prop->enums[0].name, "off"); +strcpy(tearfree_prop->enums[1].name, "on"); tearfree_prop->enums[1].value = 1; -strncpy(tearfree_prop->enums[2].name, "auto", 4); +strcpy(tearfree_prop->enums[2].name, "auto"); tearfree_prop->enums[2].value = 2; drmmode_output->props[j].mode_prop = tearfree_prop; drmmode_output->props[j].value = info->tear_free; -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu 3/4] Defer vblank event handling while waiting for a pending flip
From: Michel Dänzer This is a more robust way to prevent hangs due to nested calls to drmHandleEvent. Signed-off-by: Michel Dänzer --- src/amdgpu_drm_queue.c | 124 + src/amdgpu_drm_queue.h | 1 + src/drmmode_display.c | 18 -- src/drmmode_display.h | 4 ++ 4 files changed, 131 insertions(+), 16 deletions(-) diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c index 9c0166147..d4353e703 100644 --- a/src/amdgpu_drm_queue.c +++ b/src/amdgpu_drm_queue.c @@ -40,6 +40,7 @@ struct amdgpu_drm_queue_entry { struct xorg_list list; + uint64_t usec; uint64_t id; uintptr_t seq; void *data; @@ -47,13 +48,27 @@ struct amdgpu_drm_queue_entry { xf86CrtcPtr crtc; amdgpu_drm_handler_proc handler; amdgpu_drm_abort_proc abort; + unsigned int frame; }; static int amdgpu_drm_queue_refcnt; static struct xorg_list amdgpu_drm_queue; +static struct xorg_list amdgpu_drm_queue_deferred; static uintptr_t amdgpu_drm_queue_seq; +static void +amdgpu_drm_queue_handle_one(struct amdgpu_drm_queue_entry *e, unsigned int frame, + uint64_t usec) +{ + xorg_list_del(>list); + if (e->handler) { + e->handler(e->crtc, frame, usec, e->data); + } else + e->abort(e->crtc, e->data); + free(e); +} + /* * Handle a DRM event */ @@ -66,19 +81,80 @@ amdgpu_drm_queue_handler(int fd, unsigned int frame, unsigned int sec, xorg_list_for_each_entry_safe(e, tmp, _drm_queue, list) { if (e->seq == seq) { - xorg_list_del(>list); - if (e->handler) - e->handler(e->crtc, frame, - (uint64_t)sec * 100 + usec, - e->data); - else - e->abort(e->crtc, e->data); - free(e); + amdgpu_drm_queue_handle_one(e, frame, (uint64_t)sec * + 100 + usec); break; } } } +/* + * Handle a DRM vblank event + */ +static void +amdgpu_drm_queue_vblank_handler(int fd, unsigned int frame, unsigned int sec, + unsigned int usec, void *user_ptr) +{ + uintptr_t seq = (uintptr_t)user_ptr; + struct amdgpu_drm_queue_entry *e, *tmp; + + xorg_list_for_each_entry_safe(e, tmp, _drm_queue, list) { + if (e->seq == seq) { + AMDGPUInfoPtr info = AMDGPUPTR(e->crtc->scrn); + drmmode_ptr drmmode = >drmmode; + + e->usec = (uint64_t)sec * 100 + usec; + + if (drmmode->event_context.vblank_handler == + amdgpu_drm_queue_vblank_handler) { + amdgpu_drm_queue_handle_one(e, frame, e->usec); + } else { + xorg_list_del(>list); + e->frame = frame; + xorg_list_append(>list, +_drm_queue_deferred); + } + + break; + } + } +} + +/* + * Re-queue a DRM vblank event for deferred handling + */ +static void +amdgpu_drm_queue_vblank_defer(int fd, unsigned int frame, unsigned int sec, + unsigned int usec, void *user_ptr) +{ + amdgpu_drm_queue_vblank_handler(fd, frame, sec, usec, user_ptr); +} + +/* + * Handle deferred DRM vblank events + * + * This function must be called after amdgpu_drm_wait_pending_flip, once + * it's safe to attempt queueing a flip again + */ +void +amdgpu_drm_queue_handle_deferred(xf86CrtcPtr crtc) +{ + drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private; + drmmode_ptr drmmode = drmmode_crtc->drmmode; + struct amdgpu_drm_queue_entry *e, *tmp; + + if (drmmode_crtc->wait_flip_nesting_level == 0 || + --drmmode_crtc->wait_flip_nesting_level > 0) + return; + + xorg_list_for_each_entry_safe(e, tmp, _drm_queue_deferred, list) { + if (e->crtc == crtc) + amdgpu_drm_queue_handle_one(e, e->frame, e->usec); + } + + drmmode->event_context.vblank_handler = amdgpu_drm_queue_vblank_handler; +} + /* * Enqueue a potential drm response; when the associated response * appears, we've got data to pass to the handler from here @@ -134,12 +210,17 @@ amdgpu_drm_abort_one(struct amdgpu_drm_queue_entry *e) void amdgpu_drm_abort_client(ClientPtr client) { - struct amdgpu_drm_queue_entry *e; + struct amdgpu_drm_queue_entry
[PATCH xf86-video-amdgpu 2/4] Add amdgpu_drm_wait_pending_flip function
From: Michel Dänzer Replacing the drmmode_crtc_wait_pending_event macro. Signed-off-by: Michel Dänzer --- src/amdgpu_drm_queue.c | 13 + src/amdgpu_drm_queue.h | 1 + src/drmmode_display.c | 22 -- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c index dfe0148c8..9c0166147 100644 --- a/src/amdgpu_drm_queue.c +++ b/src/amdgpu_drm_queue.c @@ -177,6 +177,19 @@ amdgpu_drm_abort_id(uint64_t id) } } +/* + * Wait for pending page flip on given CRTC to complete + */ +void amdgpu_drm_wait_pending_flip(xf86CrtcPtr crtc) +{ + drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private; + AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn); + drmmode_ptr drmmode = drmmode_crtc->drmmode; + + while (drmmode_crtc->flip_pending && + drmHandleEvent(pAMDGPUEnt->fd, >event_context) > 0); +} + /* * Initialize the DRM event queue */ diff --git a/src/amdgpu_drm_queue.h b/src/amdgpu_drm_queue.h index 328b5e6b1..cb6d5ff59 100644 --- a/src/amdgpu_drm_queue.h +++ b/src/amdgpu_drm_queue.h @@ -49,6 +49,7 @@ uintptr_t amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client, void amdgpu_drm_abort_client(ClientPtr client); void amdgpu_drm_abort_entry(uintptr_t seq); void amdgpu_drm_abort_id(uint64_t id); +void amdgpu_drm_wait_pending_flip(xf86CrtcPtr crtc); void amdgpu_drm_queue_init(ScrnInfoPtr scrn); void amdgpu_drm_queue_close(ScrnInfoPtr scrn); diff --git a/src/drmmode_display.c b/src/drmmode_display.c index c45b79d21..615d3be8f 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -97,13 +97,6 @@ AMDGPUZaphodStringMatches(ScrnInfoPtr pScrn, const char *s, char *output_name) } -/* Wait for the boolean condition to be FALSE */ -#define drmmode_crtc_wait_pending_event(drmmode_crtc, fd, condition) \ - do {} while ((condition) && \ -drmHandleEvent(fd, _crtc->drmmode->event_context) \ -> 0); - - static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn, int width, int height, int depth, int bpp, @@ -287,8 +280,7 @@ drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode) if (drmmode_crtc->dpms_mode == DPMSModeOn && mode != DPMSModeOn) { uint32_t seq; - drmmode_crtc_wait_pending_event(drmmode_crtc, pAMDGPUEnt->fd, - drmmode_crtc->flip_pending); + amdgpu_drm_wait_pending_flip(crtc); /* * On->Off transition: record the last vblank time, @@ -1361,8 +1353,7 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode, goto done; } - drmmode_crtc_wait_pending_event(drmmode_crtc, pAMDGPUEnt->fd, - drmmode_crtc->flip_pending); + amdgpu_drm_wait_pending_flip(crtc); if (!drmmode_set_mode(crtc, fb, mode, x, y)) goto done; @@ -2329,15 +2320,11 @@ drmmode_output_set_tear_free(AMDGPUEntPtr pAMDGPUEnt, drmmode_output->tear_free = tear_free; if (crtc) { - drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private; - /* Wait for pending flips before drmmode_set_mode_major calls * drmmode_crtc_update_tear_free, to prevent a nested * drmHandleEvent call, which would hang */ - drmmode_crtc_wait_pending_event(drmmode_crtc, - pAMDGPUEnt->fd, - drmmode_crtc->flip_pending); + amdgpu_drm_wait_pending_flip(crtc); drmmode_set_mode_major(crtc, >mode, crtc->rotation, crtc->x, crtc->y); } @@ -3954,8 +3941,7 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client, amdgpu_glamor_flush(crtc->scrn); if (drmmode_crtc->scanout_update_pending) { - drmmode_crtc_wait_pending_event(drmmode_crtc, pAMDGPUEnt->fd, - drmmode_crtc->flip_pending); + amdgpu_drm_wait_pending_flip(crtc); amdgpu_drm_abort_entry(drmmode_crtc->scanout_update_pending); drmmode_crtc->scanout_update_pending = 0; } -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu 1/4] Move DRM event queue related initialization to amdgpu_drm_queue_init
From: Michel Dänzer And make amdgpu_drm_queue_handler not directly accessible outside of amdgpu_drm_queue.c. Signed-off-by: Michel Dänzer --- src/amdgpu_dri2.c | 14 -- src/amdgpu_drm_queue.c | 11 +-- src/amdgpu_drm_queue.h | 5 + src/amdgpu_kms.c | 2 +- src/drmmode_display.c | 4 5 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/amdgpu_dri2.c b/src/amdgpu_dri2.c index a9238e536..96b2d1760 100644 --- a/src/amdgpu_dri2.c +++ b/src/amdgpu_dri2.c @@ -873,13 +873,15 @@ CARD32 amdgpu_dri2_deferred_event(OsTimerPtr timer, CARD32 now, pointer data) scrn = crtc->scrn; pAMDGPUEnt = AMDGPUEntPriv(scrn); + drmmode_crtc = event_info->crtc->driver_private; ret = drmmode_get_current_ust(pAMDGPUEnt->fd, _now); if (ret) { xf86DrvMsg(scrn->scrnIndex, X_ERROR, "%s cannot get current time\n", __func__); if (event_info->drm_queue_seq) - amdgpu_drm_queue_handler(pAMDGPUEnt->fd, 0, 0, 0, - (void*)event_info->drm_queue_seq); + drmmode_crtc->drmmode->event_context. + vblank_handler(pAMDGPUEnt->fd, 0, 0, 0, + (void*)event_info->drm_queue_seq); else amdgpu_dri2_frame_event_handler(crtc, 0, 0, data); return 0; @@ -888,15 +890,15 @@ CARD32 amdgpu_dri2_deferred_event(OsTimerPtr timer, CARD32 now, pointer data) * calculate the frame number from current time * that would come from CRTC if it were running */ - drmmode_crtc = event_info->crtc->driver_private; delta_t = drm_now - (CARD64) drmmode_crtc->dpms_last_ust; delta_seq = delta_t * drmmode_crtc->dpms_last_fps; delta_seq /= 100; frame = (CARD64) drmmode_crtc->dpms_last_seq + delta_seq; if (event_info->drm_queue_seq) - amdgpu_drm_queue_handler(pAMDGPUEnt->fd, frame, drm_now / 100, -drm_now % 100, -(void*)event_info->drm_queue_seq); + drmmode_crtc->drmmode->event_context. + vblank_handler(pAMDGPUEnt->fd, frame, drm_now / 100, + drm_now % 100, + (void*)event_info->drm_queue_seq); else amdgpu_dri2_frame_event_handler(crtc, frame, drm_now, data); return 0; diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c index d1456ca84..dfe0148c8 100644 --- a/src/amdgpu_drm_queue.c +++ b/src/amdgpu_drm_queue.c @@ -57,7 +57,7 @@ static uintptr_t amdgpu_drm_queue_seq; /* * Handle a DRM event */ -void +static void amdgpu_drm_queue_handler(int fd, unsigned int frame, unsigned int sec, unsigned int usec, void *user_ptr) { @@ -181,8 +181,15 @@ amdgpu_drm_abort_id(uint64_t id) * Initialize the DRM event queue */ void -amdgpu_drm_queue_init() +amdgpu_drm_queue_init(ScrnInfoPtr scrn) { + AMDGPUInfoPtr info = AMDGPUPTR(scrn); + drmmode_ptr drmmode = >drmmode; + + drmmode->event_context.version = 2; + drmmode->event_context.vblank_handler = amdgpu_drm_queue_handler; + drmmode->event_context.page_flip_handler = amdgpu_drm_queue_handler; + if (amdgpu_drm_queue_refcnt++) return; diff --git a/src/amdgpu_drm_queue.h b/src/amdgpu_drm_queue.h index 36ee900aa..328b5e6b1 100644 --- a/src/amdgpu_drm_queue.h +++ b/src/amdgpu_drm_queue.h @@ -42,9 +42,6 @@ typedef void (*amdgpu_drm_handler_proc)(xf86CrtcPtr crtc, uint32_t seq, uint64_t usec, void *data); typedef void (*amdgpu_drm_abort_proc)(xf86CrtcPtr crtc, void *data); -void amdgpu_drm_queue_handler(int fd, unsigned int frame, - unsigned int tv_sec, unsigned int tv_usec, - void *user_ptr); uintptr_t amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client, uint64_t id, void *data, amdgpu_drm_handler_proc handler, @@ -52,7 +49,7 @@ uintptr_t amdgpu_drm_queue_alloc(xf86CrtcPtr crtc, ClientPtr client, void amdgpu_drm_abort_client(ClientPtr client); void amdgpu_drm_abort_entry(uintptr_t seq); void amdgpu_drm_abort_id(uint64_t id); -void amdgpu_drm_queue_init(); +void amdgpu_drm_queue_init(ScrnInfoPtr scrn); void amdgpu_drm_queue_close(ScrnInfoPtr scrn); #endif /* _AMDGPU_DRM_QUEUE_H_ */ diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c index 7b13d777e..e2925cc83 100644 --- a/src/amdgpu_kms.c +++ b/src/amdgpu_kms.c @@ -1407,7 +1407,7 @@ Bool AMDGPUPreInit_KMS(ScrnInfoPtr pScrn, int flags) if
Re: [PATCH] drm/amdgpu: Fix RLC safe mode test in gfx_v9_0_enter_rlc_safe_mode
On 2018-07-24 07:20 PM, Paul Menzel wrote: > On 07/20/18 18:33, Michel Dänzer wrote: >> From: Michel Dänzer >> >> We were testing the register offset, instead of the value stored in the >> register, therefore always timing out the loop. >> >> This reduces suspend time of the system in the bug report below by ~600 >> ms. >> >> Bugzilla: https://bugs.freedesktop.org/107277 >> Tested-by: Paul Menzel >> Signed-off-by: Michel Dänzer > > […] > > As this line is present since v4.12-rc1, could you please tag this to be > picked up for the stable series (4.14.x)? I added the Cc: stable tag before pushing to the internal amd-staging-drm-next branch. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH] drm/amdgpu/display: Replace CONFIG_DRM_AMD_DC_DCN1_0 with CONFIG_X86
From: Michel Dänzer Allowing CONFIG_DRM_AMD_DC_DCN1_0 to be disabled on X86 was an opportunity for display with Raven Ridge accidentally not working. Signed-off-by: Michel Dänzer --- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 2 +- drivers/gpu/drm/amd/display/Kconfig | 8 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 drivers/gpu/drm/amd/display/dc/Makefile | 2 +- .../display/dc/bios/command_table_helper2.c | 2 +- drivers/gpu/drm/amd/display/dc/calcs/Makefile | 2 +- drivers/gpu/drm/amd/display/dc/core/dc.c | 6 +++--- .../gpu/drm/amd/display/dc/core/dc_debug.c| 2 +- .../gpu/drm/amd/display/dc/core/dc_resource.c | 12 +-- drivers/gpu/drm/amd/display/dc/dc.h | 2 +- .../drm/amd/display/dc/dce/dce_clock_source.c | 6 +++--- .../drm/amd/display/dc/dce/dce_clock_source.h | 2 +- .../gpu/drm/amd/display/dc/dce/dce_clocks.c | 8 .../gpu/drm/amd/display/dc/dce/dce_clocks.h | 2 +- drivers/gpu/drm/amd/display/dc/dce/dce_dmcu.c | 6 +++--- .../amd/display/dc/dce/dce_stream_encoder.c | 20 +-- .../display/dc/dce110/dce110_hw_sequencer.c | 2 +- drivers/gpu/drm/amd/display/dc/gpio/Makefile | 2 +- .../gpu/drm/amd/display/dc/gpio/hw_factory.c | 4 ++-- .../drm/amd/display/dc/gpio/hw_translate.c| 4 ++-- .../gpu/drm/amd/display/dc/i2caux/Makefile| 2 +- .../gpu/drm/amd/display/dc/i2caux/i2caux.c| 4 ++-- .../gpu/drm/amd/display/dc/inc/core_types.h | 6 +++--- drivers/gpu/drm/amd/display/dc/irq/Makefile | 2 +- .../gpu/drm/amd/display/dc/irq/irq_service.c | 2 +- drivers/gpu/drm/amd/display/dc/os_types.h | 2 +- 26 files changed, 56 insertions(+), 64 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c index 709e4a386a0e..fb8c72851dfb 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c @@ -2200,7 +2200,7 @@ bool amdgpu_device_asic_has_dc_support(enum amd_asic_type asic_type) case CHIP_VEGA10: case CHIP_VEGA12: case CHIP_VEGA20: -#if defined(CONFIG_DRM_AMD_DC_DCN1_0) +#ifdef CONFIG_X86 case CHIP_RAVEN: #endif return amdgpu_dc != 0; diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig index 4c35625eb2c7..325083b0297e 100644 --- a/drivers/gpu/drm/amd/display/Kconfig +++ b/drivers/gpu/drm/amd/display/Kconfig @@ -9,14 +9,6 @@ config DRM_AMD_DC support for AMDGPU. This adds required support for Vega and Raven ASICs. -config DRM_AMD_DC_DCN1_0 - bool "DCN 1.0 Raven family" - depends on DRM_AMD_DC && X86 - default y - help - Choose this option if you want to have - RV family for display engine - config DEBUG_KERNEL_DC bool "Enable kgdb break in DC" depends on DRM_AMD_DC diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 952691c6f81e..8e3ebd988043 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -58,7 +58,7 @@ #include #include -#if defined(CONFIG_DRM_AMD_DC_DCN1_0) +#ifdef CONFIG_X86 #include "ivsrcid/irqsrcs_dcn_1_0.h" #include "dcn/dcn_1_0_offset.h" @@ -1188,7 +1188,7 @@ static int dce110_register_irq_handlers(struct amdgpu_device *adev) return 0; } -#if defined(CONFIG_DRM_AMD_DC_DCN1_0) +#ifdef CONFIG_X86 /* Register IRQ sources and initialize IRQ callbacks */ static int dcn10_register_irq_handlers(struct amdgpu_device *adev) { @@ -1522,7 +1522,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev) goto fail; } break; -#if defined(CONFIG_DRM_AMD_DC_DCN1_0) +#ifdef CONFIG_X86 case CHIP_RAVEN: if (dcn10_register_irq_handlers(dm->adev)) { DRM_ERROR("DM: Failed to initialize IRQ\n"); @@ -1767,7 +1767,7 @@ static int dm_early_init(void *handle) adev->mode_info.num_dig = 6; adev->mode_info.plane_type = dm_plane_type_default; break; -#if defined(CONFIG_DRM_AMD_DC_DCN1_0) +#ifdef CONFIG_X86 case CHIP_RAVEN: adev->mode_info.num_crtc = 4; adev->mode_info.num_hpd = 4; diff --git a/drivers/gpu/drm/amd/display/dc/Makefile b/drivers/gpu/drm/amd/display/dc/Makefile index aed538a4d1ba..532a515fda9a 100644 --- a/drivers/gpu/drm/amd/display/dc/Makefile +++ b/drivers/gpu/drm/amd/display/dc/Makefile @@ -25,7 +25,7 @@ DC_LIBS = basics bios calcs dce gpio i2caux irq virtual -ifdef CONFIG_DRM_AMD_DC_DCN1_0 +ifdef CONFIG_X86 DC_LIBS += dcn10 dml endif diff --git a/drivers/gpu/drm/amd/display/dc/bios/command_table_helper2.c b/drivers/gpu/drm/amd/displa
[PATCH xf86-video-ati] glamor: Invalidate cached GEM handle in radeon_set_pixmap_bo
From: Michel Dänzer We continued using the stale cached handle, causing issues e.g. when resizing the screen via RandR. Reported-by: iive on IRC Signed-off-by: Michel Dänzer --- src/radeon.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/radeon.h b/src/radeon.h index 450c69aa8..1a1edb1ba 100644 --- a/src/radeon.h +++ b/src/radeon.h @@ -735,6 +735,7 @@ static inline Bool radeon_set_pixmap_bo(PixmapPtr pPix, struct radeon_buffer *bo return TRUE; radeon_buffer_unref(>bo); + priv->handle_valid = FALSE; } drmmode_fb_reference(pRADEONEnt->fd, >fb, NULL); -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH libdrm] amdgpu: make sure to set CLOEXEC on duplicated FDs
On 2018-07-17 11:04 AM, Christian König wrote: > Otherwise we leak file descriptors into child processes. > > Signed-off-by: Christian König > --- > amdgpu/amdgpu_device.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/amdgpu/amdgpu_device.c b/amdgpu/amdgpu_device.c > index 34ac95b8..d7aec6a4 100644 > --- a/amdgpu/amdgpu_device.c > +++ b/amdgpu/amdgpu_device.c > @@ -34,6 +34,7 @@ > #include > #include > #include > +#include > > #include "xf86drm.h" > #include "amdgpu_drm.h" > @@ -205,7 +206,7 @@ int amdgpu_device_initialize(int fd, > return r; > } > if ((flag_auth) && (!flag_authexist)) { > - dev->flink_fd = dup(fd); > + dev->flink_fd = fcntl(fd, F_DUPFD_CLOEXEC, 0); > } > *major_version = dev->major_version; > *minor_version = dev->minor_version; > @@ -239,7 +240,7 @@ int amdgpu_device_initialize(int fd, > goto cleanup; > } > > - dev->fd = dup(fd); > + dev->fd = fcntl(fd, F_DUPFD_CLOEXEC, 0); > dev->flink_fd = dev->fd; > dev->major_version = version->version_major; > dev->minor_version = version->version_minor; > Nice catch. Reviewed-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu 2/2] glamor: Handle ihandle == -1 in amdgpu_glamor_set_shared_pixmap_backing
From: Michel Dänzer (Ported from radeon commit de88ea2755611bdcb18d91d8234d2ab5be8ff2e9) Signed-off-by: Michel Dänzer --- src/amdgpu_glamor.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/amdgpu_glamor.c b/src/amdgpu_glamor.c index 13d68fe36..699861f73 100644 --- a/src/amdgpu_glamor.c +++ b/src/amdgpu_glamor.c @@ -384,6 +384,7 @@ amdgpu_glamor_set_shared_pixmap_backing(PixmapPtr pixmap, void *handle) { ScreenPtr screen = pixmap->drawable.pScreen; ScrnInfoPtr scrn = xf86ScreenToScrn(screen); + int ihandle = (int)(long)handle; struct amdgpu_pixmap *priv; if (!amdgpu_set_shared_pixmap_backing(pixmap, handle)) @@ -391,7 +392,8 @@ amdgpu_glamor_set_shared_pixmap_backing(PixmapPtr pixmap, void *handle) priv = amdgpu_get_pixmap_private(pixmap); - if (!amdgpu_glamor_create_textured_pixmap(pixmap, priv->bo)) { + if (ihandle != -1 && + !amdgpu_glamor_create_textured_pixmap(pixmap, priv->bo)) { xf86DrvMsg(scrn->scrnIndex, X_ERROR, "Failed to get PRIME drawable for glamor pixmap.\n"); return FALSE; -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH v5 5/5] drm/amdgpu: move PD/PT bos on LRU again
On 2018-08-29 10:57 a.m., Christian König wrote: > Am 29.08.2018 um 09:52 schrieb Michel Dänzer: >> On 2018-08-28 7:03 p.m., Michel Dänzer wrote: >>> On 2018-08-28 11:14 a.m., Michel Dänzer wrote: >>>> On 2018-08-22 9:52 a.m., Huang Rui wrote: >>>>> The new bulk moving functionality is ready, the overhead of moving >>>>> PD/PT bos to >>>>> LRU is fixed. So move them on LRU again. >>>>> >>>>> Signed-off-by: Huang Rui >>>>> Tested-by: Mike Lothian >>>>> Tested-by: Dieter Nützel >>>>> Acked-by: Chunming Zhou >>>>> Reviewed-by: Junwei Zhang >>>>> --- >>>>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> index db1f28a..d195a3d 100644 >>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c >>>>> @@ -1107,7 +1107,7 @@ int amdgpu_vm_update_directories(struct >>>>> amdgpu_device *adev, >>>>> struct amdgpu_vm_bo_base, >>>>> vm_status); >>>>> bo_base->moved = false; >>>>> - list_del_init(_base->vm_status); >>>>> + list_move(_base->vm_status, >idle); >>>>> bo = bo_base->bo->parent; >>>>> if (!bo) >>>>> >>>> Since this change, I'm getting various badness when running piglit >>>> using >>>> radeonsi on Bonaire, see the attached dmesg excerpt. >>>> >>>> Reverting just this change on top of current amd-staging-drm-next >>>> avoids >>>> the problem. >>>> >>>> Looks like some list manipulation isn't sufficiently protected against >>>> concurrent execution? >>> KASAN pointed me to one issue: >>> https://patchwork.freedesktop.org/patch/246212/ >>> >>> However, this doesn't fully fix the problem. >> Ray, any ideas yet for solving this? If not, let's revert this change >> for now. > > I've gone over this multiple times now as well, but can't find anything > obvious wrong either. After looking at the code, one question: Why does vm->moved need a spinlock, but not vm->idle? What is protecting against concurrent access to the latter? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu 1/2] Handle ihandle == -1 in amdgpu_set_shared_pixmap_backing
From: Michel Dänzer It means to stop using the shared pixmap backing. (Ported from radeon commit 1799680f7bd84e0618f34f4c7486799521ddaf83) Signed-off-by: Michel Dänzer --- src/amdgpu_bo_helper.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/amdgpu_bo_helper.c b/src/amdgpu_bo_helper.c index 98eb04a64..6fd68846f 100644 --- a/src/amdgpu_bo_helper.c +++ b/src/amdgpu_bo_helper.c @@ -400,6 +400,9 @@ Bool amdgpu_set_shared_pixmap_backing(PixmapPtr ppix, void *fd_handle) uint32_t size = ppix->devKind * ppix->drawable.height; Bool ret; + if (ihandle == -1) + return amdgpu_set_pixmap_bo(ppix, NULL); + if (info->gbm) { struct amdgpu_buffer *bo; struct gbm_import_fd_data data; -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu 2/3] Don't use xorg_list_for_each_entry_safe for signalled flips
From: Michel Dänzer drm_wait_pending_flip can get called from drm_handle_event, in which case xorg_list_for_each_entry_safe can end up processing the same entry in both. To avoid this, just process the first list entry until the list is empty. Signed-off-by: Michel Dänzer --- src/amdgpu_drm_queue.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c index f9db81c08..ba841d1ef 100644 --- a/src/amdgpu_drm_queue.c +++ b/src/amdgpu_drm_queue.c @@ -257,8 +257,11 @@ amdgpu_drm_handle_event(int fd, drmEventContext *event_context) r = drmHandleEvent(fd, event_context); - xorg_list_for_each_entry_safe(e, tmp, _drm_flip_signalled, list) + while (!xorg_list_is_empty(_drm_flip_signalled)) { + e = xorg_list_first_entry(_drm_flip_signalled, + struct amdgpu_drm_queue_entry, list); amdgpu_drm_queue_handle_one(e); + } xorg_list_for_each_entry_safe(e, tmp, _drm_vblank_signalled, list) { drmmode_crtc_private_ptr drmmode_crtc = e->crtc->driver_private; @@ -277,12 +280,15 @@ void amdgpu_drm_wait_pending_flip(xf86CrtcPtr crtc) { drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private; AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(crtc->scrn); - struct amdgpu_drm_queue_entry *e, *tmp; + struct amdgpu_drm_queue_entry *e; drmmode_crtc->wait_flip_nesting_level++; - xorg_list_for_each_entry_safe(e, tmp, _drm_flip_signalled, list) + while (!xorg_list_is_empty(_drm_flip_signalled)) { + e = xorg_list_first_entry(_drm_flip_signalled, + struct amdgpu_drm_queue_entry, list); amdgpu_drm_queue_handle_one(e); + } while (drmmode_crtc->flip_pending && amdgpu_drm_handle_event(pAMDGPUEnt->fd, -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu 1/3] Always delete entry from list in drm_queue_handler
From: Michel Dänzer We left entries without a handler hook in the list, so the list could keep taking longer to process and use up more memory. Signed-off-by: Michel Dänzer --- src/amdgpu_drm_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c index b13d28014..f9db81c08 100644 --- a/src/amdgpu_drm_queue.c +++ b/src/amdgpu_drm_queue.c @@ -82,7 +82,7 @@ amdgpu_drm_queue_handler(struct xorg_list *signalled, unsigned int frame, xorg_list_for_each_entry_safe(e, tmp, _drm_queue, list) { if (e->seq == seq) { if (!e->handler) { - e->abort(e->crtc, e->data); + amdgpu_drm_queue_handle_one(e); break; } -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu 3/3] Bail early from drm_wait_pending_flip if there's no pending flip
From: Michel Dänzer No need to process any events in that case. Signed-off-by: Michel Dänzer --- src/amdgpu_drm_queue.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c index ba841d1ef..61732b11c 100644 --- a/src/amdgpu_drm_queue.c +++ b/src/amdgpu_drm_queue.c @@ -284,6 +284,9 @@ void amdgpu_drm_wait_pending_flip(xf86CrtcPtr crtc) drmmode_crtc->wait_flip_nesting_level++; + if (!drmmode_crtc->flip_pending) + return; + while (!xorg_list_is_empty(_drm_flip_signalled)) { e = xorg_list_first_entry(_drm_flip_signalled, struct amdgpu_drm_queue_entry, list); -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 1/3] drm/ttm: fix ttm_bo_bulk_move_helper
On 2018-08-31 3:10 p.m., Christian König wrote: > Staring at the function for six hours, just to essentially move one line > of code. That sucks, but the commit log should describe what the problem was and how this patch solves it. > Signed-off-by: Christian König > --- > drivers/gpu/drm/ttm/ttm_bo.c | 13 - > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > index 35d53d81f486..138c98902033 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo.c > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > @@ -250,15 +250,18 @@ EXPORT_SYMBOL(ttm_bo_move_to_lru_tail); > static void ttm_bo_bulk_move_helper(struct ttm_lru_bulk_move_pos *pos, > struct list_head *lru, bool is_swap) > { > + struct list_head *list; > LIST_HEAD(entries); > LIST_HEAD(before); > - struct list_head *list1, *list2; > > - list1 = is_swap ? >last->swap : >last->lru; > - list2 = is_swap ? pos->first->swap.prev : pos->first->lru.prev; > + reservation_object_assert_held(pos->last->resv); > + list = is_swap ? >last->swap : >last->lru; > + list_cut_position(, lru, list); > + > + reservation_object_assert_held(pos->first->resv); > + list = is_swap ? pos->first->swap.prev : pos->first->lru.prev; > + list_cut_position(, , list); So the problem was that the first list_cut_position call could result in list2 pointing to la-la-land? Good catch! -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati] Fix uninitialized use of local variable pitch in radeon_setup_kernel_mem
From: Michel Dänzer Fixes server reset. Pointed out by clang: ../../src/radeon_kms.c:2721:9: warning: variable 'pitch' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] if (!info->front_buffer) { ^~~ ../../src/radeon_kms.c:2765:27: note: uninitialized use occurs here pScrn->displayWidth = pitch / cpp; ^ ../../src/radeon_kms.c:2721:5: note: remove the 'if' if its condition is always true if (!info->front_buffer) { ^ ../../src/radeon_kms.c:2680:14: note: initialize the variable 'pitch' to silence this warning int pitch; ^ = 0 Signed-off-by: Michel Dänzer --- src/radeon_kms.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/radeon_kms.c b/src/radeon_kms.c index a24776811..ae69f3353 100644 --- a/src/radeon_kms.c +++ b/src/radeon_kms.c @@ -2760,10 +2760,11 @@ static Bool radeon_setup_kernel_mem(ScreenPtr pScreen) if (tiling_flags) radeon_bo_set_tiling(info->front_buffer->bo.radeon, tiling_flags, pitch); } -} -pScrn->displayWidth = pitch / cpp; + pScrn->displayWidth = pitch / cpp; +} +pitch = pScrn->displayWidth * cpp; xf86DrvMsg(pScrn->scrnIndex, X_INFO, "Front buffer size: %dK\n", pitch * pScrn->virtualY / 1024); radeon_kms_update_vram_limit(pScrn, pitch * pScrn->virtualY); -- 2.19.0.rc1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH v2 xf86-video-amdgpu] Bail early from drm_wait_pending_flip if there's no pending flip
From: Michel Dänzer No need to process any events in that case. v2: * Re-check drmmode_crtc->flip_pending after processing each event Signed-off-by: Michel Dänzer --- src/amdgpu_drm_queue.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/amdgpu_drm_queue.c b/src/amdgpu_drm_queue.c index ba841d1ef..035356234 100644 --- a/src/amdgpu_drm_queue.c +++ b/src/amdgpu_drm_queue.c @@ -284,7 +284,8 @@ void amdgpu_drm_wait_pending_flip(xf86CrtcPtr crtc) drmmode_crtc->wait_flip_nesting_level++; - while (!xorg_list_is_empty(_drm_flip_signalled)) { + while (drmmode_crtc->flip_pending && + !xorg_list_is_empty(_drm_flip_signalled)) { e = xorg_list_first_entry(_drm_flip_signalled, struct amdgpu_drm_queue_entry, list); amdgpu_drm_queue_handle_one(e); -- 2.19.0.rc1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati 2/2] Don't use xorg_list_for_each_entry_safe for signalled flips
From: Michel Dänzer drm_wait_pending_flip can get called from drm_handle_event, in which case xorg_list_for_each_entry_safe can end up processing the same entry in both. To avoid this, just process the first list entry until the list is empty. (Ported from amdgpu commit 26770be44b89b83bf39c28f2fe284c8cb92ed0c0) Signed-off-by: Michel Dänzer --- src/radeon_drm_queue.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/radeon_drm_queue.c b/src/radeon_drm_queue.c index 61a2f5cef..bf1650ea9 100644 --- a/src/radeon_drm_queue.c +++ b/src/radeon_drm_queue.c @@ -257,8 +257,11 @@ radeon_drm_handle_event(int fd, drmEventContext *event_context) r = drmHandleEvent(fd, event_context); -xorg_list_for_each_entry_safe(e, tmp, _drm_flip_signalled, list) +while (!xorg_list_is_empty(_drm_flip_signalled)) { + e = xorg_list_first_entry(_drm_flip_signalled, + struct radeon_drm_queue_entry, list); radeon_drm_queue_handle_one(e); +} xorg_list_for_each_entry_safe(e, tmp, _drm_vblank_signalled, list) { drmmode_crtc_private_ptr drmmode_crtc = e->crtc->driver_private; @@ -277,12 +280,15 @@ void radeon_drm_wait_pending_flip(xf86CrtcPtr crtc) { drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private; RADEONEntPtr pRADEONEnt = RADEONEntPriv(crtc->scrn); -struct radeon_drm_queue_entry *e, *tmp; +struct radeon_drm_queue_entry *e; drmmode_crtc->wait_flip_nesting_level++; -xorg_list_for_each_entry_safe(e, tmp, _drm_flip_signalled, list) +while (!xorg_list_is_empty(_drm_flip_signalled)) { + e = xorg_list_first_entry(_drm_flip_signalled, + struct radeon_drm_queue_entry, list); radeon_drm_queue_handle_one(e); +} while (drmmode_crtc->flip_pending && radeon_drm_handle_event(pRADEONEnt->fd, -- 2.19.0.rc1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati 1/2] Always delete entry from list in drm_queue_handler
From: Michel Dänzer We left entries without a handler hook in the list, so the list could keep taking longer to process and use up more memory. (Ported from amdgpu commit 7eea3e2cd74eed22e982319144e18ae5b1087b78) Signed-off-by: Michel Dänzer --- src/radeon_drm_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/radeon_drm_queue.c b/src/radeon_drm_queue.c index 857278fdd..61a2f5cef 100644 --- a/src/radeon_drm_queue.c +++ b/src/radeon_drm_queue.c @@ -82,7 +82,7 @@ radeon_drm_queue_handler(struct xorg_list *signalled, unsigned int frame, xorg_list_for_each_entry_safe(e, tmp, _drm_queue, list) { if (e->seq == seq) { if (!e->handler) { - e->abort(e->crtc, e->data); + radeon_drm_queue_handle_one(e); break; } -- 2.19.0.rc1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati] Bail early from drm_wait_pending_flip if there's no pending flip
From: Michel Dänzer No need to process any events in that case. (Ported from amdgpu commit ca5eb9894fff153c0a1df7bdc4a4745713309e27) Signed-off-by: Michel Dänzer --- src/radeon_drm_queue.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/radeon_drm_queue.c b/src/radeon_drm_queue.c index bf1650ea9..ea78e8e2b 100644 --- a/src/radeon_drm_queue.c +++ b/src/radeon_drm_queue.c @@ -284,7 +284,8 @@ void radeon_drm_wait_pending_flip(xf86CrtcPtr crtc) drmmode_crtc->wait_flip_nesting_level++; -while (!xorg_list_is_empty(_drm_flip_signalled)) { +while (drmmode_crtc->flip_pending && + !xorg_list_is_empty(_drm_flip_signalled)) { e = xorg_list_first_entry(_drm_flip_signalled, struct radeon_drm_queue_entry, list); radeon_drm_queue_handle_one(e); -- 2.19.0.rc1 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Fix wornings while make xmldocs
Hi Iida-san, On 2018-09-06 4:10 a.m., Masanari Iida wrote: > This patch fixes following wornings. > > ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:3011: > warning: Excess function parameter 'dev' description > in 'amdgpu_vm_get_task_info' > > ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:3012: > warning: Function parameter or member 'adev' not > described in 'amdgpu_vm_get_task_info' > > ./drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c:3012: > warning: Excess function parameter 'dev' description > in 'amdgpu_vm_get_task_info' > > Signed-off-by: Masanari Iida > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index b17771dd5ce7..861718e2be7a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -3002,7 +3002,7 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, > struct drm_file *filp) > /** > * amdgpu_vm_get_task_info - Extracts task info for a PASID. > * > - * @dev: drm device pointer > + * @adev: drm device pointer > * @pasid: PASID identifier for VM > * @task_info: task_info to fill. > */ > Queued in the amdgpu tree (with fixed spelling of "warnings"), thanks! -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout
On 2018-09-08 7:24 p.m., Dāvis Mosāns wrote: > Hello, > > With Radeon RX Vega 64 when launching LibreOffice (6.1.0.3) on Arch > Linux with 4.18.5 kernel then display freezes. > dmesg contains: > > [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled > seq=8322, emitted seq=8325 > [drm] GPU recovery disabled. > > I tested that it doesn't happen with 4.17.14 kernel. Can you bisect between 4.17 and 4.18? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH xf86-video-amdgpu] Add checking color management properties
Hi Aaron, thanks for the patch. Moving to the amd-gfx mailing list, where xf86-video-amdgpu patches are reviewed. Comments inline below. On 2018-09-10 1:14 p.m., Aaron Liu wrote: > Add gamma_lut/degamma_lut/ctm checking before pushing > staged color management properties on the CRTC. > If above object is NULL, return directly. > > Signed-off-by: Aaron Liu > --- > src/drmmode_display.c | 15 +++ > 1 file changed, 15 insertions(+) > > diff --git a/src/drmmode_display.c b/src/drmmode_display.c > index 68fca1b..006ae7c 100644 > --- a/src/drmmode_display.c > +++ b/src/drmmode_display.c > @@ -1116,6 +1116,11 @@ static int drmmode_crtc_push_cm_prop(xf86CrtcPtr crtc, > > switch (cm_prop_index) { > case CM_GAMMA_LUT: > + if (!drmmode_crtc->gamma_lut) { > + xf86DrvMsg(crtc->scrn->scrnIndex, X_WARNING, > +"gamma_lut is NULL!\n"); > + return BadRequest; > + } > /* Calculate the expected size of value in bytes */ > expected_bytes = sizeof(struct drm_color_lut) * > drmmode->gamma_lut_size; > @@ -1154,11 +1159,21 @@ static int drmmode_crtc_push_cm_prop(xf86CrtcPtr crtc, > } > break; > case CM_DEGAMMA_LUT: > + if (!drmmode_crtc->degamma_lut) { > + xf86DrvMsg(crtc->scrn->scrnIndex, X_WARNING, > +"degamma_lut is NULL!\n"); > + return BadRequest; > + } > expected_bytes = sizeof(struct drm_color_lut) * > drmmode->degamma_lut_size; > blob_data = drmmode_crtc->degamma_lut; > break; > case CM_CTM: > + if (!drmmode_crtc->ctm) { > + xf86DrvMsg(crtc->scrn->scrnIndex, X_WARNING, > +"ctm is NULL!\n"); > + return BadRequest; > + } > expected_bytes = sizeof(struct drm_color_ctm); > blob_data = drmmode_crtc->ctm; > break; > (How) were you able to actually trigger this condition in practice? The only way I could imagine is via drmmode_output_set_property, in which case I think it would be better to guard all the *cm_* related code there with if (drmmode_cm_enabled(drmmode_output->drmmode)) { ... } -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-amdgpu] Do not push the CM_GAMMA_LUT property values in drmmode_crtc_cm_init
From: Michel Dänzer The crtc->gamma_lut values aren't initialized yet at this point, and the property values are pushed again from drmmode_setup_colormap anyway. Fixes intermittent flicker due to random gamma LUT values during server startup. Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 46be29d07..6ef6a98e2 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -1863,7 +1863,7 @@ static void drmmode_crtc_cm_init(int drm_fd, xf86CrtcPtr crtc) drmmode_crtc->ctm->matrix[8] = (uint64_t)1 << 32; /* Push properties to reset properties currently in hardware */ - for (i = 0; i < CM_DEGAMMA_LUT_SIZE; i++) { + for (i = 0; i < CM_GAMMA_LUT; i++) { if (drmmode_crtc_push_cm_prop(crtc, i)) xf86DrvMsg(crtc->scrn->scrnIndex, X_ERROR, "Failed to initialize color management " -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: fix "Revert "drm/amdgpu: move PD/PT bos on LRU again""
On 2018-08-30 10:08 a.m., Christian König wrote: > This reverts commit 1156da3d4034957e7927ea68007b981942f5cbd5. > > We should review reverts as well cause that one only added an incomplete band > aided to the problem. Sorry about that. I didn't notice any issues with the same testing procedure that easily reproduced issues without the revert, so I thought it should be at least an improvement. > Correctly disable bulk moves until we have figured out why they corrupt > the lists. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > index 72f8c750e128..4a2d31e45c17 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > @@ -283,12 +283,15 @@ void amdgpu_vm_move_to_lru_tail(struct amdgpu_device > *adev, > struct ttm_bo_global *glob = adev->mman.bdev.glob; > struct amdgpu_vm_bo_base *bo_base; > > + /* TODO: Fix list corruption caused by this */ > +#if 0 > if (vm->bulk_moveable) { > spin_lock(>lru_lock); > ttm_bo_bulk_move_lru_tail(>lru_bulk_move); > spin_unlock(>lru_lock); > return; > } > +#endif Code should be removed, not #if 0'd. Anyway, with this patch, the attached warning dumps appear in dmesg about 1000 times per second at the GDM login prompt, can't even attempt to run piglit. Something else is needed, I'm afraid. In case it's relevant, note that my development machine has a secondary Turks card installed. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer Aug 30 10:36:03 kaveri kernel: [ 12.338157] WARNING: CPU: 9 PID: 1485 at drivers/gpu/drm//ttm/ttm_bo.c:228 ttm_bo_move_to_lru_tail+0x28b/0x3d0 [ttm] Aug 30 10:36:03 kaveri kernel: [ 12.338182] Modules linked in: lz4(E) lz4_compress(E) cpufreq_powersave(E) cpufreq_userspace(E) cpufreq_conservative(E) amdgpu(OE) chash(OE) gpu_sched(OE) binfmt_misc(E) nls_ascii(E) nls_cp437(E) vfat(E) fat(E) edac_mce_amd(E) radeon(OE) kvm(E) irqbypass(E) crct10dif_pclmul(E) crc32_pclmul(E) ghash_clmulni_intel(E) snd_hda_codec_realtek(E) pcbc(E) snd_hda_codec_generic(E) ttm(OE) snd_hda_codec_hdmi(E) drm_kms_helper(OE) snd_hda_intel(E) aesni_intel(E) snd_hda_codec(E) aes_x86_64(E) crypto_simd(E) wmi_bmof(E) snd_hda_core(E) drm(OE) cryptd(E) glue_helper(E) snd_hwdep(E) snd_pcm(E) snd_timer(E) r8169(E) i2c_algo_bit(E) fb_sys_fops(E) syscopyarea(E) efi_pstore(E) sp5100_tco(E) sysfillrect(E) pcspkr(E) ccp(E) snd(E) sysimgblt(E) mii(E) sg(E) efivars(E) soundcore(E) rng_core(E) i2c_piix4(E) k10temp(E) Aug 30 10:36:03 kaveri kernel: [ 12.338287] wmi(E) button(E) acpi_cpufreq(E) tcp_bbr(E) sch_fq(E) nct6775(E) hwmon_vid(E) sunrpc(E) efivarfs(E) ip_tables(E) x_tables(E) ext4(E) crc32c_generic(E) crc16(E) mbcache(E) jbd2(E) fscrypto(E) dm_mod(E) raid10(E) raid1(E) raid0(E) multipath(E) linear(E) md_mod(E) sd_mod(E) evdev(E) hid_generic(E) usbhid(E) hid(E) ahci(E) xhci_pci(E) libahci(E) xhci_hcd(E) libata(E) crc32c_intel(E) usbcore(E) scsi_mod(E) gpio_amdpt(E) gpio_generic(E) Aug 30 10:36:03 kaveri kernel: [ 12.338364] CPU: 9 PID: 1485 Comm: gnome-shel:cs0 Tainted: G OE 4.18.0-rc1+ #111 Aug 30 10:36:03 kaveri kernel: [ 12.338367] Hardware name: Micro-Star International Co., Ltd. MS-7A34/B350 TOMAHAWK (MS-7A34), BIOS 1.80 09/13/2017 Aug 30 10:36:03 kaveri kernel: [ 12.338377] RIP: 0010:ttm_bo_move_to_lru_tail+0x28b/0x3d0 [ttm] Aug 30 10:36:03 kaveri kernel: [ 12.338379] Code: c1 ea 03 80 3c 02 00 0f 85 e6 00 00 00 48 8b 83 e8 01 00 00 be ff ff ff ff 48 8d 78 60 e8 cd 07 9d ef 85 c0 0f 85 c3 fd ff ff <0f> 0b e9 bc fd ff ff 48 8d bb d0 01 00 00 48 b8 00 00 00 00 00 fc Aug 30 10:36:03 kaveri kernel: [ 12.338506] RSP: 0018:88036174f6b0 EFLAGS: 00010246 Aug 30 10:36:03 kaveri kernel: [ 12.338512] RAX: RBX: 8803cb865550 RCX: b0a4c9ed Aug 30 10:36:03 kaveri kernel: [ 12.338515] RDX: RSI: 8803eb560b20 RDI: 0246 Aug 30 10:36:03 kaveri kernel: [ 12.338517] RBP: 880376121738 R08: ed0079fa274b R09: ed0079fa274b Aug 30 10:36:03 kaveri kernel: [ 12.338520] R10: 0001 R11: ed0079fa274a R12: 880376121178 Aug 30 10:36:03 kaveri kernel: [ 12.338523] R13: 880376121738 R14: 880376121100 R15: dc00 Aug 30 10:36:03 kaveri kernel: [ 12.338527] FS: 7fa7f8caa700() GS:8803ee24() knlGS: Aug 30 10:36:03 kaveri kernel: [ 12.338530] CS: 0010 DS: ES: CR0: 80050033 Aug 30 10:36:03 kaveri kernel: [ 12.338533] CR2: 7fa7f010 CR3: 0003746b
Re: [PATCH libdrm] amdgpu: add error return value for finding bo by cpu mapping
On 2018-08-30 10:50 a.m., Junwei Zhang wrote: > If nothing is found, error should be returned. > > Signed-off-by: Junwei Zhang > > [...] > > @@ -577,10 +578,11 @@ int amdgpu_find_bo_by_cpu_mapping(amdgpu_device_handle > dev, > } else { > *buf_handle = NULL; > *offset_in_bo = 0; > + r = -EINVAL; I think -ENOENT would be better, to differentiate this error from passing invalid pointer / size parameters. With that, Reviewed-by: Michel Dänzer -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 01/11] drm/amdgpu: try allocating VRAM as power of two
On 2018-09-11 8:49 a.m., Christian König wrote: > > But another question: Why do you want to clear VRAM on allocation? We > perfectly support allocating VRAM without clearing it. Which is a problem of its own, as it can leak information from one process to another. Anyway, not clearing when userspace sets AMDGPU_GEM_CREATE_VRAM_CLEARED would break the userspace ABI, so that's no go. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 01/11] drm/amdgpu: try allocating VRAM as power of two
On 2018-09-11 9:46 a.m., Christian König wrote: > Am 11.09.2018 um 09:37 schrieb Michel Dänzer: >> On 2018-09-11 8:49 a.m., Christian König wrote: >>> But another question: Why do you want to clear VRAM on allocation? We >>> perfectly support allocating VRAM without clearing it. >> Which is a problem of its own, as it can leak information from one >> process to another. > > How about adding a AMDGPU_GEM_CREATE_VRAM_SECRED flag which triggers > clearing VRAM when the BO is freed? Doesn't sound like much better API than AMDGPU_GEM_CREATE_VRAM_CLEARED to me. Why would any process be okay with leaking its BO contents to other processes? Could that flag even prevent it in all cases, e.g. when BOs are evicted or otherwise migrated? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 01/11] drm/amdgpu: try allocating VRAM as power of two
On 2018-09-11 10:20 a.m., Christian König wrote: > Am 11.09.2018 um 09:55 schrieb Michel Dänzer: >> On 2018-09-11 9:46 a.m., Christian König wrote: >>> Am 11.09.2018 um 09:37 schrieb Michel Dänzer: >>>> On 2018-09-11 8:49 a.m., Christian König wrote: >>>>> But another question: Why do you want to clear VRAM on allocation? We >>>>> perfectly support allocating VRAM without clearing it. >>>> Which is a problem of its own, as it can leak information from one >>>> process to another. >>> How about adding a AMDGPU_GEM_CREATE_VRAM_SECRED flag which triggers >>> clearing VRAM when the BO is freed? >> Doesn't sound like much better API than AMDGPU_GEM_CREATE_VRAM_CLEARED >> to me. Why would any process be okay with leaking its BO contents to >> other processes? > > Well the content of most BOs is uninteresting to other processes, e.g. > textures. Textures can hold application window contents, which can be sensitive. Not clearing BOs can result in such contents showing up in unexpected places. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH xf86-video-amdgpu] Add checking color management properties
On 2018-09-10 5:58 p.m., Michel Dänzer wrote: > On 2018-09-10 1:14 p.m., Aaron Liu wrote: >> Add gamma_lut/degamma_lut/ctm checking before pushing >> staged color management properties on the CRTC. >> If above object is NULL, return directly. > > (How) were you able to actually trigger this condition in practice? It has become clear from AMD internal e-mails that this patch was motivated by the same problem that was fixed by https://gitlab.freedesktop.org/xorg/driver/xf86-video-amdgpu/commit/a923bedfd91d39977dbf95f296cf9b68439490f2 . This patch is thus no longer needed. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH 2/5] drm/amdgpu: always enable shadow BOs
On 2018-09-11 11:55 a.m., Christian König wrote: > Even when GPU recovery is disabled we could run into a manually > triggered recovery. > > Signed-off-by: Christian König > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 27 +-- > 1 file changed, 1 insertion(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > index a7e39c9dd14b..7db0040ca145 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c > @@ -38,31 +38,6 @@ > #include "amdgpu_trace.h" > #include "amdgpu_amdkfd.h" > > -/** > - * DOC: amdgpu_object > - * > - * This defines the interfaces to operate on an _bo buffer object > which > - * represents memory used by driver (VRAM, system memory, etc.). The driver > - * provides DRM/GEM APIs to userspace. DRM/GEM APIs then use these interfaces > - * to create/destroy/set buffer object which are then managed by the kernel > TTM > - * memory manager. > - * The interfaces are also used internally by kernel clients, including gfx, > - * uvd, etc. for kernel managed allocations used by the GPU. > - * > - */ This comment shouldn't be removed, should it? Otherwise the commit log needs to explain this, and the reference in Documentation/gpu/amdgpu.rst needs to be removed as well. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amd/display: Use 2-factor allocator calls
On 2018-07-04 07:27 PM, Kees Cook wrote: > As already done treewide, switch from open-coded multiplication to > 2-factor allocation helper. > > Signed-off-by: Kees Cook > --- > drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c > b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c > index 98edaefa2b47..ee69c949bfbf 100644 > --- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c > +++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c > @@ -1723,8 +1723,8 @@ bool mod_color_calculate_curve(enum > dc_transfer_func_predefined trans, > kvfree(rgb_regamma); > } else if (trans == TRANSFER_FUNCTION_HLG || > trans == TRANSFER_FUNCTION_HLG12) { > - rgb_regamma = kvzalloc(sizeof(*rgb_regamma) * > -(MAX_HW_POINTS + _EXTRA_POINTS), > + rgb_regamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS, > +sizeof(*rgb_regamma), > GFP_KERNEL); > if (!rgb_regamma) > goto rgb_regamma_alloc_fail; > @@ -1802,8 +1802,8 @@ bool mod_color_calculate_degamma_curve(enum > dc_transfer_func_predefined trans, > kvfree(rgb_degamma); > } else if (trans == TRANSFER_FUNCTION_HLG || > trans == TRANSFER_FUNCTION_HLG12) { > - rgb_degamma = kvzalloc(sizeof(*rgb_degamma) * > -(MAX_HW_POINTS + _EXTRA_POINTS), > + rgb_degamma = kvcalloc(MAX_HW_POINTS + _EXTRA_POINTS, > +sizeof(*rgb_degamma), > GFP_KERNEL); > if (!rgb_degamma) > goto rgb_degamma_alloc_fail; > Since the values are constant, kvcalloc incurs the overflow checking overhead for no gain. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: Proposal to merge KFD into amdgpu
On 2018-07-04 11:36 PM, Felix Kuehling wrote: > Since KFD is only supported by a single GPU driver now (amdgpu), it > makes sense to merge the two. This has been raised on the amd-gfx list > before and I've been putting it off to avoid more churn while I was > working on upstreaming KFD. Now seems a good time to pick this up again. > > At this stage there are some things that I don't expect to change > significantly: > > * Directory structure > * KFD function naming conventions > * KFD device and sysfs interfaces > > This is a rough overview of the changes I have in mind. We should be > able to implement these step-by-step and minimize disruption: > > 1. Change the build system to build KFD into amdgpu.ko > > This should make KFD similar to DAL or powerplay. It's still a mostly > separate code base and Makefile with its own directory, but gets linked > into amdgpu.ko. > > In the kernel configuration HSA_AMD would become a boolean option under > DRM_AMDGPU that controls whether KFD functionality gets built into amdgpu. > > Any code inside #if defined(CONFIG_HSA_AMD_MODULE) can be removed. > > 2. Simplify the kfd2kgd and kgd2kfg interfaces > > Function pointers in struct kgd2kfd_calls are no longer needed. These > functions can be called directly from amdgpu. > > Hardware-independent function pointers in kfd2kgd_calls are no longer > needed. These function can be called directly from amdkfd. Some of the > function pointers in kfd2kgd_calls are used for hardware abstraction > with different implementations for each GFX HW generation. These will > need to remain function pointers. > > At some later stage, the HW-specific functions could be moved into > gfx_v*.c and the function pointers added to struct amdgpu_gfx. But at > this stage I think I'd leave them where they are. > > 3. Reduce duplicate tracking of device and BO structures > > Currently KFD and AMDGPU pretend to not know each other's data > structures. If both are in the same module, we could allow KFD to access > some amdgpu structures directly (e.g. amdgpu_device and amdgpu_bo). This > way some of the duplicate tracking of devices and buffer objects could > be eliminated. > > This may present opportunities to simplify some functionality that's > currently split across both modules, such as suspend/resume, memory > management and evictions. > > Some interfaces that just query information from amdgpu could be removed > if KFD can access that information directly (e.g. firmware versions, CU > info, ...). > > Please let me know if you have any objections, suggestions, ideas ... Thumbs up from me! :) -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati 06/10] Move flush from radeon_scanout_do_update to its callers
From: Michel Dänzer No functional change intended. Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 2 ++ src/radeon_kms.c | 8 +--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index c7bec59c8..54b09730d 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -785,6 +785,7 @@ drmmode_crtc_scanout_update(xf86CrtcPtr crtc, DisplayModePtr mode, radeon_scanout_do_update(crtc, scanout_id, screen->GetWindowPixmap(screen->root), *box); + radeon_cs_flush_indirect(scrn); radeon_bo_wait(drmmode_crtc->scanout[scanout_id].bo); } } @@ -3225,6 +3226,7 @@ Bool radeon_do_pageflip(ScrnInfoPtr scrn, ClientPtr client, radeon_scanout_do_update(crtc, scanout_id, new_front, extents); + radeon_cs_flush_indirect(crtc->scrn); drmmode_crtc_wait_pending_event(drmmode_crtc, pRADEONEnt->fd, drmmode_crtc->scanout_update_pending); diff --git a/src/radeon_kms.c b/src/radeon_kms.c index 7ff66bf3c..8579aaf81 100644 --- a/src/radeon_kms.c +++ b/src/radeon_kms.c @@ -985,8 +985,6 @@ radeon_scanout_do_update(xf86CrtcPtr xf86_crtc, int scanout_id, FreeScratchGC(gc); } -radeon_cs_flush_indirect(scrn); - info->accel_state->force = force; return TRUE; @@ -1013,8 +1011,10 @@ radeon_scanout_update_handler(xf86CrtcPtr crtc, uint32_t frame, uint64_t usec, drmmode_crtc->dpms_mode == DPMSModeOn) { if (radeon_scanout_do_update(crtc, drmmode_crtc->scanout_id, screen->GetWindowPixmap(screen->root), -region->extents)) +region->extents)) { + radeon_cs_flush_indirect(crtc->scrn); RegionEmpty(region); + } } radeon_scanout_update_abort(crtc, event_data); @@ -1096,6 +1096,8 @@ radeon_scanout_flip(ScreenPtr pScreen, RADEONInfoPtr info, pScreen->GetWindowPixmap(pScreen->root), region->extents)) return; + +radeon_cs_flush_indirect(scrn); RegionEmpty(region); drm_queue_seq = radeon_drm_queue_alloc(xf86_crtc, -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati 03/10] Only initialize libdrm_radeon surface manager for >= R600
From: Michel Dänzer Not used with older GPUs. Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 5 +++-- src/radeon_bo_helper.c | 7 --- src/radeon_kms.c | 18 +++--- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 958532fb6..f99667fb1 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -135,7 +135,7 @@ static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn, if (!radeon_set_pixmap_bo(pixmap, bo)) goto fail; - if (info->ChipFamily >= CHIP_FAMILY_R600) { + if (info->surf_man) { surface = radeon_get_pixmap_surface(pixmap); if (surface) { memset(surface, 0, sizeof(struct radeon_surface)); @@ -2301,7 +2301,8 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height) aligned_height = RADEON_ALIGN(height, drmmode_get_height_align(scrn, tiling_flags)); screen_size = RADEON_ALIGN(pitch * aligned_height, RADEON_GPU_PAGE_SIZE); base_align = 4096; - if (info->ChipFamily >= CHIP_FAMILY_R600) { + + if (info->surf_man) { memset(, 0, sizeof(struct radeon_surface)); surface.npix_x = width; surface.npix_y = height; diff --git a/src/radeon_bo_helper.c b/src/radeon_bo_helper.c index 0366f613b..8245d6247 100644 --- a/src/radeon_bo_helper.c +++ b/src/radeon_bo_helper.c @@ -107,9 +107,10 @@ radeon_alloc_pixmap_bo(ScrnInfoPtr pScrn, int width, int height, int depth, pitch = RADEON_ALIGN(width, drmmode_get_pitch_align(pScrn, cpp, tiling)) * cpp; base_align = drmmode_get_base_align(pScrn, cpp, tiling); size = RADEON_ALIGN(heighta * pitch, RADEON_GPU_PAGE_SIZE); -memset(, 0, sizeof(struct radeon_surface)); -if (info->ChipFamily >= CHIP_FAMILY_R600 && info->surf_man) { +if (info->surf_man) { + memset(, 0, sizeof(struct radeon_surface)); + if (width) { surface.npix_x = width; /* need to align height to 8 for old kernel */ @@ -340,7 +341,7 @@ Bool radeon_set_shared_pixmap_backing(PixmapPtr ppix, void *fd_handle, if (!ret) goto error; -if (info->ChipFamily >= CHIP_FAMILY_R600 && info->surf_man) { +if (info->surf_man) { uint32_t tiling_flags; #ifdef USE_GLAMOR diff --git a/src/radeon_kms.c b/src/radeon_kms.c index 26810e084..861fbf97c 100644 --- a/src/radeon_kms.c +++ b/src/radeon_kms.c @@ -2228,7 +2228,16 @@ Bool RADEONScreenInit_KMS(ScreenPtr pScreen, int argc, char **argv) if (info->r600_shadow_fb == FALSE) info->directRenderingEnabled = radeon_dri2_screen_init(pScreen); -info->surf_man = radeon_surface_manager_new(pRADEONEnt->fd); +if (info->ChipFamily >= CHIP_FAMILY_R600) { + info->surf_man = radeon_surface_manager_new(pRADEONEnt->fd); + + if (!info->surf_man) { + xf86DrvMsg(pScreen->myNum, X_ERROR, + "Failed to initialize surface manager\n"); + return FALSE; + } +} + if (!info->bufmgr) info->bufmgr = radeon_bo_manager_gem_ctor(pRADEONEnt->fd); if (!info->bufmgr) { @@ -2694,12 +2703,7 @@ static Bool radeon_setup_kernel_mem(ScreenPtr pScreen) pitch = RADEON_ALIGN(pScrn->virtualX, drmmode_get_pitch_align(pScrn, cpp, tiling_flags)) * cpp; screen_size = RADEON_ALIGN(pScrn->virtualY, drmmode_get_height_align(pScrn, tiling_flags)) * pitch; base_align = drmmode_get_base_align(pScrn, cpp, tiling_flags); - if (info->ChipFamily >= CHIP_FAMILY_R600) { - if(!info->surf_man) { - xf86DrvMsg(pScreen->myNum, X_ERROR, - "failed to initialise surface manager\n"); - return FALSE; - } + if (info->surf_man) { memset(, 0, sizeof(struct radeon_surface)); surface.npix_x = pScrn->virtualX; surface.npix_y = pScrn->virtualY; -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati 05/10] Factor out radeon_surface_initialize helper
From: Michel Dänzer Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 66 +--- src/radeon.h | 5 + src/radeon_bo_helper.c | 231 + src/radeon_kms.c | 35 +-- 4 files changed, 131 insertions(+), 206 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 8dc776fa5..c7bec59c8 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -116,7 +116,6 @@ static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn, RADEONInfoPtr info = RADEONPTR(pScrn); ScreenPtr pScreen = pScrn->pScreen; PixmapPtr pixmap; - uint32_t tiling; pixmap = (*pScreen->CreatePixmap)(pScreen, 0, 0, depth, RADEON_CREATE_PIXMAP_SCANOUT); @@ -137,37 +136,8 @@ static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn, if (info->surf_man && !info->use_glamor) { struct radeon_surface *surface = radeon_get_pixmap_surface(pixmap); - memset(surface, 0, sizeof(struct radeon_surface)); - surface->npix_x = width; - surface->npix_y = height; - surface->npix_z = 1; - surface->blk_w = 1; - surface->blk_h = 1; - surface->blk_d = 1; - surface->array_size = 1; - surface->last_level = 0; - surface->bpe = bpp / 8; - surface->nsamples = 1; - surface->flags = RADEON_SURF_SCANOUT; - /* we are requiring a recent enough libdrm version */ - surface->flags |= RADEON_SURF_HAS_TILE_MODE_INDEX; - surface->flags |= RADEON_SURF_SET(RADEON_SURF_TYPE_2D, TYPE); - surface->flags |= RADEON_SURF_SET(RADEON_SURF_MODE_LINEAR_ALIGNED, MODE); - tiling = radeon_get_pixmap_tiling_flags(pixmap); - - if (tiling & RADEON_TILING_MICRO) { - surface->flags = RADEON_SURF_CLR(surface->flags, MODE); - surface->flags |= RADEON_SURF_SET(RADEON_SURF_MODE_1D, MODE); - } - if (tiling & RADEON_TILING_MACRO) { - surface->flags = RADEON_SURF_CLR(surface->flags, MODE); - surface->flags |= RADEON_SURF_SET(RADEON_SURF_MODE_2D, MODE); - } - - if (radeon_surface_best(info->surf_man, surface)) - goto fail; - - if (radeon_surface_init(info->surf_man, surface)) + if (!radeon_surface_initialize(info, surface, width, height, bpp / 8, + radeon_get_pixmap_tiling_flags(pixmap), 0)) goto fail; } @@ -2301,36 +2271,10 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height) base_align = 4096; if (info->surf_man) { - memset(, 0, sizeof(struct radeon_surface)); - surface.npix_x = width; - surface.npix_y = height; - surface.npix_z = 1; - surface.blk_w = 1; - surface.blk_h = 1; - surface.blk_d = 1; - surface.array_size = 1; - surface.last_level = 0; - surface.bpe = cpp; - surface.nsamples = 1; - surface.flags = RADEON_SURF_SCANOUT; - /* we are requiring a recent enough libdrm version */ - surface.flags |= RADEON_SURF_HAS_TILE_MODE_INDEX; - surface.flags |= RADEON_SURF_SET(RADEON_SURF_TYPE_2D, TYPE); - surface.flags |= RADEON_SURF_SET(RADEON_SURF_MODE_LINEAR_ALIGNED, MODE); - if (tiling_flags & RADEON_TILING_MICRO) { - surface.flags = RADEON_SURF_CLR(surface.flags, MODE); - surface.flags |= RADEON_SURF_SET(RADEON_SURF_MODE_1D, MODE); - } - if (tiling_flags & RADEON_TILING_MACRO) { - surface.flags = RADEON_SURF_CLR(surface.flags, MODE); - surface.flags |= RADEON_SURF_SET(RADEON_SURF_MODE_2D, MODE); - } - if (radeon_surface_best(info->surf_man, )) { - return FALSE; - } - if (radeon_surface_init(info->surf_man, )) { + if (!radeon_surface_initialize(info, , width, height, + cpp, tiling_flags, 0)) return FALSE; - } + screen_size = surface.bo_size; base_align = surface.bo_alignment; pitch = surface.level[0].pitch_bytes; diff --git a/src/radeon.h b/src/radeon.h index 63b6cf1ff..c2ae6606e 100644 --- a/src/radeon.h +++ b/src/radeon.h @@ -643,6 +643,11 @@ extern void RADEONInit
[PATCH xf86-video-ati 04/10] glamor: Don't store radeon_surfaces in pixmaps
From: Michel Dänzer Only EXA needs them. Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 77 +- src/radeon.h | 19 ++- src/radeon_bo_helper.c | 22 +--- src/radeon_glamor.c| 8 ++--- src/radeon_kms.c | 11 +++--- 5 files changed, 55 insertions(+), 82 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index f99667fb1..8dc776fa5 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -116,7 +116,6 @@ static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn, RADEONInfoPtr info = RADEONPTR(pScrn); ScreenPtr pScreen = pScrn->pScreen; PixmapPtr pixmap; - struct radeon_surface *surface; uint32_t tiling; pixmap = (*pScreen->CreatePixmap)(pScreen, 0, 0, depth, @@ -135,41 +134,41 @@ static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn, if (!radeon_set_pixmap_bo(pixmap, bo)) goto fail; - if (info->surf_man) { - surface = radeon_get_pixmap_surface(pixmap); - if (surface) { - memset(surface, 0, sizeof(struct radeon_surface)); - surface->npix_x = width; - surface->npix_y = height; - surface->npix_z = 1; - surface->blk_w = 1; - surface->blk_h = 1; - surface->blk_d = 1; - surface->array_size = 1; - surface->last_level = 0; - surface->bpe = bpp / 8; - surface->nsamples = 1; - surface->flags = RADEON_SURF_SCANOUT; - /* we are requiring a recent enough libdrm version */ - surface->flags |= RADEON_SURF_HAS_TILE_MODE_INDEX; - surface->flags |= RADEON_SURF_SET(RADEON_SURF_TYPE_2D, TYPE); - surface->flags |= RADEON_SURF_SET(RADEON_SURF_MODE_LINEAR_ALIGNED, MODE); - tiling = radeon_get_pixmap_tiling_flags(pixmap); - if (tiling & RADEON_TILING_MICRO) { - surface->flags = RADEON_SURF_CLR(surface->flags, MODE); - surface->flags |= RADEON_SURF_SET(RADEON_SURF_MODE_1D, MODE); - } - if (tiling & RADEON_TILING_MACRO) { - surface->flags = RADEON_SURF_CLR(surface->flags, MODE); - surface->flags |= RADEON_SURF_SET(RADEON_SURF_MODE_2D, MODE); - } - if (radeon_surface_best(info->surf_man, surface)) { - goto fail; - } - if (radeon_surface_init(info->surf_man, surface)) { - goto fail; - } + if (info->surf_man && !info->use_glamor) { + struct radeon_surface *surface = radeon_get_pixmap_surface(pixmap); + + memset(surface, 0, sizeof(struct radeon_surface)); + surface->npix_x = width; + surface->npix_y = height; + surface->npix_z = 1; + surface->blk_w = 1; + surface->blk_h = 1; + surface->blk_d = 1; + surface->array_size = 1; + surface->last_level = 0; + surface->bpe = bpp / 8; + surface->nsamples = 1; + surface->flags = RADEON_SURF_SCANOUT; + /* we are requiring a recent enough libdrm version */ + surface->flags |= RADEON_SURF_HAS_TILE_MODE_INDEX; + surface->flags |= RADEON_SURF_SET(RADEON_SURF_TYPE_2D, TYPE); + surface->flags |= RADEON_SURF_SET(RADEON_SURF_MODE_LINEAR_ALIGNED, MODE); + tiling = radeon_get_pixmap_tiling_flags(pixmap); + + if (tiling & RADEON_TILING_MICRO) { + surface->flags = RADEON_SURF_CLR(surface->flags, MODE); + surface->flags |= RADEON_SURF_SET(RADEON_SURF_MODE_1D, MODE); } + if (tiling & RADEON_TILING_MACRO) { + surface->flags = RADEON_SURF_CLR(surface->flags, MODE); + surface->flags |= RADEON_SURF_SET(RADEON_SURF_MODE_2D, MODE); + } + + if (radeon_surface_best(info->surf_man, surface)) + goto fail; + + if (radeon_surface_init(info->surf_man, surface)) + goto fail; } if (!info->use_glamor || @@ -2272,7 +2271,6 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height) int cpp = info-&
[PATCH xf86-video-ati 09/10] Add struct radeon_buffer
From: Michel Dänzer Inspired by amdgpu, preparation for the following change. For now, this is mostly a wrapper around struct radeon_bo, no functional change intended. Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 123 ++-- src/drmmode_display.h | 2 +- src/evergreen_exa.c | 40 - src/evergreen_state.h | 1 - src/evergreen_textured_videofuncs.c | 2 +- src/r600_exa.c | 40 - src/r600_state.h| 1 - src/r600_textured_videofuncs.c | 2 +- src/radeon.h| 31 --- src/radeon_bo_helper.c | 34 +--- src/radeon_bo_helper.h | 38 - src/radeon_dri2.c | 22 ++--- src/radeon_dri3.c | 4 +- src/radeon_exa.c| 17 ++-- src/radeon_exa_funcs.c | 43 +- src/radeon_exa_shared.c | 2 +- src/radeon_exa_shared.h | 3 +- src/radeon_glamor.c | 10 +-- src/radeon_glamor_wrappers.c| 6 +- src/radeon_kms.c| 104 --- src/radeon_present.c| 2 +- src/radeon_textured_videofuncs.c| 24 -- 22 files changed, 267 insertions(+), 284 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 3c4d94fd9..c91f5bb20 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -111,7 +111,7 @@ static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn, int width, int height, int depth, int bpp, int pitch, - struct radeon_bo *bo) + struct radeon_buffer *bo) { RADEONInfoPtr info = RADEONPTR(pScrn); ScreenPtr pScreen = pScrn->pScreen; @@ -379,7 +379,7 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode, RADEONEntPtr pRADEONEnt = RADEONEntPriv(pScrn); RADEONInfoPtr info = RADEONPTR(pScrn); PixmapPtr pixmap = info->fbcon_pixmap; - struct radeon_bo *bo; + struct radeon_buffer *bo; drmModeFBPtr fbcon; struct drm_gem_flink flink; @@ -402,10 +402,18 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode, goto out_free_fb; } - bo = radeon_bo_open(drmmode->bufmgr, flink.name, 0, 0, 0, 0); + bo = calloc(1, sizeof(struct radeon_buffer)); + if (!bo) { + xf86DrvMsg(pScrn->scrnIndex, X_ERROR, + "Couldn't allocate BO for fbcon handle\n"); + goto out_free_fb; + } + bo->ref_count = 1; + + bo->bo.radeon = radeon_bo_open(drmmode->bufmgr, flink.name, 0, 0, 0, 0); if (bo == NULL) { xf86DrvMsg(pScrn->scrnIndex, X_ERROR, - "Couldn't allocate bo for fbcon handle\n"); + "Couldn't open BO for fbcon handle\n"); goto out_free_fb; } @@ -413,7 +421,7 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode, fbcon->depth, fbcon->bpp, fbcon->pitch, bo); info->fbcon_pixmap = pixmap; - radeon_bo_unref(bo); + radeon_buffer_unref(); out_free_fb: drmModeFreeFB(fbcon); return pixmap; @@ -496,11 +504,7 @@ drmmode_crtc_scanout_destroy(drmmode_ptr drmmode, scanout->pixmap = NULL; } - if (scanout->bo) { - radeon_bo_unmap(scanout->bo); - radeon_bo_unref(scanout->bo); - scanout->bo = NULL; - } + radeon_buffer_unref(>bo); } void @@ -913,7 +917,7 @@ drmmode_set_mode_major(xf86CrtcPtr crtc, DisplayModePtr mode, fb = radeon_fb_create(pScrn, pRADEONEnt->fd, pScrn->virtualX, pScrn->virtualY, pScrn->displayWidth * info->pixel_bytes, - info->front_bo->handle); + info->front_buffer->bo.radeon->handle); /* Prevent refcnt of ad-hoc FBs from reaching 2 */ drmmode_fb_reference(pRADEONEnt->fd, _crtc->fb, NULL); drmmode_crtc->fb = fb; @@ -2232,14 +2236,12 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height) { xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn); RADEONInfoPtr info = RADEONPTR(scrn); - struct radeon_bo *old_front = NULL; + struct radeon_buffer *old_front = NULL; ScreenPtr screen = xf86ScrnToScreen(scrn
[PATCH xf86-video-ati 01/10] Drop unused drmmode_create_bo_pixmap surface parameter
From: Michel Dänzer Signed-off-by: Michel Dänzer --- src/drmmode_display.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index f056bf3b4..958532fb6 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -111,7 +111,7 @@ static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn, int width, int height, int depth, int bpp, int pitch, - struct radeon_bo *bo, struct radeon_surface *psurf) + struct radeon_bo *bo) { RADEONInfoPtr info = RADEONPTR(pScrn); ScreenPtr pScreen = pScrn->pScreen; @@ -137,9 +137,7 @@ static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn, if (info->ChipFamily >= CHIP_FAMILY_R600) { surface = radeon_get_pixmap_surface(pixmap); - if (surface && psurf) - *surface = *psurf; - else if (surface) { + if (surface) { memset(surface, 0, sizeof(struct radeon_surface)); surface->npix_x = width; surface->npix_y = height; @@ -444,7 +442,7 @@ create_pixmap_for_fbcon(drmmode_ptr drmmode, pixmap = drmmode_create_bo_pixmap(pScrn, fbcon->width, fbcon->height, fbcon->depth, fbcon->bpp, fbcon->pitch, - bo, NULL); + bo); info->fbcon_pixmap = pixmap; radeon_bo_unref(bo); out_free_fb: @@ -581,7 +579,7 @@ drmmode_crtc_scanout_create(xf86CrtcPtr crtc, struct drmmode_scanout *scanout, width, height, pScrn->depth, pScrn->bitsPerPixel, -pitch, scanout->bo, NULL); +pitch, scanout->bo); if (!scanout->pixmap) { ErrorF("failed to create CRTC scanout pixmap\n"); goto error; -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati 02/10] EXA: Remove old RADEONEXACreatePixmap hook
From: Michel Dänzer Not used by any supported version of xserver. Signed-off-by: Michel Dänzer --- src/evergreen_exa.c| 1 - src/evergreen_state.h | 1 - src/r600_exa.c | 1 - src/r600_state.h | 1 - src/radeon_exa.c | 29 - src/radeon_exa_funcs.c | 1 - 6 files changed, 34 deletions(-) diff --git a/src/evergreen_exa.c b/src/evergreen_exa.c index 858481333..41edd3453 100644 --- a/src/evergreen_exa.c +++ b/src/evergreen_exa.c @@ -2065,7 +2065,6 @@ EVERGREENDrawInit(ScreenPtr pScreen) info->accel_state->exa->MarkSync = EVERGREENMarkSync; info->accel_state->exa->WaitMarker = EVERGREENSync; -info->accel_state->exa->CreatePixmap = RADEONEXACreatePixmap; info->accel_state->exa->DestroyPixmap = RADEONEXADestroyPixmap; info->accel_state->exa->PixmapIsOffscreen = RADEONEXAPixmapIsOffscreen; info->accel_state->exa->PrepareAccess = RADEONPrepareAccess_CS; diff --git a/src/evergreen_state.h b/src/evergreen_state.h index 795d44720..ef3310025 100644 --- a/src/evergreen_state.h +++ b/src/evergreen_state.h @@ -345,7 +345,6 @@ R600SetAccelState(ScrnInfoPtr pScrn, extern Bool RADEONPrepareAccess_CS(PixmapPtr pPix, int index); extern void RADEONFinishAccess_CS(PixmapPtr pPix, int index); -extern void *RADEONEXACreatePixmap(ScreenPtr pScreen, int size, int align); extern void *RADEONEXACreatePixmap2(ScreenPtr pScreen, int width, int height, int depth, int usage_hint, int bitsPerPixel, int *new_pitch); diff --git a/src/r600_exa.c b/src/r600_exa.c index c69b8fce7..a111dd456 100644 --- a/src/r600_exa.c +++ b/src/r600_exa.c @@ -2044,7 +2044,6 @@ R600DrawInit(ScreenPtr pScreen) info->accel_state->exa->MarkSync = R600MarkSync; info->accel_state->exa->WaitMarker = R600Sync; -info->accel_state->exa->CreatePixmap = RADEONEXACreatePixmap; info->accel_state->exa->DestroyPixmap = RADEONEXADestroyPixmap; info->accel_state->exa->PixmapIsOffscreen = RADEONEXAPixmapIsOffscreen; info->accel_state->exa->PrepareAccess = RADEONPrepareAccess_CS; diff --git a/src/r600_state.h b/src/r600_state.h index fda297d31..4898e8de8 100644 --- a/src/r600_state.h +++ b/src/r600_state.h @@ -316,7 +316,6 @@ R600SetAccelState(ScrnInfoPtr pScrn, extern Bool RADEONPrepareAccess_CS(PixmapPtr pPix, int index); extern void RADEONFinishAccess_CS(PixmapPtr pPix, int index); -extern void *RADEONEXACreatePixmap(ScreenPtr pScreen, int size, int align); extern void *RADEONEXACreatePixmap2(ScreenPtr pScreen, int width, int height, int depth, int usage_hint, int bitsPerPixel, int *new_pitch); diff --git a/src/radeon_exa.c b/src/radeon_exa.c index 9106d5c65..ef60bc0c1 100644 --- a/src/radeon_exa.c +++ b/src/radeon_exa.c @@ -235,35 +235,6 @@ void RADEONFinishAccess_CS(PixmapPtr pPix, int index) } -void *RADEONEXACreatePixmap(ScreenPtr pScreen, int size, int align) -{ -ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen); -RADEONInfoPtr info = RADEONPTR(pScrn); -struct radeon_exa_pixmap_priv *new_priv; - -if (size != 0 && !info->exa_force_create && - info->exa_pixmaps == FALSE) -return NULL; - -new_priv = calloc(1, sizeof(struct radeon_exa_pixmap_priv)); -if (!new_priv) - return NULL; - -if (size == 0) - return new_priv; - -new_priv->bo = radeon_bo_open(info->bufmgr, 0, size, align, - RADEON_GEM_DOMAIN_VRAM, 0); -if (!new_priv->bo) { - free(new_priv); - ErrorF("Failed to alloc memory\n"); - return NULL; -} - -return new_priv; - -} - void *RADEONEXACreatePixmap2(ScreenPtr pScreen, int width, int height, int depth, int usage_hint, int bitsPerPixel, int *new_pitch) diff --git a/src/radeon_exa_funcs.c b/src/radeon_exa_funcs.c index da0524ede..add89458f 100644 --- a/src/radeon_exa_funcs.c +++ b/src/radeon_exa_funcs.c @@ -638,7 +638,6 @@ Bool RADEONDrawInit(ScreenPtr pScreen) } #endif -info->accel_state->exa->CreatePixmap = RADEONEXACreatePixmap; info->accel_state->exa->DestroyPixmap = RADEONEXADestroyPixmap; info->accel_state->exa->PixmapIsOffscreen = RADEONEXAPixmapIsOffscreen; info->accel_state->exa->PrepareAccess = RADEONPrepareAccess_CS; -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati 07/10] Remove throttling from radeon_dri2_copy_region2
From: Jammy Zhou Throttling should be handled by the client-side drivers. Signed-off-by: Jammy Zhou (Ported from amdgpu commit 8a34a8149860ac15e83ccdbd8d9a527d8d3e5997) Signed-off-by: Michel Dänzer --- src/radeon_dri2.c | 22 -- 1 file changed, 22 deletions(-) diff --git a/src/radeon_dri2.c b/src/radeon_dri2.c index 3b75f66f3..6f4691276 100644 --- a/src/radeon_dri2.c +++ b/src/radeon_dri2.c @@ -336,9 +336,7 @@ radeon_dri2_copy_region2(ScreenPtr pScreen, Bool vsync; Bool translate = FALSE; int off_x = 0, off_y = 0; -PixmapPtr dst_ppix; -dst_ppix = dst_private->pixmap; src_drawable = _private->pixmap->drawable; dst_drawable = _private->pixmap->drawable; @@ -355,7 +353,6 @@ radeon_dri2_copy_region2(ScreenPtr pScreen, dst_drawable = DRI2UpdatePrime(drawable, dest_buffer); if (!dst_drawable) return; - dst_ppix = (PixmapPtr)dst_drawable; if (dst_drawable != drawable) translate = TRUE; } else @@ -379,26 +376,7 @@ radeon_dri2_copy_region2(ScreenPtr pScreen, (*gc->funcs->ChangeClip) (gc, CT_REGION, copy_clip, 0); ValidateGC(dst_drawable, gc); -/* If this is a full buffer swap or frontbuffer flush, throttle on the - * previous one - */ -if (dst_private->attachment == DRI2BufferFrontLeft) { - if (REGION_NUM_RECTS(region) == 1) { - BoxPtr extents = REGION_EXTENTS(pScreen, region); - - if (extents->x1 == 0 && extents->y1 == 0 && - extents->x2 == drawable->width && - extents->y2 == drawable->height) { - struct radeon_bo *bo = radeon_get_pixmap_bo(dst_ppix); - - if (bo) - radeon_bo_wait(bo); - } - } -} - vsync = info->accel_state->vsync; - /* Driver option "SwapbuffersWait" defines if we vsync DRI2 copy-swaps. */ info->accel_state->vsync = info->swapBuffersWait; info->accel_state->force = TRUE; -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati 00/10] glamor: Use GBM for BO allocation when possible
From: Michel Dänzer This series addresses https://bugs.freedesktop.org/105381 and is mostly inspired by the amdgpu driver. Patches 1 & 2 clean up things I noticed being unused while working on this series. Patches 3-5 are struct radeon_surface related cleanups. Patches 6-8 are flush/finish related preparations. Patch 9 lays the ground work for patch 10, which finally switches the glamor code to using GBM for BO allocation when possible. Jammy Zhou (1): Remove throttling from radeon_dri2_copy_region2 Michel Dänzer (9): Drop unused drmmode_create_bo_pixmap surface parameter EXA: Remove old RADEONEXACreatePixmap hook Only initialize libdrm_radeon surface manager for >= R600 glamor: Don't store radeon_surfaces in pixmaps Factor out radeon_surface_initialize helper Move flush from radeon_scanout_do_update to its callers Refactor radeon_finish helper Add struct radeon_buffer glamor: Use GBM for BO allocation when possible configure.ac| 10 + src/Makefile.am | 4 +- src/drmmode_display.c | 252 ++--- src/drmmode_display.h | 2 +- src/evergreen_exa.c | 41 ++- src/evergreen_state.h | 2 - src/evergreen_textured_videofuncs.c | 2 +- src/r600_exa.c | 41 ++- src/r600_state.h| 2 - src/r600_textured_videofuncs.c | 2 +- src/radeon.h| 61 ++--- src/radeon_bo_helper.c | 404 ++-- src/radeon_bo_helper.h | 55 +++- src/radeon_dri2.c | 49 +--- src/radeon_dri3.c | 15 +- src/radeon_exa.c| 46 +--- src/radeon_exa_funcs.c | 44 +-- src/radeon_exa_shared.c | 2 +- src/radeon_exa_shared.h | 3 +- src/radeon_glamor.c | 57 ++-- src/radeon_glamor.h | 28 +- src/radeon_glamor_wrappers.c| 27 +- src/radeon_kms.c| 202 ++ src/radeon_present.c| 4 +- src/radeon_textured_videofuncs.c| 24 +- 25 files changed, 705 insertions(+), 674 deletions(-) -- 2.18.0 ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
[PATCH xf86-video-ati 10/10] glamor: Use GBM for BO allocation when possible
From: Michel Dänzer Inspired by amdgpu. This avoids various issues due to a GEM handle lifetime conflict between us and Mesa with current glamor. Bugzilla: https://bugs.freedesktop.org/105381 Signed-off-by: Michel Dänzer --- configure.ac | 10 src/Makefile.am| 4 +- src/drmmode_display.c | 39 -- src/radeon.h | 7 ++- src/radeon_bo_helper.c | 118 - src/radeon_bo_helper.h | 20 ++- src/radeon_dri2.c | 7 +-- src/radeon_dri3.c | 13 +++-- src/radeon_glamor.c| 47 ++-- src/radeon_glamor.h| 28 +- src/radeon_kms.c | 40 +- 11 files changed, 266 insertions(+), 67 deletions(-) diff --git a/configure.ac b/configure.ac index 11efdf0ae..f5614749f 100644 --- a/configure.ac +++ b/configure.ac @@ -138,12 +138,22 @@ if test "x$GLAMOR" != "xno"; then [Have glamor_egl_destroy_textured_pixmap API])], [], [#include "xorg-server.h" #include "glamor.h"]) + + AC_CHECK_DECL(glamor_finish, + [AC_DEFINE(HAVE_GLAMOR_FINISH, 1, +[Have glamor_finish API])], +[PKG_CHECK_MODULES(LIBGL, [gl])], + [#include "xorg-server.h" + #include "glamor.h"]) fi if test "x$GLAMOR_XSERVER" != xyes; then PKG_CHECK_MODULES(LIBGLAMOR, [glamor >= 0.6.0]) PKG_CHECK_MODULES(LIBGLAMOR_EGL, [glamor-egl]) fi + + PKG_CHECK_MODULES(GBM, [gbm >= 10.6]) + AC_DEFINE(USE_GLAMOR, 1, [Enable glamor acceleration]) else AC_MSG_RESULT([$GLAMOR]) diff --git a/src/Makefile.am b/src/Makefile.am index ed1bfa9e6..df4a95e4f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -62,8 +62,8 @@ radeon_drv_la_SOURCES = \ $(RADEON_KMS_SRCS) if GLAMOR -AM_CFLAGS += @LIBGLAMOR_CFLAGS@ -radeon_drv_la_LIBADD += @LIBGLAMOR_LIBS@ +AM_CFLAGS += @LIBGLAMOR_CFLAGS@ @GBM_CFLAGS@ +radeon_drv_la_LIBADD += @LIBGLAMOR_LIBS@ @GBM_LIBS@ radeon_drv_la_SOURCES += \ radeon_glamor_wrappers.c \ radeon_glamor.c diff --git a/src/drmmode_display.c b/src/drmmode_display.c index c91f5bb20..2773ce672 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -142,8 +142,7 @@ static PixmapPtr drmmode_create_bo_pixmap(ScrnInfoPtr pScrn, } if (!info->use_glamor || - radeon_glamor_create_textured_pixmap(pixmap, - radeon_get_pixmap_private(pixmap))) + radeon_glamor_create_textured_pixmap(pixmap, bo)) return pixmap; fail: @@ -435,8 +434,14 @@ destroy_pixmap_for_fbcon(ScrnInfoPtr pScrn) /* XXX: The current GPUVM support in the kernel doesn't allow removing * the virtual address range for this BO, so we need to keep around * the pixmap to avoid breaking glamor with GPUVM +* +* Similarly, need to keep around the pixmap with current glamor, to +* avoid issues due to a GEM handle lifetime conflict between us and +* Mesa */ - if (info->use_glamor && info->ChipFamily >= CHIP_FAMILY_CAYMAN) + if (info->use_glamor && + (info->ChipFamily >= CHIP_FAMILY_CAYMAN || +xorgGetVersion() >= XORG_VERSION_NUMERIC(1,19,99,1,0))) return; if (info->fbcon_pixmap) @@ -2277,21 +2282,23 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height) scrn->displayWidth = pitch / cpp; + if (!info->use_glamor) { #if X_BYTE_ORDER == X_BIG_ENDIAN - switch (cpp) { - case 4: - tiling_flags |= RADEON_TILING_SWAP_32BIT; - break; - case 2: - tiling_flags |= RADEON_TILING_SWAP_16BIT; - break; - } - if (info->ChipFamily < CHIP_FAMILY_R600 && - info->r600_shadow_fb && tiling_flags) - tiling_flags |= RADEON_TILING_SURFACE; + switch (cpp) { + case 4: + tiling_flags |= RADEON_TILING_SWAP_32BIT; + break; + case 2: + tiling_flags |= RADEON_TILING_SWAP_16BIT; + break; + } + if (info->ChipFamily < CHIP_FAMILY_R600 && + info->r600_shadow_fb && tiling_flags) + tiling_flags |= RADEON_TILING_SURFACE; #endif - if (tiling_flags) - radeon_bo_set_tiling(info->front_buffer->bo.radeon, tiling_flags, pitch); + if (tiling_flags) + radeon_bo_set_ti
[PATCH xf86-video-ati 08/10] Refactor radeon_finish helper
From: Michel Dänzer Signed-off-by: Michel Dänzer --- src/drmmode_display.c| 16 +++- src/radeon.h | 1 + src/radeon_bo_helper.c | 10 ++ src/radeon_bo_helper.h | 3 +++ src/radeon_glamor_wrappers.c | 21 ++--- src/radeon_kms.c | 6 ++ src/radeon_present.c | 4 +--- 7 files changed, 30 insertions(+), 31 deletions(-) diff --git a/src/drmmode_display.c b/src/drmmode_display.c index 54b09730d..3c4d94fd9 100644 --- a/src/drmmode_display.c +++ b/src/drmmode_display.c @@ -726,8 +726,7 @@ drmmode_crtc_prime_scanout_update(xf86CrtcPtr crtc, DisplayModePtr mode, gc, 0, 0, mode->HDisplay, mode->VDisplay, 0, 0); FreeScratchGC(gc); - radeon_cs_flush_indirect(scrn); - radeon_bo_wait(drmmode_crtc->scanout[0].bo); + radeon_finish(scrn, drmmode_crtc->scanout[0].bo); } } @@ -785,8 +784,7 @@ drmmode_crtc_scanout_update(xf86CrtcPtr crtc, DisplayModePtr mode, radeon_scanout_do_update(crtc, scanout_id, screen->GetWindowPixmap(screen->root), *box); - radeon_cs_flush_indirect(scrn); - radeon_bo_wait(drmmode_crtc->scanout[scanout_id].bo); + radeon_finish(scrn, drmmode_crtc->scanout[scanout_id].bo); } } @@ -2240,7 +2238,6 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height) int aligned_height; uint32_t screen_size; int cpp = info->pixel_bytes; - struct radeon_bo *front_bo; struct radeon_surface surface; uint32_t tiling_flags = 0, base_align; PixmapPtr ppix = screen->GetScreenPixmap(screen); @@ -2249,12 +2246,6 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height) if (scrn->virtualX == width && scrn->virtualY == height) return TRUE; - front_bo = info->front_bo; - radeon_cs_flush_indirect(scrn); - - if (front_bo) - radeon_bo_wait(front_bo); - if (info->allowColorTiling && !info->shadow_primary) { if (info->ChipFamily >= CHIP_FAMILY_R600) { if (info->allowColorTiling2D) { @@ -2364,8 +2355,7 @@ drmmode_xf86crtc_resize (ScrnInfoPtr scrn, int width, int height) } radeon_pixmap_clear(ppix); - radeon_cs_flush_indirect(scrn); - radeon_bo_wait(info->front_bo); + radeon_finish(scrn, info->front_bo); for (i = 0; i < xf86_config->num_crtc; i++) { xf86CrtcPtr crtc = xf86_config->crtc[i]; diff --git a/src/radeon.h b/src/radeon.h index c2ae6606e..499e89f92 100644 --- a/src/radeon.h +++ b/src/radeon.h @@ -895,6 +895,7 @@ radeon_pixmap_get_fb(PixmapPtr pix) return *fb_ptr; } + #define CP_PACKET0(reg, n) \ (RADEON_CP_PACKET0 | ((n) << 16) | ((reg) >> 2)) #define CP_PACKET1(reg0, reg1) \ diff --git a/src/radeon_bo_helper.c b/src/radeon_bo_helper.c index 376589427..7cfe91265 100644 --- a/src/radeon_bo_helper.c +++ b/src/radeon_bo_helper.c @@ -235,6 +235,16 @@ radeon_alloc_pixmap_bo(ScrnInfoPtr pScrn, int width, int height, int depth, return bo; } + +/* Flush and wait for the BO to become idle */ +void +radeon_finish(ScrnInfoPtr scrn, struct radeon_bo *bo) +{ +radeon_cs_flush_indirect(scrn); +radeon_bo_wait(bo); +} + + /* Clear the pixmap contents to black */ void radeon_pixmap_clear(PixmapPtr pixmap) diff --git a/src/radeon_bo_helper.h b/src/radeon_bo_helper.h index e1856adb1..fa99201b2 100644 --- a/src/radeon_bo_helper.h +++ b/src/radeon_bo_helper.h @@ -28,6 +28,9 @@ radeon_alloc_pixmap_bo(ScrnInfoPtr pScrn, int width, int height, int depth, int usage_hint, int bitsPerPixel, int *new_pitch, struct radeon_surface *new_surface, uint32_t *new_tiling); +extern void +radeon_finish(ScrnInfoPtr scrn, struct radeon_bo *bo); + extern void radeon_pixmap_clear(PixmapPtr pixmap); diff --git a/src/radeon_glamor_wrappers.c b/src/radeon_glamor_wrappers.c index d73742528..94700a7b9 100644 --- a/src/radeon_glamor_wrappers.c +++ b/src/radeon_glamor_wrappers.c @@ -58,13 +58,13 @@ radeon_glamor_prepare_access_cpu(ScrnInfoPtr scrn, RADEONInfoPtr info, struct radeon_bo *bo = priv->bo; int ret; - /* When falling back to swrast, flush all pending operations */ - if (need_sync) { - glamor_block_handler(scrn->pScreen); - info->gpu_flushed++; - } - if (!pixmap->devPrivate.ptr) {
Re: [PATCH] drm/amd/display/dc/dce: Fix multiple potential integer overflows
On 2018-07-04 03:13 AM, Gustavo A. R. Silva wrote: > Add suffix ULL to constant 5 and cast variables target_pix_clk_khz and > feedback_divider to uint64_t in order to avoid multiple potential integer > overflows and give the compiler complete information about the proper > arithmetic to use. > > Notice that such constant and variables are used in contexts that > expect expressions of type uint64_t (64 bits, unsigned). The current > casts to uint64_t effectively apply to each expression as a whole, > but they do not prevent them from being evaluated using 32-bit > arithmetic instead of 64-bit arithmetic. > > Also, once the expressions are properly evaluated using 64-bit > arithmentic, there is no need for the parentheses that enclose > them. > > Addresses-Coverity-ID: 1460245 ("Unintentional integer overflow") > Addresses-Coverity-ID: 1460286 ("Unintentional integer overflow") > Addresses-Coverity-ID: 1460401 ("Unintentional integer overflow") > Fixes: 4562236b3bc0 ("drm/amd/dc: Add dc display driver (v2)") > Signed-off-by: Gustavo A. R. Silva > > [...] > > @@ -145,8 +145,8 @@ static bool calculate_fb_and_fractional_fb_divider( > * of fractional feedback decimal point and the fractional FB Divider > precision > * is 2 then the equation becomes (ullfeedbackDivider + 5*100) / (10*100))*/ > > - feedback_divider += (uint64_t) > - (5 * calc_pll_cs->fract_fb_divider_precision_factor); > + feedback_divider += 5UL * > + calc_pll_cs->fract_fb_divider_precision_factor; This should be 5ULL, as the commit log says, otherwise it's still only 32 bits on 32-bit platforms. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] drm/amdgpu: Reserve fence slots for command submission
On 2018-07-04 08:55 AM, Zhang, Jerry (Junwei) wrote: > On 07/04/2018 02:34 PM, Christian König wrote: >> Am 04.07.2018 um 05:02 schrieb Junwei Zhang: >>> From: Michel Dänzer >>> >>> Without this, there could not be enough slots, which could trigger the >>> BUG_ON in reservation_object_add_shared_fence. >>> >>> v2: >>> * Jump to the error label instead of returning directly (Jerry Zhang) >>> v3: >>> * Reserve slots for command submission after VM updates (Christian >>> König) >>> >>> Cc: sta...@vger.kernel.org >>> Bugzilla: https://bugs.freedesktop.org/106418 >>> Reported-by: mikhail.v.gavri...@gmail.com >>> Signed-off-by: Michel Dänzer >>> Signed-off-by: Junwei Zhang >> >> I would put that at the end of amdgpu_bo_vm_update_pte(), but that is >> only a minor nit pick. > > At first, I really put it at the end of amdgpu_bo_vm_update_pte(). > On the 2nd thought, that func may be called by others(although it's not > for now), so I move it out of there to the caller. Makes sense to me, FWIW. Thanks Junwei and Christian! P.S. Christian, any feedback on https://patchwork.freedesktop.org/patch/232204/ ? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Re: [PATCH] dma-buf: Move BUG_ON from _add_shared_fence to _add_shared_inplace
On 2018-07-04 10:31 AM, Christian König wrote: > Am 26.06.2018 um 16:31 schrieb Michel Dänzer: >> From: Michel Dänzer >> >> Fixes the BUG_ON spuriously triggering under the following >> circumstances: >> >> * ttm_eu_reserve_buffers processes a list containing multiple BOs using >> the same reservation object, so it calls >> reservation_object_reserve_shared with that reservation object once >> for each such BO. >> * In reservation_object_reserve_shared, old->shared_count == >> old->shared_max - 1, so obj->staged is freed in preparation of an >> in-place update. >> * ttm_eu_fence_buffer_objects calls reservation_object_add_shared_fence >> once for each of the BOs above, always with the same fence. >> * The first call adds the fence in the remaining free slot, after which >> old->shared_count == old->shared_max. > > Well, the explanation here is not correct. For multiple BOs using the > same reservation object we won't call > reservation_object_add_shared_fence() multiple times because we move > those to the duplicates list in ttm_eu_reserve_buffers(). > > But this bug can still happen because we call > reservation_object_add_shared_fence() manually with fences for the same > context in a couple of places. > > One prominent case which comes to my mind are for the VM BOs during > updates. Another possibility are VRAM BOs which need to be cleared. Thanks. How about the following: * ttm_eu_reserve_buffers calls reservation_object_reserve_shared. * In reservation_object_reserve_shared, shared_count == shared_max - 1, so obj->staged is freed in preparation of an in-place update. * ttm_eu_fence_buffer_objects calls reservation_object_add_shared_fence, after which shared_count == shared_max. * The amdgpu driver also calls reservation_object_add_shared_fence for the same reservation object, and the BUG_ON triggers. However, nothing bad would happen in reservation_object_add_shared_inplace, since all fences use the same context, so they can only occupy a single slot. Prevent this by moving the BUG_ON to where an overflow would actually happen (e.g. if a buggy caller didn't call reservation_object_reserve_shared before). Also, I'll add a reference to https://bugs.freedesktop.org/106418 in v2, as I suspect this fix is necessary under the circumstances described there as well. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx