From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 4/5] drm/radeon: rework page flip handling Date: Mon, 5 May 2014 16:23:29 +0200 Message-ID: <20140505142329.GP20800@phenom.ffwll.local> References: <1398781773-2319-1-git-send-email-deathsimple@vodafone.de> <1398781773-2319-4-git-send-email-deathsimple@vodafone.de> <53634887.4000907@daenzer.net> <53639DB1.5060703@vodafone.de> <53675975.1060809@daenzer.net> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-ee0-f44.google.com (mail-ee0-f44.google.com [74.125.83.44]) by gabe.freedesktop.org (Postfix) with ESMTP id 650BF6E197 for ; Mon, 5 May 2014 07:23:34 -0700 (PDT) Received: by mail-ee0-f44.google.com with SMTP id c41so5410004eek.17 for ; Mon, 05 May 2014 07:23:33 -0700 (PDT) Content-Disposition: inline In-Reply-To: <53675975.1060809@daenzer.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Michel =?iso-8859-1?Q?D=E4nzer?= Cc: dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Mon, May 05, 2014 at 06:27:17PM +0900, Michel D=E4nzer wrote: > On 02.05.2014 22:29, Christian K=F6nig wrote: > > Am 02.05.2014 09:25, schrieb Michel D=E4nzer: > >> On 29.04.2014 23:29, Christian K=F6nig wrote: > >>> > >>> +static void radeon_flip_work_func(struct work_struct *__work) > >>> { > >> [...] > >>> + if (radeon_crtc->flip_work) { > >>> + DRM_DEBUG_DRIVER("flip queue: crtc already busy\n"); > >>> + spin_unlock_irqrestore(&crtc->dev->event_lock, flags); > >>> + goto pflip_cleanup1; > >>> + } > >> I'm a little worried about this case. AFAICT this would drop the flip = if > >> a previous one is still pending? I'm not sure current userspace can > >> actually hit this > > Yeah, that concerned me as well. The old code dropped the the new flip > > as well, so I'm pretty sure that the new handling is right and userspace > > won't hit that. > > = > > The only difference to the old code is that I've offloaded it to a > > separate thread and so can't return -EBUSY any more. > = > That's an important difference though: Userspace can react to the -EBUSY > appropriately, but if flips get dropped silently, bad things will > happen, such as the wrong buffer being scanned out. > = > Keep in mind that we don't control all userspace, e.g. I expect there > will be an increasing number of Wayland compositors using this > functionality. Rules on i915 are that if the pageflip confirmation event hasn't been sent out yet the driver returns -EBUSY. The locking for that is done using irq-save spinlocks, so should be doable to check this synchronously before latching some kinda of async worker. I didnt read one line of radeon code to check this though ;-) Cheers, Daniel -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch