From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/2] drm/i915: time out of load detect busy-waits Date: Tue, 1 May 2012 19:26:58 +0200 Message-ID: <20120501172658.GA4832@phenom.ffwll.local> References: <1334948616-16073-1-git-send-email-daniel.vetter@ffwll.ch> <1335374112_49610@CP5-2952> <20120425192650.GG5079@phenom.ffwll.local> <1335646872_81002@CP5-2952> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f171.google.com (mail-wi0-f171.google.com [209.85.212.171]) by gabe.freedesktop.org (Postfix) with ESMTP id 99AD39EB56 for ; Tue, 1 May 2012 10:26:03 -0700 (PDT) Received: by wibhj13 with SMTP id hj13so2906326wib.12 for ; Tue, 01 May 2012 10:26:02 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1335646872_81002@CP5-2952> 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: Chris Wilson Cc: Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Sat, Apr 28, 2012 at 10:00:29PM +0100, Chris Wilson wrote: > On Wed, 25 Apr 2012 21:26:50 +0200, Daniel Vetter wrote: > > On Wed, Apr 25, 2012 at 06:14:37PM +0100, Chris Wilson wrote: > > > On Fri, 20 Apr 2012 21:03:35 +0200, Daniel Vetter wrote: > > > > If we try to do that and the scanlines just wouldn't advance, we > > > > busy-hang the machine holding the modeset mutex. Not great for > > > > debugging. > > > > > > > > References: https://bugzilla.kernel.org/show_bug.cgi?id=43020 > > > > Signed-off-by: Daniel Vetter > > > > > > Reviewer hangs head in shame: > > > > > > > + if (wait_for(I915_READ(pipe_dsl_reg) >= vactive, 1000)) > > > > + DRM_ERROR("timed out waiting for vactive in " > > > > + "load_detect, scanline: %u\n", > > > > + I915_READ(pipe_dsl_reg)); > > > > + if (wait_for((dsl = I915_READ(pipe_dsl_reg)) <= vsample, 1000)) > > > > + DRM_ERROR("timed out waiting for vsample in " > > > > + "load_detect, scanline: %u\n", > > > > + I915_READ(pipe_dsl_reg)); > > > > > > wait_for() catches us out everytime we convert and existing while(), > > > because the predicate is when it stops. Perhaps if we had a wait_until, > > > but anyway the fix here is: > > > > > > if (wait_for(I915_READ(pipe_dsl_reg) < vactive, 1000)) > > > ... > > > if (wait_for((dsl = I915_READ(pipe_dsl_reg)) > vsample, 1000)) > > > ... > > dinq rectified, it never happened. Thanks for catching this. > > wait_for() has even more subtleties in store for us, the unwary coder. > By default, it uses a 1ms sleep between polling the register, chosen to > be kind whilst waiting for panel bits to power up which do take a fair > amount of time. Here, that extra delay causes us to sample the vsync > rather than the border. The quirk of the [vh]sync is that the monitor bit > of ST00 is always true. And since we always seem to pick that row to read > we always think there is a CRT present. > > The choice is either to use the busy-polling variant, wait_for_atomic, > or restructure the entire block to use a single timeout with direct > reads. And whilst you are modifying the code, convert the polling reads > to I915_READ_NOTRACE(). Thanks a lot for digging into this, I've only managed to do the bisect before I've haeded off into the w/e. I've dropped this patch for now, too much fail in it. The underlying issue of not properly doing load-detect on a active but disabled crtc is fixed, so this isn't required any more. I'll look a this again later, hopefully clue strikes me by then. -Daniel -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48