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

Nicholas Kazlauskas nicholas.kazlauskas at amd.com
Tue Sep 11 16:18:42 UTC 2018


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 at 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



More information about the amd-gfx mailing list