Message ID | 20230707224059.305474-7-andrealmeid@igalia.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9f45:0:b0:3ea:f831:8777 with SMTP id v5csp3588947vqx; Fri, 7 Jul 2023 16:04:19 -0700 (PDT) X-Google-Smtp-Source: APBJJlH2LRHlA04fL1AiK3oyBGN8X7I/mLdbeO9zbQUbfCG9QcjLdJeZXGDfAOmAMlIvArf+duKm X-Received: by 2002:a05:6402:31f0:b0:51e:2e39:9003 with SMTP id dy16-20020a05640231f000b0051e2e399003mr4039154edb.40.1688771059465; Fri, 07 Jul 2023 16:04:19 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1688771059; cv=none; d=google.com; s=arc-20160816; b=TOhJLU5LBjYMiC76MydOZcZzSaI0uEBvaP6R6STvJtIMDheP2Em2CJ8FM4FCJOe6rz xP6XoxvmRS276Y2aLy1Jb62zqUiHuBrMq0PSNoRdT7hHpb7JgELOO7EYlvOO/C4L9BMb PHfy9IwXr0ER3+5r8dENQa2PVnfnN7b4wxXys0MZU6M9spIBLEBNvbsZ79eLtfdX5WFl 471mj1zo6GHFoXalhClBIgh6gDcfbVeF1aNzQKae/vTTfQDsi1lKDG2lOAHNX7qP+r0m 8GsO/DIMi+DXyTgu/044nsOoaK+bBdZtGXwUbffQu+bvoVFUDXnGfVDiKSKbVa1fHtax 3J1g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=ILhK3ntOSguFvvoL3dtIQ/0orxl4TRRaxUfEny2T4hQ=; fh=LjT+njuVizWTSzr6iuglUnPxawOqQbLOR7Ym8u+3GU0=; b=BxsCeHOSZlNjj0xX6oP0dJcVA7MI1R3B43PURX6kWlV4g4KWZOq3XVy8Podcq5EuqB 7PpIAneFaq9RK17gmtcntYcbpacjNMnwWGzS2VHtqEJr6HDWn7N5gt18VTJWSg28s7pr 74U1xG8z95f+hdZiMyxQroftyzSV2GnGekw4U0mb5NRbx71Q0aJyAJr4a23TReFpRPXG FZmKJRiSrkzfiSOO2+cEvSCgHSD5k/7Y1e51ffWiRi+bRdQ7eHZWzxAWiDbTr8WQqXO3 ITSsHjXVO+XyM86MBVzBqaQsVkwNGEr+x+HUYySlcHTfKP8wAawWZpqXC8tvpvuhOQHf YHWA== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@igalia.com header.s=20170329 header.b="luAS5a/o"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id l19-20020a056402029300b0051e1b797279si2838947edv.166.2023.07.07.16.03.55; Fri, 07 Jul 2023 16:04:19 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=fail header.i=@igalia.com header.s=20170329 header.b="luAS5a/o"; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232560AbjGGWmI (ORCPT <rfc822;hadasmailinglist@gmail.com> + 99 others); Fri, 7 Jul 2023 18:42:08 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56006 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232505AbjGGWlx (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 7 Jul 2023 18:41:53 -0400 Received: from fanzine2.igalia.com (fanzine2.igalia.com [213.97.179.56]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 364C826BC for <linux-kernel@vger.kernel.org>; Fri, 7 Jul 2023 15:41:45 -0700 (PDT) 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:References: In-Reply-To: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:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=ILhK3ntOSguFvvoL3dtIQ/0orxl4TRRaxUfEny2T4hQ=; b=luAS5a/ov3Z7w1pwXMMISdcq0X CmkBLudpjLqOfv+LJLAAHgZSMAOwntplymB5St0mZV6gheeahYHsf0oKk1dEbckva2adI1sAtldg0 zSAfkyB5qESAnGy93JB4hhHc7vEiAIq1eGRa5uuHhV8Ye1zRG/hGSONknqFrN7KJDUsh41ixqVhCn 7pEMfWtuB6/0Rb4vY44mVDt1yIc9YleiwG15e+Ed80YgxVvmPu+KiZZx1W017yqrMNixeQtYjnRJr 5oOG8jA6fnQ2CMTbxf5k92qZOx0ViYm1r5jHbNCXCuZSKJ8sQ4SM5777oDp3S9kiK0I4V0eDj8DvU bbpZeGCQ==; Received: from [187.74.70.209] (helo=steammachine.lan) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim) id 1qHu8z-00AP6e-SZ; Sat, 08 Jul 2023 00:41:42 +0200 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, wayland-devel@lists.freedesktop.org Cc: kernel-dev@igalia.com, alexander.deucher@amd.com, christian.koenig@amd.com, pierre-eric.pelloux-prayer@amd.com, Simon Ser <contact@emersion.fr>, Rob Clark <robdclark@gmail.com>, Pekka Paalanen <ppaalanen@gmail.com>, Daniel Vetter <daniel@ffwll.ch>, Daniel Stone <daniel@fooishbar.org>, =?utf-8?b?J01hcmVrIE9sxaHDoWsn?= <maraeo@gmail.com>, Dave Airlie <airlied@gmail.com>, =?utf-8?q?Michel_D=C3=A4nzer?= <michel.daenzer@mailbox.org>, Randy Dunlap <rdunlap@infradead.org>, hwentlan@amd.com, joshua@froggi.es, ville.syrjala@linux.intel.com, Pekka Paalanen <pekka.paalanen@collabora.com>, =?utf-8?q?Andr=C3=A9_Almeida?= <andrealmeid@igalia.com> Subject: [PATCH v5 6/6] drm/doc: Define KMS atomic state set Date: Fri, 7 Jul 2023 19:40:59 -0300 Message-ID: <20230707224059.305474-7-andrealmeid@igalia.com> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230707224059.305474-1-andrealmeid@igalia.com> References: <20230707224059.305474-1-andrealmeid@igalia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1770804802215675514?= X-GMAIL-MSGID: =?utf-8?q?1770804802215675514?= |
Series |
drm: Add support for atomic async page-flip
|
|
Commit Message
André Almeida
July 7, 2023, 10:40 p.m. UTC
From: Pekka Paalanen <pekka.paalanen@collabora.com> Specify how the atomic state is maintained between userspace and kernel, plus the special case for async flips. Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com> Signed-off-by: André Almeida <andrealmeid@igalia.com> --- v4: total rework by Pekka --- Documentation/gpu/drm-uapi.rst | 41 ++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+)
Comments
On Fri, 7 Jul 2023 19:40:59 -0300 André Almeida <andrealmeid@igalia.com> wrote: > From: Pekka Paalanen <pekka.paalanen@collabora.com> > > Specify how the atomic state is maintained between userspace and > kernel, plus the special case for async flips. > > Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com> > Signed-off-by: André Almeida <andrealmeid@igalia.com> > --- > v4: total rework by Pekka > --- > Documentation/gpu/drm-uapi.rst | 41 ++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) Thank you for polishing that email into a proper patch! For patches 1 and 2: Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com> Thanks, pq > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 65fb3036a580..6a1662c08901 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -486,3 +486,44 @@ and the CRTC index is its position in this array. > > .. kernel-doc:: include/uapi/drm/drm_mode.h > :internal: > + > +KMS atomic state > +================ > + > +An atomic commit can change multiple KMS properties in an atomic fashion, > +without ever applying intermediate or partial state changes. Either the whole > +commit succeeds or fails, and it will never be applied partially. This is the > +fundamental improvement of the atomic API over the older non-atomic API which is > +referred to as the "legacy API". Applying intermediate state could unexpectedly > +fail, cause visible glitches, or delay reaching the final state. > + > +An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, which means the > +complete state change is validated but not applied. Userspace should use this > +flag to validate any state change before asking to apply it. If validation fails > +for any reason, userspace should attempt to fall back to another, perhaps > +simpler, final state. This allows userspace to probe for various configurations > +without causing visible glitches on screen and without the need to undo a > +probing change. > + > +The changes recorded in an atomic commit apply on top the current KMS state in > +the kernel. Hence, the complete new KMS state is the complete old KMS state with > +the committed property settings done on top. The kernel will automatically avoid > +no-operation changes, so it is safe and even expected for userspace to send > +redundant property settings. No-operation changes do not count towards actually > +needed changes, e.g. setting MODE_ID to a different blob with identical > +contents as the current KMS state shall not be a modeset on its own. > + > +A "modeset" is a change in KMS state that might enable, disable, or temporarily > +disrupt the emitted video signal, possibly causing visible glitches on screen. A > +modeset may also take considerably more time to complete than other kinds of > +changes, and the video sink might also need time to adapt to the new signal > +properties. Therefore a modeset must be explicitly allowed with the flag > +DRM_MODE_ATOMIC_ALLOW_MODESET. This in combination with > +DRM_MODE_ATOMIC_TEST_ONLY allows userspace to determine if a state change is > +likely to cause visible disruption on screen and avoid such changes when end > +users do not expect them. > + > +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to > +effectively change only the FB_ID property on any planes. No-operation changes > +are ignored as always. Changing any other property will cause the commit to be > +rejected.
On Fri, Jul 07, 2023 at 07:40:59PM -0300, André Almeida wrote: > From: Pekka Paalanen <pekka.paalanen@collabora.com> > > Specify how the atomic state is maintained between userspace and > kernel, plus the special case for async flips. > > Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com> > Signed-off-by: André Almeida <andrealmeid@igalia.com> > --- > v4: total rework by Pekka > --- > Documentation/gpu/drm-uapi.rst | 41 ++++++++++++++++++++++++++++++++++ > 1 file changed, 41 insertions(+) > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > index 65fb3036a580..6a1662c08901 100644 > --- a/Documentation/gpu/drm-uapi.rst > +++ b/Documentation/gpu/drm-uapi.rst > @@ -486,3 +486,44 @@ and the CRTC index is its position in this array. > > .. kernel-doc:: include/uapi/drm/drm_mode.h > :internal: > + > +KMS atomic state > +================ > + > +An atomic commit can change multiple KMS properties in an atomic fashion, > +without ever applying intermediate or partial state changes. Either the whole > +commit succeeds or fails, and it will never be applied partially. This is the > +fundamental improvement of the atomic API over the older non-atomic API which is > +referred to as the "legacy API". Applying intermediate state could unexpectedly > +fail, cause visible glitches, or delay reaching the final state. > + > +An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, which means the > +complete state change is validated but not applied. Userspace should use this > +flag to validate any state change before asking to apply it. If validation fails > +for any reason, userspace should attempt to fall back to another, perhaps > +simpler, final state. This allows userspace to probe for various configurations > +without causing visible glitches on screen and without the need to undo a > +probing change. > + > +The changes recorded in an atomic commit apply on top the current KMS state in > +the kernel. Hence, the complete new KMS state is the complete old KMS state with > +the committed property settings done on top. The kernel will automatically avoid > +no-operation changes, so it is safe and even expected for userspace to send > +redundant property settings. No-operation changes do not count towards actually > +needed changes, e.g. setting MODE_ID to a different blob with identical > +contents as the current KMS state shall not be a modeset on its own. Small clarification: The kernel indeed tries very hard to make redundant changes a no-op, and I think we should consider any issues here bugs. But it still has to check, which means it needs to acquire the right locks and put in the right (cross-crtc) synchronization points, and due to implmentation challenges it's very hard to try to avoid that in all cases. So adding redundant changes especially across crtc (and their connected planes/connectors) might result in some oversynchronization issues, and userspace should therefore avoid them if feasible. With some sentences added to clarify this: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > + > +A "modeset" is a change in KMS state that might enable, disable, or temporarily > +disrupt the emitted video signal, possibly causing visible glitches on screen. A > +modeset may also take considerably more time to complete than other kinds of > +changes, and the video sink might also need time to adapt to the new signal > +properties. Therefore a modeset must be explicitly allowed with the flag > +DRM_MODE_ATOMIC_ALLOW_MODESET. This in combination with > +DRM_MODE_ATOMIC_TEST_ONLY allows userspace to determine if a state change is > +likely to cause visible disruption on screen and avoid such changes when end > +users do not expect them. > + > +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to > +effectively change only the FB_ID property on any planes. No-operation changes > +are ignored as always. Changing any other property will cause the commit to be > +rejected. > -- > 2.41.0 >
On Saturday, July 8th, 2023 at 00:40, André Almeida <andrealmeid@igalia.com> wrote: > +KMS atomic state > +================ > + > +An atomic commit can change multiple KMS properties in an atomic fashion, > +without ever applying intermediate or partial state changes. Either the whole > +commit succeeds or fails, and it will never be applied partially. This is the > +fundamental improvement of the atomic API over the older non-atomic API which is > +referred to as the "legacy API". Applying intermediate state could unexpectedly > +fail, cause visible glitches, or delay reaching the final state. > + > +An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, which means the Can we linkify these #defines? This can be done like so: … flagged with :c:macro:`DRM_MODE_ATOMIC_TEST_ONLY`, which means … This should create a link to the docs for this flag.
On Tue, 11 Jul 2023 10:57:57 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Fri, Jul 07, 2023 at 07:40:59PM -0300, André Almeida wrote: > > From: Pekka Paalanen <pekka.paalanen@collabora.com> > > > > Specify how the atomic state is maintained between userspace and > > kernel, plus the special case for async flips. > > > > Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com> > > Signed-off-by: André Almeida <andrealmeid@igalia.com> > > --- > > v4: total rework by Pekka > > --- > > Documentation/gpu/drm-uapi.rst | 41 ++++++++++++++++++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > > > diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > > index 65fb3036a580..6a1662c08901 100644 > > --- a/Documentation/gpu/drm-uapi.rst > > +++ b/Documentation/gpu/drm-uapi.rst > > @@ -486,3 +486,44 @@ and the CRTC index is its position in this array. > > > > .. kernel-doc:: include/uapi/drm/drm_mode.h > > :internal: > > + > > +KMS atomic state > > +================ > > + > > +An atomic commit can change multiple KMS properties in an atomic fashion, > > +without ever applying intermediate or partial state changes. Either the whole > > +commit succeeds or fails, and it will never be applied partially. This is the > > +fundamental improvement of the atomic API over the older non-atomic API which is > > +referred to as the "legacy API". Applying intermediate state could unexpectedly > > +fail, cause visible glitches, or delay reaching the final state. > > + > > +An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, which means the > > +complete state change is validated but not applied. Userspace should use this > > +flag to validate any state change before asking to apply it. If validation fails > > +for any reason, userspace should attempt to fall back to another, perhaps > > +simpler, final state. This allows userspace to probe for various configurations > > +without causing visible glitches on screen and without the need to undo a > > +probing change. > > + > > +The changes recorded in an atomic commit apply on top the current KMS state in > > +the kernel. Hence, the complete new KMS state is the complete old KMS state with > > +the committed property settings done on top. The kernel will automatically avoid > > +no-operation changes, so it is safe and even expected for userspace to send > > +redundant property settings. No-operation changes do not count towards actually > > +needed changes, e.g. setting MODE_ID to a different blob with identical > > +contents as the current KMS state shall not be a modeset on its own. > > Small clarification: The kernel indeed tries very hard to make redundant > changes a no-op, and I think we should consider any issues here bugs. But > it still has to check, which means it needs to acquire the right locks and > put in the right (cross-crtc) synchronization points, and due to > implmentation challenges it's very hard to try to avoid that in all cases. > So adding redundant changes especially across crtc (and their connected > planes/connectors) might result in some oversynchronization issues, and > userspace should therefore avoid them if feasible. > > With some sentences added to clarify this: > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> After talking on IRC yesterday, we realized that the no-op rule is nowhere near as generic as I have believed. Roughly: https://oftc.irclog.whitequark.org/dri-devel/2023-07-12#1689152446-1689157291; Thanks, pq > > + > > +A "modeset" is a change in KMS state that might enable, disable, or temporarily > > +disrupt the emitted video signal, possibly causing visible glitches on screen. A > > +modeset may also take considerably more time to complete than other kinds of > > +changes, and the video sink might also need time to adapt to the new signal > > +properties. Therefore a modeset must be explicitly allowed with the flag > > +DRM_MODE_ATOMIC_ALLOW_MODESET. This in combination with > > +DRM_MODE_ATOMIC_TEST_ONLY allows userspace to determine if a state change is > > +likely to cause visible disruption on screen and avoid such changes when end > > +users do not expect them. > > + > > +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to > > +effectively change only the FB_ID property on any planes. No-operation changes > > +are ignored as always. Changing any other property will cause the commit to be > > +rejected. > > -- > > 2.41.0 > > >
Em 13/07/2023 04:51, Pekka Paalanen escreveu: > On Tue, 11 Jul 2023 10:57:57 +0200 > Daniel Vetter <daniel@ffwll.ch> wrote: > >> On Fri, Jul 07, 2023 at 07:40:59PM -0300, André Almeida wrote: >>> From: Pekka Paalanen <pekka.paalanen@collabora.com> >>> >>> Specify how the atomic state is maintained between userspace and >>> kernel, plus the special case for async flips. >>> >>> Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com> >>> Signed-off-by: André Almeida <andrealmeid@igalia.com> >>> --- >>> v4: total rework by Pekka >>> --- >>> Documentation/gpu/drm-uapi.rst | 41 ++++++++++++++++++++++++++++++++++ >>> 1 file changed, 41 insertions(+) >>> >>> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst >>> index 65fb3036a580..6a1662c08901 100644 >>> --- a/Documentation/gpu/drm-uapi.rst >>> +++ b/Documentation/gpu/drm-uapi.rst >>> @@ -486,3 +486,44 @@ and the CRTC index is its position in this array. >>> >>> .. kernel-doc:: include/uapi/drm/drm_mode.h >>> :internal: >>> + >>> +KMS atomic state >>> +================ >>> + >>> +An atomic commit can change multiple KMS properties in an atomic fashion, >>> +without ever applying intermediate or partial state changes. Either the whole >>> +commit succeeds or fails, and it will never be applied partially. This is the >>> +fundamental improvement of the atomic API over the older non-atomic API which is >>> +referred to as the "legacy API". Applying intermediate state could unexpectedly >>> +fail, cause visible glitches, or delay reaching the final state. >>> + >>> +An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, which means the >>> +complete state change is validated but not applied. Userspace should use this >>> +flag to validate any state change before asking to apply it. If validation fails >>> +for any reason, userspace should attempt to fall back to another, perhaps >>> +simpler, final state. This allows userspace to probe for various configurations >>> +without causing visible glitches on screen and without the need to undo a >>> +probing change. >>> + >>> +The changes recorded in an atomic commit apply on top the current KMS state in >>> +the kernel. Hence, the complete new KMS state is the complete old KMS state with >>> +the committed property settings done on top. The kernel will automatically avoid >>> +no-operation changes, so it is safe and even expected for userspace to send >>> +redundant property settings. No-operation changes do not count towards actually >>> +needed changes, e.g. setting MODE_ID to a different blob with identical >>> +contents as the current KMS state shall not be a modeset on its own. >> >> Small clarification: The kernel indeed tries very hard to make redundant >> changes a no-op, and I think we should consider any issues here bugs. But >> it still has to check, which means it needs to acquire the right locks and >> put in the right (cross-crtc) synchronization points, and due to >> implmentation challenges it's very hard to try to avoid that in all cases. >> So adding redundant changes especially across crtc (and their connected >> planes/connectors) might result in some oversynchronization issues, and >> userspace should therefore avoid them if feasible. >> >> With some sentences added to clarify this: >> >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > After talking on IRC yesterday, we realized that the no-op rule is > nowhere near as generic as I have believed. Roughly: > https://oftc.irclog.whitequark.org/dri-devel/2023-07-12#1689152446-1689157291; > > How about: The changes recorded in an atomic commit apply on top the current KMS state in the kernel. Hence, the complete new KMS state is the complete old KMS state with the committed property settings done on top. The kernel will try to avoid no-operation changes, so it is safe for userspace to send redundant property settings. However, the kernel can not assure that every redundant information will always result in a no-op, giving the need to take locks to check par of the state. Giving that, some redundant information can lead to a full damage path. This is not something bad by itself, but userspace need to be aware of that side effect. > Thanks, > pq > >>> + >>> +A "modeset" is a change in KMS state that might enable, disable, or temporarily >>> +disrupt the emitted video signal, possibly causing visible glitches on screen. A >>> +modeset may also take considerably more time to complete than other kinds of >>> +changes, and the video sink might also need time to adapt to the new signal >>> +properties. Therefore a modeset must be explicitly allowed with the flag >>> +DRM_MODE_ATOMIC_ALLOW_MODESET. This in combination with >>> +DRM_MODE_ATOMIC_TEST_ONLY allows userspace to determine if a state change is >>> +likely to cause visible disruption on screen and avoid such changes when end >>> +users do not expect them. >>> + >>> +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to >>> +effectively change only the FB_ID property on any planes. No-operation changes >>> +are ignored as always. Changing any other property will cause the commit to be >>> +rejected. >>> -- >>> 2.41.0 >>> >> >
On Mon, 31 Jul 2023 at 04:01, André Almeida <andrealmeid@igalia.com> wrote: > > Em 13/07/2023 04:51, Pekka Paalanen escreveu: > > On Tue, 11 Jul 2023 10:57:57 +0200 > > Daniel Vetter <daniel@ffwll.ch> wrote: > > > >> On Fri, Jul 07, 2023 at 07:40:59PM -0300, André Almeida wrote: > >>> From: Pekka Paalanen <pekka.paalanen@collabora.com> > >>> > >>> Specify how the atomic state is maintained between userspace and > >>> kernel, plus the special case for async flips. > >>> > >>> Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.com> > >>> Signed-off-by: André Almeida <andrealmeid@igalia.com> > >>> --- > >>> v4: total rework by Pekka > >>> --- > >>> Documentation/gpu/drm-uapi.rst | 41 ++++++++++++++++++++++++++++++++++ > >>> 1 file changed, 41 insertions(+) > >>> > >>> diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst > >>> index 65fb3036a580..6a1662c08901 100644 > >>> --- a/Documentation/gpu/drm-uapi.rst > >>> +++ b/Documentation/gpu/drm-uapi.rst > >>> @@ -486,3 +486,44 @@ and the CRTC index is its position in this array. > >>> > >>> .. kernel-doc:: include/uapi/drm/drm_mode.h > >>> :internal: > >>> + > >>> +KMS atomic state > >>> +================ > >>> + > >>> +An atomic commit can change multiple KMS properties in an atomic fashion, > >>> +without ever applying intermediate or partial state changes. Either the whole > >>> +commit succeeds or fails, and it will never be applied partially. This is the > >>> +fundamental improvement of the atomic API over the older non-atomic API which is > >>> +referred to as the "legacy API". Applying intermediate state could unexpectedly > >>> +fail, cause visible glitches, or delay reaching the final state. > >>> + > >>> +An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, which means the > >>> +complete state change is validated but not applied. Userspace should use this > >>> +flag to validate any state change before asking to apply it. If validation fails > >>> +for any reason, userspace should attempt to fall back to another, perhaps > >>> +simpler, final state. This allows userspace to probe for various configurations > >>> +without causing visible glitches on screen and without the need to undo a > >>> +probing change. > >>> + > >>> +The changes recorded in an atomic commit apply on top the current KMS state in > >>> +the kernel. Hence, the complete new KMS state is the complete old KMS state with > >>> +the committed property settings done on top. The kernel will automatically avoid > >>> +no-operation changes, so it is safe and even expected for userspace to send > >>> +redundant property settings. No-operation changes do not count towards actually > >>> +needed changes, e.g. setting MODE_ID to a different blob with identical > >>> +contents as the current KMS state shall not be a modeset on its own. > >> > >> Small clarification: The kernel indeed tries very hard to make redundant > >> changes a no-op, and I think we should consider any issues here bugs. But > >> it still has to check, which means it needs to acquire the right locks and > >> put in the right (cross-crtc) synchronization points, and due to > >> implmentation challenges it's very hard to try to avoid that in all cases. > >> So adding redundant changes especially across crtc (and their connected > >> planes/connectors) might result in some oversynchronization issues, and > >> userspace should therefore avoid them if feasible. > >> > >> With some sentences added to clarify this: > >> > >> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > After talking on IRC yesterday, we realized that the no-op rule is > > nowhere near as generic as I have believed. Roughly: > > https://oftc.irclog.whitequark.org/dri-devel/2023-07-12#1689152446-1689157291; > > > > > > How about: > > The changes recorded in an atomic commit apply on top the current KMS > state in the kernel. Hence, the complete new KMS state is the complete > old KMS state with the committed property settings done on top. The > kernel will try to avoid no-operation changes, so it is safe for > userspace to send redundant property settings. However, the kernel can > not assure that every redundant information will always result in a > no-op, giving the need to take locks to check par of the state. Giving > that, some redundant information can lead to a full damage path. This is > not something bad by itself, but userspace need to be aware of that side > effect. I think the addition about damage tracking should be a separate paragraph, and not mixed in with the general explanation that no-op updates might lead to oversync/overlocking issues. Because the damage tracking issue is more a question of efficiency/power usage, but should not (for most drivers/hw at least) result in delays and missed updates due to oversynchronization of updates. Also in my opinion the exact damage update rules are more a plane property issue, and should probably be clarified there if the current documentation is not clear enough. Since it's not about whether no-op updates get avoided by the kernel, but what exact damage is implied in various cases (and that implied damage has to exist, otherwise backwards compatibility is broken, but userspace can avoid these issues by setting an empty damage property for that plane update explicitly). So I think for this doc part here the discussed text is still good enough, but we might need more in other places. -Sima > > > Thanks, > > pq > > > >>> + > >>> +A "modeset" is a change in KMS state that might enable, disable, or temporarily > >>> +disrupt the emitted video signal, possibly causing visible glitches on screen. A > >>> +modeset may also take considerably more time to complete than other kinds of > >>> +changes, and the video sink might also need time to adapt to the new signal > >>> +properties. Therefore a modeset must be explicitly allowed with the flag > >>> +DRM_MODE_ATOMIC_ALLOW_MODESET. This in combination with > >>> +DRM_MODE_ATOMIC_TEST_ONLY allows userspace to determine if a state change is > >>> +likely to cause visible disruption on screen and avoid such changes when end > >>> +users do not expect them. > >>> + > >>> +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to > >>> +effectively change only the FB_ID property on any planes. No-operation changes > >>> +are ignored as always. Changing any other property will cause the commit to be > >>> +rejected. > >>> -- > >>> 2.41.0 > >>> > >> > >
diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst index 65fb3036a580..6a1662c08901 100644 --- a/Documentation/gpu/drm-uapi.rst +++ b/Documentation/gpu/drm-uapi.rst @@ -486,3 +486,44 @@ and the CRTC index is its position in this array. .. kernel-doc:: include/uapi/drm/drm_mode.h :internal: + +KMS atomic state +================ + +An atomic commit can change multiple KMS properties in an atomic fashion, +without ever applying intermediate or partial state changes. Either the whole +commit succeeds or fails, and it will never be applied partially. This is the +fundamental improvement of the atomic API over the older non-atomic API which is +referred to as the "legacy API". Applying intermediate state could unexpectedly +fail, cause visible glitches, or delay reaching the final state. + +An atomic commit can be flagged with DRM_MODE_ATOMIC_TEST_ONLY, which means the +complete state change is validated but not applied. Userspace should use this +flag to validate any state change before asking to apply it. If validation fails +for any reason, userspace should attempt to fall back to another, perhaps +simpler, final state. This allows userspace to probe for various configurations +without causing visible glitches on screen and without the need to undo a +probing change. + +The changes recorded in an atomic commit apply on top the current KMS state in +the kernel. Hence, the complete new KMS state is the complete old KMS state with +the committed property settings done on top. The kernel will automatically avoid +no-operation changes, so it is safe and even expected for userspace to send +redundant property settings. No-operation changes do not count towards actually +needed changes, e.g. setting MODE_ID to a different blob with identical +contents as the current KMS state shall not be a modeset on its own. + +A "modeset" is a change in KMS state that might enable, disable, or temporarily +disrupt the emitted video signal, possibly causing visible glitches on screen. A +modeset may also take considerably more time to complete than other kinds of +changes, and the video sink might also need time to adapt to the new signal +properties. Therefore a modeset must be explicitly allowed with the flag +DRM_MODE_ATOMIC_ALLOW_MODESET. This in combination with +DRM_MODE_ATOMIC_TEST_ONLY allows userspace to determine if a state change is +likely to cause visible disruption on screen and avoid such changes when end +users do not expect them. + +An atomic commit with the flag DRM_MODE_PAGE_FLIP_ASYNC is allowed to +effectively change only the FB_ID property on any planes. No-operation changes +are ignored as always. Changing any other property will cause the commit to be +rejected.