* [PATCH 01/12] drm: move drm_pci_agp_init into driver load functions
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
@ 2012-06-07 13:55 ` Daniel Vetter
2012-06-08 9:16 ` Jani Nikula
2012-06-07 13:55 ` [PATCH 02/12] drm: kill the REQUIRE_AGP driver flag Daniel Vetter
` (11 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:55 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
I've done an audit of everything which is being set up between the
place where drm_pci_agp_init is called currently and where the
driver's ->load function is called. Nothing seems to depend upon
dev->agp being set up correctly.
This is one big step into squashing this giant midlayer mistaken.
Though one that drm/i915 is hurting especially: We don't need (and
want) the agp layer on many chips, but we need to decide whether we
want to enable it too early.
By moving the agp setup into the drivers control, we can do an
intelligent decision in drm/i915 whether we need agp support (for
anything ums and some horrible kms userspace on specific generations)
or whether it's not required.
Also, this allows us to rip out a bit of the agp invasion from generic
drm core code in follow-up patches.
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/drm_pci.c | 2 +-
drivers/gpu/drm/drm_stub.c | 8 --------
drivers/gpu/drm/i810/i810_dma.c | 6 ++++++
drivers/gpu/drm/i915/i915_dma.c | 4 ++++
drivers/gpu/drm/mga/mga_dma.c | 4 ++++
drivers/gpu/drm/nouveau/nouveau_state.c | 4 ++++
drivers/gpu/drm/r128/r128_drv.c | 6 ++++++
drivers/gpu/drm/radeon/radeon_cp.c | 4 ++++
drivers/gpu/drm/radeon/radeon_kms.c | 4 ++++
drivers/gpu/drm/savage/savage_bci.c | 5 +++++
drivers/gpu/drm/sis/sis_drv.c | 5 +++++
drivers/gpu/drm/via/via_map.c | 4 ++++
include/drm/drmP.h | 5 +----
13 files changed, 48 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 13f3d93..833e599 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -286,6 +286,7 @@ int drm_pci_agp_init(struct drm_device *dev)
}
return 0;
}
+EXPORT_SYMBOL(drm_pci_agp_init);
static struct drm_bus drm_pci_bus = {
.bus_type = DRIVER_BUS_PCI,
@@ -294,7 +295,6 @@ static struct drm_bus drm_pci_bus = {
.set_busid = drm_pci_set_busid,
.set_unique = drm_pci_set_unique,
.irq_by_busid = drm_pci_irq_by_busid,
- .agp_init = drm_pci_agp_init,
};
/**
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
index 21bcd4a..4a4a1ed 100644
--- a/drivers/gpu/drm/drm_stub.c
+++ b/drivers/gpu/drm/drm_stub.c
@@ -289,14 +289,6 @@ int drm_fill_in_dev(struct drm_device *dev,
dev->driver = driver;
- if (dev->driver->bus->agp_init) {
- retcode = dev->driver->bus->agp_init(dev);
- if (retcode)
- goto error_out_unreg;
- }
-
-
-
retcode = drm_ctxbitmap_init(dev);
if (retcode) {
DRM_ERROR("Cannot allocate memory for context bitmap.\n");
diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c
index f920fb5..eed1126 100644
--- a/drivers/gpu/drm/i810/i810_dma.c
+++ b/drivers/gpu/drm/i810/i810_dma.c
@@ -1198,6 +1198,12 @@ static int i810_flip_bufs(struct drm_device *dev, void *data,
int i810_driver_load(struct drm_device *dev, unsigned long flags)
{
+ int ret;
+
+ ret = drm_pci_agp_init(dev);
+ if (ret)
+ return ret;
+
/* i810 has 4 more counters */
dev->counters += 4;
dev->types[6] = _DRM_STAT_IRQ;
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 97a5a58..b97ef73 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1422,6 +1422,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
int ret = 0, mmio_bar;
uint32_t aperture_size;
+ ret = drm_pci_agp_init(dev);
+ if (ret)
+ return ret;
+
info = (struct intel_device_info *) flags;
/* Refuse to load on gen6+ without kms enabled. */
diff --git a/drivers/gpu/drm/mga/mga_dma.c b/drivers/gpu/drm/mga/mga_dma.c
index 507aa3d..549816e 100644
--- a/drivers/gpu/drm/mga/mga_dma.c
+++ b/drivers/gpu/drm/mga/mga_dma.c
@@ -394,6 +394,10 @@ int mga_driver_load(struct drm_device *dev, unsigned long flags)
drm_mga_private_t *dev_priv;
int ret;
+ ret = drm_pci_agp_init(dev);
+ if (ret)
+ return ret;
+
dev_priv = kzalloc(sizeof(drm_mga_private_t), GFP_KERNEL);
if (!dev_priv)
return -ENOMEM;
diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index 19706f0..dbc881a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -1029,6 +1029,10 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
uint32_t reg0 = ~0, strap;
int ret;
+ ret = drm_pci_agp_init(dev);
+ if (ret)
+ return ret;
+
dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
if (!dev_priv) {
ret = -ENOMEM;
diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c
index 88718fa..22001ca 100644
--- a/drivers/gpu/drm/r128/r128_drv.c
+++ b/drivers/gpu/drm/r128/r128_drv.c
@@ -85,6 +85,12 @@ static struct drm_driver driver = {
int r128_driver_load(struct drm_device *dev, unsigned long flags)
{
+ int ret;
+
+ ret = drm_pci_agp_init(dev);
+ if (ret)
+ return ret;
+
pci_set_master(dev->pdev);
return drm_vblank_init(dev, 1);
}
diff --git a/drivers/gpu/drm/radeon/radeon_cp.c b/drivers/gpu/drm/radeon/radeon_cp.c
index ef67e18..1096296 100644
--- a/drivers/gpu/drm/radeon/radeon_cp.c
+++ b/drivers/gpu/drm/radeon/radeon_cp.c
@@ -2084,6 +2084,10 @@ int radeon_driver_load(struct drm_device *dev, unsigned long flags)
drm_radeon_private_t *dev_priv;
int ret = 0;
+ ret = drm_pci_agp_init(dev);
+ if (ret)
+ return ret;
+
dev_priv = kzalloc(sizeof(drm_radeon_private_t), GFP_KERNEL);
if (dev_priv == NULL)
return -ENOMEM;
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index f1016a5..61ab2fd 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -51,6 +51,10 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
struct radeon_device *rdev;
int r, acpi_status;
+ r = drm_pci_agp_init(dev);
+ if (r)
+ return r;
+
rdev = kzalloc(sizeof(struct radeon_device), GFP_KERNEL);
if (rdev == NULL) {
return -ENOMEM;
diff --git a/drivers/gpu/drm/savage/savage_bci.c b/drivers/gpu/drm/savage/savage_bci.c
index 6eb507a..6d70c4b 100644
--- a/drivers/gpu/drm/savage/savage_bci.c
+++ b/drivers/gpu/drm/savage/savage_bci.c
@@ -538,6 +538,11 @@ static void savage_fake_dma_flush(drm_savage_private_t * dev_priv)
int savage_driver_load(struct drm_device *dev, unsigned long chipset)
{
drm_savage_private_t *dev_priv;
+ int ret;
+
+ ret = drm_pci_agp_init(dev);
+ if (ret)
+ return ret;
dev_priv = kzalloc(sizeof(drm_savage_private_t), GFP_KERNEL);
if (dev_priv == NULL)
diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c
index 30d98d1..9726ee0 100644
--- a/drivers/gpu/drm/sis/sis_drv.c
+++ b/drivers/gpu/drm/sis/sis_drv.c
@@ -40,6 +40,11 @@ static struct pci_device_id pciidlist[] = {
static int sis_driver_load(struct drm_device *dev, unsigned long chipset)
{
drm_sis_private_t *dev_priv;
+ int ret;
+
+ ret = drm_pci_agp_init(dev);
+ if (ret)
+ return ret;
pci_set_master(dev->pdev);
diff --git a/drivers/gpu/drm/via/via_map.c b/drivers/gpu/drm/via/via_map.c
index 1f18225..5ab53bf 100644
--- a/drivers/gpu/drm/via/via_map.c
+++ b/drivers/gpu/drm/via/via_map.c
@@ -96,6 +96,10 @@ int via_driver_load(struct drm_device *dev, unsigned long chipset)
drm_via_private_t *dev_priv;
int ret = 0;
+ ret = drm_pci_agp_init(dev);
+ if (ret)
+ return ret;
+
dev_priv = kzalloc(sizeof(drm_via_private_t), GFP_KERNEL);
if (dev_priv == NULL)
return -ENOMEM;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 31ad880..e3437da 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -726,9 +726,6 @@ struct drm_bus {
int (*set_unique)(struct drm_device *dev, struct drm_master *master,
struct drm_unique *unique);
int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
- /* hooks that are for PCI */
- int (*agp_init)(struct drm_device *dev);
-
};
/**
@@ -1754,6 +1751,7 @@ static __inline__ int drm_pci_device_is_agp(struct drm_device *dev)
return pci_find_capability(dev->pdev, PCI_CAP_ID_AGP);
}
+extern int drm_pci_agp_init(struct drm_device *dev);
extern int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver);
extern void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver);
@@ -1761,7 +1759,6 @@ extern int drm_get_pci_dev(struct pci_dev *pdev,
const struct pci_device_id *ent,
struct drm_driver *driver);
-
/* platform section */
extern int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device);
extern void drm_platform_exit(struct drm_driver *driver, struct platform_device *platform_device);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 01/12] drm: move drm_pci_agp_init into driver load functions
2012-06-07 13:55 ` [PATCH 01/12] drm: move drm_pci_agp_init into driver load functions Daniel Vetter
@ 2012-06-08 9:16 ` Jani Nikula
2012-06-08 15:12 ` Daniel Vetter
0 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2012-06-08 9:16 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
Hi Daniel -
On Thu, 07 Jun 2012, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> I've done an audit of everything which is being set up between the
> place where drm_pci_agp_init is called currently and where the
> driver's ->load function is called. Nothing seems to depend upon
> dev->agp being set up correctly.
>
> This is one big step into squashing this giant midlayer mistaken.
> Though one that drm/i915 is hurting especially: We don't need (and
> want) the agp layer on many chips, but we need to decide whether we
> want to enable it too early.
>
> By moving the agp setup into the drivers control, we can do an
> intelligent decision in drm/i915 whether we need agp support (for
> anything ums and some horrible kms userspace on specific generations)
> or whether it's not required.
>
> Also, this allows us to rip out a bit of the agp invasion from generic
> drm core code in follow-up patches.
>
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/drm_pci.c | 2 +-
> drivers/gpu/drm/drm_stub.c | 8 --------
> drivers/gpu/drm/i810/i810_dma.c | 6 ++++++
> drivers/gpu/drm/i915/i915_dma.c | 4 ++++
> drivers/gpu/drm/mga/mga_dma.c | 4 ++++
> drivers/gpu/drm/nouveau/nouveau_state.c | 4 ++++
> drivers/gpu/drm/r128/r128_drv.c | 6 ++++++
> drivers/gpu/drm/radeon/radeon_cp.c | 4 ++++
> drivers/gpu/drm/radeon/radeon_kms.c | 4 ++++
> drivers/gpu/drm/savage/savage_bci.c | 5 +++++
> drivers/gpu/drm/sis/sis_drv.c | 5 +++++
> drivers/gpu/drm/via/via_map.c | 4 ++++
> include/drm/drmP.h | 5 +----
> 13 files changed, 48 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> index 13f3d93..833e599 100644
> --- a/drivers/gpu/drm/drm_pci.c
> +++ b/drivers/gpu/drm/drm_pci.c
> @@ -286,6 +286,7 @@ int drm_pci_agp_init(struct drm_device *dev)
> }
> return 0;
> }
> +EXPORT_SYMBOL(drm_pci_agp_init);
>
> static struct drm_bus drm_pci_bus = {
> .bus_type = DRIVER_BUS_PCI,
> @@ -294,7 +295,6 @@ static struct drm_bus drm_pci_bus = {
> .set_busid = drm_pci_set_busid,
> .set_unique = drm_pci_set_unique,
> .irq_by_busid = drm_pci_irq_by_busid,
> - .agp_init = drm_pci_agp_init,
> };
>
> /**
> diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> index 21bcd4a..4a4a1ed 100644
> --- a/drivers/gpu/drm/drm_stub.c
> +++ b/drivers/gpu/drm/drm_stub.c
> @@ -289,14 +289,6 @@ int drm_fill_in_dev(struct drm_device *dev,
>
> dev->driver = driver;
>
> - if (dev->driver->bus->agp_init) {
> - retcode = dev->driver->bus->agp_init(dev);
> - if (retcode)
> - goto error_out_unreg;
> - }
If something fails in drm_fill_in_dev() after ->agp_init(), the error
path calls drm_lastclose(), which, IIUC, deallocates anything allocated
through drm_pci_agp_init(). It's all buried in a bunch of layers, so I
may be mistaken... But where's this cleanup done if any of the init
fails after drm_pci_agp_init() in the driver load functions below?
BR,
Jani.
> -
> -
> -
> retcode = drm_ctxbitmap_init(dev);
> if (retcode) {
> DRM_ERROR("Cannot allocate memory for context bitmap.\n");
> diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c
> index f920fb5..eed1126 100644
> --- a/drivers/gpu/drm/i810/i810_dma.c
> +++ b/drivers/gpu/drm/i810/i810_dma.c
> @@ -1198,6 +1198,12 @@ static int i810_flip_bufs(struct drm_device *dev, void *data,
>
> int i810_driver_load(struct drm_device *dev, unsigned long flags)
> {
> + int ret;
> +
> + ret = drm_pci_agp_init(dev);
> + if (ret)
> + return ret;
> +
> /* i810 has 4 more counters */
> dev->counters += 4;
> dev->types[6] = _DRM_STAT_IRQ;
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 97a5a58..b97ef73 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1422,6 +1422,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> int ret = 0, mmio_bar;
> uint32_t aperture_size;
>
> + ret = drm_pci_agp_init(dev);
> + if (ret)
> + return ret;
> +
> info = (struct intel_device_info *) flags;
>
> /* Refuse to load on gen6+ without kms enabled. */
> diff --git a/drivers/gpu/drm/mga/mga_dma.c b/drivers/gpu/drm/mga/mga_dma.c
> index 507aa3d..549816e 100644
> --- a/drivers/gpu/drm/mga/mga_dma.c
> +++ b/drivers/gpu/drm/mga/mga_dma.c
> @@ -394,6 +394,10 @@ int mga_driver_load(struct drm_device *dev, unsigned long flags)
> drm_mga_private_t *dev_priv;
> int ret;
>
> + ret = drm_pci_agp_init(dev);
> + if (ret)
> + return ret;
> +
> dev_priv = kzalloc(sizeof(drm_mga_private_t), GFP_KERNEL);
> if (!dev_priv)
> return -ENOMEM;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
> index 19706f0..dbc881a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_state.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_state.c
> @@ -1029,6 +1029,10 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
> uint32_t reg0 = ~0, strap;
> int ret;
>
> + ret = drm_pci_agp_init(dev);
> + if (ret)
> + return ret;
> +
> dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
> if (!dev_priv) {
> ret = -ENOMEM;
> diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c
> index 88718fa..22001ca 100644
> --- a/drivers/gpu/drm/r128/r128_drv.c
> +++ b/drivers/gpu/drm/r128/r128_drv.c
> @@ -85,6 +85,12 @@ static struct drm_driver driver = {
>
> int r128_driver_load(struct drm_device *dev, unsigned long flags)
> {
> + int ret;
> +
> + ret = drm_pci_agp_init(dev);
> + if (ret)
> + return ret;
> +
> pci_set_master(dev->pdev);
> return drm_vblank_init(dev, 1);
> }
> diff --git a/drivers/gpu/drm/radeon/radeon_cp.c b/drivers/gpu/drm/radeon/radeon_cp.c
> index ef67e18..1096296 100644
> --- a/drivers/gpu/drm/radeon/radeon_cp.c
> +++ b/drivers/gpu/drm/radeon/radeon_cp.c
> @@ -2084,6 +2084,10 @@ int radeon_driver_load(struct drm_device *dev, unsigned long flags)
> drm_radeon_private_t *dev_priv;
> int ret = 0;
>
> + ret = drm_pci_agp_init(dev);
> + if (ret)
> + return ret;
> +
> dev_priv = kzalloc(sizeof(drm_radeon_private_t), GFP_KERNEL);
> if (dev_priv == NULL)
> return -ENOMEM;
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> index f1016a5..61ab2fd 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -51,6 +51,10 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
> struct radeon_device *rdev;
> int r, acpi_status;
>
> + r = drm_pci_agp_init(dev);
> + if (r)
> + return r;
> +
> rdev = kzalloc(sizeof(struct radeon_device), GFP_KERNEL);
> if (rdev == NULL) {
> return -ENOMEM;
> diff --git a/drivers/gpu/drm/savage/savage_bci.c b/drivers/gpu/drm/savage/savage_bci.c
> index 6eb507a..6d70c4b 100644
> --- a/drivers/gpu/drm/savage/savage_bci.c
> +++ b/drivers/gpu/drm/savage/savage_bci.c
> @@ -538,6 +538,11 @@ static void savage_fake_dma_flush(drm_savage_private_t * dev_priv)
> int savage_driver_load(struct drm_device *dev, unsigned long chipset)
> {
> drm_savage_private_t *dev_priv;
> + int ret;
> +
> + ret = drm_pci_agp_init(dev);
> + if (ret)
> + return ret;
>
> dev_priv = kzalloc(sizeof(drm_savage_private_t), GFP_KERNEL);
> if (dev_priv == NULL)
> diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c
> index 30d98d1..9726ee0 100644
> --- a/drivers/gpu/drm/sis/sis_drv.c
> +++ b/drivers/gpu/drm/sis/sis_drv.c
> @@ -40,6 +40,11 @@ static struct pci_device_id pciidlist[] = {
> static int sis_driver_load(struct drm_device *dev, unsigned long chipset)
> {
> drm_sis_private_t *dev_priv;
> + int ret;
> +
> + ret = drm_pci_agp_init(dev);
> + if (ret)
> + return ret;
>
> pci_set_master(dev->pdev);
>
> diff --git a/drivers/gpu/drm/via/via_map.c b/drivers/gpu/drm/via/via_map.c
> index 1f18225..5ab53bf 100644
> --- a/drivers/gpu/drm/via/via_map.c
> +++ b/drivers/gpu/drm/via/via_map.c
> @@ -96,6 +96,10 @@ int via_driver_load(struct drm_device *dev, unsigned long chipset)
> drm_via_private_t *dev_priv;
> int ret = 0;
>
> + ret = drm_pci_agp_init(dev);
> + if (ret)
> + return ret;
> +
> dev_priv = kzalloc(sizeof(drm_via_private_t), GFP_KERNEL);
> if (dev_priv == NULL)
> return -ENOMEM;
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 31ad880..e3437da 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -726,9 +726,6 @@ struct drm_bus {
> int (*set_unique)(struct drm_device *dev, struct drm_master *master,
> struct drm_unique *unique);
> int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
> - /* hooks that are for PCI */
> - int (*agp_init)(struct drm_device *dev);
> -
> };
>
> /**
> @@ -1754,6 +1751,7 @@ static __inline__ int drm_pci_device_is_agp(struct drm_device *dev)
>
> return pci_find_capability(dev->pdev, PCI_CAP_ID_AGP);
> }
> +extern int drm_pci_agp_init(struct drm_device *dev);
>
> extern int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver);
> extern void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver);
> @@ -1761,7 +1759,6 @@ extern int drm_get_pci_dev(struct pci_dev *pdev,
> const struct pci_device_id *ent,
> struct drm_driver *driver);
>
> -
> /* platform section */
> extern int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device);
> extern void drm_platform_exit(struct drm_driver *driver, struct platform_device *platform_device);
> --
> 1.7.7.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 01/12] drm: move drm_pci_agp_init into driver load functions
2012-06-08 9:16 ` Jani Nikula
@ 2012-06-08 15:12 ` Daniel Vetter
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-08 15:12 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
On Fri, Jun 08, 2012 at 12:16:41PM +0300, Jani Nikula wrote:
>
> Hi Daniel -
>
> On Thu, 07 Jun 2012, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > I've done an audit of everything which is being set up between the
> > place where drm_pci_agp_init is called currently and where the
> > driver's ->load function is called. Nothing seems to depend upon
> > dev->agp being set up correctly.
> >
> > This is one big step into squashing this giant midlayer mistaken.
> > Though one that drm/i915 is hurting especially: We don't need (and
> > want) the agp layer on many chips, but we need to decide whether we
> > want to enable it too early.
> >
> > By moving the agp setup into the drivers control, we can do an
> > intelligent decision in drm/i915 whether we need agp support (for
> > anything ums and some horrible kms userspace on specific generations)
> > or whether it's not required.
> >
> > Also, this allows us to rip out a bit of the agp invasion from generic
> > drm core code in follow-up patches.
> >
> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> > drivers/gpu/drm/drm_pci.c | 2 +-
> > drivers/gpu/drm/drm_stub.c | 8 --------
> > drivers/gpu/drm/i810/i810_dma.c | 6 ++++++
> > drivers/gpu/drm/i915/i915_dma.c | 4 ++++
> > drivers/gpu/drm/mga/mga_dma.c | 4 ++++
> > drivers/gpu/drm/nouveau/nouveau_state.c | 4 ++++
> > drivers/gpu/drm/r128/r128_drv.c | 6 ++++++
> > drivers/gpu/drm/radeon/radeon_cp.c | 4 ++++
> > drivers/gpu/drm/radeon/radeon_kms.c | 4 ++++
> > drivers/gpu/drm/savage/savage_bci.c | 5 +++++
> > drivers/gpu/drm/sis/sis_drv.c | 5 +++++
> > drivers/gpu/drm/via/via_map.c | 4 ++++
> > include/drm/drmP.h | 5 +----
> > 13 files changed, 48 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
> > index 13f3d93..833e599 100644
> > --- a/drivers/gpu/drm/drm_pci.c
> > +++ b/drivers/gpu/drm/drm_pci.c
> > @@ -286,6 +286,7 @@ int drm_pci_agp_init(struct drm_device *dev)
> > }
> > return 0;
> > }
> > +EXPORT_SYMBOL(drm_pci_agp_init);
> >
> > static struct drm_bus drm_pci_bus = {
> > .bus_type = DRIVER_BUS_PCI,
> > @@ -294,7 +295,6 @@ static struct drm_bus drm_pci_bus = {
> > .set_busid = drm_pci_set_busid,
> > .set_unique = drm_pci_set_unique,
> > .irq_by_busid = drm_pci_irq_by_busid,
> > - .agp_init = drm_pci_agp_init,
> > };
> >
> > /**
> > diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c
> > index 21bcd4a..4a4a1ed 100644
> > --- a/drivers/gpu/drm/drm_stub.c
> > +++ b/drivers/gpu/drm/drm_stub.c
> > @@ -289,14 +289,6 @@ int drm_fill_in_dev(struct drm_device *dev,
> >
> > dev->driver = driver;
> >
> > - if (dev->driver->bus->agp_init) {
> > - retcode = dev->driver->bus->agp_init(dev);
> > - if (retcode)
> > - goto error_out_unreg;
> > - }
>
> If something fails in drm_fill_in_dev() after ->agp_init(), the error
> path calls drm_lastclose(), which, IIUC, deallocates anything allocated
> through drm_pci_agp_init(). It's all buried in a bunch of layers, so I
> may be mistaken... But where's this cleanup done if any of the init
> fails after drm_pci_agp_init() in the driver load functions below?
Tbh I haven't check the failure paths to closely, presuming that given agp
being optional and me just moving it a bit down I surely can't break
stuff. And I've checked, I don't break things.
The real problem is that the error handling as-is is already pretty
broken. I'll look into this.
-Daniel
>
> BR,
> Jani.
>
> > -
> > -
> > -
> > retcode = drm_ctxbitmap_init(dev);
> > if (retcode) {
> > DRM_ERROR("Cannot allocate memory for context bitmap.\n");
> > diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c
> > index f920fb5..eed1126 100644
> > --- a/drivers/gpu/drm/i810/i810_dma.c
> > +++ b/drivers/gpu/drm/i810/i810_dma.c
> > @@ -1198,6 +1198,12 @@ static int i810_flip_bufs(struct drm_device *dev, void *data,
> >
> > int i810_driver_load(struct drm_device *dev, unsigned long flags)
> > {
> > + int ret;
> > +
> > + ret = drm_pci_agp_init(dev);
> > + if (ret)
> > + return ret;
> > +
> > /* i810 has 4 more counters */
> > dev->counters += 4;
> > dev->types[6] = _DRM_STAT_IRQ;
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 97a5a58..b97ef73 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1422,6 +1422,10 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> > int ret = 0, mmio_bar;
> > uint32_t aperture_size;
> >
> > + ret = drm_pci_agp_init(dev);
> > + if (ret)
> > + return ret;
> > +
> > info = (struct intel_device_info *) flags;
> >
> > /* Refuse to load on gen6+ without kms enabled. */
> > diff --git a/drivers/gpu/drm/mga/mga_dma.c b/drivers/gpu/drm/mga/mga_dma.c
> > index 507aa3d..549816e 100644
> > --- a/drivers/gpu/drm/mga/mga_dma.c
> > +++ b/drivers/gpu/drm/mga/mga_dma.c
> > @@ -394,6 +394,10 @@ int mga_driver_load(struct drm_device *dev, unsigned long flags)
> > drm_mga_private_t *dev_priv;
> > int ret;
> >
> > + ret = drm_pci_agp_init(dev);
> > + if (ret)
> > + return ret;
> > +
> > dev_priv = kzalloc(sizeof(drm_mga_private_t), GFP_KERNEL);
> > if (!dev_priv)
> > return -ENOMEM;
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
> > index 19706f0..dbc881a 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_state.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_state.c
> > @@ -1029,6 +1029,10 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
> > uint32_t reg0 = ~0, strap;
> > int ret;
> >
> > + ret = drm_pci_agp_init(dev);
> > + if (ret)
> > + return ret;
> > +
> > dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
> > if (!dev_priv) {
> > ret = -ENOMEM;
> > diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c
> > index 88718fa..22001ca 100644
> > --- a/drivers/gpu/drm/r128/r128_drv.c
> > +++ b/drivers/gpu/drm/r128/r128_drv.c
> > @@ -85,6 +85,12 @@ static struct drm_driver driver = {
> >
> > int r128_driver_load(struct drm_device *dev, unsigned long flags)
> > {
> > + int ret;
> > +
> > + ret = drm_pci_agp_init(dev);
> > + if (ret)
> > + return ret;
> > +
> > pci_set_master(dev->pdev);
> > return drm_vblank_init(dev, 1);
> > }
> > diff --git a/drivers/gpu/drm/radeon/radeon_cp.c b/drivers/gpu/drm/radeon/radeon_cp.c
> > index ef67e18..1096296 100644
> > --- a/drivers/gpu/drm/radeon/radeon_cp.c
> > +++ b/drivers/gpu/drm/radeon/radeon_cp.c
> > @@ -2084,6 +2084,10 @@ int radeon_driver_load(struct drm_device *dev, unsigned long flags)
> > drm_radeon_private_t *dev_priv;
> > int ret = 0;
> >
> > + ret = drm_pci_agp_init(dev);
> > + if (ret)
> > + return ret;
> > +
> > dev_priv = kzalloc(sizeof(drm_radeon_private_t), GFP_KERNEL);
> > if (dev_priv == NULL)
> > return -ENOMEM;
> > diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
> > index f1016a5..61ab2fd 100644
> > --- a/drivers/gpu/drm/radeon/radeon_kms.c
> > +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> > @@ -51,6 +51,10 @@ int radeon_driver_load_kms(struct drm_device *dev, unsigned long flags)
> > struct radeon_device *rdev;
> > int r, acpi_status;
> >
> > + r = drm_pci_agp_init(dev);
> > + if (r)
> > + return r;
> > +
> > rdev = kzalloc(sizeof(struct radeon_device), GFP_KERNEL);
> > if (rdev == NULL) {
> > return -ENOMEM;
> > diff --git a/drivers/gpu/drm/savage/savage_bci.c b/drivers/gpu/drm/savage/savage_bci.c
> > index 6eb507a..6d70c4b 100644
> > --- a/drivers/gpu/drm/savage/savage_bci.c
> > +++ b/drivers/gpu/drm/savage/savage_bci.c
> > @@ -538,6 +538,11 @@ static void savage_fake_dma_flush(drm_savage_private_t * dev_priv)
> > int savage_driver_load(struct drm_device *dev, unsigned long chipset)
> > {
> > drm_savage_private_t *dev_priv;
> > + int ret;
> > +
> > + ret = drm_pci_agp_init(dev);
> > + if (ret)
> > + return ret;
> >
> > dev_priv = kzalloc(sizeof(drm_savage_private_t), GFP_KERNEL);
> > if (dev_priv == NULL)
> > diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c
> > index 30d98d1..9726ee0 100644
> > --- a/drivers/gpu/drm/sis/sis_drv.c
> > +++ b/drivers/gpu/drm/sis/sis_drv.c
> > @@ -40,6 +40,11 @@ static struct pci_device_id pciidlist[] = {
> > static int sis_driver_load(struct drm_device *dev, unsigned long chipset)
> > {
> > drm_sis_private_t *dev_priv;
> > + int ret;
> > +
> > + ret = drm_pci_agp_init(dev);
> > + if (ret)
> > + return ret;
> >
> > pci_set_master(dev->pdev);
> >
> > diff --git a/drivers/gpu/drm/via/via_map.c b/drivers/gpu/drm/via/via_map.c
> > index 1f18225..5ab53bf 100644
> > --- a/drivers/gpu/drm/via/via_map.c
> > +++ b/drivers/gpu/drm/via/via_map.c
> > @@ -96,6 +96,10 @@ int via_driver_load(struct drm_device *dev, unsigned long chipset)
> > drm_via_private_t *dev_priv;
> > int ret = 0;
> >
> > + ret = drm_pci_agp_init(dev);
> > + if (ret)
> > + return ret;
> > +
> > dev_priv = kzalloc(sizeof(drm_via_private_t), GFP_KERNEL);
> > if (dev_priv == NULL)
> > return -ENOMEM;
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 31ad880..e3437da 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -726,9 +726,6 @@ struct drm_bus {
> > int (*set_unique)(struct drm_device *dev, struct drm_master *master,
> > struct drm_unique *unique);
> > int (*irq_by_busid)(struct drm_device *dev, struct drm_irq_busid *p);
> > - /* hooks that are for PCI */
> > - int (*agp_init)(struct drm_device *dev);
> > -
> > };
> >
> > /**
> > @@ -1754,6 +1751,7 @@ static __inline__ int drm_pci_device_is_agp(struct drm_device *dev)
> >
> > return pci_find_capability(dev->pdev, PCI_CAP_ID_AGP);
> > }
> > +extern int drm_pci_agp_init(struct drm_device *dev);
> >
> > extern int drm_pci_init(struct drm_driver *driver, struct pci_driver *pdriver);
> > extern void drm_pci_exit(struct drm_driver *driver, struct pci_driver *pdriver);
> > @@ -1761,7 +1759,6 @@ extern int drm_get_pci_dev(struct pci_dev *pdev,
> > const struct pci_device_id *ent,
> > struct drm_driver *driver);
> >
> > -
> > /* platform section */
> > extern int drm_platform_init(struct drm_driver *driver, struct platform_device *platform_device);
> > extern void drm_platform_exit(struct drm_driver *driver, struct platform_device *platform_device);
> > --
> > 1.7.7.6
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 02/12] drm: kill the REQUIRE_AGP driver flag
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
2012-06-07 13:55 ` [PATCH 01/12] drm: move drm_pci_agp_init into driver load functions Daniel Vetter
@ 2012-06-07 13:55 ` Daniel Vetter
2012-06-07 13:55 ` [PATCH 03/12] drm: kill USE_AGP " Daniel Vetter
` (10 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:55 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
Only i810 and i915 use this. But now that the driver's ->load function
is responsible for setting up agp, we can simply move this check in
there.
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/drm_pci.c | 6 +-----
drivers/gpu/drm/i810/i810_dma.c | 5 +++++
drivers/gpu/drm/i810/i810_drv.c | 2 +-
drivers/gpu/drm/i915/i915_dma.c | 5 +++++
drivers/gpu/drm/i915/i915_drv.c | 2 +-
include/drm/drmP.h | 1 -
6 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c
index 833e599..59e11e4 100644
--- a/drivers/gpu/drm/drm_pci.c
+++ b/drivers/gpu/drm/drm_pci.c
@@ -271,11 +271,7 @@ int drm_pci_agp_init(struct drm_device *dev)
if (drm_core_has_AGP(dev)) {
if (drm_pci_device_is_agp(dev))
dev->agp = drm_agp_init(dev);
- if (drm_core_check_feature(dev, DRIVER_REQUIRE_AGP)
- && (dev->agp == NULL)) {
- DRM_ERROR("Cannot initialize the agpgart module.\n");
- return -EINVAL;
- }
+
if (drm_core_has_MTRR(dev)) {
if (dev->agp)
dev->agp->agp_mtrr =
diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c
index eed1126..751d767 100644
--- a/drivers/gpu/drm/i810/i810_dma.c
+++ b/drivers/gpu/drm/i810/i810_dma.c
@@ -1204,6 +1204,11 @@ int i810_driver_load(struct drm_device *dev, unsigned long flags)
if (ret)
return ret;
+ if (!dev->agp) {
+ DRM_ERROR("Cannot initialize the agpgart module.\n");
+ return -EINVAL;
+ }
+
/* i810 has 4 more counters */
dev->counters += 4;
dev->types[6] = _DRM_STAT_IRQ;
diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c
index ec12f7d..6419182 100644
--- a/drivers/gpu/drm/i810/i810_drv.c
+++ b/drivers/gpu/drm/i810/i810_drv.c
@@ -56,7 +56,7 @@ static const struct file_operations i810_driver_fops = {
static struct drm_driver driver = {
.driver_features =
- DRIVER_USE_AGP | DRIVER_REQUIRE_AGP | DRIVER_USE_MTRR |
+ DRIVER_USE_AGP | DRIVER_USE_MTRR |
DRIVER_HAVE_DMA | DRIVER_DMA_QUEUE,
.dev_priv_size = sizeof(drm_i810_buf_priv_t),
.load = i810_driver_load,
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index b97ef73..494b9cb 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1426,6 +1426,11 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
if (ret)
return ret;
+ if (!dev->agp) {
+ DRM_ERROR("Cannot initialize the agpgart module.\n");
+ return -EINVAL;
+ }
+
info = (struct intel_device_info *) flags;
/* Refuse to load on gen6+ without kms enabled. */
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 238a521..e98501d 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1038,7 +1038,7 @@ static struct drm_driver driver = {
* deal with them for Intel hardware.
*/
.driver_features =
- DRIVER_USE_AGP | DRIVER_REQUIRE_AGP | /* DRIVER_USE_MTRR |*/
+ DRIVER_USE_AGP | /* DRIVER_USE_MTRR |*/
DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME,
.load = i915_driver_load,
.unload = i915_driver_unload,
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index e3437da..9906487 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -138,7 +138,6 @@ int drm_err(const char *func, const char *format, ...);
/* driver capabilities and requirements mask */
#define DRIVER_USE_AGP 0x1
-#define DRIVER_REQUIRE_AGP 0x2
#define DRIVER_USE_MTRR 0x4
#define DRIVER_PCI_DMA 0x8
#define DRIVER_SG 0x10
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 03/12] drm: kill USE_AGP driver flag
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
2012-06-07 13:55 ` [PATCH 01/12] drm: move drm_pci_agp_init into driver load functions Daniel Vetter
2012-06-07 13:55 ` [PATCH 02/12] drm: kill the REQUIRE_AGP driver flag Daniel Vetter
@ 2012-06-07 13:55 ` Daniel Vetter
2012-06-07 13:55 ` [PATCH 04/12] agp/intel-gtt: remove dead code Daniel Vetter
` (9 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:55 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
All drivers that use agp call the agp_init function now directly from
their ->load callback. All other parts check for dev->agp anyway to
check whether agp is available.
This flag has therefore outlived its usefullness, kill it.
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i810/i810_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/mga/mga_drv.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_drv.c | 2 +-
drivers/gpu/drm/r128/r128_drv.c | 2 +-
drivers/gpu/drm/radeon/radeon_drv.c | 4 ++--
drivers/gpu/drm/savage/savage_drv.c | 2 +-
drivers/gpu/drm/sis/sis_drv.c | 2 +-
drivers/gpu/drm/via/via_drv.c | 2 +-
include/drm/drmP.h | 6 +-----
10 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c
index 6419182..148cb52 100644
--- a/drivers/gpu/drm/i810/i810_drv.c
+++ b/drivers/gpu/drm/i810/i810_drv.c
@@ -56,7 +56,7 @@ static const struct file_operations i810_driver_fops = {
static struct drm_driver driver = {
.driver_features =
- DRIVER_USE_AGP | DRIVER_USE_MTRR |
+ DRIVER_USE_MTRR |
DRIVER_HAVE_DMA | DRIVER_DMA_QUEUE,
.dev_priv_size = sizeof(drm_i810_buf_priv_t),
.load = i810_driver_load,
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e98501d..818e3c7 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1038,7 +1038,7 @@ static struct drm_driver driver = {
* deal with them for Intel hardware.
*/
.driver_features =
- DRIVER_USE_AGP | /* DRIVER_USE_MTRR |*/
+ /* DRIVER_USE_MTRR |*/
DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME,
.load = i915_driver_load,
.unload = i915_driver_unload,
diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c
index f9a925d..866d07a 100644
--- a/drivers/gpu/drm/mga/mga_drv.c
+++ b/drivers/gpu/drm/mga/mga_drv.c
@@ -60,7 +60,7 @@ static const struct file_operations mga_driver_fops = {
static struct drm_driver driver = {
.driver_features =
- DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_PCI_DMA |
+ DRIVER_USE_MTRR | DRIVER_PCI_DMA |
DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED,
.dev_priv_size = sizeof(drm_mga_buf_priv_t),
.load = mga_driver_load,
diff --git a/drivers/gpu/drm/nouveau/nouveau_drv.c b/drivers/gpu/drm/nouveau/nouveau_drv.c
index cad254c..b1dc91d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drv.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drv.c
@@ -401,7 +401,7 @@ static const struct file_operations nouveau_driver_fops = {
static struct drm_driver driver = {
.driver_features =
- DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG |
+ DRIVER_PCI_DMA | DRIVER_SG |
DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
DRIVER_MODESET | DRIVER_PRIME,
.load = nouveau_load,
diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c
index 22001ca..4146db4 100644
--- a/drivers/gpu/drm/r128/r128_drv.c
+++ b/drivers/gpu/drm/r128/r128_drv.c
@@ -58,7 +58,7 @@ static const struct file_operations r128_driver_fops = {
static struct drm_driver driver = {
.driver_features =
- DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_PCI_DMA | DRIVER_SG |
+ DRIVER_USE_MTRR | DRIVER_PCI_DMA | DRIVER_SG |
DRIVER_HAVE_DMA | DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED,
.dev_priv_size = sizeof(drm_r128_buf_priv_t),
.load = r128_driver_load,
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index f0bb2b5..1cb0a02 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -239,7 +239,7 @@ static const struct file_operations radeon_driver_old_fops = {
static struct drm_driver driver_old = {
.driver_features =
- DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_PCI_DMA | DRIVER_SG |
+ DRIVER_USE_MTRR | DRIVER_PCI_DMA | DRIVER_SG |
DRIVER_HAVE_IRQ | DRIVER_HAVE_DMA | DRIVER_IRQ_SHARED,
.dev_priv_size = sizeof(drm_radeon_buf_priv_t),
.load = radeon_driver_load,
@@ -337,7 +337,7 @@ static const struct file_operations radeon_driver_kms_fops = {
static struct drm_driver kms_driver = {
.driver_features =
- DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_PCI_DMA | DRIVER_SG |
+ DRIVER_USE_MTRR | DRIVER_PCI_DMA | DRIVER_SG |
DRIVER_HAVE_IRQ | DRIVER_HAVE_DMA | DRIVER_IRQ_SHARED | DRIVER_GEM |
DRIVER_PRIME,
.dev_priv_size = 0,
diff --git a/drivers/gpu/drm/savage/savage_drv.c b/drivers/gpu/drm/savage/savage_drv.c
index 89afe0b..cef78aa 100644
--- a/drivers/gpu/drm/savage/savage_drv.c
+++ b/drivers/gpu/drm/savage/savage_drv.c
@@ -48,7 +48,7 @@ static const struct file_operations savage_driver_fops = {
static struct drm_driver driver = {
.driver_features =
- DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_HAVE_DMA | DRIVER_PCI_DMA,
+ DRIVER_USE_MTRR | DRIVER_HAVE_DMA | DRIVER_PCI_DMA,
.dev_priv_size = sizeof(drm_savage_buf_priv_t),
.load = savage_driver_load,
.firstopen = savage_driver_firstopen,
diff --git a/drivers/gpu/drm/sis/sis_drv.c b/drivers/gpu/drm/sis/sis_drv.c
index 9726ee0..7b6f042 100644
--- a/drivers/gpu/drm/sis/sis_drv.c
+++ b/drivers/gpu/drm/sis/sis_drv.c
@@ -106,7 +106,7 @@ void sis_driver_postclose(struct drm_device *dev, struct drm_file *file)
}
static struct drm_driver driver = {
- .driver_features = DRIVER_USE_AGP | DRIVER_USE_MTRR,
+ .driver_features = DRIVER_USE_MTRR,
.load = sis_driver_load,
.unload = sis_driver_unload,
.open = sis_driver_open,
diff --git a/drivers/gpu/drm/via/via_drv.c b/drivers/gpu/drm/via/via_drv.c
index 02661f3..165390d 100644
--- a/drivers/gpu/drm/via/via_drv.c
+++ b/drivers/gpu/drm/via/via_drv.c
@@ -70,7 +70,7 @@ static const struct file_operations via_driver_fops = {
static struct drm_driver driver = {
.driver_features =
- DRIVER_USE_AGP | DRIVER_USE_MTRR | DRIVER_HAVE_IRQ |
+ DRIVER_USE_MTRR | DRIVER_HAVE_IRQ |
DRIVER_IRQ_SHARED,
.load = via_driver_load,
.unload = via_driver_unload,
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 9906487..8dd4a92 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -137,7 +137,6 @@ int drm_err(const char *func, const char *format, ...);
/*@{*/
/* driver capabilities and requirements mask */
-#define DRIVER_USE_AGP 0x1
#define DRIVER_USE_MTRR 0x4
#define DRIVER_PCI_DMA 0x8
#define DRIVER_SG 0x10
@@ -1223,10 +1222,7 @@ static inline int drm_dev_to_irq(struct drm_device *dev)
#if __OS_HAS_AGP
-static inline int drm_core_has_AGP(struct drm_device *dev)
-{
- return drm_core_check_feature(dev, DRIVER_USE_AGP);
-}
+#define drm_core_has_AGP(dev) (1)
#else
#define drm_core_has_AGP(dev) (0)
#endif
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 04/12] agp/intel-gtt: remove dead code
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
` (2 preceding siblings ...)
2012-06-07 13:55 ` [PATCH 03/12] drm: kill USE_AGP " Daniel Vetter
@ 2012-06-07 13:55 ` Daniel Vetter
2012-06-07 13:55 ` [PATCH 05/12] drm/i915: stop using dev->agp->base Daniel Vetter
` (8 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:55 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
This is a leftover from the conversion of the i81x fake agp driver
over to the new intel-gtt code layoute.
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/char/agp/intel-gtt.c | 3 ---
1 files changed, 0 insertions(+), 3 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 1237e75..53c4c7f 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1556,9 +1556,6 @@ int intel_gmch_probe(struct pci_dev *pdev,
pci_set_consistent_dma_mask(intel_private.pcidev,
DMA_BIT_MASK(mask));
- /*if (bridge->driver == &intel_810_driver)
- return 1;*/
-
if (intel_gtt_init() != 0)
return 0;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 05/12] drm/i915: stop using dev->agp->base
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
` (3 preceding siblings ...)
2012-06-07 13:55 ` [PATCH 04/12] agp/intel-gtt: remove dead code Daniel Vetter
@ 2012-06-07 13:55 ` Daniel Vetter
2012-06-07 13:55 ` [PATCH 06/12] agp/intel-gtt: don't require the agp bridge on setup Daniel Vetter
` (7 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:55 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
For that to work we need to export the base address of the gtt
mmio window from intel-gtt. Also replace all other uses of
dev->agp by values we already have at hand.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/char/agp/intel-gtt.c | 5 ++---
drivers/gpu/drm/i915/i915_dma.c | 21 +++++++++++++--------
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_gem_debug.c | 3 ++-
drivers/gpu/drm/i915/intel_display.c | 2 +-
drivers/gpu/drm/i915/intel_fb.c | 4 +++-
drivers/gpu/drm/i915/intel_ringbuffer.c | 6 ++++--
include/drm/intel-gtt.h | 2 ++
9 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 53c4c7f..2aab0a0 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -66,7 +66,6 @@ static struct _intel_private {
struct pci_dev *bridge_dev;
u8 __iomem *registers;
phys_addr_t gtt_bus_addr;
- phys_addr_t gma_bus_addr;
u32 PGETBL_save;
u32 __iomem *gtt; /* I915G */
bool clear_fake_agp; /* on first access via agp, fill with scratch */
@@ -779,7 +778,7 @@ static bool intel_enable_gtt(void)
pci_read_config_dword(intel_private.pcidev, I915_GMADDR,
&gma_addr);
- intel_private.gma_bus_addr = (gma_addr & PCI_BASE_ADDRESS_MEM_MASK);
+ intel_private.base.gma_bus_addr = (gma_addr & PCI_BASE_ADDRESS_MEM_MASK);
if (INTEL_GTT_GEN >= 6)
return true;
@@ -860,7 +859,7 @@ static int intel_fake_agp_configure(void)
return -EIO;
intel_private.clear_fake_agp = true;
- agp_bridge->gart_bus_addr = intel_private.gma_bus_addr;
+ agp_bridge->gart_bus_addr = intel_private.base.gma_bus_addr;
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 494b9cb..e4203df 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1085,8 +1085,8 @@ static int i915_set_status_page(struct drm_device *dev, void *data,
ring->status_page.gfx_addr = hws->addr & (0x1ffff<<12);
- dev_priv->dri1.gfx_hws_cpu_addr = ioremap_wc(dev->agp->base + hws->addr,
- 4096);
+ dev_priv->dri1.gfx_hws_cpu_addr =
+ ioremap_wc(dev_priv->mm.gtt_base_addr + hws->addr, 4096);
if (dev_priv->dri1.gfx_hws_cpu_addr == NULL) {
i915_dma_cleanup(dev);
ring->status_page.gfx_addr = 0;
@@ -1491,15 +1491,18 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
}
aperture_size = dev_priv->mm.gtt->gtt_mappable_entries << PAGE_SHIFT;
+ dev_priv->mm.gtt_base_addr = dev_priv->mm.gtt->gma_bus_addr;
dev_priv->mm.gtt_mapping =
- io_mapping_create_wc(dev->agp->base, aperture_size);
+ io_mapping_create_wc(dev_priv->mm.gtt_base_addr,
+ aperture_size);
if (dev_priv->mm.gtt_mapping == NULL) {
ret = -EIO;
goto out_rmmap;
}
- i915_mtrr_setup(dev_priv, dev->agp->base, aperture_size);
+ i915_mtrr_setup(dev_priv, dev_priv->mm.gtt_base_addr,
+ aperture_size);
/* The i915 workqueue is primarily used for batched retirement of
* requests (and thus managing bo) once the task has been completed
@@ -1611,8 +1614,9 @@ out_gem_unload:
destroy_workqueue(dev_priv->wq);
out_mtrrfree:
if (dev_priv->mm.gtt_mtrr >= 0) {
- mtrr_del(dev_priv->mm.gtt_mtrr, dev->agp->base,
- dev->agp->agp_info.aper_size * 1024 * 1024);
+ mtrr_del(dev_priv->mm.gtt_mtrr,
+ dev_priv->mm.gtt_base_addr,
+ aperture_size);
dev_priv->mm.gtt_mtrr = -1;
}
io_mapping_free(dev_priv->mm.gtt_mapping);
@@ -1649,8 +1653,9 @@ int i915_driver_unload(struct drm_device *dev)
io_mapping_free(dev_priv->mm.gtt_mapping);
if (dev_priv->mm.gtt_mtrr >= 0) {
- mtrr_del(dev_priv->mm.gtt_mtrr, dev->agp->base,
- dev->agp->agp_info.aper_size * 1024 * 1024);
+ mtrr_del(dev_priv->mm.gtt_mtrr,
+ dev_priv->mm.gtt_base_addr,
+ dev_priv->mm.gtt->gtt_mappable_entries * PAGE_SIZE);
dev_priv->mm.gtt_mtrr = -1;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ccabadd..ae4129b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -651,6 +651,7 @@ typedef struct drm_i915_private {
unsigned long gtt_end;
struct io_mapping *gtt_mapping;
+ phys_addr_t gtt_base_addr;
int gtt_mtrr;
/** PPGTT used for aliasing the PPGTT with the GTT */
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index deaa0d4..108e4c2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1122,7 +1122,7 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
obj->fault_mappable = true;
- pfn = ((dev->agp->base + obj->gtt_offset) >> PAGE_SHIFT) +
+ pfn = ((dev_priv->mm.gtt_base_addr + obj->gtt_offset) >> PAGE_SHIFT) +
page_offset;
/* Finally, remap it using the new GTT offset */
diff --git a/drivers/gpu/drm/i915/i915_gem_debug.c b/drivers/gpu/drm/i915/i915_gem_debug.c
index a4f6aaa..bddf7be 100644
--- a/drivers/gpu/drm/i915/i915_gem_debug.c
+++ b/drivers/gpu/drm/i915/i915_gem_debug.c
@@ -132,7 +132,8 @@ i915_gem_object_check_coherency(struct drm_i915_gem_object *obj, int handle)
__func__, obj, obj->gtt_offset, handle,
obj->size / 1024);
- gtt_mapping = ioremap(dev->agp->base + obj->gtt_offset, obj->base.size);
+ gtt_mapping = ioremap(dev_priv->mm.gtt_base_addr + obj->gtt_offset,
+ obj->base.size);
if (gtt_mapping == NULL) {
DRM_ERROR("failed to map GTT space\n");
return;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5e76031..190eacc 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6954,7 +6954,7 @@ void intel_modeset_init(struct drm_device *dev)
dev->mode_config.max_width = 8192;
dev->mode_config.max_height = 8192;
}
- dev->mode_config.fb_base = dev->agp->base;
+ dev->mode_config.fb_base = dev_priv->mm.gtt_base_addr;
DRM_DEBUG_KMS("%d display pipe%s available.\n",
dev_priv->num_pipe, dev_priv->num_pipe > 1 ? "s" : "");
diff --git a/drivers/gpu/drm/i915/intel_fb.c b/drivers/gpu/drm/i915/intel_fb.c
index bf86907..e9f8338 100644
--- a/drivers/gpu/drm/i915/intel_fb.c
+++ b/drivers/gpu/drm/i915/intel_fb.c
@@ -140,7 +140,9 @@ static int intelfb_create(struct intel_fbdev *ifbdev,
info->fix.smem_start = dev->mode_config.fb_base + obj->gtt_offset;
info->fix.smem_len = size;
- info->screen_base = ioremap_wc(dev->agp->base + obj->gtt_offset, size);
+ info->screen_base =
+ ioremap_wc(dev_priv->mm.gtt_base_addr + obj->gtt_offset,
+ size);
if (!info->screen_base) {
ret = -ENOSPC;
goto out_unpin;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 89a5e7f..14025ab 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -968,6 +968,7 @@ static int intel_init_ring_buffer(struct drm_device *dev,
struct intel_ring_buffer *ring)
{
struct drm_i915_gem_object *obj;
+ struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
ring->dev = dev;
@@ -997,8 +998,9 @@ static int intel_init_ring_buffer(struct drm_device *dev,
if (ret)
goto err_unref;
- ring->virtual_start = ioremap_wc(dev->agp->base + obj->gtt_offset,
- ring->size);
+ ring->virtual_start =
+ ioremap_wc(dev_priv->mm.gtt->gma_bus_addr + obj->gtt_offset,
+ ring->size);
if (ring->virtual_start == NULL) {
DRM_ERROR("Failed to map ringbuffer.\n");
ret = -EINVAL;
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index 923afb5..8048c00 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -19,6 +19,8 @@ const struct intel_gtt {
dma_addr_t scratch_page_dma;
/* for ppgtt PDE access */
u32 __iomem *gtt;
+ /* needed for ioremap in drm/i915 */
+ phys_addr_t gma_bus_addr;
} *intel_gtt_get(void);
void intel_gtt_chipset_flush(void);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 06/12] agp/intel-gtt: don't require the agp bridge on setup
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
` (4 preceding siblings ...)
2012-06-07 13:55 ` [PATCH 05/12] drm/i915: stop using dev->agp->base Daniel Vetter
@ 2012-06-07 13:55 ` Daniel Vetter
2012-06-07 13:55 ` [PATCH 07/12] drm/i915: only enable drm agp support when required Daniel Vetter
` (6 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:55 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
We only need it to fake the agp interface and don't actually
use it in the driver anywhere. Hence conditionalize that.
This is just a prep patch to eventually disable the fake agp
driver on gen6+.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/char/agp/intel-gtt.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 2aab0a0..5e6c89e 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -1539,9 +1539,11 @@ int intel_gmch_probe(struct pci_dev *pdev,
if (!intel_private.driver)
return 0;
- bridge->driver = &intel_fake_agp_driver;
- bridge->dev_private_data = &intel_private;
- bridge->dev = pdev;
+ if (bridge) {
+ bridge->driver = &intel_fake_agp_driver;
+ bridge->dev_private_data = &intel_private;
+ bridge->dev = pdev;
+ }
intel_private.bridge_dev = pci_dev_get(pdev);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 07/12] drm/i915: only enable drm agp support when required
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
` (5 preceding siblings ...)
2012-06-07 13:55 ` [PATCH 06/12] agp/intel-gtt: don't require the agp bridge on setup Daniel Vetter
@ 2012-06-07 13:55 ` Daniel Vetter
2012-06-12 11:58 ` [Intel-gfx] " Jani Nikula
2012-06-07 13:56 ` [PATCH 08/12] drm/i915 + agp/intel-gtt: prep work for direct setup Daniel Vetter
` (5 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:55 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
We need it for all things ums (which essentially only means up to
gen5) and to support b0rked XvMC userspace on gen3.
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_dma.c | 21 ++++++++++++---------
1 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e4203df..0ab5d3d 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1422,15 +1422,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
int ret = 0, mmio_bar;
uint32_t aperture_size;
- ret = drm_pci_agp_init(dev);
- if (ret)
- return ret;
-
- if (!dev->agp) {
- DRM_ERROR("Cannot initialize the agpgart module.\n");
- return -EINVAL;
- }
-
info = (struct intel_device_info *) flags;
/* Refuse to load on gen6+ without kms enabled. */
@@ -1453,6 +1444,18 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
dev_priv->dev = dev;
dev_priv->info = info;
+ if (!drm_core_check_feature(dev, DRIVER_MODESET) ||
+ IS_GEN3(dev)) {
+ ret = drm_pci_agp_init(dev);
+ if (ret)
+ return ret;
+
+ if (!dev->agp) {
+ DRM_ERROR("Cannot initialize the agpgart module.\n");
+ return -EINVAL;
+ }
+ }
+
if (i915_get_bridge_dev(dev)) {
ret = -EIO;
goto free_priv;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [Intel-gfx] [PATCH 07/12] drm/i915: only enable drm agp support when required
2012-06-07 13:55 ` [PATCH 07/12] drm/i915: only enable drm agp support when required Daniel Vetter
@ 2012-06-12 11:58 ` Jani Nikula
2012-06-12 11:19 ` [PATCH] " Daniel Vetter
2012-06-12 12:21 ` [PATCH 07/12] " Daniel Vetter
0 siblings, 2 replies; 23+ messages in thread
From: Jani Nikula @ 2012-06-12 11:58 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
On Thu, 07 Jun 2012, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We need it for all things ums (which essentially only means up to
> gen5) and to support b0rked XvMC userspace on gen3.
>
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 21 ++++++++++++---------
> 1 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index e4203df..0ab5d3d 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1422,15 +1422,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> int ret = 0, mmio_bar;
> uint32_t aperture_size;
>
> - ret = drm_pci_agp_init(dev);
> - if (ret)
> - return ret;
> -
> - if (!dev->agp) {
> - DRM_ERROR("Cannot initialize the agpgart module.\n");
> - return -EINVAL;
> - }
> -
> info = (struct intel_device_info *) flags;
>
> /* Refuse to load on gen6+ without kms enabled. */
> @@ -1453,6 +1444,18 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> dev_priv->dev = dev;
> dev_priv->info = info;
>
> + if (!drm_core_check_feature(dev, DRIVER_MODESET) ||
> + IS_GEN3(dev)) {
> + ret = drm_pci_agp_init(dev);
> + if (ret)
> + return ret;
> +
> + if (!dev->agp) {
> + DRM_ERROR("Cannot initialize the agpgart module.\n");
> + return -EINVAL;
> + }
You need to goto free_priv in the above error paths.
Should there be a deinit of drm_pci_agp_init() if something goes wrong
afterwards?
BR,
Jani.
> + }
> +
> if (i915_get_bridge_dev(dev)) {
> ret = -EIO;
> goto free_priv;
> --
> 1.7.7.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH] drm/i915: only enable drm agp support when required
2012-06-12 11:58 ` [Intel-gfx] " Jani Nikula
@ 2012-06-12 11:19 ` Daniel Vetter
2012-06-12 12:21 ` [PATCH 07/12] " Daniel Vetter
1 sibling, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-12 11:19 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, DRI Development
We need it for all things ums (which essentially only means up to
gen5) and to support b0rked XvMC userspace on gen3.
v2: Fixup error paths as noticed by Jani Nikula.
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_dma.c | 22 +++++++++++++---------
1 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e4203df..1f6a36a 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1422,15 +1422,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
int ret = 0, mmio_bar;
uint32_t aperture_size;
- ret = drm_pci_agp_init(dev);
- if (ret)
- return ret;
-
- if (!dev->agp) {
- DRM_ERROR("Cannot initialize the agpgart module.\n");
- return -EINVAL;
- }
-
info = (struct intel_device_info *) flags;
/* Refuse to load on gen6+ without kms enabled. */
@@ -1453,6 +1444,19 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
dev_priv->dev = dev;
dev_priv->info = info;
+ if (!drm_core_check_feature(dev, DRIVER_MODESET) ||
+ IS_GEN3(dev)) {
+ ret = drm_pci_agp_init(dev);
+ if (ret)
+ goto free_priv;
+
+ if (!dev->agp) {
+ DRM_ERROR("Cannot initialize the agpgart module.\n");
+ ret = -EINVAL;
+ goto free_priv;
+ }
+ }
+
if (i915_get_bridge_dev(dev)) {
ret = -EIO;
goto free_priv;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 07/12] drm/i915: only enable drm agp support when required
2012-06-12 11:58 ` [Intel-gfx] " Jani Nikula
2012-06-12 11:19 ` [PATCH] " Daniel Vetter
@ 2012-06-12 12:21 ` Daniel Vetter
1 sibling, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-12 12:21 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
On Tue, Jun 12, 2012 at 02:58:25PM +0300, Jani Nikula wrote:
> On Thu, 07 Jun 2012, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > We need it for all things ums (which essentially only means up to
> > gen5) and to support b0rked XvMC userspace on gen3.
> >
> > Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> > drivers/gpu/drm/i915/i915_dma.c | 21 ++++++++++++---------
> > 1 files changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index e4203df..0ab5d3d 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1422,15 +1422,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> > int ret = 0, mmio_bar;
> > uint32_t aperture_size;
> >
> > - ret = drm_pci_agp_init(dev);
> > - if (ret)
> > - return ret;
> > -
> > - if (!dev->agp) {
> > - DRM_ERROR("Cannot initialize the agpgart module.\n");
> > - return -EINVAL;
> > - }
> > -
> > info = (struct intel_device_info *) flags;
> >
> > /* Refuse to load on gen6+ without kms enabled. */
> > @@ -1453,6 +1444,18 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> > dev_priv->dev = dev;
> > dev_priv->info = info;
> >
> > + if (!drm_core_check_feature(dev, DRIVER_MODESET) ||
> > + IS_GEN3(dev)) {
> > + ret = drm_pci_agp_init(dev);
> > + if (ret)
> > + return ret;
> > +
> > + if (!dev->agp) {
> > + DRM_ERROR("Cannot initialize the agpgart module.\n");
> > + return -EINVAL;
> > + }
>
> You need to goto free_priv in the above error paths.
Nice catch, will fix.
> Should there be a deinit of drm_pci_agp_init() if something goes wrong
> afterwards?
drm core should do that for use (but currently doesn't) by deinit agp if
it's initialized. The follow-up patches to wrestle the drm driver init
paths /should/ take care of this though.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 08/12] drm/i915 + agp/intel-gtt: prep work for direct setup
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
` (6 preceding siblings ...)
2012-06-07 13:55 ` [PATCH 07/12] drm/i915: only enable drm agp support when required Daniel Vetter
@ 2012-06-07 13:56 ` Daniel Vetter
2012-06-08 10:09 ` Jani Nikula
2012-06-07 13:56 ` [PATCH 09/12] drm/i915: don't check intel_agp_enabled any more Daniel Vetter
` (4 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:56 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
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@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;
+
+ /*
+ * 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;
+ }
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 08/12] drm/i915 + agp/intel-gtt: prep work for direct setup
2012-06-07 13:56 ` [PATCH 08/12] drm/i915 + agp/intel-gtt: prep work for direct setup Daniel Vetter
@ 2012-06-08 10:09 ` Jani Nikula
2012-06-08 12:39 ` Daniel Vetter
0 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2012-06-08 10:09 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
On Thu, 07 Jun 2012, Daniel Vetter <daniel.vetter@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@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 08/12] drm/i915 + agp/intel-gtt: prep work for direct setup
2012-06-08 10:09 ` Jani Nikula
@ 2012-06-08 12:39 ` Daniel Vetter
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-08 12:39 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
On Fri, Jun 08, 2012 at 01:09:10PM +0300, Jani Nikula wrote:
> On Thu, 07 Jun 2012, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > #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?
Yeah, things went a bit wrong here. I'll also see whether I can make the
cleanup path a bit more robust. Thanks for taking a look at this patch.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 09/12] drm/i915: don't check intel_agp_enabled any more
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
` (7 preceding siblings ...)
2012-06-07 13:56 ` [PATCH 08/12] drm/i915 + agp/intel-gtt: prep work for direct setup Daniel Vetter
@ 2012-06-07 13:56 ` Daniel Vetter
2012-06-12 11:24 ` [PATCH] " Daniel Vetter
2012-06-07 13:56 ` [PATCH 10/12] agp/intel-gtt: move gart base addres setup Daniel Vetter
` (3 subsequent siblings)
12 siblings, 1 reply; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:56 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
We won't enabled agp unconditionally. Furthermore we do have
the right checks for agp support in place in our ->load
function.
The only thing this variable still does is enforce the module
load order we want (and I'm not even sure whether it succeeds
for that). Hence just use it in a harmless place somewhere.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_dma.c | 6 +++++-
drivers/gpu/drm/i915/i915_drv.c | 6 ------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 29aee99..e21b3ff 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1404,6 +1404,8 @@ i915_mtrr_setup(struct drm_i915_private *dev_priv, unsigned long base,
}
}
+extern int intel_agp_enabled;
+
/**
* i915_driver_load - setup chip and create an initial config
* @dev: DRM device
@@ -1451,7 +1453,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
return ret;
if (!dev->agp) {
- DRM_ERROR("Cannot initialize the agpgart module.\n");
+ DRM_ERROR("Cannot initialize the agpgart module,"
+ "intel_agp_enabled: %i.\n",
+ intel_agp_enabled);
return -EINVAL;
}
}
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 818e3c7..1bf7597 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -119,7 +119,6 @@ MODULE_PARM_DESC(i915_enable_ppgtt,
"Enable PPGTT (default: true)");
static struct drm_driver driver;
-extern int intel_agp_enabled;
#define INTEL_VGA_DEVICE(id, info) { \
.class = PCI_BASE_CLASS_DISPLAY << 16, \
@@ -1091,11 +1090,6 @@ static struct pci_driver i915_pci_driver = {
static int __init i915_init(void)
{
- if (!intel_agp_enabled) {
- DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
- return -ENODEV;
- }
-
driver.num_ioctls = i915_max_ioctl;
/*
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH] drm/i915: don't check intel_agp_enabled any more
2012-06-07 13:56 ` [PATCH 09/12] drm/i915: don't check intel_agp_enabled any more Daniel Vetter
@ 2012-06-12 11:24 ` Daniel Vetter
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-12 11:24 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, DRI Development
We won't enabled agp unconditionally. Furthermore we do have
the right checks for agp support in place in our ->load
function.
The only thing this variable still does is enforce the module
load order we want (and I'm not even sure whether it succeeds
for that). Hence just use it in a harmless place somewhere.
v2: Fixup rebase.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_dma.c | 6 +++++-
drivers/gpu/drm/i915/i915_drv.c | 6 ------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index a8f8477..6290aea 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1404,6 +1404,8 @@ i915_mtrr_setup(struct drm_i915_private *dev_priv, unsigned long base,
}
}
+extern int intel_agp_enabled;
+
/**
* i915_driver_load - setup chip and create an initial config
* @dev: DRM device
@@ -1451,7 +1453,9 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
goto free_priv;
if (!dev->agp) {
- DRM_ERROR("Cannot initialize the agpgart module.\n");
+ DRM_ERROR("Cannot initialize the agpgart module,"
+ "intel_agp_enabled: %i.\n",
+ intel_agp_enabled);
ret = -EINVAL;
goto free_priv;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 3390213..e4df5a0 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -119,7 +119,6 @@ MODULE_PARM_DESC(i915_enable_ppgtt,
"Enable PPGTT (default: true)");
static struct drm_driver driver;
-extern int intel_agp_enabled;
#define INTEL_VGA_DEVICE(id, info) { \
.class = PCI_BASE_CLASS_DISPLAY << 16, \
@@ -1093,11 +1092,6 @@ static struct pci_driver i915_pci_driver = {
static int __init i915_init(void)
{
- if (!intel_agp_enabled) {
- DRM_ERROR("drm/i915 can't work without intel_agp module!\n");
- return -ENODEV;
- }
-
driver.num_ioctls = i915_max_ioctl;
/*
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 10/12] agp/intel-gtt: move gart base addres setup
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
` (8 preceding siblings ...)
2012-06-07 13:56 ` [PATCH 09/12] drm/i915: don't check intel_agp_enabled any more Daniel Vetter
@ 2012-06-07 13:56 ` Daniel Vetter
2012-06-07 13:56 ` [PATCH 11/12] drm/i915: call intel_enable_gtt Daniel Vetter
` (2 subsequent siblings)
12 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:56 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
We need this thing much earlier, and it doesn't make sense
in the hw enabling function intel_enable_gtt - this does not
change over a suspend/resume cycle ...
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/char/agp/intel-gtt.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 6499290..440e8d4 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -648,6 +648,7 @@ static void intel_gtt_cleanup(void)
static int intel_gtt_init(void)
{
+ u32 gma_addr;
u32 gtt_map_size;
int ret;
@@ -694,6 +695,15 @@ static int intel_gtt_init(void)
return ret;
}
+ if (INTEL_GTT_GEN <= 2)
+ pci_read_config_dword(intel_private.pcidev, I810_GMADDR,
+ &gma_addr);
+ else
+ pci_read_config_dword(intel_private.pcidev, I915_GMADDR,
+ &gma_addr);
+
+ intel_private.base.gma_bus_addr = (gma_addr & PCI_BASE_ADDRESS_MEM_MASK);
+
return 0;
}
@@ -769,18 +779,8 @@ static void i830_write_entry(dma_addr_t addr, unsigned int entry,
static bool intel_enable_gtt(void)
{
- u32 gma_addr;
u8 __iomem *reg;
- if (INTEL_GTT_GEN <= 2)
- pci_read_config_dword(intel_private.pcidev, I810_GMADDR,
- &gma_addr);
- else
- pci_read_config_dword(intel_private.pcidev, I915_GMADDR,
- &gma_addr);
-
- intel_private.base.gma_bus_addr = (gma_addr & PCI_BASE_ADDRESS_MEM_MASK);
-
if (INTEL_GTT_GEN >= 6)
return true;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 11/12] drm/i915: call intel_enable_gtt
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
` (9 preceding siblings ...)
2012-06-07 13:56 ` [PATCH 10/12] agp/intel-gtt: move gart base addres setup Daniel Vetter
@ 2012-06-07 13:56 ` Daniel Vetter
2012-06-07 13:56 ` [PATCH 12/12] agp/intel-agp: remove snb+ host bridge pciids Daniel Vetter
2012-06-12 12:46 ` [PATCH 00/12] clear up drm/agp initialization madness Jani Nikula
12 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:56 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
When drm/i915 is in control of the gtt, we need to call
the enable function at all the relevant places ourselves.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/char/agp/intel-gtt.c | 3 ++-
drivers/gpu/drm/i915/i915_gem.c | 3 +++
include/drm/intel-gtt.h | 2 ++
3 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/drivers/char/agp/intel-gtt.c b/drivers/char/agp/intel-gtt.c
index 440e8d4..7397001 100644
--- a/drivers/char/agp/intel-gtt.c
+++ b/drivers/char/agp/intel-gtt.c
@@ -777,7 +777,7 @@ static void i830_write_entry(dma_addr_t addr, unsigned int entry,
writel(addr | pte_flags, intel_private.gtt + entry);
}
-static bool intel_enable_gtt(void)
+bool intel_enable_gtt(void)
{
u8 __iomem *reg;
@@ -823,6 +823,7 @@ static bool intel_enable_gtt(void)
return true;
}
+EXPORT_SYMBOL(intel_enable_gtt);
static int i830_setup(void)
{
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 108e4c2..2884b08 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3689,6 +3689,9 @@ i915_gem_init_hw(struct drm_device *dev)
drm_i915_private_t *dev_priv = dev->dev_private;
int ret;
+ if (!intel_enable_gtt())
+ return -EIO;
+
i915_gem_l3_remap(dev);
i915_gem_init_swizzling(dev);
diff --git a/include/drm/intel-gtt.h b/include/drm/intel-gtt.h
index 84ebd71..8e29d55 100644
--- a/include/drm/intel-gtt.h
+++ b/include/drm/intel-gtt.h
@@ -27,6 +27,8 @@ int intel_gmch_probe(struct pci_dev *bridge_pdev, struct pci_dev *gpu_pdev,
struct agp_bridge_data *bridge);
void intel_gmch_remove(void);
+bool intel_enable_gtt(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
^ permalink raw reply related [flat|nested] 23+ messages in thread* [PATCH 12/12] agp/intel-agp: remove snb+ host bridge pciids
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
` (10 preceding siblings ...)
2012-06-07 13:56 ` [PATCH 11/12] drm/i915: call intel_enable_gtt Daniel Vetter
@ 2012-06-07 13:56 ` Daniel Vetter
2012-06-12 12:46 ` [PATCH 00/12] clear up drm/agp initialization madness Jani Nikula
12 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-07 13:56 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
drm/i915 now takes care itself of setting up the gtt for
these chips.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/char/agp/intel-agp.c | 11 -----------
1 files changed, 0 insertions(+), 11 deletions(-)
diff --git a/drivers/char/agp/intel-agp.c b/drivers/char/agp/intel-agp.c
index c98c568..39c4a8f 100644
--- a/drivers/char/agp/intel-agp.c
+++ b/drivers/char/agp/intel-agp.c
@@ -902,17 +902,6 @@ static struct pci_device_id agp_intel_pci_table[] = {
ID(PCI_DEVICE_ID_INTEL_IRONLAKE_M_HB),
ID(PCI_DEVICE_ID_INTEL_IRONLAKE_MA_HB),
ID(PCI_DEVICE_ID_INTEL_IRONLAKE_MC2_HB),
- ID(PCI_DEVICE_ID_INTEL_SANDYBRIDGE_HB),
- ID(PCI_DEVICE_ID_INTEL_SANDYBRIDGE_M_HB),
- ID(PCI_DEVICE_ID_INTEL_SANDYBRIDGE_S_HB),
- ID(PCI_DEVICE_ID_INTEL_IVYBRIDGE_HB),
- ID(PCI_DEVICE_ID_INTEL_IVYBRIDGE_M_HB),
- ID(PCI_DEVICE_ID_INTEL_IVYBRIDGE_S_HB),
- ID(PCI_DEVICE_ID_INTEL_VALLEYVIEW_HB),
- ID(PCI_DEVICE_ID_INTEL_HASWELL_HB),
- ID(PCI_DEVICE_ID_INTEL_HASWELL_M_HB),
- ID(PCI_DEVICE_ID_INTEL_HASWELL_S_HB),
- ID(PCI_DEVICE_ID_INTEL_HASWELL_E_HB),
{ }
};
--
1.7.7.6
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 00/12] clear up drm/agp initialization madness
2012-06-07 13:55 [PATCH 00/12] clear up drm/agp initialization madness Daniel Vetter
` (11 preceding siblings ...)
2012-06-07 13:56 ` [PATCH 12/12] agp/intel-agp: remove snb+ host bridge pciids Daniel Vetter
@ 2012-06-12 12:46 ` Jani Nikula
2012-06-12 20:22 ` Daniel Vetter
12 siblings, 1 reply; 23+ messages in thread
From: Jani Nikula @ 2012-06-12 12:46 UTC (permalink / raw)
To: Intel Graphics Development, DRI Development; +Cc: Daniel Vetter
On Thu, 07 Jun 2012, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Comments, flames and reviews highly welcome. If possible I'd like to get the 3
> drm patches at the beginning in early for 3.6 so that we can decently test it
> (and have some time to pile more stuff on top of this in drm/i915 land).
On the series and the fixes to it, with the disclaimer that I don't have
much experience with drm/i915/gtt yet:
Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com>
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 00/12] clear up drm/agp initialization madness
2012-06-12 12:46 ` [PATCH 00/12] clear up drm/agp initialization madness Jani Nikula
@ 2012-06-12 20:22 ` Daniel Vetter
0 siblings, 0 replies; 23+ messages in thread
From: Daniel Vetter @ 2012-06-12 20:22 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development
On Tue, Jun 12, 2012 at 03:46:35PM +0300, Jani Nikula wrote:
> On Thu, 07 Jun 2012, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Comments, flames and reviews highly welcome. If possible I'd like to get the 3
> > drm patches at the beginning in early for 3.6 so that we can decently test it
> > (and have some time to pile more stuff on top of this in drm/i915 land).
>
> On the series and the fixes to it, with the disclaimer that I don't have
> much experience with drm/i915/gtt yet:
>
> Reviewed-by: Jani Nikula <jani.nikula@linux.intel.com>
Ok, I've picked up all the prep patches that don't depend upon the drm
core rework. Thanks for taking a lock at these patches.
-Daniel
--
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
^ permalink raw reply [flat|nested] 23+ messages in thread