From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 04/18] drm/i915: add context information to objects Date: Thu, 29 Mar 2012 08:57:32 -0700 Message-ID: <20120329085732.4c64a4d8@bwidawsk.net> References: <1332103198-25852-1-git-send-email-ben@bwidawsk.net> <1332103198-25852-5-git-send-email-ben@bwidawsk.net> <20120328223621.GG2046@phenom.ffwll.local> <20120328172011.02f41d0e@bwidawsk.net> <20120329084756.GB4106@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id F2A1E9E79D for ; Thu, 29 Mar 2012 08:57:42 -0700 (PDT) In-Reply-To: <20120329084756.GB4106@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, 29 Mar 2012 10:47:56 +0200 Daniel Vetter wrote: > On Wed, Mar 28, 2012 at 05:20:11PM -0700, Ben Widawsky wrote: > > On Thu, 29 Mar 2012 00:36:21 +0200 > > Daniel Vetter wrote: > > > > > On Sun, Mar 18, 2012 at 01:39:44PM -0700, Ben Widawsky wrote: > > > > Handy mostly for assertions. > > > > > > > > Signed-off-by: Ben Widawsky > > > > > > 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. Yes, but gem handles are not specific to i915, whereas context id numbers are. Also the tracepoint that is a problem is switch, not create/destroy > > > 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 I still think the pros (easy debug) outweigh the cons (not really on exactly what you don't like). I don't see any harm that can come from this. /me braces for harm Anyway, as you said in the other email, I'm expecting more serious feedback. If I end up redoing a bunch of stuff, this can go too if you really don't like it.