xf86-video-amdgpu: Branch 'master' - 10 commits

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Mon Mar 16 14:34:12 UTC 2020


 src/amdgpu_drv.h      |    2 
 src/amdgpu_kms.c      |   11 --
 src/amdgpu_probe.c    |  251 +++++++++++++++++++++-----------------------------
 src/amdgpu_probe.h    |    1 
 src/drmmode_display.c |    6 -
 5 files changed, 113 insertions(+), 158 deletions(-)

New commits:
commit 42a3148ae14c6fd0d2e2e9013971188ca721d8f8
Author: Emil Velikov <emil.velikov at collabora.com>
Date:   Sat Dec 10 18:53:37 2016 +0000

    Factor out common code to amdgpu_probe()
    
    Keep the distinct pci/platform screen management in the separate probe
    entry point and fold the rest into a single function.
    
    v2: Rebase
    
    Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
    Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
    Acked-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index 30e2d67..7ce9090 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -241,16 +241,14 @@ error_amdgpu:
 	return FALSE;
 }
 
-static Bool amdgpu_get_scrninfo(int entity_num, struct pci_device *pci_dev)
+static Bool
+amdgpu_probe(ScrnInfoPtr pScrn, int entity_num,
+	     struct pci_device *pci_dev, struct xf86_platform_device *dev)
 {
-	ScrnInfoPtr pScrn = NULL;
 	EntityInfoPtr pEnt = NULL;
 	DevUnion *pPriv;
 	AMDGPUEntPtr pAMDGPUEnt;
 
-	pScrn = xf86ConfigPciEntity(pScrn, 0, entity_num, NULL,
-				    NULL, NULL, NULL, NULL, NULL);
-
 	if (!pScrn)
 		return FALSE;
 
@@ -258,7 +256,6 @@ static Bool amdgpu_get_scrninfo(int entity_num, struct pci_device *pci_dev)
 	pScrn->driverName = AMDGPU_DRIVER_NAME;
 	pScrn->name = AMDGPU_NAME;
 	pScrn->Probe = NULL;
-
 	pScrn->PreInit = AMDGPUPreInit_KMS;
 	pScrn->ScreenInit = AMDGPUScreenInit_KMS;
 	pScrn->SwitchMode = AMDGPUSwitchMode_KMS;
@@ -286,7 +283,7 @@ static Bool amdgpu_get_scrninfo(int entity_num, struct pci_device *pci_dev)
 			goto error;
 
 		pAMDGPUEnt = pPriv->ptr;
-		if (!amdgpu_device_setup(pScrn, pci_dev, NULL, pAMDGPUEnt))
+		if (!amdgpu_device_setup(pScrn, pci_dev, dev, pAMDGPUEnt))
 			goto error;
 
 		pAMDGPUEnt->fd_ref = 1;
@@ -321,7 +318,10 @@ static Bool
 amdgpu_pci_probe(DriverPtr pDriver,
 		 int entity_num, struct pci_device *device, intptr_t match_data)
 {
-	return amdgpu_get_scrninfo(entity_num, device);
+	ScrnInfoPtr pScrn = xf86ConfigPciEntity(NULL, 0, entity_num, NULL,
+						NULL, NULL, NULL, NULL, NULL);
+
+	return amdgpu_probe(pScrn, entity_num, device, NULL);
 }
 
 static Bool AMDGPUDriverFunc(ScrnInfoPtr scrn, xorgDriverFuncOp op, void *data)
@@ -350,9 +350,6 @@ amdgpu_platform_probe(DriverPtr pDriver,
 {
 	ScrnInfoPtr pScrn;
 	int scr_flags = 0;
-	EntityInfoPtr pEnt = NULL;
-	DevUnion *pPriv;
-	AMDGPUEntPtr pAMDGPUEnt;
 
 	if (!dev->pdev)
 		return FALSE;
@@ -365,65 +362,7 @@ amdgpu_platform_probe(DriverPtr pDriver,
 		xf86SetEntityShared(entity_num);
 	xf86AddEntityToScreen(pScrn, entity_num);
 
-	pScrn->driverVersion = AMDGPU_VERSION_CURRENT;
-	pScrn->driverName = AMDGPU_DRIVER_NAME;
-	pScrn->name = AMDGPU_NAME;
-	pScrn->Probe = NULL;
-	pScrn->PreInit = AMDGPUPreInit_KMS;
-	pScrn->ScreenInit = AMDGPUScreenInit_KMS;
-	pScrn->SwitchMode = AMDGPUSwitchMode_KMS;
-	pScrn->AdjustFrame = AMDGPUAdjustFrame_KMS;
-	pScrn->EnterVT = AMDGPUEnterVT_KMS;
-	pScrn->LeaveVT = AMDGPULeaveVT_KMS;
-	pScrn->FreeScreen = AMDGPUFreeScreen_KMS;
-	pScrn->ValidMode = AMDGPUValidMode;
-
-	pEnt = xf86GetEntityInfo(entity_num);
-
-	/* Create a AMDGPUEntity for all chips, even with old single head
-	 * Radeon, need to use pAMDGPUEnt for new monitor detection routines.
-	 */
-	xf86SetEntitySharable(entity_num);
-
-	if (gAMDGPUEntityIndex == -1)
-		gAMDGPUEntityIndex = xf86AllocateEntityPrivateIndex();
-
-	pPriv = xf86GetEntityPrivate(pEnt->index, gAMDGPUEntityIndex);
-
-	if (!pPriv->ptr) {
-
-		pPriv->ptr = xnfcalloc(sizeof(AMDGPUEntRec), 1);
-
-		pAMDGPUEnt = pPriv->ptr;
-		if (!amdgpu_device_setup(pScrn, NULL, dev, pAMDGPUEnt))
-			goto error;
-
-		pAMDGPUEnt->fd_ref = 1;
-
-	} else {
-		pAMDGPUEnt = pPriv->ptr;
-
-		if (pAMDGPUEnt->fd_ref == ARRAY_SIZE(pAMDGPUEnt->scrn)) {
-			xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
-				   "Only up to %u Zaphod instances supported\n",
-				   (unsigned)ARRAY_SIZE(pAMDGPUEnt->scrn));
-			goto error;
-		}
-
-		pAMDGPUEnt->fd_ref++;
-	}
-
-	xf86SetEntityInstanceForScreen(pScrn, pEnt->index,
-				       xf86GetNumEntityInstances(pEnt->
-								 index)
-				       - 1);
-	free(pEnt);
-
-	return TRUE;
-
-error:
-	free(pEnt);
-	return FALSE;
+	return amdgpu_probe(pScrn, entity_num, NULL, dev);
 }
 #endif
 
commit eeaaf370854b63966f0b5adbd00d2e6809b773c1
Author: Emil Velikov <emil.velikov at collabora.com>
Date:   Sat Mar 31 15:00:16 2018 +0100

    Introduce amdgpu_device_setup helper
    
    It folds the device specifics (open fd, device init) into a single
    place.
    
    v2:
     - Rebase
     - Pass pAMDGPUEnt to amdgpu_device_setup (Michel)
    
    Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
    Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
    Acked-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index a59b4ff..30e2d67 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -212,6 +212,35 @@ static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn,
 	return TRUE;
 }
 
+static Bool amdgpu_device_setup(ScrnInfoPtr pScrn,
+				struct pci_device *pci_dev,
+				struct xf86_platform_device *platform_dev,
+				AMDGPUEntPtr pAMDGPUEnt)
+{
+	uint32_t major_version;
+	uint32_t minor_version;
+
+	pAMDGPUEnt->platform_dev = platform_dev;
+	if (!amdgpu_open_drm_master(pScrn, pci_dev, platform_dev,
+				    pAMDGPUEnt))
+		return FALSE;
+
+	if (amdgpu_device_initialize(pAMDGPUEnt->fd,
+				     &major_version,
+				     &minor_version,
+				     &pAMDGPUEnt->pDev)) {
+		xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
+			   "amdgpu_device_initialize failed\n");
+		goto error_amdgpu;
+	}
+
+	return TRUE;
+
+error_amdgpu:
+	amdgpu_kernel_close_fd(pAMDGPUEnt);
+	return FALSE;
+}
+
 static Bool amdgpu_get_scrninfo(int entity_num, struct pci_device *pci_dev)
 {
 	ScrnInfoPtr pScrn = NULL;
@@ -252,27 +281,16 @@ static Bool amdgpu_get_scrninfo(int entity_num, struct pci_device *pci_dev)
 	pPriv = xf86GetEntityPrivate(pEnt->index, gAMDGPUEntityIndex);
 
 	if (!pPriv->ptr) {
-		uint32_t major_version;
-		uint32_t minor_version;
-
 		pPriv->ptr = xnfcalloc(sizeof(AMDGPUEntRec), 1);
 		if (!pPriv->ptr)
 			goto error;
 
 		pAMDGPUEnt = pPriv->ptr;
-		if (!amdgpu_open_drm_master(pScrn, pci_dev, NULL, pAMDGPUEnt))
+		if (!amdgpu_device_setup(pScrn, pci_dev, NULL, pAMDGPUEnt))
 			goto error;
 
 		pAMDGPUEnt->fd_ref = 1;
 
-		if (amdgpu_device_initialize(pAMDGPUEnt->fd,
-					     &major_version,
-					     &minor_version,
-					     &pAMDGPUEnt->pDev)) {
-			xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
-				   "amdgpu_device_initialize failed\n");
-			goto error_amdgpu;
-		}
 	} else {
 		pAMDGPUEnt = pPriv->ptr;
 
@@ -294,8 +312,6 @@ static Bool amdgpu_get_scrninfo(int entity_num, struct pci_device *pci_dev)
 
 	return TRUE;
 
-error_amdgpu:
-	amdgpu_kernel_close_fd(pAMDGPUEnt);
 error:
 	free(pEnt);
 	return FALSE;
@@ -375,25 +391,15 @@ amdgpu_platform_probe(DriverPtr pDriver,
 	pPriv = xf86GetEntityPrivate(pEnt->index, gAMDGPUEntityIndex);
 
 	if (!pPriv->ptr) {
-		uint32_t major_version;
-		uint32_t minor_version;
 
 		pPriv->ptr = xnfcalloc(sizeof(AMDGPUEntRec), 1);
+
 		pAMDGPUEnt = pPriv->ptr;
-		pAMDGPUEnt->platform_dev = dev;
-		if (!amdgpu_open_drm_master(pScrn, NULL, dev, pAMDGPUEnt))
+		if (!amdgpu_device_setup(pScrn, NULL, dev, pAMDGPUEnt))
 			goto error;
 
 		pAMDGPUEnt->fd_ref = 1;
 
-		if (amdgpu_device_initialize(pAMDGPUEnt->fd,
-					     &major_version,
-					     &minor_version,
-					     &pAMDGPUEnt->pDev)) {
-			xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
-				   "amdgpu_device_initialize failed\n");
-			goto error_amdgpu;
-		}
 	} else {
 		pAMDGPUEnt = pPriv->ptr;
 
@@ -415,8 +421,6 @@ amdgpu_platform_probe(DriverPtr pDriver,
 
 	return TRUE;
 
-error_amdgpu:
-	amdgpu_kernel_close_fd(pAMDGPUEnt);
 error:
 	free(pEnt);
 	return FALSE;
commit 1c9742e304f4d198628cdc9487049cde472c7285
Author: Emil Velikov <emil.velikov at collabora.com>
Date:   Sat Mar 31 14:27:52 2018 +0100

    Kill off drmOpen/Close/drmSetInterfaceVersion in favour of drmDevices
    
    The former has very subtle semantics (see the implementation in libdrm
    for details) which were required in the UMS days.
    
    With drmDevices around, we have enough information to build our
    heuristics and avoid drmOpen all together.
    
    In the odd case drmGetDevices2() can take a few extra cycles, so use a
    reasonably sized local array.
    
    v2:
     - Rebase
     - Rework now that amdgpu_kernel_mode_enabled() is staying
     - Keep amdgpu_bus_id()
     - Use local drmDevice array.
    
    v3:
     - Correct error handling (Michel)
     - Preserve the "am I master" check (Michel)
     - Always initialise the fd variable
    
    v4:
     - Don't print "-1" on drmGetDevices2 failure (Michel)
     - Use uppercase DRM (Michel)
    
    v5:
     - Rebase on top of amdgpu_bus_id() rework
     - Pass both pci and platform dev to amdgpu_kernel_open_fd() (Michel)
     - Indent local_drmIsMaster() with tabs (Michel)
    
    Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
    Acked-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index ebd2aa7..a59b4ff 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -77,6 +77,16 @@ static void AMDGPUIdentify(int flags)
 	xf86PrintChipsets(AMDGPU_NAME, "Driver for AMD Radeon", AMDGPUAny);
 }
 
+static Bool amdgpu_device_matches(const drmDevicePtr device,
+				  const struct pci_device *dev)
+{
+	return (device->bustype == DRM_BUS_PCI &&
+		device->businfo.pci->domain == dev->domain &&
+		device->businfo.pci->bus == dev->bus &&
+		device->businfo.pci->dev == dev->dev &&
+		device->businfo.pci->func == dev->func);
+}
+
 static Bool amdgpu_kernel_mode_enabled(ScrnInfoPtr pScrn)
 {
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
@@ -105,9 +115,11 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn,
 				 struct xf86_platform_device *platform_dev,
 				 AMDGPUEntPtr pAMDGPUEnt)
 {
+#define MAX_DRM_DEVICES 64
+	drmDevicePtr devices[MAX_DRM_DEVICES];
 	struct pci_device *dev;
 	const char *path;
-	int fd;
+	int fd = -1, i, ret;
 
 	if (platform_dev)
 		dev = platform_dev->pdev;
@@ -138,12 +150,28 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn,
 	if (!amdgpu_kernel_mode_enabled(pScrn))
 		return -1;
 
-	fd = drmOpen(NULL, pAMDGPUEnt->busid);
+	ret = drmGetDevices2(0, devices, ARRAY_SIZE(devices));
+	if (ret == -1) {
+		xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
+			   "[drm] Failed to retrieve DRM devices information.\n");
+		return -1;
+	}
+	for (i = 0; i < ret; i++) {
+		if (amdgpu_device_matches(devices[i], dev) &&
+		    devices[i]->available_nodes & (1 << DRM_NODE_PRIMARY)) {
+			path = devices[i]->nodes[DRM_NODE_PRIMARY];
+			fd = open(path, O_RDWR | O_CLOEXEC);
+			break;
+		}
+	}
+	drmFreeDevices(devices, ret);
+
 	if (fd == -1)
 		xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
 			   "[drm] Failed to open DRM device for %s: %s\n",
 			   pAMDGPUEnt->busid, strerror(errno));
 	return fd;
+#undef MAX_DRM_DEVICES
 }
 
 void amdgpu_kernel_close_fd(AMDGPUEntPtr pAMDGPUEnt)
@@ -152,32 +180,31 @@ void amdgpu_kernel_close_fd(AMDGPUEntPtr pAMDGPUEnt)
 	if (!(pAMDGPUEnt->platform_dev &&
 	      pAMDGPUEnt->platform_dev->flags & XF86_PDEV_SERVER_FD))
 #endif
-		drmClose(pAMDGPUEnt->fd);
+		close(pAMDGPUEnt->fd);
 	pAMDGPUEnt->fd = -1;
 }
 
-static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn, AMDGPUEntPtr pAMDGPUEnt,
-				   struct pci_device *pci_dev)
+/* Pull a local version of the helper. It's available since 2.4.98 yet
+ * it may be too new for some distributions.
+ */
+static int local_drmIsMaster(int fd)
 {
-	drmSetVersion sv;
-	int err;
+	return drmAuthMagic(fd, 0) != -EACCES;
+}
 
-	pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, pci_dev, NULL, pAMDGPUEnt);
+static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn,
+				   struct pci_device *pci_dev,
+				   struct xf86_platform_device *platform_dev,
+				   AMDGPUEntPtr pAMDGPUEnt)
+{
+	pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, pci_dev, platform_dev, pAMDGPUEnt);
 	if (pAMDGPUEnt->fd == -1)
 		return FALSE;
 
-	/* Check that what we opened was a master or a master-capable FD,
-	 * by setting the version of the interface we'll use to talk to it.
-	 * (see DRIOpenDRMMaster() in DRI1)
-	 */
-	sv.drm_di_major = 1;
-	sv.drm_di_minor = 1;
-	sv.drm_dd_major = -1;
-	sv.drm_dd_minor = -1;
-	err = drmSetInterfaceVersion(pAMDGPUEnt->fd, &sv);
-	if (err != 0) {
+	/* Check that what we opened is a master or a master-capable FD */
+	if (!local_drmIsMaster(pAMDGPUEnt->fd)) {
 		xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
-			   "[drm] failed to set drm interface version.\n");
+			   "[drm] device is not DRM master.\n");
 		amdgpu_kernel_close_fd(pAMDGPUEnt);
 		return FALSE;
 	}
@@ -233,7 +260,7 @@ static Bool amdgpu_get_scrninfo(int entity_num, struct pci_device *pci_dev)
 			goto error;
 
 		pAMDGPUEnt = pPriv->ptr;
-		if (!amdgpu_open_drm_master(pScrn, pAMDGPUEnt, pci_dev))
+		if (!amdgpu_open_drm_master(pScrn, pci_dev, NULL, pAMDGPUEnt))
 			goto error;
 
 		pAMDGPUEnt->fd_ref = 1;
@@ -354,8 +381,7 @@ amdgpu_platform_probe(DriverPtr pDriver,
 		pPriv->ptr = xnfcalloc(sizeof(AMDGPUEntRec), 1);
 		pAMDGPUEnt = pPriv->ptr;
 		pAMDGPUEnt->platform_dev = dev;
-		pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, NULL, dev, pAMDGPUEnt);
-		if (pAMDGPUEnt->fd < 0)
+		if (!amdgpu_open_drm_master(pScrn, NULL, dev, pAMDGPUEnt))
 			goto error;
 
 		pAMDGPUEnt->fd_ref = 1;
commit 2dd730784e632056c75a0fd62b33206b5fc01602
Author: Emil Velikov <emil.velikov at collabora.com>
Date:   Tue Jul 16 22:04:57 2019 +0100

    Use the device_id straight from gpu_info
    
    This way we can remove the PciInfo and Chipset from the AMDGPUInfoRec.
    
    Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
    Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
    Acked-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_drv.h b/src/amdgpu_drv.h
index 0ae171f..aae8603 100644
--- a/src/amdgpu_drv.h
+++ b/src/amdgpu_drv.h
@@ -248,8 +248,6 @@ extern DevScreenPrivateKeyRec amdgpu_device_private_key;
 
 typedef struct {
 	EntityInfoPtr pEnt;
-	struct pci_device *PciInfo;
-	int Chipset;
 	uint32_t family;
 	struct gbm_device *gbm;
 
diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index 15b8327..f0831d2 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -1421,20 +1421,13 @@ static Bool AMDGPUPreInitChipType_KMS(ScrnInfoPtr pScrn,
 	AMDGPUInfoPtr info = AMDGPUPTR(pScrn);
 	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
 
-	info->Chipset = info->PciInfo->device_id;
 	pScrn->chipset = (char*)amdgpu_get_marketing_name(pAMDGPUEnt->pDev);
 	if (!pScrn->chipset)
 		pScrn->chipset = "Unknown AMD Radeon GPU";
 
-	if (info->Chipset < 0) {
-		xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
-			   "Chipset \"%s\" is not recognized\n",
-			   pScrn->chipset);
-		return FALSE;
-	}
 	xf86DrvMsg(pScrn->scrnIndex, X_PROBED,
 		   "Chipset: \"%s\" (ChipID = 0x%04x)\n",
-		   pScrn->chipset, info->Chipset);
+		   pScrn->chipset, gpu_info->asic_id);
 
 	info->family = gpu_info->family_id;
 
@@ -1576,7 +1569,6 @@ Bool AMDGPUPreInit_KMS(ScrnInfoPtr pScrn, int flags)
 		xf86SetPrimInitDone(pScrn->entityList[0]);
 	}
 
-	info->PciInfo = xf86GetPciInfoForEntity(info->pEnt->index);
 	pScrn->monitor = pScrn->confScreen->monitor;
 
 	if (!AMDGPUPreInitVisual(pScrn))
commit 655b3c55b9a6233091d4dc5d2e80a0373aa3e2d6
Author: Emil Velikov <emil.velikov at collabora.com>
Date:   Wed Jul 17 00:04:39 2019 +0100

    Reuse the existing busid string
    
    Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
    Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
    Acked-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/drmmode_display.c b/src/drmmode_display.c
index c410d0b..2c0b96c 100644
--- a/src/drmmode_display.c
+++ b/src/drmmode_display.c
@@ -3432,7 +3432,7 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
 	unsigned int crtcs_needed = 0;
 	unsigned int crtcs_got = 0;
 	drmModeResPtr mode_res;
-	char *bus_id_string, *provider_name;
+	char *provider_name;
 
 	xf86CrtcConfigInit(pScrn, &drmmode_xf86crtc_config_funcs);
 
@@ -3495,9 +3495,7 @@ Bool drmmode_pre_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, int cpp)
 	/* workout clones */
 	drmmode_clones_init(pScrn, drmmode, mode_res);
 
-	bus_id_string = DRICreatePCIBusID(info->PciInfo);
-	XNFasprintf(&provider_name, "%s @ %s", pScrn->chipset, bus_id_string);
-	free(bus_id_string);
+	XNFasprintf(&provider_name, "%s @ %s", pScrn->chipset, pAMDGPUEnt->busid);
 	xf86ProviderSetup(pScrn, NULL, provider_name);
 	free(provider_name);
 
commit b357a8474074d911d1c03572d4d9db3ee420633a
Author: Emil Velikov <emil.velikov at collabora.com>
Date:   Wed Jul 17 00:01:51 2019 +0100

    Store the busid string in AMDGPUEnt
    
    This way we can reuse it, instead of redoing it later on.
    
    v2: Pass the AMDGPUEnt as argument.
    v3: free() the string at AMDGPUFreeRec (Michel)
    v4: Inline amdgpu_bus_id, move at top of mdgpu_kernel_open_fd (Michel)
    
    Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
    Reviewed-by: Michel Dänzer <michel.daenzer at amd.com> (v3)
    Acked-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index 94d1c0b..15b8327 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -283,6 +283,7 @@ static void AMDGPUFreeRec(ScrnInfoPtr pScrn)
 			amdgpu_unwrap_property_requests(pScrn);
 			amdgpu_device_deinitialize(pAMDGPUEnt->pDev);
 			amdgpu_kernel_close_fd(pAMDGPUEnt);
+			free(pAMDGPUEnt->busid);
 			free(pPriv->ptr);
 			pPriv->ptr = NULL;
 		}
diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index 8250156..ebd2aa7 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -77,19 +77,11 @@ static void AMDGPUIdentify(int flags)
 	xf86PrintChipsets(AMDGPU_NAME, "Driver for AMD Radeon", AMDGPUAny);
 }
 
-static char *amdgpu_bus_id(ScrnInfoPtr pScrn, struct pci_device *dev)
-{
-	char *busid;
-
-	XNFasprintf(&busid, "pci:%04x:%02x:%02x.%u",
-		    dev->domain, dev->bus, dev->dev, dev->func);
-
-	return busid;
-}
-
-static Bool amdgpu_kernel_mode_enabled(ScrnInfoPtr pScrn, char *busIdString)
+static Bool amdgpu_kernel_mode_enabled(ScrnInfoPtr pScrn)
 {
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+	AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(pScrn);
+	const char *busIdString = pAMDGPUEnt->busid;
 	int ret = drmCheckModesettingSupported(busIdString);
 
 	if (ret) {
@@ -110,13 +102,21 @@ static Bool amdgpu_kernel_mode_enabled(ScrnInfoPtr pScrn, char *busIdString)
 
 static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn,
 				 struct pci_device *pci_dev,
-				 struct xf86_platform_device *platform_dev)
+				 struct xf86_platform_device *platform_dev,
+				 AMDGPUEntPtr pAMDGPUEnt)
 {
 	struct pci_device *dev;
 	const char *path;
-	char *busid;
 	int fd;
 
+	if (platform_dev)
+		dev = platform_dev->pdev;
+	else
+		dev = pci_dev;
+
+	XNFasprintf(&pAMDGPUEnt->busid, "pci:%04x:%02x:%02x.%u",
+		    dev->domain, dev->bus, dev->dev, dev->func);
+
 	if (platform_dev) {
 #ifdef ODEV_ATTRIB_FD
 		fd = xf86_get_platform_device_int_attrib(platform_dev,
@@ -135,24 +135,14 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn,
 #endif
 	}
 
-	if (platform_dev)
-		dev = platform_dev->pdev;
-	else
-		dev = pci_dev;
-
-	busid = amdgpu_bus_id(pScrn, dev);
-
-	if (!amdgpu_kernel_mode_enabled(pScrn, busid)) {
-		free(busid);
+	if (!amdgpu_kernel_mode_enabled(pScrn))
 		return -1;
-	}
 
-	fd = drmOpen(NULL, busid);
+	fd = drmOpen(NULL, pAMDGPUEnt->busid);
 	if (fd == -1)
 		xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
 			   "[drm] Failed to open DRM device for %s: %s\n",
-			   busid, strerror(errno));
-	free(busid);
+			   pAMDGPUEnt->busid, strerror(errno));
 	return fd;
 }
 
@@ -172,7 +162,7 @@ static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn, AMDGPUEntPtr pAMDGPUEnt,
 	drmSetVersion sv;
 	int err;
 
-	pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, pci_dev, NULL);
+	pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, pci_dev, NULL, pAMDGPUEnt);
 	if (pAMDGPUEnt->fd == -1)
 		return FALSE;
 
@@ -364,7 +354,7 @@ amdgpu_platform_probe(DriverPtr pDriver,
 		pPriv->ptr = xnfcalloc(sizeof(AMDGPUEntRec), 1);
 		pAMDGPUEnt = pPriv->ptr;
 		pAMDGPUEnt->platform_dev = dev;
-		pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, NULL, dev);
+		pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, NULL, dev, pAMDGPUEnt);
 		if (pAMDGPUEnt->fd < 0)
 			goto error;
 
diff --git a/src/amdgpu_probe.h b/src/amdgpu_probe.h
index 306c9a5..fea4d39 100644
--- a/src/amdgpu_probe.h
+++ b/src/amdgpu_probe.h
@@ -69,6 +69,7 @@ typedef struct {
 	ScrnInfoPtr scrn[6];
 	struct xf86_platform_device *platform_dev;
 	char *render_node;
+	char *busid;
 } AMDGPUEntRec, *AMDGPUEntPtr;
 
 extern void amdgpu_kernel_close_fd(AMDGPUEntPtr pAMDGPUEnt);
commit 2c0c154a838060eb683599faf9cbfa3e66dd42c8
Author: Emil Velikov <emil.velikov at collabora.com>
Date:   Tue Jul 16 23:39:30 2019 +0100

    Remove NULL check after a "cannot fail" function
    
    XNFasprintf cannot fail - aka busid cannot be NULL.
    
    Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
    Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
    Acked-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index 229202b..8250156 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -84,10 +84,6 @@ static char *amdgpu_bus_id(ScrnInfoPtr pScrn, struct pci_device *dev)
 	XNFasprintf(&busid, "pci:%04x:%02x:%02x.%u",
 		    dev->domain, dev->bus, dev->dev, dev->func);
 
-	if (!busid)
-		xf86DrvMsgVerb(pScrn->scrnIndex, X_ERROR, 0,
-			       "AMDGPU: Failed to generate bus ID string\n");
-
 	return busid;
 }
 
@@ -145,8 +141,6 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn,
 		dev = pci_dev;
 
 	busid = amdgpu_bus_id(pScrn, dev);
-	if (!busid)
-		return -1;
 
 	if (!amdgpu_kernel_mode_enabled(pScrn, busid)) {
 		free(busid);
commit 16ae0d06c6711a36c814618e06bf2be53079af81
Author: Emil Velikov <emil.velikov at collabora.com>
Date:   Tue Jul 16 23:37:05 2019 +0100

    Fixup the amdgpu_bus_id() string format
    
    The func is a u, instead of a signed int.
    
    v2: Drop the precision - s/1u/u/ (Michel)
    
    Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
    Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
    Acked-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index 1db78a4..229202b 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -81,7 +81,7 @@ static char *amdgpu_bus_id(ScrnInfoPtr pScrn, struct pci_device *dev)
 {
 	char *busid;
 
-	XNFasprintf(&busid, "pci:%04x:%02x:%02x.%d",
+	XNFasprintf(&busid, "pci:%04x:%02x:%02x.%u",
 		    dev->domain, dev->bus, dev->dev, dev->func);
 
 	if (!busid)
commit abbe23fae70b7f3bc7033d7603d331570677d431
Author: Emil Velikov <emil.velikov at collabora.com>
Date:   Sat Dec 10 14:30:16 2016 +0000

    Remove drmCheckModesettingSupported and kernel module loading, on Linux
    
    The former of these is a UMS artefact which gives incorrect and
    misleading promise whether KMS is supported. Not to mention that
    AMDGPU is a only KMS driver.
    
    In a similar fashion xf86LoadKernelModule() is a relic of the times,
    where platforms had no scheme of detecting and loading the appropriate
    kernel module.
    
    Notes:
     - Since there is no reply from Robert the code is still around, behind
    a FreeBSD guard.
     - If FreeBSD still needs this they should look and fix it ASAP, as:
       - wayland itself or compositors do _not_ load kernel modules
       - the kernel module should be loaded early to control the clocks/fan,
    hence temperature of the card
    
    v2: Keep the code as FreeBSD only, add 'Notes' in the commit message.
    
    Cc: Robert Millan <rmh at freebsd.org>
    Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
    Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
    Acked-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index 7abe13d..1db78a4 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -93,20 +93,20 @@ static char *amdgpu_bus_id(ScrnInfoPtr pScrn, struct pci_device *dev)
 
 static Bool amdgpu_kernel_mode_enabled(ScrnInfoPtr pScrn, char *busIdString)
 {
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
 	int ret = drmCheckModesettingSupported(busIdString);
 
-#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
 	if (ret) {
 		if (xf86LoadKernelModule("amdgpukms"))
 			ret = drmCheckModesettingSupported(busIdString);
 	}
-#endif
 	if (ret) {
 		xf86DrvMsgVerb(pScrn->scrnIndex, X_INFO, 0,
 			       "[KMS] drm report modesetting isn't supported.\n");
 		return FALSE;
 	}
 
+#endif
 	xf86DrvMsgVerb(pScrn->scrnIndex, X_INFO, 0,
 		       "[KMS] Kernel modesetting enabled.\n");
 	return TRUE;
commit 0b3bc7addf9b5989bfad7c2c31979a15f5ba701d
Author: Emil Velikov <emil.velikov at collabora.com>
Date:   Sat Dec 10 14:28:19 2016 +0000

    Use ODEV_ATTRIB_PATH where possible for the device node.
    
    Use the device node path, if the server knows it.
    
    Note:
    ODEV_ATTRIB_PATH was introduced with xserver 1.13 - the minimum version
    required to build amdgpu. Yet it's defined in xf86platformBus.h. With
    the header included only when XSERVER_PLATFORM_BUS is set.
    
    Keep things obvious and use a ODEV_ATTRIB_PATH guard.
    
    v2: Rebase, add commit message
    
    Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
    Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>
    Acked-by: Alex Deucher <alexander.deucher at amd.com>

diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index 2dc934f..7abe13d 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -33,6 +33,8 @@
 #include <errno.h>
 #include <string.h>
 #include <stdlib.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 
 /*
  * Authors:
@@ -115,18 +117,28 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn,
 				 struct xf86_platform_device *platform_dev)
 {
 	struct pci_device *dev;
+	const char *path;
 	char *busid;
 	int fd;
 
-#ifdef ODEV_ATTRIB_FD
 	if (platform_dev) {
+#ifdef ODEV_ATTRIB_FD
 		fd = xf86_get_platform_device_int_attrib(platform_dev,
 							 ODEV_ATTRIB_FD, -1);
 		if (fd != -1)
 			return fd;
-	}
 #endif
 
+#ifdef ODEV_ATTRIB_PATH
+		path = xf86_get_platform_device_attrib(platform_dev,
+						       ODEV_ATTRIB_PATH);
+
+		fd = open(path, O_RDWR | O_CLOEXEC);
+		if (fd != -1)
+			return fd;
+#endif
+	}
+
 	if (platform_dev)
 		dev = platform_dev->pdev;
 	else


More information about the xorg-commit mailing list