On 28.10.2015 22:24, Jammy Zhou wrote:
> The crash is caused by the NULL value returned by AMDGPUPTR(pScrn),
> because the driverPrivate is not allocated yet in PciProbe phase,
> and it is usually done in the PreInit phase.
>
> Use pAMDGPUEnt->fd instead of info->dri2.drm_fd to avoid AMDGPUInfoPtr
> related code in amdgpu_open_drm_master, so that the crash can be fixed.
>
> v3: some more cleanup
> v2: switch to pAMDGPUEnt->fd, and update the commit message
>
> Signed-off-by: Jammy Zhou <[email protected]>
> ---
> src/amdgpu_probe.c | 37 +++++++++++++------------------------
> 1 file changed, 13 insertions(+), 24 deletions(-)
>
> diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
> index 481271b..f6e675c 100644
> --- a/src/amdgpu_probe.c
> +++ b/src/amdgpu_probe.c
> @@ -147,24 +147,14 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn,
> struct pci_device *dev,
> return fd;
> }
>
> -static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn)
> +static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn, AMDGPUEntPtr
> pAMDGPUEnt,
> + struct pci_device *pci_dev, int entity_num)
Sorry I wasn't explicit about this before, but please don't add an
unused entity_num parameter.
> - if (pAMDGPUEnt->fd) {
> - xf86DrvMsg(pScrn->scrnIndex, X_INFO,
> - " reusing fd for second head\n");
> -
> - info->drmmode.fd = info->dri2.drm_fd = pAMDGPUEnt->fd;
> - pAMDGPUEnt->fd_ref++;
> - return TRUE;
> - }
You could rebase on top of the attached patch, since this is a logically
separate change from the crash fix.
> -static Bool amdgpu_get_scrninfo(int entity_num, void *pci_dev)
> +static Bool amdgpu_get_scrninfo(int entity_num, struct pci_device *pci_dev)
> {
> ScrnInfoPtr pScrn = NULL;
> EntityInfoPtr pEnt;
[...]
> @@ -252,8 +244,6 @@ static Bool amdgpu_get_scrninfo(int entity_num, void
> *pci_dev)
> "amdgpu_device_initialize failed\n");
> goto error_amdgpu;
> }
> - } else {
> - pAMDGPUEnt = pPriv->ptr;
> }
>
> xf86SetEntityInstanceForScreen(pScrn, pEnt->index,
> @@ -266,7 +256,6 @@ static Bool amdgpu_get_scrninfo(int entity_num, void
> *pci_dev)
>
> error_amdgpu:
> drmClose(pAMDGPUEnt->fd);
> - pAMDGPUEnt->fd = 0;
> error_fd:
> free(pPriv->ptr);
> return FALSE;
> @@ -276,7 +265,7 @@ static Bool
> amdgpu_pci_probe(DriverPtr pDriver,
> int entity_num, struct pci_device *device, intptr_t match_data)
> {
> - return amdgpu_get_scrninfo(entity_num, (void *)device);
> + return amdgpu_get_scrninfo(entity_num, device);
> }
>
> static Bool AMDGPUDriverFunc(ScrnInfoPtr scrn, xorgDriverFuncOp op, void
> *data)
>
These changes make sense, but are again logically separate from the
crash fix.
If you want, I can send out a new series with the logical changes split
up into separate patches.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
From eb618a4008821dcbf558066058a02b1b74ad6be4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michel=20D=C3=A4nzer?= <[email protected]>
Date: Wed, 28 Oct 2015 17:56:13 +0900
Subject: [PATCH xf86-video-amdgpu] Remove dead code from
amdgpu_open_drm_master
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
amdgpu_get_scrninfo allocates the memory pointed to by pAMDGPUEnt just
before it calls amdgpu_open_drm_master, so pAMDGPUEnt->fd is always 0
here.
Signed-off-by: Michel Dänzer <[email protected]>
---
src/amdgpu_probe.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c
index 481271b..3fedfab 100644
--- a/src/amdgpu_probe.c
+++ b/src/amdgpu_probe.c
@@ -154,15 +154,6 @@ static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn)
drmSetVersion sv;
int err;
- if (pAMDGPUEnt->fd) {
- xf86DrvMsg(pScrn->scrnIndex, X_INFO,
- " reusing fd for second head\n");
-
- info->drmmode.fd = info->dri2.drm_fd = pAMDGPUEnt->fd;
- pAMDGPUEnt->fd_ref++;
- return TRUE;
- }
-
info->dri2.drm_fd = amdgpu_kernel_open_fd(pScrn, info->PciInfo, NULL);
if (info->dri2.drm_fd == -1)
return FALSE;
--
2.6.1
_______________________________________________
xorg-driver-ati mailing list
[email protected]
http://lists.x.org/mailman/listinfo/xorg-driver-ati
