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

Dhinakaran Pandiyan
2019-01-17T21:26:22Z

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