diff mbox

[1/2] intel_get_llc_size: Small tool to query LLC size

Message ID 1373568787-3145-1-git-send-email-ben@bwidawsk.net (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Widawsky July 11, 2013, 6:53 p.m. UTC
v2: Use the new param

CC: Chad Versace <chad.versace@linux.intel.com>
CC: Bryan Bell <bryan.j.bell@intel.com>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 tools/Makefile.am          |  1 +
 tools/intel_get_llc_size.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)
 create mode 100644 tools/intel_get_llc_size.c

Comments

Chad Versace July 12, 2013, 5:39 p.m. UTC | #1
On 07/11/2013 11:53 AM, Ben Widawsky wrote:
> v2: Use the new param
>
> CC: Chad Versace <chad.versace@linux.intel.com>
> CC: Bryan Bell <bryan.j.bell@intel.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>   tools/Makefile.am          |  1 +
>   tools/intel_get_llc_size.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 59 insertions(+)
>   create mode 100644 tools/intel_get_llc_size.c
>
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 2519169..a064b65 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -9,6 +9,7 @@ bin_PROGRAMS = 				\
>   	intel_bios_dumper 		\
>   	intel_bios_reader 		\
>   	intel_error_decode 		\
> +	intel_get_llc_size 		\
>   	intel_gpu_top 			\
>   	intel_gpu_time 			\
>   	intel_gtt 			\
> diff --git a/tools/intel_get_llc_size.c b/tools/intel_get_llc_size.c
> new file mode 100644
> index 0000000..498e252
> --- /dev/null
> +++ b/tools/intel_get_llc_size.c
> @@ -0,0 +1,58 @@
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#include <sys/ioctl.h>
> +#include "drmtest.h"
> +#include "i915_drm.h"
> +
> +#define LOCAL__I915_PARAM_LLC_SIZE 27
> +static int get_llc_size(int fd)
> +{
> +	struct drm_i915_getparam gp;
> +	int size;
> +
> +	gp.param = LOCAL__I915_PARAM_LLC_SIZE;
> +	gp.value = &size;
> +
> +	if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)))
> +		return 0;
> +
> +	return size;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +	int size, fd = drm_open_any();
> +
> +	size = get_llc_size(fd);
> +
> +	if (size == 0)
> +		fprintf(stdout, "Doesn't have LLC\n");
> +	else if (size == 1)
> +		fprintf(stdout, "Kernel is too old to determine LLC size\n");
> +	else
> +		fprintf(stdout, "LLC size = %dK\n", size>>10);

This if-tree looks wrong. If the kernel doesn't know about I915_PARAM_LLC_SIZE,
the ioctl returns -1, and get_llc_size() returns 0, and the if-tree prints the
wrong error.

How about making get_llc_size() return -1 on error? Or am I missing something here?

> +
> +	return 0;
> +}
>
Ben Widawsky July 12, 2013, 5:49 p.m. UTC | #2
On Fri, Jul 12, 2013 at 10:39:51AM -0700, Chad Versace wrote:
> On 07/11/2013 11:53 AM, Ben Widawsky wrote:
> >v2: Use the new param
> >
> >CC: Chad Versace <chad.versace@linux.intel.com>
> >CC: Bryan Bell <bryan.j.bell@intel.com>
> >Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> >---
> >  tools/Makefile.am          |  1 +
> >  tools/intel_get_llc_size.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 59 insertions(+)
> >  create mode 100644 tools/intel_get_llc_size.c
> >
> >diff --git a/tools/Makefile.am b/tools/Makefile.am
> >index 2519169..a064b65 100644
> >--- a/tools/Makefile.am
> >+++ b/tools/Makefile.am
> >@@ -9,6 +9,7 @@ bin_PROGRAMS = 				\
> >  	intel_bios_dumper 		\
> >  	intel_bios_reader 		\
> >  	intel_error_decode 		\
> >+	intel_get_llc_size 		\
> >  	intel_gpu_top 			\
> >  	intel_gpu_time 			\
> >  	intel_gtt 			\
> >diff --git a/tools/intel_get_llc_size.c b/tools/intel_get_llc_size.c
> >new file mode 100644
> >index 0000000..498e252
> >--- /dev/null
> >+++ b/tools/intel_get_llc_size.c
> >@@ -0,0 +1,58 @@
> >+/*
> >+ * Copyright © 2013 Intel Corporation
> >+ *
> >+ * Permission is hereby granted, free of charge, to any person obtaining a
> >+ * copy of this software and associated documentation files (the "Software"),
> >+ * to deal in the Software without restriction, including without limitation
> >+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> >+ * and/or sell copies of the Software, and to permit persons to whom the
> >+ * Software is furnished to do so, subject to the following conditions:
> >+ *
> >+ * The above copyright notice and this permission notice (including the next
> >+ * paragraph) shall be included in all copies or substantial portions of the
> >+ * Software.
> >+ *
> >+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> >+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> >+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> >+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> >+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> >+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> >+ * DEALINGS IN THE SOFTWARE.
> >+ *
> >+ */
> >+
> >+#include <sys/ioctl.h>
> >+#include "drmtest.h"
> >+#include "i915_drm.h"
> >+
> >+#define LOCAL__I915_PARAM_LLC_SIZE 27
> >+static int get_llc_size(int fd)
> >+{
> >+	struct drm_i915_getparam gp;
> >+	int size;
> >+
> >+	gp.param = LOCAL__I915_PARAM_LLC_SIZE;
> >+	gp.value = &size;
> >+
> >+	if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)))
> >+		return 0;
> >+
> >+	return size;
> >+}
> >+
> >+int main(int argc, char **argv)
> >+{
> >+	int size, fd = drm_open_any();
> >+
> >+	size = get_llc_size(fd);
> >+
> >+	if (size == 0)
> >+		fprintf(stdout, "Doesn't have LLC\n");
> >+	else if (size == 1)
> >+		fprintf(stdout, "Kernel is too old to determine LLC size\n");
> >+	else
> >+		fprintf(stdout, "LLC size = %dK\n", size>>10);
> 
> This if-tree looks wrong. If the kernel doesn't know about I915_PARAM_LLC_SIZE,
> the ioctl returns -1, and get_llc_size() returns 0, and the if-tree prints the
> wrong error.
> 
> How about making get_llc_size() return -1 on error? Or am I missing something here?
>

Looks like the tool needed updating after the last param change and I
missed it. It should have been correct before (though in the case of no
HAS_LLC param it was also bad). Your suggestion is the right thing to do.

If Daniel decides to merge the kernel patch, I will fix this.

> 
> >+
> >+	return 0;
> >+}
> >
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/tools/Makefile.am b/tools/Makefile.am
index 2519169..a064b65 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -9,6 +9,7 @@  bin_PROGRAMS = 				\
 	intel_bios_dumper 		\
 	intel_bios_reader 		\
 	intel_error_decode 		\
+	intel_get_llc_size 		\
 	intel_gpu_top 			\
 	intel_gpu_time 			\
 	intel_gtt 			\
diff --git a/tools/intel_get_llc_size.c b/tools/intel_get_llc_size.c
new file mode 100644
index 0000000..498e252
--- /dev/null
+++ b/tools/intel_get_llc_size.c
@@ -0,0 +1,58 @@ 
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include <sys/ioctl.h>
+#include "drmtest.h"
+#include "i915_drm.h"
+
+#define LOCAL__I915_PARAM_LLC_SIZE 27
+static int get_llc_size(int fd)
+{
+	struct drm_i915_getparam gp;
+	int size;
+
+	gp.param = LOCAL__I915_PARAM_LLC_SIZE;
+	gp.value = &size;
+
+	if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)))
+		return 0;
+
+	return size;
+}
+
+int main(int argc, char **argv)
+{
+	int size, fd = drm_open_any();
+
+	size = get_llc_size(fd);
+
+	if (size == 0)
+		fprintf(stdout, "Doesn't have LLC\n");
+	else if (size == 1)
+		fprintf(stdout, "Kernel is too old to determine LLC size\n");
+	else
+		fprintf(stdout, "LLC size = %dK\n", size>>10);
+
+	return 0;
+}