Re: [igt-dev] [PATCH i-g-t 2/8] lib/params: start renaming functions igt_params_*

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 22 Apr 2020, Petri Latvala <petri.latvala@intel.com> wrote:
> On Wed, Apr 22, 2020 at 11:34:44AM +0300, Jani Nikula wrote:
>> On Wed, 22 Apr 2020, Petri Latvala <petri.latvala@intel.com> wrote:
>> > On Tue, Apr 21, 2020 at 07:17:19PM +0300, Juha-Pekka Heikkila wrote:
>> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> >> ---
>> >>  lib/drmtest.c                         |  2 +-
>> >>  lib/i915/gem_submission.c             |  2 +-
>> >>  lib/igt_aux.c                         |  3 +--
>> >>  lib/igt_gt.c                          |  2 +-
>> >>  lib/igt_params.c                      | 12 +++++-------
>> >>  lib/igt_params.h                      |  5 ++---
>> >>  tests/i915/gem_ctx_exec.c             |  2 +-
>> >>  tests/i915/gem_ctx_persistence.c      |  9 ++++-----
>> >>  tests/i915/gem_mmap_gtt.c             |  2 +-
>> >>  tests/i915/gem_reset_stats.c          |  6 ++----
>> >>  tests/i915/sysfs_heartbeat_interval.c |  3 ++-
>> >>  tests/i915/sysfs_preempt_timeout.c    |  3 ++-
>> >>  tests/i915/sysfs_timeslice_duration.c |  3 ++-
>> >>  13 files changed, 25 insertions(+), 29 deletions(-)
>> >> 
>> >> diff --git a/lib/drmtest.c b/lib/drmtest.c
>> >> index 1fc39925..17067843 100644
>> >> --- a/lib/drmtest.c
>> >> +++ b/lib/drmtest.c
>> >> @@ -393,7 +393,7 @@ static void __cancel_work_at_exit(int fd)
>> >>  {
>> >>  	igt_terminate_spins(); /* for older kernels */
>> >>  
>> >> -	igt_sysfs_set_parameter(fd, "reset", "%x", -1u /* any method */);
>> >> +	igt_params_set(fd, "reset", "%x", -1u /* any method */);
>> >
>> >
>> > Hmm. Now that we're renaming anyway, we have a good chance to consider
>> > if the name is good.
>> >
>> > igt_module_params_set()? No, the point of this series is to make the
>> > params not module-specific. igt_device_params_set()?
>> >
>> > I don't have a strong opinion on this and I can live with
>> > igt_params_set() as well. Just calling out for opinions! Arek?
>> >
>> > As for the patch, whatever the name, straight up renames all around.
>> > Reviewed-by: Petri Latvala <petri.latvala@intel.com>
>> 
>> The way I envisioned it is that it would look at the debugfs for device
>> params, and if they don't exist, it should fall back to the module
>> params for older kernels. Hence just igt_params_set to avoid being
>> specific about device vs. module. Also, shorter.
>
> I was more concerned with the confusion between params for the kernel
> vs params for other operations. But I'm well prepared to accept that
> the confusion exists only in my own head.

Ah. I'll leave it up to your discretion if the confusion is bad
enough. At least I'm not hung up on the naming, so long as we unify the
naming, something this series tries to do as a side effect.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev