Re: [PATCH 04/18] drm/i915: add context information to objects

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Mar 28, 2012 at 05:20:11PM -0700, Ben Widawsky wrote:
> On Thu, 29 Mar 2012 00:36:21 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Sun, Mar 18, 2012 at 01:39:44PM -0700, Ben Widawsky wrote:
> > > Handy mostly for assertions.
> > > 
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > 
> > I don't see the point of carrying around a obj->context_id -
> > context_id's are file_priv, so they're not that useful in the
> > tracepoint. I suggest you just drop the contex_id arg there - the obj
> > pointers are global and imo good enough for identification. And the
> > few BUG_ON's don't seem really useful either.
> > -Daniel
> 
> obj->context_id was requested by Chris to clarify the trace events. I
> vaguely recall telling him that you won't like it.

That's easily shot down on the grounds that:
- we currently don't dump the id/handles of gem objects into our traces
- your tracepoints dump the context_id at create/destroy time, so with a
  bit of python this can be re-added in userspace.

> I personally dislike using object pointer though I do agree it serves
> the same purpose.
> 
> The other nice thing about having the context id is it makes it easy in
> debug situations to find out if a certain object is a context object.
> But no use case for that yet.

My gripes with obj->context_id is that it smells like a layering
violation. We don't have such special stuff for other special things like
rings. The only exception is framebuffer when pageflipping, but I expect
that we'll need to clean this up sooner or later (because we need to be
able to move framebuffers sooner or later anyway, so they need better
integration with the gem eviction code).
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48