[PATCH 3/3] drm/amdgpu/gfx: free memory of mqd backup

[Xiangliang Yu]

Need to free mqd backup when destroying ring.

Signed-off-by: Xiangliang Yu 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 2202f02..23cf6fa 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -7379,12 +7379,14 @@ static void gfx_v8_0_compute_mqd_soft_fini(struct 
amdgpu_device *adev)
 
for (i = 0; i < adev->gfx.num_compute_rings; i++) {
ring = >gfx.compute_ring[i];
+   kfree(adev->gfx.mec.mqd_backup[i]);
amdgpu_bo_free_kernel(>mqd_obj,
  >mqd_gpu_addr,
  >mqd_ptr);
}
 
ring = >gfx.kiq.ring;
+   kfree(adev->gfx.mec.mqd_backup[AMDGPU_MAX_COMPUTE_RINGS]);
amdgpu_bo_free_kernel(>mqd_obj,
  >mqd_gpu_addr,
  >mqd_ptr);
-- 
2.7.4

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

[PATCH 1/3] drm/amdgpu: change pointer of mqd_ptr to void

[Xiangliang Yu]

vi_mqd is only used by VI family but mqd_ptr is common for all
ASIC, so change the pointer to void.

Signed-off-by: Xiangliang Yu 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c| 26 +-
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 3fd4ce8..a1bfc4e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -162,7 +162,7 @@ struct amdgpu_ring {
u32 queue;
struct amdgpu_bo*mqd_obj;
uint64_tmqd_gpu_addr;
-   struct vi_mqd   *mqd_ptr;
+   void*mqd_ptr;
u32 doorbell_index;
booluse_doorbell;
unsignedwptr_offs;
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 772c42b..2202f02 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -4940,8 +4940,10 @@ static int gfx_v8_0_kiq_resume(struct amdgpu_device 
*adev)
gfx_v8_0_cp_compute_enable(adev, true);
 
ring = >gfx.kiq.ring;
-   if (!amdgpu_bo_kmap(ring->mqd_obj, (void **)>mqd_ptr)) {
-   r = gfx_v8_0_kiq_init_queue(ring, ring->mqd_ptr, 
ring->mqd_gpu_addr);
+   if (!amdgpu_bo_kmap(ring->mqd_obj, >mqd_ptr)) {
+   r = gfx_v8_0_kiq_init_queue(ring,
+   (struct vi_mqd *)ring->mqd_ptr,
+   ring->mqd_gpu_addr);
amdgpu_bo_kunmap(ring->mqd_obj);
ring->mqd_ptr = NULL;
if (r)
@@ -4952,8 +4954,10 @@ static int gfx_v8_0_kiq_resume(struct amdgpu_device 
*adev)
 
for (i = 0; i < adev->gfx.num_compute_rings; i++) {
ring = >gfx.compute_ring[i];
-   if (!amdgpu_bo_kmap(ring->mqd_obj, (void **)>mqd_ptr)) {
-   r = gfx_v8_0_kiq_init_queue(ring, ring->mqd_ptr, 
ring->mqd_gpu_addr);
+   if (!amdgpu_bo_kmap(ring->mqd_obj, >mqd_ptr)) {
+   r = gfx_v8_0_kiq_init_queue(ring,
+   (struct vi_mqd 
*)ring->mqd_ptr,
+   ring->mqd_gpu_addr);
amdgpu_bo_kunmap(ring->mqd_obj);
ring->mqd_ptr = NULL;
if (r)
@@ -7333,7 +7337,7 @@ static int gfx_v8_0_compute_mqd_soft_init(struct 
amdgpu_device *adev)
if (!ring->mqd_obj) {
r = amdgpu_bo_create_kernel(adev, sizeof(struct vi_mqd), 
PAGE_SIZE,
AMDGPU_GEM_DOMAIN_GTT, 
>mqd_obj,
-   >mqd_gpu_addr, (void 
**)>mqd_ptr);
+   >mqd_gpu_addr, 
>mqd_ptr);
if (r) {
dev_warn(adev->dev, "failed to create ring mqd ob 
(%d)", r);
return r;
@@ -7352,7 +7356,7 @@ static int gfx_v8_0_compute_mqd_soft_init(struct 
amdgpu_device *adev)
if (!ring->mqd_obj) {
r = amdgpu_bo_create_kernel(adev, sizeof(struct 
vi_mqd), PAGE_SIZE,
AMDGPU_GEM_DOMAIN_GTT, 
>mqd_obj,
-   >mqd_gpu_addr, 
(void **)>mqd_ptr);
+   >mqd_gpu_addr, 
>mqd_ptr);
if (r) {
dev_warn(adev->dev, "failed to create ring mqd 
ob (%d)", r);
return r;
@@ -7375,9 +7379,13 @@ static void gfx_v8_0_compute_mqd_soft_fini(struct 
amdgpu_device *adev)
 
for (i = 0; i < adev->gfx.num_compute_rings; i++) {
ring = >gfx.compute_ring[i];
-   amdgpu_bo_free_kernel(>mqd_obj, >mqd_gpu_addr, 
(void **)>mqd_ptr);
+   amdgpu_bo_free_kernel(>mqd_obj,
+ >mqd_gpu_addr,
+ >mqd_ptr);
}
 
ring = >gfx.kiq.ring;
-   amdgpu_bo_free_kernel(>mqd_obj, >mqd_gpu_addr, (void 
**)>mqd_ptr);
-}
\ No newline at end of file
+   amdgpu_bo_free_kernel(>mqd_obj,
+ >mqd_gpu_addr,
+ >mqd_ptr);
+}
-- 
2.7.4

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

Re: [PATCH 1/1] drm/amdgpu: export gfx capability by gpu info

[Edward O'Callaghan]

On 02/16/2017 04:46 PM, Zhang, Jerry wrote:
> Hi Edward,
> 
>> Question, is this a bit premature to have a new function and struct for 
>> this, could
>> it just be put in amdgpu_gfx for now or are we expecting amdgpu_gfx_cap to
>> start growing quickly with far more cap state info?
> Yes, we will add more in the future.
> 
> Ideally we'd like to add a feature structure to export all supported by 
> current amdgpu_device.
> Now it's only one for gfx, so I insert a structure in amdgpu_gfx, which was 
> verified by vulkan carrizo. 
> Then I send it out for guys review whether to add in the amdgpu_device 
> directly
> or keep current style till we have other features besides gfx.

Hmmk, its nothing to stress about I was just more curious about it.

Reviewed-by: Edward O'Callaghan 

Kindly,
Edward.

> 
> Regards,
> Jerry (Junwei Zhang)
> 
> Linux Base Graphics
> SRDC Software Development
> _
> 
> 
>> -Original Message-
>> From: Edward O'Callaghan [mailto:funfunc...@folklore1984.net]
>> Sent: Thursday, February 16, 2017 12:38
>> To: Zhang, Jerry; amd-gfx@lists.freedesktop.org
>> Subject: Re: [PATCH 1/1] drm/amdgpu: export gfx capability by gpu info
>>
>> Hi,
>>
>> Question, is this a bit premature to have a new function and struct for 
>> this, could
>> it just be put in amdgpu_gfx for now or are we expecting amdgpu_gfx_cap to
>> start growing quickly with far more cap state info?
>>
>> Kind Regards,
>> Edward.
>>
>> On 02/16/2017 01:53 PM, Junwei Zhang wrote:
>>> Change-Id: Ibf3e4dbb7deb83271adabc275c9b7a0e0652541a
>>> Signed-off-by: Junwei Zhang 
>>> ---
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu.h |  6 ++
>>>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  2 ++
>>>  drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c   |  6 ++
>>>  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   |  6 ++
>>>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 14 ++
>>>  include/uapi/drm/amdgpu_drm.h   |  1 +
>>>  6 files changed, 35 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> index 1ad3f08..cdc2b2a 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>>> @@ -869,6 +869,10 @@ struct amdgpu_gfx_funcs {
>>> void (*read_wave_sgprs)(struct amdgpu_device *adev, uint32_t simd,
>>> uint32_t wave, uint32_t start, uint32_t size, uint32_t *dst);  };
>>>
>>> +struct amdgpu_gfx_cap {
>>> +   uint32_t gc_double_offchip_lds_buf;
>>> +};
>>> +
>>>  struct amdgpu_gfx {
>>> struct mutexgpu_clock_mutex;
>>> struct amdgpu_gca_configconfig;
>>> @@ -911,6 +915,8 @@ struct amdgpu_gfx {
>>> /* reset mask */
>>> uint32_tgrbm_soft_reset;
>>> uint32_tsrbm_soft_reset;
>>> +
>>> +   struct amdgpu_gfx_cap   cap;
>>>  };
>>>
>>>  int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> index 7f59608..e7aa382 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>>> @@ -618,6 +618,8 @@ static int amdgpu_info_ioctl(struct drm_device *dev,
>> void *data, struct drm_file
>>> cap.flag |= AMDGPU_CAPABILITY_DIRECT_GMA_FLAG;
>>> cap.direct_gma_size = amdgpu_direct_gma_size;
>>> }
>>> +   cap.gc_double_offchip_lds_buf =
>>> +   adev->gfx.cap.gc_double_offchip_lds_buf;
>>> return copy_to_user(out, ,
>>> min((size_t)size, sizeof(cap))) ? -EFAULT : 
>>> 0;
>>> }
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>>> index ce75d46..a1e221b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
>>> @@ -1534,6 +1534,11 @@ static void gfx_v6_0_setup_spi(struct
>> amdgpu_device *adev,
>>> mutex_unlock(>grbm_idx_mutex);
>>>  }
>>>
>>> +static void gfx_v6_0_cap_init(struct amdgpu_device *adev) {
>>> +   adev->gfx.cap.gc_double_offchip_lds_buf = 1; }
>>> +
>>>  static void gfx_v6_0_gpu_init(struct amdgpu_device *adev)  {
>>> u32 gb_addr_config = 0;
>>> @@ -1692,6 +1697,7 @@ static void gfx_v6_0_gpu_init(struct amdgpu_device
>> *adev)
>>>  adev->gfx.config.max_cu_per_sh);
>>>
>>> gfx_v6_0_get_cu_info(adev);
>>> +   gfx_v6_0_cap_init(adev);
>>>
>>> WREG32(mmCP_QUEUE_THRESHOLDS, ((0x16 <<
>> CP_QUEUE_THRESHOLDS__ROQ_IB1_START__SHIFT) |
>>>(0x2b <<
>> CP_QUEUE_THRESHOLDS__ROQ_IB2_START__SHIFT)));
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>> index aaf66fe..e1e97ae 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>>> +++ 

Re: Change queue/pipe split between amdkfd and amdgpu

[Edward O'Callaghan]

On 02/16/2017 03:00 PM, Bridgman, John wrote:
> Any objections to authorizing Oded to post the kfdtest binary he is using to 
> some public place (if not there already) so others (like Andres) can test 
> changes which touch on amdkfd ? 
> 
> We should check it for embarrassing symbols but otherwise it should be OK.

someone was up late for a dead line? lol

> 
> That said, since we are getting perilously close to actually sending dGPU 
> support changes upstream we will need (IMO) to maintain a sanitized source 
> repo for kfdtest as well... sharing the binary just gets us started.
> 

Hi John,

Yes, this is the sort of thing I've been referring to for some time now.
We definitely need some kind of centralized mechanism to test/validate
kfd stuff so if you can get this out that would be great! A binary would
be a start, I am sure we can made do and its certainly better than
nothing, however source much like what happened with UMR would be of
course ideal.

I suggest to you that it would perhaps be good if we could arrange some
kind of IRC meeting regarding kfd? Since it seems there is a bit of
fragmented effort here. I have my own ioctl()'s locally for pinning for
my own project which I am not sure are suitable to just upstream as AMD
has its own take so what should we do? I heard so much about dGPU
support for a couple of years now but only seen bits thrown over the
wall. Can we begin a more serious incremental approach happening ASAP?
I created #amdkfd on freenode some time ago which a couple of interested
academics and users hang.

Kind Regards,
Edward.

> Thanks,
> John
> 
>> -Original Message-
>> From: Oded Gabbay [mailto:oded.gab...@gmail.com]
>> Sent: Friday, February 10, 2017 12:57 PM
>> To: Andres Rodriguez
>> Cc: Kuehling, Felix; Bridgman, John; amd-gfx@lists.freedesktop.org;
>> Deucher, Alexander; Jay Cornwall
>> Subject: Re: Change queue/pipe split between amdkfd and amdgpu
>>
>> I don't have a repo, nor do I have the source code.
>> It is a tool that we developed inside AMD (when I was working there), and
>> after I left AMD I got permission to use the binary for regressions testing.
>>
>> Oded
>>
>> On Fri, Feb 10, 2017 at 6:33 PM, Andres Rodriguez 
>> wrote:
>>> Hey Oded,
>>>
>>> Where can I find a repo with kfdtest?
>>>
>>> I tried looking here bit couldn't find it:
>>>
>>> https://cgit.freedesktop.org/~gabbayo/
>>>
>>> -Andres
>>>
>>>
>>>
>>> On 2017-02-10 05:35 AM, Oded Gabbay wrote:

 So the warning in dmesg is gone of course, but the test (that I
 mentioned in previous email) still fails, and this time it caused the
 kernel to crash. In addition, now other tests fail as well, e.g.
 KFDEventTest.SignalEvent

 I honestly suggest to take some time to debug this patch-set on an
 actual Kaveri machine and then re-send the patches.

 Thanks,
 Oded

 log of crash from KFDQMTest.CreateMultipleCpQueues:

 [  160.900137] kfd: qcm fence wait loop timeout expired [
 160.900143] kfd: the cp might be in an unrecoverable state due to an
 unsuccessful queues preemption [  160.916765] show_signal_msg: 36
 callbacks suppressed [  160.916771] kfdtest[2498]: segfault at
 17f8a ip 7f8ae932ee5d sp 7ffc52219cd0 error 4 in
 libhsakmt-1.so.0.0.1[7f8ae932b000+8000]
 [  163.152229] kfd: qcm fence wait loop timeout expired [
 163.152250] BUG: unable to handle kernel NULL pointer dereference at
 005a [  163.152299] IP:
 kfd_get_process_device_data+0x6/0x30 [amdkfd] [  163.152323] PGD
 2333aa067 [  163.152323] PUD 230f64067 [  163.152335] PMD 0

 [  163.152364] Oops:  [#1] SMP
 [  163.152379] Modules linked in: joydev edac_mce_amd edac_core
 input_leds kvm_amd snd_hda_codec_realtek kvm irqbypass
 snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel
>> snd_hda_codec
 crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_core
 snd_hwdep pcbc snd_pcm aesni_intel snd_seq_midi snd_seq_midi_event
 snd_rawmidi snd_seq aes_x86_64 crypto_simd snd_seq_device
>> glue_helper
 cryptd snd_timer snd fam15h_power k10temp soundcore i2c_piix4 shpchp
 tpm_infineon mac_hid parport_pc ppdev nfsd auth_rpcgss nfs_acl lockd
 lp grace sunrpc parport autofs4 hid_logitech_hidpp hid_logitech_dj
 hid_generic usbhid hid uas usb_storage amdkfd amd_iommu_v2 radeon
 i2c_algo_bit ttm drm_kms_helper syscopyarea ahci sysfillrect
 sysimgblt libahci fb_sys_fops drm r8169 mii fjes video [  163.152668]
 CPU: 3 PID: 2498 Comm: kfdtest Not tainted 4.10.0-rc5+ #3 [
 163.152695] Hardware name: Gigabyte Technology Co., Ltd. To be filled
 by O.E.M./F2A88XM-D3H, BIOS F5 01/09/2014 [  163.152735] task:
 995e73d16580 task.stack: b41144458000 [  163.152764] RIP:
 0010:kfd_get_process_device_data+0x6/0x30 [amdkfd] [  163.152790]
 RSP: 0018:b4114445bab0 EFLAGS: 00010246 [  163.152812] RAX:
 ffea 

RE: Change queue/pipe split between amdkfd and amdgpu

[Bridgman, John]

Any objections to authorizing Oded to post the kfdtest binary he is using to 
some public place (if not there already) so others (like Andres) can test 
changes which touch on amdkfd ? 

We should check it for embarrassing symbols but otherwise it should be OK.

That said, since we are getting perilously close to actually sending dGPU 
support changes upstream we will need (IMO) to maintain a sanitized source repo 
for kfdtest as well... sharing the binary just gets us started.

Thanks,
John

>-Original Message-
>From: Oded Gabbay [mailto:oded.gab...@gmail.com]
>Sent: Friday, February 10, 2017 12:57 PM
>To: Andres Rodriguez
>Cc: Kuehling, Felix; Bridgman, John; amd-gfx@lists.freedesktop.org;
>Deucher, Alexander; Jay Cornwall
>Subject: Re: Change queue/pipe split between amdkfd and amdgpu
>
>I don't have a repo, nor do I have the source code.
>It is a tool that we developed inside AMD (when I was working there), and
>after I left AMD I got permission to use the binary for regressions testing.
>
>Oded
>
>On Fri, Feb 10, 2017 at 6:33 PM, Andres Rodriguez 
>wrote:
>> Hey Oded,
>>
>> Where can I find a repo with kfdtest?
>>
>> I tried looking here bit couldn't find it:
>>
>> https://cgit.freedesktop.org/~gabbayo/
>>
>> -Andres
>>
>>
>>
>> On 2017-02-10 05:35 AM, Oded Gabbay wrote:
>>>
>>> So the warning in dmesg is gone of course, but the test (that I
>>> mentioned in previous email) still fails, and this time it caused the
>>> kernel to crash. In addition, now other tests fail as well, e.g.
>>> KFDEventTest.SignalEvent
>>>
>>> I honestly suggest to take some time to debug this patch-set on an
>>> actual Kaveri machine and then re-send the patches.
>>>
>>> Thanks,
>>> Oded
>>>
>>> log of crash from KFDQMTest.CreateMultipleCpQueues:
>>>
>>> [  160.900137] kfd: qcm fence wait loop timeout expired [
>>> 160.900143] kfd: the cp might be in an unrecoverable state due to an
>>> unsuccessful queues preemption [  160.916765] show_signal_msg: 36
>>> callbacks suppressed [  160.916771] kfdtest[2498]: segfault at
>>> 17f8a ip 7f8ae932ee5d sp 7ffc52219cd0 error 4 in
>>> libhsakmt-1.so.0.0.1[7f8ae932b000+8000]
>>> [  163.152229] kfd: qcm fence wait loop timeout expired [
>>> 163.152250] BUG: unable to handle kernel NULL pointer dereference at
>>> 005a [  163.152299] IP:
>>> kfd_get_process_device_data+0x6/0x30 [amdkfd] [  163.152323] PGD
>>> 2333aa067 [  163.152323] PUD 230f64067 [  163.152335] PMD 0
>>>
>>> [  163.152364] Oops:  [#1] SMP
>>> [  163.152379] Modules linked in: joydev edac_mce_amd edac_core
>>> input_leds kvm_amd snd_hda_codec_realtek kvm irqbypass
>>> snd_hda_codec_generic snd_hda_codec_hdmi snd_hda_intel
>snd_hda_codec
>>> crct10dif_pclmul crc32_pclmul ghash_clmulni_intel snd_hda_core
>>> snd_hwdep pcbc snd_pcm aesni_intel snd_seq_midi snd_seq_midi_event
>>> snd_rawmidi snd_seq aes_x86_64 crypto_simd snd_seq_device
>glue_helper
>>> cryptd snd_timer snd fam15h_power k10temp soundcore i2c_piix4 shpchp
>>> tpm_infineon mac_hid parport_pc ppdev nfsd auth_rpcgss nfs_acl lockd
>>> lp grace sunrpc parport autofs4 hid_logitech_hidpp hid_logitech_dj
>>> hid_generic usbhid hid uas usb_storage amdkfd amd_iommu_v2 radeon
>>> i2c_algo_bit ttm drm_kms_helper syscopyarea ahci sysfillrect
>>> sysimgblt libahci fb_sys_fops drm r8169 mii fjes video [  163.152668]
>>> CPU: 3 PID: 2498 Comm: kfdtest Not tainted 4.10.0-rc5+ #3 [
>>> 163.152695] Hardware name: Gigabyte Technology Co., Ltd. To be filled
>>> by O.E.M./F2A88XM-D3H, BIOS F5 01/09/2014 [  163.152735] task:
>>> 995e73d16580 task.stack: b41144458000 [  163.152764] RIP:
>>> 0010:kfd_get_process_device_data+0x6/0x30 [amdkfd] [  163.152790]
>>> RSP: 0018:b4114445bab0 EFLAGS: 00010246 [  163.152812] RAX:
>>> ffea RBX: 995e75909c00 RCX:
>>> 
>>> [  163.152841] RDX:  RSI: ffea RDI:
>>> 995e75909600
>>> [  163.152869] RBP: b4114445bae0 R08: 000252a5 R09:
>>> 0414
>>> [  163.152898] R10:  R11: b412d38d R12:
>>> ffc2
>>> [  163.152926] R13:  R14: 995e75909ca8 R15:
>>> 995e75909c00
>>> [  163.152956] FS:  7f8ae975e740() GS:995e7ed8()
>>> knlGS:
>>> [  163.152988] CS:  0010 DS:  ES:  CR0: 80050033 [
>>> 163.153012] CR2: 005a CR3: 0002216ab000 CR4:
>>> 000406e0
>>> [  163.153041] Call Trace:
>>> [  163.153059]  ? destroy_queues_cpsch+0x166/0x190 [amdkfd] [
>>> 163.153086]  execute_queues_cpsch+0x2e/0xc0 [amdkfd] [  163.153113]
>>> destroy_queue_cpsch+0xbd/0x140 [amdkfd] [  163.153139]
>>> pqm_destroy_queue+0x111/0x1d0 [amdkfd] [  163.153164]
>>> pqm_uninit+0x3f/0xb0 [amdkfd] [  163.153186]
>>> kfd_unbind_process_from_device+0x51/0xd0 [amdkfd] [  163.153214]
>>> iommu_pasid_shutdown_callback+0x20/0x30 [amdkfd] [  163.153239]
>>> mn_release+0x37/0x70 [amd_iommu_v2] [  163.153261]
>>> 

Re: [PATCH 1/1] drm/amdgpu: export gfx capability by gpu info

[Edward O'Callaghan]

Hi,

Question, is this a bit premature to have a new function and struct for
this, could it just be put in amdgpu_gfx for now or are we expecting
amdgpu_gfx_cap to start growing quickly with far more cap state info?

Kind Regards,
Edward.

On 02/16/2017 01:53 PM, Junwei Zhang wrote:
> Change-Id: Ibf3e4dbb7deb83271adabc275c9b7a0e0652541a
> Signed-off-by: Junwei Zhang 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h |  6 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  2 ++
>  drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c   |  6 ++
>  drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   |  6 ++
>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 14 ++
>  include/uapi/drm/amdgpu_drm.h   |  1 +
>  6 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 1ad3f08..cdc2b2a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -869,6 +869,10 @@ struct amdgpu_gfx_funcs {
>   void (*read_wave_sgprs)(struct amdgpu_device *adev, uint32_t simd, 
> uint32_t wave, uint32_t start, uint32_t size, uint32_t *dst);
>  };
>  
> +struct amdgpu_gfx_cap {
> + uint32_t gc_double_offchip_lds_buf;
> +};
> +
>  struct amdgpu_gfx {
>   struct mutexgpu_clock_mutex;
>   struct amdgpu_gca_configconfig;
> @@ -911,6 +915,8 @@ struct amdgpu_gfx {
>   /* reset mask */
>   uint32_tgrbm_soft_reset;
>   uint32_tsrbm_soft_reset;
> +
> + struct amdgpu_gfx_cap   cap;
>  };
>  
>  int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 7f59608..e7aa382 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -618,6 +618,8 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
> *data, struct drm_file
>   cap.flag |= AMDGPU_CAPABILITY_DIRECT_GMA_FLAG;
>   cap.direct_gma_size = amdgpu_direct_gma_size;
>   }
> + cap.gc_double_offchip_lds_buf =
> + adev->gfx.cap.gc_double_offchip_lds_buf;
>   return copy_to_user(out, ,
>   min((size_t)size, sizeof(cap))) ? -EFAULT : 
> 0;
>   }
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> index ce75d46..a1e221b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> @@ -1534,6 +1534,11 @@ static void gfx_v6_0_setup_spi(struct amdgpu_device 
> *adev,
>   mutex_unlock(>grbm_idx_mutex);
>  }
>  
> +static void gfx_v6_0_cap_init(struct amdgpu_device *adev)
> +{
> + adev->gfx.cap.gc_double_offchip_lds_buf = 1;
> +}
> +
>  static void gfx_v6_0_gpu_init(struct amdgpu_device *adev)
>  {
>   u32 gb_addr_config = 0;
> @@ -1692,6 +1697,7 @@ static void gfx_v6_0_gpu_init(struct amdgpu_device 
> *adev)
>adev->gfx.config.max_cu_per_sh);
>  
>   gfx_v6_0_get_cu_info(adev);
> + gfx_v6_0_cap_init(adev);
>  
>   WREG32(mmCP_QUEUE_THRESHOLDS, ((0x16 << 
> CP_QUEUE_THRESHOLDS__ROQ_IB1_START__SHIFT) |
>  (0x2b << 
> CP_QUEUE_THRESHOLDS__ROQ_IB2_START__SHIFT)));
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index aaf66fe..e1e97ae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -1876,6 +1876,11 @@ static void gmc_v7_0_init_compute_vmid(struct 
> amdgpu_device *adev)
>   mutex_unlock(>srbm_mutex);
>  }
>  
> +static void gfx_v7_0_cap_init(struct amdgpu_device *adev)
> +{
> + adev->gfx.cap.gc_double_offchip_lds_buf = 1;
> +}
> +
>  /**
>   * gfx_v7_0_gpu_init - setup the 3D engine
>   *
> @@ -1900,6 +1905,7 @@ static void gfx_v7_0_gpu_init(struct amdgpu_device 
> *adev)
>  
>   gfx_v7_0_setup_rb(adev);
>   gfx_v7_0_get_cu_info(adev);
> + gfx_v7_0_cap_init(adev);
>  
>   /* set HW defaults for 3D engine */
>   WREG32(mmCP_MEQ_THRESHOLDS,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index ce05e38..934fc71 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -3826,6 +3826,19 @@ static void gfx_v8_0_init_compute_vmid(struct 
> amdgpu_device *adev)
>   mutex_unlock(>srbm_mutex);
>  }
>  
> +static void gfx_v8_0_cap_init(struct amdgpu_device *adev)
> +{
> + switch (adev->asic_type) {
> + default:
> + adev->gfx.cap.gc_double_offchip_lds_buf = 1;
> + break;
> + case CHIP_CARRIZO:
> + case CHIP_STONEY:
> + adev->gfx.cap.gc_double_offchip_lds_buf = 0;
> + break;
> + }
> +}
> +
>  static void 

[PATCH 1/1] drm/amdgpu: export gfx capability by gpu info

[Junwei Zhang]

Change-Id: Ibf3e4dbb7deb83271adabc275c9b7a0e0652541a
Signed-off-by: Junwei Zhang 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h |  6 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c |  2 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c   |  6 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   |  6 ++
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 14 ++
 include/uapi/drm/amdgpu_drm.h   |  1 +
 6 files changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 1ad3f08..cdc2b2a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -869,6 +869,10 @@ struct amdgpu_gfx_funcs {
void (*read_wave_sgprs)(struct amdgpu_device *adev, uint32_t simd, 
uint32_t wave, uint32_t start, uint32_t size, uint32_t *dst);
 };
 
+struct amdgpu_gfx_cap {
+   uint32_t gc_double_offchip_lds_buf;
+};
+
 struct amdgpu_gfx {
struct mutexgpu_clock_mutex;
struct amdgpu_gca_configconfig;
@@ -911,6 +915,8 @@ struct amdgpu_gfx {
/* reset mask */
uint32_tgrbm_soft_reset;
uint32_tsrbm_soft_reset;
+
+   struct amdgpu_gfx_cap   cap;
 };
 
 int amdgpu_ib_get(struct amdgpu_device *adev, struct amdgpu_vm *vm,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 7f59608..e7aa382 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -618,6 +618,8 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
cap.flag |= AMDGPU_CAPABILITY_DIRECT_GMA_FLAG;
cap.direct_gma_size = amdgpu_direct_gma_size;
}
+   cap.gc_double_offchip_lds_buf =
+   adev->gfx.cap.gc_double_offchip_lds_buf;
return copy_to_user(out, ,
min((size_t)size, sizeof(cap))) ? -EFAULT : 
0;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
index ce75d46..a1e221b 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
@@ -1534,6 +1534,11 @@ static void gfx_v6_0_setup_spi(struct amdgpu_device 
*adev,
mutex_unlock(>grbm_idx_mutex);
 }
 
+static void gfx_v6_0_cap_init(struct amdgpu_device *adev)
+{
+   adev->gfx.cap.gc_double_offchip_lds_buf = 1;
+}
+
 static void gfx_v6_0_gpu_init(struct amdgpu_device *adev)
 {
u32 gb_addr_config = 0;
@@ -1692,6 +1697,7 @@ static void gfx_v6_0_gpu_init(struct amdgpu_device *adev)
 adev->gfx.config.max_cu_per_sh);
 
gfx_v6_0_get_cu_info(adev);
+   gfx_v6_0_cap_init(adev);
 
WREG32(mmCP_QUEUE_THRESHOLDS, ((0x16 << 
CP_QUEUE_THRESHOLDS__ROQ_IB1_START__SHIFT) |
   (0x2b << 
CP_QUEUE_THRESHOLDS__ROQ_IB2_START__SHIFT)));
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
index aaf66fe..e1e97ae 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
@@ -1876,6 +1876,11 @@ static void gmc_v7_0_init_compute_vmid(struct 
amdgpu_device *adev)
mutex_unlock(>srbm_mutex);
 }
 
+static void gfx_v7_0_cap_init(struct amdgpu_device *adev)
+{
+   adev->gfx.cap.gc_double_offchip_lds_buf = 1;
+}
+
 /**
  * gfx_v7_0_gpu_init - setup the 3D engine
  *
@@ -1900,6 +1905,7 @@ static void gfx_v7_0_gpu_init(struct amdgpu_device *adev)
 
gfx_v7_0_setup_rb(adev);
gfx_v7_0_get_cu_info(adev);
+   gfx_v7_0_cap_init(adev);
 
/* set HW defaults for 3D engine */
WREG32(mmCP_MEQ_THRESHOLDS,
diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index ce05e38..934fc71 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -3826,6 +3826,19 @@ static void gfx_v8_0_init_compute_vmid(struct 
amdgpu_device *adev)
mutex_unlock(>srbm_mutex);
 }
 
+static void gfx_v8_0_cap_init(struct amdgpu_device *adev)
+{
+   switch (adev->asic_type) {
+   default:
+   adev->gfx.cap.gc_double_offchip_lds_buf = 1;
+   break;
+   case CHIP_CARRIZO:
+   case CHIP_STONEY:
+   adev->gfx.cap.gc_double_offchip_lds_buf = 0;
+   break;
+   }
+}
+
 static void gfx_v8_0_gpu_init(struct amdgpu_device *adev)
 {
u32 tmp, sh_static_mem_cfg;
@@ -3839,6 +3852,7 @@ static void gfx_v8_0_gpu_init(struct amdgpu_device *adev)
gfx_v8_0_tiling_mode_table_init(adev);
gfx_v8_0_setup_rb(adev);
gfx_v8_0_get_cu_info(adev);
+   gfx_v8_0_cap_init(adev);
 
/* XXX SH_MEM regs */
/* where to put LDS, scratch, GPUVM in FSA64 space */
diff --git a/include/uapi/drm/amdgpu_drm.h b/include/uapi/drm/amdgpu_drm.h

Re: [PATCH v2 3/3] drm/amdgpu: fix lock cleanup during buffer creation

[Michel Dänzer]

On 16/02/17 04:10 AM, Nicolai Hähnle wrote:
> From: Nicolai Hähnle 
> 
> Open-code the initial ttm_bo_validate call, so that we can properly
> unlock the reservation lock when it fails. Also, properly destruct
> the reservation object when the first part of TTM BO initialization
> fails.
> 
> Actual deadlocks caused by the missing unlock should have been fixed
> by "drm/ttm: never add BO that failed to validate to the LRU list",
> superseding the flawed fix in commit 38fc4856ad98 ("drm/amdgpu: fix
> a potential deadlock in amdgpu_bo_create_restricted()").
> 
> This change fixes remaining recursive locking errors that can be seen
> with lock debugging enabled, and avoids the error of freeing a locked
> mutex.
> 
> v2: use ttm_bo_unref for error handling, and rebase on latest changes
> 
> Fixes: 12a852219583 ("drm/amdgpu: improve AMDGPU_GEM_CREATE_VRAM_CLEARED 
> handling (v2)")
> Signed-off-by: Nicolai Hähnle 
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 28 ++--
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> index c2e57f7..15944ea 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> @@ -404,15 +404,31 @@ int amdgpu_bo_create_restricted(struct amdgpu_device 
> *adev,
>   }
>  
>   initial_bytes_moved = atomic64_read(>num_bytes_moved);
> - r = ttm_bo_init(>mman.bdev, >tbo, size, type,
> - >placement, page_align, !kernel, NULL,
> - acc_size, sg, resv ? resv : >tbo.ttm_resv,
> - _ttm_bo_destroy);
> + r = ttm_bo_init_top(>mman.bdev, >tbo, size, type,
> + page_align, NULL,
> + acc_size, sg, resv ? resv : >tbo.ttm_resv,
> + _ttm_bo_destroy);
> +
> + if (likely(r == 0))
> + r = ttm_bo_validate(>tbo, >placement, !kernel, false);
> +
> + if (unlikely(r != 0)) {
> + struct ttm_buffer_object *tbo = >tbo;
> +
> + if (!resv)
> + ww_mutex_unlock(>tbo.ttm_resv.lock);
> + ttm_bo_unref();
> + return r;
> + }

I think this would be clearer as

if (unlikely(r != 0)) {
struct ttm_buffer_object *tbo = >tbo;

if (!resv)
ww_mutex_unlock(>tbo.ttm_resv.lock);
ttm_bo_unref();
return r;
}

r = ttm_bo_validate(>tbo, >placement, !kernel, false);
[...]


>   amdgpu_cs_report_moved_bytes(adev,
>   atomic64_read(>num_bytes_moved) - initial_bytes_moved);
>  
> - if (unlikely(r != 0))
> - return r;
> + if (!(bo->tbo.mem.placement & TTM_PL_FLAG_NO_EVICT)) {
> + spin_lock(>tbo.glob->lru_lock);
> + ttm_bo_add_to_lru(>tbo);
> + spin_unlock(>tbo.glob->lru_lock);
> + }

It's a bit ugly to open-code this logic in driver code. Maybe add
another TTM helper which calls ttm_bo_validate and ttm_bo_add_to_lru?


-- 
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 v4 2/2] drm/amdgpu: expose GPU sensor related information

[Samuel Pitoiset]

This includes shader/memory clocks, temperature, GPU load, etc.

v2: - add sub-queries for AMDPGU_INFO_GPU_SENSOR_*
- do not break the ABI
v3: - return -ENOENT when amdgpu_dpm == 0
- expose more sensor queries
v4: - s/GPU_POWER/GPU_AVG_POWER/
- improve VDDNB/VDDGFX query description
- fix amdgpu_dpm check

Signed-off-by: Samuel Pitoiset 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 75 +
 include/uapi/drm/amdgpu_drm.h   | 20 +
 3 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index f275a6b54e9f..e9d58aeb30fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -59,9 +59,10 @@
  * - 3.7.0 - Add support for VCE clock list packet
  * - 3.8.0 - Add support raster config init in the kernel
  * - 3.9.0 - Add support for memory query info about VRAM and GTT.
+ * - 3.10.0 - Add support for sensor query info (clocks, temp, etc).
  */
 #define KMS_DRIVER_MAJOR   3
-#define KMS_DRIVER_MINOR   9
+#define KMS_DRIVER_MINOR   10
 #define KMS_DRIVER_PATCHLEVEL  0
 
 int amdgpu_vram_limit = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index d5f9d6a4b661..d76fa3bbca1d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -241,6 +241,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
uint32_t ui32 = 0;
uint64_t ui64 = 0;
int i, found;
+   int ui32_size = sizeof(ui32);
 
if (!info->return_size || !info->return_pointer)
return -EINVAL;
@@ -597,6 +598,80 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
return -EINVAL;
}
}
+   case AMDGPU_INFO_SENSOR: {
+   struct pp_gpu_power query = {0};
+   int size = sizeof(query);
+
+   if (amdgpu_dpm == 0)
+   return -ENOENT;
+
+   switch (info->sensor_info.type) {
+   case AMDGPU_INFO_SENSOR_GFX_SCLK:
+   /* get sclk in Mhz */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_GFX_SCLK,
+  (void *), _size)) {
+   return -EINVAL;
+   }
+   ui32 /= 100;
+   break;
+   case AMDGPU_INFO_SENSOR_GFX_MCLK:
+   /* get mclk in Mhz */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_GFX_MCLK,
+  (void *), _size)) {
+   return -EINVAL;
+   }
+   ui32 /= 100;
+   break;
+   case AMDGPU_INFO_SENSOR_GPU_TEMP:
+   /* get temperature in millidegrees C */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_GPU_TEMP,
+  (void *), _size)) {
+   return -EINVAL;
+   }
+   break;
+   case AMDGPU_INFO_SENSOR_GPU_LOAD:
+   /* get GPU load */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_GPU_LOAD,
+  (void *), _size)) {
+   return -EINVAL;
+   }
+   break;
+   case AMDGPU_INFO_SENSOR_GPU_AVG_POWER:
+   /* get average GPU power */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_GPU_POWER,
+  (void *), )) {
+   return -EINVAL;
+   }
+   ui32 = query.average_gpu_power >> 8;
+   break;
+   case AMDGPU_INFO_SENSOR_VDDNB:
+   /* get VDDNB in millivolts */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_VDDNB,
+  (void *), _size)) {
+   return -EINVAL;
+   }
+   break;
+   case AMDGPU_INFO_SENSOR_VDDGFX:
+   /* get VDDGFX in millivolts */
+   if 

