Would it not make more sense to set pAMDGPUEnt->fd to -1 beneath the error_amdgpu tag?
On 10/28/15 08: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) > { > - AMDGPUInfoPtr info = AMDGPUPTR(pScrn); > - AMDGPUEntPtr pAMDGPUEnt = AMDGPUEntPriv(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) > + pAMDGPUEnt->fd = amdgpu_kernel_open_fd(pScrn, pci_dev, NULL); > + if (pAMDGPUEnt->fd == -1) > return FALSE; > > /* Check that what we opened was a master or a master-capable FD, > @@ -175,18 +165,18 @@ static Bool amdgpu_open_drm_master(ScrnInfoPtr pScrn) > sv.drm_di_minor = 1; > sv.drm_dd_major = -1; > sv.drm_dd_minor = -1; > - err = drmSetInterfaceVersion(info->dri2.drm_fd, &sv); > + err = drmSetInterfaceVersion(pAMDGPUEnt->fd, &sv); > if (err != 0) { > xf86DrvMsg(pScrn->scrnIndex, X_ERROR, > "[drm] failed to set drm interface version.\n"); > - drmClose(info->dri2.drm_fd); > + drmClose(pAMDGPUEnt->fd); > return FALSE; > } > > return TRUE; > } > > -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; > @@ -236,11 +226,13 @@ static Bool amdgpu_get_scrninfo(int entity_num, void > *pci_dev) > uint32_t minor_version; > > pPriv->ptr = xnfcalloc(sizeof(AMDGPUEntRec), 1); > - pAMDGPUEnt = pPriv->ptr; > + if (!pPriv->ptr) > + return FALSE; > > - if (amdgpu_open_drm_master(pScrn)) { > + pAMDGPUEnt = pPriv->ptr; > + if (!amdgpu_open_drm_master(pScrn, pAMDGPUEnt, > + pci_dev, entity_num)) > goto error_fd; > - } > > pAMDGPUEnt->fd_ref = 1; > > @@ -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) _______________________________________________ xorg-driver-ati mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-driver-ati
