From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 2/2] drm/i915: use semaphores for the display plane Date: Thu, 22 Mar 2012 09:56:07 +0000 Message-ID: References: <1332375553-7174-1-git-send-email-ben@bwidawsk.net> <1332375553-7174-3-git-send-email-ben@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 0E6149E8DC for ; Thu, 22 Mar 2012 02:56:11 -0700 (PDT) In-Reply-To: <1332375553-7174-3-git-send-email-ben@bwidawsk.net> 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: intel-gfx@lists.freedesktop.org Cc: Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org On Wed, 21 Mar 2012 17:19:13 -0700, Ben Widawsky wrote: > In theory this will have performance and power improvements. Performance > because we don't need to stall when the scanout BO is busy, and power > because we don't have to stall when the BO is busy ie. we can get the > work done sooner and put the CPU to sleep (and one less interrupt > required). > > Signed-off-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_gem.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index ce2fee5..96ad162 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3041,9 +3041,7 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, > * any flushes to be pipelined (for pageflips). > * > * For the display plane, we want to be in the GTT but out of any write > - * domains. So in many ways this looks like set_to_gtt_domain() apart from the > - * ability to pipeline the waits, pinning and any additional subtleties > - * that may differentiate the display plane from ordinary buffers. > + * domains. So in many ways this looks like set_to_gtt_domain(). ...apart from the whole pinning and pipelining. It looks less like set-to-gtt-domain over time. Not an improvement. I'll be the first to admit that it is not a great comment, but at least it does try to capture why we don't just treat the display plane as GTT. A better comment would explain our concept of display plane and pipelining. > */ > int > i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > @@ -3058,8 +3056,8 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > return ret; > > if (pipelined != obj->ring) { > - ret = i915_gem_object_wait_rendering(obj); > - if (ret == -ERESTARTSYS) > + ret = i915_gem_object_sync(obj, pipelined); > + if (ret) > return ret; > } This looks eerily familiar. -Chris -- Chris Wilson, Intel Open Source Technology Centre