Re: [PATCH v3 2/2] drm/amdgpu: expose GPU sensor related information

[Samuel Pitoiset]

On 02/15/2017 08:15 PM, Samuel Pitoiset wrote:

This includes shader/memory clocks, temperature, GPU load, etc.

v2: - add sub-queries for AMDPGU_INFO_GPU_SENSOR_*
- do not break the ABI
v3: - return -ENOENT when amdgpu_dpm == 0
- expose more sensor queries

Signed-off-by: Samuel Pitoiset 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 75 +
 include/uapi/drm/amdgpu_drm.h   | 20 +
 3 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index f275a6b54e9f..e9d58aeb30fb 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -59,9 +59,10 @@
  * - 3.7.0 - Add support for VCE clock list packet
  * - 3.8.0 - Add support raster config init in the kernel
  * - 3.9.0 - Add support for memory query info about VRAM and GTT.
+ * - 3.10.0 - Add support for sensor query info (clocks, temp, etc).
  */
 #define KMS_DRIVER_MAJOR   3
-#define KMS_DRIVER_MINOR   9
+#define KMS_DRIVER_MINOR   10
 #define KMS_DRIVER_PATCHLEVEL  0

 int amdgpu_vram_limit = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index d5f9d6a4b661..8de57db99449 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -241,6 +241,7 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
uint32_t ui32 = 0;
uint64_t ui64 = 0;
int i, found;
+   int ui32_size = sizeof(ui32);

if (!info->return_size || !info->return_pointer)
return -EINVAL;
@@ -597,6 +598,80 @@ static int amdgpu_info_ioctl(struct drm_device *dev, void 
*data, struct drm_file
return -EINVAL;
}
}
+   case AMDGPU_INFO_SENSOR: {
+   struct pp_gpu_power query = {0};
+   int size = sizeof(query);
+
+   if (amdgpu_dpm != 0)


if (amdgpu_dpm == 0)
   return -ENOENT;

Fixed locally.


+   return -ENOENT;
+
+   switch (info->sensor_info.type) {
+   case AMDGPU_INFO_SENSOR_GFX_SCLK:
+   /* get sclk in Mhz */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_GFX_SCLK,
+  (void *), _size)) {
+   return -EINVAL;
+   }
+   ui32 /= 100;
+   break;
+   case AMDGPU_INFO_SENSOR_GFX_MCLK:
+   /* get mclk in Mhz */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_GFX_MCLK,
+  (void *), _size)) {
+   return -EINVAL;
+   }
+   ui32 /= 100;
+   break;
+   case AMDGPU_INFO_SENSOR_GPU_TEMP:
+   /* get temperature in millidegrees C */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_GPU_TEMP,
+  (void *), _size)) {
+   return -EINVAL;
+   }
+   break;
+   case AMDGPU_INFO_SENSOR_GPU_LOAD:
+   /* get GPU load */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_GPU_LOAD,
+  (void *), _size)) {
+   return -EINVAL;
+   }
+   break;
+   case AMDGPU_INFO_SENSOR_GPU_POWER:
+   /* get average GPU power */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_GPU_POWER,
+  (void *), )) {
+   return -EINVAL;
+   }
+   ui32 = query.average_gpu_power >> 8;
+   break;
+   case AMDGPU_INFO_SENSOR_VDDNB:
+   /* get VDDNB in millivolts */
+   if (amdgpu_dpm_read_sensor(adev,
+  AMDGPU_PP_SENSOR_VDDNB,
+  (void *), _size)) {
+   return -EINVAL;
+   }
+   break;
+   case AMDGPU_INFO_SENSOR_VDDGFX:
+   /* get VDDGFX in millivolts */
+   if 

[PATCH v2 2/3] drm/ttm: fix the documentation of ttm_bo_init

[Nicolai Hähnle]

From: Nicolai Hähnle 

As the comment says: callers of ttm_bo_init cannot rely on having the
only reference to the BO when the function returns successfully.

Signed-off-by: Nicolai Hähnle 
---
 include/drm/ttm/ttm_bo_api.h | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 6e8eaee..5add713 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -520,7 +520,11 @@ extern int ttm_bo_init_top(struct ttm_bo_device *bdev,
  * As this object may be part of a larger structure, this function,
  * together with the @destroy function,
  * enables driver-specific objects derived from a ttm_buffer_object.
- * On successful return, the object kref and list_kref are set to 1.
+ *
+ * On successful return, the caller owns an object kref to @bo. The kref and
+ * list_kref are usually set to 1, but note that in some situations, other
+ * tasks may already be holding references to @bo as well.
+ *
  * If a failure occurs, the function will call the @destroy function, or
  * kfree() if @destroy is NULL. Thus, after a failure, dereferencing @bo is
  * illegal and will likely cause memory corruption.
-- 
2.9.3

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

[PATCH v2 3/3] drm/amdgpu: fix lock cleanup during buffer creation

[Nicolai Hähnle]

From: Nicolai Hähnle 

Open-code the initial ttm_bo_validate call, so that we can properly
unlock the reservation lock when it fails. Also, properly destruct
the reservation object when the first part of TTM BO initialization
fails.

Actual deadlocks caused by the missing unlock should have been fixed
by "drm/ttm: never add BO that failed to validate to the LRU list",
superseding the flawed fix in commit 38fc4856ad98 ("drm/amdgpu: fix
a potential deadlock in amdgpu_bo_create_restricted()").

This change fixes remaining recursive locking errors that can be seen
with lock debugging enabled, and avoids the error of freeing a locked
mutex.

v2: use ttm_bo_unref for error handling, and rebase on latest changes

Fixes: 12a852219583 ("drm/amdgpu: improve AMDGPU_GEM_CREATE_VRAM_CLEARED 
handling (v2)")
Signed-off-by: Nicolai Hähnle 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 28 ++--
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index c2e57f7..15944ea 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -404,15 +404,31 @@ int amdgpu_bo_create_restricted(struct amdgpu_device 
*adev,
}
 
initial_bytes_moved = atomic64_read(>num_bytes_moved);
-   r = ttm_bo_init(>mman.bdev, >tbo, size, type,
-   >placement, page_align, !kernel, NULL,
-   acc_size, sg, resv ? resv : >tbo.ttm_resv,
-   _ttm_bo_destroy);
+   r = ttm_bo_init_top(>mman.bdev, >tbo, size, type,
+   page_align, NULL,
+   acc_size, sg, resv ? resv : >tbo.ttm_resv,
+   _ttm_bo_destroy);
+
+   if (likely(r == 0))
+   r = ttm_bo_validate(>tbo, >placement, !kernel, false);
+
+   if (unlikely(r != 0)) {
+   struct ttm_buffer_object *tbo = >tbo;
+
+   if (!resv)
+   ww_mutex_unlock(>tbo.ttm_resv.lock);
+   ttm_bo_unref();
+   return r;
+   }
+
amdgpu_cs_report_moved_bytes(adev,
atomic64_read(>num_bytes_moved) - initial_bytes_moved);
 
-   if (unlikely(r != 0))
-   return r;
+   if (!(bo->tbo.mem.placement & TTM_PL_FLAG_NO_EVICT)) {
+   spin_lock(>tbo.glob->lru_lock);
+   ttm_bo_add_to_lru(>tbo);
+   spin_unlock(>tbo.glob->lru_lock);
+   }
 
bo->tbo.priority = ilog2(bo->tbo.num_pages);
if (kernel)
-- 
2.9.3

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

Re: [PATCH v3] drm/amdgpu: expose amdgpu_sensors on pre-powerplay chips

[Tom St Denis]

On 15/02/17 01:32 PM, Samuel Pitoiset wrote:

read_sensor() has been recently implemented for dpm based boards
which means amdgpu_sensors can now be exposed.

v2: - make sure read_sensor is not NULL on dpm chips
- keep sanity check for powerplay chips
v3: - make sure amdgpu_dpm != 0

Cc: Tom St Denis 
Signed-off-by: Samuel Pitoiset 


Cool, you can throw my R-b on this and the first patch (which adds the 
sensors).  Christian/Alex/etc can RB the DRM part.


Reviewed-by: Tom St Denis 

Thanks,
Tom



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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6f021e70f15f..d63c44383660 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3198,12 +3198,18 @@ static ssize_t amdgpu_debugfs_sensor_read(struct file 
*f, char __user *buf,
if (size & 3 || *pos & 0x3)
return -EINVAL;

+   if (amdgpu_dpm == 0)
+   return -EINVAL;
+
/* convert offset to sensor number */
idx = *pos >> 2;

valuesize = sizeof(values);
if (adev->powerplay.pp_funcs && adev->powerplay.pp_funcs->read_sensor)
r = adev->powerplay.pp_funcs->read_sensor(adev->powerplay.pp_handle, 
idx, [0], );
+   else if (adev->pm.funcs && adev->pm.funcs->read_sensor)
+   r = adev->pm.funcs->read_sensor(adev, idx, [0],
+   );
else
return -EINVAL;




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

[PATCH v3] drm/amdgpu: expose amdgpu_sensors on pre-powerplay chips

[Samuel Pitoiset]

read_sensor() has been recently implemented for dpm based boards
which means amdgpu_sensors can now be exposed.

v2: - make sure read_sensor is not NULL on dpm chips
- keep sanity check for powerplay chips
v3: - make sure amdgpu_dpm != 0

Cc: Tom St Denis 
Signed-off-by: Samuel Pitoiset 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 6f021e70f15f..d63c44383660 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -3198,12 +3198,18 @@ static ssize_t amdgpu_debugfs_sensor_read(struct file 
*f, char __user *buf,
if (size & 3 || *pos & 0x3)
return -EINVAL;
 
+   if (amdgpu_dpm == 0)
+   return -EINVAL;
+
/* convert offset to sensor number */
idx = *pos >> 2;
 
valuesize = sizeof(values);
if (adev->powerplay.pp_funcs && adev->powerplay.pp_funcs->read_sensor)
r = 
adev->powerplay.pp_funcs->read_sensor(adev->powerplay.pp_handle, idx, 
[0], );
+   else if (adev->pm.funcs && adev->pm.funcs->read_sensor)
+   r = adev->pm.funcs->read_sensor(adev, idx, [0],
+   );
else
return -EINVAL;
 
-- 
2.11.1

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

Re: [PATCH] drm/amdgpu: change pte definitions to 64 bit

[Christian König]

Am 15.02.2017 um 19:09 schrieb Alex Xie:

Change-Id: Iaeeb253cc5c028c93c536ee6ea08213c7fa8f299
Signed-off-by: Alex Xie 


Ah, good catch. I've missed that one.

Reviewed-by: Christian König .


---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
index 51fa12f..cc389af 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
@@ -63,7 +63,7 @@ struct amdgpu_bo_list_entry;
  #define AMDGPU_PTE_READABLE   (1ULL << 5)
  #define AMDGPU_PTE_WRITEABLE  (1ULL << 6)
  
-#define AMDGPU_PTE_FRAG(x)	((x & 0x1f) << 7)

+#define AMDGPU_PTE_FRAG(x) ((x & 0x1fULL) << 7)
  
  #define AMDGPU_PTE_PRT		(1ULL << 63)
  



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

Re: [PATCH 1/3] drm/amdgpu: minor PRT turnoff fix

[Nicolai Hähnle]

Hi Christian,

On 15.02.2017 16:59, Christian König wrote:

Nicolai could you give that set a try?

It should fix your problems with PRT tear down on process crash.


Yes, it fixes those issues for me, thanks! The first two patches have my 
R-b, for the third one I don't really understand the bug that it fixes, 
but I have to leave soon, so...


Thanks
Nicolai


Regards,
Christian.

Am 15.02.2017 um 15:57 schrieb Christian König:

From: Christian König 

When two VMs stop using PRT support at the same time we might
not disable it in the right order otherwise.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index bc32239..447cda5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1200,7 +1200,8 @@ static void amdgpu_vm_prt_cb(struct fence
*fence, struct fence_cb *_cb)
  {
  struct amdgpu_prt_cb *cb = container_of(_cb, struct
amdgpu_prt_cb, cb);
  -amdgpu_vm_update_prt_state(cb->adev);
+if (atomic_dec_return(>adev->vm_manager.num_prt_mappings) == 0)
+amdgpu_vm_update_prt_state(cb->adev);
  kfree(cb);
  }
  @@ -1219,17 +1220,14 @@ static void amdgpu_vm_free_mapping(struct
amdgpu_device *adev,
 struct amdgpu_bo_va_mapping *mapping,
 struct fence *fence)
  {
-if ((mapping->flags & AMDGPU_PTE_PRT) &&
-atomic_dec_return(>vm_manager.num_prt_mappings) == 0) {
+if (mapping->flags & AMDGPU_PTE_PRT) {
  struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct
amdgpu_prt_cb),
 GFP_KERNEL);
cb->adev = adev;
  if (!fence || fence_add_callback(fence, >cb,
- amdgpu_vm_prt_cb)) {
-amdgpu_vm_update_prt_state(adev);
-kfree(cb);
-}
+ amdgpu_vm_prt_cb))
+amdgpu_vm_prt_cb(fence, >cb);
  }
  kfree(mapping);
  }



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


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

Re: [PATCH 1/3] drm/amdgpu: minor PRT turnoff fix

[Christian König]

Nicolai could you give that set a try?

It should fix your problems with PRT tear down on process crash.

Regards,
Christian.

Am 15.02.2017 um 15:57 schrieb Christian König:

From: Christian König 

When two VMs stop using PRT support at the same time we might
not disable it in the right order otherwise.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index bc32239..447cda5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1200,7 +1200,8 @@ static void amdgpu_vm_prt_cb(struct fence *fence, struct 
fence_cb *_cb)
  {
struct amdgpu_prt_cb *cb = container_of(_cb, struct amdgpu_prt_cb, cb);
  
-	amdgpu_vm_update_prt_state(cb->adev);

+   if (atomic_dec_return(>adev->vm_manager.num_prt_mappings) == 0)
+   amdgpu_vm_update_prt_state(cb->adev);
kfree(cb);
  }
  
@@ -1219,17 +1220,14 @@ static void amdgpu_vm_free_mapping(struct amdgpu_device *adev,

   struct amdgpu_bo_va_mapping *mapping,
   struct fence *fence)
  {
-   if ((mapping->flags & AMDGPU_PTE_PRT) &&
-   atomic_dec_return(>vm_manager.num_prt_mappings) == 0) {
+   if (mapping->flags & AMDGPU_PTE_PRT) {
struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct amdgpu_prt_cb),
   GFP_KERNEL);
  
  		cb->adev = adev;

if (!fence || fence_add_callback(fence, >cb,
-amdgpu_vm_prt_cb)) {
-   amdgpu_vm_update_prt_state(adev);
-   kfree(cb);
-   }
+amdgpu_vm_prt_cb))
+   amdgpu_vm_prt_cb(fence, >cb);
}
kfree(mapping);
  }



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

Re: [PATCH] drm/amdgpu: expose amdgpu_sensors on pre-powerplay chips

[Samuel Pitoiset]

On 02/15/2017 04:01 PM, Tom St Denis wrote:

On 14/02/17 09:56 PM, Deucher, Alexander wrote:

-Original Message-
From: Michel Dänzer [mailto:mic...@daenzer.net]
Sent: Tuesday, February 14, 2017 9:52 PM
To: Deucher, Alexander; 'Samuel Pitoiset'; StDenis, Tom
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: expose amdgpu_sensors on pre-
powerplay chips

On 15/02/17 01:17 AM, Deucher, Alexander wrote:

From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On

Behalf

Of Samuel Pitoiset


Otherwise, it seems to work though the GPU_TEMP reads as "5000" on

my

Kaveri (meaning temp of 5C which isn't true).

I suspect the dpm code is reading the wrong register to get the
temp but
we can fix that in another change later on.


No idea. It was just a copy-n-paste. Maybe the initial code is buggy?


I don’t know that the GPU temp sensor on CI based APUs actually works
properly.  I think the package temperature is exposed via the CPU
thermals.


Seems to work fine with radeon on my two Kaveri laptops, FWIW.

On my desktop Kaveri, unplausibly low values (around 9-10C) are reported
at idle for both GPU and CPU. With load, both go up to around 30C, so it
seems like the values make sense in a relative sense but not in an
absolute one.


I think  I vaguely recall that it might actually be power rather than
temperature.  Need to check with the SMU guys.



On my retail kaveri (with latest bios...) it seems to track with load
but isn't really accurate (it'll jump double digits back and forth)

Eitherway though if Samuel updates the patches to include a check for
amdgpu_drm we should be able to commit them.


Sure, I will send a v3.



I've already created a patch for umr to read them, just waiting on the
kernel side to land.

Tom

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

Re: [PATCH] drm/amd/display: Fix 64-bit division, yet again

[Emil Velikov]

On 15 February 2017 at 14:47, Harry Wentland  wrote:
> On 2017-02-15 06:44 AM, Emil Velikov wrote:
>>
>> On 14 February 2017 at 20:36, Harry Wentland 
>> wrote:
>>>
>>> Also make the code somewhat more readable.
>>>
>> I'd suggest reaching to the team to integrate scripts/checkpatch.pl in
>> the pre-commit hook.
>>
>> It will help you improve the coding standard and, as you mentioned, it
>> "make[s] the code somewhat more readable".
>>
>
> Thanks, Emil. We actually have that hooked up at pre-submission in our
> internal repo but don't block submission on warnings and allow a couple
> people to push anyways despite errors because we seem to get some
> unnecessary errors on complex macros that can't really be handled
> differently.
>
Some ideas:
 - nobody is exempt - everyone has to go through the checks/hooks
 - can not push if there are more than X warnings and Y errors
 - whitelist patches that include changes to the problematic macros
Btw, do you recall which macros are responsible?

> Need to think on how to improve that. It's a pain being the bad guy
> constantly and people don't always see the importance of good coding and
> commit msg style.
>
One possible solution is to check that the commit message is at least
A% of the diffstat - refusing to commit + sending a bunch of links
about why you want good commit messages.

Afaict all of the above can be automated, thus nobody will be an
exception to the rule. With time all that can go away, but until then
it will save you/Alex a) time and b) being the bad guys.
It will take a few rounds to tweaking the X/Y/A numbers though ;-)

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

Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function

[Nicolai Hähnle]

On 15.02.2017 14:35, Nicolai Hähnle wrote:

On 14.02.2017 13:51, Christian König wrote:

Am 14.02.2017 um 13:00 schrieb Nicolai Hähnle:

On 14.02.2017 11:49, Christian König wrote:

Am 14.02.2017 um 11:37 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.

Signed-off-by: Nicolai Hähnle 


Please squash that into your other patch. It fixes another bug, but I
don't think fixing one bug just to run into another is really a good
idea.


I don't understand. I'm not aware that this patch fixes anything, it
just enables the subsequent fix in amdgpu in patch #2. I don't think
squashing those together is a good idea (one is in ttm, the other in
amdgpu).


Ok, forget it I've messed up the different reference count.

With at least initializing bo->kref and bo->destroy before returning the
first error the patch is Reviewed-by: Christian König
.


Thanks. Does this apply to patches #2 and #3 as well?


Well, there's some minor necessary rebase fixes, so I'll probably just 
send out a new version once I got to test it.


Nicolai

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

Re: [PATCH] drm/amdgpu: expose amdgpu_sensors on pre-powerplay chips

[Tom St Denis]

On 14/02/17 09:56 PM, Deucher, Alexander wrote:

-Original Message-
From: Michel Dänzer [mailto:mic...@daenzer.net]
Sent: Tuesday, February 14, 2017 9:52 PM
To: Deucher, Alexander; 'Samuel Pitoiset'; StDenis, Tom
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: expose amdgpu_sensors on pre-
powerplay chips

On 15/02/17 01:17 AM, Deucher, Alexander wrote:

From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On

Behalf

Of Samuel Pitoiset


Otherwise, it seems to work though the GPU_TEMP reads as "5000" on

my

Kaveri (meaning temp of 5C which isn't true).

I suspect the dpm code is reading the wrong register to get the temp but
we can fix that in another change later on.


No idea. It was just a copy-n-paste. Maybe the initial code is buggy?


I don’t know that the GPU temp sensor on CI based APUs actually works
properly.  I think the package temperature is exposed via the CPU
thermals.


Seems to work fine with radeon on my two Kaveri laptops, FWIW.

On my desktop Kaveri, unplausibly low values (around 9-10C) are reported
at idle for both GPU and CPU. With load, both go up to around 30C, so it
seems like the values make sense in a relative sense but not in an
absolute one.


I think  I vaguely recall that it might actually be power rather than 
temperature.  Need to check with the SMU guys.



On my retail kaveri (with latest bios...) it seems to track with load 
but isn't really accurate (it'll jump double digits back and forth)


Eitherway though if Samuel updates the patches to include a check for 
amdgpu_drm we should be able to commit them.


I've already created a patch for umr to read them, just waiting on the 
kernel side to land.


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

[PATCH 1/3] drm/amdgpu: minor PRT turnoff fix

[Christian König]

From: Christian König 

When two VMs stop using PRT support at the same time we might
not disable it in the right order otherwise.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index bc32239..447cda5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1200,7 +1200,8 @@ static void amdgpu_vm_prt_cb(struct fence *fence, struct 
fence_cb *_cb)
 {
struct amdgpu_prt_cb *cb = container_of(_cb, struct amdgpu_prt_cb, cb);
 
-   amdgpu_vm_update_prt_state(cb->adev);
+   if (atomic_dec_return(>adev->vm_manager.num_prt_mappings) == 0)
+   amdgpu_vm_update_prt_state(cb->adev);
kfree(cb);
 }
 
@@ -1219,17 +1220,14 @@ static void amdgpu_vm_free_mapping(struct amdgpu_device 
*adev,
   struct amdgpu_bo_va_mapping *mapping,
   struct fence *fence)
 {
-   if ((mapping->flags & AMDGPU_PTE_PRT) &&
-   atomic_dec_return(>vm_manager.num_prt_mappings) == 0) {
+   if (mapping->flags & AMDGPU_PTE_PRT) {
struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct amdgpu_prt_cb),
   GFP_KERNEL);
 
cb->adev = adev;
if (!fence || fence_add_callback(fence, >cb,
-amdgpu_vm_prt_cb)) {
-   amdgpu_vm_update_prt_state(adev);
-   kfree(cb);
-   }
+amdgpu_vm_prt_cb))
+   amdgpu_vm_prt_cb(fence, >cb);
}
kfree(mapping);
 }
-- 
2.5.0

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

[PATCH 3/3] drm/amdgpu: fix PRT teardown on VM fini v2

[Christian König]

From: Christian König 

v2: new approach fixing this by registering a fence callback for
all users of the VM on teardown

Signed-off-by: Christian König 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 106 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h |   2 +-
 2 files changed, 82 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index c11b6b6..d3437ad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1188,22 +1188,31 @@ static void amdgpu_vm_update_prt_state(struct 
amdgpu_device *adev)
bool enable;
 
spin_lock_irqsave(>vm_manager.prt_lock, flags);
-   enable = !!atomic_read(>vm_manager.num_prt_mappings);
+   enable = !!atomic_read(>vm_manager.num_prt_users);
adev->gart.gart_funcs->set_prt(adev, enable);
spin_unlock_irqrestore(>vm_manager.prt_lock, flags);
 }
 
 /**
+ * amdgpu_vm_prt_put - add a PRT user
+ */
+static void amdgpu_vm_prt_get(struct amdgpu_device *adev)
+{
+   if (atomic_inc_return(>vm_manager.num_prt_users) == 1)
+   amdgpu_vm_update_prt_state(adev);
+}
+
+/**
  * amdgpu_vm_prt_put - drop a PRT user
  */
 static void amdgpu_vm_prt_put(struct amdgpu_device *adev)
 {
-   if (atomic_dec_return(>vm_manager.num_prt_mappings) == 0)
+   if (atomic_dec_return(>vm_manager.num_prt_users) == 0)
amdgpu_vm_update_prt_state(adev);
 }
 
 /**
- * amdgpu_vm_prt - callback for updating the PRT status
+ * amdgpu_vm_prt_cb - callback for updating the PRT status
  */
 static void amdgpu_vm_prt_cb(struct fence *fence, struct fence_cb *_cb)
 {
@@ -1214,6 +1223,29 @@ static void amdgpu_vm_prt_cb(struct fence *fence, struct 
fence_cb *_cb)
 }
 
 /**
+ * amdgpu_vm_add_prt_cb - add callback for updating the PRT status
+ */
+static void amdgpu_vm_add_prt_cb(struct amdgpu_device *adev,
+struct fence *fence)
+{
+   struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct amdgpu_prt_cb),
+  GFP_KERNEL);
+
+   if (!cb) {
+   /* Last resort when we are OOM */
+   if (fence)
+   fence_wait(fence, false);
+
+   amdgpu_vm_prt_put(cb->adev);
+   } else {
+   cb->adev = adev;
+   if (!fence || fence_add_callback(fence, >cb,
+amdgpu_vm_prt_cb))
+   amdgpu_vm_prt_cb(fence, >cb);
+   }
+}
+
+/**
  * amdgpu_vm_free_mapping - free a mapping
  *
  * @adev: amdgpu_device pointer
@@ -1228,24 +1260,47 @@ static void amdgpu_vm_free_mapping(struct amdgpu_device 
*adev,
   struct amdgpu_bo_va_mapping *mapping,
   struct fence *fence)
 {
-   if (mapping->flags & AMDGPU_PTE_PRT) {
-   struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct amdgpu_prt_cb),
-  GFP_KERNEL);
+   if (mapping->flags & AMDGPU_PTE_PRT)
+   amdgpu_vm_add_prt_cb(adev, fence);
+   kfree(mapping);
+}
 
-   if (!cb) {
-   /* Last resort when we are OOM */
-   if (fence)
-   fence_wait(fence, false);
+/**
+ * amdgpu_vm_prt_fini - finish all prt mappings
+ *
+ * @adev: amdgpu_device pointer
+ * @vm: requested vm
+ *
+ * Register a cleanup callback to disable PRT support after VM dies.
+ */
+static void amdgpu_vm_prt_fini(struct amdgpu_device *adev, struct amdgpu_vm 
*vm)
+{
+   struct reservation_object *resv = vm->page_directory->tbo.resv;
+   struct fence *excl, **shared;
+   unsigned i, shared_count;
+   int r;
 
-   amdgpu_vm_prt_put(cb->adev);
-   } else {
-   cb->adev = adev;
-   if (!fence || fence_add_callback(fence, >cb,
-amdgpu_vm_prt_cb))
-   amdgpu_vm_prt_cb(fence, >cb);
-   }
+   r = reservation_object_get_fences_rcu(resv, ,
+ _count, );
+   if (r) {
+   /* Not enough memory to grab the fence list, as last resort
+* block for all the fences to complete.
+*/
+   reservation_object_wait_timeout_rcu(resv, true, false,
+   MAX_SCHEDULE_TIMEOUT);
+   return;
}
-   kfree(mapping);
+
+   /* Add a callback for each fence in the reservation object */
+   amdgpu_vm_prt_get(adev);
+   amdgpu_vm_add_prt_cb(adev, excl);
+
+   for (i = 0; i < shared_count; ++i) {
+   amdgpu_vm_prt_get(adev);
+   amdgpu_vm_add_prt_cb(adev, shared[i]);
+   }
+
+   

[PATCH 2/3] drm/amdgpu: add OOM fallback on PRT teardown

[Christian König]

From: Christian König 

Don't assume kmalloc will always succeed.

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 447cda5..c11b6b6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -1194,14 +1194,22 @@ static void amdgpu_vm_update_prt_state(struct 
amdgpu_device *adev)
 }
 
 /**
+ * amdgpu_vm_prt_put - drop a PRT user
+ */
+static void amdgpu_vm_prt_put(struct amdgpu_device *adev)
+{
+   if (atomic_dec_return(>vm_manager.num_prt_mappings) == 0)
+   amdgpu_vm_update_prt_state(adev);
+}
+
+/**
  * amdgpu_vm_prt - callback for updating the PRT status
  */
 static void amdgpu_vm_prt_cb(struct fence *fence, struct fence_cb *_cb)
 {
struct amdgpu_prt_cb *cb = container_of(_cb, struct amdgpu_prt_cb, cb);
 
-   if (atomic_dec_return(>adev->vm_manager.num_prt_mappings) == 0)
-   amdgpu_vm_update_prt_state(cb->adev);
+   amdgpu_vm_prt_put(cb->adev);
kfree(cb);
 }
 
@@ -1224,10 +1232,18 @@ static void amdgpu_vm_free_mapping(struct amdgpu_device 
*adev,
struct amdgpu_prt_cb *cb = kmalloc(sizeof(struct amdgpu_prt_cb),
   GFP_KERNEL);
 
-   cb->adev = adev;
-   if (!fence || fence_add_callback(fence, >cb,
-amdgpu_vm_prt_cb))
-   amdgpu_vm_prt_cb(fence, >cb);
+   if (!cb) {
+   /* Last resort when we are OOM */
+   if (fence)
+   fence_wait(fence, false);
+
+   amdgpu_vm_prt_put(cb->adev);
+   } else {
+   cb->adev = adev;
+   if (!fence || fence_add_callback(fence, >cb,
+amdgpu_vm_prt_cb))
+   amdgpu_vm_prt_cb(fence, >cb);
+   }
}
kfree(mapping);
 }
-- 
2.5.0

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

Re: [PATCH] drm/amd/display: Fix 64-bit division, yet again

[Harry Wentland]

On 2017-02-15 06:44 AM, Emil Velikov wrote:

On 14 February 2017 at 20:36, Harry Wentland  wrote:

Also make the code somewhat more readable.


I'd suggest reaching to the team to integrate scripts/checkpatch.pl in
the pre-commit hook.

It will help you improve the coding standard and, as you mentioned, it
"make[s] the code somewhat more readable".



Thanks, Emil. We actually have that hooked up at pre-submission in our 
internal repo but don't block submission on warnings and allow a couple 
people to push anyways despite errors because we seem to get some 
unnecessary errors on complex macros that can't really be handled 
differently.


Need to think on how to improve that. It's a pain being the bad guy 
constantly and people don't always see the importance of good coding and 
commit msg style.


Harry


-Emil


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

Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function

[Nicolai Hähnle]

On 15.02.2017 04:16, zhoucm1 wrote:



On 2017年02月14日 18:37, Nicolai Hähnle wrote:

From: Nicolai Hähnle 

Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.

Signed-off-by: Nicolai Hähnle 
---
  drivers/gpu/drm/ttm/ttm_bo.c | 62
+---
  include/drm/ttm/ttm_bo_api.h | 45 
  2 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
  }
  EXPORT_SYMBOL(ttm_bo_validate);
  -int ttm_bo_init(struct ttm_bo_device *bdev,
-struct ttm_buffer_object *bo,
-unsigned long size,
-enum ttm_bo_type type,
-struct ttm_placement *placement,
-uint32_t page_alignment,
-bool interruptible,
-struct file *persistent_swap_storage,
-size_t acc_size,
-struct sg_table *sg,
-struct reservation_object *resv,
-void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+uint32_t page_alignment,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
  {
  int ret = 0;
  unsigned long num_pages;
  struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
-bool locked;
ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);
  if (ret) {
  pr_err("Out of kernel memory\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  return -ENOMEM;
  }
num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
  if (num_pages == 0) {
  pr_err("Illegal buffer object size\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  ttm_mem_global_free(mem_glob, acc_size);
  return -EINVAL;
  }
@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
  ret = drm_vma_offset_add(>vma_manager, >vma_node,
   bo->mem.num_pages);

if (ret && !resv), we should call
reservation_object_fini(>ttm_resv), right?


FWIW, you were right about this (and also mutex_destroy needs to be 
called for wu_mutex, etc.). But I'm following Christian's suggestion of 
having the caller use ttm_bo_unref for error cleanup, so all this error 
cleanup needn't be duplicated.


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

Re: next build: 208 builds: 3 failed, 205 passed, 5 errors, 23 warnings (next-20170215)

[James Hogan]

On Wed, Feb 15, 2017 at 09:56:48AM +0100, Arnd Bergmann wrote:
> On Wed, Feb 15, 2017 at 9:30 AM, kernelci.org bot  wrote:
> > bcm47xx_defconfig (mips) — FAIL, 1 error, 0 warnings, 0 section mismatches
> >
> > Errors:
> > /home/buildslave/workspace/khilman-kbuilder/next/build-mips/../net/sched/sch_fq_codel.c:468:
> > undefined reference to `tcf_destroy_chain'
> 
> I have not looked at this one yet, appears to be caused by commit
> cf1facda2f61 ("sched: move tcf_proto_destroy and tcf_destroy_chain
> helpers into cls_api")

So effectively CONFIG_NET_SCH_FQ_CODEL (which bcm47xx_defconfig sets
=y), and just over half of the other packet schedulers now implicitly
depend on CONFIG_NET_CLS (which bcm47xx_defconfig doesn't set =y).

Perhaps revert would be best since the change looks of questionable
value to me, as all the users of it are in sch_*.c anyway.

> > ip27_defconfig (mips) — FAIL, 2 errors, 0 warnings, 0 section mismatches
> >
> > Errors:
> > drivers/net/ethernet/qlogic/qlge/qlge_main.c:4819:1: error: insn does not
> > satisfy its constraints:
> > drivers/net/ethernet/qlogic/qlge/qlge_main.c:4819:1: internal compiler
> > error: in extract_constrain_insn, at recog.c:2190
> 
> broken gcc release, apparently fixed in gcc-7 (can't reproduce here at least).
> I suggested a workaround, but got no reply so far:
> 
> http://www.spinics.net/lists/mips/msg66465.html

I'll look into that.

> 
> > xway_defconfig (mips) — FAIL, 2 errors, 0 warnings, 0 section mismatches
> >
> > Errors:
> > (.text+0x14e10): undefined reference to `physical_memsize'
> > (.text+0x14e14): undefined reference to `physical_memsize'
> 
> Hauke already did a patch in December, but it has't made it into linux-mips
> so far:
> 
> https://marc.info/?l=linux-mips=148612428414708=3

Hauke: you mentioned sending a new version of this patch. I'll apply the
original patch for now, as looking at the VPE loader, zero doesn't sound
entirely unreasonable:

/*
 * The sde-kit passes 'memsize' to __start in $a3, so set something
 * here...  Or set $a3 to zero and define DFLT_STACK_SIZE and
 * DFLT_HEAP_SIZE when you compile your program
 */
mttgpr(6, v->ntcs);
mttgpr(7, physical_memsize);

Cheers
James


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

Re: next build: 208 builds: 3 failed, 205 passed, 5 errors, 23 warnings (next-20170215)

[Jiri Pirko]

Wed, Feb 15, 2017 at 10:38:07AM CET, james.ho...@imgtec.com wrote:
>On Wed, Feb 15, 2017 at 09:56:48AM +0100, Arnd Bergmann wrote:
>> On Wed, Feb 15, 2017 at 9:30 AM, kernelci.org bot  wrote:
>> > bcm47xx_defconfig (mips) — FAIL, 1 error, 0 warnings, 0 section mismatches
>> >
>> > Errors:
>> > /home/buildslave/workspace/khilman-kbuilder/next/build-mips/../net/sched/sch_fq_codel.c:468:
>> > undefined reference to `tcf_destroy_chain'
>> 
>> I have not looked at this one yet, appears to be caused by commit
>> cf1facda2f61 ("sched: move tcf_proto_destroy and tcf_destroy_chain
>> helpers into cls_api")
>
>So effectively CONFIG_NET_SCH_FQ_CODEL (which bcm47xx_defconfig sets
>=y), and just over half of the other packet schedulers now implicitly
>depend on CONFIG_NET_CLS (which bcm47xx_defconfig doesn't set =y).
>
>Perhaps revert would be best since the change looks of questionable
>value to me, as all the users of it are in sch_*.c anyway.

Fixing this now.


>
>> > ip27_defconfig (mips) — FAIL, 2 errors, 0 warnings, 0 section mismatches
>> >
>> > Errors:
>> > drivers/net/ethernet/qlogic/qlge/qlge_main.c:4819:1: error: insn does not
>> > satisfy its constraints:
>> > drivers/net/ethernet/qlogic/qlge/qlge_main.c:4819:1: internal compiler
>> > error: in extract_constrain_insn, at recog.c:2190
>> 
>> broken gcc release, apparently fixed in gcc-7 (can't reproduce here at 
>> least).
>> I suggested a workaround, but got no reply so far:
>> 
>> http://www.spinics.net/lists/mips/msg66465.html
>
>I'll look into that.
>
>> 
>> > xway_defconfig (mips) — FAIL, 2 errors, 0 warnings, 0 section mismatches
>> >
>> > Errors:
>> > (.text+0x14e10): undefined reference to `physical_memsize'
>> > (.text+0x14e14): undefined reference to `physical_memsize'
>> 
>> Hauke already did a patch in December, but it has't made it into linux-mips
>> so far:
>> 
>> https://marc.info/?l=linux-mips=148612428414708=3
>
>Hauke: you mentioned sending a new version of this patch. I'll apply the
>original patch for now, as looking at the VPE loader, zero doesn't sound
>entirely unreasonable:
>
>/*
> * The sde-kit passes 'memsize' to __start in $a3, so set something
> * here...  Or set $a3 to zero and define DFLT_STACK_SIZE and
> * DFLT_HEAP_SIZE when you compile your program
> */
>mttgpr(6, v->ntcs);
>mttgpr(7, physical_memsize);
>
>Cheers
>James


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

Re: next build: 208 builds: 3 failed, 205 passed, 5 errors, 23 warnings (next-20170215)

[Arnd Bergmann]

On Wed, Feb 15, 2017 at 9:30 AM, kernelci.org bot  wrote:
> next build: 208 builds: 3 failed, 205 passed, 5 errors, 23 warnings

> Errors and Warnings Detected:
>
> arm64: gcc version 5.3.1 20160412 (Linaro GCC 5.3-2016.05)
> defconfig+CONFIG_KASAN=y 4 warnings
> arm: gcc version 5.3.1 20160412 (Linaro GCC 5.3-2016.05)
> allmodconfig 7 warnings
> imx_v6_v7_defconfig 2 warnings
> mips: gcc version 6.3.0 (GCC)
> bcm47xx_defconfig 1 error
> ip27_defconfig 2 errors
> xway_defconfig 2 errors
> x86: gcc version 4.8.4 (Ubuntu 4.8.4-2ubuntu1~14.04.1)
> allmodconfig 5 warnings
> allmodconfig+CONFIG_OF=n 5 warnings

A couple of extra problems here that Olof's build bot didn't find (I
replied on those
already, won't duplicate those). In total, we seem to be nine patches short of
a clean build now, which is the best we've had since kernelci added MIPS
builds (we had a clean build for one day before that).

> allmodconfig (arm) — PASS, 0 errors, 7 warnings, 0 section mismatches
>
> Warnings:
> drivers/iio/adc/rcar-gyroadc.c:429:27: warning: 'num_channels' may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> drivers/iio/adc/rcar-gyroadc.c:426:22: warning: 'sample_width' may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> drivers/iio/adc/rcar-gyroadc.c:428:23: warning: 'channels' may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> drivers/iio/adc/rcar-gyroadc.c:398:26: warning: 'adcmode' may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> drivers/media/platform/coda/imx-vdoa.c:333:181: warning: passing argument 1
> of '__platform_driver_register' discards 'const' qualifier from pointer
> target type [-Wdiscarded-qualifiers]
> drivers/media/platform/coda/imx-vdoa.c:333:625: warning: passing argument 1
> of 'platform_driver_unregister' discards 'const' qualifier from pointer
> target type [-Wdiscarded-qualifiers]

Covered in my reply to Olof's build bot.

> drivers/usb/gadget/udc/atmel_usba_udc.c:632:550: warning: 'ept_cfg' may be
> used uninitialized in this function [-Wmaybe-uninitialized]

My patch was acked, but not picked up by Felipe or Greg so far:

https://www.spinics.net/lists/linux-usb/msg153162.html

> allmodconfig (x86) — PASS, 0 errors, 5 warnings, 0 section mismatches
>
> Warnings:
> lib/raid6/recov_avx512.c:387:2: warning: #warning "your version of binutils
> lacks AVX512 support" [-Wcpp]

long-standing problem on the kernelci.org x86 toolchain, the only
solution is to update the build bot.

> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:7263:2: warning: missing braces around
> initializer [-Wmissing-braces]
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:7263:2: warning: (near initialization
> for 'ce_payload.regular') [-Wmissing-braces]
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:7290:2: warning: missing braces around
> initializer [-Wmissing-braces]
> drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c:7290:2: warning: (near initialization
> for 'de_payload.regular') [-Wmissing-braces]

This only happens on old compilers. I sent a patch, no reply so far:

https://patchwork.kernel.org/patch/9554695/

> bcm47xx_defconfig (mips) — FAIL, 1 error, 0 warnings, 0 section mismatches
>
> Errors:
> /home/buildslave/workspace/khilman-kbuilder/next/build-mips/../net/sched/sch_fq_codel.c:468:
> undefined reference to `tcf_destroy_chain'

I have not looked at this one yet, appears to be caused by commit
cf1facda2f61 ("sched: move tcf_proto_destroy and tcf_destroy_chain
helpers into cls_api")

> defconfig+CONFIG_KASAN=y (arm64) — PASS, 0 errors, 4 warnings, 0 section
> mismatches
>
> Warnings:
> net/bridge/br_netlink.c:1339:1: warning: the frame size of 2544 bytes is
> larger than 2048 bytes [-Wframe-larger-than=]
> net/wireless/nl80211.c:1415:1: warning: the frame size of 2208 bytes is
> larger than 2048 bytes [-Wframe-larger-than=]
> net/wireless/nl80211.c:1904:1: warning: the frame size of 3840 bytes is
> larger than 2048 bytes [-Wframe-larger-than=]
> net/wireless/nl80211.c:4443:1: warning: the frame size of 2240 bytes is
> larger than 2048 bytes [-Wframe-larger-than=]

I have an experimental patch, still working on a better solution. This might
take some more time, but it's been like this from the start with KASAN.

> ip27_defconfig (mips) — FAIL, 2 errors, 0 warnings, 0 section mismatches
>
> Errors:
> drivers/net/ethernet/qlogic/qlge/qlge_main.c:4819:1: error: insn does not
> satisfy its constraints:
> drivers/net/ethernet/qlogic/qlge/qlge_main.c:4819:1: internal compiler
> error: in extract_constrain_insn, at recog.c:2190

broken gcc release, apparently fixed in gcc-7 (can't reproduce here at least).
I suggested a workaround, but got no reply so far:

http://www.spinics.net/lists/mips/msg66465.html

> xway_defconfig (mips) — FAIL, 2 errors, 0 warnings, 0 section mismatches
>
> Errors:
> (.text+0x14e10): undefined reference to `physical_memsize'
> (.text+0x14e14): undefined reference to `physical_memsize'

Hauke already did a patch in December, but it 

Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function

[Nicolai Hähnle]

On 14.02.2017 13:51, Christian König wrote:

Am 14.02.2017 um 13:00 schrieb Nicolai Hähnle:

On 14.02.2017 11:49, Christian König wrote:

Am 14.02.2017 um 11:37 schrieb Nicolai Hähnle:

From: Nicolai Hähnle 

Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.

Signed-off-by: Nicolai Hähnle 


Please squash that into your other patch. It fixes another bug, but I
don't think fixing one bug just to run into another is really a good
idea.


I don't understand. I'm not aware that this patch fixes anything, it
just enables the subsequent fix in amdgpu in patch #2. I don't think
squashing those together is a good idea (one is in ttm, the other in
amdgpu).


Ok, forget it I've messed up the different reference count.

With at least initializing bo->kref and bo->destroy before returning the
first error the patch is Reviewed-by: Christian König
.


Thanks. Does this apply to patches #2 and #3 as well?

Cheers,
Nicolai




Regards,
Christian.





Additional to that one comment below.


---
  drivers/gpu/drm/ttm/ttm_bo.c | 62
+---
  include/drm/ttm/ttm_bo_api.h | 45 
  2 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c
b/drivers/gpu/drm/ttm/ttm_bo.c
index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object
*bo,
  }
  EXPORT_SYMBOL(ttm_bo_validate);
  -int ttm_bo_init(struct ttm_bo_device *bdev,
-struct ttm_buffer_object *bo,
-unsigned long size,
-enum ttm_bo_type type,
-struct ttm_placement *placement,
-uint32_t page_alignment,
-bool interruptible,
-struct file *persistent_swap_storage,
-size_t acc_size,
-struct sg_table *sg,
-struct reservation_object *resv,
-void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+uint32_t page_alignment,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
  {
  int ret = 0;
  unsigned long num_pages;
  struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
-bool locked;
ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);
  if (ret) {
  pr_err("Out of kernel memory\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  return -ENOMEM;
  }
num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
  if (num_pages == 0) {
  pr_err("Illegal buffer object size\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  ttm_mem_global_free(mem_glob, acc_size);
  return -EINVAL;
  }


I would move those checks after all the field initializations. This way
the structure has at least a valid content and we can safely use
ttm_bo_unref on it.


That feels odd to me, since the return value indicates that the buffer
wasn't properly initialized, but I don't feel strongly about it.

Cheers,
Nicolai




Regards,
Christian.


@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
  ret = drm_vma_offset_add(>vma_manager, >vma_node,
   bo->mem.num_pages);
  +return ret;
+}
+EXPORT_SYMBOL(ttm_bo_init_top);
+
+int ttm_bo_init(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+struct ttm_placement *placement,
+uint32_t page_alignment,
+bool interruptible,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
+{
+bool locked;
+int ret;
+
+ret = ttm_bo_init_top(bdev, bo, size, type, page_alignment,
+  persistent_swap_storage, acc_size, sg, resv,
+  destroy);
+if (ret) {
+if (destroy)
+(*destroy)(bo);
+else
+kfree(bo);
+return ret;
+}
+
  /* passed reservation objects should already be locked,
   * since otherwise lockdep will be angered in radeon.
   */
diff --git a/include/drm/ttm/ttm_bo_api.h
b/include/drm/ttm/ttm_bo_api.h
index f195899..d44b8e4 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -453,6 +453,51 @@ size_t 

Re: [PATCH] drm/amd/display: Fix 64-bit division, yet again

[Emil Velikov]

On 14 February 2017 at 20:36, Harry Wentland  wrote:
> Also make the code somewhat more readable.
>
I'd suggest reaching to the team to integrate scripts/checkpatch.pl in
the pre-commit hook.

It will help you improve the coding standard and, as you mentioned, it
"make[s] the code somewhat more readable".

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

Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function

[zhoucm1]

On 2017年02月15日 18:43, Nicolai Hähnle wrote:

On 15.02.2017 04:16, zhoucm1 wrote:

On 2017年02月14日 18:37, Nicolai Hähnle wrote:

From: Nicolai Hähnle 

Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.

Signed-off-by: Nicolai Hähnle 
---
  drivers/gpu/drm/ttm/ttm_bo.c | 62
+---
  include/drm/ttm/ttm_bo_api.h | 45 
  2 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c 
b/drivers/gpu/drm/ttm/ttm_bo.c

index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object 
*bo,

  }
  EXPORT_SYMBOL(ttm_bo_validate);
  -int ttm_bo_init(struct ttm_bo_device *bdev,
-struct ttm_buffer_object *bo,
-unsigned long size,
-enum ttm_bo_type type,
-struct ttm_placement *placement,
-uint32_t page_alignment,
-bool interruptible,
-struct file *persistent_swap_storage,
-size_t acc_size,
-struct sg_table *sg,
-struct reservation_object *resv,
-void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+uint32_t page_alignment,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
  {
  int ret = 0;
  unsigned long num_pages;
  struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
-bool locked;
ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);
  if (ret) {
  pr_err("Out of kernel memory\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  return -ENOMEM;
  }
num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
  if (num_pages == 0) {
  pr_err("Illegal buffer object size\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  ttm_mem_global_free(mem_glob, acc_size);
  return -EINVAL;
  }
@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
  ret = drm_vma_offset_add(>vma_manager, >vma_node,
   bo->mem.num_pages);

if (ret && !resv), we should call
reservation_object_fini(>ttm_resv), right?


Good point, though actually, ret can never be != 0 here, so this can 
be simplified a bit.




  +return ret;
+}
+EXPORT_SYMBOL(ttm_bo_init_top);
+
+int ttm_bo_init(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+struct ttm_placement *placement,
+uint32_t page_alignment,
+bool interruptible,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
+{
+bool locked;
+int ret;
+

Can we lock resv anyway before ttm_bo_init_top like what you did in
patch #3? if yes, seems we don't need patch#3 any more, right?


if (!resv) {
bool locked;

reservation_object_init(>tbo.ttm_resv);
locked = ww_mutex_trylock(>tbo.ttm_resv.lock);
WARN_ON(!locked);
}
r = ttm_bo_init_top(>mman.bdev, >tbo, size, type,
page_align, NULL,
acc_size, sg, resv ? resv : 
>tbo.ttm_resv,

_ttm_bo_destroy);


No, because there's still different behavior when it comes to 
unlocking. There are three different behaviors that are needed:


1. resv == NULL, and leaving ttm_bo_init with the BO unreserved.

2. resv != NULL, and not changing the reserved status during 
initialization.


3. resv != NULL, and leaving initialization with the BO reserved, but 
unlocking when the BO is destroyed.


1+2 can be expressed well with the ttm_bo_init interface, but 3 cannot.

I think a possible alternative would be to change ttm_bo_init so that 
it always returns on success with the BO reserved, but that would 
require changing all the drivers, and I don't really see the advantage 
over just being more explicit in each driver.

OK, make sense, then wait Alex to submit to staging branch and verify it.

Thanks,
David Zhou


Cheers,
Nicolai



Regards,
David Zhou

+ret = ttm_bo_init_top(bdev, bo, size, type, page_alignment,
+  persistent_swap_storage, acc_size, sg, resv,
+  destroy);
+if (ret) {
+if 

Re: [PATCH 1/3] drm/ttm: split BO structure initialization into a separate function

[Nicolai Hähnle]

On 15.02.2017 04:16, zhoucm1 wrote:

On 2017年02月14日 18:37, Nicolai Hähnle wrote:

From: Nicolai Hähnle 

Allow callers to opt out of calling ttm_bo_validate immediately. This
allows more flexibility in how locking of the reservation object is
done, which is needed to fix a locking bug (destroy locked mutex)
in amdgpu.

Signed-off-by: Nicolai Hähnle 
---
  drivers/gpu/drm/ttm/ttm_bo.c | 62
+---
  include/drm/ttm/ttm_bo_api.h | 45 
  2 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 76bee42..ce4c0f5 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1120,41 +1120,30 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
  }
  EXPORT_SYMBOL(ttm_bo_validate);
  -int ttm_bo_init(struct ttm_bo_device *bdev,
-struct ttm_buffer_object *bo,
-unsigned long size,
-enum ttm_bo_type type,
-struct ttm_placement *placement,
-uint32_t page_alignment,
-bool interruptible,
-struct file *persistent_swap_storage,
-size_t acc_size,
-struct sg_table *sg,
-struct reservation_object *resv,
-void (*destroy) (struct ttm_buffer_object *))
+int ttm_bo_init_top(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+uint32_t page_alignment,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
  {
  int ret = 0;
  unsigned long num_pages;
  struct ttm_mem_global *mem_glob = bdev->glob->mem_glob;
-bool locked;
ret = ttm_mem_global_alloc(mem_glob, acc_size, false, false);
  if (ret) {
  pr_err("Out of kernel memory\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  return -ENOMEM;
  }
num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
  if (num_pages == 0) {
  pr_err("Illegal buffer object size\n");
-if (destroy)
-(*destroy)(bo);
-else
-kfree(bo);
  ttm_mem_global_free(mem_glob, acc_size);
  return -EINVAL;
  }
@@ -1204,6 +1193,37 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
  ret = drm_vma_offset_add(>vma_manager, >vma_node,
   bo->mem.num_pages);

if (ret && !resv), we should call
reservation_object_fini(>ttm_resv), right?


Good point, though actually, ret can never be != 0 here, so this can be 
simplified a bit.




  +return ret;
+}
+EXPORT_SYMBOL(ttm_bo_init_top);
+
+int ttm_bo_init(struct ttm_bo_device *bdev,
+struct ttm_buffer_object *bo,
+unsigned long size,
+enum ttm_bo_type type,
+struct ttm_placement *placement,
+uint32_t page_alignment,
+bool interruptible,
+struct file *persistent_swap_storage,
+size_t acc_size,
+struct sg_table *sg,
+struct reservation_object *resv,
+void (*destroy) (struct ttm_buffer_object *))
+{
+bool locked;
+int ret;
+

Can we lock resv anyway before ttm_bo_init_top like what you did in
patch #3? if yes, seems we don't need patch#3 any more, right?


if (!resv) {
bool locked;

reservation_object_init(>tbo.ttm_resv);
locked = ww_mutex_trylock(>tbo.ttm_resv.lock);
WARN_ON(!locked);
}
r = ttm_bo_init_top(>mman.bdev, >tbo, size, type,
page_align, NULL,
acc_size, sg, resv ? resv : >tbo.ttm_resv,
_ttm_bo_destroy);


No, because there's still different behavior when it comes to unlocking. 
There are three different behaviors that are needed:


1. resv == NULL, and leaving ttm_bo_init with the BO unreserved.

2. resv != NULL, and not changing the reserved status during initialization.

3. resv != NULL, and leaving initialization with the BO reserved, but 
unlocking when the BO is destroyed.


1+2 can be expressed well with the ttm_bo_init interface, but 3 cannot.

I think a possible alternative would be to change ttm_bo_init so that it 
always returns on success with the BO reserved, but that would require 
changing all the drivers, and I don't really see the advantage over just 
being more explicit in each driver.


Cheers,
Nicolai



Regards,
David Zhou

+ret = ttm_bo_init_top(bdev, bo, size, type, page_alignment,
+  persistent_swap_storage, acc_size, sg, resv,
+  destroy);
+if (ret) {
+if (destroy)
+(*destroy)(bo);
+else
+kfree(bo);
+return ret;
+}
+
  /* passed reservation 

Re: How to catch wild writes to system memory, and IOMMU errors

[Christian König]

Uff, well that's quite a problem you ran into here.

IOMMU might not help here, cause when it would be the GPU we would have 
made a mapping once and then the page in question is never unmapped (IIRC).


To confirm if it is really the GPU writing those bytes I would add a 
trace point to amdgpu_ttm_tt_populate() to see which pages the GPU got 
assigned.


If the page with the corruption is not in that list it is unlikely (but 
not impossible) that the GPU is the one doing the corruption.


Good luck,
Christian.

Am 14.02.2017 um 14:20 schrieb Nicolai Hähnle:

Hi all,

on an amd-staging-4.9 kernel with lock debugging and KASAN enabled, I 
am seeing a bug where I suspect that the GPU may be writing into 
system memory where it shouldn't.


I can reproduce errors fairly reliable by running a parallel piglit 
run on 8 cores with Tonga.


See exhibit1 and exhibit2 for two of the errors that were reported. As 
you can see, poison data of a dead object was overwritten.


If that was done by a use-after-free in kernel code, I would expect to 
see a KASAN error about it, but I don't. Furthermore, the pattern of 
overwritten values is quite unusual: single bytes, with 8 byte stride, 
many times all of them the same value. This is the kind of pattern 
that could fit GPU writes to an 8-bit texture.


See kasan-corrupted for another type of report that I've seen. This 
report looks like KASAN's internal data structures were corrupted, 
leading to a crash.


Needless to say, while I can reproduce those crashes fairly reliably, 
they are totally non-deterministic.


So the question is how to figure out where the bad memory writes happen.

I noticed that the IOMMU on the system was disabled by the BIOS, so I 
enabled it, in the hopes that that would catch bad GPU behavior.


Well, this leads to lots of IO_PAGE_FAULT message during the amdgpu 
module initialization (see dmesg-iommu). When running piglit, however, 
I get the same type of random memory corruption errors / crashes as 
before, and no IOMMU errors.


Any ideas on (a) what kind of tools could be helpful in tracking this 
problem down (if any...), and (b) where in the code the problem lies?


I suspect something's wrong with GART mappings when buffers are moved, 
but that's pretty vague...


Thanks,
Nicolai


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



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

Re: [PATCH] drm/amdgpu: Remove bo.priority compiler warning

[Christian König]

Yeah, Nicolai was faster than you.

Patch is already reviewed and should be committed.

Christian.

Am 14.02.2017 um 19:30 schrieb Felix Kuehling:

I think Nicolai beat you to this with his patch "drm/ttm: make
TTM_MAX_BO_PRIORITY unsigned".


On 17-02-14 01:03 PM, Kent Russell wrote:

Addresses this compile warning:

warning: comparison of distinct pointer types lacks a cast [enabled by
default]
   bo->tbo.priority = min(bo->tbo.priority, TTM_MAX_BO_PRIORITY - 1);

  Kent



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

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



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