Re: [igt-dev] [PATCH i-g-t 01/15] perf_pmu: Support multi-tile in rc6 subtest

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Fri, 12 May 2023 17:08:24 -0700, Umesh Nerlige Ramappa wrote:
>
> On Fri, May 12, 2023 at 01:14:37PM +0100, Tvrtko Ursulin wrote:
> >
> > On 12/05/2023 03:28, Dixit, Ashutosh wrote:
> >> On Fri, 05 May 2023 17:55:14 -0700, Umesh Nerlige Ramappa wrote:
> >>> @@ -290,6 +299,12 @@ enum drm_i915_pmu_engine_sample {
> >>>
> >>>  #define I915_PMU_LAST /* Deprecated - do not use */ I915_PMU_RC6_RESIDENCY
> >>>
> >>> +#define __I915_PMU_ACTUAL_FREQUENCY(gt)		___I915_PMU_OTHER(gt, 0)
> >>> +#define __I915_PMU_REQUESTED_FREQUENCY(gt)	___I915_PMU_OTHER(gt, 1)
> >>> +#define __I915_PMU_INTERRUPTS(gt)		___I915_PMU_OTHER(gt, 2)
> >>> +#define __I915_PMU_RC6_RESIDENCY(gt)		___I915_PMU_OTHER(gt, 3)
> >>> +#define __I915_PMU_SOFTWARE_GT_AWAKE_TIME(gt)	___I915_PMU_OTHER(gt, 4)
> >>> +
> >>>  /* Each region is a minimum of 16k, and there are at most 255 of them.
> >>>   */
> >>>  #define I915_NR_TEX_REGIONS 255	/* table size 2k - maximum due to use
> >>> diff --git a/tests/i915/perf_pmu.c b/tests/i915/perf_pmu.c
> >>> index c5f083bbd..97ad09d76 100644
> >>> --- a/tests/i915/perf_pmu.c
> >>> +++ b/tests/i915/perf_pmu.c
> >>> @@ -1707,8 +1707,16 @@ static bool wait_for_suspended(int gem_fd)
> >>>	return suspended;
> >>>  }
> >>>
> >>> +static int open_forcewake_handle(int fd, unsigned int gt)
> >>> +{
> >>> +	if (getenv("IGT_NO_FORCEWAKE"))
> >>> +		return -1;
> >>> +
> >>> +	return igt_debugfs_gt_open(fd, gt, "forcewake_user", O_WRONLY);
> >>> +}
> >>
> >> Let's create a new function igt_open_forcewake_gt_handle() below
> >> igt_open_forcewake_handle() in lib/igt_gt.c and add this code there so the
> >> code can be shared.
> >
> > Typically we'd move to lib/ only when there are 2-3 callers and so it is
> > clear helper is useful. Don't know, I am okay either way.
> >
> leaving as is

Chicken and egg. Not moving to lib/ pretty much ensures local copies will
profilerate. Hence my suggestion. Anyway.

Thanks.
--
Ashutosh