Am 11.05.2017 um 09:35 schrieb Huang Rui:
> On Thu, May 11, 2017 at 02:41:56PM +0800, Christian König wrote:
>> Am 11.05.2017 um 07:42 schrieb Huang Rui:
>>> Signed-off-by: Huang Rui <ray.huang at amd.com>
>>> ---
>>>
>>> V1 -> V2:
>>> - park the scheduler thread for each ring to avoid conflict with commands
>> from
>>> active apps.
>>>
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 50
>> ++++++++++++++++++++++++++++--
>>> 1 file changed, 48 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd
>> /amdgpu/amdgpu_device.c
>>> index 19ac196..04a63b5 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -3643,14 +3643,60 @@ static int amdgpu_debugfs_test_ib(struct seq_file *m,
>> void *data)
>>> return 0;
>>> }
>>>
>>> +static int amdgpu_ring_tests(struct amdgpu_device *adev)
>>> +{
>>> + unsigned i;
>>> + int r = 0;
>>> +
>>> + for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
>>> + struct amdgpu_ring *ring = adev->rings[i];
>>> +
>>> + if (!ring || !ring->ready || !ring->sched.thread)
>>> + continue;
>>> +
>>> + /* hold on the scheduler */
>>> + kthread_park(ring->sched.thread);
>>> +
>>> + r = amdgpu_ring_test_ring(ring);
>>> + if (r) {
>>> + ring->ready = false;
>> Don't mess with the ready flag here.
>>
>>> + DRM_ERROR("amdgpu: failed to test ring %d (%d).\n",
>>> + i, r);
>>> + }
>>> +
>>> + /* go on the scheduler */
>>> + kthread_unpark(ring->sched.thread);
>>> + }
>>> +
>>> + return r;
>>> +}
>>> +
>>> +static int amdgpu_debugfs_test_ring(struct seq_file *m, void *data)
>>> +{
>>> + struct drm_info_node *node = (struct drm_info_node *) m->private;
>>> + struct drm_device *dev = node->minor->dev;
>>> + struct amdgpu_device *adev = dev->dev_private;
>>> + int r = 0;
>>> +
>>> + seq_printf(m, "run ring test:\n");
>>> + r = amdgpu_ring_tests(adev);
>> Why a separate function for this?
>>
> I think it might be re-used by other side in future.
Well in this case we should create the function when we actually use it
and not just because a possible use some times in the future.
>> Additional to that I agree with Dave that when we have the IB test the
>> ring test is not necessary any more.
>>
>> We just do this on boot/resume separately to be able to narrow down
>> problems faster when we see in the logs that one fails but the other
>> succeeds.
>>
> Yeah, you know, we only want to expose ib test orignally. When I write that
> codes, I found the ring tests are also able to re-use the debugfs list. :)
>
> Is there anything that might break the ring at runtime? If not, I can drop
> this patch.
Yeah, the ring tests messes with the ready flags, so I would rather drop
this patch.
Christian.
>
> Thanks,
> Rui
