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

[Liu, Monk]

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