Re: [PATCH v2 i-g-t 1/2] lib/igt_sysfs: Introduce engine_class property helpers

["Grzegorzek, Dominik" <dominik.grzegorzek@intel.com>]

Hi Dominik,

Since you made this generic it looks good to me, and I would vote for merging it to igt_sysfs,
as this could be useful function for anyone who would like to manipulate/read engine params.
However, Zbigniew had some objections againt it under mine previous comment in which I asked you to
place it here. Let's hear from him if this is acceptable in a current form. If not, I would just
move those functions as they are to the test file.

Regards, 
Dominik

On Wed, 2024-10-02 at 13:29 +0200, Dominik Karol Piątkowski wrote:
> Introduce the following engine_class helpers:
>  - xe_sysfs_engine_class_get_property
>    Convenience wrapper to get value of given property for given engine
>    class on given gt.
>  - xe_sysfs_engine_class_set_property
>    Convenience wrapper to set given property for given engine class on
>    given gt to given value.
> 
> Signed-off-by: Dominik Karol Piątkowski <dominik.karol.piatkowski@intel.com>
> ---
>  lib/igt_sysfs.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_sysfs.h |  5 +++
>  2 files changed, 90 insertions(+)
> 
> diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
> index aec0bb53d..8fe6d7ede 100644
> --- a/lib/igt_sysfs.c
> +++ b/lib/igt_sysfs.c
> @@ -1360,3 +1360,88 @@ int xe_sysfs_get_num_tiles(int xe_device)
>  
>  	return num_tiles;
>  }
> +
> +/**
> + * xe_sysfs_engine_class_get_property
> + * @xe_device: fd of the device
> + * @gt: gt number
> + * @class: engine class
> + * @property: property of engine class to retrieve
> + * @value: pointer for storing read value
> + *
> + * Convenience wrapper to get value of given property for given engine class on given gt.
> + *
> + * Returns: true on success, false on failure.
> + */
> +bool xe_sysfs_engine_class_get_property(int xe_device, int gt, uint16_t class, const char *property,
> +					uint32_t *value)
> +{
> +	int engines_fd;
> +
> +	engines_fd = xe_sysfs_engine_open(xe_device, gt, class);
> +
> +	if (engines_fd == -1) {
> +		igt_debug("Failed to open %s on gt%d.\n", xe_engine_class_to_str(class), gt);
> +
> +		return false;
> +	}
> +
> +	if (!__igt_sysfs_get_u32(engines_fd, property, value)) {
> +		igt_debug("Failed to read %s property of %s on gt%d.\n", property,
> +			  xe_engine_class_to_str(class), gt);
> +		close(engines_fd);
> +
> +		return false;
> +	}
> +
> +	close(engines_fd);
> +
> +	return true;
> +}
> +
> +/**
> + * xe_sysfs_engine_class_set_property
> + * @xe_device: fd of the device
> + * @gt: gt number
> + * @class: engine class
> + * @property: property of engine class to be modified
> + * @new_value: value to be set
> + * @old_value: pointer for storing old value, can be NULL
> + *
> + * Convenience wrapper to set given property for given engine class on given gt to given value.
> + *
> + * Returns: true on success, false on failure.
> + */
> +bool xe_sysfs_engine_class_set_property(int xe_device, int gt, uint16_t class, const char *property,
> +					uint32_t new_value, uint32_t *old_value)
> +{
> +	int engines_fd;
> +
> +	engines_fd = xe_sysfs_engine_open(xe_device, gt, class);
> +
> +	if (engines_fd == -1) {
> +		igt_debug("Failed to open %s on gt%d.\n", xe_engine_class_to_str(class), gt);
> +
> +		return false;
> +	}
> +
> +	if (old_value && !__igt_sysfs_get_u32(engines_fd, property, old_value)) {
> +		igt_debug("Failed to read %s property of %s on gt%d.\n", property,
> +			  xe_engine_class_to_str(class), gt);
> +		close(engines_fd);
> +
> +		return false;
> +	}
> +
> +	if (!__igt_sysfs_set_u32(engines_fd, property, new_value)) {
> +		igt_debug("Failed to write %s property of %s on gt%d.\n", property,
> +			  xe_engine_class_to_str(class), gt);
> +		close(engines_fd);
> +
> +		return false;
> +	}
> +
> +	close(engines_fd);
> +
> +	return true;
> +}
> diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
> index 2a1e3b1bf..3cede07f9 100644
> --- a/lib/igt_sysfs.h
> +++ b/lib/igt_sysfs.h
> @@ -175,4 +175,9 @@ int xe_sysfs_get_num_tiles(int xe_device);
>  char *xe_sysfs_engine_path(int xe_device, int gt, int class, char *path, int pathlen);
>  int xe_sysfs_engine_open(int xe_device, int gt, int class);
>  
> +bool xe_sysfs_engine_class_get_property(int xe_device, int gt, uint16_t class, const char *property,
> +					uint32_t *value);
> +bool xe_sysfs_engine_class_set_property(int xe_device, int gt, uint16_t class, const char *property,
> +					uint32_t new_value, uint32_t *old_value);
> +
>  #endif /* __IGT_SYSFS_H__ */