[PATCH xf86-video-amdgpu 1/2] Refactor amdgpu_kernel_close_fd helper

[PATCH xf86-video-amdgpu 1/2] Refactor amdgpu_kernel_close_fd helper

[Michel Dänzer]

From: Michel Dänzer <michel.daenzer@amd.com>

Preparation for the following change.

Assign pAMDGPUEnt->fd = -1 instead of 0 when we're not using the file
descriptor anymore.

Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 src/amdgpu_kms.c   |  7 +------
 src/amdgpu_probe.c | 10 ++++++++++
 src/amdgpu_probe.h |  2 ++
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
index 31c2e86f2..92bf5fadf 100644
--- a/src/amdgpu_kms.c
+++ b/src/amdgpu_kms.c
@@ -132,12 +132,7 @@ static void AMDGPUFreeRec(ScrnInfoPtr pScrn)
 		pAMDGPUEnt->fd_ref--;
 		if (!pAMDGPUEnt->fd_ref) {
 			amdgpu_device_deinitialize(pAMDGPUEnt->pDev);
-#ifdef XF86_PDEV_SERVER_FD
-			if (!(pAMDGPUEnt->platform_dev &&
-			      pAMDGPUEnt->platform_dev->flags & XF86_PDEV_SERVER_FD))
-#endif
-				drmClose(pAMDGPUEnt->fd);
-			pAMDGPUEnt->fd = 0;
+			amdgpu_kernel_close_fd(pAMDGPUEnt);
 		}
 	}
 
diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index 94da7f623..fc93ac6a2 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -134,6 +134,16 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn, char *busid,
 	return fd;
 }
 
+void amdgpu_kernel_close_fd(AMDGPUEntPtr pAMDGPUEnt)
+{
+#ifdef XF86_PDEV_SERVER_FD
+	if (!(pAMDGPUEnt->platform_dev &&
+	      pAMDGPUEnt->platform_dev->flags & XF86_PDEV_SERVER_FD))
+#endif
+		drmClose(pAMDGPUEnt->fd);
+	pAMDGPUEnt->fd = -1;
+}
+
 static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn, AMDGPUEntPtr pAMDGPUEnt,
 				   char *busid)
 {
diff --git a/src/amdgpu_probe.h b/src/amdgpu_probe.h
index 0f43233c3..9e4a9a451 100644
--- a/src/amdgpu_probe.h
+++ b/src/amdgpu_probe.h
@@ -71,6 +71,8 @@ typedef struct {
 	char *render_node;
 } AMDGPUEntRec, *AMDGPUEntPtr;
 
+extern void amdgpu_kernel_close_fd(AMDGPUEntPtr pAMDGPUEnt);
+
 extern const OptionInfoRec *AMDGPUOptionsWeak(void);
 
 extern Bool AMDGPUPreInit_KMS(ScrnInfoPtr, int);
-- 
2.11.0

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

[PATCH xf86-video-amdgpu 2/2] amdgpu_probe: Do not close server managed drm fds

[Michel Dänzer]

From: Hans De Goede <hdegoede@redhat.com>

This fixes the xserver only seeing AMD/ATI devices supported by the amdgpu
driver, as by the time xf86-video-ati gets a chance to probe them, the
fd has been closed.

This fixes e.g. Xorg not seeing the dGPU on a Lenovo Thinkpad E465 laptop
with a CARRIZO iGPU and a HAINAN dGPU.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>

v2: Rebased on top of new patch 1.
Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>
---
 src/amdgpu_probe.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index fc93ac6a2..fb62cb811 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -166,7 +166,7 @@ static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn, AMDGPUEntPtr pAMDGPUEnt,
 	if (err != 0) {
 		xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
 			   "[drm] failed to set drm interface version.\n");
-		drmClose(pAMDGPUEnt->fd);
+		amdgpu_kernel_close_fd(pAMDGPUEnt);
 		return FALSE;
 	}
 
@@ -254,7 +254,7 @@ static Bool amdgpu_get_scrninfo(int entity_num, struct pci_device *pci_dev)
 	return TRUE;
 
 error_amdgpu:
-	drmClose(pAMDGPUEnt->fd);
+	amdgpu_kernel_close_fd(pAMDGPUEnt);
 error_fd:
 	free(pPriv->ptr);
 error:
@@ -349,6 +349,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, busid, dev);
 		if (pAMDGPUEnt->fd < 0)
 			goto error_fd;
@@ -367,7 +368,6 @@ amdgpu_platform_probe(DriverPtr pDriver,
 		pAMDGPUEnt = pPriv->ptr;
 		pAMDGPUEnt->fd_ref++;
 	}
-	pAMDGPUEnt->platform_dev = dev;
 
 	xf86SetEntityInstanceForScreen(pScrn, pEnt->index,
 				       xf86GetNumEntityInstances(pEnt->
@@ -379,7 +379,7 @@ amdgpu_platform_probe(DriverPtr pDriver,
 	return TRUE;
 
 error_amdgpu:
-	drmClose(pAMDGPUEnt->fd);
+	amdgpu_kernel_close_fd(pAMDGPUEnt);
 error_fd:
 	free(pPriv->ptr);
 error:
-- 
2.11.0

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

Re: [PATCH xf86-video-amdgpu 1/2] Refactor amdgpu_kernel_close_fd helper

[Hans de Goede]

Hi,

On 06-03-17 11:02, Michel Dänzer wrote:
> From: Michel Dänzer <michel.daenzer@amd.com>
>
> Preparation for the following change.
>
> Assign pAMDGPUEnt->fd = -1 instead of 0 when we're not using the file
> descriptor anymore.
>
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Thank you for picking my original patch for this up, I still had
it on my to-do list but it got buried under other to-do list
items.

Both patches look good to me:

Reviewed-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>  src/amdgpu_kms.c   |  7 +------
>  src/amdgpu_probe.c | 10 ++++++++++
>  src/amdgpu_probe.h |  2 ++
>  3 files changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/src/amdgpu_kms.c b/src/amdgpu_kms.c
> index 31c2e86f2..92bf5fadf 100644
> --- a/src/amdgpu_kms.c
> +++ b/src/amdgpu_kms.c
> @@ -132,12 +132,7 @@ static void AMDGPUFreeRec(ScrnInfoPtr pScrn)
>  		pAMDGPUEnt->fd_ref--;
>  		if (!pAMDGPUEnt->fd_ref) {
>  			amdgpu_device_deinitialize(pAMDGPUEnt->pDev);
> -#ifdef XF86_PDEV_SERVER_FD
> -			if (!(pAMDGPUEnt->platform_dev &&
> -			      pAMDGPUEnt->platform_dev->flags & XF86_PDEV_SERVER_FD))
> -#endif
> -				drmClose(pAMDGPUEnt->fd);
> -			pAMDGPUEnt->fd = 0;
> +			amdgpu_kernel_close_fd(pAMDGPUEnt);
>  		}
>  	}
>
> diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
> index 94da7f623..fc93ac6a2 100644
> --- a/src/amdgpu_probe.c
> +++ b/src/amdgpu_probe.c
> @@ -134,6 +134,16 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn, char *busid,
>  	return fd;
>  }
>
> +void amdgpu_kernel_close_fd(AMDGPUEntPtr pAMDGPUEnt)
> +{
> +#ifdef XF86_PDEV_SERVER_FD
> +	if (!(pAMDGPUEnt->platform_dev &&
> +	      pAMDGPUEnt->platform_dev->flags & XF86_PDEV_SERVER_FD))
> +#endif
> +		drmClose(pAMDGPUEnt->fd);
> +	pAMDGPUEnt->fd = -1;
> +}
> +
>  static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn, AMDGPUEntPtr pAMDGPUEnt,
>  				   char *busid)
>  {
> diff --git a/src/amdgpu_probe.h b/src/amdgpu_probe.h
> index 0f43233c3..9e4a9a451 100644
> --- a/src/amdgpu_probe.h
> +++ b/src/amdgpu_probe.h
> @@ -71,6 +71,8 @@ typedef struct {
>  	char *render_node;
>  } AMDGPUEntRec, *AMDGPUEntPtr;
>
> +extern void amdgpu_kernel_close_fd(AMDGPUEntPtr pAMDGPUEnt);
> +
>  extern const OptionInfoRec *AMDGPUOptionsWeak(void);
>
>  extern Bool AMDGPUPreInit_KMS(ScrnInfoPtr, int);
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH xf86-video-amdgpu 2/2] amdgpu_probe: Do not close server managed drm fds

[Alex Deucher]

On Mon, Mar 6, 2017 at 5:02 AM, Michel Dänzer <michel@daenzer.net> wrote:
> From: Hans De Goede <hdegoede@redhat.com>
>
> This fixes the xserver only seeing AMD/ATI devices supported by the amdgpu
> driver, as by the time xf86-video-ati gets a chance to probe them, the
> fd has been closed.
>
> This fixes e.g. Xorg not seeing the dGPU on a Lenovo Thinkpad E465 laptop
> with a CARRIZO iGPU and a HAINAN dGPU.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>
> v2: Rebased on top of new patch 1.
> Signed-off-by: Michel Dänzer <michel.daenzer@amd.com>

Series is:
Reviewed-by: Alex Deucher@amd.com>

> ---
>  src/amdgpu_probe.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
> index fc93ac6a2..fb62cb811 100644
> --- a/src/amdgpu_probe.c
> +++ b/src/amdgpu_probe.c
> @@ -166,7 +166,7 @@ static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn, AMDGPUEntPtr pAMDGPUEnt,
>         if (err != 0) {
>                 xf86DrvMsg(pScrn->scrnIndex, X_ERROR,
>                            "[drm] failed to set drm interface version.\n");
> -               drmClose(pAMDGPUEnt->fd);
> +               amdgpu_kernel_close_fd(pAMDGPUEnt);
>                 return FALSE;
>         }
>
> @@ -254,7 +254,7 @@ static Bool amdgpu_get_scrninfo(int entity_num, struct pci_device *pci_dev)
>         return TRUE;
>
>  error_amdgpu:
> -       drmClose(pAMDGPUEnt->fd);
> +       amdgpu_kernel_close_fd(pAMDGPUEnt);
>  error_fd:
>         free(pPriv->ptr);
>  error:
> @@ -349,6 +349,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, busid, dev);
>                 if (pAMDGPUEnt->fd < 0)
>                         goto error_fd;
> @@ -367,7 +368,6 @@ amdgpu_platform_probe(DriverPtr pDriver,
>                 pAMDGPUEnt = pPriv->ptr;
>                 pAMDGPUEnt->fd_ref++;
>         }
> -       pAMDGPUEnt->platform_dev = dev;
>
>         xf86SetEntityInstanceForScreen(pScrn, pEnt->index,
>                                        xf86GetNumEntityInstances(pEnt->
> @@ -379,7 +379,7 @@ amdgpu_platform_probe(DriverPtr pDriver,
>         return TRUE;
>
>  error_amdgpu:
> -       drmClose(pAMDGPUEnt->fd);
> +       amdgpu_kernel_close_fd(pAMDGPUEnt);
>  error_fd:
>         free(pPriv->ptr);
>  error:
> --
> 2.11.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx