From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [RFC][PATCH 4/4] drm: i915: Atomic pageflip WIP Date: Fri, 14 Sep 2012 15:27:05 +0100 Message-ID: <84c8a8$5o4ac7@orsmga001.jf.intel.com> References: <1347464827-14187-1-git-send-email-ville.syrjala@linux.intel.com> <1347464827-14187-5-git-send-email-ville.syrjala@linux.intel.com> <84c8a8$5o3vcu@orsmga001.jf.intel.com> <20120914142130.GE19732@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0280933347==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id E06AC9F307 for ; Fri, 14 Sep 2012 07:27:21 -0700 (PDT) In-Reply-To: <20120914142130.GE19732@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0280933347== Content-Type: text/plain On Fri, 14 Sep 2012 17:21:30 +0300, Ville Syrjälä wrote: > On Fri, Sep 14, 2012 at 02:57:26PM +0100, Chris Wilson wrote: > > On Wed, 12 Sep 2012 18:47:07 +0300, ville.syrjala@linux.intel.com wrote: > > > +static void intel_flip_finish(struct drm_flip *flip) > > > +{ > > > + struct intel_flip *intel_flip = > > > + container_of(flip, struct intel_flip, base); > > > + struct drm_device *dev = intel_flip->crtc->dev; > > > + > > > + if (intel_flip->old_bo) { > > > + mutex_lock(&dev->struct_mutex); > > > + > > > + intel_finish_fb(intel_flip->old_bo); > > > > So if I understand correctly, this code is called after the flip is > > already complete? > > Yes. > > > The intel_finish_fb() exists to flush pending batches and flips on the > > current fb, prior to changing the scanout registers. (There is a > > hardware dependency such that the GPU may be executing a command that > > required the current modesetting.) In the case of flip completion, all > > of those dependencies have already been retired and so the finish should > > be a no-op. And so it should no be required, nor the changes to > > intel_finish_fb (which should have included a change in the name to > > indicate that is now taking the fb_obj). > > Actually I'm not quite sure where this intel_finish_fb() call originated. > Based on the name it didn't make sense to me, but I left it there for > now. Hmm. OK it came from one patch from Imre while I was on vacation. > I suppose he got it from intel_pipe_set_base() which does call > intel_finish_fb() on the old fb. Why does it do that? It all boils down to the modeset being asynchronous to the GPU processing the command stream. So we may be currently processing a batch that is waiting on the pipe to go past a particular scanline, and if the modesetting were to disable that pipe, or to change its size, then we risk the WAIT_FOR_EVENT never completing - leading to hangcheck detecting the frozen display and an angry user. The other aspect is to synchronize the modeset with any outstanding pageflips. -Chris -- Chris Wilson, Intel Open Source Technology Centre --===============0280933347== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel --===============0280933347==--