Received: by 10.213.65.68 with SMTP id h4csp664162imn; Wed, 4 Apr 2018 05:18:11 -0700 (PDT) X-Google-Smtp-Source: AIpwx4+vqJkHmCUd+rsGICQsVpfy/N4wxVGdJ+BiiLKIFC0vgPVSi1sVYo9/Rti3H52reumL2o7r X-Received: by 10.101.102.24 with SMTP id w24mr11952124pgv.326.1522844291899; Wed, 04 Apr 2018 05:18:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1522844291; cv=none; d=google.com; s=arc-20160816; b=ws1EtcZiICyBpfroX2ami0FDmAGnb+5p8TZZB0r2sYWlwYgdXbjufPG+A36gVIN9pA q8+yUiDqNYDXimlxGvVyuGMmmIis6vR4y4Zd5Y1HlmLxrvJ5qaFnK4de89CybJE4BAvi HswdBhWqtG/HGVepM3L9h5lHovTc3q5XkmL6N+UXBmdlqKkWQQVRTqvHqRPdLxSuD+9t TDeTr5Fp2+HBYyZ+RIWKl6YyazXUZCi93hh78gOv3qNizNjgMWAmvySDdqmgQkPFK0m2 Jrr9/YWFVbTMxubPT4mtl2qXduYdhQKSq48Chtcg3Hci3S7Mh5nLps75LFnxjLz6jY8D NLgA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:mail-followup-to :message-id:subject:cc:to:from:date:dkim-signature :arc-authentication-results; bh=WHwLJePkD9dp9ZnJv0fn+Ix5IT/oiz75jTzepLCOBsA=; b=NNsul0AmgqMhL4iK2qqu7MJdXfj5GcI4U3CbERFE4XyplICyTLuuwGfdleD7L+YJHR cTDatGIwz9sqRyyxpPuNv4ve9qF7rAqgZX6pMlKAUlflYPK5nVEqG8sSgNFUZkMIRnF6 /hrH1wzGuLWpQwhPCMpqZZwHQ+y4Qh56hA21xa4IwQDDF2HaR+TOG7GQioCtqtRp0V14 zy6ABPvmy6ZSw3eWeOV1x/hSWGy5mXk0/kj4qlTcLICFqaQLzmtfCFTAy4akQLWTf0mq zjZdhj0zT+BebUHW0YFh/7a++vpapT6D0fEIdivcR/Ix7p1JgmAdCIE2PaZlmlC+TZz4 MHqw== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@ffwll.ch header.s=google header.b=EdK0x3FT; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id o69si4000374pfi.322.2018.04.04.05.17.57; Wed, 04 Apr 2018 05:18:11 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=fail header.i=@ffwll.ch header.s=google header.b=EdK0x3FT; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751403AbeDDMQY (ORCPT + 99 others); Wed, 4 Apr 2018 08:16:24 -0400 Received: from mail-wm0-f66.google.com ([74.125.82.66]:38730 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751306AbeDDMQW (ORCPT ); Wed, 4 Apr 2018 08:16:22 -0400 Received: by mail-wm0-f66.google.com with SMTP id i3so18123779wmf.3 for ; Wed, 04 Apr 2018 05:16:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=sender:date:from:to:cc:subject:message-id:mail-followup-to :references:mime-version:content-disposition:in-reply-to:user-agent; bh=WHwLJePkD9dp9ZnJv0fn+Ix5IT/oiz75jTzepLCOBsA=; b=EdK0x3FTSHB7rVXN7q6Ybly1wA82P9zhYr5TlsV8uB992K7pJgkn6wQVwOdpUlmLgr fYXeseMD7cpl1yXfhJWZo71QqnrfPs9FEOzG6k8G9jlSOy3RzGP5uMrAPf6O8lJcxxhC 22X9hij0RNQ9FD3zKYOPhKYmAkOWJChlptBE8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to:user-agent; bh=WHwLJePkD9dp9ZnJv0fn+Ix5IT/oiz75jTzepLCOBsA=; b=M/VVYQvr/Y/xy2R339m3JO9i4LARvZTZ7Jes/k5SZRUz6gfZW0E3HQSkh5lZE+zbJS +oScI4KB3hktYF8bRONIMDFEN+FzpSSXHk/Xo21yF2P48qu1f38uJ0CDTrCceEWTbnLo z0SBAZLsRpNrij9qBNioM4bC1jrEmnekR1ISeuryLsUleAsmJrtvpWmDxJDDy2FSnD8l /7Kpysvxt88awxdHCFhT4LSwTSFZ6LhKnPPnnYtSInjGH45+WODEsYzgRBUShCbwZguu ZOrnDK41wS+b5lVHyg1X9bbfBWAx6DTAQUkUdWdAJCWnhinPczbZ6qKsFItG47F3BLHa kFPw== X-Gm-Message-State: AElRT7EsDyR3Zabh4O+zVVJuCxvVkmfWdRCdsWMx7/wlubEJDOXGGetV OHJVruQV6E+5W2e0qd+z45cveg== X-Received: by 10.80.165.66 with SMTP id z2mr20411588edb.300.1522844180833; Wed, 04 Apr 2018 05:16:20 -0700 (PDT) Received: from phenom.ffwll.local (212-51-149-109.fiber7.init7.net. [212.51.149.109]) by smtp.gmail.com with ESMTPSA id n30sm3481608edd.45.2018.04.04.05.16.19 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 04 Apr 2018 05:16:19 -0700 (PDT) Date: Wed, 4 Apr 2018 14:16:17 +0200 From: Daniel Vetter To: Rob Clark Cc: Thomas Hellstrom , Noralf =?iso-8859-1?Q?Tr=F8nnes?= , dri-devel , freedreno , linux-arm-msm , Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , Lukasz Spintzyk , Deepak Singh Rawat , Gustavo Padovan , Maarten Lankhorst , Sean Paul , David Airlie , Linux Kernel Mailing List Subject: Re: [RFC] drm/atomic+msm: add helper to implement legacy dirtyfb Message-ID: <20180404121617.GK3881@phenom.ffwll.local> Mail-Followup-To: Rob Clark , Thomas Hellstrom , Noralf =?iso-8859-1?Q?Tr=F8nnes?= , dri-devel , freedreno , linux-arm-msm , Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , Lukasz Spintzyk , Deepak Singh Rawat , Gustavo Padovan , Maarten Lankhorst , Sean Paul , David Airlie , Linux Kernel Mailing List References: <20180403224225.26776-1-robdclark@gmail.com> <307adb15-412a-1fe2-f116-27fe5b4a657d@shipmail.org> <20180404084320.GA3881@phenom.ffwll.local> <20180404095604.GD3881@phenom.ffwll.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux phenom 4.15.0-1-amd64 User-Agent: Mutt/1.9.4 (2018-02-28) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Apr 04, 2018 at 07:40:32AM -0400, Rob Clark wrote: > On Wed, Apr 4, 2018 at 5:56 AM, Daniel Vetter wrote: > > On Wed, Apr 04, 2018 at 11:10:08AM +0200, Thomas Hellstrom wrote: > >> On 04/04/2018 10:43 AM, Daniel Vetter wrote: > >> > On Wed, Apr 04, 2018 at 10:22:21AM +0200, Thomas Hellstrom wrote: > >> > > Hi, > >> > > > >> > > On 04/04/2018 08:58 AM, Daniel Vetter wrote: > >> > > > On Wed, Apr 4, 2018 at 12:42 AM, Rob Clark wrote: > >> > > > > Add an atomic helper to implement dirtyfb support. This is needed to > >> > > > > support DSI command-mode panels with x11 userspace (ie. when we can't > >> > > > > rely on pageflips to trigger a flush to the panel). > >> > > > > > >> > > > > To signal to the driver that the async atomic update needs to > >> > > > > synchronize with fences, even though the fb didn't change, the > >> > > > > drm_atomic_state::dirty flag is added. > >> > > > > > >> > > > > Signed-off-by: Rob Clark > >> > > > > --- > >> > > > > Background: there are a number of different folks working on getting > >> > > > > upstream kernel working on various different phones/tablets with qcom > >> > > > > SoC's.. many of them have command mode panels, so we kind of need a > >> > > > > way to support the legacy dirtyfb ioctl for x11 support. > >> > > > > > >> > > > > I know there is work on a proprer non-legacy atomic property for > >> > > > > userspace to communicate dirty-rect(s) to the kernel, so this can > >> > > > > be improved from triggering a full-frame flush once that is in > >> > > > > place. But we kinda needa a stop-gap solution. > >> > > > > > >> > > > > I had considered an in-driver solution for this, but things get a > >> > > > > bit tricky if userspace ands up combining dirtyfb ioctls with page- > >> > > > > flips, because we need to synchronize setting various CTL.FLUSH bits > >> > > > > with setting the CTL.START bit. (ie. really all we need to do for > >> > > > > cmd mode panels is bang CTL.START, but is this ends up racing with > >> > > > > pageflips setting FLUSH bits, then bad things.) The easiest soln > >> > > > > is to wrap this up as an atomic commit and rely on the worker to > >> > > > > serialize things. Hence adding an atomic dirtyfb helper. > >> > > > > > >> > > > > I guess at least the helper, with some small addition to translate > >> > > > > and pass-thru the dirty rect(s) is useful to the final atomic dirty- > >> > > > > rect property solution. Depending on how far off that is, a stop- > >> > > > > gap solution could be useful. > >> > > > Adding Noralf, who iirc already posted the full dirty helpers already somewhere. > >> > > > -Daniel > >> > > I've asked Deepak to RFC the core changes suggested for the full dirty blob > >> > > on dri-devel. It builds on DisplayLink's suggestion, with a simple helper to > >> > > get to the desired coordinates. > >> > > > >> > > One thing to perhaps discuss is how we would like to fit this with > >> > > front-buffer rendering and the dirty ioctl. In the page-flip context, the > >> > > dirty rects, like egl's swapbuffer_with_damage is a hint to restrict the > >> > > damage region that can be fully ignored by the driver, new content is > >> > > indicated by a new framebuffer. > >> > > > >> > > We could do the same for frontbuffer rendering: Either set a dirty flag like > >> > > you do here, or provide a content_age state member. Since we clear the dirty > >> > > flag on state copies, I guess that would be sufficient. The blob rectangles > >> > > would then become a hint to restrict the damage region. > >> > I'm not entirely following here - I thought for frontbuffer rendering the > >> > dirty rects have always just been a hint, and that the driver was always > >> > free to re-upload the entire buffer to the screen. > >> > > >> > And through a helper like Rob's proposing here (and have floated around in > >> > different versions already) we'd essentially map a frontbuffer dirtyfb > >> > call to a fake flip with dirty rect. Manual upload drivers already need to > >> > upload the entire screen if they get a flip, since some userspace uses > >> > that to flush out frontbuffer rendering (instead of calling dirtyfb). > >> > > >> > So from that pov the new dirty flag is kinda not necessary imo. > >> > > >> > > Another approach would be to have the presence of dirty rects without > >> > > framebuffer change to indicate frontbuffer rendering. > >> > > > >> > > I think I like the first approach best, although it may be tempting for > >> > > user-space apps to just set the dirty bit instead of providing the full > >> > > damage region. > >> > Or I'm not following you here, because I don't quite see the difference > >> > between dirtyfb and a flip. > >> > -Daniel > >> > >> OK, let me rephrase: > >> > >> From the driver's point-of-view, in the atomic world, new content and the > >> need for manual upload is indicated by a change in fb attached to the plane. > >> > >> With Rob's patch here, (correct me if I'm wrong) in addition new content and > >> the need for manual upload is identified by the dirty flag, (since the fb > >> stays the same and the driver thus never identifies a page-flip). > > > > Hm, I'm not entirely sure Rob's approach is correct. Imo a pageflip > > (atomic or not) should result in the entire buffer getting uploaded. The > > dirty flag is kinda redundant, a flip with the same buffer works the same > > way as a dirtyfb with the entire buffer as the dirty rectangle. > > Userspace could do a pageflip, but (with buffer-age extension) only > re-render part of the frame. So there is a use-case for full frame > pageflip with hints about what region(s) changed since previous frame. > > (Of course userspace could screw that up and get different results on > "command" vs "video" style display.. in that case, it gets to keep > both pieces) I'm not against dirty on flips with the same buffer as an optimization. Disagreement here is around whether a flip to the same buffer means: a) nothing changed, upload nothing b) entire buffer might have changed I'm proposing we standardize on b) (with maybe the exception of legacy cursor ioctls because those are special) and add the dirty rects as an option to optimize this more. Either way you'd always have to follow the implicit fence (which is what your msm hunk seems to be doing, but only for dirtyfb). -Daniel > > BR, > -R > > > >> In both these situations, clip rects can provide a hint to restrict the > >> dirty region. > >> > >> Now I was thinking about the preferred way for user-space to communicate > >> front buffer rendering through the atomic ioctl: > >> > >> 1) Expose a dirty (or content_age property) > >> 2) Attach a clip-rect blob property. > >> 3) Fake a page-flip by ping-ponging two frame-buffers pointing to the same > >> underlying buffer object. > >> > >> Are you saying that people are already using 3) and we should keep using > >> that? > > > > I'm saying they're using 3b), flip the same buffer wrapped in the same > > drm_framebuffer, and expect it to work. > > > > The only advantage dirtyfb has is that it allows you to supply the > > optimized upload rectangles, but at the cost of a funny api (it's working > > on the fb, not the plane/crtc you want to upload) and lack of drm_event to > > confirm when exactly you uploaded your stuff. But imo they should be the > > same underlying operation. > > > > Also note that atomic helpers don't optimize out plane flips for same > > buffers. We only optimize out some of the waiting, in a failed attempt at > > making cursors stall less, but that's not fixed with the async plane > > update stuff. And we can obviously optimize out the prepare/cleanup hooks, > > because the buffer should be pinned already. > > > > So imo for a quick fix I think we need: > > 1) Fix drivers (or at least msm) to upload buffers for manual upload on > > all flips (well, all screen updates). Probably should do the fence waiting > > unconditionally too (and handle cursors with the new async stuff). > > 2) Tiny helper that remaps a dirtyfb call into a nonblocking atomic > > commit, which currently means we're throwing the dirty rect optimization > > into the wind. But that could easily be added to the drm_plane_state, > > without exposing it to userspace as a property just yet. > > > > Cheers, Daniel > > -- > > Daniel Vetter > > Software Engineer, Intel Corporation > > http://blog.ffwll.ch > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch