[PATCH v2 2/2] drm/amdgpu: export test ring debugfs interface

[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.

> 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.

Thanks,
Rui