All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
To: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Cc: Eric Anholt <eric@anholt.net>, David Airlie <airlied@linux.ie>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Eben Upton <eben@raspberrypi.org>,
	Daniel Vetter <daniel@ffwll.ch>,
	Boris Brezillon <boris.brezillon@bootlin.com>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Subject: [PATCH v3 1/4] drm/vc4: Wait for display list synchronization when completing commit
Date: Tue,  8 Jan 2019 15:50:53 +0100	[thread overview]
Message-ID: <20190108145056.2276-2-paul.kocialkowski@bootlin.com> (raw)
In-Reply-To: <20190108145056.2276-1-paul.kocialkowski@bootlin.com>

During an atomic commit, the HVS is configured with a display list
for the channel matching the associated CRTC. The Pixel Valve (CRTC)
and encoder are also configured for the new setup at that time.
While the Pixel Valve and encoder are reconfigured synchronously, the
HVS is only reconfigured after the display list address (DISPLIST) has
been updated to the current display list address (DISPLACTX), which is
the responsibility of the hardware.

The time frame during which the HVS is still running on its previous
configuration but the CRTC and encoder have been reconfigured already
can lead to a number of synchronization issues. They will eventually
cause errors reported on the FIFOs, such as underruns.

With underrun detection enabled (from Boris Brezillon's series), this
leads to unreliable underrun detection with random false positives.

To ensure a coherent state, wait for each enabled channel of the HVS
to synchronize its current display list address. This fixes the issue
of random underrun reporting on commits.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/vc4/vc4_drv.h  |  1 +
 drivers/gpu/drm/vc4/vc4_hvs.c  | 17 +++++++++++++++++
 drivers/gpu/drm/vc4/vc4_kms.c  |  2 ++
 drivers/gpu/drm/vc4/vc4_regs.h |  2 ++
 4 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index c24b078f0593..955f157f5ad0 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -772,6 +772,7 @@ void vc4_irq_reset(struct drm_device *dev);
 extern struct platform_driver vc4_hvs_driver;
 void vc4_hvs_dump_state(struct drm_device *dev);
 int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused);
+void vc4_hvs_sync_dlist(struct drm_device *dev);
 
 /* vc4_kms.c */
 int vc4_kms_load(struct drm_device *dev);
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 5d8c749c9749..1ba60b8e0c2d 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -166,6 +166,23 @@ static int vc4_hvs_upload_linear_kernel(struct vc4_hvs *hvs,
 	return 0;
 }
 
+void vc4_hvs_sync_dlist(struct drm_device *dev)
+{
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < SCALER_CHANNELS_COUNT; i++) {
+		if (!(HVS_READ(SCALER_DISPCTRLX(i)) & SCALER_DISPCTRLX_ENABLE))
+			continue;
+
+		ret = wait_for(HVS_READ(SCALER_DISPLACTX(i)) ==
+			       HVS_READ(SCALER_DISPLISTX(i)), 1000);
+		WARN(ret, "Timeout waiting for channel %d display list sync\n",
+		     i);
+	}
+}
+
 static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
 {
 	struct platform_device *pdev = to_platform_device(dev);
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 0490edb192a1..2d66a2b57a91 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -155,6 +155,8 @@ vc4_atomic_complete_commit(struct drm_atomic_state *state)
 
 	drm_atomic_helper_commit_hw_done(state);
 
+	vc4_hvs_sync_dlist(dev);
+
 	drm_atomic_helper_wait_for_flip_done(dev, state);
 
 	drm_atomic_helper_cleanup_planes(dev, state);
diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h
index 931088014272..50c653309aec 100644
--- a/drivers/gpu/drm/vc4/vc4_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_regs.h
@@ -212,6 +212,8 @@
 
 #define PV_HACT_ACT				0x30
 
+#define SCALER_CHANNELS_COUNT			3
+
 #define SCALER_DISPCTRL                         0x00000000
 /* Global register for clock gating the HVS */
 # define SCALER_DISPCTRL_ENABLE			BIT(31)
-- 
2.20.1


WARNING: multiple messages have this Message-ID (diff)
From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
To: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Cc: Maxime Ripard <maxime.ripard@bootlin.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Eben Upton <eben@raspberrypi.org>,
	David Airlie <airlied@linux.ie>,
	Boris Brezillon <boris.brezillon@bootlin.com>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Subject: [PATCH v3 1/4] drm/vc4: Wait for display list synchronization when completing commit
Date: Tue,  8 Jan 2019 15:50:53 +0100	[thread overview]
Message-ID: <20190108145056.2276-2-paul.kocialkowski@bootlin.com> (raw)
In-Reply-To: <20190108145056.2276-1-paul.kocialkowski@bootlin.com>

During an atomic commit, the HVS is configured with a display list
for the channel matching the associated CRTC. The Pixel Valve (CRTC)
and encoder are also configured for the new setup at that time.
While the Pixel Valve and encoder are reconfigured synchronously, the
HVS is only reconfigured after the display list address (DISPLIST) has
been updated to the current display list address (DISPLACTX), which is
the responsibility of the hardware.

The time frame during which the HVS is still running on its previous
configuration but the CRTC and encoder have been reconfigured already
can lead to a number of synchronization issues. They will eventually
cause errors reported on the FIFOs, such as underruns.

With underrun detection enabled (from Boris Brezillon's series), this
leads to unreliable underrun detection with random false positives.

To ensure a coherent state, wait for each enabled channel of the HVS
to synchronize its current display list address. This fixes the issue
of random underrun reporting on commits.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/vc4/vc4_drv.h  |  1 +
 drivers/gpu/drm/vc4/vc4_hvs.c  | 17 +++++++++++++++++
 drivers/gpu/drm/vc4/vc4_kms.c  |  2 ++
 drivers/gpu/drm/vc4/vc4_regs.h |  2 ++
 4 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index c24b078f0593..955f157f5ad0 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -772,6 +772,7 @@ void vc4_irq_reset(struct drm_device *dev);
 extern struct platform_driver vc4_hvs_driver;
 void vc4_hvs_dump_state(struct drm_device *dev);
 int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused);
+void vc4_hvs_sync_dlist(struct drm_device *dev);
 
 /* vc4_kms.c */
 int vc4_kms_load(struct drm_device *dev);
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index 5d8c749c9749..1ba60b8e0c2d 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -166,6 +166,23 @@ static int vc4_hvs_upload_linear_kernel(struct vc4_hvs *hvs,
 	return 0;
 }
 
+void vc4_hvs_sync_dlist(struct drm_device *dev)
+{
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < SCALER_CHANNELS_COUNT; i++) {
+		if (!(HVS_READ(SCALER_DISPCTRLX(i)) & SCALER_DISPCTRLX_ENABLE))
+			continue;
+
+		ret = wait_for(HVS_READ(SCALER_DISPLACTX(i)) ==
+			       HVS_READ(SCALER_DISPLISTX(i)), 1000);
+		WARN(ret, "Timeout waiting for channel %d display list sync\n",
+		     i);
+	}
+}
+
 static int vc4_hvs_bind(struct device *dev, struct device *master, void *data)
 {
 	struct platform_device *pdev = to_platform_device(dev);
diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
index 0490edb192a1..2d66a2b57a91 100644
--- a/drivers/gpu/drm/vc4/vc4_kms.c
+++ b/drivers/gpu/drm/vc4/vc4_kms.c
@@ -155,6 +155,8 @@ vc4_atomic_complete_commit(struct drm_atomic_state *state)
 
 	drm_atomic_helper_commit_hw_done(state);
 
+	vc4_hvs_sync_dlist(dev);
+
 	drm_atomic_helper_wait_for_flip_done(dev, state);
 
 	drm_atomic_helper_cleanup_planes(dev, state);
diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h
index 931088014272..50c653309aec 100644
--- a/drivers/gpu/drm/vc4/vc4_regs.h
+++ b/drivers/gpu/drm/vc4/vc4_regs.h
@@ -212,6 +212,8 @@
 
 #define PV_HACT_ACT				0x30
 
+#define SCALER_CHANNELS_COUNT			3
+
 #define SCALER_DISPCTRL                         0x00000000
 /* Global register for clock gating the HVS */
 # define SCALER_DISPCTRL_ENABLE			BIT(31)
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2019-01-08 14:51 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-08 14:50 [PATCH v3 0/4] drm/vc4: Add a load tracker Paul Kocialkowski
2019-01-08 14:50 ` Paul Kocialkowski
2019-01-08 14:50 ` Paul Kocialkowski [this message]
2019-01-08 14:50   ` [PATCH v3 1/4] drm/vc4: Wait for display list synchronization when completing commit Paul Kocialkowski
2019-01-08 18:21   ` Daniel Vetter
2019-01-08 18:21     ` Daniel Vetter
2019-01-09 16:52     ` Paul Kocialkowski
2019-01-09 20:34       ` Daniel Vetter
2019-01-09 20:34         ` Daniel Vetter
2019-01-23 18:34   ` Eric Anholt
2019-01-23 18:34     ` Eric Anholt
2019-01-25 14:50     ` Paul Kocialkowski
2019-01-25 14:50       ` Paul Kocialkowski
2019-01-25 17:55       ` Eric Anholt
2019-01-25 17:55         ` Eric Anholt
2019-01-08 14:50 ` [PATCH v3 2/4] drm/vc4: Report underrun errors Paul Kocialkowski
2019-01-08 14:50   ` Paul Kocialkowski
2019-01-23 18:47   ` Eric Anholt
2019-01-25 14:43     ` Paul Kocialkowski
2019-01-25 14:43       ` Paul Kocialkowski
2019-01-25 17:52       ` Eric Anholt
2019-01-08 14:50 ` [PATCH v3 3/4] drm/vc4: Add a load tracker to prevent HVS underflow errors Paul Kocialkowski
2019-01-08 14:50   ` Paul Kocialkowski
2019-01-08 14:50 ` [PATCH v3 4/4] drm/vc4: Add a debugfs entry to disable/enable the load tracker Paul Kocialkowski
2019-01-08 14:50   ` Paul Kocialkowski

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=20190108145056.2276-2-paul.kocialkowski@bootlin.com \
    --to=paul.kocialkowski@bootlin.com \
    --cc=airlied@linux.ie \
    --cc=boris.brezillon@bootlin.com \
    --cc=daniel@ffwll.ch \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eben@raspberrypi.org \
    --cc=eric@anholt.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.ripard@bootlin.com \
    --cc=thomas.petazzoni@bootlin.com \
    /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.