Re: [PATCH] intel_get_llc_size: Small tool to query LLC size

[Ben Widawsky <ben@bwidawsk.net>]

On Wed, Jul 10, 2013 at 07:15:43PM +0200, Daniel Vetter wrote:
> On Wed, Jul 10, 2013 at 09:58:38AM -0700, Ben Widawsky wrote:
> > On Wed, Jul 10, 2013 at 08:34:18AM +0200, Daniel Vetter wrote:
> > > On Tue, Jul 09, 2013 at 07:58:03PM -0700, Ben Widawsky wrote:
> > > > CC: Chad Versace <chad.versace@linux.intel.com>
> > > > CC: Bryan Bell <bryan.j.bell@intel.com>
> > > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > > 
> > > So I think we should run this from igt and check its return value. And
> > > since we've had a few bugs with other (currently untested) igt tools, can
> > > you please add a new igt_tools testcase which just runs those? I'm
> > > thinking of intel_reg_dumper and intel_reg_read (with some render ring
> > > register that exists everywhere) on top of running intel_get_llc_size
> > > here.
> > 
> > What would you like exactly for the check? Just to make sure platforms
> > that should have LLC return a value > 1?
> 
> Just check that it runs without an non-zero exit code.

That doesn't really check the interface works as advertised though. Same
for the below.

> 
> > As for testing the other tools, I personally want no involvement with
> > intel_reg_dumper, since I feel maintaining it is a losing effort and
> > would rather see effort put into quick_dump. I'll write tests for
> > reg_read/reg_write though.
> 
> Again just checking whether the stuff runs without non-zero exit code. I
> guess the same holds for quick_dump. As long as we still use reg_dumper
> for debugging regressions I'd would be imo really good to have a little
> bit of assurance that it's not completely broken.
> 
> Cheers, Daniel

That's fine... just asking you to volunteer someone else who actually
uses that tool for it. I never use reg_dumper, and don't ask others to
use it.

> 
> > > And please also add eLLC size querying.
> > 
> > When I see review and signs of those getting merged, I'll gladly do
> > that. For simple, potentially very useful patches, they're sitting
> > around doing nothing for an awfully long time. And, I can no longer
> > easily test them. So poop.
> > 
> > > -Daniel
> > > 
> > > > ---
> > > >  tools/Makefile.am          |  1 +
> > > >  tools/intel_get_llc_size.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++
> > > >  2 files changed, 58 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..bd384d2
> > > > --- /dev/null
> > > > +++ b/tools/intel_get_llc_size.c
> > > > @@ -0,0 +1,57 @@
> > > > +/*
> > > > + * 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"
> > > > +
> > > > +static int get_llc_size(int fd)
> > > > +{
> > > > +	struct drm_i915_getparam gp;
> > > > +	int size;
> > > > +
> > > > +	gp.param = I915_PARAM_HAS_LLC;
> > > > +	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;
> > > > +}
> > > > -- 
> > > > 1.8.3.2
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > 
> > > -- 
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > 
> > -- 
> > Ben Widawsky, Intel Open Source Technology Center
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx