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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Oct 26, 2016 at 04:17:45PM +0100, Robert Bragg wrote:
> On 26 Oct 2016 9:54 a.m., "Chris Wilson" <chris@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@gmail.com> wrote:
> > >
> > >      On 25 October 2016 at 00:19, Robert Bragg <[2]robert@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;

OT:
What kind of MUA are you using that mangles quoted mails like this? I've
not seen it on intel-gfx before. mesa-dev seems rife with it, but as I
rarely read that in any great detail I've managed to ignore it there.
Anyways, it makes it espesially hard to navigate long mails since mutt's
'S' (skip quoted text) no longer works correctly.

> > >      > +
> > >      > +                       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

> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel