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@
> >
> > On 25 October 2016 at 00:19, Robert Bragg <[2]robert@sixbynine.org>
> > wrote:
> >
> >
> >
> > > diff --git a/drivers/gpu/drm/i915/i915_
> > b/drivers/gpu/drm/i915/i915_
> > > index 3448d05..ea24814 100644
> > > --- a/drivers/gpu/drm/i915/i915_
> > > +++ b/drivers/gpu/drm/i915/i915_
> > > @@ -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