Some more VC4 vblank fixes and timestamping improvements.

Some more VC4 vblank fixes and timestamping improvements.

[Mario Kleiner]

Hi Eric and all,

this batch prevents setting modes one shouldn't set,
adds precise vblank timestamping for interlaced video
modes, and one fix for vblank en/disable during crtc
en/disable.

All successfully tested, also with timing measurement equipment,
on the RPi 2B.

Thanks,
-mario

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

[PATCH 1/5] drm/vc4: Disallow interlaced modes on DPI.

[Mario Kleiner]

We already don't expose such modes to userspace, but make
sure userspace can't sneak some interlaced mode in.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_dpi.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_dpi.c b/drivers/gpu/drm/vc4/vc4_dpi.c
index 275fedb..1e1f6b8 100644
--- a/drivers/gpu/drm/vc4/vc4_dpi.c
+++ b/drivers/gpu/drm/vc4/vc4_dpi.c
@@ -340,9 +340,20 @@ static void vc4_dpi_encoder_enable(struct drm_encoder *encoder)
 	}
 }
 
+static bool vc4_dpi_encoder_mode_fixup(struct drm_encoder *encoder,
+				       const struct drm_display_mode *mode,
+				       struct drm_display_mode *adjusted_mode)
+{
+	if (adjusted_mode->flags & DRM_MODE_FLAG_INTERLACE)
+		return false;
+
+	return true;
+}
+
 static const struct drm_encoder_helper_funcs vc4_dpi_encoder_helper_funcs = {
 	.disable = vc4_dpi_encoder_disable,
 	.enable = vc4_dpi_encoder_enable,
+	.mode_fixup = vc4_dpi_encoder_mode_fixup,
 };
 
 static const struct of_device_id vc4_dpi_dt_match[] = {
-- 
2.7.0

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

[PATCH 2/5] drm/vc4: Fix handling of interlaced video modes.

[Mario Kleiner]

We must not apply CRTC_INTERLACE_HALVE_V to interlaced modes during
mode enumeration, as drm_helper_probe_single_connector_modes
does, so wrap it and reset the effect of CRTC_INTERLACE_HALVE_V
on affected interlaced modes.

Also mode_fixup interlaced modes passed in from user space.

This fixes the vblank timestamping constants and entries in
the mode->crtc_xxx fields needed for precise vblank timestamping.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 18 ++++++++++++++++++
 drivers/gpu/drm/vc4/vc4_hdmi.c | 29 +++++++++++++++++++++++++++--
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 8fc2b73..a479d3d 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -532,6 +532,23 @@ static void vc4_crtc_enable(struct drm_crtc *crtc)
 		   CRTC_READ(PV_V_CONTROL) | PV_VCONTROL_VIDEN);
 }
 
+static bool vc4_crtc_mode_fixup(struct drm_crtc *crtc,
+				const struct drm_display_mode *mode,
+				struct drm_display_mode *adjusted_mode)
+{
+	/*
+	 * Interlaced video modes got CRTC_INTERLACE_HALVE_V applied when
+	 * coming from user space. We don't want this, as it screws up
+	 * vblank timestamping, so fix it up.
+	 */
+	drm_mode_set_crtcinfo(adjusted_mode, 0);
+
+	DRM_DEBUG_KMS("[CRTC:%d] adjusted_mode :\n", crtc->base.id);
+	drm_mode_debug_printmodeline(adjusted_mode);
+
+	return true;
+}
+
 static int vc4_crtc_atomic_check(struct drm_crtc *crtc,
 				 struct drm_crtc_state *state)
 {
@@ -819,6 +836,7 @@ static const struct drm_crtc_helper_funcs vc4_crtc_helper_funcs = {
 	.mode_set_nofb = vc4_crtc_mode_set_nofb,
 	.disable = vc4_crtc_disable,
 	.enable = vc4_crtc_enable,
+	.mode_fixup = vc4_crtc_mode_fixup,
 	.atomic_check = vc4_crtc_atomic_check,
 	.atomic_flush = vc4_crtc_atomic_flush,
 };
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 4452f36..68ad106 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -208,10 +208,35 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
 	return ret;
 }
 
