Received: by 10.213.65.68 with SMTP id h4csp1949976imn; Thu, 5 Apr 2018 06:37:14 -0700 (PDT) X-Google-Smtp-Source: AIpwx4/lIcQf56TIt1+5MNHnTguFrshyoibQVw7qv8UsibL0Y7PrGJkyjLIjaIQwQHzKhhJu2yGX X-Received: by 10.101.99.26 with SMTP id g26mr15103728pgv.442.1522935434056; Thu, 05 Apr 2018 06:37:14 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1522935434; cv=none; d=google.com; s=arc-20160816; b=ESRJQ1RnpLRbJzBhaRBsVAFJJ7wv41Id7P7PH4WPrcls4svLvbp9ghsN2/cKx+X0nQ Rp6/gUY3ZB4EJntUG1BJIne/SQOm9fxgBUamEv14IKpzBz+X5RMSYmSLPx6yPNjvd+Ov Hccra18y32CKv4bB2J70NvKidR6cxWU0PQswHQ6/1Y75PzEfeebkJ5/6KzIGE+q8ezHQ ZNEgmmmNAtK1A+t9+upBtKmmScEqqbrAGWFtxE0vV1IEaSp5CLhNq+tRD3ARAn0Tw9U7 lx1RgyiVD8/dpVi7pZWBHyHPdVAO7KNCpCudmPtqCdVR0W8nptNjmJs4J4K3b6zo0t6n 3pcQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:cc:to:subject:message-id:date:from :references:in-reply-to:mime-version:dkim-signature :arc-authentication-results; bh=kGz363ZadZfGu4N/cOkRLMASOKecfUdPn7h0Qjb6yH4=; b=vyP9AalbkW3tH3Kxds573RafHsaCNp+CL+xWF4rUaPDkAlvyQiriMDPD2Ho0ahBk/S Svaqrwni3otWkeo8fL1jJ9M6OY+PUd5l6KcOUobsCqA3BG1fAUNyFuOsLNAm8ZKbHNvB IxTm2EQGRCDFYzBy3BCcMU9JD4u95FJniGhl6/KCmmF1ZjFLbS4jsTbGq7WvYutGBZUW kxo+W/sJSKYsphOWGALgNarJaGzjL63TxUDlznUgbxgCWtgQonnjtQiehV/+UW4FbZbD 4A6mjoXXeDitbNfBreujPWhi1KoF8+sLhxJJqpJ0ewnz3yqYiC6j6sI8vJZMCL5162q0 2T5w== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@ffwll.ch header.s=google header.b=Bheb9Gqv; 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 g24-v6si8339757plj.40.2018.04.05.06.36.59; Thu, 05 Apr 2018 06:37:14 -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=Bheb9Gqv; 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 S1751470AbeDENfs (ORCPT + 99 others); Thu, 5 Apr 2018 09:35:48 -0400 Received: from mail-io0-f178.google.com ([209.85.223.178]:40175 "EHLO mail-io0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751418AbeDENfp (ORCPT ); Thu, 5 Apr 2018 09:35:45 -0400 Received: by mail-io0-f178.google.com with SMTP id e79so30644487ioi.7 for ; Thu, 05 Apr 2018 06:35:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=kGz363ZadZfGu4N/cOkRLMASOKecfUdPn7h0Qjb6yH4=; b=Bheb9GqvWn25CsJc2phFv/6Y+5uU59y4tSuSGfTDskBZKVt43r0ix4a74TDUoGvoj9 bYKaJn06vVS768tPUr/wL0TAJ69rFg1rH1DUsIvhClHwzXfEC4m8V/qLkfJMNmtQ8ovb ak4gJkP2qjXbI+/H0nvBipR1n5dawJQTWkOaw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=kGz363ZadZfGu4N/cOkRLMASOKecfUdPn7h0Qjb6yH4=; b=SM0kVfirfEDcW/f6iUUGEknJWPieBV3Xf6iwGUvHZYN+WEt8kXUYqRkzp86KXT+Wf+ xe011KlXBSXOuAyObRn4s+gyDqgEtJgoJBH6WgnRi+N7YYD/OeDJAaKEf33YSLGQu19O wqP7p+0fu8uf7KC5fJyg6sAFAYBXHTPbun7LDBO5OBWXUMl2g72f9DQoMXEEtD4/4niN H/2dNa2yz/uaGnbohF4hPU/lqXbK694hRyaG/fL6LDq5Ecqc7U7/LLOdliIgMPxIMe0h Z37d+YWaRd0LLHbnmkZkH6eIemLtVs3Kqalysly/Sjeh1/a/RBSMmFHs2dLIaymMRm8Z glkA== X-Gm-Message-State: AElRT7FOZ96x9h5aDH7sJtnkEkrXMGLz7yZMtiNh/dc9uONnL9kiA5DT n6NGxmFz+/9AM3/hfa7b/Z0hpT3UsdHys9qcNKNVug== X-Received: by 10.107.179.134 with SMTP id c128mr20448934iof.278.1522935339941; Thu, 05 Apr 2018 06:35:39 -0700 (PDT) MIME-Version: 1.0 Received: by 10.79.40.129 with HTTP; Thu, 5 Apr 2018 06:35:39 -0700 (PDT) X-Originating-IP: [212.51.149.109] In-Reply-To: <7616bf87-bd2e-5003-af35-bc95f0e2a912@shipmail.org> References: <20180403224225.26776-1-robdclark@gmail.com> <307adb15-412a-1fe2-f116-27fe5b4a657d@shipmail.org> <20180404084320.GA3881@phenom.ffwll.local> <20180404095604.GD3881@phenom.ffwll.local> <7616bf87-bd2e-5003-af35-bc95f0e2a912@shipmail.org> From: Daniel Vetter Date: Thu, 5 Apr 2018 15:35:39 +0200 X-Google-Sender-Auth: cb9a9weuXnIS6iEwQxeNXKWteYU Message-ID: Subject: Re: [RFC] drm/atomic+msm: add helper to implement legacy dirtyfb To: Thomas Hellstrom Cc: Rob Clark , =?UTF-8?Q?Noralf_Tr=C3=B8nnes?= , dri-devel , freedreno , linux-arm-msm , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , Lukasz Spintzyk , Deepak Singh Rawat , Gustavo Padovan , Maarten Lankhorst , Sean Paul , David Airlie , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Apr 5, 2018 at 3:30 PM, Thomas Hellstrom wrote: > On 04/04/2018 11: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. >> >>> 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. >> > > FWIW, vmwgfx has always treated a dirtyfb as a pure front-buffer like > rendering operation without any synchronization, > We've guaranteed that only the rects that are present are uploaded, but only > xf86-video-vmware has taken advantage of this to keep > CPU- and GPU rendered content apart. > I think we've at some point run into problems with number of cliprects, (Old > KDE lock screen?) and use multiple dirtyfb for the same > update... Ok, if we can hit this in practices then I think it's ok to have the limit. Just need to make sure userspace actually condenses the cliprects down to something within the limit, since with atomic flips you can't just do multiple updates - that would tear badly. Wrt not syncing: I think general use pretty clearly says lots of userspace renders into buffers with gpus (not even necessarily your own) and then expects dirtyfb or a flip to upload all the bits. Otherwise Rob Clark wouldn't need his patch. Given that I think we need to make general semantics follow that requirement - I don't expect it'll harm vmwgfx since it doesn't render into the frontbuffer anyway? > IIRC the reason for working with the fb, is that it's much easier for > user-space, which doesn't have to care where planes are scanning out and > why. > "Present my new content on any screen that's affected". Yeah, dirtyfb makes tons of sense for frontbuffer-rendering X, which also doesn't do per-scanout pixmaps. But for atomic flips you really want to flip on the crtc (or well plane), since otherwise with multiple planes it comes up all teared up. vmwgfx doesn't care I guess since you only have 1 primary plane, but all the SoC chips very much do. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch