[PATCH xf86-video-amdgpu 0/6] xf86-video-amdgpu integration for DRM variable refresh rate API

[PATCH xf86-video-amdgpu 0/6] xf86-video-amdgpu integration for DRM variable refresh rate API

[Nicholas Kazlauskas]

These patches are part of a proposed new interface for supporting variable refresh rate via DRM properties.

https://patchwork.freedesktop.org/series/49486/

When notified of a window that is FreeSync capable via X these patches help track when the window is fullscreen to manage the variable_refresh property on the CRTC.

=== Adaptive sync and variable refresh rate ===

Adaptive sync is part of the DisplayPort spec and allows for graphics adapters to drive displays with varying frame timings.

Variable refresh rate (VRR) is essentially the same, but defined for HDMI.

=== Use cases for variable refresh rate ===

Variable frame (flip) timings don't align well with fixed refresh rate displays. This results in stuttering, tearing and/or input lag. By adjusting the display refresh rate dynamically these issues can be reduced or eliminated.

However, not all content is suitable for dynamic refresh adaptation. Content that is flipped infrequently or at random intervals tends to fair poorly. Multiple clients trying to flip under the same screen can similarly interfere with prediction.

Userland needs a way to let the driver know when the content on the screen is suitable for variable refresh rate and if the user wishes to have the feature enabled.

=== DRM API to support variable refresh rates ===

This patch introduces a new API via atomic properties on the DRM connector and CRTC.

The connector has two new optional properties:

* bool variable_refresh_capable - set by the driver if the hardware is capable of supporting variable refresh tech

* bool variable_refresh_enabled - set by the user to enable variable refresh adjustment over the connector

The CRTC has one additional default property:

* bool variable_refresh - a content hint to the driver specifying that the CRTC contents are suitable for variable refresh adjustment

== Overview for DRM driver developers ===

Driver developers can attach the optional connector properties via drm_connector_attach_variable_refresh_properties on connectors that support variable refresh (typically DP or HDMI).

The variable_refresh_capable property should be managed as the output on the connector changes. The property is read only from userspace.

The variable_refresh_enabled property is intended to be a property controlled by userland as a global on/off switch for variable refresh technology. It should be checked before enabling variable refresh rate.

=== Overview for Userland developers ==

The variable_refresh property on the CRTC should be set to true when the CRTCs are suitable for variable refresh rate. In practice this is probably an application like a game - a single window that covers the whole CRTC surface and is the only client issuing flips.

To demonstrate the suitability of the API for variable refresh and dynamic adaptation there are additional patches using this API that implement adaptive variable refresh across kernel and userland projects:

- DRM (dri-devel)
- amdgpu DRM kernel driver (amd-gfx)
- xf86-video-amdgpu (amd-gfx)
- mesa (mesa-dev)

These patches enable adaptive variable refresh on X for AMD hardware provided that the user sets the variable_refresh_enabled property to true on supported connectors (ie. using xrandr --set-prop).

They have been tested on upstream userland under GNOME/KDE desktop environments under single and multi-monitor setups for a number of GL applications. Most games and benchmarks should work as expected provided that the compositor correctly unredirects the application's surface. KDE seems to have the best support for this with an explicit option to disable tearing support.

Full implementation details for these changes can be reviewed in their respective mailing lists.

=== Previous discussions ===

These patches are based upon feedback from patches and feedback from two previous threads on the subject which are linked below for reference:

https://lists.freedesktop.org/archives/amd-gfx/2018-April/021047.html
https://lists.freedesktop.org/archives/dri-devel/2017-October/155207.html

Nicholas Kazlauskas

Hawking Zhang (5):
  Enable/Disable freesync when enter/exit fullscreen game v2
  Set freesync capability when client has fullscreen size drawable
  Do not fail the X request when there is just BadDrawable/BadMatch
  Handle Alt+Tab case when freesync enabled in steam client v2
  Do not issue freesync ioctl from present unflip

Nicholas Kazlauskas (1):
  Replace amdgpu ioctl with CRTC properties for enabling FreeSync

 src/Makefile.am        |   2 +
 src/amdgpu_dri2.c      |   1 +
 src/amdgpu_drv.h       |  10 ++
 src/amdgpu_extension.c | 210 +++++++++++++++++++++++++++++++++++++++++
 src/amdgpu_extension.h |  53 +++++++++++
 src/amdgpu_kms.c       |  60 ++++++++++++
 src/amdgpu_present.c   |   1 +
 src/drmmode_display.c  |  88 +++++++++++++++++
 src/drmmode_display.h  |   1 +
 9 files changed, 426 insertions(+)
 create mode 100644 src/amdgpu_extension.c
 create mode 100644 src/amdgpu_extension.h

-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu 1/6] Enable/Disable freesync when enter/exit fullscreen game v2

[Nicholas Kazlauskas]

From: Hawking Zhang <Hawking.Zhang@amd.com>

v2:
resolve undefined symbol in xserver 1.16

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
Signed-off-by: Nicholas Kazlauskas <Nicholas.Kazlauskas@amd.com>
---
 src/Makefile.am        |   2 +
 src/amdgpu_dri2.c      |  24 ++++++
 src/amdgpu_drv.h       |   5 ++
 src/amdgpu_extension.c | 176 +++++++++++++++++++++++++++++++++++++++++
 src/amdgpu_extension.h |  52 ++++++++++++
 src/amdgpu_kms.c       |   4 +
 src/amdgpu_present.c   |  14 ++++
 7 files changed, 277 insertions(+)
 create mode 100644 src/amdgpu_extension.c
 create mode 100644 src/amdgpu_extension.h

diff --git a/src/Makefile.am b/src/Makefile.am
index c23c87d..240cb89 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -47,6 +47,7 @@ amdgpu_drv_ladir = @moduledir@/drivers
 amdgpu_drv_la_SOURCES = \
 	amdgpu_video.c \
 	amdgpu_misc.c amdgpu_probe.c \
+	amdgpu_extension.c \
 	$(AMDGPU_KMS_SRCS)
 
 AM_CFLAGS += @LIBGLAMOR_CFLAGS@
@@ -64,6 +65,7 @@ EXTRA_DIST = \
 	amdgpu_drv.h \
 	amdgpu_pixmap.h \
 	amdgpu_probe.h \
+	amdgpu_extension.h \
 	amdgpu_version.h \
 	amdgpu_video.h \
 	simple_list.h \
diff --git a/src/amdgpu_dri2.c b/src/amdgpu_dri2.c
index 96b2d17..8c601c8 100644
--- a/src/amdgpu_dri2.c
+++ b/src/amdgpu_dri2.c
@@ -34,6 +34,7 @@
 #include "amdgpu_glamor.h"
 #include "amdgpu_video.h"
 #include "amdgpu_pixmap.h"
+#include "amdgpu_extension.h"
 
 #ifdef DRI2
 
@@ -527,6 +528,13 @@ amdgpu_dri2_schedule_flip(xf86CrtcPtr crtc, ClientPtr client,
 	xf86DrvMsgVerb(scrn->scrnIndex, X_INFO, AMDGPU_LOGLEVEL_DEBUG,
 		       "%s:%d fevent[%p]\n", __func__, __LINE__, flip_info);
 
+	if (info->freesync_capable_client &&
+	    info->freesync_enabled == FALSE) {
+		AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
+		if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, TRUE))
+			info->freesync_enabled = TRUE;
+	}
+
 	/* Page flip the full screen buffer */
 	back_priv = back->driverPrivate;
 	if (amdgpu_do_pageflip(scrn, client, back_priv->pixmap,
@@ -680,6 +688,8 @@ static void amdgpu_dri2_frame_event_handler(xf86CrtcPtr crtc, uint32_t seq,
 {
 	DRI2FrameEventPtr event = event_data;
 	ScrnInfoPtr scrn = crtc->scrn;
+	AMDGPUInfoPtr info = AMDGPUPTR(scrn);
+	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
 	DrawablePtr drawable;
 	int status;
 	int swap_type;
@@ -710,6 +720,12 @@ static void amdgpu_dri2_frame_event_handler(xf86CrtcPtr crtc, uint32_t seq,
 		}
 		/* else fall through to exchange/blit */
 	case DRI2_SWAP:
+		if (info->freesync_capable_client &&
+		    info->freesync_enabled == TRUE) {
+			if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
+				info->freesync_enabled = FALSE;
+		}
+
 		if (DRI2CanExchange(drawable) &&
 		    can_exchange(scrn, drawable, event->front, event->back)) {
 			amdgpu_dri2_exchange_buffers(drawable, event->front,
@@ -1076,6 +1092,8 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
 {
 	ScreenPtr screen = draw->pScreen;
 	ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
+	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
+	AMDGPUInfoPtr info = AMDGPUPTR(scrn);
 	xf86CrtcPtr crtc = amdgpu_dri2_drawable_crtc(draw, TRUE);
 	uint32_t msc_delta;
 	drmVBlankSeqType type;
@@ -1248,6 +1266,12 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
 	return TRUE;
 
 blit_fallback:
+	if (info->freesync_capable_client &&
+	    info->freesync_enabled == TRUE) {
+		if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
+			info->freesync_enabled = FALSE;
+	}
+
 	if (swap_info) {
 		swap_info->type = DRI2_SWAP;
 		amdgpu_dri2_schedule_event(FALLBACK_SWAP_DELAY, swap_info);
diff --git a/src/amdgpu_drv.h b/src/amdgpu_drv.h
index 45bc394..91bb829 100644
--- a/src/amdgpu_drv.h
+++ b/src/amdgpu_drv.h
@@ -303,6 +303,11 @@ typedef struct {
 	/* kms pageflipping */
 	Bool allowPageFlip;
 
+	/* freesync */
+	ClientPtr freesync_capable_client;
+	uint32_t client_resource_id;
+	Bool freesync_enabled;
+
 	/* cursor size */
 	int cursor_w;
 	int cursor_h;
diff --git a/src/amdgpu_extension.c b/src/amdgpu_extension.c
new file mode 100644
index 0000000..1c984df
--- /dev/null
+++ b/src/amdgpu_extension.c
@@ -0,0 +1,176 @@
+/*
+ * Copyright © 2016 Advanced Micro Devices, Inc.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "xf86.h"
+#include "xf86Crtc.h"
+#include "xf86drm.h"
+#include "dix.h"
+#include "dixstruct.h"
+#include "extnsionst.h"
+#include "resource.h"
+#include "scrnintstr.h"
+
+#include "amdgpu_drm.h"
+#include "amdgpu_extension.h"
+#include "amdgpu_drv.h"
+
+static RESTYPE RT_AMDGPUCLIENT;
+
+static int ProcAMDGPUFreesyncCapability(ClientPtr client)
+{
+	REQUEST(xAMDGPUFreesyncCapabilityReq);
+	REQUEST_SIZE_MATCH(xAMDGPUFreesyncCapabilityReq);
+
+	if (stuff->screen >= screenInfo.numScreens) {
+		client->errorValue = stuff->screen;
+		return BadValue;
+	}
+
+	ScreenPtr pScreen = screenInfo.screens[stuff->screen];
+	ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
+	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
+	XID cliResId;
+
+	if (info->freesync_capable_client == NULL) {
+		info->freesync_capable_client = client;
+		cliResId = FakeClientID(client->index);
+		if (AddResource(cliResId, RT_AMDGPUCLIENT, (void *)pScrn))
+			info->client_resource_id = cliResId;
+	}
+
+	return client->noClientException;
+}
+
+static int AMDGPUDispatch(ClientPtr client)
+{
+	REQUEST(xReq);
+
+	switch(stuff->data) {
+	case X_AMDGPUFreesyncCapability:
+		return ProcAMDGPUFreesyncCapability(client);
+	default:
+		return BadRequest;
+	}
+}
+
+static int AMDGPUSwapDispatch(ClientPtr client __attribute__((unused)))
+{
+	return BadRequest;
+}
+
+static void AMDGPUResetExtension(ExtensionEntry *extEntry __attribute__((unused)))
+{
+	RT_AMDGPUCLIENT = RT_NONE;
+}
+
+static void AMDGPUExtensionInit (void)
+{
+	ExtensionEntry *extEntry = NULL;
+
+	extEntry = AddExtension(AMDGPU_EXTENSION_NAME,
+				AMDGPU_EXTENSION_EVENTS,
+				AMDGPU_EXTENSION_ERRORS,
+				AMDGPUDispatch,
+				AMDGPUSwapDispatch,
+				AMDGPUResetExtension,
+				StandardMinorOpcode);
+	if (!extEntry) {
+		ErrorF("AddExtension failed\n");
+		return;
+	}
+
+	RT_AMDGPUCLIENT = CreateNewResourceType(AMDGPUClientGone, "AMDGPUClient");
+}
+
+static ExtensionModule AMDGPU_Ext = {
+	AMDGPUExtensionInit,
+	AMDGPU_EXTENSION_NAME,
+	NULL
+};
+
+void AMDGPUExtensionSetup(void)
+{
+#if XORG_VERSION_CURRENT < XORG_VERSION_NUMERIC(1,15,99,904,0)
+	LoadExtension(&AMDGPU_Ext, FALSE);
+#else
+	int size = 1;
+	LoadExtensionList(&AMDGPU_Ext, size, FALSE);
+#endif
+}
+
+int AMDGPUFreesyncControl(int fd, Bool enable_freesync)
+{
+	int ret = -1;
+	struct drm_amdgpu_freesync args;
+
+	memset(&args, 0, sizeof(args));
+	if (enable_freesync) {
+		args.op = AMDGPU_FREESYNC_FULLSCREEN_ENTER;
+                ret = drmCommandWriteRead(fd, DRM_AMDGPU_FREESYNC,
+                                          &args, sizeof(args));
+                if (ret)
+                        ErrorF("Fail to enable freesync mode.\n");
+
+	} else {
+		args.op = AMDGPU_FREESYNC_FULLSCREEN_EXIT;
+		ret = drmCommandWriteRead(fd, DRM_AMDGPU_FREESYNC,
+					  &args, sizeof(args));
+		if (ret)
+			ErrorF("Fail to disable freesync mode.\n");
+	}
+
+	return ret;
+}
+
+void AMDGPUFreeResourceByType(ScreenPtr pScreen)
+{
+	ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
+	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
+
+	if (info->client_resource_id)
+		FreeResourceByType(info->client_resource_id,
+				   RT_AMDGPUCLIENT, FALSE);
+	return;
+}
+
+int AMDGPUClientGone(void *data, XID id)
+{
+	ScrnInfoPtr pScrn = (ScrnInfoPtr)data;
+	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
+	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
+
+	if (id == (XID)info->client_resource_id) {
+		info->client_resource_id = None;
+		info->freesync_capable_client = NULL;
+	}
+
+	if (info->freesync_enabled == TRUE) {
+		if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
+			info->freesync_enabled = FALSE;
+	}
+
+        return 1;
+}
diff --git a/src/amdgpu_extension.h b/src/amdgpu_extension.h
new file mode 100644
index 0000000..87f479f
--- /dev/null
+++ b/src/amdgpu_extension.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright © 2016 Advanced Micro Devices, Inc.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+#ifndef _AMDGPU_EXTENSION_H_
+#define _AMDGPU_EXTENSION_H_
+
+#include "screenint.h"
+
+#define AMDGPU_EXTENSION_NAME	"AMDGPU"
+#define AMDGPU_EXTENSION_EVENTS	1
+#define AMDGPU_EXTENSION_ERRORS 0
+
+#ifndef	XREQ_SZ
+#define XREQ_SZ(name)	sizeof(x##name##Req)
+#endif
+
+#define X_AMDGPUFreesyncCapability	0
+
+/* Requests must be mulitple of 4 bytes */
+typedef struct _AMDGPUFreesyncCapabilityReq {
+	CARD8	reqType;
+	CARD8	amdgpuReqType;
+	CARD16	length	B16;
+	CARD32	screen	B32;
+	CARD32	drawable B32;
+} xAMDGPUFreesyncCapabilityReq;
+
+#define sz_xAMDGPUFreesyncCapabilityReq		XREQ_SZ(AMDGPUFreesyncCapability)
+
+extern void AMDGPUExtensionSetup(void);
+extern int AMDGPUFreesyncControl(int fd, Bool enable_freesync);
+extern void AMDGPUFreeResourceByType(ScreenPtr pScreen);
+extern int AMDGPUClientGone(void *data, XID id);
+#endif /* _AMDGPU_EXTENSION_H_ */
diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index 6017c91..dc98473 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -59,6 +59,7 @@
 
 #include "amdgpu_bo_helper.h"
 #include "amdgpu_pixmap.h"
+#include "amdgpu_extension.h"
 
 #include <gbm.h>
 
@@ -1885,6 +1886,9 @@ Bool AMDGPUScreenInit_KMS(ScreenPtr pScreen, int argc, char **argv)
 		xf86DrvMsg(pScrn->scrnIndex, X_INFO, "2D and 3D acceleration disabled\n");
 	}
 
+	/* AMDGPU extension setup */
+	AMDGPUExtensionSetup();
+
 	/* Init DPMS */
 	xf86DrvMsgVerb(pScrn->scrnIndex, X_INFO, AMDGPU_LOGLEVEL_DEBUG,
 		       "Initializing DPMS\n");
diff --git a/src/amdgpu_present.c b/src/amdgpu_present.c
index eabe0f3..8b70b38 100644
--- a/src/amdgpu_present.c
+++ b/src/amdgpu_present.c
@@ -45,6 +45,7 @@
 #include "amdgpu_glamor.h"
 #include "amdgpu_pixmap.h"
 #include "amdgpu_video.h"
+#include "amdgpu_extension.h"
 
 #include "present.h"
 
@@ -321,6 +322,13 @@ amdgpu_present_flip(RRCrtcPtr crtc, uint64_t event_id, uint64_t target_msc,
 	if (!amdgpu_present_check_flip(crtc, screen->root, pixmap, sync_flip))
 		return ret;
 
+	if (info->freesync_capable_client &&
+	    info->freesync_enabled == FALSE) {
+		AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
+		if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, TRUE))
+			info->freesync_enabled = TRUE;
+	}
+
 	event = calloc(1, sizeof(struct amdgpu_present_vblank_event));
 	if (!event)
 		return ret;
@@ -362,6 +370,12 @@ amdgpu_present_unflip(ScreenPtr screen, uint64_t event_id)
 	if (!amdgpu_present_check_unflip(scrn))
 		goto modeset;
 
+	if (info->freesync_capable_client &&
+		info->freesync_enabled == TRUE) {
+				if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
+			info->freesync_enabled = FALSE;
+		}
+
 	event = calloc(1, sizeof(struct amdgpu_present_vblank_event));
 	if (!event) {
 		ErrorF("%s: calloc failed, display might freeze\n", __func__);
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu 2/6] Set freesync capability when client has fullscreen size drawable

[Nicholas Kazlauskas]

From: Hawking Zhang <Hawking.Zhang@amd.com>

OGL send freesync request to ddx driver when it makes a drawable as current
DDX driver only set the client to be freesync capable when it is a fullscreen
size one.

Change-Id: Ie25ff11f58104546c52a253d6a5f85aa62532d4d
Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
Reviewed-by: Flora Cui <Flora.Cui@amd.com>
---
 src/amdgpu_extension.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/amdgpu_extension.c b/src/amdgpu_extension.c
index 1c984df..eadb742 100644
--- a/src/amdgpu_extension.c
+++ b/src/amdgpu_extension.c
@@ -32,6 +32,8 @@
 #include "extnsionst.h"
 #include "resource.h"
 #include "scrnintstr.h"
+#include "dixaccess.h"
+#include "pixmap.h"
 
 #include "amdgpu_drm.h"
 #include "amdgpu_extension.h"
@@ -53,8 +55,16 @@ static int ProcAMDGPUFreesyncCapability(ClientPtr client)
 	ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
 	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
 	XID cliResId;
+	DrawablePtr pDrawable = NULL;
+	if (dixLookupDrawable(&pDrawable, (Drawable)stuff->drawable,
+			      client, 0, DixReadAccess))
+		return BadValue;
 
-	if (info->freesync_capable_client == NULL) {
+	if (info->freesync_capable_client == NULL &&
+	    pDrawable->x == pDrawable->pScreen->x &&
+	    pDrawable->y == pDrawable->pScreen->y &&
+	    pDrawable->width == pDrawable->pScreen->width &&
+	    pDrawable->height == pDrawable->pScreen->height) {
 		info->freesync_capable_client = client;
 		cliResId = FakeClientID(client->index);
 		if (AddResource(cliResId, RT_AMDGPUCLIENT, (void *)pScrn))
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu 3/6] Do not fail the X request when there is just BadDrawable/BadMatch

[Nicholas Kazlauskas]

From: Hawking Zhang <Hawking.Zhang@amd.com>

There is BadDrawable/BadMatch case for dixLookupDrawable. But DDX driver don't
need to fail the request with BadValue. Instead, only make sure the drawable is
successfully found and check its size

Change-Id: I1ca6e04d611b2d5e81a54e500c90fb1644675f67
Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
Reviewed-by: Qiang Yu <Qiang.Yu@amd.com>
---
 src/amdgpu_extension.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/amdgpu_extension.c b/src/amdgpu_extension.c
index eadb742..ab7d6e3 100644
--- a/src/amdgpu_extension.c
+++ b/src/amdgpu_extension.c
@@ -56,11 +56,13 @@ static int ProcAMDGPUFreesyncCapability(ClientPtr client)
 	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
 	XID cliResId;
 	DrawablePtr pDrawable = NULL;
-	if (dixLookupDrawable(&pDrawable, (Drawable)stuff->drawable,
-			      client, 0, DixReadAccess))
-		return BadValue;
+	int ret = -1;
+
+	ret = dixLookupDrawable(&pDrawable, (Drawable)stuff->drawable,
+				client, 0, DixReadAccess);
 
-	if (info->freesync_capable_client == NULL &&
+	if (!ret &&
+	    info->freesync_capable_client == NULL &&
 	    pDrawable->x == pDrawable->pScreen->x &&
 	    pDrawable->y == pDrawable->pScreen->y &&
 	    pDrawable->width == pDrawable->pScreen->width &&
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu 4/6] Handle Alt+Tab case when freesync enabled in steam client v2

[Nicholas Kazlauskas]

From: Hawking Zhang <Hawking.Zhang@amd.com>

v2:
fix ddx build warning unused var

The change supports the following scenario when freesync enabled for steam game
1). use Alt+Tab to run-time switch between windowed mode and fullscreen mode
2). use option setting to switch between windowed mode and fullscreen mode
Also verified on unigine heaven via setting check/un-check fullscreen

Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
Reviewed-by: Flora Cui <Flora.Cui@amd.com>
---
 src/amdgpu_dri2.c      |  9 ++++++---
 src/amdgpu_drv.h       |  8 ++++++--
 src/amdgpu_extension.c | 25 +++++++++++++++----------
 src/amdgpu_kms.c       | 41 +++++++++++++++++++++++++++++++++++++++++
 src/amdgpu_present.c   |  3 ++-
 5 files changed, 70 insertions(+), 16 deletions(-)

diff --git a/src/amdgpu_dri2.c b/src/amdgpu_dri2.c
index 8c601c8..c14ffb1 100644
--- a/src/amdgpu_dri2.c
+++ b/src/amdgpu_dri2.c
@@ -528,7 +528,8 @@ amdgpu_dri2_schedule_flip(xf86CrtcPtr crtc, ClientPtr client,
 	xf86DrvMsgVerb(scrn->scrnIndex, X_INFO, AMDGPU_LOGLEVEL_DEBUG,
 		       "%s:%d fevent[%p]\n", __func__, __LINE__, flip_info);
 
-	if (info->freesync_capable_client &&
+	if (info->allow_freesync &&
+	    info->freesync_client &&
 	    info->freesync_enabled == FALSE) {
 		AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
 		if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, TRUE))
@@ -720,7 +721,8 @@ static void amdgpu_dri2_frame_event_handler(xf86CrtcPtr crtc, uint32_t seq,
 		}
 		/* else fall through to exchange/blit */
 	case DRI2_SWAP:
-		if (info->freesync_capable_client &&
+		if (info->allow_freesync &&
+		    info->freesync_client &&
 		    info->freesync_enabled == TRUE) {
 			if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
 				info->freesync_enabled = FALSE;
@@ -1266,7 +1268,8 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
 	return TRUE;
 
 blit_fallback:
-	if (info->freesync_capable_client &&
+	if (info->allow_freesync &&
+	    info->freesync_client &&
 	    info->freesync_enabled == TRUE) {
 		if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
 			info->freesync_enabled = FALSE;
diff --git a/src/amdgpu_drv.h b/src/amdgpu_drv.h
index 91bb829..71274d1 100644
--- a/src/amdgpu_drv.h
+++ b/src/amdgpu_drv.h
@@ -248,6 +248,8 @@ typedef struct {
 	uint32_t family;
 	struct gbm_device *gbm;
 
+	ClipNotifyProcPtr ClipNotify;
+
 	Bool(*CloseScreen) (ScreenPtr pScreen);
 
 	void (*BlockHandler) (BLOCKHANDLER_ARGS_DECL);
@@ -304,9 +306,11 @@ typedef struct {
 	Bool allowPageFlip;
 
 	/* freesync */
-	ClientPtr freesync_capable_client;
-	uint32_t client_resource_id;
+	ClientPtr freesync_client;
+	DrawablePtr freesync_drawable;
+	XID freesync_client_id;
 	Bool freesync_enabled;
+	Bool allow_freesync;
 
 	/* cursor size */
 	int cursor_w;
diff --git a/src/amdgpu_extension.c b/src/amdgpu_extension.c
index ab7d6e3..33c8987 100644
--- a/src/amdgpu_extension.c
+++ b/src/amdgpu_extension.c
@@ -62,15 +62,17 @@ static int ProcAMDGPUFreesyncCapability(ClientPtr client)
 				client, 0, DixReadAccess);
 
 	if (!ret &&
-	    info->freesync_capable_client == NULL &&
 	    pDrawable->x == pDrawable->pScreen->x &&
 	    pDrawable->y == pDrawable->pScreen->y &&
 	    pDrawable->width == pDrawable->pScreen->width &&
 	    pDrawable->height == pDrawable->pScreen->height) {
-		info->freesync_capable_client = client;
-		cliResId = FakeClientID(client->index);
-		if (AddResource(cliResId, RT_AMDGPUCLIENT, (void *)pScrn))
-			info->client_resource_id = cliResId;
+		if (!info->freesync_client) {
+			info->freesync_client = client;
+			cliResId = FakeClientID(client->index);
+			if (AddResource(cliResId, RT_AMDGPUCLIENT, (void *)pScrn))
+				info->freesync_client_id = cliResId;
+		}
+		info->freesync_drawable = pDrawable;
 	}
 
 	return client->noClientException;
@@ -162,8 +164,8 @@ void AMDGPUFreeResourceByType(ScreenPtr pScreen)
 	ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
 	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
 
-	if (info->client_resource_id)
-		FreeResourceByType(info->client_resource_id,
+	if (info->freesync_client_id)
+		FreeResourceByType(info->freesync_client_id,
 				   RT_AMDGPUCLIENT, FALSE);
 	return;
 }
@@ -174,9 +176,10 @@ int AMDGPUClientGone(void *data, XID id)
 	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
 	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
 
-	if (id == (XID)info->client_resource_id) {
-		info->client_resource_id = None;
-		info->freesync_capable_client = NULL;
+	if (id == info->freesync_client_id) {
+		info->freesync_client_id = None;
+		info->freesync_client = NULL;
+		info->freesync_drawable = NULL;
 	}
 
 	if (info->freesync_enabled == TRUE) {
@@ -184,5 +187,7 @@ int AMDGPUClientGone(void *data, XID id)
 			info->freesync_enabled = FALSE;
 	}
 
+	info->allow_freesync = FALSE;
+
         return 1;
 }
diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index dc98473..88af2fa 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -1722,6 +1722,42 @@ static Bool AMDGPUCloseScreen_KMS(ScreenPtr pScreen)
 	return pScreen->CloseScreen(pScreen);
 }
 
+static void AMDGPUClipNotify(WindowPtr win, int dx, int dy)
+{
+	ScreenPtr pScreen = win->drawable.pScreen;
+	ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
+	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
+	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
+	ClientPtr client = wClient(win);
+
+	if (info->ClipNotify) {
+		pScreen->ClipNotify = info->ClipNotify;
+		pScreen->ClipNotify(win, dx, dy);
+		info->ClipNotify = pScreen->ClipNotify;
+		pScreen->ClipNotify = AMDGPUClipNotify;
+	}
+
+	if (!info->freesync_client)
+		return;
+
+	if (client == info->freesync_client &&
+	    win->drawable.id == info->freesync_drawable->id) {
+		if (win->drawable.x == pScreen->x &&
+		    win->drawable.y == pScreen->y &&
+		    win->drawable.width == pScreen->width &&
+		    win->drawable.height == pScreen->height) {
+			info->allow_freesync = TRUE;
+		} else {
+			info->allow_freesync = FALSE;
+			if (info->freesync_enabled == TRUE &&
+			    !AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
+				info->freesync_enabled = FALSE;
+		}
+	}
+
+	return;
+}
+
 void AMDGPUFreeScreen_KMS(ScrnInfoPtr pScrn)
 {
 	xf86DrvMsgVerb(pScrn->scrnIndex, X_INFO, AMDGPU_LOGLEVEL_DEBUG,
@@ -1946,6 +1982,11 @@ Bool AMDGPUScreenInit_KMS(ScreenPtr pScreen, int argc, char **argv)
 	pScreen->SyncSharedPixmap = amdgpu_sync_shared_pixmap;
 #endif
 
+	if (!info->shadow_fb) {
+		info->ClipNotify = pScreen->ClipNotify;
+		pScreen->ClipNotify = AMDGPUClipNotify;
+	}
+
 	if (!xf86CrtcScreenInit(pScreen))
 		return FALSE;
 
diff --git a/src/amdgpu_present.c b/src/amdgpu_present.c
index 8b70b38..66d264c 100644
--- a/src/amdgpu_present.c
+++ b/src/amdgpu_present.c
@@ -322,7 +322,8 @@ amdgpu_present_flip(RRCrtcPtr crtc, uint64_t event_id, uint64_t target_msc,
 	if (!amdgpu_present_check_flip(crtc, screen->root, pixmap, sync_flip))
 		return ret;
 
-	if (info->freesync_capable_client &&
+	if (info->allow_freesync &&
+	    info->freesync_client &&
 	    info->freesync_enabled == FALSE) {
 		AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
 		if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, TRUE))
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu 5/6] Do not issue freesync ioctl from present unflip

[Nicholas Kazlauskas]

From: Hawking Zhang <Hawking.Zhang@amd.com>

It prefers that freesync is enable/disable per client lifecycle
rather than per flip. With the patch, freesync will be disabled
when clientgone or switch to windowed mode.

Change-Id: I2c725bd045c4855f9e1436f0791755b0a47a6ecc
Signed-off-by: Hawking Zhang <Hawking.Zhang@amd.com>
Reviewed-by: Flora Cui <Flora.Cui@amd.com>
---
 src/amdgpu_present.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/amdgpu_present.c b/src/amdgpu_present.c
index 66d264c..9ed000b 100644
--- a/src/amdgpu_present.c
+++ b/src/amdgpu_present.c
@@ -371,12 +371,6 @@ amdgpu_present_unflip(ScreenPtr screen, uint64_t event_id)
 	if (!amdgpu_present_check_unflip(scrn))
 		goto modeset;
 
-	if (info->freesync_capable_client &&
-		info->freesync_enabled == TRUE) {
-				if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
-			info->freesync_enabled = FALSE;
-		}
-
 	event = calloc(1, sizeof(struct amdgpu_present_vblank_event));
 	if (!event) {
 		ErrorF("%s: calloc failed, display might freeze\n", __func__);
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH xf86-video-amdgpu 6/6] Replace amdgpu ioctl with CRTC properties for enabling FreeSync

[Nicholas Kazlauskas]

The support for per-CRTC variable refresh control via DRM
properties means that the driver specific FreeSync IOCTL can be
replaced.

In order to accomodate this new behavior existing changes to how
and when FreeSync is enabled/disabled was needed.

There are 3 behavioral differences this patch introduces:

1. FreeSync clients are considered fullscreen when they cover an
entire CRTC instead of the X Screen.

This allows FreeSync to be enabled in multi-monitor situations.

2. FreeSync is enabled immediately upon receiving the FreeSync request.

This replaces the deferred enablment on flip. The CRTC can be targeted
directly and there's no need to wait until the flip. The DC kernel
driver will take care of enabling/disabling on the next flip.

The caveat here is that some windows (like desktop compositor surfaces)
will have FreeSync enabled if they are not explicitly blacklisted
(ie. in mesa).

Since these typically didn't change position or size they were ignored
previously. There was a bug here, however - if you hotplug monitors
in certain configurations then you'd be stuck with FreeSync enabled on
the Desktop until reboot.

3. Freesync is disabled when the Drawable's Window is destroyed instead
of when the client resource is removed.

The need for allocation of a resource to handle this is unecessary
when we get callbacks for each Window being destroyed on the screen.

The remaining changes are small cleanups that shouldn't affect
functional behavior.

Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 src/amdgpu_dri2.c      |  26 -------
 src/amdgpu_drv.h       |   5 +-
 src/amdgpu_extension.c | 149 +++++++++++++++++++++++------------------
 src/amdgpu_extension.h |   7 +-
 src/amdgpu_kms.c       |  45 ++++++++-----
 src/amdgpu_present.c   |   8 ---
 src/drmmode_display.c  |  88 ++++++++++++++++++++++++
 src/drmmode_display.h  |   1 +
 8 files changed, 209 insertions(+), 120 deletions(-)

diff --git a/src/amdgpu_dri2.c b/src/amdgpu_dri2.c
index c14ffb1..17eb287 100644
--- a/src/amdgpu_dri2.c
+++ b/src/amdgpu_dri2.c
@@ -528,14 +528,6 @@ amdgpu_dri2_schedule_flip(xf86CrtcPtr crtc, ClientPtr client,
 	xf86DrvMsgVerb(scrn->scrnIndex, X_INFO, AMDGPU_LOGLEVEL_DEBUG,
 		       "%s:%d fevent[%p]\n", __func__, __LINE__, flip_info);
 
-	if (info->allow_freesync &&
-	    info->freesync_client &&
-	    info->freesync_enabled == FALSE) {
-		AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
-		if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, TRUE))
-			info->freesync_enabled = TRUE;
-	}
-
 	/* Page flip the full screen buffer */
 	back_priv = back->driverPrivate;
 	if (amdgpu_do_pageflip(scrn, client, back_priv->pixmap,
@@ -689,8 +681,6 @@ static void amdgpu_dri2_frame_event_handler(xf86CrtcPtr crtc, uint32_t seq,
 {
 	DRI2FrameEventPtr event = event_data;
 	ScrnInfoPtr scrn = crtc->scrn;
-	AMDGPUInfoPtr info = AMDGPUPTR(scrn);
-	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
 	DrawablePtr drawable;
 	int status;
 	int swap_type;
@@ -721,13 +711,6 @@ static void amdgpu_dri2_frame_event_handler(xf86CrtcPtr crtc, uint32_t seq,
 		}
 		/* else fall through to exchange/blit */
 	case DRI2_SWAP:
-		if (info->allow_freesync &&
-		    info->freesync_client &&
-		    info->freesync_enabled == TRUE) {
-			if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
-				info->freesync_enabled = FALSE;
-		}
-
 		if (DRI2CanExchange(drawable) &&
 		    can_exchange(scrn, drawable, event->front, event->back)) {
 			amdgpu_dri2_exchange_buffers(drawable, event->front,
@@ -1094,8 +1077,6 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
 {
 	ScreenPtr screen = draw->pScreen;
 	ScrnInfoPtr scrn = xf86ScreenToScrn(screen);
-	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
-	AMDGPUInfoPtr info = AMDGPUPTR(scrn);
 	xf86CrtcPtr crtc = amdgpu_dri2_drawable_crtc(draw, TRUE);
 	uint32_t msc_delta;
 	drmVBlankSeqType type;
@@ -1268,13 +1249,6 @@ static int amdgpu_dri2_schedule_swap(ClientPtr client, DrawablePtr draw,
 	return TRUE;
 
 blit_fallback:
-	if (info->allow_freesync &&
-	    info->freesync_client &&
-	    info->freesync_enabled == TRUE) {
-		if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
-			info->freesync_enabled = FALSE;
-	}
-
 	if (swap_info) {
 		swap_info->type = DRI2_SWAP;
 		amdgpu_dri2_schedule_event(FALLBACK_SWAP_DELAY, swap_info);
diff --git a/src/amdgpu_drv.h b/src/amdgpu_drv.h
index 71274d1..b6425d7 100644
--- a/src/amdgpu_drv.h
+++ b/src/amdgpu_drv.h
@@ -249,6 +249,7 @@ typedef struct {
 	struct gbm_device *gbm;
 
 	ClipNotifyProcPtr ClipNotify;
+	DestroyWindowProcPtr DestroyWindow;
 
 	Bool(*CloseScreen) (ScreenPtr pScreen);
 
@@ -307,8 +308,8 @@ typedef struct {
 
 	/* freesync */
 	ClientPtr freesync_client;
-	DrawablePtr freesync_drawable;
-	XID freesync_client_id;
+	xf86CrtcPtr freesync_crtc;
+	XID freesync_xid;
 	Bool freesync_enabled;
 	Bool allow_freesync;
 
diff --git a/src/amdgpu_extension.c b/src/amdgpu_extension.c
index 33c8987..5a43f55 100644
--- a/src/amdgpu_extension.c
+++ b/src/amdgpu_extension.c
@@ -39,11 +39,31 @@
 #include "amdgpu_extension.h"
 #include "amdgpu_drv.h"
 
-static RESTYPE RT_AMDGPUCLIENT;
+static xf86CrtcPtr FindFullscreenCRTC(ScrnInfoPtr scrn, WindowPtr win)
+{
+	xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
+	uint32_t i;
+
+	for (i = 0; i < xf86_config->num_crtc; i++) {
+		xf86CrtcPtr crtc = xf86_config->crtc[i];
+		if (!crtc || crtc->scrn != scrn)
+			continue;
+
+		if (crtc->bounds.x1 == win->drawable.x &&
+		    crtc->bounds.x2 == win->drawable.x + win->drawable.width &&
+		    crtc->bounds.y1 == win->drawable.y &&
+		    crtc->bounds.y2 == win->drawable.y + win->drawable.height)
+				return crtc;
+	}
+
+	return NULL;
+}
 
 static int ProcAMDGPUFreesyncCapability(ClientPtr client)
 {
 	REQUEST(xAMDGPUFreesyncCapabilityReq);
+	int ret;
+
 	REQUEST_SIZE_MATCH(xAMDGPUFreesyncCapabilityReq);
 
 	if (stuff->screen >= screenInfo.numScreens) {
@@ -51,28 +71,37 @@ static int ProcAMDGPUFreesyncCapability(ClientPtr client)
 		return BadValue;
 	}
 
-	ScreenPtr pScreen = screenInfo.screens[stuff->screen];
-	ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
-	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
-	XID cliResId;
-	DrawablePtr pDrawable = NULL;
-	int ret = -1;
-
-	ret = dixLookupDrawable(&pDrawable, (Drawable)stuff->drawable,
-				client, 0, DixReadAccess);
-
-	if (!ret &&
-	    pDrawable->x == pDrawable->pScreen->x &&
-	    pDrawable->y == pDrawable->pScreen->y &&
-	    pDrawable->width == pDrawable->pScreen->width &&
-	    pDrawable->height == pDrawable->pScreen->height) {
-		if (!info->freesync_client) {
-			info->freesync_client = client;
-			cliResId = FakeClientID(client->index);
-			if (AddResource(cliResId, RT_AMDGPUCLIENT, (void *)pScrn))
-				info->freesync_client_id = cliResId;
+	WindowPtr win = NULL;
+	ret = dixLookupWindow(
+		&win,
+		(XID)stuff->drawable,
+		client,
+		DixReadAccess);
+
+	if (ret == 0) {
+		ScreenPtr pScreen = win->drawable.pScreen;
+		ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
+		AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
+		xf86CrtcPtr crtc = AMDGPUGetFreesyncCRTC(win);
+
+		if (crtc) {
+			if (!info->freesync_client) {
+				info->freesync_client = client;
+				info->freesync_crtc = crtc;
+				info->freesync_xid = win->drawable.id;
+				info->allow_freesync = TRUE;
+				xf86DrvMsgVerb(
+					pScrn->scrnIndex,
+					X_INFO,
+					AMDGPU_LOGLEVEL_DEBUG,
+					"Activating window for Freesync: %x\n",
+					win->drawable.id);
+
+				AMDGPUSetFreesync(crtc, TRUE);
+			}
+		} else if (info->freesync_client == client) {
+			AMDGPUResetFreesync(pScrn);
 		}
-		info->freesync_drawable = pDrawable;
 	}
 
 	return client->noClientException;
@@ -97,7 +126,6 @@ static int AMDGPUSwapDispatch(ClientPtr client __attribute__((unused)))
 
 static void AMDGPUResetExtension(ExtensionEntry *extEntry __attribute__((unused)))
 {
-	RT_AMDGPUCLIENT = RT_NONE;
 }
 
 static void AMDGPUExtensionInit (void)
@@ -115,8 +143,6 @@ static void AMDGPUExtensionInit (void)
 		ErrorF("AddExtension failed\n");
 		return;
 	}
-
-	RT_AMDGPUCLIENT = CreateNewResourceType(AMDGPUClientGone, "AMDGPUClient");
 }
 
 static ExtensionModule AMDGPU_Ext = {
@@ -135,59 +161,50 @@ void AMDGPUExtensionSetup(void)
 #endif
 }
 
-int AMDGPUFreesyncControl(int fd, Bool enable_freesync)
+extern xf86CrtcPtr AMDGPUGetFreesyncCRTC(WindowPtr win)
 {
-	int ret = -1;
-	struct drm_amdgpu_freesync args;
-
-	memset(&args, 0, sizeof(args));
-	if (enable_freesync) {
-		args.op = AMDGPU_FREESYNC_FULLSCREEN_ENTER;
-                ret = drmCommandWriteRead(fd, DRM_AMDGPU_FREESYNC,
-                                          &args, sizeof(args));
-                if (ret)
-                        ErrorF("Fail to enable freesync mode.\n");
-
-	} else {
-		args.op = AMDGPU_FREESYNC_FULLSCREEN_EXIT;
-		ret = drmCommandWriteRead(fd, DRM_AMDGPU_FREESYNC,
-					  &args, sizeof(args));
-		if (ret)
-			ErrorF("Fail to disable freesync mode.\n");
-	}
+	ScreenPtr pScreen = win->drawable.pScreen;
+	ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
 
-	return ret;
+	if (win->visibility != VisibilityUnobscured &&
+	    win->visibility != VisibilityPartiallyObscured)
+	    return NULL;
+
+	return FindFullscreenCRTC(pScrn, win);
 }
 
-void AMDGPUFreeResourceByType(ScreenPtr pScreen)
+void AMDGPUResetFreesync(ScrnInfoPtr pScrn)
 {
-	ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
 	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
 
-	if (info->freesync_client_id)
-		FreeResourceByType(info->freesync_client_id,
-				   RT_AMDGPUCLIENT, FALSE);
-	return;
+	AMDGPUSetFreesync(info->freesync_crtc, FALSE);
+
+	info->freesync_client = NULL;
+	info->freesync_crtc = NULL;
+	info->freesync_xid = None;
+	info->allow_freesync = FALSE;
 }
 
-int AMDGPUClientGone(void *data, XID id)
+extern void AMDGPUSetFreesync(xf86CrtcPtr crtc, Bool enabled)
 {
-	ScrnInfoPtr pScrn = (ScrnInfoPtr)data;
-	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
-	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
+	if (crtc) {
+		ScrnInfoPtr pScrn = crtc->scrn;
+		xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(pScrn);
+		AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
+		Bool found = FALSE;
+		uint32_t i;
 
-	if (id == info->freesync_client_id) {
-		info->freesync_client_id = None;
-		info->freesync_client = NULL;
-		info->freesync_drawable = NULL;
-	}
+		if (info->freesync_enabled == enabled)
+			return;
 
-	if (info->freesync_enabled == TRUE) {
-		if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
-			info->freesync_enabled = FALSE;
-	}
+		for (i = 0; i < xf86_config->num_crtc; ++i)
+			if (xf86_config->crtc[i] == crtc)
+				found = TRUE;
 
-	info->allow_freesync = FALSE;
+		if (!found)
+			return;
 
-        return 1;
+		if (drmmode_set_variable_refresh(crtc, enabled))
+			info->freesync_enabled = enabled;
+	}
 }
diff --git a/src/amdgpu_extension.h b/src/amdgpu_extension.h
index 87f479f..c150d63 100644
--- a/src/amdgpu_extension.h
+++ b/src/amdgpu_extension.h
@@ -46,7 +46,8 @@ typedef struct _AMDGPUFreesyncCapabilityReq {
 #define sz_xAMDGPUFreesyncCapabilityReq		XREQ_SZ(AMDGPUFreesyncCapability)
 
 extern void AMDGPUExtensionSetup(void);
-extern int AMDGPUFreesyncControl(int fd, Bool enable_freesync);
-extern void AMDGPUFreeResourceByType(ScreenPtr pScreen);
-extern int AMDGPUClientGone(void *data, XID id);
+extern void AMDGPUResetFreesync(ScrnInfoPtr pScrn);
+extern xf86CrtcPtr AMDGPUGetFreesyncCRTC(WindowPtr win);
+extern void AMDGPUSetFreesync(xf86CrtcPtr crtc, Bool enabled);
+
 #endif /* _AMDGPU_EXTENSION_H_ */
diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index 88af2fa..158990b 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -1727,8 +1727,6 @@ static void AMDGPUClipNotify(WindowPtr win, int dx, int dy)
 	ScreenPtr pScreen = win->drawable.pScreen;
 	ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
 	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
-	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
-	ClientPtr client = wClient(win);
 
 	if (info->ClipNotify) {
 		pScreen->ClipNotify = info->ClipNotify;
@@ -1737,25 +1735,39 @@ static void AMDGPUClipNotify(WindowPtr win, int dx, int dy)
 		pScreen->ClipNotify = AMDGPUClipNotify;
 	}
 
-	if (!info->freesync_client)
-		return;
-
-	if (client == info->freesync_client &&
-	    win->drawable.id == info->freesync_drawable->id) {
-		if (win->drawable.x == pScreen->x &&
-		    win->drawable.y == pScreen->y &&
-		    win->drawable.width == pScreen->width &&
-		    win->drawable.height == pScreen->height) {
+	if (info->freesync_client &&
+	    info->freesync_xid == win->drawable.id) {
+		xf86CrtcPtr crtc = AMDGPUGetFreesyncCRTC(win);
+		if (crtc) {
 			info->allow_freesync = TRUE;
+			info->freesync_crtc = crtc;
+			AMDGPUSetFreesync(info->freesync_crtc, TRUE);
 		} else {
 			info->allow_freesync = FALSE;
-			if (info->freesync_enabled == TRUE &&
-			    !AMDGPUFreesyncControl(pAMDGPUEnt->fd, FALSE))
-				info->freesync_enabled = FALSE;
+			AMDGPUSetFreesync(info->freesync_crtc, FALSE);
 		}
 	}
+}
 
-	return;
+static Bool AMDGPUDestroyWindow(WindowPtr win){
+	ScreenPtr pScreen = win->drawable.pScreen;
+	ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen);
+	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
+	Bool result = TRUE;
+
+	if (info->freesync_client &&
+	    info->freesync_xid == win->drawable.id) {
+		AMDGPUResetFreesync(pScrn);
+	}
+
+	if (info->DestroyWindow) {
+		pScreen->DestroyWindow = info->DestroyWindow;
+		result = pScreen->DestroyWindow(win);
+		info->DestroyWindow = pScreen->DestroyWindow;
+		pScreen->DestroyWindow = AMDGPUDestroyWindow;
+	}
+
+	return result;
 }
 
 void AMDGPUFreeScreen_KMS(ScrnInfoPtr pScrn)
@@ -1982,6 +1994,9 @@ Bool AMDGPUScreenInit_KMS(ScreenPtr pScreen, int argc, char **argv)
 	pScreen->SyncSharedPixmap = amdgpu_sync_shared_pixmap;
 #endif
 
+	info->DestroyWindow = pScreen->DestroyWindow;
+	pScreen->DestroyWindow = AMDGPUDestroyWindow;
+
 	if (!info->shadow_fb) {
 		info->ClipNotify = pScreen->ClipNotify;
 		pScreen->ClipNotify = AMDGPUClipNotify;
diff --git a/src/amdgpu_present.c b/src/amdgpu_present.c
index 9ed000b..4d4bddc 100644
--- a/src/amdgpu_present.c
+++ b/src/amdgpu_present.c
@@ -322,14 +322,6 @@ amdgpu_present_flip(RRCrtcPtr crtc, uint64_t event_id, uint64_t target_msc,
 	if (!amdgpu_present_check_flip(crtc, screen->root, pixmap, sync_flip))
 		return ret;
 
-	if (info->allow_freesync &&
-	    info->freesync_client &&
-	    info->freesync_enabled == FALSE) {
-		AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(scrn);
-		if (!AMDGPUFreesyncControl(pAMDGPUEnt->fd, TRUE))
-			info->freesync_enabled = TRUE;
-	}
-
 	event = calloc(1, sizeof(struct amdgpu_present_vblank_event));
 	if (!event)
 		return ret;
diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index 6ef6a98..d06606a 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -268,6 +268,94 @@ int drmmode_crtc_get_ust_msc(xf86CrtcPtr crtc, CARD64 *ust, CARD64 *msc)
 	return Success;
 }
 
+static uint32_t drmmode_get_prop_id(
+	uint32_t drm_fd,
+	drmModeObjectPropertiesPtr props,
+	char const* name)
+{
+	uint32_t i, prop_id = 0;
+
+	for (i = 0; !prop_id && props->count_props; ++i) {
+		drmModePropertyPtr drm_prop =
+			drmModeGetProperty(drm_fd, props->props[i]);
+
+		if (!drm_prop)
+			continue;
+
+		if (strcmp(drm_prop->name, name) == 0)
+			prop_id = drm_prop->prop_id;
+
+		drmModeFreeProperty(drm_prop);
+	}
+
+	return prop_id;
+}
+
+static uint32_t drmmode_get_variable_refresh_prop_id(
+	uint32_t drm_fd,
+	uint32_t crtc_id)
+{
+	drmModeObjectPropertiesPtr drm_props;
+	uint32_t prop_id = 0;
+
+	if (!drm_fd || !crtc_id)
+		return 0;
+
+	drm_props = drmModeObjectGetProperties(
+		drm_fd,
+		crtc_id,
+		DRM_MODE_OBJECT_CRTC);
+
+	if (!drm_props)
+		return 0;
+
+	prop_id = drmmode_get_prop_id(
+		drm_fd, drm_props, "VARIABLE_REFRESH");
+
+	drmModeFreeObjectProperties(drm_props);
+
+	return prop_id;
+}
+
+Bool drmmode_set_variable_refresh(xf86CrtcPtr crtc, Bool enabled)
+{
+	ScrnInfoPtr pScrn = crtc->scrn;
+	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
+	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
+	drmmode_crtc_private_ptr drmmode_crtc = crtc->driver_private;
+	uint32_t crtc_id, prop_id;
+	int result = 0;
+
+	if (!drmmode_crtc || !drmmode_crtc->mode_crtc)
+		result = -1;
+
+	if (result == 0) {
+		crtc_id = drmmode_crtc->mode_crtc->crtc_id;
+		prop_id = drmmode_get_variable_refresh_prop_id(
+			pAMDGPUEnt->fd, crtc_id);
+		if (!prop_id)
+			result = -1;
+	}
+
+	if (result == 0)
+		result = drmModeObjectSetProperty(
+			pAMDGPUEnt->fd,
+			crtc_id,
+			DRM_MODE_OBJECT_CRTC,
+			prop_id,
+			enabled);
+
+	xf86DrvMsgVerb(pScrn->scrnIndex,
+		X_INFO,
+		AMDGPU_LOGLEVEL_DEBUG,
+		"Set variable refresh status: enabled=%d, result=%d, client: %x\n",
+		(int)(enabled != 0),
+		(int)(result == 0),
+		info->freesync_xid);
+
+	return (result == 0);
+}
+
 static void
 drmmode_do_crtc_dpms(xf86CrtcPtr crtc, int mode)
 {
diff --git a/src/drmmode_display.h b/src/drmmode_display.h
index c245ae8..2e139a9 100644
--- a/src/drmmode_display.h
+++ b/src/drmmode_display.h
@@ -255,6 +255,7 @@ Bool amdgpu_do_pageflip(ScrnInfoPtr scrn, ClientPtr client,
 			uint32_t target_msc);
 int drmmode_crtc_get_ust_msc(xf86CrtcPtr crtc, CARD64 *ust, CARD64 *msc);
 int drmmode_get_current_ust(int drm_fd, CARD64 * ust);
+extern Bool drmmode_set_variable_refresh(xf86CrtcPtr crtc, Bool enabled);
 
 Bool drmmode_wait_vblank(xf86CrtcPtr crtc, drmVBlankSeqType type,
 			 uint32_t target_seq, unsigned long signal,
-- 
2.18.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH xf86-video-amdgpu 0/6] xf86-video-amdgpu integration for DRM variable refresh rate API

[Michel Dänzer]

Hi Nicholas,


thanks for the patches.


On 2018-09-11 6:18 p.m., Nicholas Kazlauskas wrote:
> These patches are part of a proposed new interface for supporting variable refresh rate via DRM properties.
> 
> https://patchwork.freedesktop.org/series/49486/
> 
> When notified of a window that is FreeSync capable via X these patches help track when the window is fullscreen to manage the variable_refresh property on the CRTC.

I'm afraid the Xorg driver support will have to be more or less redone
from scratch for upstreaming:

Whether or not a client wants variable refresh rate enabled can be
tracked via the window property mechanism supported by the core X11
protocol, no need for a protocol extension.

That should also allow simpler tracking of when variable refresh rate
can actually be enabled: It can be enabled while a window is flipping,
and its corresponding property allows it. This should be straightforward
with the Present extension, because that also explicitly marks the end
of a window flipping (via an "unflip"). DRI2 is trickier; it's probably
okay not to support variable refresh rate with that, at least initially.


I can look into this after the upcoming Xorg driver 18.1 releases. Or I
can give guidance if one of you wants to look into it.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH xf86-video-amdgpu 0/6] xf86-video-amdgpu integration for DRM variable refresh rate API

[Kazlauskas, Nicholas]

On 09/12/2018 04:13 AM, Michel Dänzer wrote:
> 
> Hi Nicholas,
> 
> 
> thanks for the patches.
> 
> 
> On 2018-09-11 6:18 p.m., Nicholas Kazlauskas wrote:
>> These patches are part of a proposed new interface for supporting variable refresh rate via DRM properties.
>>
>> https://patchwork.freedesktop.org/series/49486/
>>
>> When notified of a window that is FreeSync capable via X these patches help track when the window is fullscreen to manage the variable_refresh property on the CRTC.
> 
> I'm afraid the Xorg driver support will have to be more or less redone
> from scratch for upstreaming:
> 
> Whether or not a client wants variable refresh rate enabled can be
> tracked via the window property mechanism supported by the core X11
> protocol, no need for a protocol extension.
> 
> That should also allow simpler tracking of when variable refresh rate
> can actually be enabled: It can be enabled while a window is flipping,
> and its corresponding property allows it. This should be straightforward
> with the Present extension, because that also explicitly marks the end
> of a window flipping (via an "unflip"). DRI2 is trickier; it's probably
> okay not to support variable refresh rate with that, at least initially.
> 
> 
> I can look into this after the upcoming Xorg driver 18.1 releases. Or I
> can give guidance if one of you wants to look into it.
> 
> 

I can a look into this. I agree that the extension method is less than 
ideal - in being vendor specific mostly. It does have the nice property 
that it remains independent of the application's render backend, though.

I imagine you're suggesting specifying a window property hint like 
_NET_WM_BYPASS_COMPOSITOR - maybe a define new one like 
_NET_WM_VARIABLE_REFRESH (even though the two are closely related in 
terms of fullscreen behavior). Then in each backend the property could 
probably be checked before the present as appropriate.

This patch series already has the problem you're describing about DRI2 
where there's not a nice explicit end notification for flipping - it's 
only disabled when the window no longer covers the CRTC or the window is 
destroyed (which is the case for most applications but likely not all).

Thanks for the feedback.

Nicholas Kazlauskas
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH xf86-video-amdgpu 0/6] xf86-video-amdgpu integration for DRM variable refresh rate API

[Michel Dänzer]

On 2018-09-12 2:48 p.m., Kazlauskas, Nicholas wrote:
> On 09/12/2018 04:13 AM, Michel Dänzer wrote:
>> On 2018-09-11 6:18 p.m., Nicholas Kazlauskas wrote:
>>> These patches are part of a proposed new interface for supporting
>>> variable refresh rate via DRM properties.
>>>
>>> https://patchwork.freedesktop.org/series/49486/
>>>
>>> When notified of a window that is FreeSync capable via X these
>>> patches help track when the window is fullscreen to manage the
>>> variable_refresh property on the CRTC.
>>
>> I'm afraid the Xorg driver support will have to be more or less redone
>> from scratch for upstreaming:
>>
>> Whether or not a client wants variable refresh rate enabled can be
>> tracked via the window property mechanism supported by the core X11
>> protocol, no need for a protocol extension.
>>
>> That should also allow simpler tracking of when variable refresh rate
>> can actually be enabled: It can be enabled while a window is flipping,
>> and its corresponding property allows it. This should be straightforward
>> with the Present extension, because that also explicitly marks the end
>> of a window flipping (via an "unflip"). DRI2 is trickier; it's probably
>> okay not to support variable refresh rate with that, at least initially.
>>
>>
>> I can look into this after the upcoming Xorg driver 18.1 releases. Or I
>> can give guidance if one of you wants to look into it.
> 
> I can a look into this. I agree that the extension method is less than
> ideal - in being vendor specific mostly. It does have the nice property
> that it remains independent of the application's render backend, though.

Not sure what you mean by that. Surely a window property is just as
independent. :)


> I imagine you're suggesting specifying a window property hint like
> _NET_WM_BYPASS_COMPOSITOR - maybe a define new one like
> _NET_WM_VARIABLE_REFRESH (even though the two are closely related in
> terms of fullscreen behavior). Then in each backend the property could
> probably be checked before the present as appropriate.

Right. (Not sure the _NET_WM prefix is appropriate, as it's nothing to
do with the window manager, but that's a detail)


> This patch series already has the problem you're describing about DRI2
> where there's not a nice explicit end notification for flipping - it's
> only disabled when the window no longer covers the CRTC or the window is
> destroyed (which is the case for most applications but likely not all).

Right. I think it's better to just not bother with DRI2, at least until
there's a specific scenario where variable refresh is really needed with
DRI2.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH xf86-video-amdgpu 0/6] xf86-video-amdgpu integration for DRM variable refresh rate API

[Kazlauskas, Nicholas]

On 09/12/2018 12:22 PM, Michel Dänzer wrote:
> On 2018-09-12 2:48 p.m., Kazlauskas, Nicholas wrote:
>> On 09/12/2018 04:13 AM, Michel Dänzer wrote:
>>> On 2018-09-11 6:18 p.m., Nicholas Kazlauskas wrote:
>>>> These patches are part of a proposed new interface for supporting
>>>> variable refresh rate via DRM properties.
>>>>
>>>> https://patchwork.freedesktop.org/series/49486/
>>>>
>>>> When notified of a window that is FreeSync capable via X these
>>>> patches help track when the window is fullscreen to manage the
>>>> variable_refresh property on the CRTC.
>>>
>>> I'm afraid the Xorg driver support will have to be more or less redone
>>> from scratch for upstreaming:
>>>
>>> Whether or not a client wants variable refresh rate enabled can be
>>> tracked via the window property mechanism supported by the core X11
>>> protocol, no need for a protocol extension.
>>>
>>> That should also allow simpler tracking of when variable refresh rate
>>> can actually be enabled: It can be enabled while a window is flipping,
>>> and its corresponding property allows it. This should be straightforward
>>> with the Present extension, because that also explicitly marks the end
>>> of a window flipping (via an "unflip"). DRI2 is trickier; it's probably
>>> okay not to support variable refresh rate with that, at least initially.
>>>
>>>
>>> I can look into this after the upcoming Xorg driver 18.1 releases. Or I
>>> can give guidance if one of you wants to look into it.
>>
>> I can a look into this. I agree that the extension method is less than
>> ideal - in being vendor specific mostly. It does have the nice property
>> that it remains independent of the application's render backend, though.
> 
> Not sure what you mean by that. Surely a window property is just as
> independent. :)

I was mostly referring to being independent from the backend specific 
code in the DDX driver - DRI2/DRI3/present etc. This change turns the 
notification from being a callback into polling.

> 
> 
>> I imagine you're suggesting specifying a window property hint like
>> _NET_WM_BYPASS_COMPOSITOR - maybe a define new one like
>> _NET_WM_VARIABLE_REFRESH (even though the two are closely related in
>> terms of fullscreen behavior). Then in each backend the property could
>> probably be checked before the present as appropriate.
> 
> Right. (Not sure the _NET_WM prefix is appropriate, as it's nothing to
> do with the window manager, but that's a detail)

I guess an application could operate independently from the window 
manager with variable refresh enabled but it's not certainly a common 
use case. I suppose this would probably need additional documentation 
patches for the extended window manager standards.

_NET_VARIABLE_REFRESH is probably fine as a name.

> 
> 
>> This patch series already has the problem you're describing about DRI2
>> where there's not a nice explicit end notification for flipping - it's
>> only disabled when the window no longer covers the CRTC or the window is
>> destroyed (which is the case for most applications but likely not all).
> 
> Right. I think it's better to just not bother with DRI2, at least until
> there's a specific scenario where variable refresh is really needed with
> DRI2.
> 
> 

Nicholas Kazlauskas
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx