Tiny fixes - resent

Tiny fixes - resent

[Mario Kleiner]

Hi, a resend of updated versions of the earlier patches:

Patch 1 should really get into Linux 4.1 to avoid tegra breaking
user-space clients.

Patch 2 reviewed-by Michel, updated to take his feedback into account.

Patch 3 is modified to not conflict with Daniel Vetter's patch
"drm/vblank: Fixup and document timestamp update/read barriers"

Patch 4 will be needed for drm/qxl to compile with Daniel's fixup
patch applied.

thanks,
-mario

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

[PATCH 1/5] drm/tegra: Don't use vblank_disable_immediate on incapable driver.

[Mario Kleiner]

Tegra would not only need a hardware vblank counter that
increments at leading edge of vblank, but also support
for instantaneous high precision vblank timestamp queries, ie.
a proper implementation of dev->driver->get_vblank_timestamp().

Without these, there can be off-by-one errors during vblank
disable/enable if the scanout is inside vblank at en/disable
time, and additionally clients will never see any useable
vblank timestamps when querying via drmWaitVblank ioctl. This
would negatively affect swap scheduling under X11 and Wayland.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/tegra/drm.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 1833abd..bfad15a 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -173,7 +173,6 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)
 	drm->irq_enabled = true;
 
 	/* syncpoints are used for full 32-bit hardware VBLANK counters */
-	drm->vblank_disable_immediate = true;
 	drm->max_vblank_count = 0xffffffff;
 
 	err = drm_vblank_init(drm, drm->mode_config.num_crtc);
-- 
1.9.1

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

[PATCH 2/5] drm: Prevent invalid use of vblank_disable_immediate. (v2)

[Mario Kleiner]

For a kms driver to support immediate disable of vblank
irq's reliably without introducing off by one errors or
other mayhem for clients, it must not only support a
hardware vblank counter query, but also high precision
vblank timestamping, so vblank count and timestamp can be
instantaneously reinitialzed to valid values. Additionally
the exposed hardware counter must behave as if it is
incrementing at leading edge of vblank to avoid off by
one errors during reinitialization of the counter while
the display happens to be inside or close to vblank.

Check during drm_vblank_init that a driver which claims to
be capable of vblank_disable_immediate at least supports
high precision timestamping and prevent use of instant
disable if that isn't present as a minimum requirement.

v2: Changed from DRM_ERROR to DRM_INFO and made message
    more clear, as suggested by Michel Dänzer.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Reviewed-by: Michel Dänzer <michel.daenzer@amd.com>

Cc: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_irq.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 9c166b4..152d1de 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -352,6 +352,13 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs)
 	else
 		DRM_INFO("No driver support for vblank timestamp query.\n");
 
+	/* Must have precise timestamping for reliable vblank instant disable */
+	if (dev->vblank_disable_immediate && !dev->driver->get_vblank_timestamp) {
+		dev->vblank_disable_immediate = false;
+		DRM_INFO("Setting vblank_disable_immediate to false because "
+			 "get_vblank_timestamp == NULL\n");
+	}
+
 	dev->vblank_disable_allowed = false;
 
 	return 0;
-- 
1.9.1

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

[PATCH 3/5] drm: Zero out invalid vblank timestamp in drm_update_vblank_count. (v2)

[Mario Kleiner]

Since commit 844b03f27739135fe1fed2fef06da0ffc4c7a081 we make
sure that after vblank irq off, we return the last valid
(vblank count, vblank timestamp) pair to clients, e.g., during
modesets, which is good.

An overlooked side effect of that commit for kms drivers without
support for precise vblank timestamping is that at vblank irq
enable, when we update the vblank counter from the hw counter, we
can't update the corresponding vblank timestamp, so now we have a
totally mismatched timestamp for the new count to confuse clients.

Restore old client visible behaviour from before Linux 3.18, but
zero out the timestamp at vblank counter update (instead of disable
as in original implementation) if we can't generate a meaningful
timestamp immediately for the new vblank counter. This will fix
this regression, so callers know they need to retry again later
if they need a valid timestamp, but at the same time preserves
the improvements made in the commit mentioned above.

v2: Rebased on top of Daniel Vetter's fixup and documentation
    patch for timestamp updates. Drop request for stable kernel
    backport as this would be more difficult, unless the original
    patch would get applied to stable kernels.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_irq.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 152d1de..44e6a20b 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -161,10 +161,13 @@ static void drm_update_vblank_count(struct drm_device *dev, int crtc)
 
 	/*
 	 * Only reinitialize corresponding vblank timestamp if high-precision query
-	 * available and didn't fail. Will reinitialize delayed at next vblank
-	 * interrupt in that case.
+	 * available and didn't fail. Otherwise reinitialize delayed at next vblank
+	 * interrupt and assign 0 for now, to mark the vblanktimestamp as invalid.
 	 */
-	store_vblank(dev, crtc, diff, rc ? &t_vblank : NULL);
+	if (!rc)
+		t_vblank = (struct timeval) {0, 0};
+
+	store_vblank(dev, crtc, diff, &t_vblank);
 }
 
 /*
-- 
1.9.1

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

[PATCH 4/5] drm/qxl: Fix qxl_noop_get_vblank_counter()

[Mario Kleiner]

This breaks under the vblank timestamp cleanup patch
by Daniel Vetter. Also it is pointless to return anything
but zero (or any other constant) if the function doesn't
actually query a hw vblank counter. The bogus return of
the current drm vblank counter via direct readout or via
drm_vblank_count() is found in many of the new kms drivers,
but it does exactly nothing different from returning any
arbitrary constant - it's a no operation.

Let's simply return 0 - Easy and fast.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 1d9b80c..577dc45 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -198,7 +198,7 @@ static int qxl_pm_restore(struct device *dev)
 
 static u32 qxl_noop_get_vblank_counter(struct drm_device *dev, int crtc)
 {
-	return dev->vblank[crtc].count.counter;
+	return 0;
 }
 
 static int qxl_noop_enable_vblank(struct drm_device *dev, int crtc)
-- 
1.9.1

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

Re: Tiny fixes - resent

[Daniel Vetter]

On Mon, May 04, 2015 at 06:29:43AM +0200, Mario Kleiner wrote:
> Hi, a resend of updated versions of the earlier patches:
> 
> Patch 1 should really get into Linux 4.1 to avoid tegra breaking
> user-space clients.
> 
> Patch 2 reviewed-by Michel, updated to take his feedback into account.
> 
> Patch 3 is modified to not conflict with Daniel Vetter's patch
> "drm/vblank: Fixup and document timestamp update/read barriers"
> 
> Patch 4 will be needed for drm/qxl to compile with Daniel's fixup
> patch applied.

Merged patch 2-4 to topic/drm-misc on top of my memory barrier patch. I'll
leave 1 to Thierry for tegra-fixes. And there doesn't seem to be a patch 5
somehow, was that intentional? The patches are numbered n/5.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Tiny fixes - resent

[Mario Kleiner]

On 05/04/2015 11:15 AM, Daniel Vetter wrote:
> On Mon, May 04, 2015 at 06:29:43AM +0200, Mario Kleiner wrote:
>> Hi, a resend of updated versions of the earlier patches:
>>
>> Patch 1 should really get into Linux 4.1 to avoid tegra breaking
>> user-space clients.
>>
>> Patch 2 reviewed-by Michel, updated to take his feedback into account.
>>
>> Patch 3 is modified to not conflict with Daniel Vetter's patch
>> "drm/vblank: Fixup and document timestamp update/read barriers"
>>
>> Patch 4 will be needed for drm/qxl to compile with Daniel's fixup
>> patch applied.
>
> Merged patch 2-4 to topic/drm-misc on top of my memory barrier patch. I'll
> leave 1 to Thierry for tegra-fixes. And there doesn't seem to be a patch 5
> somehow, was that intentional? The patches are numbered n/5.
> -Daniel
>

Thanks. There isn't a patch 5, that was just some hack of mine that 
accidentally slipped into the numbering.

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