[igt-dev] [PATCH i-g-t] lib/igt_debugfs: Add timeouts to opening pipe CRC fd.

[Dhinakaran Pandiyan]

On Thu, 2018-04-05 at 22:10 +0000, Souza, Jose wrote:
> On Thu, 2018-04-05 at 22:15 +0100, Chris Wilson wrote:
> > Quoting Souza, Jose (2018-04-05 22:03:46)
> > > On Thu, 2018-04-05 at 12:49 +0200, Maarten Lankhorst wrote:
> > > > This will fix the PSR tests to fail slightly faster, since they
> > > > wait
> > > > indefinitely for a CRC that never comes during open.

Because the pipe is not active and hence doesn't generate crc's any
more? The timeouts are needed but any idea how we should deal with the
lack of CRCs? 

> > > > 
> > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.c
> > > > om>
> > > > ---
> > > >  lib/igt_debugfs.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> > > > index 8adc02e9cc47..094df564b6f4 100644
> > > > --- a/lib/igt_debugfs.c
> > > > +++ b/lib/igt_debugfs.c
> > > > @@ -757,7 +757,10 @@ void igt_pipe_crc_start(igt_pipe_crc_t
> > > > *pipe_crc)
> > > >  
> > > >       sprintf(buf, "crtc-%d/crc/data", pipe_crc->pipe);
> > > >  
> > > > +     igt_set_timeout(5, "Opening crc fd, which waits for first
> > > > CRC.");
> > > >       pipe_crc->crc_fd = openat(pipe_crc->dir, buf, pipe_crc-
> > > > > flags);
> > > > 
> > > > +     igt_reset_timeout();
> > > 
> > > Hum I was able to reproduce this one too in a KBL, I was thinking
> > > in
> > > add a timeout in the kernel side, if it don't get the CRC buffer
> > > filled
> > > it would return a error.
> > 
> > That would be unexpected behaviour for a read() interface. If you
> > want a
> > timeout, implement poll().
> 
> Not change the read() but return a error in open() when timeout
> happens:
> 
> "
> diff --git a/drivers/gpu/drm/drm_debugfs_crc.c
> b/drivers/gpu/drm/drm_debugfs_crc.c
> index 9f8312137cad..223cd45b3aff 100644
> --- a/drivers/gpu/drm/drm_debugfs_crc.c
> +++ b/drivers/gpu/drm/drm_debugfs_crc.c
> @@ -206,9 +206,10 @@ static int crtc_crc_open(struct inode *inode,
> struct file *filep)
>  	 * guess when this particular piece of HW will be ready to
> start
>  	 * generating CRCs.
>  	 */
> -	ret = wait_event_interruptible_lock_irq(crc->wq,
> -						crtc_crc_data_count(cr
> c),
> -						crc->lock);
> +	ret = wait_event_interruptible_lock_irq_timeout(crc->wq,
> +							crtc_crc_data_
> count(crc),
> +							crc->lock,
> +							msecs_to_jiffi
> es(2500));
>  	spin_unlock_irq(&crc->lock);
>  
>  	if (ret)
> "
> 
> 
> > -Chris
> _______________________________________________
> igt-dev mailing list
> igt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev