[ Moving to the amd-gfx mailing list, where xf86-video-amdgpu (and -ati) patches are reviewed ] On 18/10/16 11:48 PM, Hans de Goede wrote: > 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. That basically makes sense, but I have one question: Why does amdgpu get probed in the first place in that case? I was under the impression that this should only happen for GPUs bound to the amdgpu kernel driver, due to Section "OutputClass" in /usr/share/X11/xorg.conf.d/10-amdgpu.conf . > diff --git a/src/amdgpu_probe.c b/src/amdgpu_probe.c > index 213d245..5d17fe5 100644 > --- a/src/amdgpu_probe.c > +++ b/src/amdgpu_probe.c > @@ -142,6 +142,15 @@ static int amdgpu_kernel_open_fd(ScrnInfoPtr pScrn, char *busid, > return fd; > } > > +static 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); > +} AMDGPUFreeRec could also be refactored to call this function. > @@ -164,7 +173,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; > } > @@ -252,7 +261,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: These two hunks aren't really necessary, right? These only get called from amdgpu_pci_probe, in which case pAMDGPUEnt->platform_dev remains NULL. > @@ -347,6 +356,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; > @@ -365,7 +375,6 @@ amdgpu_platform_probe(DriverPtr pDriver, > pAMDGPUEnt = pPriv->ptr; > pAMDGPUEnt->fd_ref++; > } > - pAMDGPUEnt->platform_dev = dev; > > xf86SetEntityInstanceForScreen(pScrn, pEnt->index, > xf86GetNumEntityInstances(pEnt-> These two hunks aren't really necessary either, are they? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer