On Thu, 07 Jun 2012, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> To be able to directly set up the intel-gtt code from drm/i915 and
> avoid setting up the fake-agp driver we need to prepare a few things:
> - pass both the bridge and gpu pci_dev to the probe function and add
> code to handle the gpu pdev both being present (for drm/i915) and
> not present (fake agp).
> - add refcounting to the remove function so that unloading drm/i915
> doesn't kill the fake agp driver
>
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/char/agp/intel-agp.c | 5 +++--
> drivers/char/agp/intel-agp.h | 3 ---
> drivers/char/agp/intel-gtt.c | 38 ++++++++++++++++++++++++++++++--------
> drivers/gpu/drm/i915/i915_dma.c | 13 +++++++++++--
> include/drm/intel-gtt.h | 4 ++++
> 5 files changed, 48 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
> index 764f70c..c98c568 100644
> --- a/drivers/char/agp/intel-agp.c
> +++ b/drivers/char/agp/intel-agp.c
> @@ -12,6 +12,7 @@
> #include <asm/smp.h>
> #include "agp.h"
> #include "intel-agp.h"
> +#include <drm/intel-gtt.h>
>
> int intel_agp_enabled;
> EXPORT_SYMBOL(intel_agp_enabled);
> @@ -747,7 +748,7 @@ static int __devinit agp_intel_probe(struct pci_dev *pdev,
>
> bridge->capndx = cap_ptr;
>
> - if (intel_gmch_probe(pdev, bridge))
> + if (intel_gmch_probe(pdev, NULL, bridge))
> goto found_gmch;
>
> for (i = 0; intel_agp_chipsets[i].name != NULL; i++) {
> @@ -824,7 +825,7 @@ static void __devexit agp_intel_remove(struct pci_dev *pdev)
>
> agp_remove_bridge(bridge);
>
> - intel_gmch_remove(pdev);
> + intel_gmch_remove();
>
> agp_put_bridge(bridge);
> }
> diff --git a/drivers/char/agp/intel-agp.h b/drivers/char/agp/intel-agp.h
> index c009175..cf2e764 100644
> --- a/drivers/char/agp/intel-agp.h
> +++ b/drivers/char/agp/intel-agp.h
> @@ -250,7 +250,4 @@
> #define PCI_DEVICE_ID_INTEL_HASWELL_SDV 0x0c16 /* SDV */
> #define PCI_DEVICE_ID_INTEL_HASWELL_E_HB 0x0c04
>
> -int intel_gmch_probe(struct pci_dev *pdev,
> - struct agp_bridge_data *bridge);
> -void intel_gmch_remove(struct pci_dev *pdev);
> #endif
> diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
> index 5e6c89e..6499290 100644
> --- a/drivers/char/agp/intel-gtt.c
> +++ b/drivers/char/agp/intel-gtt.c
> @@ -75,6 +75,7 @@ static struct _intel_private {
> struct resource ifp_resource;
> int resource_valid;
> struct page *scratch_page;
> + int refcount;
> } intel_private;
>
> #define INTEL_GTT_GEN intel_private.driver->gen
> @@ -1522,14 +1523,32 @@ static int find_gmch(u16 device)
> return 1;
> }
>
> -int intel_gmch_probe(struct pci_dev *pdev,
> - struct agp_bridge_data *bridge)
> +int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
> + struct agp_bridge_data *bridge)
> {
> int i, mask;
> - intel_private.driver = NULL;
A possibly clueless question: should intel_gmch_remove() do this now,
since intel_private.driver and intel_private.refcount are linked
together below?
> +
> + /*
> + * Can be called from the fake agp driver but also directly from
> + * drm/i915.ko. Hence we need to check whether everything is set up
> + * already.
> + */
> + if (intel_private.driver) {
> + intel_private.refcount++;
> + return 1;
> + }
Judging by the commit message, is it intentional that the first call to
probe does not increase the refcount? The cleanup will now happen only
if probe and remove are called twice or more. Should this perhaps be
clarified in a code comment in addition to the commit message?
BR,
Jani.
>
> for (i = 0; intel_gtt_chipsets[i].name != NULL; i++) {
> - if (find_gmch(intel_gtt_chipsets[i].gmch_chip_id)) {
> + if (gpu_pdev) {
> + if (gpu_pdev->device ==
> + intel_gtt_chipsets[i].gmch_chip_id) {
> + intel_private.pcidev = pci_dev_get(gpu_pdev);
> + intel_private.driver =
> + intel_gtt_chipsets[i].gtt_driver;
> +
> + break;
> + }
> + } else if (find_gmch(intel_gtt_chipsets[i].gmch_chip_id)) {
> intel_private.driver =
> intel_gtt_chipsets[i].gtt_driver;
> break;
> @@ -1542,12 +1561,12 @@ int intel_gmch_probe(struct pci_dev *pdev,
> if (bridge) {
> bridge->driver = &intel_fake_agp_driver;
> bridge->dev_private_data = &intel_private;
> - bridge->dev = pdev;
> + bridge->dev = bridge_pdev;
> }
>
> - intel_private.bridge_dev = pci_dev_get(pdev);
> + intel_private.bridge_dev = pci_dev_get(bridge_pdev);
>
> - dev_info(&pdev->dev, "Intel %s Chipset\n", intel_gtt_chipsets[i].name);
> + dev_info(&bridge_pdev->dev, "Intel %s Chipset\n", intel_gtt_chipsets[i].name);
>
> mask = intel_private.driver->dma_mask_size;
> if (pci_set_dma_mask(intel_private.pcidev, DMA_BIT_MASK(mask)))
> @@ -1577,8 +1596,11 @@ void intel_gtt_chipset_flush(void)
> }
> EXPORT_SYMBOL(intel_gtt_chipset_flush);
>
> -void intel_gmch_remove(struct pci_dev *pdev)
> +void intel_gmch_remove(void)
> {
> + if (--intel_private.refcount)
> + return;
> +
> if (intel_private.pcidev)
> pci_dev_put(intel_private.pcidev);
> if (intel_private.bridge_dev)
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 0ab5d3d..29aee99 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1486,11 +1486,18 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> goto put_bridge;
> }
>
> + ret = intel_gmch_probe(dev_priv->bridge_dev, dev->pdev, NULL);
> + if (!ret) {
> + DRM_ERROR("failed to set up gmch\n");
> + ret = -EIO;
> + goto out_rmmap;
> + }
> +
> dev_priv->mm.gtt = intel_gtt_get();
> if (!dev_priv->mm.gtt) {
> DRM_ERROR("Failed to initialize GTT\n");
> ret = -ENODEV;
> - goto out_rmmap;
> + goto put_gmch;
> }
>
> aperture_size = dev_priv->mm.gtt->gtt_mappable_entries << PAGE_SHIFT;
> @@ -1501,7 +1508,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> aperture_size);
> if (dev_priv->mm.gtt_mapping == NULL) {
> ret = -EIO;
> - goto out_rmmap;
> + goto put_gmch;
> }
>
> i915_mtrr_setup(dev_priv, dev_priv->mm.gtt_base_addr,
> @@ -1623,6 +1630,8 @@ out_mtrrfree:
> dev_priv->mm.gtt_mtrr = -1;
> }
> io_mapping_free(dev_priv->mm.gtt_mapping);
> +put_gmch:
> + intel_gmch_remove();
> out_rmmap:
> pci_iounmap(dev->pdev, dev_priv->regs);
> put_bridge:
> diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
> index 8048c00..84ebd71 100644
> --- a/include/drm/intel-gtt.h
> +++ b/include/drm/intel-gtt.h
> @@ -23,6 +23,10 @@ const struct intel_gtt {
> phys_addr_t gma_bus_addr;
> } *intel_gtt_get(void);
>
> +int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
> + struct agp_bridge_data *bridge);
> +void intel_gmch_remove(void);
> +
> void intel_gtt_chipset_flush(void);
> void intel_gtt_unmap_memory(struct scatterlist *sg_list, int num_sg);
> void intel_gtt_clear_range(unsigned int first_entry, unsigned int num_entries);
> --
> 1.7.7.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
