From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v3 2/4] drm/i915: Wait for pending flips in intel_pipe_set_base() Date: Mon, 3 Dec 2012 18:42:33 +0200 Message-ID: <20121203164233.GI21547@intel.com> References: <1354041298-18898-1-git-send-email-ville.syrjala@linux.intel.com> <1354041298-18898-3-git-send-email-ville.syrjala@linux.intel.com> <20121128205118.GB3202@phenom.ffwll.local> <20121130140120.GG21547@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id 1C2A7E5C77 for ; Mon, 3 Dec 2012 08:42:37 -0800 (PST) Content-Disposition: inline In-Reply-To: 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 Fri, Nov 30, 2012 at 04:44:04PM +0100, Daniel Vetter wrote: > On Fri, Nov 30, 2012 at 3:01 PM, Ville Syrj=E4l=E4 > wrote: > > I had another look at this, and I think you're aiming for something oth= er > > than what this patch is trying to do. > = > Oh, I know that I'm trying to volunteer you to do a bit more ... it's > why I'm the bastard maintainer ;-) > = > > The thing is that we are holding struct_mutex while waiting for pending > > flips here, so we just need to get out asap to allow the reset work do > > its thing. > = > Hm, I didn't notice that we're holding the struct_mutex there, so I > guess we need to indeed bail out. Or rework the finsh_fb vs. gpu hang > logic to not require the struct_mutex in finish_fb ... > = > > Completing pending page flips when a reset occurs is separate issue. > > This code never did any of that, and I don't see why it should. We > > would need to complete them anyway regardless of whether anyone is > > currently waiting for them. > = > Yeah, that's my idea, but I haven't though through the details (see my > ignorance with locking details above ...). > = > > Perhaps we can just call intel_finish_page_flip() from the reset > > code and call it a day. I suppose doing that could end up unpinning > > the current scanout buffer in case the display registers were never > > written with the new values. One solution for that would be to always > > do a set_base() after a reset. That would make sure we end up showing > > the latest buffer at least, although the contents could be total > > garbage. > = > Hm, I think first we should aim to no longer hang either the kernel or > leave stuck userspace (since the drm_event never shows up) behind. Atm > a gpu hang is allowed to corrupt pretty much everything. Later on, > when we successfully avoid the hung kernel/userspace problems we can > worry about making it look decent. Judging by how long it took to get > basic reset working somewhat okish, it'll take a while. And we need > some form of testcase to exercise the code. At least the current patch would avoid having the process stuck in D state. So I think it's better than nothing. I do have other things on my plate, so I don't want to spend any more time on this currently. -- = Ville Syrj=E4l=E4 Intel OTC