From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v4 3/5] drm/i915: Make sprite updates atomic Date: Thu, 13 Feb 2014 18:43:51 +0200 Message-ID: <20140213164351.GE3852@intel.com> References: <1392306174-9148-1-git-send-email-ville.syrjala@linux.intel.com> <1392306174-9148-4-git-send-email-ville.syrjala@linux.intel.com> <20140213160152.GC29357@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 314B5FB20B for ; Thu, 13 Feb 2014 08:44:05 -0800 (PST) Content-Disposition: inline In-Reply-To: <20140213160152.GC29357@nuc-i3427.alporthouse.com> 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: Chris Wilson , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Feb 13, 2014 at 04:01:52PM +0000, Chris Wilson wrote: > On Thu, Feb 13, 2014 at 05:42:52PM +0200, ville.syrjala@linux.intel.com w= rote: > > From: Ville Syrj=E4l=E4 > > = > > Add a mechanism by which we can evade the leading edge of vblank. This > > guarantees that no two sprite register writes will straddle on either > > side of the vblank start, and that means all the writes will be latched > > together in one atomic operation. > > = > > We do the vblank evade by checking the scanline counter, and if it's too > > close to the start of vblank (too close has been hardcoded to 100usec > > for now), we will wait for the vblank start to pass. In order to > > eliminate random delayes from the rest of the system, we operate with > > interrupts disabled, except when waiting for the vblank obviously. > > = > > Note that we now go digging through pipe_to_crtc_mapping[] in the > > vblank interrupt handler, which is a bit dangerous since we set up > > interrupts before the crtcs. However in this case since it's the vblank > > interrupt, we don't actually unmask it until some piece of code > > requests it. > > = > > v2: preempt_check_resched() calls after local_irq_enable() (Jesse) > > Hook up the vblank irq stuff on BDW as well > > v3: Pass intel_crtc instead of drm_crtc (Daniel) > > Warn if crtc.mutex isn't locked (Daniel) > > Add an explicit compiler barrier and document the barriers (Daniel) > > Note the irq vs. modeset setup madness in the commit message (Danie= l) > > v4: Use prepare_to_wait() & co. directly and eliminate vbl_received > = > intel_pipe_update_start / intel_pipe_update_end are unbalanced (end() > does too much in the cases where start() failed.) Ah the drm_vblank_get() fail. Should never happen. But I guess I could make intel_pipe_update_start() return something to tell the caller whether it needs to call intel_pipe_update_end(). > = > intel_pipe_update_start should check for min <=3D 0 (i.e. > usecs_to_scanline() returns a value greater than vblank_start). Mode w/ vertical active portion of < 100 usec. Sounds rather unlikely, but I suppose I could add a check for that too while I'm at it. > = > intel_pipe_update_end() could do a sanity check that it is in the same > frame as start() and so give us warning when the code is broken. I think I had something like that a long time, but it vanished along the way. I'll add it. Getting a clear indication that things went south is better than just some silent glitch on the screen. On gen2 we don't have a frame counter but since it always returns 0, I don't even need to add a special case. We just can't detect the failure on gen2. > = > Looks like drm_handle_vblank() could be moved to > intel_pipe_handle_vblank() for a small refactoring win. Hmm, yeah. I just need to make intel_pipe_handle_vblank() propagate the return value from drm_handle_vblank(). Seems cleaner, consider it done. -- = Ville Syrj=E4l=E4 Intel OTC