Message ID | 20240116045159.1015510-1-andrealmeid@igalia.com |
---|---|
Headers |
Return-Path: <linux-kernel+bounces-26960-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:7300:42cf:b0:101:a8e8:374 with SMTP id q15csp51183dye; Mon, 15 Jan 2024 20:52:51 -0800 (PST) X-Google-Smtp-Source: AGHT+IGHHwFUJPAzNMBQXlsblLMCFYWk8HeGHf/ARNd7uciwBWIlM15kMF12NAYzDrYdpY4vXiJj X-Received: by 2002:a17:906:c10c:b0:a26:cee8:3713 with SMTP id do12-20020a170906c10c00b00a26cee83713mr3595996ejc.55.1705380771083; Mon, 15 Jan 2024 20:52:51 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1705380771; cv=none; d=google.com; s=arc-20160816; b=j1wAQkBewKrUEgiaRbpNwxyk1qnEgsm/xu3HLJe/zte5sTDkT8HNuu7tI6drSmqEOv mTXQcEt46jXPjOmc/3tPlrcWPyz3plUh7sWvsAMTWqA8Ekq4x2ijJp7QvzD95r2MmTxC fERbBy9LNl9hO2BMutaIe6wWBHzFZBQ//t61UDzJD+RIGSkSrKByhUWQQ0YjMtruf/lc kn/MX0FccDNwODKtQJa9ht769kfeq08iqCX8Xy5d8UGetwyp50HHtaSaq+abWFYS6azb /XVLmkaT48msbLQAkpioYNZkN1Udzh5aVcjzozOtvK6ybrFmhhxKAz+vStln83EezbVE zLmQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:ime-version:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:cc:to :from:dkim-signature; bh=lClxhiG7kPciwzOgT7N9lpSqHzzXmh68CO/DdbLniM4=; fh=WFNpP7Q9IqVMu6ZfYNblOj+2MIUEddpCWX628c47/2c=; b=lU8OOH5v2cYcFQ+S/EJRTe3v624yjoIRRCe0PIo/D9kF0cacXVOK21YEuS3l/3bAwm Uh4jSvnx0VUXkT450xP3HI4WlTN9i8mRPrWXTfKt394Q1XwDg9K0W/dLGpbE37hmi+Eu OCe05FN9aA8o6cgzWXoupCPD/Ce0N+sD4JjsFC8Rrco/RC9/Lv+IBFMuYo125m3fyWNl dhnMDSZ8tiw3FRhvxpn6tKDFY7E7d3FTjQzzKSeMtakb5ATwl+tZzkXjSYBCE4fHMWEv UkdHgDM21wlj1APWR7vdDnTq6dQzjXmfJFeGhXOKDGUpg6umifydldtP+WbB7nnaR84X VuIA== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@igalia.com header.s=20170329 header.b=WCWJvECM; spf=pass (google.com: domain of linux-kernel+bounces-26960-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-26960-ouuuleilei=gmail.com@vger.kernel.org" Received: from am.mirrors.kernel.org (am.mirrors.kernel.org. [147.75.80.249]) by mx.google.com with ESMTPS id a24-20020a17090640d800b00a2de58581e6si1364246ejk.926.2024.01.15.20.52.50 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 15 Jan 2024 20:52:51 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-26960-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249; Authentication-Results: mx.google.com; dkim=fail header.i=@igalia.com header.s=20170329 header.b=WCWJvECM; spf=pass (google.com: domain of linux-kernel+bounces-26960-ouuuleilei=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) smtp.mailfrom="linux-kernel+bounces-26960-ouuuleilei=gmail.com@vger.kernel.org" Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by am.mirrors.kernel.org (Postfix) with ESMTPS id ADE051F2384B for <ouuuleilei@gmail.com>; Tue, 16 Jan 2024 04:52:50 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id C327C1171F; Tue, 16 Jan 2024 04:52:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=igalia.com header.i=@igalia.com header.b="WCWJvECM" Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 482EE11181 for <linux-kernel@vger.kernel.org>; Tue, 16 Jan 2024 04:52:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=igalia.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=igalia.com DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:MIME-Version:Message-ID: Date:Subject:Cc:To:From:Sender:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=lClxhiG7kPciwzOgT7N9lpSqHzzXmh68CO/DdbLniM4=; b=WCWJvECMzP18zlfiqAgmLtYK9B 7FoSNN0Rx8W4NA4lp7OfrC+VCLJ6jNC7axeCVcvVI1qyTym5ARO2W/mYSCyEWBtctnhwpQ5h2lGqe 9cwx0Tr1wfwEp18q6n97bdVDNcrDdbTTOkyYQK88Fi1QKCMPNixb4lm8rdU6S/GoNhoy3KZcWheZQ YgD6jiSygxm2HKWkR+ReauFXYb5d9yPiVSGTo1lsbDRB0Il3LngupnHkt84qozAN31stOezZsjP7V oVRJBdKAmb8NySiMftR+adkjM1ZKOu7MDpM7TbsH6nH798+luyXOrn6esqueBgtdPDMmF3Ojh0Ns0 mW7wAUwQ==; Received: from [177.45.63.147] (helo=steammachine.lan) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim) id 1rPbQn-006r0R-Be; Tue, 16 Jan 2024 05:52:09 +0100 From: =?utf-8?q?Andr=C3=A9_Almeida?= <andrealmeid@igalia.com> To: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org Cc: kernel-dev@igalia.com, alexander.deucher@amd.com, christian.koenig@amd.com, Simon Ser <contact@emersion.fr>, Pekka Paalanen <ppaalanen@gmail.com>, daniel@ffwll.ch, Daniel Stone <daniel@fooishbar.org>, =?utf-8?b?J01hcmVrIE9sxaHDoWsn?= <maraeo@gmail.com>, Dave Airlie <airlied@gmail.com>, ville.syrjala@linux.intel.com, Xaver Hugl <xaver.hugl@gmail.com>, =?utf-8?q?Andr=C3=A9_Almeida?= <andrealmeid@igalia.com> Subject: [PATCH 0/2] drm/atomic: Allow drivers to write their own plane check for async Date: Tue, 16 Jan 2024 01:51:57 -0300 Message-ID: <20240116045159.1015510-1-andrealmeid@igalia.com> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 IME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1788221347389685233 X-GMAIL-MSGID: 1788221347389685233 |
Series |
drm/atomic: Allow drivers to write their own plane check for async
|
|
Message
André Almeida
Jan. 16, 2024, 4:51 a.m. UTC
Hi, AMD hardware can do more on the async flip path than just the primary plane, so to lift up the current restrictions, this patchset allows drivers to write their own check for planes for async flips. I'm not sure if adding something new to drm_plane_funcs is the right way to do, because if we want to expand the other object types (crtc, connector) we would need to add their own drm_XXX_funcs, so feedbacks are welcome! André André Almeida (2): drm/atomic: Allow drivers to write their own plane check for async flips drm/amdgpu: Implement check_async_props for planes .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 30 +++++++++ drivers/gpu/drm/drm_atomic_uapi.c | 62 ++++++++++++++----- include/drm/drm_atomic_uapi.h | 12 ++++ include/drm/drm_plane.h | 5 ++ 4 files changed, 92 insertions(+), 17 deletions(-)
Comments
On Tue, 16 Jan 2024 01:51:57 -0300 André Almeida <andrealmeid@igalia.com> wrote: > Hi, > > AMD hardware can do more on the async flip path than just the primary plane, so > to lift up the current restrictions, this patchset allows drivers to write their > own check for planes for async flips. Hi, what's the userspace story for this, how could userspace know it could do more? What kind of userspace would take advantage of this and in what situations? Or is this not meant for generic userspace? Thanks, pq > I'm not sure if adding something new to drm_plane_funcs is the right way to do, > because if we want to expand the other object types (crtc, connector) we would > need to add their own drm_XXX_funcs, so feedbacks are welcome! > > André > > André Almeida (2): > drm/atomic: Allow drivers to write their own plane check for async > flips > drm/amdgpu: Implement check_async_props for planes > > .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 30 +++++++++ > drivers/gpu/drm/drm_atomic_uapi.c | 62 ++++++++++++++----- > include/drm/drm_atomic_uapi.h | 12 ++++ > include/drm/drm_plane.h | 5 ++ > 4 files changed, 92 insertions(+), 17 deletions(-) >
Hi Pekka, Em 16/01/2024 06:45, Pekka Paalanen escreveu: > On Tue, 16 Jan 2024 01:51:57 -0300 > André Almeida <andrealmeid@igalia.com> wrote: > >> Hi, >> >> AMD hardware can do more on the async flip path than just the primary plane, so >> to lift up the current restrictions, this patchset allows drivers to write their >> own check for planes for async flips. > > Hi, > > what's the userspace story for this, how could userspace know it could do more? > What kind of userspace would take advantage of this and in what situations? > > Or is this not meant for generic userspace? Sorry, I forgot to document this. So the idea is that userspace will query what they can do here with DRM_MODE_ATOMIC_TEST_ONLY calls, instead of having capabilities for each prop. > > > Thanks, > pq > >> I'm not sure if adding something new to drm_plane_funcs is the right way to do, >> because if we want to expand the other object types (crtc, connector) we would >> need to add their own drm_XXX_funcs, so feedbacks are welcome! >> >> André >> >> André Almeida (2): >> drm/atomic: Allow drivers to write their own plane check for async >> flips >> drm/amdgpu: Implement check_async_props for planes >> >> .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 30 +++++++++ >> drivers/gpu/drm/drm_atomic_uapi.c | 62 ++++++++++++++----- >> include/drm/drm_atomic_uapi.h | 12 ++++ >> include/drm/drm_plane.h | 5 ++ >> 4 files changed, 92 insertions(+), 17 deletions(-) >> >
On Tue, 16 Jan 2024 08:50:59 -0300 André Almeida <andrealmeid@igalia.com> wrote: > Hi Pekka, > > Em 16/01/2024 06:45, Pekka Paalanen escreveu: > > On Tue, 16 Jan 2024 01:51:57 -0300 > > André Almeida <andrealmeid@igalia.com> wrote: > > > >> Hi, > >> > >> AMD hardware can do more on the async flip path than just the primary plane, so > >> to lift up the current restrictions, this patchset allows drivers to write their > >> own check for planes for async flips. > > > > Hi, > > > > what's the userspace story for this, how could userspace know it could do more? > > What kind of userspace would take advantage of this and in what situations? > > > > Or is this not meant for generic userspace? > > Sorry, I forgot to document this. So the idea is that userspace will > query what they can do here with DRM_MODE_ATOMIC_TEST_ONLY calls, > instead of having capabilities for each prop. That's the theory, but do you have a practical example? What other planes and props would one want change in some specific use case? Is it just "all or nothing", or would there be room to choose and pick which props you change and which you don't based on what the driver supports? If the latter, then relying on TEST_ONLY might be yet another combinatorial explosion to iterate through. Thanks, pq > >> I'm not sure if adding something new to drm_plane_funcs is the right way to do, > >> because if we want to expand the other object types (crtc, connector) we would > >> need to add their own drm_XXX_funcs, so feedbacks are welcome! > >> > >> André > >> > >> André Almeida (2): > >> drm/atomic: Allow drivers to write their own plane check for async > >> flips > >> drm/amdgpu: Implement check_async_props for planes > >> > >> .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 30 +++++++++ > >> drivers/gpu/drm/drm_atomic_uapi.c | 62 ++++++++++++++----- > >> include/drm/drm_atomic_uapi.h | 12 ++++ > >> include/drm/drm_plane.h | 5 ++ > >> 4 files changed, 92 insertions(+), 17 deletions(-) > >> > >
+ Joshua Em 16/01/2024 10:14, Pekka Paalanen escreveu: > On Tue, 16 Jan 2024 08:50:59 -0300 > André Almeida <andrealmeid@igalia.com> wrote: > >> Hi Pekka, >> >> Em 16/01/2024 06:45, Pekka Paalanen escreveu: >>> On Tue, 16 Jan 2024 01:51:57 -0300 >>> André Almeida <andrealmeid@igalia.com> wrote: >>> >>>> Hi, >>>> >>>> AMD hardware can do more on the async flip path than just the primary plane, so >>>> to lift up the current restrictions, this patchset allows drivers to write their >>>> own check for planes for async flips. >>> >>> Hi, >>> >>> what's the userspace story for this, how could userspace know it could do more? >>> What kind of userspace would take advantage of this and in what situations? >>> >>> Or is this not meant for generic userspace? >> >> Sorry, I forgot to document this. So the idea is that userspace will >> query what they can do here with DRM_MODE_ATOMIC_TEST_ONLY calls, >> instead of having capabilities for each prop. > > That's the theory, but do you have a practical example? > > What other planes and props would one want change in some specific use > case? > > Is it just "all or nothing", or would there be room to choose and pick > which props you change and which you don't based on what the driver > supports? If the latter, then relying on TEST_ONLY might be yet another > combinatorial explosion to iterate through. > That's a good question, maybe Simon, Xaver or Joshua can share how they were planning to use this on Gamescope or Kwin. > > Thanks, > pq > >>>> I'm not sure if adding something new to drm_plane_funcs is the right way to do, >>>> because if we want to expand the other object types (crtc, connector) we would >>>> need to add their own drm_XXX_funcs, so feedbacks are welcome! >>>> >>>> André >>>> >>>> André Almeida (2): >>>> drm/atomic: Allow drivers to write their own plane check for async >>>> flips >>>> drm/amdgpu: Implement check_async_props for planes >>>> >>>> .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 30 +++++++++ >>>> drivers/gpu/drm/drm_atomic_uapi.c | 62 ++++++++++++++----- >>>> include/drm/drm_atomic_uapi.h | 12 ++++ >>>> include/drm/drm_plane.h | 5 ++ >>>> 4 files changed, 92 insertions(+), 17 deletions(-) >>>> >>> >
On Tue, 16 Jan 2024 17:10:18 +0100 Xaver Hugl <xaver.hugl@gmail.com> wrote: > My plan is to require support for IN_FENCE_FD at least. If the driver > doesn't > allow tearing with that, then tearing just doesn't happen. That's an excellent point. I think this is important enough in its own right, that it should be called out in the patch series. Is it important enough to be special-cased, e.g. to be always allowed with async commits? Now that I think of it, if userspace needs to wait for the in-fence itself before kicking KMS async, that would defeat much of the async's point, right? And cases where in-fence is not necessary are so rare they might not even exist? So if driver/hardware cannot do IN_FENCE_FD with async, is there any use of supporting async to begin with? > For overlay planes though, it depends on how the compositor prioritizes > things. > If the compositor prioritizes overlay planes and would like to do tearing > if possible, > then this patch works. Ok, I can see that. > If the compositor prioritizes tearing and would like to do overlay planes > if possible, > it would have to know that switching to synchronous commits for a single > frame, > setting up the overlay planes and then switching back to async commits > works, and > that can't be figured out with TEST_ONLY commits. I had to ponder a bit why. So I guess the synchronous commit in between is because driver/hardware may not be able to enable/disable extra planes in async, so you need a synchronous commit to set them up, but afterwards updates can tear. The comment about Intel needing one more synchronous commit when switching from sync to async updates comes to mind as well, would that be a problem? > So I think having a CAP or immutable plane property to signal that async > commits > with overlay and/or cursor planes is supported would be useful. Async cursor planes a good point, particularly moving them around. I'm not too informed about the prior/on-going efforts to allow cursor movement more often than refresh rate, I recall something about amending atomic commits? How would these interact? I suppose the kernel still prevents any new async commit while a previous commit is not finished, so amending commits would still be necessary for cursor plane motion? Or would it, if you time "big commits" to finish quickly and then spam async "cursor commits" in the mean time? Thanks, pq > Am Di., 16. Jan. 2024 um 14:35 Uhr schrieb André Almeida < > andrealmeid@igalia.com>: > > > + Joshua > > > > Em 16/01/2024 10:14, Pekka Paalanen escreveu: > > > On Tue, 16 Jan 2024 08:50:59 -0300 > > > André Almeida <andrealmeid@igalia.com> wrote: > > > > > >> Hi Pekka, > > >> > > >> Em 16/01/2024 06:45, Pekka Paalanen escreveu: > > >>> On Tue, 16 Jan 2024 01:51:57 -0300 > > >>> André Almeida <andrealmeid@igalia.com> wrote: > > >>> > > >>>> Hi, > > >>>> > > >>>> AMD hardware can do more on the async flip path than just the primary > > plane, so > > >>>> to lift up the current restrictions, this patchset allows drivers to > > write their > > >>>> own check for planes for async flips. > > >>> > > >>> Hi, > > >>> > > >>> what's the userspace story for this, how could userspace know it could > > do more? > > >>> What kind of userspace would take advantage of this and in what > > situations? > > >>> > > >>> Or is this not meant for generic userspace? > > >> > > >> Sorry, I forgot to document this. So the idea is that userspace will > > >> query what they can do here with DRM_MODE_ATOMIC_TEST_ONLY calls, > > >> instead of having capabilities for each prop. > > > > > > That's the theory, but do you have a practical example? > > > > > > What other planes and props would one want change in some specific use > > > case? > > > > > > Is it just "all or nothing", or would there be room to choose and pick > > > which props you change and which you don't based on what the driver > > > supports? If the latter, then relying on TEST_ONLY might be yet another > > > combinatorial explosion to iterate through. > > > > > > > That's a good question, maybe Simon, Xaver or Joshua can share how they > > were planning to use this on Gamescope or Kwin. > > > > > > > > Thanks, > > > pq > > > > > >>>> I'm not sure if adding something new to drm_plane_funcs is the right > > way to do, > > >>>> because if we want to expand the other object types (crtc, connector) > > we would > > >>>> need to add their own drm_XXX_funcs, so feedbacks are welcome! > > >>>> > > >>>> André > > >>>> > > >>>> André Almeida (2): > > >>>> drm/atomic: Allow drivers to write their own plane check for async > > >>>> flips > > >>>> drm/amdgpu: Implement check_async_props for planes > > >>>> > > >>>> .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 30 +++++++++ > > >>>> drivers/gpu/drm/drm_atomic_uapi.c | 62 > > ++++++++++++++----- > > >>>> include/drm/drm_atomic_uapi.h | 12 ++++ > > >>>> include/drm/drm_plane.h | 5 ++ > > >>>> 4 files changed, 92 insertions(+), 17 deletions(-) > > >>>> > > >>> > > > > >
Am Mi., 17. Jan. 2024 um 09:55 Uhr schrieb Pekka Paalanen <ppaalanen@gmail.com>: > Is it important enough to be special-cased, e.g. to be always allowed > with async commits? I thought so, and sent a patch to dri-devel to make it happen, but there are some concerns about untested driver paths. https://lists.freedesktop.org/archives/dri-devel/2024-January/437511.html > Now that I think of it, if userspace needs to wait for the in-fence > itself before kicking KMS async, that would defeat much of the async's > point, right? And cases where in-fence is not necessary are so rare > they might not even exist? > > So if driver/hardware cannot do IN_FENCE_FD with async, is there any > use of supporting async to begin with? KWin never commits a buffer where IN_FENCE_FD would actually delay the pageflip; it's really only used to disable implicit sync, as there's some edge cases where it can wrongly delay the pageflip. The waiting for buffers to become readable on the compositor side isn't really significant in terms of latency. If hardware doesn't support IN_FENCE_FD with async commits, checking if the fence is already signaled at commit time would thus still make things work, at least for KWin. > > If the compositor prioritizes tearing and would like to do overlay planes > > if possible, > > it would have to know that switching to synchronous commits for a single > > frame, > > setting up the overlay planes and then switching back to async commits > > works, and > > that can't be figured out with TEST_ONLY commits. > > I had to ponder a bit why. So I guess the synchronous commit in between > is because driver/hardware may not be able to enable/disable extra > planes in async, so you need a synchronous commit to set them up, but > afterwards updates can tear. The hardware could be a factor, yes, but I've been thinking more about the API. With this patchset, the compositor is still only allowed to change a limited set of plane properties - but it needs to set at least SRC_X, SRC_Y, CRTC_X etc on the overlay plane(s) to the correct values before it can only change the allowed properties again. > The comment about Intel needing one more synchronous commit when > switching from sync to async updates comes to mind as well, would that > be a problem? With only one synchronous update, the compositor could theoretically mask the issue by committing it right before vblank; with that one implicitly-sync "async" commit though, you'd definitely get one frame without async commits. Having a flag for a sync-but-then-async-again commit could solve that issue. In practice I don't think anyone will notice one or two frames without async commits. It should be a pretty rare occurance, usually when the game or match starts or an overlay gets opened, so I doubt it's worth putting effort in to fix that. > > So I think having a CAP or immutable plane property to signal that async > > commits > > with overlay and/or cursor planes is supported would be useful. > > Async cursor planes a good point, particularly moving them around. I'm > not too informed about the prior/on-going efforts to allow cursor > movement more often than refresh rate, I recall something about > amending atomic commits? How would these interact? > > I suppose the kernel still prevents any new async commit while a > previous commit is not finished, so amending commits would still be > necessary for cursor plane motion? Or would it, if you time "big > commits" to finish quickly and then spam async "cursor commits" in the > mean time? With async commits for cursor planes I'm really only talking about getting to use the cursor plane while doing async commits on the primary plane. FWIW I personally consider the amend stuff mostly solved - KWin does that internally since a few months ago now, with a separate thread to amend and even reorder commits in a queue, and only actually commit immediately before vblank. > > Thanks, > pq > > > Am Di., 16. Jan. 2024 um 14:35 Uhr schrieb André Almeida < > > andrealmeid@igalia.com>: > > > > > + Joshua > > > > > > Em 16/01/2024 10:14, Pekka Paalanen escreveu: > > > > On Tue, 16 Jan 2024 08:50:59 -0300 > > > > André Almeida <andrealmeid@igalia.com> wrote: > > > > > > > >> Hi Pekka, > > > >> > > > >> Em 16/01/2024 06:45, Pekka Paalanen escreveu: > > > >>> On Tue, 16 Jan 2024 01:51:57 -0300 > > > >>> André Almeida <andrealmeid@igalia.com> wrote: > > > >>> > > > >>>> Hi, > > > >>>> > > > >>>> AMD hardware can do more on the async flip path than just the primary > > > plane, so > > > >>>> to lift up the current restrictions, this patchset allows drivers to > > > write their > > > >>>> own check for planes for async flips. > > > >>> > > > >>> Hi, > > > >>> > > > >>> what's the userspace story for this, how could userspace know it could > > > do more? > > > >>> What kind of userspace would take advantage of this and in what > > > situations? > > > >>> > > > >>> Or is this not meant for generic userspace? > > > >> > > > >> Sorry, I forgot to document this. So the idea is that userspace will > > > >> query what they can do here with DRM_MODE_ATOMIC_TEST_ONLY calls, > > > >> instead of having capabilities for each prop. > > > > > > > > That's the theory, but do you have a practical example? > > > > > > > > What other planes and props would one want change in some specific use > > > > case? > > > > > > > > Is it just "all or nothing", or would there be room to choose and pick > > > > which props you change and which you don't based on what the driver > > > > supports? If the latter, then relying on TEST_ONLY might be yet another > > > > combinatorial explosion to iterate through. > > > > > > > > > > That's a good question, maybe Simon, Xaver or Joshua can share how they > > > were planning to use this on Gamescope or Kwin. > > > > > > > > > > > Thanks, > > > > pq > > > > > > > >>>> I'm not sure if adding something new to drm_plane_funcs is the right > > > way to do, > > > >>>> because if we want to expand the other object types (crtc, connector) > > > we would > > > >>>> need to add their own drm_XXX_funcs, so feedbacks are welcome! > > > >>>> > > > >>>> André > > > >>>> > > > >>>> André Almeida (2): > > > >>>> drm/atomic: Allow drivers to write their own plane check for async > > > >>>> flips > > > >>>> drm/amdgpu: Implement check_async_props for planes > > > >>>> > > > >>>> .../amd/display/amdgpu_dm/amdgpu_dm_plane.c | 30 +++++++++ > > > >>>> drivers/gpu/drm/drm_atomic_uapi.c | 62 > > > ++++++++++++++----- > > > >>>> include/drm/drm_atomic_uapi.h | 12 ++++ > > > >>>> include/drm/drm_plane.h | 5 ++ > > > >>>> 4 files changed, 92 insertions(+), 17 deletions(-) > > > >>>> > > > >>> > > > > > > > >
On 2024-01-17 13:57, Xaver Hugl wrote: > Am Mi., 17. Jan. 2024 um 09:55 Uhr schrieb Pekka Paalanen <ppaalanen@gmail.com>: >> Is it important enough to be special-cased, e.g. to be always allowed >> with async commits? > > I thought so, and sent a patch to dri-devel to make it happen, but > there are some > concerns about untested driver paths. > https://lists.freedesktop.org/archives/dri-devel/2024-January/437511.html > >> Now that I think of it, if userspace needs to wait for the in-fence >> itself before kicking KMS async, that would defeat much of the async's >> point, right? And cases where in-fence is not necessary are so rare >> they might not even exist? >> >> So if driver/hardware cannot do IN_FENCE_FD with async, is there any >> use of supporting async to begin with? > > KWin never commits a buffer where IN_FENCE_FD would actually delay the > pageflip; it's really only used to disable implicit sync, as there's some edge > cases where it can wrongly delay the pageflip. The waiting for buffers to become > readable on the compositor side isn't really significant in terms of latency. > > If hardware doesn't support IN_FENCE_FD with async commits, checking if the > fence is already signaled at commit time would thus still make things work, at > least for KWin. That's how IN_FENCE_FD (and implicit sync) is handled anyway, in common code: It waits for all fences to signal before calling into the driver to commit the atomic commit. I can't see why this wouldn't work with async commits, the same as with synchronous ones, with any driver.