[PATCH 1/3] drm/amdgpu: Drop DRIVER_USE_AGP

[PATCH 1/3] drm/amdgpu: Drop DRIVER_USE_AGP

[Daniel Vetter]

This doesn't do anything except auto-init drm_agp support when you
call drm_get_pci_dev(). Which amdgpu stopped doing with

commit b58c11314a1706bf094c489ef5cb28f76478c704
Author: Alex Deucher <alexander.deucher@amd.com>
Date:   Fri Jun 2 17:16:31 2017 -0400

    drm/amdgpu: drop deprecated drm_get_pci_dev and drm_put_dev

No idea whether this was intentional or accidental breakage, but I
guess anyone who manages to boot a this modern gpu behind an agp
bridge deserves a price. A price I never expect anyone to ever collect
:-)

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Hawking Zhang <Hawking.Zhang@amd.com>
Cc: Xiaojie Yuan <xiaojie.yuan@amd.com>
Cc: Evan Quan <evan.quan@amd.com>
Cc: "Tianci.Yin" <tianci.yin@amd.com>
Cc: "Marek Olšák" <marek.olsak@amd.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 4598836c5fa4..6cea92017109 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1379,7 +1379,7 @@ int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv)
 
 static struct drm_driver kms_driver = {
 	.driver_features =
-	    DRIVER_USE_AGP | DRIVER_ATOMIC |
+	    DRIVER_ATOMIC |
 	    DRIVER_GEM |
 	    DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ |
 	    DRIVER_SYNCOBJ_TIMELINE,
-- 
2.24.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Daniel Vetter]

It's the last user, and more importantly, it's the last non-legacy
user of anything in drm_pci.c.

The only tricky bit is the agp initialization. But a close look shows
that radeon does not use the drm_agp midlayer (the main use of that is
drm_bufs for legacy drivers), and instead could use the agp subsystem
directly (like nouveau does already). Hence we can just pull this in
too.

A further step would be to entirely drop the use of drm_device->agp,
but feels like too much churn just for this patch.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/radeon_drv.c | 43 +++++++++++++++++++++++++++--
 drivers/gpu/drm/radeon/radeon_kms.c |  6 ++++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 49ce2e7d5f9e..59f8186a2415 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -37,6 +37,7 @@
 #include <linux/vga_switcheroo.h>
 #include <linux/mmu_notifier.h>
 
+#include <drm/drm_agpsupport.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_helper.h>
@@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *ent)
 {
 	unsigned long flags = 0;
+	struct drm_device *dev;
 	int ret;
 
 	if (!ent)
@@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
-	return drm_get_pci_dev(pdev, ent, &kms_driver);
+	dev = drm_dev_alloc(&kms_driver, &pdev->dev);
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
+
+	ret = pci_enable_device(pdev);
+	if (ret)
+		goto err_free;
+
+	dev->pdev = pdev;
+#ifdef __alpha__
+	dev->hose = pdev->sysdata;
+#endif
+
+	pci_set_drvdata(pdev, dev);
+
+	if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
+		dev->agp = drm_agp_init(dev);
+	if (dev->agp) {
+		dev->agp->agp_mtrr = arch_phys_wc_add(
+			dev->agp->agp_info.aper_base,
+			dev->agp->agp_info.aper_size *
+			1024 * 1024);
+	}
+
+	ret = drm_dev_register(dev, ent->driver_data);
+	if (ret)
+		goto err_agp;
+
+	return 0;
+
+err_agp:
+	if (dev->agp)
+		arch_phys_wc_del(dev->agp->agp_mtrr);
+	kfree(dev->agp);
+	pci_disable_device(pdev);
+err_free:
+	drm_dev_put(dev);
+	return ret;
 }
 
 static void
@@ -562,7 +601,7 @@ static const struct file_operations radeon_driver_kms_fops = {
 
 static struct drm_driver kms_driver = {
 	.driver_features =
-	    DRIVER_USE_AGP | DRIVER_GEM | DRIVER_RENDER,
+	    DRIVER_GEM | DRIVER_RENDER,
 	.load = radeon_driver_load_kms,
 	.open = radeon_driver_open_kms,
 	.postclose = radeon_driver_postclose_kms,
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index cab891f86dc0..58176db85952 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -32,6 +32,7 @@
 #include <linux/uaccess.h>
 #include <linux/vga_switcheroo.h>
 
+#include <drm/drm_agpsupport.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_file.h>
 #include <drm/drm_ioctl.h>
@@ -77,6 +78,11 @@ void radeon_driver_unload_kms(struct drm_device *dev)
 	radeon_modeset_fini(rdev);
 	radeon_device_fini(rdev);
 
+	if (dev->agp)
+		arch_phys_wc_del(dev->agp->agp_mtrr);
+	kfree(dev->agp);
+	dev->agp = NULL;
+
 done_free:
 	kfree(rdev);
 	dev->dev_private = NULL;
-- 
2.24.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Daniel Vetter]

It's the last user, and more importantly, it's the last non-legacy
user of anything in drm_pci.c.

The only tricky bit is the agp initialization. But a close look shows
that radeon does not use the drm_agp midlayer (the main use of that is
drm_bufs for legacy drivers), and instead could use the agp subsystem
directly (like nouveau does already). Hence we can just pull this in
too.

A further step would be to entirely drop the use of drm_device->agp,
but feels like too much churn just for this patch.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: amd-gfx@lists.freedesktop.org
---
 drivers/gpu/drm/radeon/radeon_drv.c | 43 +++++++++++++++++++++++++++--
 drivers/gpu/drm/radeon/radeon_kms.c |  6 ++++
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 49ce2e7d5f9e..59f8186a2415 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -37,6 +37,7 @@
 #include <linux/vga_switcheroo.h>
 #include <linux/mmu_notifier.h>
 
+#include <drm/drm_agpsupport.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_fb_helper.h>
@@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *ent)
 {
 	unsigned long flags = 0;
+	struct drm_device *dev;
 	int ret;
 
 	if (!ent)
@@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
 	if (ret)
 		return ret;
 
-	return drm_get_pci_dev(pdev, ent, &kms_driver);
+	dev = drm_dev_alloc(&kms_driver, &pdev->dev);
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
+
+	ret = pci_enable_device(pdev);
+	if (ret)
+		goto err_free;
+
+	dev->pdev = pdev;
+#ifdef __alpha__
+	dev->hose = pdev->sysdata;
+#endif
+
+	pci_set_drvdata(pdev, dev);
+
+	if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
+		dev->agp = drm_agp_init(dev);
+	if (dev->agp) {
+		dev->agp->agp_mtrr = arch_phys_wc_add(
+			dev->agp->agp_info.aper_base,
+			dev->agp->agp_info.aper_size *
+			1024 * 1024);
+	}
+
+	ret = drm_dev_register(dev, ent->driver_data);
+	if (ret)
+		goto err_agp;
+
+	return 0;
+
+err_agp:
+	if (dev->agp)
+		arch_phys_wc_del(dev->agp->agp_mtrr);
+	kfree(dev->agp);
+	pci_disable_device(pdev);
+err_free:
+	drm_dev_put(dev);
+	return ret;
 }
 
 static void
@@ -562,7 +601,7 @@ static const struct file_operations radeon_driver_kms_fops = {
 
 static struct drm_driver kms_driver = {
 	.driver_features =
-	    DRIVER_USE_AGP | DRIVER_GEM | DRIVER_RENDER,
+	    DRIVER_GEM | DRIVER_RENDER,
 	.load = radeon_driver_load_kms,
 	.open = radeon_driver_open_kms,
 	.postclose = radeon_driver_postclose_kms,
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index cab891f86dc0..58176db85952 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -32,6 +32,7 @@
 #include <linux/uaccess.h>
 #include <linux/vga_switcheroo.h>
 
+#include <drm/drm_agpsupport.h>
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_file.h>
 #include <drm/drm_ioctl.h>
@@ -77,6 +78,11 @@ void radeon_driver_unload_kms(struct drm_device *dev)
 	radeon_modeset_fini(rdev);
 	radeon_device_fini(rdev);
 
+	if (dev->agp)
+		arch_phys_wc_del(dev->agp->agp_mtrr);
+	kfree(dev->agp);
+	dev->agp = NULL;
+
 done_free:
 	kfree(rdev);
 	dev->dev_private = NULL;
-- 
2.24.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/3] drm/pci: Unexport drm_get_pci_dev

[Daniel Vetter]

Only user left is the shadow attach for legacy drivers.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_pci.c | 22 +++-------------------
 include/drm/drm_pci.h     | 11 -----------
 2 files changed, 3 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index c6bb98729a26..cc5af271a1b1 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -75,7 +75,6 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali
 
 	return dmah;
 }
-
 EXPORT_SYMBOL(drm_pci_alloc);
 
 /**
@@ -191,23 +190,9 @@ void drm_pci_agp_destroy(struct drm_device *dev)
 	}
 }
 
-/**
- * drm_get_pci_dev - Register a PCI device with the DRM subsystem
- * @pdev: PCI device
- * @ent: entry from the PCI ID table that matches @pdev
- * @driver: DRM device driver
- *
- * Attempt to gets inter module "drm" information. If we are first
- * then register the character device and inter module information.
- * Try and register, if we fail to register, backout previous work.
- *
- * NOTE: This function is deprecated, please use drm_dev_alloc() and
- * drm_dev_register() instead and remove your &drm_driver.load callback.
- *
- * Return: 0 on success or a negative error code on failure.
- */
-int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
-		    struct drm_driver *driver)
+static int drm_get_pci_dev(struct pci_dev *pdev,
+			   const struct pci_device_id *ent,
+			   struct drm_driver *driver)
 {
 	struct drm_device *dev;
 	int ret;
@@ -250,7 +235,6 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 	drm_dev_put(dev);
 	return ret;
 }
-EXPORT_SYMBOL(drm_get_pci_dev);
 
 #ifdef CONFIG_DRM_LEGACY
 
diff --git a/include/drm/drm_pci.h b/include/drm/drm_pci.h
index 9031e217b506..3941b0255ecf 100644
--- a/include/drm/drm_pci.h
+++ b/include/drm/drm_pci.h
@@ -45,10 +45,6 @@ struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size,
 				     size_t align);
 void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
 
-int drm_get_pci_dev(struct pci_dev *pdev,
-		    const struct pci_device_id *ent,
-		    struct drm_driver *driver);
-
 #else
 
 static inline struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev,
@@ -62,13 +58,6 @@ static inline void drm_pci_free(struct drm_device *dev,
 {
 }
 
-static inline int drm_get_pci_dev(struct pci_dev *pdev,
-				  const struct pci_device_id *ent,
-				  struct drm_driver *driver)
-{
-	return -ENOSYS;
-}
-
 #endif
 
 #endif /* _DRM_PCI_H_ */
-- 
2.24.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/amdgpu: Drop DRIVER_USE_AGP

[Emil Velikov]

On Sat, 22 Feb 2020 at 17:54, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> This doesn't do anything except auto-init drm_agp support when you
> call drm_get_pci_dev(). Which amdgpu stopped doing with
>
> commit b58c11314a1706bf094c489ef5cb28f76478c704
> Author: Alex Deucher <alexander.deucher@amd.com>
> Date:   Fri Jun 2 17:16:31 2017 -0400
>
>     drm/amdgpu: drop deprecated drm_get_pci_dev and drm_put_dev
>
> No idea whether this was intentional or accidental breakage, but I
> guess anyone who manages to boot a this modern gpu behind an agp
> bridge deserves a price. A price I never expect anyone to ever collect
> :-)
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Hawking Zhang <Hawking.Zhang@amd.com>
> Cc: Xiaojie Yuan <xiaojie.yuan@amd.com>
> Cc: Evan Quan <evan.quan@amd.com>
> Cc: "Tianci.Yin" <tianci.yin@amd.com>
> Cc: "Marek Olšák" <marek.olsak@amd.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

I've had this patch locally for ages, but never sent it out:

Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/amdgpu: Drop DRIVER_USE_AGP

[Alex Deucher]

On Sat, Feb 22, 2020 at 12:54 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> This doesn't do anything except auto-init drm_agp support when you
> call drm_get_pci_dev(). Which amdgpu stopped doing with
>
> commit b58c11314a1706bf094c489ef5cb28f76478c704
> Author: Alex Deucher <alexander.deucher@amd.com>
> Date:   Fri Jun 2 17:16:31 2017 -0400
>
>     drm/amdgpu: drop deprecated drm_get_pci_dev and drm_put_dev
>
> No idea whether this was intentional or accidental breakage, but I
> guess anyone who manages to boot a this modern gpu behind an agp
> bridge deserves a price. A price I never expect anyone to ever collect
> :-)
>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Hawking Zhang <Hawking.Zhang@amd.com>
> Cc: Xiaojie Yuan <xiaojie.yuan@amd.com>
> Cc: Evan Quan <evan.quan@amd.com>
> Cc: "Tianci.Yin" <tianci.yin@amd.com>
> Cc: "Marek Olšák" <marek.olsak@amd.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Series is:
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
I'm happy to take the patches through my tree or drm-misc.

Alex


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 4598836c5fa4..6cea92017109 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1379,7 +1379,7 @@ int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv)
>
>  static struct drm_driver kms_driver = {
>         .driver_features =
> -           DRIVER_USE_AGP | DRIVER_ATOMIC |
> +           DRIVER_ATOMIC |
>             DRIVER_GEM |
>             DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ |
>             DRIVER_SYNCOBJ_TIMELINE,
> --
> 2.24.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Emil Velikov]

On Sat, 22 Feb 2020 at 17:54, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> It's the last user, and more importantly, it's the last non-legacy
> user of anything in drm_pci.c.
>
> The only tricky bit is the agp initialization. But a close look shows
> that radeon does not use the drm_agp midlayer (the main use of that is
> drm_bufs for legacy drivers), and instead could use the agp subsystem
> directly (like nouveau does already). Hence we can just pull this in
> too.
>
> A further step would be to entirely drop the use of drm_device->agp,
> but feels like too much churn just for this patch.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> ---
>  drivers/gpu/drm/radeon/radeon_drv.c | 43 +++++++++++++++++++++++++++--
>  drivers/gpu/drm/radeon/radeon_kms.c |  6 ++++
>  2 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 49ce2e7d5f9e..59f8186a2415 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -37,6 +37,7 @@
>  #include <linux/vga_switcheroo.h>
>  #include <linux/mmu_notifier.h>
>
> +#include <drm/drm_agpsupport.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_fb_helper.h>
> @@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>                             const struct pci_device_id *ent)
>  {
>         unsigned long flags = 0;
> +       struct drm_device *dev;
>         int ret;
>
>         if (!ent)
> @@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>         if (ret)
>                 return ret;
>
> -       return drm_get_pci_dev(pdev, ent, &kms_driver);
> +       dev = drm_dev_alloc(&kms_driver, &pdev->dev);
> +       if (IS_ERR(dev))
> +               return PTR_ERR(dev);
> +
> +       ret = pci_enable_device(pdev);
> +       if (ret)
> +               goto err_free;
> +
> +       dev->pdev = pdev;
> +#ifdef __alpha__
> +       dev->hose = pdev->sysdata;
> +#endif
> +
> +       pci_set_drvdata(pdev, dev);
> +
> +       if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> +               dev->agp = drm_agp_init(dev);
> +       if (dev->agp) {
> +               dev->agp->agp_mtrr = arch_phys_wc_add(
> +                       dev->agp->agp_info.aper_base,
> +                       dev->agp->agp_info.aper_size *
> +                       1024 * 1024);
> +       }
> +
IMHO a better solution is kill off the drm_agpsupport.c dependency all together.
As-is it's still used, making the legacy vs not line really moot.

Especially, since the AGP ioctl (in the legacy code) can manipulate
the underlying state.

Off the top of my head, in radeon_agp_init():
 - at the top agp_backend_acquire() + agp_copy_info()
 - followed up by existing mode magic
 - opencode the enable - agp_enable() + acquired = true;
 - mtrr = arch_phys_wc_add() and the rest

In radeon_agp_fini()
 - if !acquired { agp_backend_release(); acquired = false }


Something like ^^ should result in a net diffstat of around zero.
All thanks to the interesting layer that drm_agp is ;-)

With this in place we can make move drm_device::agp and
DRM_IOCTL_AGP_INFO behind CONFIG_DRM_LEGACY.

-Emil
P.S. Watch out for radeon_ttm.c warnings/errors
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Emil Velikov]

On Sat, 22 Feb 2020 at 17:54, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> It's the last user, and more importantly, it's the last non-legacy
> user of anything in drm_pci.c.
>
> The only tricky bit is the agp initialization. But a close look shows
> that radeon does not use the drm_agp midlayer (the main use of that is
> drm_bufs for legacy drivers), and instead could use the agp subsystem
> directly (like nouveau does already). Hence we can just pull this in
> too.
>
> A further step would be to entirely drop the use of drm_device->agp,
> but feels like too much churn just for this patch.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: amd-gfx@lists.freedesktop.org
> ---
>  drivers/gpu/drm/radeon/radeon_drv.c | 43 +++++++++++++++++++++++++++--
>  drivers/gpu/drm/radeon/radeon_kms.c |  6 ++++
>  2 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 49ce2e7d5f9e..59f8186a2415 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -37,6 +37,7 @@
>  #include <linux/vga_switcheroo.h>
>  #include <linux/mmu_notifier.h>
>
> +#include <drm/drm_agpsupport.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_fb_helper.h>
> @@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>                             const struct pci_device_id *ent)
>  {
>         unsigned long flags = 0;
> +       struct drm_device *dev;
>         int ret;
>
>         if (!ent)
> @@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>         if (ret)
>                 return ret;
>
> -       return drm_get_pci_dev(pdev, ent, &kms_driver);
> +       dev = drm_dev_alloc(&kms_driver, &pdev->dev);
> +       if (IS_ERR(dev))
> +               return PTR_ERR(dev);
> +
> +       ret = pci_enable_device(pdev);
> +       if (ret)
> +               goto err_free;
> +
> +       dev->pdev = pdev;
> +#ifdef __alpha__
> +       dev->hose = pdev->sysdata;
> +#endif
> +
> +       pci_set_drvdata(pdev, dev);
> +
> +       if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> +               dev->agp = drm_agp_init(dev);
> +       if (dev->agp) {
> +               dev->agp->agp_mtrr = arch_phys_wc_add(
> +                       dev->agp->agp_info.aper_base,
> +                       dev->agp->agp_info.aper_size *
> +                       1024 * 1024);
> +       }
> +
IMHO a better solution is kill off the drm_agpsupport.c dependency all together.
As-is it's still used, making the legacy vs not line really moot.

Especially, since the AGP ioctl (in the legacy code) can manipulate
the underlying state.

Off the top of my head, in radeon_agp_init():
 - at the top agp_backend_acquire() + agp_copy_info()
 - followed up by existing mode magic
 - opencode the enable - agp_enable() + acquired = true;
 - mtrr = arch_phys_wc_add() and the rest

In radeon_agp_fini()
 - if !acquired { agp_backend_release(); acquired = false }


Something like ^^ should result in a net diffstat of around zero.
All thanks to the interesting layer that drm_agp is ;-)

With this in place we can make move drm_device::agp and
DRM_IOCTL_AGP_INFO behind CONFIG_DRM_LEGACY.

-Emil
P.S. Watch out for radeon_ttm.c warnings/errors
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/3] drm/pci: Unexport drm_get_pci_dev

[Emil Velikov]

On Sat, 22 Feb 2020 at 17:54, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> Only user left is the shadow attach for legacy drivers.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Going the extra step, as outlined in 2/3 would be great. But if the
series goes as-is, 2/3 and 3/3 are:
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/amdgpu: Drop DRIVER_USE_AGP

[Daniel Vetter]

On Mon, Feb 24, 2020 at 5:10 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>
> On Sat, Feb 22, 2020 at 12:54 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > This doesn't do anything except auto-init drm_agp support when you
> > call drm_get_pci_dev(). Which amdgpu stopped doing with
> >
> > commit b58c11314a1706bf094c489ef5cb28f76478c704
> > Author: Alex Deucher <alexander.deucher@amd.com>
> > Date:   Fri Jun 2 17:16:31 2017 -0400
> >
> >     drm/amdgpu: drop deprecated drm_get_pci_dev and drm_put_dev
> >
> > No idea whether this was intentional or accidental breakage, but I
> > guess anyone who manages to boot a this modern gpu behind an agp
> > bridge deserves a price. A price I never expect anyone to ever collect
> > :-)
> >
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: Hawking Zhang <Hawking.Zhang@amd.com>
> > Cc: Xiaojie Yuan <xiaojie.yuan@amd.com>
> > Cc: Evan Quan <evan.quan@amd.com>
> > Cc: "Tianci.Yin" <tianci.yin@amd.com>
> > Cc: "Marek Olšák" <marek.olsak@amd.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>
> Series is:
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> I'm happy to take the patches through my tree or drm-misc.

I don't have anything building on top of this, just random things from
my tree. Reason I sent it out is Laurent's series to make a const
drm_driver possible, but I don't think they'll conflict. So amd trees
for the series is perfectly fine and probably simplest.
-Daniel

>
> Alex
>
>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > index 4598836c5fa4..6cea92017109 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > @@ -1379,7 +1379,7 @@ int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv)
> >
> >  static struct drm_driver kms_driver = {
> >         .driver_features =
> > -           DRIVER_USE_AGP | DRIVER_ATOMIC |
> > +           DRIVER_ATOMIC |
> >             DRIVER_GEM |
> >             DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ |
> >             DRIVER_SYNCOBJ_TIMELINE,
> > --
> > 2.24.1
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Daniel Vetter]

On Mon, Feb 24, 2020 at 5:31 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Sat, 22 Feb 2020 at 17:54, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > It's the last user, and more importantly, it's the last non-legacy
> > user of anything in drm_pci.c.
> >
> > The only tricky bit is the agp initialization. But a close look shows
> > that radeon does not use the drm_agp midlayer (the main use of that is
> > drm_bufs for legacy drivers), and instead could use the agp subsystem
> > directly (like nouveau does already). Hence we can just pull this in
> > too.
> >
> > A further step would be to entirely drop the use of drm_device->agp,
> > but feels like too much churn just for this patch.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > Cc: amd-gfx@lists.freedesktop.org
> > ---
> >  drivers/gpu/drm/radeon/radeon_drv.c | 43 +++++++++++++++++++++++++++--
> >  drivers/gpu/drm/radeon/radeon_kms.c |  6 ++++
> >  2 files changed, 47 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> > index 49ce2e7d5f9e..59f8186a2415 100644
> > --- a/drivers/gpu/drm/radeon/radeon_drv.c
> > +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> > @@ -37,6 +37,7 @@
> >  #include <linux/vga_switcheroo.h>
> >  #include <linux/mmu_notifier.h>
> >
> > +#include <drm/drm_agpsupport.h>
> >  #include <drm/drm_crtc_helper.h>
> >  #include <drm/drm_drv.h>
> >  #include <drm/drm_fb_helper.h>
> > @@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> >                             const struct pci_device_id *ent)
> >  {
> >         unsigned long flags = 0;
> > +       struct drm_device *dev;
> >         int ret;
> >
> >         if (!ent)
> > @@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> >         if (ret)
> >                 return ret;
> >
> > -       return drm_get_pci_dev(pdev, ent, &kms_driver);
> > +       dev = drm_dev_alloc(&kms_driver, &pdev->dev);
> > +       if (IS_ERR(dev))
> > +               return PTR_ERR(dev);
> > +
> > +       ret = pci_enable_device(pdev);
> > +       if (ret)
> > +               goto err_free;
> > +
> > +       dev->pdev = pdev;
> > +#ifdef __alpha__
> > +       dev->hose = pdev->sysdata;
> > +#endif
> > +
> > +       pci_set_drvdata(pdev, dev);
> > +
> > +       if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> > +               dev->agp = drm_agp_init(dev);
> > +       if (dev->agp) {
> > +               dev->agp->agp_mtrr = arch_phys_wc_add(
> > +                       dev->agp->agp_info.aper_base,
> > +                       dev->agp->agp_info.aper_size *
> > +                       1024 * 1024);
> > +       }
> > +
> IMHO a better solution is kill off the drm_agpsupport.c dependency all together.
> As-is it's still used, making the legacy vs not line really moot.
>
> Especially, since the AGP ioctl (in the legacy code) can manipulate
> the underlying state.
>
> Off the top of my head, in radeon_agp_init():
>  - at the top agp_backend_acquire() + agp_copy_info()
>  - followed up by existing mode magic
>  - opencode the enable - agp_enable() + acquired = true;
>  - mtrr = arch_phys_wc_add() and the rest
>
> In radeon_agp_fini()
>  - if !acquired { agp_backend_release(); acquired = false }
>
>
> Something like ^^ should result in a net diffstat of around zero.
> All thanks to the interesting layer that drm_agp is ;-)
>
> With this in place we can make move drm_device::agp and
> DRM_IOCTL_AGP_INFO behind CONFIG_DRM_LEGACY.

Yeah could be done, but I was more out to get the drm_pci.c stuff, not
the agp stuff. But I did realize that nouveau also just directly
accesses the agp bridge stuff, so we could indeed nuke the midlayer
here. The legacy agp stuff ioctl stuff should be behind DRIVER_LEGACY
checks already, so no way to cause harm that way (I hope at least, if
there's a gap we'd need to close it).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Daniel Vetter]

On Mon, Feb 24, 2020 at 5:31 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
>
> On Sat, 22 Feb 2020 at 17:54, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> >
> > It's the last user, and more importantly, it's the last non-legacy
> > user of anything in drm_pci.c.
> >
> > The only tricky bit is the agp initialization. But a close look shows
> > that radeon does not use the drm_agp midlayer (the main use of that is
> > drm_bufs for legacy drivers), and instead could use the agp subsystem
> > directly (like nouveau does already). Hence we can just pull this in
> > too.
> >
> > A further step would be to entirely drop the use of drm_device->agp,
> > but feels like too much churn just for this patch.
> >
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > Cc: amd-gfx@lists.freedesktop.org
> > ---
> >  drivers/gpu/drm/radeon/radeon_drv.c | 43 +++++++++++++++++++++++++++--
> >  drivers/gpu/drm/radeon/radeon_kms.c |  6 ++++
> >  2 files changed, 47 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> > index 49ce2e7d5f9e..59f8186a2415 100644
> > --- a/drivers/gpu/drm/radeon/radeon_drv.c
> > +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> > @@ -37,6 +37,7 @@
> >  #include <linux/vga_switcheroo.h>
> >  #include <linux/mmu_notifier.h>
> >
> > +#include <drm/drm_agpsupport.h>
> >  #include <drm/drm_crtc_helper.h>
> >  #include <drm/drm_drv.h>
> >  #include <drm/drm_fb_helper.h>
> > @@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> >                             const struct pci_device_id *ent)
> >  {
> >         unsigned long flags = 0;
> > +       struct drm_device *dev;
> >         int ret;
> >
> >         if (!ent)
> > @@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> >         if (ret)
> >                 return ret;
> >
> > -       return drm_get_pci_dev(pdev, ent, &kms_driver);
> > +       dev = drm_dev_alloc(&kms_driver, &pdev->dev);
> > +       if (IS_ERR(dev))
> > +               return PTR_ERR(dev);
> > +
> > +       ret = pci_enable_device(pdev);
> > +       if (ret)
> > +               goto err_free;
> > +
> > +       dev->pdev = pdev;
> > +#ifdef __alpha__
> > +       dev->hose = pdev->sysdata;
> > +#endif
> > +
> > +       pci_set_drvdata(pdev, dev);
> > +
> > +       if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> > +               dev->agp = drm_agp_init(dev);
> > +       if (dev->agp) {
> > +               dev->agp->agp_mtrr = arch_phys_wc_add(
> > +                       dev->agp->agp_info.aper_base,
> > +                       dev->agp->agp_info.aper_size *
> > +                       1024 * 1024);
> > +       }
> > +
> IMHO a better solution is kill off the drm_agpsupport.c dependency all together.
> As-is it's still used, making the legacy vs not line really moot.
>
> Especially, since the AGP ioctl (in the legacy code) can manipulate
> the underlying state.
>
> Off the top of my head, in radeon_agp_init():
>  - at the top agp_backend_acquire() + agp_copy_info()
>  - followed up by existing mode magic
>  - opencode the enable - agp_enable() + acquired = true;
>  - mtrr = arch_phys_wc_add() and the rest
>
> In radeon_agp_fini()
>  - if !acquired { agp_backend_release(); acquired = false }
>
>
> Something like ^^ should result in a net diffstat of around zero.
> All thanks to the interesting layer that drm_agp is ;-)
>
> With this in place we can make move drm_device::agp and
> DRM_IOCTL_AGP_INFO behind CONFIG_DRM_LEGACY.

Yeah could be done, but I was more out to get the drm_pci.c stuff, not
the agp stuff. But I did realize that nouveau also just directly
accesses the agp bridge stuff, so we could indeed nuke the midlayer
here. The legacy agp stuff ioctl stuff should be behind DRIVER_LEGACY
checks already, so no way to cause harm that way (I hope at least, if
there's a gap we'd need to close it).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Daniel Vetter]

On Mon, Feb 24, 2020 at 9:46 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> On Mon, Feb 24, 2020 at 5:31 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > On Sat, 22 Feb 2020 at 17:54, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > It's the last user, and more importantly, it's the last non-legacy
> > > user of anything in drm_pci.c.
> > >
> > > The only tricky bit is the agp initialization. But a close look shows
> > > that radeon does not use the drm_agp midlayer (the main use of that is
> > > drm_bufs for legacy drivers), and instead could use the agp subsystem
> > > directly (like nouveau does already). Hence we can just pull this in
> > > too.
> > >
> > > A further step would be to entirely drop the use of drm_device->agp,
> > > but feels like too much churn just for this patch.
> > >
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > Cc: "Christian König" <christian.koenig@amd.com>
> > > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > > Cc: amd-gfx@lists.freedesktop.org
> > > ---
> > >  drivers/gpu/drm/radeon/radeon_drv.c | 43 +++++++++++++++++++++++++++--
> > >  drivers/gpu/drm/radeon/radeon_kms.c |  6 ++++
> > >  2 files changed, 47 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> > > index 49ce2e7d5f9e..59f8186a2415 100644
> > > --- a/drivers/gpu/drm/radeon/radeon_drv.c
> > > +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> > > @@ -37,6 +37,7 @@
> > >  #include <linux/vga_switcheroo.h>
> > >  #include <linux/mmu_notifier.h>
> > >
> > > +#include <drm/drm_agpsupport.h>
> > >  #include <drm/drm_crtc_helper.h>
> > >  #include <drm/drm_drv.h>
> > >  #include <drm/drm_fb_helper.h>
> > > @@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> > >                             const struct pci_device_id *ent)
> > >  {
> > >         unsigned long flags = 0;
> > > +       struct drm_device *dev;
> > >         int ret;
> > >
> > >         if (!ent)
> > > @@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> > >         if (ret)
> > >                 return ret;
> > >
> > > -       return drm_get_pci_dev(pdev, ent, &kms_driver);
> > > +       dev = drm_dev_alloc(&kms_driver, &pdev->dev);
> > > +       if (IS_ERR(dev))
> > > +               return PTR_ERR(dev);
> > > +
> > > +       ret = pci_enable_device(pdev);
> > > +       if (ret)
> > > +               goto err_free;
> > > +
> > > +       dev->pdev = pdev;
> > > +#ifdef __alpha__
> > > +       dev->hose = pdev->sysdata;
> > > +#endif
> > > +
> > > +       pci_set_drvdata(pdev, dev);
> > > +
> > > +       if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> > > +               dev->agp = drm_agp_init(dev);
> > > +       if (dev->agp) {
> > > +               dev->agp->agp_mtrr = arch_phys_wc_add(
> > > +                       dev->agp->agp_info.aper_base,
> > > +                       dev->agp->agp_info.aper_size *
> > > +                       1024 * 1024);
> > > +       }
> > > +
> > IMHO a better solution is kill off the drm_agpsupport.c dependency all together.
> > As-is it's still used, making the legacy vs not line really moot.
> >
> > Especially, since the AGP ioctl (in the legacy code) can manipulate
> > the underlying state.
> >
> > Off the top of my head, in radeon_agp_init():
> >  - at the top agp_backend_acquire() + agp_copy_info()
> >  - followed up by existing mode magic
> >  - opencode the enable - agp_enable() + acquired = true;
> >  - mtrr = arch_phys_wc_add() and the rest
> >
> > In radeon_agp_fini()
> >  - if !acquired { agp_backend_release(); acquired = false }
> >
> >
> > Something like ^^ should result in a net diffstat of around zero.
> > All thanks to the interesting layer that drm_agp is ;-)
> >
> > With this in place we can make move drm_device::agp and
> > DRM_IOCTL_AGP_INFO behind CONFIG_DRM_LEGACY.
>
> Yeah could be done, but I was more out to get the drm_pci.c stuff, not
> the agp stuff. But I did realize that nouveau also just directly
> accesses the agp bridge stuff, so we could indeed nuke the midlayer
> here. The legacy agp stuff ioctl stuff should be behind DRIVER_LEGACY
> checks already, so no way to cause harm that way (I hope at least, if
> there's a gap we'd need to close it).

Haha, I should read code before hitting send.

It's totally wide open root hole. I guess no one ever bothered to run
a fuzzer on a radeon.ko or amdgpu.ko (before the patch to stop using
drm_get_pci_dev at least) hw. And I never cared since we've killed the
fake agp stuff that i915.ko used a long time ago.

So yeah I think at least sprinkling DRIVER_LEGACY checks over all
this, and reviewing old radeon userspace, would be really good. Once
we have that we can then do the second step and retire the agp
midlayer. But first we need to add the uapi checks/changes.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Daniel Vetter]

On Mon, Feb 24, 2020 at 9:46 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> On Mon, Feb 24, 2020 at 5:31 PM Emil Velikov <emil.l.velikov@gmail.com> wrote:
> >
> > On Sat, 22 Feb 2020 at 17:54, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > It's the last user, and more importantly, it's the last non-legacy
> > > user of anything in drm_pci.c.
> > >
> > > The only tricky bit is the agp initialization. But a close look shows
> > > that radeon does not use the drm_agp midlayer (the main use of that is
> > > drm_bufs for legacy drivers), and instead could use the agp subsystem
> > > directly (like nouveau does already). Hence we can just pull this in
> > > too.
> > >
> > > A further step would be to entirely drop the use of drm_device->agp,
> > > but feels like too much churn just for this patch.
> > >
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > Cc: "Christian König" <christian.koenig@amd.com>
> > > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > > Cc: amd-gfx@lists.freedesktop.org
> > > ---
> > >  drivers/gpu/drm/radeon/radeon_drv.c | 43 +++++++++++++++++++++++++++--
> > >  drivers/gpu/drm/radeon/radeon_kms.c |  6 ++++
> > >  2 files changed, 47 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> > > index 49ce2e7d5f9e..59f8186a2415 100644
> > > --- a/drivers/gpu/drm/radeon/radeon_drv.c
> > > +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> > > @@ -37,6 +37,7 @@
> > >  #include <linux/vga_switcheroo.h>
> > >  #include <linux/mmu_notifier.h>
> > >
> > > +#include <drm/drm_agpsupport.h>
> > >  #include <drm/drm_crtc_helper.h>
> > >  #include <drm/drm_drv.h>
> > >  #include <drm/drm_fb_helper.h>
> > > @@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> > >                             const struct pci_device_id *ent)
> > >  {
> > >         unsigned long flags = 0;
> > > +       struct drm_device *dev;
> > >         int ret;
> > >
> > >         if (!ent)
> > > @@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
> > >         if (ret)
> > >                 return ret;
> > >
> > > -       return drm_get_pci_dev(pdev, ent, &kms_driver);
> > > +       dev = drm_dev_alloc(&kms_driver, &pdev->dev);
> > > +       if (IS_ERR(dev))
> > > +               return PTR_ERR(dev);
> > > +
> > > +       ret = pci_enable_device(pdev);
> > > +       if (ret)
> > > +               goto err_free;
> > > +
> > > +       dev->pdev = pdev;
> > > +#ifdef __alpha__
> > > +       dev->hose = pdev->sysdata;
> > > +#endif
> > > +
> > > +       pci_set_drvdata(pdev, dev);
> > > +
> > > +       if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> > > +               dev->agp = drm_agp_init(dev);
> > > +       if (dev->agp) {
> > > +               dev->agp->agp_mtrr = arch_phys_wc_add(
> > > +                       dev->agp->agp_info.aper_base,
> > > +                       dev->agp->agp_info.aper_size *
> > > +                       1024 * 1024);
> > > +       }
> > > +
> > IMHO a better solution is kill off the drm_agpsupport.c dependency all together.
> > As-is it's still used, making the legacy vs not line really moot.
> >
> > Especially, since the AGP ioctl (in the legacy code) can manipulate
> > the underlying state.
> >
> > Off the top of my head, in radeon_agp_init():
> >  - at the top agp_backend_acquire() + agp_copy_info()
> >  - followed up by existing mode magic
> >  - opencode the enable - agp_enable() + acquired = true;
> >  - mtrr = arch_phys_wc_add() and the rest
> >
> > In radeon_agp_fini()
> >  - if !acquired { agp_backend_release(); acquired = false }
> >
> >
> > Something like ^^ should result in a net diffstat of around zero.
> > All thanks to the interesting layer that drm_agp is ;-)
> >
> > With this in place we can make move drm_device::agp and
> > DRM_IOCTL_AGP_INFO behind CONFIG_DRM_LEGACY.
>
> Yeah could be done, but I was more out to get the drm_pci.c stuff, not
> the agp stuff. But I did realize that nouveau also just directly
> accesses the agp bridge stuff, so we could indeed nuke the midlayer
> here. The legacy agp stuff ioctl stuff should be behind DRIVER_LEGACY
> checks already, so no way to cause harm that way (I hope at least, if
> there's a gap we'd need to close it).

Haha, I should read code before hitting send.

It's totally wide open root hole. I guess no one ever bothered to run
a fuzzer on a radeon.ko or amdgpu.ko (before the patch to stop using
drm_get_pci_dev at least) hw. And I never cared since we've killed the
fake agp stuff that i915.ko used a long time ago.

So yeah I think at least sprinkling DRIVER_LEGACY checks over all
this, and reviewing old radeon userspace, would be really good. Once
we have that we can then do the second step and retire the agp
midlayer. But first we need to add the uapi checks/changes.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/3] drm/pci: Unexport drm_get_pci_dev

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 3273 bytes --]

Hi

Am 22.02.20 um 18:54 schrieb Daniel Vetter:
> Only user left is the shadow attach for legacy drivers.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_pci.c | 22 +++-------------------
>  include/drm/drm_pci.h     | 11 -----------
>  2 files changed, 3 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index c6bb98729a26..cc5af271a1b1 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -75,7 +75,6 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali
>  
>  	return dmah;
>  }
> -
>  EXPORT_SYMBOL(drm_pci_alloc);
>  
>  /**
> @@ -191,23 +190,9 @@ void drm_pci_agp_destroy(struct drm_device *dev)
>  	}
>  }
>  
> -/**
> - * drm_get_pci_dev - Register a PCI device with the DRM subsystem
> - * @pdev: PCI device
> - * @ent: entry from the PCI ID table that matches @pdev
> - * @driver: DRM device driver
> - *
> - * Attempt to gets inter module "drm" information. If we are first
> - * then register the character device and inter module information.
> - * Try and register, if we fail to register, backout previous work.
> - *
> - * NOTE: This function is deprecated, please use drm_dev_alloc() and
> - * drm_dev_register() instead and remove your &drm_driver.load callback.
> - *
> - * Return: 0 on success or a negative error code on failure.
> - */
> -int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
> -		    struct drm_driver *driver)
> +static int drm_get_pci_dev(struct pci_dev *pdev,
> +			   const struct pci_device_id *ent,
> +			   struct drm_driver *driver)
>  {
>  	struct drm_device *dev;
>  	int ret;
> @@ -250,7 +235,6 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
>  	drm_dev_put(dev);
>  	return ret;
>  }
> -EXPORT_SYMBOL(drm_get_pci_dev);
>  
>  #ifdef CONFIG_DRM_LEGACY

drm_get_pci_dev() is now only used by some legacy code. It should be
protected by CONFIG_DRM_LEGACY. With this change

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

>  
> diff --git a/include/drm/drm_pci.h b/include/drm/drm_pci.h
> index 9031e217b506..3941b0255ecf 100644
> --- a/include/drm/drm_pci.h
> +++ b/include/drm/drm_pci.h
> @@ -45,10 +45,6 @@ struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size,
>  				     size_t align);
>  void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
>  
> -int drm_get_pci_dev(struct pci_dev *pdev,
> -		    const struct pci_device_id *ent,
> -		    struct drm_driver *driver);
> -
>  #else
>  
>  static inline struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev,
> @@ -62,13 +58,6 @@ static inline void drm_pci_free(struct drm_device *dev,
>  {
>  }
>  
> -static inline int drm_get_pci_dev(struct pci_dev *pdev,
> -				  const struct pci_device_id *ent,
> -				  struct drm_driver *driver)
> -{
> -	return -ENOSYS;
> -}
> -
>  #endif
>  
>  #endif /* _DRM_PCI_H_ */
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] drm/amdgpu: Drop DRIVER_USE_AGP

[Alex Deucher]

On Mon, Feb 24, 2020 at 3:43 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> On Mon, Feb 24, 2020 at 5:10 PM Alex Deucher <alexdeucher@gmail.com> wrote:
> >
> > On Sat, Feb 22, 2020 at 12:54 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > >
> > > This doesn't do anything except auto-init drm_agp support when you
> > > call drm_get_pci_dev(). Which amdgpu stopped doing with
> > >
> > > commit b58c11314a1706bf094c489ef5cb28f76478c704
> > > Author: Alex Deucher <alexander.deucher@amd.com>
> > > Date:   Fri Jun 2 17:16:31 2017 -0400
> > >
> > >     drm/amdgpu: drop deprecated drm_get_pci_dev and drm_put_dev
> > >
> > > No idea whether this was intentional or accidental breakage, but I
> > > guess anyone who manages to boot a this modern gpu behind an agp
> > > bridge deserves a price. A price I never expect anyone to ever collect
> > > :-)
> > >
> > > Cc: Alex Deucher <alexander.deucher@amd.com>
> > > Cc: "Christian König" <christian.koenig@amd.com>
> > > Cc: Hawking Zhang <Hawking.Zhang@amd.com>
> > > Cc: Xiaojie Yuan <xiaojie.yuan@amd.com>
> > > Cc: Evan Quan <evan.quan@amd.com>
> > > Cc: "Tianci.Yin" <tianci.yin@amd.com>
> > > Cc: "Marek Olšák" <marek.olsak@amd.com>
> > > Cc: Hans de Goede <hdegoede@redhat.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >
> > Series is:
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> > I'm happy to take the patches through my tree or drm-misc.
>
> I don't have anything building on top of this, just random things from
> my tree. Reason I sent it out is Laurent's series to make a const
> drm_driver possible, but I don't think they'll conflict. So amd trees
> for the series is perfectly fine and probably simplest.

Applied.  thanks!

Alex

> -Daniel
>
> >
> > Alex
> >
> >
> > > ---
> > >  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > index 4598836c5fa4..6cea92017109 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> > > @@ -1379,7 +1379,7 @@ int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv)
> > >
> > >  static struct drm_driver kms_driver = {
> > >         .driver_features =
> > > -           DRIVER_USE_AGP | DRIVER_ATOMIC |
> > > +           DRIVER_ATOMIC |
> > >             DRIVER_GEM |
> > >             DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ |
> > >             DRIVER_SYNCOBJ_TIMELINE,
> > > --
> > > 2.24.1
> > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Alex Deucher]

On Sat, Feb 22, 2020 at 12:54 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> It's the last user, and more importantly, it's the last non-legacy
> user of anything in drm_pci.c.
>
> The only tricky bit is the agp initialization. But a close look shows
> that radeon does not use the drm_agp midlayer (the main use of that is
> drm_bufs for legacy drivers), and instead could use the agp subsystem
> directly (like nouveau does already). Hence we can just pull this in
> too.
>
> A further step would be to entirely drop the use of drm_device->agp,
> but feels like too much churn just for this patch.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: amd-gfx@lists.freedesktop.org

Applied.  Thanks!

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_drv.c | 43 +++++++++++++++++++++++++++--
>  drivers/gpu/drm/radeon/radeon_kms.c |  6 ++++
>  2 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 49ce2e7d5f9e..59f8186a2415 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -37,6 +37,7 @@
>  #include <linux/vga_switcheroo.h>
>  #include <linux/mmu_notifier.h>
>
> +#include <drm/drm_agpsupport.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_fb_helper.h>
> @@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>                             const struct pci_device_id *ent)
>  {
>         unsigned long flags = 0;
> +       struct drm_device *dev;
>         int ret;
>
>         if (!ent)
> @@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>         if (ret)
>                 return ret;
>
> -       return drm_get_pci_dev(pdev, ent, &kms_driver);
> +       dev = drm_dev_alloc(&kms_driver, &pdev->dev);
> +       if (IS_ERR(dev))
> +               return PTR_ERR(dev);
> +
> +       ret = pci_enable_device(pdev);
> +       if (ret)
> +               goto err_free;
> +
> +       dev->pdev = pdev;
> +#ifdef __alpha__
> +       dev->hose = pdev->sysdata;
> +#endif
> +
> +       pci_set_drvdata(pdev, dev);
> +
> +       if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> +               dev->agp = drm_agp_init(dev);
> +       if (dev->agp) {
> +               dev->agp->agp_mtrr = arch_phys_wc_add(
> +                       dev->agp->agp_info.aper_base,
> +                       dev->agp->agp_info.aper_size *
> +                       1024 * 1024);
> +       }
> +
> +       ret = drm_dev_register(dev, ent->driver_data);
> +       if (ret)
> +               goto err_agp;
> +
> +       return 0;
> +
> +err_agp:
> +       if (dev->agp)
> +               arch_phys_wc_del(dev->agp->agp_mtrr);
> +       kfree(dev->agp);
> +       pci_disable_device(pdev);
> +err_free:
> +       drm_dev_put(dev);
> +       return ret;
>  }
>
>  static void
> @@ -562,7 +601,7 @@ static const struct file_operations radeon_driver_kms_fops = {
>
>  static struct drm_driver kms_driver = {
>         .driver_features =
> -           DRIVER_USE_AGP | DRIVER_GEM | DRIVER_RENDER,
> +           DRIVER_GEM | DRIVER_RENDER,
>         .load = radeon_driver_load_kms,
>         .open = radeon_driver_open_kms,
>         .postclose = radeon_driver_postclose_kms,
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index cab891f86dc0..58176db85952 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -32,6 +32,7 @@
>  #include <linux/uaccess.h>
>  #include <linux/vga_switcheroo.h>
>
> +#include <drm/drm_agpsupport.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_file.h>
>  #include <drm/drm_ioctl.h>
> @@ -77,6 +78,11 @@ void radeon_driver_unload_kms(struct drm_device *dev)
>         radeon_modeset_fini(rdev);
>         radeon_device_fini(rdev);
>
> +       if (dev->agp)
> +               arch_phys_wc_del(dev->agp->agp_mtrr);
> +       kfree(dev->agp);
> +       dev->agp = NULL;
> +
>  done_free:
>         kfree(rdev);
>         dev->dev_private = NULL;
> --
> 2.24.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/3] drm/radeon: Inline drm_get_pci_dev

[Alex Deucher]

On Sat, Feb 22, 2020 at 12:54 PM Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> It's the last user, and more importantly, it's the last non-legacy
> user of anything in drm_pci.c.
>
> The only tricky bit is the agp initialization. But a close look shows
> that radeon does not use the drm_agp midlayer (the main use of that is
> drm_bufs for legacy drivers), and instead could use the agp subsystem
> directly (like nouveau does already). Hence we can just pull this in
> too.
>
> A further step would be to entirely drop the use of drm_device->agp,
> but feels like too much churn just for this patch.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: amd-gfx@lists.freedesktop.org

Applied.  Thanks!

Alex

> ---
>  drivers/gpu/drm/radeon/radeon_drv.c | 43 +++++++++++++++++++++++++++--
>  drivers/gpu/drm/radeon/radeon_kms.c |  6 ++++
>  2 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 49ce2e7d5f9e..59f8186a2415 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -37,6 +37,7 @@
>  #include <linux/vga_switcheroo.h>
>  #include <linux/mmu_notifier.h>
>
> +#include <drm/drm_agpsupport.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_drv.h>
>  #include <drm/drm_fb_helper.h>
> @@ -322,6 +323,7 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>                             const struct pci_device_id *ent)
>  {
>         unsigned long flags = 0;
> +       struct drm_device *dev;
>         int ret;
>
>         if (!ent)
> @@ -362,7 +364,44 @@ static int radeon_pci_probe(struct pci_dev *pdev,
>         if (ret)
>                 return ret;
>
> -       return drm_get_pci_dev(pdev, ent, &kms_driver);
> +       dev = drm_dev_alloc(&kms_driver, &pdev->dev);
> +       if (IS_ERR(dev))
> +               return PTR_ERR(dev);
> +
> +       ret = pci_enable_device(pdev);
> +       if (ret)
> +               goto err_free;
> +
> +       dev->pdev = pdev;
> +#ifdef __alpha__
> +       dev->hose = pdev->sysdata;
> +#endif
> +
> +       pci_set_drvdata(pdev, dev);
> +
> +       if (pci_find_capability(dev->pdev, PCI_CAP_ID_AGP))
> +               dev->agp = drm_agp_init(dev);
> +       if (dev->agp) {
> +               dev->agp->agp_mtrr = arch_phys_wc_add(
> +                       dev->agp->agp_info.aper_base,
> +                       dev->agp->agp_info.aper_size *
> +                       1024 * 1024);
> +       }
> +
> +       ret = drm_dev_register(dev, ent->driver_data);
> +       if (ret)
> +               goto err_agp;
> +
> +       return 0;
> +
> +err_agp:
> +       if (dev->agp)
> +               arch_phys_wc_del(dev->agp->agp_mtrr);
> +       kfree(dev->agp);
> +       pci_disable_device(pdev);
> +err_free:
> +       drm_dev_put(dev);
> +       return ret;
>  }
>
>  static void
> @@ -562,7 +601,7 @@ static const struct file_operations radeon_driver_kms_fops = {
>
>  static struct drm_driver kms_driver = {
>         .driver_features =
> -           DRIVER_USE_AGP | DRIVER_GEM | DRIVER_RENDER,
> +           DRIVER_GEM | DRIVER_RENDER,
>         .load = radeon_driver_load_kms,
>         .open = radeon_driver_open_kms,
>         .postclose = radeon_driver_postclose_kms,
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index cab891f86dc0..58176db85952 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -32,6 +32,7 @@
>  #include <linux/uaccess.h>
>  #include <linux/vga_switcheroo.h>
>
> +#include <drm/drm_agpsupport.h>
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_file.h>
>  #include <drm/drm_ioctl.h>
> @@ -77,6 +78,11 @@ void radeon_driver_unload_kms(struct drm_device *dev)
>         radeon_modeset_fini(rdev);
>         radeon_device_fini(rdev);
>
> +       if (dev->agp)
> +               arch_phys_wc_del(dev->agp->agp_mtrr);
> +       kfree(dev->agp);
> +       dev->agp = NULL;
> +
>  done_free:
>         kfree(rdev);
>         dev->dev_private = NULL;
> --
> 2.24.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH] drm/pci: Unexport drm_get_pci_dev

[Daniel Vetter]

Only user left is the shadow attach for legacy drivers.

v2: Shift the #ifdef CONFIG_DRM_LEGACY to now also include
drm_get_pci_dev() (Thomas)

Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Emil Velikov <emil.velikov@collabora.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_pci.c | 26 +++++---------------------
 include/drm/drm_pci.h     | 11 -----------
 2 files changed, 5 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index c6bb98729a26..5218475ad7e7 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -75,7 +75,6 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali
 
 	return dmah;
 }
-
 EXPORT_SYMBOL(drm_pci_alloc);
 
 /**
@@ -191,23 +190,11 @@ void drm_pci_agp_destroy(struct drm_device *dev)
 	}
 }
 
-/**
- * drm_get_pci_dev - Register a PCI device with the DRM subsystem
- * @pdev: PCI device
- * @ent: entry from the PCI ID table that matches @pdev
- * @driver: DRM device driver
- *
- * Attempt to gets inter module "drm" information. If we are first
- * then register the character device and inter module information.
- * Try and register, if we fail to register, backout previous work.
- *
- * NOTE: This function is deprecated, please use drm_dev_alloc() and
- * drm_dev_register() instead and remove your &drm_driver.load callback.
- *
- * Return: 0 on success or a negative error code on failure.
- */
-int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
-		    struct drm_driver *driver)
+#ifdef CONFIG_DRM_LEGACY
+
+static int drm_get_pci_dev(struct pci_dev *pdev,
+			   const struct pci_device_id *ent,
+			   struct drm_driver *driver)
 {
 	struct drm_device *dev;
 	int ret;
@@ -250,9 +237,6 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
 	drm_dev_put(dev);
 	return ret;
 }
-EXPORT_SYMBOL(drm_get_pci_dev);
-
-#ifdef CONFIG_DRM_LEGACY
 
 /**
  * drm_legacy_pci_init - shadow-attach a legacy DRM PCI driver
diff --git a/include/drm/drm_pci.h b/include/drm/drm_pci.h
index 9031e217b506..3941b0255ecf 100644
--- a/include/drm/drm_pci.h
+++ b/include/drm/drm_pci.h
@@ -45,10 +45,6 @@ struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size,
 				     size_t align);
 void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
 
-int drm_get_pci_dev(struct pci_dev *pdev,
-		    const struct pci_device_id *ent,
-		    struct drm_driver *driver);
-
 #else
 
 static inline struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev,
@@ -62,13 +58,6 @@ static inline void drm_pci_free(struct drm_device *dev,
 {
 }
 
-static inline int drm_get_pci_dev(struct pci_dev *pdev,
-				  const struct pci_device_id *ent,
-				  struct drm_driver *driver)
-{
-	return -ENOSYS;
-}
-
 #endif
 
 #endif /* _DRM_PCI_H_ */
-- 
2.24.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/pci: Unexport drm_get_pci_dev

[Daniel Vetter]

On Tue, Feb 25, 2020 at 05:58:35PM +0100, Daniel Vetter wrote:
> Only user left is the shadow attach for legacy drivers.
> 
> v2: Shift the #ifdef CONFIG_DRM_LEGACY to now also include
> drm_get_pci_dev() (Thomas)
> 
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Emil Velikov <emil.velikov@collabora.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Emil Velikov <emil.velikov@collabora.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>

Pushed to drm-misc-next now that the radeon/amdgpu patches have been
backmerged.
-Daniel

> ---
>  drivers/gpu/drm/drm_pci.c | 26 +++++---------------------
>  include/drm/drm_pci.h     | 11 -----------
>  2 files changed, 5 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index c6bb98729a26..5218475ad7e7 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -75,7 +75,6 @@ drm_dma_handle_t *drm_pci_alloc(struct drm_device * dev, size_t size, size_t ali
>  
>  	return dmah;
>  }
> -
>  EXPORT_SYMBOL(drm_pci_alloc);
>  
>  /**
> @@ -191,23 +190,11 @@ void drm_pci_agp_destroy(struct drm_device *dev)
>  	}
>  }
>  
> -/**
> - * drm_get_pci_dev - Register a PCI device with the DRM subsystem
> - * @pdev: PCI device
> - * @ent: entry from the PCI ID table that matches @pdev
> - * @driver: DRM device driver
> - *
> - * Attempt to gets inter module "drm" information. If we are first
> - * then register the character device and inter module information.
> - * Try and register, if we fail to register, backout previous work.
> - *
> - * NOTE: This function is deprecated, please use drm_dev_alloc() and
> - * drm_dev_register() instead and remove your &drm_driver.load callback.
> - *
> - * Return: 0 on success or a negative error code on failure.
> - */
> -int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
> -		    struct drm_driver *driver)
> +#ifdef CONFIG_DRM_LEGACY
> +
> +static int drm_get_pci_dev(struct pci_dev *pdev,
> +			   const struct pci_device_id *ent,
> +			   struct drm_driver *driver)
>  {
>  	struct drm_device *dev;
>  	int ret;
> @@ -250,9 +237,6 @@ int drm_get_pci_dev(struct pci_dev *pdev, const struct pci_device_id *ent,
>  	drm_dev_put(dev);
>  	return ret;
>  }
> -EXPORT_SYMBOL(drm_get_pci_dev);
> -
> -#ifdef CONFIG_DRM_LEGACY
>  
>  /**
>   * drm_legacy_pci_init - shadow-attach a legacy DRM PCI driver
> diff --git a/include/drm/drm_pci.h b/include/drm/drm_pci.h
> index 9031e217b506..3941b0255ecf 100644
> --- a/include/drm/drm_pci.h
> +++ b/include/drm/drm_pci.h
> @@ -45,10 +45,6 @@ struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size,
>  				     size_t align);
>  void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
>  
> -int drm_get_pci_dev(struct pci_dev *pdev,
> -		    const struct pci_device_id *ent,
> -		    struct drm_driver *driver);
> -
>  #else
>  
>  static inline struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev,
> @@ -62,13 +58,6 @@ static inline void drm_pci_free(struct drm_device *dev,
>  {
>  }
>  
> -static inline int drm_get_pci_dev(struct pci_dev *pdev,
> -				  const struct pci_device_id *ent,
> -				  struct drm_driver *driver)
> -{
> -	return -ENOSYS;
> -}
> -
>  #endif
>  
>  #endif /* _DRM_PCI_H_ */
> -- 
> 2.24.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel