[PATCH i-g-t] tests/kms_cursor_legacy: Reduce debug loglevel dynamically

[Samala, Pranay]

Hi All,

Thanks for pointing that out. I'll float a fix for this issue.

Regards,
Pranay.

> -----Original Message-----
> From: Wheeler, Daniel <Daniel.Wheeler at amd.com>
> Sent: Wednesday, February 19, 2025 8:09 PM
> To: Nikula, Jani <jani.nikula at intel.com>; Li, Sun peng (Leo)
> <Sunpeng.Li at amd.com>; Samala, Pranay <pranay.samala at intel.com>; igt-
> dev at lists.freedesktop.org
> Cc: B S, Karthik <karthik.b.s at intel.com>; Joshi, Kunal1 <kunal1.joshi at intel.com>;
> Lattannavar, Sameer <sameer.lattannavar at intel.com>; Bhanuprakash Modem
> <bhanuprakash.modem at intel.com>; Kamil Konieczny
> <kamil.konieczny at linux.intel.com>; Juha-Pekka Heikkila
> <juhapekka.heikkila at gmail.com>; Dixit, Ashutosh <ashutosh.dixit at intel.com>
> Subject: RE: [PATCH i-g-t] tests/kms_cursor_legacy: Reduce debug loglevel
> dynamically
> 
> [Public]
> 
> Hey just checking  in to see if there has been any movement on this one. I'm still
> having to revert it from my local IGT repo or my runs take three times as long to
> complete/logs end up 10,000 times larger than they should be.
> 
> Thank you,
> 
> Dan Wheeler
> Sr. Technologist  |  AMD
> SW Display
> --------------------------------------------------------------------------------------------------
> ----------------
> 1 Commerce Valley Dr E, Thornhill, ON L3T 7X6 Facebook |  Twitter |  amd.com
> 
> 
> -----Original Message-----
> From: Jani Nikula <jani.nikula at intel.com>
> Sent: Monday, February 10, 2025 10:58 AM
> To: Li, Sun peng (Leo) <Sunpeng.Li at amd.com>; Pranay Samala
> <pranay.samala at intel.com>; igt-dev at lists.freedesktop.org
> Cc: karthik.b.s at intel.com; kunal1.joshi at intel.com;
> sameer.lattannavar at intel.com; Wheeler, Daniel <Daniel.Wheeler at amd.com>;
> Bhanuprakash Modem <bhanuprakash.modem at intel.com>; Kamil Konieczny
> <kamil.konieczny at linux.intel.com>; Juha-Pekka Heikkila
> <juhapekka.heikkila at gmail.com>; Ashutosh Dixit <ashutosh.dixit at intel.com>
> Subject: Re: [PATCH i-g-t] tests/kms_cursor_legacy: Reduce debug loglevel
> dynamically
> 
> On Mon, 10 Feb 2025, Leo Li <sunpeng.li at amd.com> wrote:
> > Hi Pranay,
> >
> > This change can also increase log verbosity; we observe this with amdgpu.
> > Details inline.
> >
> > On 2025-01-28 00:28, Pranay Samala wrote:
> >> This test is getting killed due to exceeding disk usage limit on CI
> >> environment because its logging to many logs.
> >>
> >> So dynamically reducing the debug log level to 10.
> >>
> >> Signed-off-by: Pranay Samala <pranay.samala at intel.com>
> >> ---
> >>   tests/kms_cursor_legacy.c | 11 +++++++++++
> >>   1 file changed, 11 insertions(+)
> >>
> >> diff --git a/tests/kms_cursor_legacy.c b/tests/kms_cursor_legacy.c
> >> index 1f082df2d..533972cd4 100644
> >> --- a/tests/kms_cursor_legacy.c
> >> +++ b/tests/kms_cursor_legacy.c
> >> @@ -40,6 +40,7 @@
> >>   #include "igt_psr.h"
> >>   #include "igt_rand.h"
> >>   #include "igt_stats.h"
> >> +#include "igt_sysfs.h"
> >>
> >>   /**
> >>    * SUBTEST: %s-%s
> >> @@ -1840,6 +1841,7 @@ igt_main
> >>      };
> >>
> >>      igt_fixture {
> >> +            int dir, current_log_level;
> >>              display.drm_fd = drm_open_driver_master(DRIVER_ANY);
> >>              kmstest_set_vt_graphics_mode();
> >>
> >> @@ -1850,6 +1852,15 @@ igt_main
> >>               * fetch is enabled, so switching PSR1 for this whole test.
> >>               */
> >>              intel_psr2_restore =
> >> i915_psr2_sel_fetch_to_psr1(display.drm_fd,
> >> NULL);
> >> +
> >> +            dir = igt_sysfs_drm_module_params_open();
> >> +            if (dir >= 0) {
> >> +                    current_log_level = igt_drm_debug_level_get(dir);
> >> +                    close(dir);
> >> +
> >> +                    if (current_log_level > 10)
> >> +                            igt_drm_debug_level_update(10);
> >
> > Isn't the DRM debug mask a bitmask?
> > https://elixir.bootlin.com/linux/v6.13.1/source/include/drm/drm_print.
> > h#L97
> >
> > So setting 10 isn't guaranteed to reduce verbosity, depending on how
> > the DRM driver uses the various drm_* debug prints.
> 
> Ugh. Yes.
> 
> > What is guaranteed to reduce verbosity is to unset unwanted bits.
> > Maybe that is the better approach?
> >
> > For example, instead of
> > `if (current_log_level > 10)`,
> >
> > Do
> >
> > `igt_drm_debug_level_update(current_log_level & !(DRM_UT_CORE | ...);`
> 
> Ditto for commit a2ab0ec12ef4 ("tests/kms_atomic_transition: Reducing debug
> loglevel dynamically"). It got to v6 and nobody noticed?
> 
> Ditto for commit 56b91193b825 ("lib/igt_sysfs: Implement dynamic adjustment of
> debug log level") adding the whole thing. It should not be referred to as "level"
> because it has nothing to do with "level". It also makes no sense to print it out in
> decimal; hex makes more sense.
> 
> 
> BR,
> Jani.
> 
> 
> >
> > Thanks,
> > Leo
> >
> >> +            }
> >>      }
> >>
> >>      igt_describe("Test checks how many cursor updates we can fit between
> vblanks "
> >
> 
> --
> Jani Nikula, Intel