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

[Christian König]

Sounds logical to me as well.

Please also add a document describing what the CP does here.

BTW: Is that limited to the GFX pipeline or do the Compute pipes the same?

Regards,
Christian.

Am 13.10.2017 um 11:36 schrieb Ding, Pixel:
> Get it.
> —
> Sincerely Yours,
> Pixel
>
>
>
>
>
>
>
>
> On 13/10/2017, 5:28 PM, "Liu, Monk" <Monk.Liu at amd.com> wrote:
>
>> @Ding, Pixel
>>
>> +		seq_printf(m, "Last preempted      0x%08x\n",
>> +			   le32_to_cpu(*(ring->fence_drv.cpu_addr + 2)));
>>
>> Please handle other type fence as well:
>>
>> Preempted fence
>> Reset fence
>> Reset and preempted fence
>>
>>
>> BR Monk
>> -----Original Message-----
>> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf Of Christian K?nig
>> Sent: 2017年10月13日 17:10
>> To: Ding, Pixel <Pixel.Ding at amd.com>; amd-gfx at lists.freedesktop.org
>> Cc: Li, Bingley <Bingley.Li at amd.com>
>> Subject: Re: [PATCH 3/4] drm/amdgpu: report preemption fence via amdgpu_fence_info
>>
>> 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;
>>>>>>>      }
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx