[V3,09/29] drm/i915: deprecate pci_get_bus_and_slot()

Message ID 1511801886-6753-10-git-send-email-okaya@codeaurora.org
State New
Delegated to: Bjorn Helgaas
Headers show
Series
  • PCI: deprecate pci_get_bus_and_slot()
Related show

Commit Message

Sinan Kaya Nov. 27, 2017, 4:57 p.m.
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(-)

Comments

Sinan Kaya Nov. 27, 2017, 6:50 p.m. | #1
+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;
>
Ville Syrjälä Nov. 28, 2017, 3:30 p.m. | #2
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
Sinan Kaya Nov. 28, 2017, 4:29 p.m. | #3
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.
Sinan Kaya Dec. 3, 2017, 7:29 p.m. | #4
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.

Patch

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;