| Message ID | 1511801886-6753-10-git-send-email-okaya@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
+dri-devel@lists.freedesktop.org On 11/27/2017 11:57 AM, Sinan Kaya wrote: > pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as > where a PCI device is present. This restricts the device drivers to be > reused for other domain numbers. > > Getting ready to remove pci_get_bus_and_slot() function in favor of > pci_get_domain_bus_and_slot(). > > Extract the domain number from drm_device and pass it into > pci_get_domain_bus_and_slot() function. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > drivers/gpu/drm/i915/i915_drv.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 9f45cfe..5a8cb79 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -419,7 +419,10 @@ static int i915_getparam(struct drm_device *dev, void *data, > > static int i915_get_bridge_dev(struct drm_i915_private *dev_priv) > { > - dev_priv->bridge_dev = pci_get_bus_and_slot(0, PCI_DEVFN(0, 0)); > + int domain = pci_domain_nr(dev_priv->drm.pdev->bus); > + > + dev_priv->bridge_dev = > + pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0)); > if (!dev_priv->bridge_dev) { > DRM_ERROR("bridge device not found\n"); > return -1; >
On Mon, Nov 27, 2017 at 11:57:46AM -0500, Sinan Kaya wrote: > pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as > where a PCI device is present. This restricts the device drivers to be > reused for other domain numbers. > > Getting ready to remove pci_get_bus_and_slot() function in favor of > pci_get_domain_bus_and_slot(). > > Extract the domain number from drm_device and pass it into > pci_get_domain_bus_and_slot() function. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > --- > drivers/gpu/drm/i915/i915_drv.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 9f45cfe..5a8cb79 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -419,7 +419,10 @@ static int i915_getparam(struct drm_device *dev, void *data, > > static int i915_get_bridge_dev(struct drm_i915_private *dev_priv) > { > - dev_priv->bridge_dev = pci_get_bus_and_slot(0, PCI_DEVFN(0, 0)); > + int domain = pci_domain_nr(dev_priv->drm.pdev->bus); > + > + dev_priv->bridge_dev = > + pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0)); Maybe just pci_get_slot(pdev->bus, PCI_DEVFN(0, 0)) ? I guess if we want to be pedantic we could go for: bus = pci_find_host_bridge(pdev->bus)->bus; pci_get_slot(bus, PCI_DEVFN(0, 0)) but I think the GPU should always be on the root bus, so the simpler form should be fine. > if (!dev_priv->bridge_dev) { > DRM_ERROR("bridge device not found\n"); > return -1; > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 11/28/2017 10:30 AM, Ville Syrjälä wrote: >> + dev_priv->bridge_dev = >> + pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0)); > Maybe just pci_get_slot(pdev->bus, PCI_DEVFN(0, 0)) ? > > I guess if we want to be pedantic we could go for: > > bus = pci_find_host_bridge(pdev->bus)->bus; > pci_get_slot(bus, PCI_DEVFN(0, 0)) > > but I think the GPU should always be on the root bus, so the simpler > form should be fine. > All three of these should be correct. I'll use pci_get_slot(pdev->bus, PCI_DEVFN(0, 0)) as you suggested.
On 11/28/2017 11:29 AM, Sinan Kaya wrote: > On 11/28/2017 10:30 AM, Ville Syrjälä wrote: >>> + dev_priv->bridge_dev = >>> + pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0)); >> Maybe just pci_get_slot(pdev->bus, PCI_DEVFN(0, 0)) ? >> >> I guess if we want to be pedantic we could go for: >> >> bus = pci_find_host_bridge(pdev->bus)->bus; >> pci_get_slot(bus, PCI_DEVFN(0, 0)) >> >> but I think the GPU should always be on the root bus, so the simpler >> form should be fine. >> > > All three of these should be correct. > > I'll use pci_get_slot(pdev->bus, PCI_DEVFN(0, 0)) as you suggested. > Now that I think about this more, I think my version is a simpler change and does not introduce "new features" by assuming GPU and host to be on the same bus similar to the original code. Original code could have used pci_get_slot() too. Since all of them are correct, mine is slightly more correct; I'd like to keep mine.
Hi, I sent this individual i915 patch to our CI, and it is passing on all platforms: https://patchwork.freedesktop.org/series/34822/ Is it ok if I merge this to drm-tip already? Regards, Joonas On Mon, 2017-11-27 at 13:50 -0500, Sinan Kaya wrote: > +dri-devel@lists.freedesktop.org > > On 11/27/2017 11:57 AM, Sinan Kaya wrote: > > pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as > > where a PCI device is present. This restricts the device drivers to be > > reused for other domain numbers. > > > > Getting ready to remove pci_get_bus_and_slot() function in favor of > > pci_get_domain_bus_and_slot(). > > > > Extract the domain number from drm_device and pass it into > > pci_get_domain_bus_and_slot() function. > > > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > > --- > > drivers/gpu/drm/i915/i915_drv.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > index 9f45cfe..5a8cb79 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -419,7 +419,10 @@ static int i915_getparam(struct drm_device *dev, void *data, > > > > static int i915_get_bridge_dev(struct drm_i915_private *dev_priv) > > { > > - dev_priv->bridge_dev = pci_get_bus_and_slot(0, PCI_DEVFN(0, 0)); > > + int domain = pci_domain_nr(dev_priv->drm.pdev->bus); > > + > > + dev_priv->bridge_dev = > > + pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0)); > > if (!dev_priv->bridge_dev) { > > DRM_ERROR("bridge device not found\n"); > > return -1; > > > >
On 12/12/2017 9:04 AM, Joonas Lahtinen wrote: > Hi, > > I sent this individual i915 patch to our CI, and it is passing on all platforms: > > https://patchwork.freedesktop.org/series/34822/ > > Is it ok if I merge this to drm-tip already? As long as you have this change in your tree, it should be safe. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/include/linux/pci.h?id=7912af5c835bd86f2b0347a480e0f40e2fab30d0 > > Regards, Joonas >
On Tue, 2017-12-12 at 19:07 -0500, Sinan Kaya wrote: > On 12/12/2017 9:04 AM, Joonas Lahtinen wrote: > > Hi, > > > > I sent this individual i915 patch to our CI, and it is passing on > > all platforms: > > > > https://patchwork.freedesktop.org/series/34822/ > > > > Is it ok if I merge this to drm-tip already? > > As long as you have this change in your tree, it should be safe. > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/include/linux/pci.h?id=7912af5c835bd86f2b0347a480e0f40e2fab30d0 > We don't yet. Rodrigo, can you please pull the above patch in once we get a backmerge? Regards, Joonas
On Mon, Nov 27, 2017 at 11:57:46AM -0500, Sinan Kaya wrote: > pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as > where a PCI device is present. This restricts the device drivers to be > reused for other domain numbers. > > Getting ready to remove pci_get_bus_and_slot() function in favor of > pci_get_domain_bus_and_slot(). > > Extract the domain number from drm_device and pass it into > pci_get_domain_bus_and_slot() function. > > Signed-off-by: Sinan Kaya <okaya@codeaurora.org> I don't know what happened to this, and it didn't make it into v4.16-rc1. I applied it to pci/deprecate-get-bus-and-slot for v4.17 along with the patch that actually removes pci_get_bus_and_slot(). > --- > drivers/gpu/drm/i915/i915_drv.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 9f45cfe..5a8cb79 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -419,7 +419,10 @@ static int i915_getparam(struct drm_device *dev, void *data, > > static int i915_get_bridge_dev(struct drm_i915_private *dev_priv) > { > - dev_priv->bridge_dev = pci_get_bus_and_slot(0, PCI_DEVFN(0, 0)); > + int domain = pci_domain_nr(dev_priv->drm.pdev->bus); > + > + dev_priv->bridge_dev = > + pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0)); > if (!dev_priv->bridge_dev) { > DRM_ERROR("bridge device not found\n"); > return -1; > -- > 1.9.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Fri, 16 Feb 2018, Bjorn Helgaas <helgaas@kernel.org> wrote: > On Mon, Nov 27, 2017 at 11:57:46AM -0500, Sinan Kaya wrote: >> pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as >> where a PCI device is present. This restricts the device drivers to be >> reused for other domain numbers. >> >> Getting ready to remove pci_get_bus_and_slot() function in favor of >> pci_get_domain_bus_and_slot(). >> >> Extract the domain number from drm_device and pass it into >> pci_get_domain_bus_and_slot() function. >> >> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > > I don't know what happened to this, and it didn't make it into > v4.16-rc1. I applied it to pci/deprecate-get-bus-and-slot for v4.17 > along with the patch that actually removes pci_get_bus_and_slot(). It fell between the cracks as we couldn't apply it before getting a backmerge on the dependency. Sorry about that. Ack for merging through your tree. Thanks, Jani. > >> --- >> drivers/gpu/drm/i915/i915_drv.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c >> index 9f45cfe..5a8cb79 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.c >> +++ b/drivers/gpu/drm/i915/i915_drv.c >> @@ -419,7 +419,10 @@ static int i915_getparam(struct drm_device *dev, void *data, >> >> static int i915_get_bridge_dev(struct drm_i915_private *dev_priv) >> { >> - dev_priv->bridge_dev = pci_get_bus_and_slot(0, PCI_DEVFN(0, 0)); >> + int domain = pci_domain_nr(dev_priv->drm.pdev->bus); >> + >> + dev_priv->bridge_dev = >> + pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0)); >> if (!dev_priv->bridge_dev) { >> DRM_ERROR("bridge device not found\n"); >> return -1; >> -- >> 1.9.1 >> >> >> _______________________________________________ >> linux-arm-kernel mailing list >> linux-arm-kernel@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoting Jani Nikula (2018-02-19 11:34:34) > On Fri, 16 Feb 2018, Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Mon, Nov 27, 2017 at 11:57:46AM -0500, Sinan Kaya wrote: > >> pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as > >> where a PCI device is present. This restricts the device drivers to be > >> reused for other domain numbers. > >> > >> Getting ready to remove pci_get_bus_and_slot() function in favor of > >> pci_get_domain_bus_and_slot(). > >> > >> Extract the domain number from drm_device and pass it into > >> pci_get_domain_bus_and_slot() function. > >> > >> Signed-off-by: Sinan Kaya <okaya@codeaurora.org> > > > > I don't know what happened to this, and it didn't make it into > > v4.16-rc1. I applied it to pci/deprecate-get-bus-and-slot for v4.17 > > along with the patch that actually removes pci_get_bus_and_slot(). > > It fell between the cracks as we couldn't apply it before getting a > backmerge on the dependency. Sorry about that. > > Ack for merging through your tree. I just retested the patch and it still passes CI. We also now have the dependency in our tree through the backmerge, so I can send this for the next drm-next pull request. Either way suits me. Regards, Joonas
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 9f45cfe..5a8cb79 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -419,7 +419,10 @@ static int i915_getparam(struct drm_device *dev, void *data, static int i915_get_bridge_dev(struct drm_i915_private *dev_priv) { - dev_priv->bridge_dev = pci_get_bus_and_slot(0, PCI_DEVFN(0, 0)); + int domain = pci_domain_nr(dev_priv->drm.pdev->bus); + + dev_priv->bridge_dev = + pci_get_domain_bus_and_slot(domain, 0, PCI_DEVFN(0, 0)); if (!dev_priv->bridge_dev) { DRM_ERROR("bridge device not found\n"); return -1;
pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as where a PCI device is present. This restricts the device drivers to be reused for other domain numbers. Getting ready to remove pci_get_bus_and_slot() function in favor of pci_get_domain_bus_and_slot(). Extract the domain number from drm_device and pass it into pci_get_domain_bus_and_slot() function. Signed-off-by: Sinan Kaya <okaya@codeaurora.org> --- drivers/gpu/drm/i915/i915_drv.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)