[Intel-gfx] [Beignet] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams
On Fri, Mar 13, 2015 at 05:59:13PM +0100, Daniel Vetter wrote: > On Fri, Mar 13, 2015 at 09:51:57AM -0700, Jeff McGee wrote: > > On Fri, Mar 13, 2015 at 05:32:41PM +0100, Daniel Vetter wrote: > > > On Fri, Mar 13, 2015 at 05:09:46PM +0800, Zhigang Gong wrote: > > > > My only concern is about the following macros: > > > > > > > > > +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL 33 > > > > > +#define LOCAL_I915_PARAM_EU_TOTAL34 > > > > > > > > How about to just use the definitons in the kernel header file? > > > > For an example: > > > > > > > > #include > > > > > > > > #ifdef LOCAL_I915_PARAM_SUBSLICE_TOTAL > > > > //Put all the code into this block. > > > > #endif > > > > > > > > Then we can avoid put the same definitions in different files, > > > > and we can avoid unecessary testing on an old kernel which doesn't > > > > have this kernel interface. > > > > > > > > For all the other part, it LGTM. > > > > > > > > Reviewed-by: Zhigang Gong > > > > > > Once we update the libdrm requirements in igt we tend to go around and > > > replace all the now obsolete LOCAL_ defines. Imo not worth doing extra > > > work until then. > > > > > > Patch applied, thanks. > > > -Daniel > > > > > > > Patch applied? Do you want me to make the name change first? Should the > > kernel part be reviewed and merged first? > > Forgot my own review comment already ;-) Fixed up with a follow-up patch. > And I'll pull the kernel part in now too. > -Daniel Thanks. That leaves just the libdrm patch and version bump before Beignet can merge their patches. -Jeff
[Beignet] [Intel-gfx] [PATCH i-g-t v3] tests/core_getparams: Create new test core_getparams
On Fri, Mar 13, 2015 at 05:32:41PM +0100, Daniel Vetter wrote: > On Fri, Mar 13, 2015 at 05:09:46PM +0800, Zhigang Gong wrote: > > My only concern is about the following macros: > > > > > +#define LOCAL_I915_PARAM_SUBSLICE_TOTAL 33 > > > +#define LOCAL_I915_PARAM_EU_TOTAL34 > > > > How about to just use the definitons in the kernel header file? > > For an example: > > > > #include > > > > #ifdef LOCAL_I915_PARAM_SUBSLICE_TOTAL > > //Put all the code into this block. > > #endif > > > > Then we can avoid put the same definitions in different files, > > and we can avoid unecessary testing on an old kernel which doesn't > > have this kernel interface. > > > > For all the other part, it LGTM. > > > > Reviewed-by: Zhigang Gong > > Once we update the libdrm requirements in igt we tend to go around and > replace all the now obsolete LOCAL_ defines. Imo not worth doing extra > work until then. > > Patch applied, thanks. > -Daniel > Patch applied? Do you want me to make the name change first? Should the kernel part be reviewed and merged first? -Jeff > > > > Thanks, > > Zhigang Gong. > > > > On Thu, Mar 12, 2015 at 05:26:25PM -0700, jeff.mcgee at intel.com wrote: > > > From: Jeff McGee > > > > > > New test core_getparams consists of 2 subtests, each one testing > > > the ability of userspace to query the correct value of a GT config > > > attribute: subslice total or EU total. drm/i915 implementation of > > > these queries is required for Cherryview and Gen9+ devices (non- > > > simulated). > > > > > > v2: Duplicate small amount of new libdrm functionality to avoid > > > bumping libdrm version requirement (Daniel). Convert some > > > igt_asserts to the appropriate comparison variants. Add a > > > test description. > > > v3: Actually use the LOCAL GETPARAM defines. Otherwise can't build > > > against older libdrm as intended by v2. > > > > > > For: VIZ-4636 > > > Signed-off-by: Jeff McGee > > > --- > > > tests/.gitignore | 1 + > > > tests/Makefile.sources | 1 + > > > tests/core_getparams.c | 167 > > > + > > > 3 files changed, 169 insertions(+) > > > create mode 100644 tests/core_getparams.c > > > > > > diff --git a/tests/.gitignore b/tests/.gitignore > > > index 426cc67..c742308 100644 > > > --- a/tests/.gitignore > > > +++ b/tests/.gitignore > > > @@ -1,6 +1,7 @@ > > > # Please keep sorted alphabetically > > > core_get_client_auth > > > core_getclient > > > +core_getparams > > > core_getstats > > > core_getversion > > > drm_import_export > > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > > > index 51e8376..999c8f8 100644 > > > --- a/tests/Makefile.sources > > > +++ b/tests/Makefile.sources > > > @@ -15,6 +15,7 @@ NOUVEAU_TESTS_M = \ > > > > > > TESTS_progs_M = \ > > > core_get_client_auth \ > > > + core_getparams \ > > > drv_suspend \ > > > drv_hangman \ > > > gem_bad_reloc \ > > > diff --git a/tests/core_getparams.c b/tests/core_getparams.c > > > new file mode 100644 > > > index 000..2855d06 > > > --- /dev/null > > > +++ b/tests/core_getparams.c > > > @@ -0,0 +1,167 @@ > > > +/* > > > + * Copyright © 2014 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 > > > + * IMP
[Beignet] [PATCH 2/2 v2] Query the driver directly for compute units and subslice
On Thu, Mar 12, 2015 at 10:08:54AM +0800, Zhigang Gong wrote: > LGTM, > > Reviewed-by: Zhigang Gong > > Thanks. > Thanks for the review, Zhigang. With beignet portion reviewed, review should be able to proceed for the i915, libdrm, and igt parts. These are all quite simple. Can someone(s) please review? -Jeff
[Beignet] [Intel-gfx] [PATCH i-g-t 2/2] configure: Bump required libdrm version to 2.4.60
On Wed, Mar 11, 2015 at 08:21:36AM +0100, Daniel Vetter wrote: > On Tue, Mar 10, 2015 at 01:06:44PM -0700, Jeff McGee wrote: > > On Tue, Mar 10, 2015 at 07:47:03PM +0100, Daniel Vetter wrote: > > > On Tue, Mar 10, 2015 at 01:58:52PM -0400, Rob Clark wrote: > > > > On Tue, Mar 10, 2015 at 12:59 PM, Jeff McGee > > > > wrote: > > > > > On Tue, Mar 10, 2015 at 08:37:30AM +0100, Daniel Vetter wrote: > > > > >> On Mon, Mar 09, 2015 at 04:41:02PM -0700, jeff.mcgee at intel.com > > > > >> wrote: > > > > >> > From: Jeff McGee > > > > >> > > > > > >> > tests/core_getparams needs the new libdrm interfaces for > > > > >> > querying subslice and EU counts. > > > > >> > > > > > >> > For: VIZ-4636 > > > > >> > Signed-off-by: Jeff McGee > > > > >> > --- > > > > >> > configure.ac | 2 +- > > > > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > >> > > > > > >> > diff --git a/configure.ac b/configure.ac > > > > >> > index 16d6a2e..88a1c3d 100644 > > > > >> > --- a/configure.ac > > > > >> > +++ b/configure.ac > > > > >> > @@ -82,7 +82,7 @@ if test "x$GCC" = "xyes"; then > > > > >> > fi > > > > >> > AC_SUBST(ASSEMBLER_WARN_CFLAGS) > > > > >> > > > > > >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.52 libdrm]) > > > > >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.60 libdrm]) > > > > >> > > > > >> Please don't and instead copypaste the new structs/defines with a > > > > >> local_ > > > > >> prefix like we do it for all the other new igt testcases. Forcing > > > > >> libdrm > > > > >> to get updated for igt all the time can get annoying fast. > > > > >> -Daniel > > > > >> > > > > > In this case I'm trying to exercise new API functions in libdrm which > > > > > wrap the GETPARAM ioctl. Would you rather me bypass the wrapper to > > > > > avoid requiring updated libdrm? I can do that, but it fails to test > > > > > the > > > > > complete path that client would use. > > > > > > > > > > > > Am I missing something, or does 2.4.60 not exist yet? > > > > > > > > That said dependency bumps for igt seem like less of an issue than > > > > dependency bumps for mesa.. I mean if you are using igt you are > > > > probably on the latest anyways.. I'm not sure why Daniel is so > > > > concerned about that.. > > > > > > > > (but dependency bumps to something that doesn't exist yet should > > > > perhaps be avoided) > > > > > > I'd like to avoid massive depency loops for igt tests so that I can merge > > > the testcase right when the patches land in -nightly. Otherwise there's > > > always a small delay involved where regression can creep in. Also if I > > > have to update libdrm every time I update igt that's annoying since > > > without that I don't have to install/update anything at all - I run igt > > > in-place. And we've used the LOCAL_ prefixes for pretty much every abi > > > addition in igt tests thus far. > > > -Daniel > > > > I understand that and it certainly makes sense when libdrm is only > > providing defines or structs. But as I said, in this case there is > > code in libdrm (the wrapper) that we could test as part of the > > complete path. Are you recommending that I implement duplicate > > wrapper functions in igt with the local prefix? > > Sorry I totally didn't realize that. Generally we don't have a lot of igt > testcase for libdrm really, imo it's usually simpler to just add the > interface to each part. Since this is such a simple one there's no need to > have a low-level test and the libdrm test on top, but that's what I'd do > if there's something worth testing in libdrm. Because for complex > functionality I really want to get the bare-metal tests in together with > the kernel part. Stalling for libdrm release could take longer. > > And yes, personally I'd just have open-coded the getparam call here in > igt, but that's just a bikeshed. > -Daniel > -- Scratch this patch. I just sent v2 of the previous patch that removes the dependency on libdrm update. -Jeff
[Intel-gfx] [Beignet] [PATCH i-g-t 2/2] configure: Bump required libdrm version to 2.4.60
On Tue, Mar 10, 2015 at 07:47:03PM +0100, Daniel Vetter wrote: > On Tue, Mar 10, 2015 at 01:58:52PM -0400, Rob Clark wrote: > > On Tue, Mar 10, 2015 at 12:59 PM, Jeff McGee > > wrote: > > > On Tue, Mar 10, 2015 at 08:37:30AM +0100, Daniel Vetter wrote: > > >> On Mon, Mar 09, 2015 at 04:41:02PM -0700, jeff.mcgee at intel.com wrote: > > >> > From: Jeff McGee > > >> > > > >> > tests/core_getparams needs the new libdrm interfaces for > > >> > querying subslice and EU counts. > > >> > > > >> > For: VIZ-4636 > > >> > Signed-off-by: Jeff McGee > > >> > --- > > >> > configure.ac | 2 +- > > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > > >> > > > >> > diff --git a/configure.ac b/configure.ac > > >> > index 16d6a2e..88a1c3d 100644 > > >> > --- a/configure.ac > > >> > +++ b/configure.ac > > >> > @@ -82,7 +82,7 @@ if test "x$GCC" = "xyes"; then > > >> > fi > > >> > AC_SUBST(ASSEMBLER_WARN_CFLAGS) > > >> > > > >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.52 libdrm]) > > >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.60 libdrm]) > > >> > > >> Please don't and instead copypaste the new structs/defines with a local_ > > >> prefix like we do it for all the other new igt testcases. Forcing libdrm > > >> to get updated for igt all the time can get annoying fast. > > >> -Daniel > > >> > > > In this case I'm trying to exercise new API functions in libdrm which > > > wrap the GETPARAM ioctl. Would you rather me bypass the wrapper to > > > avoid requiring updated libdrm? I can do that, but it fails to test the > > > complete path that client would use. > > > > > > Am I missing something, or does 2.4.60 not exist yet? > > > > That said dependency bumps for igt seem like less of an issue than > > dependency bumps for mesa.. I mean if you are using igt you are > > probably on the latest anyways.. I'm not sure why Daniel is so > > concerned about that.. > > > > (but dependency bumps to something that doesn't exist yet should > > perhaps be avoided) > > I'd like to avoid massive depency loops for igt tests so that I can merge > the testcase right when the patches land in -nightly. Otherwise there's > always a small delay involved where regression can creep in. Also if I > have to update libdrm every time I update igt that's annoying since > without that I don't have to install/update anything at all - I run igt > in-place. And we've used the LOCAL_ prefixes for pretty much every abi > addition in igt tests thus far. > -Daniel I understand that and it certainly makes sense when libdrm is only providing defines or structs. But as I said, in this case there is code in libdrm (the wrapper) that we could test as part of the complete path. Are you recommending that I implement duplicate wrapper functions in igt with the local prefix? -Jeff
[Beignet] [Intel-gfx] [PATCH i-g-t 2/2] configure: Bump required libdrm version to 2.4.60
On Tue, Mar 10, 2015 at 01:58:52PM -0400, Rob Clark wrote: > On Tue, Mar 10, 2015 at 12:59 PM, Jeff McGee wrote: > > On Tue, Mar 10, 2015 at 08:37:30AM +0100, Daniel Vetter wrote: > >> On Mon, Mar 09, 2015 at 04:41:02PM -0700, jeff.mcgee at intel.com wrote: > >> > From: Jeff McGee > >> > > >> > tests/core_getparams needs the new libdrm interfaces for > >> > querying subslice and EU counts. > >> > > >> > For: VIZ-4636 > >> > Signed-off-by: Jeff McGee > >> > --- > >> > configure.ac | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/configure.ac b/configure.ac > >> > index 16d6a2e..88a1c3d 100644 > >> > --- a/configure.ac > >> > +++ b/configure.ac > >> > @@ -82,7 +82,7 @@ if test "x$GCC" = "xyes"; then > >> > fi > >> > AC_SUBST(ASSEMBLER_WARN_CFLAGS) > >> > > >> > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.52 libdrm]) > >> > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.60 libdrm]) > >> > >> Please don't and instead copypaste the new structs/defines with a local_ > >> prefix like we do it for all the other new igt testcases. Forcing libdrm > >> to get updated for igt all the time can get annoying fast. > >> -Daniel > >> > > In this case I'm trying to exercise new API functions in libdrm which > > wrap the GETPARAM ioctl. Would you rather me bypass the wrapper to > > avoid requiring updated libdrm? I can do that, but it fails to test the > > complete path that client would use. > > > Am I missing something, or does 2.4.60 not exist yet? > > That said dependency bumps for igt seem like less of an issue than > dependency bumps for mesa.. I mean if you are using igt you are > probably on the latest anyways.. I'm not sure why Daniel is so > concerned about that.. > > (but dependency bumps to something that doesn't exist yet should > perhaps be avoided) > > BR, > -R > Hi Rob. This igt change is contigent upon my libdrm changes which would in fact bump that version to 2.4.60 after adding an API. That change is also posted and waiting review. I guess I should have stated that dependency here to begin with. http://lists.freedesktop.org/archives/intel-gfx/2015-March/061101.html Jeff > > > -Jeff > > > >> > PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10]) > >> > PKG_CHECK_MODULES(OVERLAY_XVLIB, [xv x11 xext dri2proto >= 2.6], > >> > enable_overlay_xvlib=yes, enable_overlay_xvlib=no) > >> > PKG_CHECK_MODULES(OVERLAY_XLIB, [cairo-xlib dri2proto >= 2.6], > >> > enable_overlay_xlib=yes, enable_overlay_xlib=no) > >> > -- > >> > 2.3.0 > >> > > >> > ___ > >> > Intel-gfx mailing list > >> > Intel-gfx at 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 > >> ___ > >> Beignet mailing list > >> Beignet at lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/beignet > > ___ > > dri-devel mailing list > > dri-devel at lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Beignet] [Intel-gfx] [PATCH i-g-t 2/2] configure: Bump required libdrm version to 2.4.60
On Tue, Mar 10, 2015 at 08:37:30AM +0100, Daniel Vetter wrote: > On Mon, Mar 09, 2015 at 04:41:02PM -0700, jeff.mcgee at intel.com wrote: > > From: Jeff McGee > > > > tests/core_getparams needs the new libdrm interfaces for > > querying subslice and EU counts. > > > > For: VIZ-4636 > > Signed-off-by: Jeff McGee > > --- > > configure.ac | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/configure.ac b/configure.ac > > index 16d6a2e..88a1c3d 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -82,7 +82,7 @@ if test "x$GCC" = "xyes"; then > > fi > > AC_SUBST(ASSEMBLER_WARN_CFLAGS) > > > > -PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.52 libdrm]) > > +PKG_CHECK_MODULES(DRM, [libdrm_intel >= 2.4.60 libdrm]) > > Please don't and instead copypaste the new structs/defines with a local_ > prefix like we do it for all the other new igt testcases. Forcing libdrm > to get updated for igt all the time can get annoying fast. > -Daniel > In this case I'm trying to exercise new API functions in libdrm which wrap the GETPARAM ioctl. Would you rather me bypass the wrapper to avoid requiring updated libdrm? I can do that, but it fails to test the complete path that client would use. -Jeff > > PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10]) > > PKG_CHECK_MODULES(OVERLAY_XVLIB, [xv x11 xext dri2proto >= 2.6], > > enable_overlay_xvlib=yes, enable_overlay_xvlib=no) > > PKG_CHECK_MODULES(OVERLAY_XLIB, [cairo-xlib dri2proto >= 2.6], > > enable_overlay_xlib=yes, enable_overlay_xlib=no) > > -- > > 2.3.0 > > > > ___ > > Intel-gfx mailing list > > Intel-gfx at 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 > ___ > Beignet mailing list > Beignet at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/beignet
[Beignet] [PATCH] drm/i915: Export total subslice and EU counts
On Thu, Mar 05, 2015 at 12:35:55PM +0800, Zhigang Gong wrote:
> There is one minor conflict when apply the KMD patch to latest
> drm-intel-nightly branch. It should be easy to fix.
>
> Another issue is that IMO, we should bump libdrm's version number
> when increase these new APIs. Then in Beignet, we can check the
> libdrm version at build time and determine whether we will use
> these new interfaces. Thus, we can avoid breaking beignet on
> those systems which have previous libdrm/kernel installed.
>
> The other parts of the whole patchset,
> including patches for KMD/libdrm/Intel gpu tools and Beignet,
> all look good to me.
>
> And I just tested it on BDW and SKL platforms, it works fine.
>
Can you add your Reviewed-by tag to at least the Beignet patches?
I think Daniel wants to see that before moving forward with the
rest. Thanks
Jeff
> Thanks,
> Zhigang Gong.
>
> On Mon, Mar 02, 2015 at 03:37:32PM -0800, jeff.mcgee at intel.com wrote:
> > From: Jeff McGee
> >
> > Setup new I915_GETPARAM ioctl entries for subslice total and
> > EU total. Userspace drivers need these values when constructing
> > GPGPU commands. This kernel query method is intended to replace
> > the PCI ID-based tables that userspace drivers currently maintain.
> > The kernel driver can employ fuse register reads as needed to
> > ensure the most accurate determination of GT config attributes.
> > This first became important with Cherryview in which the config
> > could differ between devices with the same PCI ID.
> >
> > The kernel detection of these values is device-specific and not
> > included in this patch. Because zero is not a valid value for any of
> > these parameters, a value of zero is interpreted as unknown for the
> > device. Userspace drivers should continue to maintain ID-based tables
> > for older devices not supported by the new query method.
> >
> > For: VIZ-4636
> > Signed-off-by: Jeff McGee
> > ---
> > drivers/gpu/drm/i915/i915_dma.c | 10 ++
> > include/uapi/drm/i915_drm.h | 2 ++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index 053e178..9350ea2 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -150,6 +150,16 @@ static int i915_getparam(struct drm_device *dev, void
> > *data,
> > case I915_PARAM_MMAP_VERSION:
> > value = 1;
> > break;
> > + case I915_PARAM_SUBSLICE_TOTAL:
> > + value = INTEL_INFO(dev)->subslice_total;
> > + if (!value)
> > + return -ENODEV;
> > + break;
> > + case I915_PARAM_EU_TOTAL:
> > + value = INTEL_INFO(dev)->eu_total;
> > + if (!value)
> > + return -ENODEV;
> > + break;
> > default:
> > DRM_DEBUG("Unknown parameter %d\n", param->param);
> > return -EINVAL;
> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index 6eed16b..8672efc 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -347,6 +347,8 @@ typedef struct drm_i915_irq_wait {
> > #define I915_PARAM_HAS_COHERENT_PHYS_GTT 29
> > #define I915_PARAM_MMAP_VERSION 30
> > #define I915_PARAM_HAS_BSD2 31
> > +#define I915_PARAM_SUBSLICE_TOTAL 32
> > +#define I915_PARAM_EU_TOTAL 33
> >
> > typedef struct drm_i915_getparam {
> > int param;
> > --
> > 2.3.0
> >
> > ___
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
> ___
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
[Beignet] [PATCH] drm/i915: Export total subslice and EU counts
On Thu, Mar 05, 2015 at 12:35:55PM +0800, Zhigang Gong wrote:
> There is one minor conflict when apply the KMD patch to latest
> drm-intel-nightly branch. It should be easy to fix.
>
> Another issue is that IMO, we should bump libdrm's version number
> when increase these new APIs. Then in Beignet, we can check the
> libdrm version at build time and determine whether we will use
> these new interfaces. Thus, we can avoid breaking beignet on
> those systems which have previous libdrm/kernel installed.
>
Right. I can append a libdrm patch to bump the version. And then I
suppose I will follow the process to make a new release. Not sure
right now how that works. First time going through it.
Also, how should we test for the libdrm version and conditionally
use the API? Is there a previous example of this in Beignet that I
could follow?
Jeff
> The other parts of the whole patchset,
> including patches for KMD/libdrm/Intel gpu tools and Beignet,
> all look good to me.
>
> And I just tested it on BDW and SKL platforms, it works fine.
>
> Thanks,
> Zhigang Gong.
>
> On Mon, Mar 02, 2015 at 03:37:32PM -0800, jeff.mcgee at intel.com wrote:
> > From: Jeff McGee
> >
> > Setup new I915_GETPARAM ioctl entries for subslice total and
> > EU total. Userspace drivers need these values when constructing
> > GPGPU commands. This kernel query method is intended to replace
> > the PCI ID-based tables that userspace drivers currently maintain.
> > The kernel driver can employ fuse register reads as needed to
> > ensure the most accurate determination of GT config attributes.
> > This first became important with Cherryview in which the config
> > could differ between devices with the same PCI ID.
> >
> > The kernel detection of these values is device-specific and not
> > included in this patch. Because zero is not a valid value for any of
> > these parameters, a value of zero is interpreted as unknown for the
> > device. Userspace drivers should continue to maintain ID-based tables
> > for older devices not supported by the new query method.
> >
> > For: VIZ-4636
> > Signed-off-by: Jeff McGee
> > ---
> > drivers/gpu/drm/i915/i915_dma.c | 10 ++
> > include/uapi/drm/i915_drm.h | 2 ++
> > 2 files changed, 12 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c
> > b/drivers/gpu/drm/i915/i915_dma.c
> > index 053e178..9350ea2 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -150,6 +150,16 @@ static int i915_getparam(struct drm_device *dev, void
> > *data,
> > case I915_PARAM_MMAP_VERSION:
> > value = 1;
> > break;
> > + case I915_PARAM_SUBSLICE_TOTAL:
> > + value = INTEL_INFO(dev)->subslice_total;
> > + if (!value)
> > + return -ENODEV;
> > + break;
> > + case I915_PARAM_EU_TOTAL:
> > + value = INTEL_INFO(dev)->eu_total;
> > + if (!value)
> > + return -ENODEV;
> > + break;
> > default:
> > DRM_DEBUG("Unknown parameter %d\n", param->param);
> > return -EINVAL;
> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index 6eed16b..8672efc 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -347,6 +347,8 @@ typedef struct drm_i915_irq_wait {
> > #define I915_PARAM_HAS_COHERENT_PHYS_GTT 29
> > #define I915_PARAM_MMAP_VERSION 30
> > #define I915_PARAM_HAS_BSD2 31
> > +#define I915_PARAM_SUBSLICE_TOTAL 32
> > +#define I915_PARAM_EU_TOTAL 33
> >
> > typedef struct drm_i915_getparam {
> > int param;
> > --
> > 2.3.0
> >
> > ___
> > Beignet mailing list
> > Beignet at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/beignet
> ___
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
[PATCH] drm/i915: Export total subslice and EU counts
On Mon, Mar 02, 2015 at 03:37:32PM -0800, jeff.mcgee at intel.com wrote: > From: Jeff McGee > > Setup new I915_GETPARAM ioctl entries for subslice total and > EU total. Userspace drivers need these values when constructing > GPGPU commands. This kernel query method is intended to replace > the PCI ID-based tables that userspace drivers currently maintain. > The kernel driver can employ fuse register reads as needed to > ensure the most accurate determination of GT config attributes. > This first became important with Cherryview in which the config > could differ between devices with the same PCI ID. > > The kernel detection of these values is device-specific and not > included in this patch. Because zero is not a valid value for any of > these parameters, a value of zero is interpreted as unknown for the > device. Userspace drivers should continue to maintain ID-based tables > for older devices not supported by the new query method. > We already have total EU detection support for Cherryview but we need to add detection of total subslice. That support is included in the below-linked series which has been reviewed but not yet merged. http://lists.freedesktop.org/archives/intel-gfx/2015-February/060945.html Jeff
[PATCH] intel: Export GT config attributes
On Fri, Jan 09, 2015 at 10:56:16AM +0800, Zhenyu Wang wrote:
> On 2014.12.18 12:12:33 -0600, jeff.mcgee at intel.com wrote:
> > diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h
> > index 15dd01d..be38adf 100644
> > --- a/include/drm/i915_drm.h
> > +++ b/include/drm/i915_drm.h
> > @@ -340,6 +340,10 @@ typedef struct drm_i915_irq_wait {
> > #define I915_PARAM_HAS_EXEC_HANDLE_LUT 26
> > #define I915_PARAM_HAS_WT 27
> > #define I915_PARAM_CMD_PARSER_VERSION 28
> > +#define I915_PARAM_SLICE_TOTAL 30
> > +#define I915_PARAM_SUBSLICE_TOTAL 31
> > +#define I915_PARAM_EU_TOTAL 32
> > +#define I915_PARAM_THREADS_PER_EU 33
> >
> > typedef struct drm_i915_getparam {
> > int param;
> > diff --git a/intel/intel_bufmgr.h b/intel/intel_bufmgr.h
> > index be83a56..90e535d 100644
> > --- a/intel/intel_bufmgr.h
> > +++ b/intel/intel_bufmgr.h
> > @@ -264,6 +264,11 @@ int drm_intel_get_reset_stats(drm_intel_context *ctx,
> > uint32_t *active,
> > uint32_t *pending);
> >
> > +int drm_intel_get_slice_total(int fd, unsigned int *slice_total);
> > +int drm_intel_get_subslice_total(int fd, unsigned int *subslice_total);
> > +int drm_intel_get_eu_total(int fd, unsigned int *eu_total);
> > +int drm_intel_get_threads_per_eu(int fd, unsigned int *threads_per_eu);
> > +
>
> I think we normally use drm_intel_bufmgr in API instead of fd directly.
> And this config info can be initialized when bufmgr init, so later request
> can get info without calling ioctl everytime.
>
> Also please cc intel-gfx list for any intel related patch.
>
> thanks.
>
The implementation doesn't require a drm_intel_bufmgr instance to
query and return these values, so I think it is best to ask for just
the drm file descriptor from client. I see this approach also in use
by drm_intel_get_aperture_sizes function. Let me know if there is a
strong reason from client perspective to use drm_intel_bufmgr instead
of fd.
I thought about caching these values during bufmgr init, but decided
there was no benefit. Some clients won't need these at all. And those
that do should be calling the query functions once - these values are
unchanging.
Jeff
> > /** @{ Compatibility defines to keep old code building despite the symbol
> > rename
> > * from dri_* to drm_intel_*
> > */
> > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> > index 14e92c9..e0b13e7 100644
> > --- a/intel/intel_bufmgr_gem.c
> > +++ b/intel/intel_bufmgr_gem.c
> > @@ -3293,6 +3293,69 @@ drm_intel_reg_read(drm_intel_bufmgr *bufmgr,
> > return ret;
> > }
> >
> > +drm_public int
> > +drm_intel_get_slice_total(int fd, unsigned int *slice_total)
> > +{
> > + drm_i915_getparam_t gp;
> > + int ret;
> > +
> > + VG_CLEAR(gp);
> > + gp.value = (int*)slice_total;
> > + gp.param = I915_PARAM_SLICE_TOTAL;
> > + ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, );
> > + if (ret)
> > + return -errno;
> > +
> > + return 0;
> > +}
> > +
> > +drm_public int
> > +drm_intel_get_subslice_total(int fd, unsigned int *subslice_total)
> > +{
> > + drm_i915_getparam_t gp;
> > + int ret;
> > +
> > + VG_CLEAR(gp);
> > + gp.value = (int*)subslice_total;
> > + gp.param = I915_PARAM_SUBSLICE_TOTAL;
> > + ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, );
> > + if (ret)
> > + return -errno;
> > +
> > + return 0;
> > +}
> > +
> > +drm_public int
> > +drm_intel_get_eu_total(int fd, unsigned int *eu_total)
> > +{
> > + drm_i915_getparam_t gp;
> > + int ret;
> > +
> > + VG_CLEAR(gp);
> > + gp.value = (int*)eu_total;
> > + gp.param = I915_PARAM_EU_TOTAL;
> > + ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, );
> > + if (ret)
> > + return -errno;
> > +
> > + return 0;
> > +}
> > +
> > +drm_public int
> > +drm_intel_get_threads_per_eu(int fd, unsigned int *threads_per_eu)
> > +{
> > + drm_i915_getparam_t gp;
> > + int ret;
> > +
> > + VG_CLEAR(gp);
> > + gp.value = (int*)threads_per_eu;
> > + gp.param = I915_PARAM_THREADS_PER_EU;
> > + ret = drmIoctl(fd, DRM_IOCTL_I915_GETPARAM, );
> > + if (ret)
> > + return -errno;
> > +
> > + return 0;
> > +}
> >
> > /**
> > * Annotate the given bo for use in aub dumping.
> > --
> > 2.2.0
> >
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Open Source Technology Center, Intel ltd.
>
> $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
[PATCH] intel: Export GT config attributes
Link to the archived drm/i915 patches corresponding to this change: http://lists.freedesktop.org/archives/intel-gfx/2014-December/057817.html
