Re: [Intel-gfx] [PATCH] drm/i915: Add i915 perf infrastructure

From: Robert Bragg <robert@sixbynine.org>
To: Matthew Auld <matthew.william.auld@gmail.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>,
	Sourab Gupta <sourab.gupta@intel.com>,
	ML dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Add i915 perf infrastructure
Date: Tue, 11 Oct 2016 12:03:10 -0700	[thread overview]
Message-ID: <CAMou1-3uOz5aJn+2mS0TbEnUYoNUVxwm_AqQ3k5+Kqc-D5LfJA@mail.gmail.com> (raw)
In-Reply-To: <CAM0jSHOw-J2q+35hOSv_ACkACxWOmn+yB+QmMweCoXgH=rOO6Q@mail.gmail.com>

[-- Attachment #1.1: Type: text/plain, Size: 2387 bytes --]

On Fri, Oct 7, 2016 at 10:10 AM, Matthew Auld <
matthew.william.auld@gmail.com> wrote:

> On 14 September 2016 at 16:32, Robert Bragg <robert@sixbynine.org> wrote:
>
> > +
> > +int i915_perf_open_ioctl_locked(struct drm_device *dev,
> > +                               struct drm_i915_perf_open_param *param,
> > +                               struct perf_open_properties *props,
> > +                               struct drm_file *file)
> > +{
> This should be static and also let's just make it take dev_priv directly.
>

Ah, yep, done.

> > +               case DRM_I915_PERF_PROP_MAX:
> > +                       BUG();
> We already handle this case above, but I guess we still need this in
> order to silence gcc...
>

right, and preferable to having a default: case, for the future compiler
warning to handle any new properties here.

> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index 03725fe..77fe79b 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -258,6 +258,7 @@ typedef struct _drm_i915_sarea {
> >  #define DRM_I915_GEM_USERPTR           0x33
> >  #define DRM_I915_GEM_CONTEXT_GETPARAM  0x34
> >  #define DRM_I915_GEM_CONTEXT_SETPARAM  0x35
> > +#define DRM_I915_PERF_OPEN             0x36
> >
> >  #define DRM_IOCTL_I915_INIT            DRM_IOW( DRM_COMMAND_BASE +
> DRM_I915_INIT, drm_i915_init_t)
> >  #define DRM_IOCTL_I915_FLUSH           DRM_IO ( DRM_COMMAND_BASE +
> DRM_I915_FLUSH)
> > @@ -311,6 +312,7 @@ typedef struct _drm_i915_sarea {
> >  #define DRM_IOCTL_I915_GEM_USERPTR                     DRM_IOWR
> (DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR, struct drm_i915_gem_userptr)
> >  #define DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM    DRM_IOWR
> (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_GETPARAM, struct
> drm_i915_gem_context_param)
> >  #define DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM    DRM_IOWR
> (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_SETPARAM, struct
> drm_i915_gem_context_param)
> > +#define DRM_IOCTL_I915_PERF_OPEN       DRM_IOR(DRM_COMMAND_BASE +
> DRM_I915_PERF_OPEN, struct drm_i915_perf_open_param)
> As you already pointed out this will need to be IOW.
>

Yeah, changed locally after I realised the mistake here, just didn't get
around to posting the patch.

Also applied the notes to not redundantly init some vars, spurious new
line, redundant include.

Thanks,
- Robert

[-- Attachment #1.2: Type: text/html, Size: 3575 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel