| Message ID | 1515739924-5308-1-git-send-email-sagar.a.kamble@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 12 Jan 2018 07:52:04 +0100, Sagar Arun Kamble <sagar.a.kamble@intel.com> wrote: > i915 expects GuC log level to be specified as: > 0: disabled > 1: enabled (verbosity level 0 = min) > 2: enabled (verbosity level 1) > 3: enabled (verbosity level 2) > 4: enabled (verbosity level 3 = max) > > Remove the earlier internal layout based logging control from > guc_log_control and send new expected values. > > Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> > --- Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> with small bikeshedding below... > tools/intel_guc_logger.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/tools/intel_guc_logger.c b/tools/intel_guc_logger.c > index 031fd84..e695497 100644 > --- a/tools/intel_guc_logger.c > +++ b/tools/intel_guc_logger.c > @@ -51,18 +51,27 @@ uint32_t test_duration, max_filesize; > pthread_cond_t underflow_cond, overflow_cond; > bool stop_logging, discard_oldlogs, capturing_stopped; > -static void guc_log_control(bool enable_logging) > +static void guc_log_control(bool enable, uint32_t log_level) > { > int control_fd; > char data[19]; > - uint64_t val; > + uint64_t val = 0; > int ret; Btw, this shouldn't hurt: igt_assert_lte(log_level, 3); > + /* > + * i915 expects GuC log level to be specified as: > + * 0: disabled > + * 1: enabled (verbosity level 0 = min) > + * 2: enabled (verbosity level 1) > + * 3: enabled (verbosity level 2) > + * 4: enabled (verbosity level 3 = max) > + */ > + if (enable) > + val = log_level + 1; control = enable ? log_level + 1 : 0; > + > control_fd = igt_debugfs_open(-1, CONTROL_FILE_NAME, O_WRONLY); > igt_assert_f(control_fd >= 0, "couldn't open the guc log control > file\n"); > - val = enable_logging ? ((verbosity_level << 4) | 0x1) : 0; > - > ret = snprintf(data, sizeof(data), "0x%" PRIx64, val); Btw, I'm wondering why we didn't use "fprintf(control_fd, ...)" here > igt_assert(ret > 2 && ret < sizeof(data)); > @@ -288,7 +297,7 @@ static void init_main_thread(void) > /* Enable the logging, it may not have been enabled from boot and so > * the relay file also wouldn't have been created. > */ > - guc_log_control(true); > + guc_log_control(true, verbosity_level); > open_relay_file(); > open_output_file(); > @@ -420,7 +429,7 @@ int main(int argc, char **argv) > } while (!stop_logging); > /* Pause logging on the GuC side */ > - guc_log_control(false); > + guc_log_control(false, 0); > /* Signal flusher thread to make an exit */ > capturing_stopped = 1;
On 1/12/2018 6:51 PM, Michal Wajdeczko wrote: > On Fri, 12 Jan 2018 07:52:04 +0100, Sagar Arun Kamble > <sagar.a.kamble@intel.com> wrote: > >> i915 expects GuC log level to be specified as: >> 0: disabled >> 1: enabled (verbosity level 0 = min) >> 2: enabled (verbosity level 1) >> 3: enabled (verbosity level 2) >> 4: enabled (verbosity level 3 = max) >> >> Remove the earlier internal layout based logging control from >> guc_log_control and send new expected values. >> >> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> >> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> >> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> >> Cc: Chris Wilson <chris@chris-wilson.co.uk> >> --- > > Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com> Thanks for the review. Will shared updated rev. > > with small bikeshedding below... > >> tools/intel_guc_logger.c | 21 +++++++++++++++------ >> 1 file changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/tools/intel_guc_logger.c b/tools/intel_guc_logger.c >> index 031fd84..e695497 100644 >> --- a/tools/intel_guc_logger.c >> +++ b/tools/intel_guc_logger.c >> @@ -51,18 +51,27 @@ uint32_t test_duration, max_filesize; >> pthread_cond_t underflow_cond, overflow_cond; >> bool stop_logging, discard_oldlogs, capturing_stopped; >> -static void guc_log_control(bool enable_logging) >> +static void guc_log_control(bool enable, uint32_t log_level) >> { >> int control_fd; >> char data[19]; >> - uint64_t val; >> + uint64_t val = 0; >> int ret; > > Btw, this shouldn't hurt: > > igt_assert_lte(log_level, 3); > Yes >> + /* >> + * i915 expects GuC log level to be specified as: >> + * 0: disabled >> + * 1: enabled (verbosity level 0 = min) >> + * 2: enabled (verbosity level 1) >> + * 3: enabled (verbosity level 2) >> + * 4: enabled (verbosity level 3 = max) >> + */ >> + if (enable) >> + val = log_level + 1; > > control = enable ? log_level + 1 : 0; > Ok >> + >> control_fd = igt_debugfs_open(-1, CONTROL_FILE_NAME, O_WRONLY); >> igt_assert_f(control_fd >= 0, "couldn't open the guc log control >> file\n"); >> - val = enable_logging ? ((verbosity_level << 4) | 0x1) : 0; >> - >> ret = snprintf(data, sizeof(data), "0x%" PRIx64, val); > > Btw, I'm wondering why we didn't use "fprintf(control_fd, ...)" here > I remember that using read instead of fread was optimization to speed up the buffering of logs. For consistency I believe write is used instead fwrite. >> igt_assert(ret > 2 && ret < sizeof(data)); >> @@ -288,7 +297,7 @@ static void init_main_thread(void) >> /* Enable the logging, it may not have been enabled from boot >> and so >> * the relay file also wouldn't have been created. >> */ >> - guc_log_control(true); >> + guc_log_control(true, verbosity_level); >> open_relay_file(); >> open_output_file(); >> @@ -420,7 +429,7 @@ int main(int argc, char **argv) >> } while (!stop_logging); >> /* Pause logging on the GuC side */ >> - guc_log_control(false); >> + guc_log_control(false, 0); >> /* Signal flusher thread to make an exit */ >> capturing_stopped = 1;
diff --git a/tools/intel_guc_logger.c b/tools/intel_guc_logger.c index 031fd84..e695497 100644 --- a/tools/intel_guc_logger.c +++ b/tools/intel_guc_logger.c @@ -51,18 +51,27 @@ uint32_t test_duration, max_filesize; pthread_cond_t underflow_cond, overflow_cond; bool stop_logging, discard_oldlogs, capturing_stopped; -static void guc_log_control(bool enable_logging) +static void guc_log_control(bool enable, uint32_t log_level) { int control_fd; char data[19]; - uint64_t val; + uint64_t val = 0; int ret; + /* + * i915 expects GuC log level to be specified as: + * 0: disabled + * 1: enabled (verbosity level 0 = min) + * 2: enabled (verbosity level 1) + * 3: enabled (verbosity level 2) + * 4: enabled (verbosity level 3 = max) + */ + if (enable) + val = log_level + 1; + control_fd = igt_debugfs_open(-1, CONTROL_FILE_NAME, O_WRONLY); igt_assert_f(control_fd >= 0, "couldn't open the guc log control file\n"); - val = enable_logging ? ((verbosity_level << 4) | 0x1) : 0; - ret = snprintf(data, sizeof(data), "0x%" PRIx64, val); igt_assert(ret > 2 && ret < sizeof(data)); @@ -288,7 +297,7 @@ static void init_main_thread(void) /* Enable the logging, it may not have been enabled from boot and so * the relay file also wouldn't have been created. */ - guc_log_control(true); + guc_log_control(true, verbosity_level); open_relay_file(); open_output_file(); @@ -420,7 +429,7 @@ int main(int argc, char **argv) } while (!stop_logging); /* Pause logging on the GuC side */ - guc_log_control(false); + guc_log_control(false, 0); /* Signal flusher thread to make an exit */ capturing_stopped = 1;
i915 expects GuC log level to be specified as: 0: disabled 1: enabled (verbosity level 0 = min) 2: enabled (verbosity level 1) 3: enabled (verbosity level 2) 4: enabled (verbosity level 3 = max) Remove the earlier internal layout based logging control from guc_log_control and send new expected values. Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- tools/intel_guc_logger.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)