From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 01/11] drm/i915: Accurately track when we mark the hardware as idle/busy Date: Wed, 5 Mar 2014 14:13:15 +0100 Message-ID: <20140305131315.GG17001@phenom.ffwll.local> References: <1393001548-2883-1-git-send-email-przanoni@gmail.com> <1393001548-2883-2-git-send-email-przanoni@gmail.com> <20140221165557.GF17020@nuc-i3427.alporthouse.com> <20140221172734.GG17020@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f178.google.com (mail-ea0-f178.google.com [209.85.215.178]) by gabe.freedesktop.org (Postfix) with ESMTP id DE9B0FA68A for ; Wed, 5 Mar 2014 05:13:20 -0800 (PST) Received: by mail-ea0-f178.google.com with SMTP id a15so1015938eae.23 for ; Wed, 05 Mar 2014 05:13:20 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Paulo Zanoni Cc: Intel Graphics Development , Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Fri, Feb 21, 2014 at 04:34:29PM -0300, Paulo Zanoni wrote: > 2014-02-21 14:27 GMT-03:00 Chris Wilson : > > On Fri, Feb 21, 2014 at 02:04:32PM -0300, Paulo Zanoni wrote: > >> 2014-02-21 13:55 GMT-03:00 Chris Wilson : > >> > On Fri, Feb 21, 2014 at 01:52:18PM -0300, Paulo Zanoni wrote: > >> >> From: Chris Wilson > >> >> > >> >> We currently call intel_mark_idle() too often, as we do so as a > >> >> side-effect of processing the request queue. However, we the calls to > >> >> intel_mark_idle() are expected to be paired with a call to > >> >> intel_mark_busy() (or else we try to idle the hardware by accessing > >> >> registers that are already disabled). Make the idle/busy tracking > >> >> explicit to prevent the multiple calls. > >> >> > >> >> v2: From Paulo > >> >> - Make it compile > >> >> - Drop the __i915_add_request chunk > >> >> > >> >> Reported-by: Paulo Zanoni > >> >> Tested-by: Paulo Zanoni > >> >> Signed-off-by: Chris Wilson > >> >> Cc: Paulo Zanoni > >> >> Signed-off-by: Paulo Zanoni > >> >> --- > >> >> drivers/gpu/drm/i915/i915_drv.h | 8 ++++++++ > >> >> drivers/gpu/drm/i915/intel_display.c | 9 +++++++++ > >> >> 2 files changed, 17 insertions(+) > >> >> > >> >> > >> >> Chris did not reply to my review comments yet, so I just went and implemented > >> >> them. We need at least an ACK form him here before merging. > >> > > >> > Didn't see them... Why have you altered the logic? > >> > >> See the comment at the __i915_add_request chunk: > >> > >> http://lists.freedesktop.org/archives/intel-gfx/2014-February/040334.html > > > > Oh, I didn't look for comments inline. > >> > >> Maybe I just broke your patch :) > >> If my review doesn't make sense, we can stick to your version, it > >> should do the job, and I can retest everything easily. > > > > If there was a pending work item, the call to intel_mark_busy() would > > return false. So we can revamp the logic around there a little bit. The > > reason for the change should be self-evident - the previous code lost its > > way in the transition to multiple rings arguing over a global property > > Just to avoid any possible confusions when/if we merge this series: > Chris sent a new version of this patch on the original mail thread. Just to double check: Have I merged the right version? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch