Re: [PATCH 01/15] drm/i915: Add ctx getparam ioctl parameter to retrieve ctx unique id

From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel@ffwll.ch>,
	Deepak <deepak.s@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 01/15] drm/i915: Add ctx getparam ioctl parameter to retrieve ctx unique id
Date: Thu, 28 Jul 2016 11:37:49 +0200	[thread overview]
Message-ID: <20160728093659.GA6232@phenom.ffwll.local> (raw)
In-Reply-To: <20160727105036.GF20724@nuc-i3427.alporthouse.com>

On Wed, Jul 27, 2016 at 11:50:36AM +0100, Chris Wilson wrote:
> On Wed, Jul 27, 2016 at 12:19:00PM +0200, Daniel Vetter wrote:
> > On Wed, Jul 27, 2016 at 02:48:38PM +0530, Deepak wrote:
> > > 
> > > 
> > > 
> > > On 06/02/2016 10:48 AM, sourab.gupta@intel.com wrote:
> > > > From: Sourab Gupta <sourab.gupta@intel.com>
> > > > 
> > > > This patch adds a new ctx getparam ioctl parameter, which can be used to
> > > > retrieve ctx unique id by userspace.
> > > > 
> > > > This can be used by userspace to map the i915 perf samples with their
> > > > particular ctx's, since those would be having ctx unique id's.
> > > > Otherwise the userspace has no way of maintaining this association,
> > > > since it has the knowledge of only per-drm file specific ctx handles.
> > > > 
> > > > Signed-off-by: Sourab Gupta <sourab.gupta@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/i915/i915_gem_context.c | 3 +++
> > > >   include/uapi/drm/i915_drm.h             | 1 +
> > > >   2 files changed, 4 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > > > index e974451..09f5178 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > > > @@ -1001,6 +1001,9 @@ int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
> > > >   		else
> > > >   			args->value = to_i915(dev)->ggtt.base.total;
> > > >   		break;
> > > > +	case I915_CONTEXT_PARAM_HW_ID:
> > > > +		args->value = ctx->hw_id;
> > > > +		break;
> > > >   	default:
> > > >   		ret = -EINVAL;
> > > >   		break;
> > > > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > > > index 4a1bcfd8..0badc16 100644
> > > > --- a/include/uapi/drm/i915_drm.h
> > > > +++ b/include/uapi/drm/i915_drm.h
> > > > @@ -1171,6 +1171,7 @@ struct drm_i915_gem_context_param {
> > > >   #define I915_CONTEXT_PARAM_BAN_PERIOD	0x1
> > > >   #define I915_CONTEXT_PARAM_NO_ZEROMAP	0x2
> > > >   #define I915_CONTEXT_PARAM_GTT_SIZE	0x3
> > > > +#define I915_CONTEXT_PARAM_HW_ID	0x4
> > > >   	__u64 value;
> > > >   };
> > > Patch looks good to me
> > > 
> > > Reviewed-by: Deepak S <deepak.s@linux.intel.com>
> > 
> > ctx->hw_id gets recycled without any involvement from userspace. How
> > exactly is this supposed to work?
> 
> ctx->hw_id is invariant for the lifetime of the context (and so we limit
> the number of live contents to meet hw constraints, ~1 million).

Argh, I was tricked by the comment in assign_hw_id - I thought it's
talking about the hw_id only, not about the entire ctx.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx