All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maxime Ripard <mripard@kernel.org>
To: David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	 Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	 Thomas Zimmermann <tzimmermann@suse.de>,
	Emma Anholt <emma@anholt.net>
Cc: linux-kselftest@vger.kernel.org,
	"Brendan Higgins" <brendan.higgins@linux.dev>,
	"Javier Martinez Canillas" <javierm@redhat.com>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	"Maíra Canal" <mairacanal@riseup.net>,
	"Maxime Ripard" <mripard@kernel.org>,
	"David Gow" <davidgow@google.com>,
	kunit-dev@googlegroups.com
Subject: [PATCH v2 11/11] drm/vc4: tests: pv-muxing: Document test scenario
Date: Thu, 20 Jul 2023 13:15:56 +0200	[thread overview]
Message-ID: <20230720-kms-kunit-actions-rework-v2-11-175017bd56ab@kernel.org> (raw)
In-Reply-To: <20230720-kms-kunit-actions-rework-v2-0-175017bd56ab@kernel.org>

We've had a couple of tests that weren't really obvious, nor did they
document what they were supposed to test. Document that to make it
hopefully more obvious.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
index 5f9f5626329d..61622e951031 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
@@ -845,6 +845,13 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes
 	KUNIT_EXPECT_NE(test, hdmi0_channel, hdmi1_channel);
 }
 
+/*
+ * This test makes sure that we never change the FIFO of an active HVS
+ * channel if we disable a FIFO with a lower index.
+ *
+ * Doing so would result in a FIFO stall and would disrupt an output
+ * supposed to be unaffected by the commit.
+ */
 static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
 {
 	struct drm_modeset_acquire_ctx *ctx;
@@ -924,6 +931,21 @@ static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
 	}
 }
 
+/*
+ * Test that if we affect a single output, only the CRTC state of that
+ * output will be pulled in the global atomic state.
+ *
+ * This is relevant for two things:
+ *
+ *   - If we don't have that state at all, we are unlikely to affect the
+ *     FIFO muxing. This is somewhat redundant with
+ *     drm_test_vc5_pv_muxing_bugs_stable_fifo()
+ *
+ *   - KMS waits for page flips to occur on all the CRTC found in the
+ *     CRTC state. Since the CRTC is unaffected, we would over-wait, but
+ *     most importantly run into corner cases like waiting on an
+ *     inactive CRTC that never completes.
+ */
 static void
 drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct kunit *test)
 {

-- 
2.41.0


WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <mripard@kernel.org>
To: David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Emma Anholt <emma@anholt.net>
Cc: "Maíra Canal" <mairacanal@riseup.net>,
	"Javier Martinez Canillas" <javierm@redhat.com>,
	dri-devel@lists.freedesktop.org,
	"David Gow" <davidgow@google.com>,
	linux-kselftest@vger.kernel.org, kunit-dev@googlegroups.com,
	linux-kernel@vger.kernel.org,
	"Maxime Ripard" <mripard@kernel.org>,
	"Brendan Higgins" <brendan.higgins@linux.dev>
Subject: [PATCH v2 11/11] drm/vc4: tests: pv-muxing: Document test scenario
Date: Thu, 20 Jul 2023 13:15:56 +0200	[thread overview]
Message-ID: <20230720-kms-kunit-actions-rework-v2-11-175017bd56ab@kernel.org> (raw)
In-Reply-To: <20230720-kms-kunit-actions-rework-v2-0-175017bd56ab@kernel.org>

We've had a couple of tests that weren't really obvious, nor did they
document what they were supposed to test. Document that to make it
hopefully more obvious.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
index 5f9f5626329d..61622e951031 100644
--- a/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
+++ b/drivers/gpu/drm/vc4/tests/vc4_test_pv_muxing.c
@@ -845,6 +845,13 @@ static void drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable(struct kunit *tes
 	KUNIT_EXPECT_NE(test, hdmi0_channel, hdmi1_channel);
 }
 
+/*
+ * This test makes sure that we never change the FIFO of an active HVS
+ * channel if we disable a FIFO with a lower index.
+ *
+ * Doing so would result in a FIFO stall and would disrupt an output
+ * supposed to be unaffected by the commit.
+ */
 static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
 {
 	struct drm_modeset_acquire_ctx *ctx;
@@ -924,6 +931,21 @@ static void drm_test_vc5_pv_muxing_bugs_stable_fifo(struct kunit *test)
 	}
 }
 
+/*
+ * Test that if we affect a single output, only the CRTC state of that
+ * output will be pulled in the global atomic state.
+ *
+ * This is relevant for two things:
+ *
+ *   - If we don't have that state at all, we are unlikely to affect the
+ *     FIFO muxing. This is somewhat redundant with
+ *     drm_test_vc5_pv_muxing_bugs_stable_fifo()
+ *
+ *   - KMS waits for page flips to occur on all the CRTC found in the
+ *     CRTC state. Since the CRTC is unaffected, we would over-wait, but
+ *     most importantly run into corner cases like waiting on an
+ *     inactive CRTC that never completes.
+ */
 static void
 drm_test_vc5_pv_muxing_bugs_subsequent_crtc_enable_too_many_crtc_state(struct kunit *test)
 {

-- 
2.41.0


  parent reply	other threads:[~2023-07-20 11:16 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-20 11:15 [PATCH v2 00/11] drm: kunit: Switch to kunit actions Maxime Ripard
2023-07-20 11:15 ` Maxime Ripard
2023-07-20 11:15 ` [PATCH v2 01/11] drm/tests: helpers: " Maxime Ripard
2023-07-20 11:15   ` Maxime Ripard
2023-07-20 17:14   ` kernel test robot
2023-07-20 17:14     ` kernel test robot
2023-07-24 11:19     ` Maxime Ripard
2023-07-24 11:19       ` Maxime Ripard
2023-07-20 11:15 ` [PATCH v2 02/11] drm/tests: client-modeset: Remove call to drm_kunit_helper_free_device() Maxime Ripard
2023-07-20 11:15   ` Maxime Ripard
2023-07-20 11:15 ` [PATCH v2 03/11] drm/tests: modes: " Maxime Ripard
2023-07-20 11:15   ` Maxime Ripard
2023-07-20 11:15 ` [PATCH v2 04/11] drm/tests: probe-helper: " Maxime Ripard
2023-07-20 11:15   ` Maxime Ripard
2023-07-20 11:15 ` [PATCH v2 05/11] drm/tests: helpers: Create a helper to allocate a locking ctx Maxime Ripard
2023-07-20 11:15   ` Maxime Ripard
2023-07-20 14:37   ` kernel test robot
2023-07-20 14:37     ` kernel test robot
2023-07-20 11:15 ` [PATCH v2 06/11] drm/tests: helpers: Create a helper to allocate an atomic state Maxime Ripard
2023-07-20 11:15   ` Maxime Ripard
2023-07-20 18:06   ` kernel test robot
2023-07-20 18:06     ` kernel test robot
2023-07-20 11:15 ` [PATCH v2 07/11] drm/vc4: tests: pv-muxing: Remove call to drm_kunit_helper_free_device() Maxime Ripard
2023-07-20 11:15   ` Maxime Ripard
2023-07-20 11:15 ` [PATCH v2 08/11] drm/vc4: tests: mock: Use a kunit action to unregister DRM device Maxime Ripard
2023-07-20 11:15   ` Maxime Ripard
2023-07-20 11:15 ` [PATCH v2 09/11] drm/vc4: tests: pv-muxing: Switch to managed locking init Maxime Ripard
2023-07-20 11:15   ` Maxime Ripard
2023-07-20 11:15 ` [PATCH v2 10/11] drm/vc4: tests: Switch to atomic state allocation helper Maxime Ripard
2023-07-20 11:15   ` Maxime Ripard
2023-07-20 11:15 ` Maxime Ripard [this message]
2023-07-20 11:15   ` [PATCH v2 11/11] drm/vc4: tests: pv-muxing: Document test scenario Maxime Ripard
2023-07-23 14:22 ` [PATCH v2 00/11] drm: kunit: Switch to kunit actions Maira Canal
2023-07-23 14:22   ` Maira Canal
2023-07-31 12:23 ` Maxime Ripard
2023-07-31 12:23   ` Maxime Ripard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230720-kms-kunit-actions-rework-v2-11-175017bd56ab@kernel.org \
    --to=mripard@kernel.org \
    --cc=airlied@gmail.com \
    --cc=brendan.higgins@linux.dev \
    --cc=daniel@ffwll.ch \
    --cc=davidgow@google.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=emma@anholt.net \
    --cc=javierm@redhat.com \
    --cc=kunit-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mairacanal@riseup.net \
    --cc=tzimmermann@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.