Re: [PATCH i-g-t 4/6] lib: Add igt_pipe_crc_new_nonblock()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Dec 21, 2015 at 04:53:34PM +0100, Daniel Vetter wrote:
> On Fri, Dec 18, 2015 at 07:25:48PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > 
> > Add support for reading the CRC in non-blocking mode. Useful for tests
> > that want to start the CRC capture, then do a bunch of operations, then
> > collect however many CRCs that got generated. The current
> > igt_pipe_crc_new() + igt_pipe_crc_get_crcs() method would block until
> > it gets the requested number of CRCs, whreas in non-blocking mode we
> > can just read as many as got generated thus far.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> >  lib/igt_debugfs.c | 136 +++++++++++++++++++++++++++++++++++-------------------
> >  lib/igt_debugfs.h |   4 +-
> >  2 files changed, 91 insertions(+), 49 deletions(-)
> > 
> > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> > index a3d015267e15..1d625ad4ba50 100644
> > --- a/lib/igt_debugfs.c
> > +++ b/lib/igt_debugfs.c
> > @@ -298,6 +298,7 @@ struct _igt_pipe_crc {
> >  	int crc_fd;
> >  	int line_len;
> >  	int buffer_len;
> > +	int flags;
> >  
> >  	enum pipe pipe;
> >  	enum intel_pipe_crc_source source;
> > @@ -389,19 +390,8 @@ void igt_require_pipe_crc(void)
> >  	fclose(ctl);
> >  }
> >  
> > -/**
> > - * igt_pipe_crc_new:
> > - * @pipe: display pipe to use as source
> > - * @source: CRC tap point to use as source
> > - *
> > - * This sets up a new pipe CRC capture object for the given @pipe and @source.
> > - *
> > - * Returns: A pipe CRC object if the given @pipe and @source. The library
> > - * assumes that the source is always available since recent kernels support at
> > - * least INTEL_PIPE_CRC_SOURCE_AUTO everywhere.
> > - */
> > -igt_pipe_crc_t *
> > -igt_pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source)
> > +static igt_pipe_crc_t *
> > +pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source, int flags)
> >  {
> >  	igt_pipe_crc_t *pipe_crc;
> >  	char buf[128];
> > @@ -414,18 +404,55 @@ igt_pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source)
> >  	igt_assert(pipe_crc->ctl_fd != -1);
> >  
> >  	sprintf(buf, "i915_pipe_%s_crc", kmstest_pipe_name(pipe));
> > -	pipe_crc->crc_fd = igt_debugfs_open(buf, O_RDONLY);
> > +	pipe_crc->crc_fd = igt_debugfs_open(buf, flags);
> >  	igt_assert(pipe_crc->crc_fd != -1);
> >  
> >  	pipe_crc->line_len = PIPE_CRC_LINE_LEN;
> >  	pipe_crc->buffer_len = PIPE_CRC_BUFFER_LEN;
> >  	pipe_crc->pipe = pipe;
> >  	pipe_crc->source = source;
> > +	pipe_crc->flags = flags;
> >  
> >  	return pipe_crc;
> >  }
> >  
> >  /**
> > + * igt_pipe_crc_new:
> > + * @pipe: display pipe to use as source
> > + * @source: CRC tap point to use as source
> > + *
> > + * This sets up a new pipe CRC capture object for the given @pipe and @source
> > + * in blocking mode.
> > + *
> > + * Returns: A pipe CRC object if the given @pipe and @source. The library
> > + * assumes that the source is always available since recent kernels support at
> > + * least INTEL_PIPE_CRC_SOURCE_AUTO everywhere.
> > + */
> > +igt_pipe_crc_t *
> > +igt_pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source)
> > +{
> > +	return pipe_crc_new(pipe, source, O_RDONLY);
> > +}
> > +
> > +/**
> > + * igt_pipe_crc_new_nonblock:
> > + * @pipe: display pipe to use as source
> > + * @source: CRC tap point to use as source
> > + *
> > + * This sets up a new pipe CRC capture object for the given @pipe and @source
> > + * in nonblocking mode.
> > + *
> > + * Returns: A pipe CRC object if the given @pipe and @source. The library
> > + * assumes that the source is always available since recent kernels support at
> > + * least INTEL_PIPE_CRC_SOURCE_AUTO everywhere.
> > + */
> > +igt_pipe_crc_t *
> > +igt_pipe_crc_new_nonblock(enum pipe pipe, enum intel_pipe_crc_source source)
> > +{
> > +	return pipe_crc_new(pipe, source, O_RDONLY | O_NONBLOCK);
> > +}
> > +
> > +/**
> >   * igt_pipe_crc_free:
> >   * @pipe_crc: pipe CRC object
> >   *
> > @@ -441,6 +468,39 @@ void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc)
> >  	free(pipe_crc);
> >  }
> >  
> > +static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
> > +{
> > +	int n;
> > +
> > +	crc->n_words = 5;
> > +	n = sscanf(line, "%8u %8x %8x %8x %8x %8x", &crc->frame, &crc->crc[0],
> > +		   &crc->crc[1], &crc->crc[2], &crc->crc[3], &crc->crc[4]);
> > +	return n == 6;
> > +}
> > +
> > +static int read_one_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
> > +{
> > +	ssize_t bytes_read;
> > +	char buf[pipe_crc->buffer_len];
> > +
> > +	igt_set_timeout(5, "CRC reading");
> > +	bytes_read = read(pipe_crc->crc_fd, &buf, pipe_crc->line_len);
> > +	igt_reset_timeout();
> > +
> > +	if (bytes_read < 0 && errno == EAGAIN) {
> > +		igt_assert(pipe_crc->flags & O_NONBLOCK);
> > +		bytes_read = 0;
> > +	} else {
> > +		igt_assert_eq(bytes_read, pipe_crc->line_len);
> > +	}
> > +	buf[bytes_read] = '\0';
> > +
> > +	if (bytes_read && !pipe_crc_init_from_string(out, buf))
> > +		return -EINVAL;
> > +
> > +	return bytes_read;
> > +}
> > +
> >  /**
> >   * igt_pipe_crc_start:
> >   * @pipe_crc: pipe CRC object
> > @@ -449,7 +509,7 @@ void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc)
> >   */
> >  void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
> >  {
> > -	igt_crc_t *crcs = NULL;
> > +	igt_crc_t crc;
> >  
> >  	igt_assert(igt_pipe_crc_do_start(pipe_crc));
> >  
> > @@ -460,8 +520,10 @@ void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc)
> >  	 * On CHV sometimes the second CRC is bonkers as well, so don't trust
> >  	 * that one either.
> >  	 */
> > -	igt_pipe_crc_get_crcs(pipe_crc, 2, &crcs);
> > -	free(crcs);
> > +	while (read_one_crc(pipe_crc, &crc) == 0)
> > +		usleep(1000);
> > +	while (read_one_crc(pipe_crc, &crc) == 0)
> > +		usleep(1000);
> >  }
> >  
> >  /**
> > @@ -478,34 +540,6 @@ void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc)
> >  	igt_assert_eq(write(pipe_crc->ctl_fd, buf, strlen(buf)), strlen(buf));
> >  }
> >  
> > -static bool pipe_crc_init_from_string(igt_crc_t *crc, const char *line)
> > -{
> > -	int n;
> > -
> > -	crc->n_words = 5;
> > -	n = sscanf(line, "%8u %8x %8x %8x %8x %8x", &crc->frame, &crc->crc[0],
> > -		   &crc->crc[1], &crc->crc[2], &crc->crc[3], &crc->crc[4]);
> > -	return n == 6;
> > -}
> > -
> > -static bool read_one_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
> > -{
> > -	ssize_t bytes_read;
> > -	char buf[pipe_crc->buffer_len];
> > -
> > -	igt_set_timeout(5, "CRC reading");
> > -	bytes_read = read(pipe_crc->crc_fd, &buf, pipe_crc->line_len);
> > -	igt_reset_timeout();
> > -
> > -	igt_assert_eq(bytes_read, pipe_crc->line_len);
> > -	buf[bytes_read] = '\0';
> > -
> > -	if (!pipe_crc_init_from_string(out, buf))
> > -		return false;
> > -
> > -	return true;
> > -}
> > -
> >  /**
> >   * igt_pipe_crc_get_crcs:
> >   * @pipe_crc: pipe CRC object
> > @@ -519,7 +553,7 @@ static bool read_one_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out)
> >   * Callers must start and stop the capturing themselves by calling
> >   * igt_pipe_crc_start() and igt_pipe_crc_stop().
> 
> Please update the gtkdoc with the return value semantics.

done

> 
> >   */
> > -void
> > +int
> >  igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
> >  		      igt_crc_t **out_crcs)
> >  {
> > @@ -530,14 +564,19 @@ igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
> >  
> >  	do {
> >  		igt_crc_t *crc = &crcs[n];
> > +		int ret;
> >  
> > -		if (!read_one_crc(pipe_crc, crc))
> > +		ret = read_one_crc(pipe_crc, crc);
> > +		if (ret < 0)
> >  			continue;
> > +		if (ret == 0)
> > +			break;
> >  
> >  		n++;
> >  	} while (n < n_crcs);
> >  
> >  	*out_crcs = crcs;
> > +	return n;
> >  }
> >  
> >  static void crc_sanity_checks(igt_crc_t *crc)
> > @@ -576,7 +615,8 @@ void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out_crc)
> >  	igt_debug_wait_for_keypress("crc");
> >  
> >  	igt_pipe_crc_start(pipe_crc);
> > -	read_one_crc(pipe_crc, out_crc);
> > +	while (read_one_crc(pipe_crc, out_crc) == 0)
> > +		usleep(1000);
> >  	igt_pipe_crc_stop(pipe_crc);
> >  
> >  	crc_sanity_checks(out_crc);
> > diff --git a/lib/igt_debugfs.h b/lib/igt_debugfs.h
> > index 24018eb355e4..6c6e2858d8f6 100644
> > --- a/lib/igt_debugfs.h
> > +++ b/lib/igt_debugfs.h
> > @@ -115,10 +115,12 @@ char *igt_crc_to_string(igt_crc_t *crc);
> >  void igt_require_pipe_crc(void);
> >  igt_pipe_crc_t *
> >  igt_pipe_crc_new(enum pipe pipe, enum intel_pipe_crc_source source);
> > +igt_pipe_crc_t *
> > +igt_pipe_crc_new_nonblock(enum pipe pipe, enum intel_pipe_crc_source source);
> >  void igt_pipe_crc_free(igt_pipe_crc_t *pipe_crc);
> >  void igt_pipe_crc_start(igt_pipe_crc_t *pipe_crc);
> >  void igt_pipe_crc_stop(igt_pipe_crc_t *pipe_crc);
> > -void igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
> > +int igt_pipe_crc_get_crcs(igt_pipe_crc_t *pipe_crc, int n_crcs,
> >  			   igt_crc_t **out_crcs);
> 
> __must_check here to make sure callers check that they have enough crcs or
> do something else sensible. I think we should also update
> kms_pipe_crc_basic.

and done

> -Daniel
> 
> 
> >  void igt_pipe_crc_collect_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out_crc);
> >  
> > -- 
> > 2.4.10
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux