[PATCH 3/4] drm/amdgpu: report preemption fence via amdgpu_fence_info

[Ding, Pixel]

Yes I see the “drm/amdgpu: use 256 bit buffers for all wb allocations (v2)”.

Hi Christian, 

So it seems all good, right?

— 
Sincerely Yours,
Pixel







On 13/10/2017, 5:19 PM, "Liu, Monk" <Monk.Liu at amd.com> wrote:

>Pixel,
>
>On drm-next, we always allocate 8DW for all WB, you can check the wb_get routine on detail
>
>BR Monk
>
>-----Original Message-----
>From: Ding, Pixel 
>Sent: 2017年10月13日 17:19
>To: Koenig, Christian <Christian.Koenig at amd.com>; amd-gfx at lists.freedesktop.org; Liu, Monk <Monk.Liu at amd.com>
>Cc: Li, Bingley <Bingley.Li at amd.com>
>Subject: Re: [PATCH 3/4] drm/amdgpu: report preemption fence via amdgpu_fence_info
>
>OK I get it…
>when we use the fence_offs to submit fence to HW, in fact it’s a 8 DW fence not a 2 DW.
>
>The format is:
>Completed Fence      0x0 Fence written here if frame completed normally
>Preempted Fence      0x2 Bit set in CP_VMID_PREEMPT and preemption occurred
>Reset Fence          0x4 Bit is set in CP_VMID_RESET and reset occurred
>Preempted then Reset 0x6 Both preemption and reset occurred
>
>					
>				
>			
>		
>	
>Then I notice that the 8DW wb allocation patch is missed on staging driver.
>
>
>Hi Monk,
>
>Can you take a look? I thought the 8DW WB patch is already upstream.
>
>— 
>Sincerely Yours,
>Pixel
>
>
>
>
>
>
>
>On 13/10/2017, 5:10 PM, "Koenig, Christian" <Christian.Koenig at amd.com> wrote:
>
>>Hi Pixel,
>>
>>> My understanding is that CP will write seqno back to preempted fence offset when preemption occurred.
>>That is correct.
>>
>>But my question is why you want to print a different value here:
>>> +		seq_printf(m, "Last preempted      0x%08x\n",
>>> +			   le32_to_cpu(*(ring->fence_drv.cpu_addr + 2)));
>>
>>That code prints two dw after the address where the CPU writes the 
>>seqno. Where is the code which setups the CP to do this?
>>
>>As far as I can see that line should always print just zero (or some 
>>random number if the memory is actually not initialized to anything).
>>
>>Regards,
>>Christian.
>>
>>Am 13.10.2017 um 11:03 schrieb Ding, Pixel:
>>> My understanding is that CP will write seqno back to preempted fence offset when preemption occurred. Then if there is a value here we can generally know packet with which fence is preempted. Should driver handle any other thing for this?
>>> 					
>>> 				
>>> 			
>>> 		
>>> 	
>>>
>>>
>>> —
>>> Sincerely Yours,
>>> Pixel
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> On 13/10/2017, 4:51 PM, "Koenig, Christian" <Christian.Koenig at amd.com> wrote:
>>>
>>>>> Is it OK to limit this information only for SRIOV VF on Tonga and Vega whose format is known? It can help use to identify if MCBP is working correctly or not.
>>>> The question is where is this code for Tonga and Vega? I can't find a
>>>> reference to fence_offs in the GFX8 nor GFX9 code we have in
>>>> amd-staging-drm-next.
>>>>
>>>> And if it doesn't depend on the fence_offs in the ring printing it like
>>>> this is just pure speculation.
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>> Am 13.10.2017 um 10:41 schrieb Ding, Pixel:
>>>>> Thanks Christian,
>>>>>
>>>>> I’m not sure if I get your point, but yes the preemption fence offset could be changed.
>>>>>
>>>>> Is it OK to limit this information only for SRIOV VF on Tonga and Vega whose format is known? It can help use to identify if MCBP is working correctly or not.
>>>>>
>>>>>
>>>>> —
>>>>> Sincerely Yours,
>>>>> Pixel
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On 13/10/2017, 4:34 PM, "Christian König" <ckoenig.leichtzumerken at gmail.com> wrote:
>>>>>
>>>>>> Am 13.10.2017 um 10:26 schrieb Pixel Ding:
>>>>>>> From: pding <Pixel.Ding at amd.com>
>>>>>>>
>>>>>>> Only report fence for GFX ring. This can help checking MCBP feature.
>>>>>>>
>>>>>>> Signed-off-by: pding <Pixel.Ding at amd.com>
>>>>>>> ---
>>>>>>>     drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 7 +++++++
>>>>>>>     1 file changed, 7 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>>>>>>> index 09d5a5c..2044758 100644
>>>>>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>>>>>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
>>>>>>> @@ -645,6 +645,13 @@ static int amdgpu_debugfs_fence_info(struct seq_file *m, void *data)
>>>>>>>     			   atomic_read(&ring->fence_drv.last_seq));
>>>>>>>     		seq_printf(m, "Last emitted        0x%08x\n",
>>>>>>>     			   ring->fence_drv.sync_seq);
>>>>>>> +
>>>>>>> +		if (ring->funcs->type != AMDGPU_RING_TYPE_GFX)
>>>>>>> +			break;
>>>>>> That should probably be "continue" instead of break, or otherwise you
>>>>>> don't print the other fences any more.
>>>>>>
>>>>>>> +
>>>>>>> +		seq_printf(m, "Last preempted      0x%08x\n",
>>>>>>> +			   le32_to_cpu(*(ring->fence_drv.cpu_addr + 2)));
>>>>>> Is the code to put the preemption fence there already upstream?
>>>>>>
>>>>>> If yes do we really do this like that for all supported generations?
>>>>>>
>>>>>> Regards,
>>>>>> Christian.
>>>>>>
>>>>>>> +
>>>>>>>     	}
>>>>>>>     	return 0;
>>>>>>>     }
>>
>>