Re: [PATCH v1] tests/intel/xe_debugfs: Extend gt test to check few debugfs entries

[Matthew Brost <matthew.brost@intel.com>]

On Wed, Dec 18, 2024 at 03:32:06PM +0100, Peter Senna Tschudin wrote:
> 
> 
> On 18.12.2024 12:57, Gurram, Pravalika wrote:
> > 
> > 
> >> -----Original Message-----
> >> From: Gurram, Pravalika
> >> Sent: Wednesday, December 18, 2024 3:42 PM
> >> To: Peter Senna Tschudin <peter.senna@linux.intel.com>; igt-
> >> dev@lists.freedesktop.org
> >> Subject: RE: [PATCH v1] tests/intel/xe_debugfs: Extend gt test to check few
> >> debugfs entries
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Peter Senna Tschudin <peter.senna@linux.intel.com>
> >>> Sent: Wednesday, December 18, 2024 3:07 PM
> >>> To: Gurram, Pravalika <pravalika.gurram@intel.com>; igt-
> >>> dev@lists.freedesktop.org
> >>> Subject: Re: [PATCH v1] tests/intel/xe_debugfs: Extend gt test to
> >>> check few debugfs entries
> >>>
> >>> Hi Pravalika,
> >>>
> >>> Two more cents.
> >>>
> >>> On 18.12.2024 09:56, Peter Senna Tschudin wrote:
> >>>> Hi Pravalika,
> >>>>
> >>>> Please see my comment below.
> >>>>
> >>>> On 17.12.2024 14:56, Pravalika Gurram wrote:
> >>>>> Read and dump  below debugfs entries.
> >>>>> ggtt
> >>>>> register-save-restore
> >>>>> workarounds
> >>>>> default_lrc_rcs
> >>>>> default_lrc_ccs
> >>>>> default_lrc_bcs
> >>>>> default_lrc_vcs
> >>>>> default_lrc_vecs
> >>>>> hwconfig"
> >>>>>
> >>>>> Signed-off-by: Pravalika Gurram <pravalika.gurram@intel.com>
> >>>>> ---
> >>>>>  tests/intel/xe_debugfs.c | 46
> >>>>> ++++++++++++++++++++++++++++++++++++++++
> >>>>>  1 file changed, 46 insertions(+)
> >>>>>
> >>>>> diff --git a/tests/intel/xe_debugfs.c b/tests/intel/xe_debugfs.c
> >>>>> index 700575031..bcbb5036a 100644
> >>>>> --- a/tests/intel/xe_debugfs.c
> >>>>> +++ b/tests/intel/xe_debugfs.c
> >>>>> @@ -162,6 +162,16 @@ test_gt(int fd, int gt_id)
> >>>>>  		"pat",
> >>>>>  		"mocs",
> >>>>>  //		"force_reset"
> >>>>> +		"ggtt",
> >>>>> +		"register-save-restore",
> >>>>> +		"workarounds",
> >>>>> +		"default_lrc_rcs",
> >>>>> +		"default_lrc_ccs",
> >>>>> +		"default_lrc_bcs",
> >>>>> +		"default_lrc_vcs",
> >>>>> +		"default_lrc_vecs",
> >>>>> +		"hwconfig"
> >>>>> +
> >>>
> >>> 1. Are we confident that these will always be present? I am asking because
> >>>    of the igt_assert() that will abort when an entry is not found.
> >>>

If the driver has loaded those entries will be present with the current
code base. ofc, debugfs entries are not uAPI so can change at anytime.
With that, the IGT needs to be changed in step with the KMD. It seems
fine to me to have a hard coded in table in an IGT asserting entries are
present.

Matt

> >> From kernel code point of view these debugfs need to be created on device
> >> boot up
> >>
> >> static const struct drm_info_list debugfs_list[] = {
> >>         {"hw_engines", .show = xe_gt_debugfs_simple_show, .data =
> >> hw_engines},
> >>         {"force_reset", .show = xe_gt_debugfs_simple_show, .data =
> >> force_reset},
> >>         {"force_reset_sync", .show = xe_gt_debugfs_simple_show, .data =
> >> force_reset_sync},
> >>         {"sa_info", .show = xe_gt_debugfs_simple_show, .data = sa_info},
> >>         {"topology", .show = xe_gt_debugfs_simple_show, .data = topology},
> >>         {"steering", .show = xe_gt_debugfs_simple_show, .data = steering},
> >>         {"ggtt", .show = xe_gt_debugfs_simple_show, .data = ggtt},
> >>         {"powergate_info", .show = xe_gt_debugfs_simple_show, .data =
> >> powergate_info},
> >>         {"register-save-restore", .show = xe_gt_debugfs_simple_show, .data =
> >> register_save_restore},
> >>         {"workarounds", .show = xe_gt_debugfs_simple_show, .data =
> >> workarounds},
> >>         {"pat", .show = xe_gt_debugfs_simple_show, .data = pat},
> >>         {"mocs", .show = xe_gt_debugfs_simple_show, .data = mocs},
> >>         {"default_lrc_rcs", .show = xe_gt_debugfs_simple_show, .data =
> >> rcs_default_lrc},
> >>         {"default_lrc_ccs", .show = xe_gt_debugfs_simple_show, .data =
> >> ccs_default_lrc},
> >>         {"default_lrc_bcs", .show = xe_gt_debugfs_simple_show, .data =
> >> bcs_default_lrc},
> >>         {"default_lrc_vcs", .show = xe_gt_debugfs_simple_show, .data =
> >> vcs_default_lrc},
> >>         {"default_lrc_vecs", .show = xe_gt_debugfs_simple_show, .data =
> >> vecs_default_lrc},
> >>         {"stats", .show = xe_gt_debugfs_simple_show, .data =
> >> xe_gt_stats_print_info},
> >>         {"hwconfig", .show = xe_gt_debugfs_simple_show, .data = hwconfig}, };
> >>> 2. Why don't we simply scan for available files instead of hard coding
> >>>    expected files?
> >>>
> >>> Thanks
> >> The test scope itself is like we need to check whether hard-coded entries
> >> that are created are present or not if not assert.
> >> and these changes are to improve the code coverage
> >>
> >> -- Pravlika
> > @Brost, Matthew could you please comment on question asked by peter
> 
> The problem is that the version of the kernel determines which files to create
> and IGT will always be lagging behind. If code coverage is the main objective,
> implementing something similar in C is likely to do the job:
> 
> $ cd /sys/kernel/debug/dri/0000:00:02.0/gt0
> $ find . -type f -exec cat {} \;
> 
> Matt, am I missing something or can we simply scan for files at the right
> directories?