Re: [PATCH i-g-t 2/2] tests/intel/perf_pmu: Migrate to unbind + unload

[Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>]

On Mon, Nov 04, 2024 at 02:49:42PM -0600, Lucas De Marchi wrote:
>On Mon, Nov 04, 2024 at 11:25:06AM -0800, Umesh Nerlige Ramappa wrote:
>>On Tue, Oct 22, 2024 at 10:05:02PM -0700, Lucas De Marchi wrote:
>>>There isn't much point in testing we can't remove the module directly
>>>after removing *some* known dependencies. The point in this test is to
>>>make sure that if the HW vanishes, driver still behaves. That is
>>>currently not true, as it will cause perf to explode.
>>>
>>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>
>>LGTM,
>>
>>Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>
>I'm actually thinking about splitting this patch. See below.
>
>>
>>Thanks,
>>Umesh
>>>---
>>>tests/intel/perf_pmu.c | 22 ++++++++++++----------
>>>1 file changed, 12 insertions(+), 10 deletions(-)
>>>
>>>diff --git a/tests/intel/perf_pmu.c b/tests/intel/perf_pmu.c
>>>index bfa2d501a..6eb4f09d1 100644
>>>--- a/tests/intel/perf_pmu.c
>>>+++ b/tests/intel/perf_pmu.c
>>>@@ -2282,7 +2282,6 @@ static void test_unload(unsigned int num_engines)
>>>		int fd[4 + num_engines * 3], i;
>>>		uint64_t *buf;
>>>		int count = 0, ret;
>>>-		char *who = NULL;
>>>		int i915;
>>>
>>>		i915 = __drm_open_driver(DRIVER_INTEL);
>>>@@ -2337,23 +2336,26 @@ static void test_unload(unsigned int num_engines)
>>>
>>>		igt_debug("Read %d events from perf and trial unload\n", count);
>>>		pmu_read_multi(fd[0], count, buf);
>>>-		ret = __igt_i915_driver_unload(&who);
>
>I think I will drop this part first, so I can remove the
>__igt_i915_driver_unload() and finish migrating i915 to unbind + unload.
>
>Then when the kernel is fixed (which will probably happen on 6.13 or
>6.14) we can think about adding back the additions below for unbind
>while perf is open. Thoughts?

Agree. I just assumed that you will merge this series only after the 
kernel changes land, but I am okay with the above.

Thanks,
Umesh
>
>Lucas De Marchi