[i-g-t,v2] lib/igt_debugfs: Unify crc collection

Submitted by Mika Kahola on Jan. 17, 2019, 11:41 a.m.

Details

Message ID 20190117114125.1252-1-mika.kahola@intel.com
State New
Headers show
Series "lib/igt_debugfs: Unify crc collection" ( rev: 2 ) in IGT

Not browsing as part of any series.

Commit Message

Mika Kahola Jan. 17, 2019, 11:41 a.m.
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.

v2: We don't need to drain the pipe right after we start
    collecting crc's (Dhinakaran)

Signed-off-by: Mika Kahola <mika.kahola@intel.com>
---
 lib/igt_debugfs.c | 58 +++++++++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 30 deletions(-)

Patch hide | download patch | download mbox

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
  */

Comments

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@intel.com>
> 
> v2: We don't need to drain the pipe right after we start
>     collecting crc's (Dhinakaran)
> 
> Signed-off-by: Mika Kahola <mika.kahola@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
>   */
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@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@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
> >   */
> 
>