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