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

[Emil Velikov]

On Sat, 22 Feb 2020 at 17:54, Daniel Vetter <daniel.vetter at 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 at intel.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: "Christian König" <christian.koenig at amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou at amd.com>
> Cc: amd-gfx at 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