[PATCH v7 06/11] drm/i915: Enable i915 perf stream for Haswell OA unit

Robert Bragg robert at sixbynine.org
Wed Oct 26 15:17:45 UTC 2016


On 26 Oct 2016 9:54 a.m., "Chris Wilson" <chris at chris-wilson.co.uk> wrote:
>
> On Wed, Oct 26, 2016 at 12:51:58AM +0100, Robert Bragg wrote:
> >    On Tue, Oct 25, 2016 at 10:35 PM, Matthew Auld
> >    <[1]matthew.william.auld at gmail.com> wrote:
> >
> >      On 25 October 2016 at 00:19, Robert Bragg <[2]robert at sixbynine.org>
> >      wrote:
> >
> >
> >
> >      > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >      b/drivers/gpu/drm/i915/i915_drv.h
> >      > index 3448d05..ea24814 100644
> >      > --- a/drivers/gpu/drm/i915/i915_drv.h
> >      > +++ b/drivers/gpu/drm/i915/i915_drv.h
> >      > @@ -1764,6 +1764,11 @@ struct intel_wm_config {
> >
> >      >
> >      >  struct drm_i915_private {
> >      > @@ -2149,16 +2164,46 @@ struct drm_i915_private {
> >      >
> >      >         struct {
> >      >                 bool initialized;
> >      > +
> >      >                 struct mutex lock;
> >      >                 struct list_head streams;
> >      >
> >      > +               spinlock_t hook_lock;
> >      > +
> >      >                 struct {
> >      > -                       u32 metrics_set;
> >      > +                       struct i915_perf_stream
*exclusive_stream;
> >      > +
> >      > +                       u32 specific_ctx_id;
> >      Can we just get rid of this, now that the vma remains pinned we can
> >      simply get the ggtt address at the time of configuring the
OA_CONTROL
> >      register ?
> >
> >    I considered that, but would ideally prefer to keep it considering
the
> >    gen8+ patches to come. For gen8+ (with execlists) the context ID
isn't a
> >    gtt offset.
>
> In terms of symmetry, keeping the vma you pinned and unpinning the same
> later makes its ownership much clearer. (And I do want the owner of each
> pin to be clear, for when we start enabling debug to catch the VMA
> leaks.)

Keeping our own pointer to the pinned vma could be a clarification.

Considering Matt's comments too, I'm thinking I'll put the pinning and
specific_ctx_id initialization together with setting stream->ctx, keeping
the state together under the stream. It's going to potentially mean
redundantly pinning the ctx for the sake of the ID in the future for
streams that don't really need it, but I think it's probably not worth
worrying about that.

- Robert

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20161026/74d1f24a/attachment.html>


More information about the dri-devel mailing list