+/*
+ * drm_helper_probe_single_connector_modes() applies drm_mode_set_crtcinfo to
+ * all modes with flag CRTC_INTERLACE_HALVE_V. We don't want this, as it
+ * screws up vblank timestamping for interlaced modes, so fix it up.
+ */
+static int vc4_hdmi_connector_probe_modes(struct drm_connector *connector,
+					  uint32_t maxX, uint32_t maxY)
+{
+	struct drm_display_mode *mode;
+	int count;
+
+	count = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
+	if (count == 0)
+		return 0;
+
+	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] probed adapted modes :\n",
+		      connector->base.id, connector->name);
+	list_for_each_entry(mode, &connector->modes, head) {
+		drm_mode_set_crtcinfo(mode, 0);
+		drm_mode_debug_printmodeline(mode);
+	}
+
+	return count;
+}
+
 static const struct drm_connector_funcs vc4_hdmi_connector_funcs = {
 	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = vc4_hdmi_connector_detect,
-	.fill_modes = drm_helper_probe_single_connector_modes,
+	.fill_modes = vc4_hdmi_connector_probe_modes,
 	.destroy = vc4_hdmi_connector_destroy,
 	.reset = drm_atomic_helper_connector_reset,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
@@ -246,7 +271,7 @@ static struct drm_connector *vc4_hdmi_connector_init(struct drm_device *dev,
 	connector->polled = (DRM_CONNECTOR_POLL_CONNECT |
 			     DRM_CONNECTOR_POLL_DISCONNECT);
 
-	connector->interlace_allowed = 0;
+	connector->interlace_allowed = 1;
 	connector->doublescan_allowed = 0;
 
 	drm_mode_connector_attach_encoder(connector, encoder);
-- 
2.7.0

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

[PATCH 3/5] drm/vc4: Reject doublescan modes.

[Mario Kleiner]

We can't handle doublescan modes at the moment, so if
userspace tries to set one, reject the mode set.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index a479d3d..2bfa247 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -536,6 +536,13 @@ static bool vc4_crtc_mode_fixup(struct drm_crtc *crtc,
 				const struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
 {
+	/* Do not allow doublescan modes from user space */
+	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLSCAN) {
+		DRM_DEBUG_KMS("[CRTC:%d] Doublescan mode rejected.\n",
+			      crtc->base.id);
+		return false;
+	}
+
 	/*
 	 * Interlaced video modes got CRTC_INTERLACE_HALVE_V applied when
 	 * coming from user space. We don't want this, as it screws up
-- 
2.7.0

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

[PATCH 4/5] drm/vc4: Enable precise vblank timestamping for interlaced modes.

[Mario Kleiner]

On top of the interlaced video mode fix and with some additional
adjustments, this now works well. It has almost the same accuracy
as on regular progressive scan modes.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 2bfa247..7ffdad5 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -163,14 +163,6 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
 	int vblank_lines;
 	int ret = 0;
 
-	/*
-	 * XXX Doesn't work well in interlaced mode yet, partially due
-	 * to problems in vc4 kms or drm core interlaced mode handling,
-	 * so disable for now in interlaced mode.
-	 */
-	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
-		return ret;
-
 	/* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
 
 	/* Get optional system timestamp before query. */
@@ -191,10 +183,15 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
 
 	/* Vertical position of hvs composed scanline. */
 	*vpos = VC4_GET_FIELD(val, SCALER_DISPSTATX_LINE);
+	*hpos = 0;
+
+	if (mode->flags & DRM_MODE_FLAG_INTERLACE) {
+		*vpos /= 2;
 
-	/* No hpos info available. */
-	if (hpos)
-		*hpos = 0;
+		/* Use hpos to correct for field offset in interlaced mode. */
+		if (VC4_GET_FIELD(val, SCALER_DISPSTATX_FRAME_COUNT) % 2)
+			*hpos += mode->crtc_htotal / 2;
+	}
 
 	/* This is the offset we need for translating hvs -> pv scanout pos. */
 	fifo_lines = vc4_crtc->cob_size / mode->crtc_hdisplay;
@@ -217,8 +214,6 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
 		 * position of the PV.
 		 */
 		*vpos -= fifo_lines + 1;
-		if (mode->flags & DRM_MODE_FLAG_INTERLACE)
-			*vpos /= 2;
 
 		ret |= DRM_SCANOUTPOS_ACCURATE;
 		return ret;
-- 
2.7.0

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

[PATCH 5/5] drm/vc4: Enable/Disable vblanks properly in crtc en/disable.

[Mario Kleiner]

Add missing drm_crtc_vblank_on/off() calls so vblank irq
handling/updating/timestamping never runs with a crtc shut down
or during its shutdown/startup, as that causes large jumps in
vblank count and trouble for compositors.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/vc4/vc4_crtc.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 7ffdad5..2682f07 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -475,6 +475,9 @@ static void vc4_crtc_disable(struct drm_crtc *crtc)
 	int ret;
 	require_hvs_enabled(dev);
 
+	/* Disable vblank irq handling before crtc is disabled. */
+	drm_crtc_vblank_off(crtc);
+
 	CRTC_WRITE(PV_V_CONTROL,
 		   CRTC_READ(PV_V_CONTROL) & ~PV_VCONTROL_VIDEN);
 	ret = wait_for(!(CRTC_READ(PV_V_CONTROL) & PV_VCONTROL_VIDEN), 1);
@@ -525,6 +528,9 @@ static void vc4_crtc_enable(struct drm_crtc *crtc)
 	/* Turn on the pixel valve, which will emit the vstart signal. */
 	CRTC_WRITE(PV_V_CONTROL,
 		   CRTC_READ(PV_V_CONTROL) | PV_VCONTROL_VIDEN);
+
+	/* Enable vblank irq handling after crtc is started. */
+	drm_crtc_vblank_on(crtc);
 }
 
 static bool vc4_crtc_mode_fixup(struct drm_crtc *crtc,
-- 
2.7.0

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

Re: Some more VC4 vblank fixes and timestamping improvements.

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 524 bytes --]

Mario Kleiner <mario.kleiner.de@gmail.com> writes:

> Hi Eric and all,
>
> this batch prevents setting modes one shouldn't set,
> adds precise vblank timestamping for interlaced video
> modes, and one fix for vblank en/disable during crtc
> en/disable.
>
> All successfully tested, also with timing measurement equipment,
> on the RPi 2B.

This all seems pretty reasonable to me.  My question for modesetting
gurus would be: Is mode_fixup the place to be filtering out
unsupportable modes from the user (or their displays)?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: Some more VC4 vblank fixes and timestamping improvements.

[Daniel Vetter]

On Thu, Jul 21, 2016 at 12:58:16PM -0700, Eric Anholt wrote:
> Mario Kleiner <mario.kleiner.de@gmail.com> writes:
> 
> > Hi Eric and all,
> >
> > this batch prevents setting modes one shouldn't set,
> > adds precise vblank timestamping for interlaced video
> > modes, and one fix for vblank en/disable during crtc
> > en/disable.
> >
> > All successfully tested, also with timing measurement equipment,
> > on the RPi 2B.
> 
> This all seems pretty reasonable to me.  My question for modesetting
> gurus would be: Is mode_fixup the place to be filtering out
> unsupportable modes from the user (or their displays)?

Yup. And I wanted to forever add some glue so that drivers could share the
same validation logic between mode_fixup (for modesets) and mode_valid
(for probing). But never came up with a helper scaffolding that looked
satisfactory. The icky bit is that mode_fixup wants an adjusted mode, and
we'd need to fake that somehow to reuse the same code in mode_valid. It's
even worse for atomic, where atomic_check (the new mode_fixup) gets an
entire atomic state. But I still think for simpler hw (anything which
doesn't have funny crtc/encoder specific restrictions) we should be able
to create something that just works.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Some more VC4 vblank fixes and timestamping improvements.

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 441 bytes --]

Mario Kleiner <mario.kleiner.de@gmail.com> writes:

> Hi Eric and all,
>
> this batch prevents setting modes one shouldn't set,
> adds precise vblank timestamping for interlaced video
> modes, and one fix for vblank en/disable during crtc
> en/disable.
>
> All successfully tested, also with timing measurement equipment,
> on the RPi 2B.

I finally got around to making new -next branches, and pulled all of
these to drm-vc4-next.  Thanks!

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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