[PATCH libdrm] libdrm: Fix issue about differrent domainID but same BDF

[Emil Velikov]

Hi all,

This patch causes unnecessary round trip by openning the nodes. As
mentioned previously this could be trivially fixed [1].

Even Emily acknowledged that [1], yet the sub-par fix was merged. Can
we revert+fixup this properly?

Thanks
Emil

[1] https://lists.freedesktop.org/archives/amd-gfx/2019-February/031573.html

On Fri, 22 Feb 2019 at 21:05, Alex Deucher <alexdeucher at gmail.com> wrote:
>
> Pushed.  Thanks!
>
> Alex
>
> On Thu, Feb 21, 2019 at 9:36 PM Deng, Emily <Emily.Deng at amd.com> wrote:
> >
> > Hi Alex,
> >     Please help, thanks.
> >
> > Best wishes
> > Emily Deng
> >
> >
> >
> > >-----Original Message-----
> > >From: Alex Deucher <alexdeucher at gmail.com>
> > >Sent: Friday, February 22, 2019 12:13 AM
> > >To: Deng, Emily <Emily.Deng at amd.com>; Maling list - DRI developers <dri-
> > >devel at lists.freedesktop.org>
> > >Cc: amd-gfx list <amd-gfx at lists.freedesktop.org>
> > >Subject: Re: [PATCH libdrm] libdrm: Fix issue about differrent domainID but same
> > >BDF
> > >
> > >On Thu, Feb 14, 2019 at 2:53 AM Emily Deng <Emily.Deng at amd.com> wrote:
> > >>
> > >> For multiple GPUs which has the same BDF, but has different domain ID,
> > >> the drmOpenByBusid will return the wrong fd when startx.
> > >>
> > >> The reproduce sequence as below:
> > >> 1. Call drmOpenByBusid to open Card0, then will return the right fd0,
> > >> and the
> > >> fd0 is master privilege;
> > >> 2. Call drmOpenByBusid to open Card1. In function drmOpenByBusid, it
> > >> will open Card0 first, this time, the fd1 for opening Card0 is not
> > >> master privilege, and will call drmSetInterfaceVersion to identify the
> > >> domain ID feature, as the fd1 is not master privilege, then
> > >> drmSetInterfaceVersion will fail, and then won't compare domain ID, then
> > >return the wrong fd for Card1.
> > >>
> > >> Solution:
> > >> First loop search the best match fd about drm 1.4.
> > >>
> > >> Signed-off-by: Emily Deng <Emily.Deng at amd.com>
> > >
> > >Reviewed-by: Alex Deucher <alexander.deucher at amd.com>
> > >
> > >Do you need someone to commit this for you?
> > >
> > >Alex
> > >
> > >> ---
> > >>  xf86drm.c | 23 +++++++++++++++++++++++
> > >>  1 file changed, 23 insertions(+)
> > >>
> > >> diff --git a/xf86drm.c b/xf86drm.c
> > >> index 336d64d..b60e029 100644
> > >> --- a/xf86drm.c
> > >> +++ b/xf86drm.c
> > >> @@ -584,11 +584,34 @@ static int drmOpenByBusid(const char *busid, int
> > >type)
> > >>      if (base < 0)
> > >>          return -1;
> > >>
> > >> +    /* We need to try for 1.4 first for proper PCI domain support */
> > >>      drmMsg("drmOpenByBusid: Searching for BusID %s\n", busid);
> > >>      for (i = base; i < base + DRM_MAX_MINOR; i++) {
> > >>          fd = drmOpenMinor(i, 1, type);
> > >>          drmMsg("drmOpenByBusid: drmOpenMinor returns %d\n", fd);
> > >>          if (fd >= 0) {
> > >> +            sv.drm_di_major = 1;
> > >> +            sv.drm_di_minor = 4;
> > >> +            sv.drm_dd_major = -1;        /* Don't care */
> > >> +            sv.drm_dd_minor = -1;        /* Don't care */
> > >> +            if (!drmSetInterfaceVersion(fd, &sv)) {
> > >> +                buf = drmGetBusid(fd);
> > >> +                drmMsg("drmOpenByBusid: drmGetBusid reports %s\n", buf);
> > >> +                if (buf && drmMatchBusID(buf, busid, 1)) {
> > >> +                    drmFreeBusid(buf);
> > >> +                    return fd;
> > >> +                }
> > >> +                if (buf)
> > >> +                    drmFreeBusid(buf);
> > >> +            }
> > >> +            close(fd);
> > >> +        }
> > >> +    }
> > >> +
> > >> +   for (i = base; i < base + DRM_MAX_MINOR; i++) {
> > >> +        fd = drmOpenMinor(i, 1, type);
> > >> +        drmMsg("drmOpenByBusid: drmOpenMinor returns %d\n", fd);
> > >> +        if (fd >= 0) {
> > >>              /* We need to try for 1.4 first for proper PCI domain support
> > >>               * and if that fails, we know the kernel is busted
> > >>               */
> > >> --
> > >> 2.7.4
> > >>
> > >> _______________________________________________
> > >> amd-gfx mailing list
> > >> amd-gfx at lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel