On Mon, 01 Apr 2019, Ashutosh Dixit <ashutosh.dixit at intel.com> wrote:
> For debug of audio issues in power management and driver reload tests,
> additional kernel logs may be useful, both in dmesg as well as
> ftrace. Add the infrastructure to generate these logs and enable these
> logs for selected tests.
>
> At present igt_runner and other CI infrastructure does not capture the
> ftrace buffer. Therefore, to avoid changes to igt_runner and the CI
> infrastructure the ftrace buffer is dumped to stdout at the end of
> each test.
>
> v4: remove system() calls, dump audio kernel logs from igt_fixture
> v2, v3: versions to fix CI
>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Signed-off-by: Ashutosh Dixit <ashutosh.dixit at intel.com>
> ---
> lib/igt_audio.c | 40 +++++++++++++++++++++++++++++++++++
> lib/igt_audio.h | 2 ++
> tests/i915/i915_module_load.c | 8 +++++++
> tests/i915/i915_pm_rpm.c | 8 ++++++-
> tests/i915/i915_suspend.c | 8 +++++--
> 5 files changed, 63 insertions(+), 3 deletions(-)
>
> diff --git a/lib/igt_audio.c b/lib/igt_audio.c
> index a0592d53..8a7841be 100644
> --- a/lib/igt_audio.c
> +++ b/lib/igt_audio.c
> @@ -31,6 +31,8 @@
>
> #include "igt_audio.h"
> #include "igt_core.h"
> +#include "igt_debugfs.h"
> +#include "igt_ftrace.h"
>
> #define FREQS_MAX 8
>
> @@ -323,3 +325,41 @@ bool audio_signal_detect(struct audio_signal *signal, int channels,
>
> return true;
> }
> +
> +static const char *audio_dmesg_en[] = {
> + "file sound/* +p",
> +};
> +
> +static const char *audio_dmesg_dis[] = {
> + "file sound/* -p",
> +};
> +
> +static const char *audio_ftr[] = {
> + "events/sst/enable",
> + "events/intel-sst/enable",
> + "events/asoc/enable",
> + "events/i2c/enable",
> + "events/hda/enable",
> + "events/hda_controller/enable",
> + "events/hda_intel/enable",
> +};
> +
> +void igt_enable_audio_klog(void)
> +{
> + igt_set_dynamic_debug(audio_dmesg_en,
> + sizeof(audio_dmesg_en)/sizeof(char*));
ARRAY_SIZE()
> + igt_ftrace_set_events(audio_ftr,
> + sizeof(audio_ftr)/sizeof(char*), true);
> + __igt_ftrace_enable("nop", NULL);
> +}
> +
> +void igt_disable_audio_klog(void)
> +{
> + igt_ftrace_disable();
> + igt_ftrace_set_events(audio_ftr,
> + sizeof(audio_ftr)/sizeof(char*), false);
> + igt_ftrace_dump("Ftrace output");
> + igt_ftrace_clear();
> + igt_set_dynamic_debug(audio_dmesg_dis,
> + sizeof(audio_dmesg_dis)/sizeof(char*));
> +}
I still think ftrace and dynamic debug should be separate. It's
surprising that "enable klog" enables tracing.
BR,
Jani.
> diff --git a/lib/igt_audio.h b/lib/igt_audio.h
> index b3b658a4..dab5ab5a 100644
> --- a/lib/igt_audio.h
> +++ b/lib/igt_audio.h
> @@ -40,5 +40,7 @@ void audio_signal_clean(struct audio_signal *signal);
> void audio_signal_fill(struct audio_signal *signal, short *buffer, int frames);
> bool audio_signal_detect(struct audio_signal *signal, int channels,
> int sampling_rate, short *buffer, int frames);
> +void igt_enable_audio_klog(void);
> +void igt_disable_audio_klog(void);
>
> #endif
> diff --git a/tests/i915/i915_module_load.c b/tests/i915/i915_module_load.c
> index 7fe83520..0d569a55 100644
> --- a/tests/i915/i915_module_load.c
> +++ b/tests/i915/i915_module_load.c
> @@ -326,6 +326,10 @@ hda_dynamic_debug(bool enable)
>
> igt_main
> {
> + igt_fixture {
> + igt_enable_audio_klog();
> + }
> +
> igt_subtest("reload") {
> int load_error;
>
> @@ -365,5 +369,9 @@ igt_main
> /* inject_fault() leaves the module unloaded */
> }
>
> + igt_fixture {
> + igt_disable_audio_klog();
> + }
> +
> /* Subtests should unload the module themselves if they use modparams */
> }
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index 03de609c..126d68ac 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -1975,8 +1975,10 @@ int main(int argc, char *argv[])
> /* Skip instead of failing in case the machine is not prepared to reach
> * PC8+. We don't want bug reports from cases where the machine is just
> * not properly configured. */
> - igt_fixture
> + igt_fixture {
> + igt_enable_audio_klog();
> igt_require(setup_environment());
> + }
>
> if (stay)
> igt_subtest("stay")
> @@ -2118,5 +2120,9 @@ int main(int argc, char *argv[])
> igt_i915_driver_unload();
> }
>
> + igt_fixture {
> + igt_disable_audio_klog();
> + }
> +
> igt_exit();
> }
> diff --git a/tests/i915/i915_suspend.c b/tests/i915/i915_suspend.c
> index 17c68cc1..527e974e 100644
> --- a/tests/i915/i915_suspend.c
> +++ b/tests/i915/i915_suspend.c
> @@ -207,8 +207,10 @@ igt_main
> {
> igt_skip_on_simulation();
>
> - igt_fixture
> + igt_fixture {
> fd = drm_open_driver(DRIVER_INTEL);
> + igt_enable_audio_klog();
> + }
>
> igt_subtest("fence-restore-tiled2untiled")
> test_fence_restore(fd, true, false);
> @@ -243,6 +245,8 @@ igt_main
> igt_subtest("forcewake-hibernate")
> test_forcewake(fd, true);
>
> - igt_fixture
> + igt_fixture {
> + igt_disable_audio_klog();
> close(fd);
> + }
> }
--
Jani Nikula, Intel Open Source Graphics Center