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>,
	Daniel Vetter <daniel@ffwll.ch>,
	Maxime Ripard <maxime.ripard@bootlin.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Eben Upton <eben@raspberrypi.org>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Subject: [PATCH v4 3/4] drm/vc4: Detect and ignore underruns caused by out-of-sync dlists
Date: Wed,  6 Feb 2019 15:49:05 +0100	[thread overview]
Message-ID: <20190206144906.24304-4-paul.kocialkowski@bootlin.com> (raw)
In-Reply-To: <20190206144906.24304-1-paul.kocialkowski@bootlin.com>

When the pipeline is reconfigured with a different mode, changes take
effect immediately for the CRTC and encoder while the HVS takes some
time to switch the active display list. This results in a period of
time where the pipeline is out of sync, that is very likely to cause
an underrun to be reported. Because the underrun is not related to the
new configuration, reporting it to userspace is a false positive.

Since this only concerns the first frame and we have no immediate fix
to avoid this situation, detect and work around it.

A helper is introduced to return whether the enabled display channels
have their display list in sync. This hint is set when unmasking the
underrun interrupt and it is updated in the interrupt itself.

With that, underrun reports that happen when the display list is out of
sync are ignored. The interrupt is kept enabled so that proper underrun
indications can be pick up as soon as the new display list takes over.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/vc4/vc4_drv.h  |  8 ++++++++
 drivers/gpu/drm/vc4/vc4_hvs.c  | 35 ++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/vc4/vc4_regs.h |  2 ++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index ea596791231d..a6ed55da0d10 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -192,6 +192,13 @@ struct vc4_dev {
 	 */
 	atomic_t underrun;
 
+	/* Stores whether the display lists were syncronized when unmasking the
+	 * underrun IRQ. This is used to skip underruns reported when the
+	 * pipeline was reconfigured but the previous display list is still
+	 * active.
+	 */
+	bool underrun_dlist_sync;
+
 	struct work_struct overflow_mem_work;
 
 	int power_refcount;
@@ -792,6 +799,7 @@ 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);
 int vc4_hvs_debugfs_underrun(struct seq_file *m, void *unused);
+bool vc4_hvs_check_dlist_sync(struct drm_device *dev);
 void vc4_hvs_unmask_underrun(struct drm_device *dev);
 void vc4_hvs_mask_underrun(struct drm_device *dev);
 
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index d5bc3bcd3e51..53ba24aed8fd 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -179,6 +179,23 @@ static int vc4_hvs_upload_linear_kernel(struct vc4_hvs *hvs,
 	return 0;
 }
 
+bool vc4_hvs_check_dlist_sync(struct drm_device *dev)
+{
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+	unsigned int i;
+
+	for (i = 0; i < SCALER_CHANNELS_COUNT; i++) {
+		if (!(HVS_READ(SCALER_DISPCTRLX(i)) & SCALER_DISPCTRLX_ENABLE))
+			continue;
+
+		if (HVS_READ(SCALER_DISPLACTX(i)) !=
+		    HVS_READ(SCALER_DISPLISTX(i)))
+			return false;
+	}
+
+	return true;
+}
+
 void vc4_hvs_mask_underrun(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -196,6 +213,9 @@ void vc4_hvs_unmask_underrun(struct drm_device *dev)
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	u32 dispctrl = HVS_READ(SCALER_DISPCTRL);
 
+	/* An underrun will occur when the display lists are out of sync. */
+	vc4->underrun_dlist_sync = vc4_hvs_check_dlist_sync(dev);
+
 	dispctrl |= SCALER_DISPCTRL_DSPEISLUR(0) |
 		    SCALER_DISPCTRL_DSPEISLUR(1) |
 		    SCALER_DISPCTRL_DSPEISLUR(2);
@@ -220,19 +240,30 @@ static irqreturn_t vc4_hvs_irq_handler(int irq, void *data)
 	struct drm_device *dev = data;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	irqreturn_t irqret = IRQ_NONE;
+	bool dlist_sync;
 	u32 status;
 
+	dlist_sync = vc4_hvs_check_dlist_sync(dev);
 	status = HVS_READ(SCALER_DISPSTAT);
 
 	if (status & (SCALER_DISPSTAT_EUFLOW(0) |
 		      SCALER_DISPSTAT_EUFLOW(1) |
 		      SCALER_DISPSTAT_EUFLOW(2))) {
-		vc4_hvs_mask_underrun(dev);
-		vc4_hvs_report_underrun(dev);
+		/* An underrun will be reported when the current display list
+		 * was not yet updated by the hardware to the requested one and
+		 * the other elements of the pipeline were already reconfigured.
+		 * Ignore it in that case.
+		 */
+		if (vc4->underrun_dlist_sync && dlist_sync) {
+			vc4_hvs_mask_underrun(dev);
+			vc4_hvs_report_underrun(dev);
+		}
 
 		irqret = IRQ_HANDLED;
 	}
 
+	vc4->underrun_dlist_sync = dlist_sync;
+
 	HVS_WRITE(SCALER_DISPSTAT, SCALER_DISPSTAT_IRQMASK(0) |
 				   SCALER_DISPSTAT_IRQMASK(1) |
 				   SCALER_DISPSTAT_IRQMASK(2));
diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h
index e0834de8410f..c0c5fadaf7e3 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>,
	Eben Upton <eben@raspberrypi.org>,
	David Airlie <airlied@linux.ie>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: [PATCH v4 3/4] drm/vc4: Detect and ignore underruns caused by out-of-sync dlists
Date: Wed,  6 Feb 2019 15:49:05 +0100	[thread overview]
Message-ID: <20190206144906.24304-4-paul.kocialkowski@bootlin.com> (raw)
In-Reply-To: <20190206144906.24304-1-paul.kocialkowski@bootlin.com>

When the pipeline is reconfigured with a different mode, changes take
effect immediately for the CRTC and encoder while the HVS takes some
time to switch the active display list. This results in a period of
time where the pipeline is out of sync, that is very likely to cause
an underrun to be reported. Because the underrun is not related to the
new configuration, reporting it to userspace is a false positive.

Since this only concerns the first frame and we have no immediate fix
to avoid this situation, detect and work around it.

A helper is introduced to return whether the enabled display channels
have their display list in sync. This hint is set when unmasking the
underrun interrupt and it is updated in the interrupt itself.

With that, underrun reports that happen when the display list is out of
sync are ignored. The interrupt is kept enabled so that proper underrun
indications can be pick up as soon as the new display list takes over.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
---
 drivers/gpu/drm/vc4/vc4_drv.h  |  8 ++++++++
 drivers/gpu/drm/vc4/vc4_hvs.c  | 35 ++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/vc4/vc4_regs.h |  2 ++
 3 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index ea596791231d..a6ed55da0d10 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -192,6 +192,13 @@ struct vc4_dev {
 	 */
 	atomic_t underrun;
 
+	/* Stores whether the display lists were syncronized when unmasking the
+	 * underrun IRQ. This is used to skip underruns reported when the
+	 * pipeline was reconfigured but the previous display list is still
+	 * active.
+	 */
+	bool underrun_dlist_sync;
+
 	struct work_struct overflow_mem_work;
 
 	int power_refcount;
@@ -792,6 +799,7 @@ 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);
 int vc4_hvs_debugfs_underrun(struct seq_file *m, void *unused);
+bool vc4_hvs_check_dlist_sync(struct drm_device *dev);
 void vc4_hvs_unmask_underrun(struct drm_device *dev);
 void vc4_hvs_mask_underrun(struct drm_device *dev);
 
diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
index d5bc3bcd3e51..53ba24aed8fd 100644
--- a/drivers/gpu/drm/vc4/vc4_hvs.c
+++ b/drivers/gpu/drm/vc4/vc4_hvs.c
@@ -179,6 +179,23 @@ static int vc4_hvs_upload_linear_kernel(struct vc4_hvs *hvs,
 	return 0;
 }
 
+bool vc4_hvs_check_dlist_sync(struct drm_device *dev)
+{
+	struct vc4_dev *vc4 = to_vc4_dev(dev);
+	unsigned int i;
+
+	for (i = 0; i < SCALER_CHANNELS_COUNT; i++) {
+		if (!(HVS_READ(SCALER_DISPCTRLX(i)) & SCALER_DISPCTRLX_ENABLE))
+			continue;
+
+		if (HVS_READ(SCALER_DISPLACTX(i)) !=
+		    HVS_READ(SCALER_DISPLISTX(i)))
+			return false;
+	}
+
+	return true;
+}
+
 void vc4_hvs_mask_underrun(struct drm_device *dev)
 {
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
@@ -196,6 +213,9 @@ void vc4_hvs_unmask_underrun(struct drm_device *dev)
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	u32 dispctrl = HVS_READ(SCALER_DISPCTRL);
 
+	/* An underrun will occur when the display lists are out of sync. */
+	vc4->underrun_dlist_sync = vc4_hvs_check_dlist_sync(dev);
+
 	dispctrl |= SCALER_DISPCTRL_DSPEISLUR(0) |
 		    SCALER_DISPCTRL_DSPEISLUR(1) |
 		    SCALER_DISPCTRL_DSPEISLUR(2);
@@ -220,19 +240,30 @@ static irqreturn_t vc4_hvs_irq_handler(int irq, void *data)
 	struct drm_device *dev = data;
 	struct vc4_dev *vc4 = to_vc4_dev(dev);
 	irqreturn_t irqret = IRQ_NONE;
+	bool dlist_sync;
 	u32 status;
 
+	dlist_sync = vc4_hvs_check_dlist_sync(dev);
 	status = HVS_READ(SCALER_DISPSTAT);
 
 	if (status & (SCALER_DISPSTAT_EUFLOW(0) |
 		      SCALER_DISPSTAT_EUFLOW(1) |
 		      SCALER_DISPSTAT_EUFLOW(2))) {
-		vc4_hvs_mask_underrun(dev);
-		vc4_hvs_report_underrun(dev);
+		/* An underrun will be reported when the current display list
+		 * was not yet updated by the hardware to the requested one and
+		 * the other elements of the pipeline were already reconfigured.
+		 * Ignore it in that case.
+		 */
+		if (vc4->underrun_dlist_sync && dlist_sync) {
+			vc4_hvs_mask_underrun(dev);
+			vc4_hvs_report_underrun(dev);
+		}
 
 		irqret = IRQ_HANDLED;
 	}
 
+	vc4->underrun_dlist_sync = dlist_sync;
+
 	HVS_WRITE(SCALER_DISPSTAT, SCALER_DISPSTAT_IRQMASK(0) |
 				   SCALER_DISPSTAT_IRQMASK(1) |
 				   SCALER_DISPSTAT_IRQMASK(2));
diff --git a/drivers/gpu/drm/vc4/vc4_regs.h b/drivers/gpu/drm/vc4/vc4_regs.h
index e0834de8410f..c0c5fadaf7e3 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

  parent reply	other threads:[~2019-02-06 14:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-06 14:49 [PATCH v4 0/4] drm/vc4: Add a load tracker Paul Kocialkowski
2019-02-06 14:49 ` [PATCH v4 1/4] drm/vc4: Report HVS underrun errors Paul Kocialkowski
2019-02-06 23:51   ` Eric Anholt
2019-02-06 23:51     ` Eric Anholt
2019-02-06 14:49 ` [PATCH v4 2/4] drm/vc4: Add a load tracker to prevent HVS underflow errors Paul Kocialkowski
2019-02-06 14:49 ` Paul Kocialkowski [this message]
2019-02-06 14:49   ` [PATCH v4 3/4] drm/vc4: Detect and ignore underruns caused by out-of-sync dlists Paul Kocialkowski
2019-02-06 23:51   ` Eric Anholt
2019-02-06 23:51     ` Eric Anholt
2019-02-20 14:30     ` Paul Kocialkowski
2019-02-06 14:49 ` [PATCH v4 4/4] drm/vc4: Add a debugfs entry to disable/enable the load tracker 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=20190206144906.24304-4-paul.kocialkowski@bootlin.com \
    --to=paul.kocialkowski@bootlin.com \
    --cc=airlied@linux.ie \
    --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.