Re: [PATCH i-g-t] lib/chipset: Cache devid

[Daniel Vetter <daniel@ffwll.ch>]

On Tue, Feb 10, 2015 at 10:50:33PM +0000, Chris Wilson wrote:
> On Tue, Feb 10, 2015 at 11:45:12PM +0100, Daniel Vetter wrote:
> > On Tue, Feb 10, 2015 at 11:39:45PM +0100, Daniel Vetter wrote:
> > > On Tue, Feb 10, 2015 at 11:37:42PM +0100, Daniel Vetter wrote:
> > > > On Tue, Feb 10, 2015 at 10:28:22PM +0000, Chris Wilson wrote:
> > > > > On Tue, Feb 10, 2015 at 10:59:16PM +0100, Daniel Vetter wrote:
> > > > > > Chris Wilson complained that this adds a lot of noise to the test
> > > > > > startup when full debugging is enabled, so let's cache it. We can do
> > > > > > that since there's only ever one intel gpu in a given system.
> > > > > > 
> > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > > > 
> > > > > Couldn't we move the devid cache to lib/drmtest.c::is_intel() ?
> > > > 
> > > > Sounds like just another place where we should use the helper from
> > > > intel_chipset.c. Next patch in-flight ...
> > > 
> > > Ok I'm blind, is_intel can fail. So I guess I should extract a new
> > > __get_drm_devid which can fail, put the caching in there (plus override)
> > > and use that in in intel_chipset.c ...
> > 
> > Doesn't really work since doing the ioctl is part of the dance we do to
> > figure out whether the fd is really an intel or not :(
> 
> Something like:

Yeah, lgtm.
-Daniel

> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 7cdef36..4090a4a 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -72,6 +72,8 @@
>   * and [batchbuffer](intel-gpu-tools-intel-batchbuffer.html) libraries as dependencies.
>   */
>  
> +uint16_t __drm_device_id;
> +
>  static int is_i915_device(int fd)
>  {
>         drm_version_t version;
> @@ -100,7 +102,11 @@ is_intel(int fd)
>         if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)))
>                 return 0;
>  
> -       return IS_INTEL(devid);
> +       if (!IS_INTEL(devid))
> +               return 0;
> +
> +       __drm_device_id = devid;
> +       return 1;
>  }
>  
>  static void check_stop_rings(void)
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 508cc83..fabf43e 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -70,6 +70,8 @@ static inline void *igt_mmap64(void *addr, size_t length, int prot, int flags,
>   */
>  #define ALIGN(v, a) (((v) + (a)-1) & ~((a)-1))
>  
> +extern uint16_t __drm_device_id;
> +
>  int drm_get_card(void);
>  int __drm_open_any(void);
>  int drm_open_any(void);
> diff --git a/lib/intel_chipset.c b/lib/intel_chipset.c
> index fafd232..33177c6 100644
> --- a/lib/intel_chipset.c
> +++ b/lib/intel_chipset.c
> @@ -125,26 +125,15 @@ intel_get_pci_device(void)
>  uint32_t
>  intel_get_drm_devid(int fd)
>  {
> -       uint32_t devid = 0;
>         const char *override;
>  
> -       override = getenv("INTEL_DEVID_OVERRIDE");
> -       if (override) {
> -               devid = strtod(override, NULL);
> -       } else {
> -               struct drm_i915_getparam gp;
> -               int ret;
> -
> -               memset(&gp, 0, sizeof(gp));
> -               gp.param = I915_PARAM_CHIPSET_ID;
> -               gp.value = (int *)&devid;
> -
> -               ret = ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp));
> -               igt_assert(ret == 0);
> -               errno = 0;
> -       }
> +       igt_assert(__drm_device_id);
>  
> -       return devid;
> +       override = getenv("INTEL_DEVID_OVERRIDE");
> +       if (override)
> +               return strtod(override, NULL);
> +       else
> +               return __drm_device_id;
>  }
>  
>  /**
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx