[igt-dev] [PATCH i-g-t v2] lib/igt_debugfs: Unify crc collection

[Kahola, Mika]

On Thu, 2019-01-17 at 13:26 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2019-01-17 at 13:41 +0200, Mika Kahola wrote:
> > For one shot crc collection, let's use the same helper function as
> > we
> > use for continuous crc collection. With this patch, we first drain
> > the
> > pipe from queued crc values and read the fresh crc.
> 
> I don't see the benefit of unifying, but patch looks harmless.
> 
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
Pushed! Thanks for the review!

The patch was more on cleanup/styling related as we use these helper
functions when collecting crc's continuously but not when collectiong
one shot crc. No functional changes.

> > 
> > v2: We don't need to drain the pipe right after we start
> >     collecting crc's (Dhinakaran)
> > 
> > Signed-off-by: Mika Kahola <mika.kahola at intel.com>
> > ---
> >  lib/igt_debugfs.c | 58 +++++++++++++++++++++++--------------------
> > ----
> >  1 file changed, 28 insertions(+), 30 deletions(-)
> > 
> > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c
> > index a3aca846..13f4f1c1 100644
> > --- a/lib/igt_debugfs.c
> > +++ b/lib/igt_debugfs.c
> > @@ -901,36 +901,6 @@ static void crc_sanity_checks(igt_crc_t *crc)
> >  	igt_warn_on_f(all_zero, "Suspicious CRC: All values are 0.\n");
> >  }
> >  
> > -/**
> > - * igt_pipe_crc_collect_crc:
> > - * @pipe_crc: pipe CRC object
> > - * @out_crc: buffer for the captured CRC values
> > - *
> > - * Read a single CRC from @pipe_crc. This function blocks until
> > the
> > CRC is
> > - * retrieved, irrespective of whether @pipe_crc has been opened
> > with
> > - * igt_pipe_crc_new() or igt_pipe_crc_new_nonblock().  @out_crc
> > must
> > be
> > - * allocated by the caller.
> > - *
> > - * This function takes care of the pipe_crc book-keeping, it will
> > start/stop
> > - * the collection of the CRC.
> > - *
> > - * This function also calls the interactive debug with the "crc"
> > domain, so you
> > - * can make use of this feature to actually see the screen that is
> > being CRC'd.
> > - *
> > - * For continuous CRC collection look at igt_pipe_crc_start(),
> > - * igt_pipe_crc_get_crcs() and igt_pipe_crc_stop().
> > - */
> > -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);
> > -	igt_pipe_crc_stop(pipe_crc);
> > -
> > -	crc_sanity_checks(out_crc);
> > -}
> > -
> >  /**
> >   * igt_pipe_crc_drain:
> >   * @pipe_crc: pipe CRC object
> > @@ -1004,6 +974,34 @@ igt_pipe_crc_get_current(int drm_fd,
> > igt_pipe_crc_t *pipe_crc, igt_crc_t *crc)
> >  	crc_sanity_checks(crc);
> >  }
> >  
> > +/**
> > + * igt_pipe_crc_collect_crc:
> > + * @pipe_crc: pipe CRC object
> > + * @out_crc: buffer for the captured CRC values
> > + *
> > + * Read a single CRC from @pipe_crc. This function blocks until
> > the
> > CRC is
> > + * retrieved, irrespective of whether @pipe_crc has been opened
> > with
> > + * igt_pipe_crc_new() or igt_pipe_crc_new_nonblock().  @out_crc
> > must
> > be
> > + * allocated by the caller.
> > + *
> > + * This function takes care of the pipe_crc book-keeping, it will
> > start/stop
> > + * the collection of the CRC.
> > + *
> > + * This function also calls the interactive debug with the "crc"
> > domain, so you
> > + * can make use of this feature to actually see the screen that is
> > being CRC'd.
> > + *
> > + * For continuous CRC collection look at igt_pipe_crc_start(),
> > + * igt_pipe_crc_get_crcs() and igt_pipe_crc_stop().
> > + */
> > +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);
> > +	igt_pipe_crc_get_single(pipe_crc, out_crc);
> > +	igt_pipe_crc_stop(pipe_crc);
> > +}
> > +
> >  /*
> >   * Drop caches
> >   */
> 
>