From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH 1/7] drm/i915: Mark context switch likely Date: Fri, 5 Apr 2013 09:44:54 -0700 Message-ID: <20130405164453.GA23599@bwidawsk.net> References: <1365118914-15753-1-git-send-email-ben@bwidawsk.net> <1365118914-15753-2-git-send-email-ben@bwidawsk.net> <87d2u9qwqh.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from shiva.localdomain (209-20-75-48.static.cloud-ips.com [209.20.75.48]) by gabe.freedesktop.org (Postfix) with ESMTP id C3CAAE66AA for ; Fri, 5 Apr 2013 09:42:36 -0700 (PDT) Content-Disposition: inline In-Reply-To: <87d2u9qwqh.fsf@intel.com> 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: Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Apr 05, 2013 at 10:09:58AM +0300, Jani Nikula wrote: > On Fri, 05 Apr 2013, Ben Widawsky wrote: > > Only the very first switch doesn't take the path. > > > > Signed-off-by: Ben Widawsky > > --- > > drivers/gpu/drm/i915/i915_gem_context.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > > index 94d873a..aa080ea 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_context.c > > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > > @@ -390,7 +390,7 @@ static int do_switch(struct i915_hw_context *to) > > * is a bit suboptimal because the retiring can occur simply after the > > * MI_SET_CONTEXT instead of when the next seqno has completed. > > */ > > - if (from_obj != NULL) { > > + if (likely(from_obj)) { > > Looking at the function, is this, uh, likely to make a difference? > Probably not. > Same goes for the unlikely in patches 4/7. (Yes, patch_es_ 4/7 - there's > *two* patches 4/7 in the series! :o) I don't see two, weird. > > I'm just generally wary of adding (un)likely annotations. I don't think > it matters that the annotation itself is obviously correct; IMHO the > performance impact should matter. > > Right. I actually stuck it in by accident, and decided to keep it only because it serves as nice documentation. I suppose a comment would do the same thing, but this also had a chance (albeit slight) to improve things. > > BR, > Jani. > > > > from_obj->base.read_domains = I915_GEM_DOMAIN_INSTRUCTION; > > i915_gem_object_move_to_active(from_obj, ring); > > /* As long as MI_SET_CONTEXT is serializing, ie. it flushes the > > -- > > 1.8.2 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ben Widawsky, Intel Open Source Technology Center