[i-g-t,v3,1/3] lib/ioctl_wrapper: Add i915_get_param helper

Submitted by Lukasz Kalamarz on Nov. 26, 2018, 3:36 p.m.

Details

Message ID 20181126153624.3690-1-lukasz.kalamarz@intel.com
State New
Series "Series without cover letter"
Headers show

Commit Message

Lukasz Kalamarz Nov. 26, 2018, 3:36 p.m.
getparam is used in few places across IGT, but no helper function
is used to reduce code duplication.
v2: Added doc part and changed return value in case of error

Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>

Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Katarzyna Dec <katarzyna.dec@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 lib/ioctl_wrappers.c | 22 ++++++++++++++++++++++
 lib/ioctl_wrappers.h |  1 +
 2 files changed, 23 insertions(+)

Patch hide | download patch | download mbox

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 9f255508..aec4f15e 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -468,6 +468,28 @@  void gem_sync(int fd, uint32_t handle)
 	errno = 0;
 }
 
+/**
+ * drm_get_param:
+ * @fd: open i915 drm file descriptor
+ * @param: drm parameter we want to read
+ *
+ * Helper function that execute GETPARAM ioctl for a given parameter.
+ *
+ * Return: Read value from GETPARAM
+ */
+int drm_get_param(int fd, uint32_t param)
+{
+	int value;
+	drm_i915_getparam_t gp = {
+		.param = param,
+		.value = &value
+	};
+
+	if (igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) == -1)
+		return -EINVAL;
+
+	return value;
+}
 
 bool gem_create__has_stolen_support(int fd)
 {
diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
index b22b36b0..90174d13 100644
--- a/lib/ioctl_wrappers.h
+++ b/lib/ioctl_wrappers.h
@@ -74,6 +74,7 @@  int __gem_set_domain(int fd, uint32_t handle, uint32_t read, uint32_t write);
 void gem_set_domain(int fd, uint32_t handle, uint32_t read, uint32_t write);
 int gem_wait(int fd, uint32_t handle, int64_t *timeout_ns);
 void gem_sync(int fd, uint32_t handle);
+int drm_get_param(int fd, uint32_t param);
 bool gem_create__has_stolen_support(int fd);
 uint32_t __gem_create_stolen(int fd, uint64_t size);
 uint32_t gem_create_stolen(int fd, uint64_t size);

Comments

Eric Anholt Nov. 26, 2018, 8:49 p.m.
Lukasz Kalamarz <lukasz.kalamarz@intel.com> writes:

> getparam is used in few places across IGT, but no helper function
> is used to reduce code duplication.
> v2: Added doc part and changed return value in case of error
>
> Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
>
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>  lib/ioctl_wrappers.c | 22 ++++++++++++++++++++++
>  lib/ioctl_wrappers.h |  1 +
>  2 files changed, 23 insertions(+)
>
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 9f255508..aec4f15e 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -468,6 +468,28 @@ void gem_sync(int fd, uint32_t handle)
>  	errno = 0;
>  }
>  
> +/**
> + * drm_get_param:
> + * @fd: open i915 drm file descriptor
> + * @param: drm parameter we want to read

Could we please call this i915_get_param, to match the commit message,
instead of drm_get_param()?  I know igt used to call i915-only ioctls
drm_* or gem_*, but it would be nice to see new functions use a more
appropriate namespace.

(I wouldn't block this, but if people decide not to do that then the
commit message subject should at least change)
Daniele Ceraolo Spurio Nov. 28, 2018, 9:34 p.m.
On 26/11/2018 07:36, Lukasz Kalamarz wrote:
> getparam is used in few places across IGT, but no helper function
> is used to reduce code duplication.
> v2: Added doc part and changed return value in case of error
> 
> Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>
> 
> Cc: Michal Winiarski <michal.winiarski@intel.com>
> Cc: Katarzyna Dec <katarzyna.dec@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   lib/ioctl_wrappers.c | 22 ++++++++++++++++++++++
>   lib/ioctl_wrappers.h |  1 +
>   2 files changed, 23 insertions(+)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 9f255508..aec4f15e 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -468,6 +468,28 @@ void gem_sync(int fd, uint32_t handle)
>   	errno = 0;
>   }
>   
> +/**
> + * drm_get_param:
> + * @fd: open i915 drm file descriptor
> + * @param: drm parameter we want to read
> + *
> + * Helper function that execute GETPARAM ioctl for a given parameter.
> + *
> + * Return: Read value from GETPARAM
> + */
> +int drm_get_param(int fd, uint32_t param)
> +{
> +	int value;
> +	drm_i915_getparam_t gp = {
> +		.param = param,
> +		.value = &value
> +	};
> +
> +	if (igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp) == -1)
> +		return -EINVAL;

Returning the errno value would be more appropriate here IMO.

Daniele

> +
> +	return value;
> +}
>   
>   bool gem_create__has_stolen_support(int fd)
>   {
> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
> index b22b36b0..90174d13 100644
> --- a/lib/ioctl_wrappers.h
> +++ b/lib/ioctl_wrappers.h
> @@ -74,6 +74,7 @@ int __gem_set_domain(int fd, uint32_t handle, uint32_t read, uint32_t write);
>   void gem_set_domain(int fd, uint32_t handle, uint32_t read, uint32_t write);
>   int gem_wait(int fd, uint32_t handle, int64_t *timeout_ns);
>   void gem_sync(int fd, uint32_t handle);
> +int drm_get_param(int fd, uint32_t param);
>   bool gem_create__has_stolen_support(int fd);
>   uint32_t __gem_create_stolen(int fd, uint64_t size);
>   uint32_t gem_create_stolen(int fd, uint64_t size);
>