[PATCH xf86-video-amdgpu 1/2] Add m4 directory

2018-08-24 Thread Michel Dänzer
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

2018-08-24 Thread Michel Dänzer
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

2018-08-28 Thread Michel Dänzer

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

2018-08-28 Thread Michel Dänzer
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

2018-08-28 Thread Michel Dänzer
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

2018-08-28 Thread Michel Dänzer
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

2018-08-29 Thread Michel Dänzer
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

2018-08-29 Thread Michel Dänzer
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

2018-08-29 Thread 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.


-- 
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

2018-08-22 Thread Michel Dänzer

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

2018-08-23 Thread Michel Dänzer
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

2018-08-23 Thread Michel Dänzer
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]

2018-07-20 Thread Michel Dänzer
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]

2018-07-20 Thread Michel Dänzer
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

2018-07-20 Thread Michel Dänzer
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

2018-07-20 Thread Michel Dänzer
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

2018-07-17 Thread Michel Dänzer
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

2018-07-17 Thread Michel Dänzer
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

2018-07-17 Thread Michel Dänzer
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

2018-07-18 Thread Michel Dänzer
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

2018-07-18 Thread Michel Dänzer
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

2018-07-18 Thread 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.


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

2018-07-18 Thread Michel Dänzer
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

2018-07-18 Thread Michel Dänzer
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

2018-07-18 Thread Michel Dänzer
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

2018-07-16 Thread Michel Dänzer
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

2018-07-16 Thread Michel Dänzer
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)

2018-07-16 Thread Michel Dänzer
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

2018-07-17 Thread 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:
>>>> 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

2018-07-17 Thread Michel Dänzer
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

2018-07-17 Thread Michel Dänzer
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

2018-07-17 Thread 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:
>>> 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

2018-07-17 Thread 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.


-- 
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

2018-07-23 Thread Michel Dänzer
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

2018-07-23 Thread Michel Dänzer
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

2018-07-23 Thread 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?


-- 
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

2018-07-23 Thread Michel Dänzer
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

2018-07-23 Thread Michel Dänzer
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

2018-07-19 Thread Michel Dänzer
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]

2018-07-19 Thread Michel Dänzer
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]

2018-07-19 Thread Michel Dänzer
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

2018-07-19 Thread Michel Dänzer
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

2018-07-19 Thread Michel Dänzer
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.

2018-07-19 Thread Michel Dänzer
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]

2018-07-19 Thread Michel Dänzer
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

2018-07-19 Thread Michel Dänzer
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

2018-07-19 Thread Michel Dänzer
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.

2018-07-19 Thread Michel Dänzer
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

2018-07-16 Thread Michel Dänzer
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

2018-07-16 Thread Michel Dänzer
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

2018-07-16 Thread 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'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

2018-07-25 Thread Michel Dänzer
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

2018-07-24 Thread Michel Dänzer
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

2018-07-24 Thread Michel Dänzer
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

2018-07-24 Thread Michel Dänzer
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

2018-07-24 Thread Michel Dänzer
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

2018-07-24 Thread Michel Dänzer
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

2018-07-24 Thread Michel Dänzer
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

2018-07-17 Thread Michel Dänzer
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

2018-07-17 Thread Michel Dänzer
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

2018-07-17 Thread Michel Dänzer
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

2018-08-29 Thread Michel Dänzer
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

2018-08-29 Thread Michel Dänzer
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

2018-08-29 Thread Michel Dänzer
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

2018-08-29 Thread Michel Dänzer
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

2018-08-29 Thread Michel Dänzer
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

2018-08-29 Thread Michel Dänzer
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

2018-08-31 Thread Michel Dänzer
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

2018-09-05 Thread Michel Dänzer
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

2018-09-05 Thread Michel Dänzer
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

2018-09-05 Thread Michel Dänzer
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

2018-09-05 Thread Michel Dänzer
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

2018-09-07 Thread Michel Dänzer
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

2018-09-07 Thread Michel Dänzer

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

2018-09-10 Thread Michel Dänzer
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

2018-09-10 Thread Michel Dänzer


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

2018-08-31 Thread Michel Dänzer
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""

2018-08-30 Thread Michel Dänzer
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

2018-08-30 Thread Michel Dänzer
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

2018-09-11 Thread 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.

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

2018-09-11 Thread 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? 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

2018-09-11 Thread Michel Dänzer
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

2018-09-11 Thread Michel Dänzer
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

2018-09-11 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-05 Thread Michel Dänzer
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

2018-07-04 Thread Michel Dänzer
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

2018-07-04 Thread Michel Dänzer
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

2018-07-04 Thread Michel Dänzer
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


<    6   7   8   9   10   11   12   13   14   15   >