xf86: allow DDX driver for GPU/PCI hot-plug

The current X server infrastructure sets modesetting driver as default driver to handle PCI-hotplug of a GPU device. This prevents the respective DDX driver (like AMDGPU DDX driver) to take control of the card.

This patch:

• fine-tunes the PCI-hotplug infrastructure to allow the respective DDX driver to be loaded.
• scans and updates the PCI device list before adding the new GPU device in platform, so that the association of the platform device and PCI device is in place (dev->pdev).

Cc: Alex Deucher alexandar.deucher@amd.com

Reviewed-by: Adam Jackson ajax@redhat.com Acked-by: Alex Deucher alexander.deucher@amd.com Acked-by: Martin Roukala martin.roukala@mupuf.org Signed-off-by: Shashank Sharma shashank.sharma@amd.com

Acked-by: Alex Deucher alexander.deucher@amd.com

approved this merge request

@daenzer @mupuf : Would you guys mind having a look at the patch pls ?

• Shashank

@p12tic may I know if this patch looks good to merge, or any suggestions on code review ?

• Shashank

unapproved this merge request

added 3 commits

• c54c1b24...2e6edeb5 - 2 commits from branch xorg:master
• 2bbcce77 - xf86: allow DDX driver for GPU/PCI hot-plug

Compare with previous version

It will take me some time to review the patch. Ideally the reviewer would be someone else, but I can be a reviewer of last resort.

@ajax any comments if you have a chance?

Looks plausible to me.

Reviewed-by: Adam Jackson <ajax@redhat.com>

Thank you Adam.

added 12 commits

• 2bbcce77...7d2014e7 - 11 commits from branch xorg:master
• f3d25584 - xf86: allow DDX driver for GPU/PCI hot-plug

Compare with previous version

@p12tic : Does it looks good to merge now ?

• Shashank

@contactshashanksharma You will need to update your patch to add Alex's and Adam's feedback.

While you are at it, you may also fix the typo in your commit message (resoective -> respective), and you can add my Acked-by: Martin Roukala <martin.roukala@mupuf.org> :)

Got it @mupuf, and hello Updated the commit message to reflect the feedbacks.

• Shashank

Heya, you updated the description of the MR, not the commit. Mind updating the actual commit message?

changed the description

I'll give this a try with nvidia_drv.so real quick. I had a patch series to do something similar ages ago but ran into problems with the server's screen private handling not working well with hotplug.

Okay, it looks like this doesn't actually cause the server to try nvidia_drv because the driver name it gets is "nvidia-drm" and there's no X driver with that name:

(II) LoadModule: "nvidia-drm"
(WW) Warning, couldn't open module nvidia-drm
(EE) Failed to load module "nvidia-drm" (module does not exist, 0)

That's probably just as well since hotplugging the nvidia driver doesn't work yet.

So the current patch seems alright for now. Eventually if we sort out the hotplug issues between xserver and the nvidia driver, we'll probably want something like this patch: aplattner/xserver@655e59f2

Hey Aaron, thanks for trying this series out for Nvidia setup. This code extracts the driver name from the attribs->driver info, so probably nvidia DDX driver's name is not getting reflected properly in the attributes. Just for such cases we have the fallback method where we will still continue with the modesetting driver (which was the original code flow).

• Shashank

It's not that it's not getting reflected properly, but rather that the driver name reported here is the name of the DRM kernel driver, which isn't necessarily the same as the name of the Xorg driver that corresponds to that device. So here with nvidia-drm vs. nvidia, but see also i915 vs. intel.

The proper way to correlate a DRM driver name to an Xorg driver name is via OutputClass, so just taking the DRM driver name and trying to load an Xorg module with the same name is only going to work in some cases by coincidence. IMO it would be better to take something like my patch to apply OutputClass-based matches and then add something like a new DriverFunc flag to allow drivers to opt into hotplug support.

Hey Aaron, Thanks for this input, let me have a look on the output class based approach.

• Shashank

@mupuf @agd5f @ajax @aplattner @p12tic : While updating the merge request for V2, I by mistake created a new MR 879 (!879 (diffs)). (I so miss git send-email :)). No change in the code, but just commit messgae. Can we please contiue the merge process there ?

• Shashank

closed

mentioned in merge request !879 (merged)

mentioned in merge request !903 (closed)