[PATCH v2 00/12] drm/gma500: Various cleanups to GEM code

[PATCH v2 00/12] drm/gma500: Various cleanups to GEM code

[Thomas Zimmermann]

Refactor and simplify various parts of the memory management. This
includes locking, initialization and finalizer functions, and code
organization.

Tested on Atom N2800 hardware.

v2:
	* put common code in psb_gtt_{init,fini,resume}() into
	  helpers (Sam, Patrik)

Thomas Zimmermann (12):
  drm/gma500: Remove struct psb_gem_object.npage
  drm/gma500: Acquire reservation lock for GEM objects
  drm/gma500: Move GTT locking into GTT helpers
  drm/gma500: Remove struct psb_gtt.sem sempahore
  drm/gma500: Move GTT setup and restoration into helper funtions
  drm/gma500: Move GTT resume logic out of psb_gtt_init()
  drm/gma500: Cleanup GTT uninit and error handling
  drm/gma500: Split GTT init/resume/fini into GTT and GEM functions
  drm/gma500: Inline psb_gtt_restore()
  drm/gma500: Move GEM memory management functions to gem.c
  drm/gma500: Move GTT enable and disable code into helpers
  drm/gma500: Move GTT memory-range setup into helper

 drivers/gpu/drm/gma500/gem.c         | 161 ++++++++++++++-
 drivers/gpu/drm/gma500/gem.h         |  13 +-
 drivers/gpu/drm/gma500/gma_display.c |   8 +-
 drivers/gpu/drm/gma500/gtt.c         | 295 +++++++++++++--------------
 drivers/gpu/drm/gma500/gtt.h         |   8 +-
 drivers/gpu/drm/gma500/power.c       |   5 +-
 drivers/gpu/drm/gma500/psb_drv.c     |  13 +-
 drivers/gpu/drm/gma500/psb_drv.h     |   1 -
 8 files changed, 317 insertions(+), 187 deletions(-)


base-commit: 710a019ad85e96e66f7d75ee7f4733cdd8a2b0d0
prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
prerequisite-patch-id: 6e1032c6302461624f33194c8b8f37103a3fa6ef
-- 
2.35.1

[PATCH v2 01/12] drm/gma500: Remove struct psb_gem_object.npage

[Thomas Zimmermann]

Calculate the number of pages in the BO's backing storage from
the size. Remove the npage field.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gem.c         | 9 +++++----
 drivers/gpu/drm/gma500/gem.h         | 1 -
 drivers/gpu/drm/gma500/gma_display.c | 8 +++-----
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index 8d65af80bb08..fe7d242567c0 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -51,7 +51,6 @@ int psb_gem_pin(struct psb_gem_object *pobj)
 			     (gpu_base + pobj->offset), npages, 0, 0,
 			     PSB_MMU_CACHED_MEMORY);
 
-	pobj->npage = npages;
 	pobj->pages = pages;
 
 out:
@@ -71,6 +70,7 @@ void psb_gem_unpin(struct psb_gem_object *pobj)
 	struct drm_device *dev = obj->dev;
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	u32 gpu_base = dev_priv->gtt.gatt_start;
+	unsigned long npages;
 
 	mutex_lock(&dev_priv->gtt_mutex);
 
@@ -81,16 +81,17 @@ void psb_gem_unpin(struct psb_gem_object *pobj)
 	if (pobj->in_gart || pobj->stolen)
 		goto out;
 
+	npages = obj->size / PAGE_SIZE;
+
 	psb_mmu_remove_pages(psb_mmu_get_default_pd(dev_priv->mmu),
-			     (gpu_base + pobj->offset), pobj->npage, 0, 0);
+			     (gpu_base + pobj->offset), npages, 0, 0);
 	psb_gtt_remove_pages(dev_priv, &pobj->resource);
 
 	/* Reset caching flags */
-	set_pages_array_wb(pobj->pages, pobj->npage);
+	set_pages_array_wb(pobj->pages, npages);
 
 	drm_gem_put_pages(obj, pobj->pages, true, false);
 	pobj->pages = NULL;
-	pobj->npage = 0;
 
 out:
 	mutex_unlock(&dev_priv->gtt_mutex);
diff --git a/drivers/gpu/drm/gma500/gem.h b/drivers/gpu/drm/gma500/gem.h
index 79cced40c87f..96ef864b4bf1 100644
--- a/drivers/gpu/drm/gma500/gem.h
+++ b/drivers/gpu/drm/gma500/gem.h
@@ -23,7 +23,6 @@ struct psb_gem_object {
 	bool stolen;			/* Backed from stolen RAM */
 	bool mmapping;			/* Is mmappable */
 	struct page **pages;		/* Backing pages if present */
-	int npage;			/* Number of backing pages */
 };
 
 static inline struct psb_gem_object *to_psb_gem_object(struct drm_gem_object *obj)
diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index 60ba7de59139..dd801404cf99 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -391,11 +391,9 @@ int gma_crtc_cursor_set(struct drm_crtc *crtc,
 			goto unref_cursor;
 		}
 
-		/* Prevent overflow */
-		if (pobj->npage > 4)
-			cursor_pages = 4;
-		else
-			cursor_pages = pobj->npage;
+		cursor_pages = obj->size / PAGE_SIZE;
+		if (cursor_pages > 4)
+			cursor_pages = 4; /* Prevent overflow */
 
 		/* Copy the cursor to cursor mem */
 		tmp_dst = dev_priv->vram_addr + cursor_pobj->offset;
-- 
2.35.1

[PATCH v2 02/12] drm/gma500: Acquire reservation lock for GEM objects

[Thomas Zimmermann]

Protect concurrent access to struct psb_gem_object by acquiring
the GEM object's reservation lock; as it's supposed to be. The
use of the GTT mutex can now be moved into GTT code.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gem.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index fe7d242567c0..026ce11f7460 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -31,6 +31,10 @@ int psb_gem_pin(struct psb_gem_object *pobj)
 	unsigned int npages;
 	int ret;
 
+	ret = dma_resv_lock(obj->resv, NULL);
+	if (drm_WARN_ONCE(dev, ret, "dma_resv_lock() failed, ret=%d\n", ret))
+		return ret;
+
 	mutex_lock(&dev_priv->gtt_mutex);
 
 	if (pobj->in_gart || pobj->stolen)
@@ -56,11 +60,13 @@ int psb_gem_pin(struct psb_gem_object *pobj)
 out:
 	++pobj->in_gart;
 	mutex_unlock(&dev_priv->gtt_mutex);
+	dma_resv_unlock(obj->resv);
 
 	return 0;
 
 err_mutex_unlock:
 	mutex_unlock(&dev_priv->gtt_mutex);
+	dma_resv_unlock(obj->resv);
 	return ret;
 }
 
@@ -71,6 +77,11 @@ void psb_gem_unpin(struct psb_gem_object *pobj)
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	u32 gpu_base = dev_priv->gtt.gatt_start;
 	unsigned long npages;
+	int ret;
+
+	ret = dma_resv_lock(obj->resv, NULL);
+	if (drm_WARN_ONCE(dev, ret, "dma_resv_lock() failed, ret=%d\n", ret))
+		return;
 
 	mutex_lock(&dev_priv->gtt_mutex);
 
@@ -95,6 +106,7 @@ void psb_gem_unpin(struct psb_gem_object *pobj)
 
 out:
 	mutex_unlock(&dev_priv->gtt_mutex);
+	dma_resv_unlock(obj->resv);
 }
 
 static vm_fault_t psb_gem_fault(struct vm_fault *vmf);
-- 
2.35.1

[PATCH v2 03/12] drm/gma500: Move GTT locking into GTT helpers

[Thomas Zimmermann]

Acquire the GTT mutex in psb_gtt_{insert,remove}_pages(). Remove
locking from callers. Also remove the GTT locking around the resume
code. Resume does not run concurrently with other GTT operations.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gem.c | 11 ++---------
 drivers/gpu/drm/gma500/gtt.c | 24 +++++++++++-------------
 2 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index 026ce11f7460..2f44535f58e7 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -35,15 +35,13 @@ int psb_gem_pin(struct psb_gem_object *pobj)
 	if (drm_WARN_ONCE(dev, ret, "dma_resv_lock() failed, ret=%d\n", ret))
 		return ret;
 
-	mutex_lock(&dev_priv->gtt_mutex);
-
 	if (pobj->in_gart || pobj->stolen)
 		goto out; /* already mapped */
 
 	pages = drm_gem_get_pages(obj);
 	if (IS_ERR(pages)) {
 		ret = PTR_ERR(pages);
-		goto err_mutex_unlock;
+		goto err_dma_resv_unlock;
 	}
 
 	npages = obj->size / PAGE_SIZE;
@@ -59,13 +57,11 @@ int psb_gem_pin(struct psb_gem_object *pobj)
 
 out:
 	++pobj->in_gart;
-	mutex_unlock(&dev_priv->gtt_mutex);
 	dma_resv_unlock(obj->resv);
 
 	return 0;
 
-err_mutex_unlock:
-	mutex_unlock(&dev_priv->gtt_mutex);
+err_dma_resv_unlock:
 	dma_resv_unlock(obj->resv);
 	return ret;
 }
@@ -83,8 +79,6 @@ void psb_gem_unpin(struct psb_gem_object *pobj)
 	if (drm_WARN_ONCE(dev, ret, "dma_resv_lock() failed, ret=%d\n", ret))
 		return;
 
-	mutex_lock(&dev_priv->gtt_mutex);
-
 	WARN_ON(!pobj->in_gart);
 
 	--pobj->in_gart;
@@ -105,7 +99,6 @@ void psb_gem_unpin(struct psb_gem_object *pobj)
 	pobj->pages = NULL;
 
 out:
-	mutex_unlock(&dev_priv->gtt_mutex);
 	dma_resv_unlock(obj->resv);
 }
 
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 309ffe921bfd..4202e88e5f84 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -74,11 +74,7 @@ static u32 __iomem *psb_gtt_entry(struct drm_psb_private *pdev, const struct res
 	return pdev->gtt_map + (offset >> PAGE_SHIFT);
 }
 
-/*
- * Take our preallocated GTT range and insert the GEM object into
- * the GTT. This is protected via the gtt mutex which the caller
- * must hold.
- */
+/* Acquires GTT mutex internally. */
 void psb_gtt_insert_pages(struct drm_psb_private *pdev, const struct resource *res,
 			  struct page **pages)
 {
@@ -86,6 +82,8 @@ void psb_gtt_insert_pages(struct drm_psb_private *pdev, const struct resource *r
 	u32 __iomem *gtt_slot;
 	u32 pte;
 
+	mutex_lock(&pdev->gtt_mutex);
+
 	/* Write our page entries into the GTT itself */
 
 	npages = resource_size(res) >> PAGE_SHIFT;
@@ -98,19 +96,19 @@ void psb_gtt_insert_pages(struct drm_psb_private *pdev, const struct resource *r
 
 	/* Make sure all the entries are set before we return */
 	ioread32(gtt_slot - 1);
+
+	mutex_unlock(&pdev->gtt_mutex);
 }
 
-/*
- * Remove a preallocated GTT range from the GTT. Overwrite all the
- * page table entries with the dummy page. This is protected via the gtt
- * mutex which the caller must hold.
- */
+/* Acquires GTT mutex internally. */
 void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *res)
 {
 	resource_size_t npages, i;
 	u32 __iomem *gtt_slot;
 	u32 pte;
 
+	mutex_lock(&pdev->gtt_mutex);
+
 	/* Install scratch page for the resource */
 
 	pte = psb_gtt_mask_pte(page_to_pfn(pdev->scratch_page), PSB_MMU_CACHED_MEMORY);
@@ -123,6 +121,8 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *r
 
 	/* Make sure all the entries are set before we return */
 	ioread32(gtt_slot - 1);
+
+	mutex_unlock(&pdev->gtt_mutex);
 }
 
 static void psb_gtt_alloc(struct drm_device *dev)
@@ -306,8 +306,6 @@ int psb_gtt_restore(struct drm_device *dev)
 	struct psb_gem_object *pobj;
 	unsigned int restored = 0, total = 0, size = 0;
 
-	/* On resume, the gtt_mutex is already initialized */
-	mutex_lock(&dev_priv->gtt_mutex);
 	psb_gtt_init(dev, 1);
 
 	while (r != NULL) {
@@ -325,7 +323,7 @@ int psb_gtt_restore(struct drm_device *dev)
 		r = r->sibling;
 		total++;
 	}
-	mutex_unlock(&dev_priv->gtt_mutex);
+
 	DRM_DEBUG_DRIVER("Restored %u of %u gtt ranges (%u KB)", restored,
 			 total, (size / 1024));
 
-- 
2.35.1

[PATCH v2 04/12] drm/gma500: Remove struct psb_gtt.sem sempahore

[Thomas Zimmermann]

The semaphore at struct psb_mmu_driver.sem protects access to the MMU
fields. Additional locking with struct psb_gtt.sem is unnecessary. Remove
the field and related code.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gtt.c     | 7 -------
 drivers/gpu/drm/gma500/gtt.h     | 1 -
 drivers/gpu/drm/gma500/psb_drv.c | 4 ----
 3 files changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 4202e88e5f84..c7b7cb1f2d13 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -125,12 +125,6 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *r
 	mutex_unlock(&pdev->gtt_mutex);
 }
 
-static void psb_gtt_alloc(struct drm_device *dev)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	init_rwsem(&dev_priv->gtt.sem);
-}
-
 void psb_gtt_takedown(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
@@ -166,7 +160,6 @@ int psb_gtt_init(struct drm_device *dev, int resume)
 	if (!resume) {
 		mutex_init(&dev_priv->gtt_mutex);
 		mutex_init(&dev_priv->mmap_mutex);
-		psb_gtt_alloc(dev);
 	}
 
 	pg = &dev_priv->gtt;
diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h
index ff1dcdd1ff52..31500533ac45 100644
--- a/drivers/gpu/drm/gma500/gtt.h
+++ b/drivers/gpu/drm/gma500/gtt.h
@@ -22,7 +22,6 @@ struct psb_gtt {
 	unsigned gatt_pages;
 	unsigned long stolen_size;
 	unsigned long vram_stolen_size;
-	struct rw_semaphore sem;
 };
 
 /* Exported functions */
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index eeb681be9c95..7227a8e44d23 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -184,13 +184,11 @@ static void psb_driver_unload(struct drm_device *dev)
 	if (dev_priv->mmu) {
 		struct psb_gtt *pg = &dev_priv->gtt;
 
-		down_read(&pg->sem);
 		psb_mmu_remove_pfn_sequence(
 			psb_mmu_get_default_pd
 			(dev_priv->mmu),
 			pg->mmu_gatt_start,
 			dev_priv->vram_stolen_size >> PAGE_SHIFT);
-		up_read(&pg->sem);
 		psb_mmu_driver_takedown(dev_priv->mmu);
 		dev_priv->mmu = NULL;
 	}
@@ -345,12 +343,10 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
 		return ret;
 
 	/* Add stolen memory to SGX MMU */
-	down_read(&pg->sem);
 	ret = psb_mmu_insert_pfn_sequence(psb_mmu_get_default_pd(dev_priv->mmu),
 					  dev_priv->stolen_base >> PAGE_SHIFT,
 					  pg->gatt_start,
 					  pg->stolen_size >> PAGE_SHIFT, 0);
-	up_read(&pg->sem);
 
 	psb_mmu_set_pd_context(psb_mmu_get_default_pd(dev_priv->mmu), 0);
 	psb_mmu_set_pd_context(dev_priv->pf_pd, 1);
-- 
2.35.1

[PATCH v2 05/12] drm/gma500: Move GTT setup and restoration into helper funtions

[Thomas Zimmermann]

The GTT init and restore functions contain logic to populate the
GTT entries. Move the code into helper functions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gtt.c | 115 +++++++++++++++++++++--------------
 1 file changed, 68 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index c7b7cb1f2d13..acd50ee26b03 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -144,18 +144,79 @@ void psb_gtt_takedown(struct drm_device *dev)
 		iounmap(dev_priv->gtt_map);
 }
 
+/* Clear GTT. Use a scratch page to avoid accidents or scribbles. */
+static void psb_gtt_clear(struct drm_psb_private *pdev)
+{
+	resource_size_t pfn_base;
+	unsigned long i;
+	uint32_t pte;
+
+	pfn_base = page_to_pfn(pdev->scratch_page);
+	pte = psb_gtt_mask_pte(pfn_base, PSB_MMU_CACHED_MEMORY);
+
+	for (i = 0; i < pdev->gtt.gtt_pages; ++i)
+		iowrite32(pte, pdev->gtt_map + i);
+
+	(void)ioread32(pdev->gtt_map + i - 1);
+}
+
+/* Insert vram stolen pages into the GTT. */
+static void psb_gtt_populate_stolen(struct drm_psb_private *pdev)
+{
+	struct drm_device *dev = &pdev->dev;
+	unsigned int pfn_base;
+	unsigned int i, num_pages;
+	uint32_t pte;
+
+	pfn_base = pdev->stolen_base >> PAGE_SHIFT;
+	num_pages = pdev->vram_stolen_size >> PAGE_SHIFT;
+
+	drm_dbg(dev, "Set up %u stolen pages starting at 0x%08x, GTT offset %dK\n",
+		num_pages, pfn_base << PAGE_SHIFT, 0);
+
+	for (i = 0; i < num_pages; ++i) {
+		pte = psb_gtt_mask_pte(pfn_base + i, PSB_MMU_CACHED_MEMORY);
+		iowrite32(pte, pdev->gtt_map + i);
+	}
+
+	(void)ioread32(pdev->gtt_map + i - 1);
+}
+
+/* Re-insert all pinned GEM objects into GTT. */
+static void psb_gtt_populate_resources(struct drm_psb_private *pdev)
+{
+	unsigned int restored = 0, total = 0, size = 0;
+	struct resource *r = pdev->gtt_mem->child;
+	struct drm_device *dev = &pdev->dev;
+	struct psb_gem_object *pobj;
+
+	while (r) {
+		/*
+		 * TODO: GTT restoration needs a refactoring, so that we don't have to touch
+		 *       struct psb_gem_object here. The type represents a GEM object and is
+		 *       not related to the GTT itself.
+		 */
+		pobj = container_of(r, struct psb_gem_object, resource);
+		if (pobj->pages) {
+			psb_gtt_insert_pages(pdev, &pobj->resource, pobj->pages);
+			size += resource_size(&pobj->resource);
+			++restored;
+		}
+		r = r->sibling;
+		++total;
+	}
+
+	drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total, (size / 1024));
+}
+
 int psb_gtt_init(struct drm_device *dev, int resume)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	unsigned gtt_pages;
 	unsigned long stolen_size, vram_stolen_size;
-	unsigned i, num_pages;
-	unsigned pfn_base;
 	struct psb_gtt *pg;
-
 	int ret = 0;
-	uint32_t pte;
 
 	if (!resume) {
 		mutex_init(&dev_priv->gtt_mutex);
@@ -262,29 +323,9 @@ int psb_gtt_init(struct drm_device *dev, int resume)
 		goto out_err;
 	}
 
-	/*
-	 * Insert vram stolen pages into the GTT
-	 */
-
-	pfn_base = dev_priv->stolen_base >> PAGE_SHIFT;
-	num_pages = vram_stolen_size >> PAGE_SHIFT;
-	dev_dbg(dev->dev, "Set up %d stolen pages starting at 0x%08x, GTT offset %dK\n",
-		num_pages, pfn_base << PAGE_SHIFT, 0);
-	for (i = 0; i < num_pages; ++i) {
-		pte = psb_gtt_mask_pte(pfn_base + i, PSB_MMU_CACHED_MEMORY);
-		iowrite32(pte, dev_priv->gtt_map + i);
-	}
-
-	/*
-	 * Init rest of GTT to the scratch page to avoid accidents or scribbles
-	 */
-
-	pfn_base = page_to_pfn(dev_priv->scratch_page);
-	pte = psb_gtt_mask_pte(pfn_base, PSB_MMU_CACHED_MEMORY);
-	for (; i < gtt_pages; ++i)
-		iowrite32(pte, dev_priv->gtt_map + i);
+	psb_gtt_clear(dev_priv);
+	psb_gtt_populate_stolen(dev_priv);
 
-	(void) ioread32(dev_priv->gtt_map + i - 1);
 	return 0;
 
 out_err:
@@ -295,30 +336,10 @@ int psb_gtt_init(struct drm_device *dev, int resume)
 int psb_gtt_restore(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct resource *r = dev_priv->gtt_mem->child;
-	struct psb_gem_object *pobj;
-	unsigned int restored = 0, total = 0, size = 0;
 
 	psb_gtt_init(dev, 1);
 
-	while (r != NULL) {
-		/*
-		 * TODO: GTT restoration needs a refactoring, so that we don't have to touch
-		 *       struct psb_gem_object here. The type represents a GEM object and is
-		 *       not related to the GTT itself.
-		 */
-		pobj = container_of(r, struct psb_gem_object, resource);
-		if (pobj->pages) {
-			psb_gtt_insert_pages(dev_priv, &pobj->resource, pobj->pages);
-			size += pobj->resource.end - pobj->resource.start;
-			restored++;
-		}
-		r = r->sibling;
-		total++;
-	}
-
-	DRM_DEBUG_DRIVER("Restored %u of %u gtt ranges (%u KB)", restored,
-			 total, (size / 1024));
+	psb_gtt_populate_resources(dev_priv);
 
 	return 0;
 }
-- 
2.35.1

[PATCH v2 06/12] drm/gma500: Move GTT resume logic out of psb_gtt_init()

[Thomas Zimmermann]

The current implementation of psb_gtt_init() also does resume
handling. Move the resume code into its own helper.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gtt.c     | 122 ++++++++++++++++++++++++++-----
 drivers/gpu/drm/gma500/gtt.h     |   2 +-
 drivers/gpu/drm/gma500/psb_drv.c |   2 +-
 3 files changed, 104 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index acd50ee26b03..43ad3ec38c80 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -209,7 +209,7 @@ static void psb_gtt_populate_resources(struct drm_psb_private *pdev)
 	drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total, (size / 1024));
 }
 
-int psb_gtt_init(struct drm_device *dev, int resume)
+int psb_gtt_init(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
@@ -218,10 +218,8 @@ int psb_gtt_init(struct drm_device *dev, int resume)
 	struct psb_gtt *pg;
 	int ret = 0;
 
-	if (!resume) {
-		mutex_init(&dev_priv->gtt_mutex);
-		mutex_init(&dev_priv->mmap_mutex);
-	}
+	mutex_init(&dev_priv->gtt_mutex);
+	mutex_init(&dev_priv->mmap_mutex);
 
 	pg = &dev_priv->gtt;
 
@@ -290,13 +288,6 @@ int psb_gtt_init(struct drm_device *dev, int resume)
 	dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
 			dev_priv->stolen_base, vram_stolen_size / 1024);
 
-	if (resume && (gtt_pages != pg->gtt_pages) &&
-	    (stolen_size != pg->stolen_size)) {
-		dev_err(dev->dev, "GTT resume error.\n");
-		ret = -EINVAL;
-		goto out_err;
-	}
-
 	pg->gtt_pages = gtt_pages;
 	pg->stolen_size = stolen_size;
 	dev_priv->vram_stolen_size = vram_stolen_size;
@@ -304,19 +295,14 @@ int psb_gtt_init(struct drm_device *dev, int resume)
 	/*
 	 *	Map the GTT and the stolen memory area
 	 */
-	if (!resume)
-		dev_priv->gtt_map = ioremap(pg->gtt_phys_start,
-						gtt_pages << PAGE_SHIFT);
+	dev_priv->gtt_map = ioremap(pg->gtt_phys_start, gtt_pages << PAGE_SHIFT);
 	if (!dev_priv->gtt_map) {
 		dev_err(dev->dev, "Failure to map gtt.\n");
 		ret = -ENOMEM;
 		goto out_err;
 	}
 
-	if (!resume)
-		dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base,
-						 stolen_size);
-
+	dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
 	if (!dev_priv->vram_addr) {
 		dev_err(dev->dev, "Failure to map stolen base.\n");
 		ret = -ENOMEM;
@@ -333,11 +319,107 @@ int psb_gtt_init(struct drm_device *dev, int resume)
 	return ret;
 }
 
+static int psb_gtt_resume(struct drm_device *dev)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	unsigned int gtt_pages;
+	unsigned long stolen_size, vram_stolen_size;
+	struct psb_gtt *pg;
+	int ret = 0;
+
+	pg = &dev_priv->gtt;
+
+	/* Enable the GTT */
+	pci_read_config_word(pdev, PSB_GMCH_CTRL, &dev_priv->gmch_ctrl);
+	pci_write_config_word(pdev, PSB_GMCH_CTRL,
+			      dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
+
+	dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
+	PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
+	(void) PSB_RVDC32(PSB_PGETBL_CTL);
+
+	/* The root resource we allocate address space from */
+	dev_priv->gtt_initialized = 1;
+
+	pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
+
+	/*
+	 *	The video mmu has a hw bug when accessing 0x0D0000000.
+	 *	Make gatt start at 0x0e000,0000. This doesn't actually
+	 *	matter for us but may do if the video acceleration ever
+	 *	gets opened up.
+	 */
+	pg->mmu_gatt_start = 0xE0000000;
+
+	pg->gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
+	gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE) >> PAGE_SHIFT;
+	/* CDV doesn't report this. In which case the system has 64 gtt pages */
+	if (pg->gtt_start == 0 || gtt_pages == 0) {
+		dev_dbg(dev->dev, "GTT PCI BAR not initialized.\n");
+		gtt_pages = 64;
+		pg->gtt_start = dev_priv->pge_ctl;
+	}
+
+	pg->gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
+	pg->gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE) >> PAGE_SHIFT;
+	dev_priv->gtt_mem = &pdev->resource[PSB_GATT_RESOURCE];
+
+	if (pg->gatt_pages == 0 || pg->gatt_start == 0) {
+		static struct resource fudge;	/* Preferably peppermint */
+		/*
+		 * This can occur on CDV systems. Fudge it in this case. We
+		 * really don't care what imaginary space is being allocated
+		 * at this point.
+		 */
+		dev_dbg(dev->dev, "GATT PCI BAR not initialized.\n");
+		pg->gatt_start = 0x40000000;
+		pg->gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
+		/*
+		 * This is a little confusing but in fact the GTT is providing
+		 * a view from the GPU into memory and not vice versa. As such
+		 *  this is really allocating space that is not the same as the
+		 *  CPU address space on CDV.
+		 */
+		fudge.start = 0x40000000;
+		fudge.end = 0x40000000 + 128 * 1024 * 1024 - 1;
+		fudge.name = "fudge";
+		fudge.flags = IORESOURCE_MEM;
+		dev_priv->gtt_mem = &fudge;
+	}
+
+	pci_read_config_dword(pdev, PSB_BSM, &dev_priv->stolen_base);
+	vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base - PAGE_SIZE;
+	stolen_size = vram_stolen_size;
+
+	dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
+			dev_priv->stolen_base, vram_stolen_size / 1024);
+
+	if ((gtt_pages != pg->gtt_pages) && (stolen_size != pg->stolen_size)) {
+		dev_err(dev->dev, "GTT resume error.\n");
+		ret = -EINVAL;
+		goto out_err;
+	}
+
+	pg->gtt_pages = gtt_pages;
+	pg->stolen_size = stolen_size;
+	dev_priv->vram_stolen_size = vram_stolen_size;
+
+	psb_gtt_clear(dev_priv);
+	psb_gtt_populate_stolen(dev_priv);
+
+	return 0;
+
+out_err:
+	psb_gtt_takedown(dev);
+	return ret;
+}
+
 int psb_gtt_restore(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 
-	psb_gtt_init(dev, 1);
+	psb_gtt_resume(dev);
 
 	psb_gtt_populate_resources(dev_priv);
 
diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h
index 31500533ac45..cb270ea40794 100644
--- a/drivers/gpu/drm/gma500/gtt.h
+++ b/drivers/gpu/drm/gma500/gtt.h
@@ -25,7 +25,7 @@ struct psb_gtt {
 };
 
 /* Exported functions */
-extern int psb_gtt_init(struct drm_device *dev, int resume);
+int psb_gtt_init(struct drm_device *dev);
 extern void psb_gtt_takedown(struct drm_device *dev);
 extern int psb_gtt_restore(struct drm_device *dev);
 
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 7227a8e44d23..2891a3dc8d2e 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -324,7 +324,7 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
 
 	set_pages_uc(dev_priv->scratch_page, 1);
 
-	ret = psb_gtt_init(dev, 0);
+	ret = psb_gtt_init(dev);
 	if (ret)
 		goto out_err;
 
-- 
2.35.1

[PATCH v2 07/12] drm/gma500: Cleanup GTT uninit and error handling

[Thomas Zimmermann]

Replace psb_gtt_takedown() with finalizer function that is only called
for unloading the driver. Use roll-back pattern for error handling in
psb_gtt_init() and _resume(). Also fixes a bug where vmap_addr was never
unmapped.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gtt.c     | 49 ++++++++++++++++----------------
 drivers/gpu/drm/gma500/gtt.h     |  2 +-
 drivers/gpu/drm/gma500/psb_drv.c |  2 +-
 drivers/gpu/drm/gma500/psb_drv.h |  1 -
 4 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 43ad3ec38c80..99c644a5c5cb 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -125,23 +125,20 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *r
 	mutex_unlock(&pdev->gtt_mutex);
 }
 
-void psb_gtt_takedown(struct drm_device *dev)
+void psb_gtt_fini(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 
-	if (dev_priv->gtt_map) {
-		iounmap(dev_priv->gtt_map);
-		dev_priv->gtt_map = NULL;
-	}
-	if (dev_priv->gtt_initialized) {
-		pci_write_config_word(pdev, PSB_GMCH_CTRL,
-				      dev_priv->gmch_ctrl);
-		PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
-		(void) PSB_RVDC32(PSB_PGETBL_CTL);
-	}
-	if (dev_priv->vram_addr)
-		iounmap(dev_priv->gtt_map);
+	iounmap(dev_priv->vram_addr);
+	iounmap(dev_priv->gtt_map);
+
+	pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
+	PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+	(void)PSB_RVDC32(PSB_PGETBL_CTL);
+
+	mutex_destroy(&dev_priv->mmap_mutex);
+	mutex_destroy(&dev_priv->gtt_mutex);
 }
 
 /* Clear GTT. Use a scratch page to avoid accidents or scribbles. */
@@ -233,8 +230,6 @@ int psb_gtt_init(struct drm_device *dev)
 	(void) PSB_RVDC32(PSB_PGETBL_CTL);
 
 	/* The root resource we allocate address space from */
-	dev_priv->gtt_initialized = 1;
-
 	pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
 
 	/*
@@ -299,14 +294,14 @@ int psb_gtt_init(struct drm_device *dev)
 	if (!dev_priv->gtt_map) {
 		dev_err(dev->dev, "Failure to map gtt.\n");
 		ret = -ENOMEM;
-		goto out_err;
+		goto err_gtt_disable;
 	}
 
 	dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
 	if (!dev_priv->vram_addr) {
 		dev_err(dev->dev, "Failure to map stolen base.\n");
 		ret = -ENOMEM;
-		goto out_err;
+		goto err_iounmap;
 	}
 
 	psb_gtt_clear(dev_priv);
@@ -314,8 +309,14 @@ int psb_gtt_init(struct drm_device *dev)
 
 	return 0;
 
-out_err:
-	psb_gtt_takedown(dev);
+err_iounmap:
+	iounmap(dev_priv->gtt_map);
+err_gtt_disable:
+	pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
+	PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+	(void)PSB_RVDC32(PSB_PGETBL_CTL);
+	mutex_destroy(&dev_priv->mmap_mutex);
+	mutex_destroy(&dev_priv->gtt_mutex);
 	return ret;
 }
 
@@ -340,8 +341,6 @@ static int psb_gtt_resume(struct drm_device *dev)
 	(void) PSB_RVDC32(PSB_PGETBL_CTL);
 
 	/* The root resource we allocate address space from */
-	dev_priv->gtt_initialized = 1;
-
 	pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
 
 	/*
@@ -398,7 +397,7 @@ static int psb_gtt_resume(struct drm_device *dev)
 	if ((gtt_pages != pg->gtt_pages) && (stolen_size != pg->stolen_size)) {
 		dev_err(dev->dev, "GTT resume error.\n");
 		ret = -EINVAL;
-		goto out_err;
+		goto err_gtt_disable;
 	}
 
 	pg->gtt_pages = gtt_pages;
@@ -410,8 +409,10 @@ static int psb_gtt_resume(struct drm_device *dev)
 
 	return 0;
 
-out_err:
-	psb_gtt_takedown(dev);
+err_gtt_disable:
+	pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
+	PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+	(void)PSB_RVDC32(PSB_PGETBL_CTL);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h
index cb270ea40794..5839d5d06adc 100644
--- a/drivers/gpu/drm/gma500/gtt.h
+++ b/drivers/gpu/drm/gma500/gtt.h
@@ -26,7 +26,7 @@ struct psb_gtt {
 
 /* Exported functions */
 int psb_gtt_init(struct drm_device *dev);
-extern void psb_gtt_takedown(struct drm_device *dev);
+void psb_gtt_fini(struct drm_device *dev);
 extern int psb_gtt_restore(struct drm_device *dev);
 
 int psb_gtt_allocate_resource(struct drm_psb_private *pdev, struct resource *res,
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 2891a3dc8d2e..41be6e1ac7f9 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -192,7 +192,7 @@ static void psb_driver_unload(struct drm_device *dev)
 		psb_mmu_driver_takedown(dev_priv->mmu);
 		dev_priv->mmu = NULL;
 	}
-	psb_gtt_takedown(dev);
+	psb_gtt_fini(dev);
 	if (dev_priv->scratch_page) {
 		set_pages_wb(dev_priv->scratch_page, 1);
 		__free_page(dev_priv->scratch_page);
diff --git a/drivers/gpu/drm/gma500/psb_drv.h b/drivers/gpu/drm/gma500/psb_drv.h
index 0439b10d3db5..553d03190ce1 100644
--- a/drivers/gpu/drm/gma500/psb_drv.h
+++ b/drivers/gpu/drm/gma500/psb_drv.h
@@ -408,7 +408,6 @@ struct drm_psb_private {
 	uint32_t stolen_base;
 	u8 __iomem *vram_addr;
 	unsigned long vram_stolen_size;
-	int gtt_initialized;
 	u16 gmch_ctrl;		/* Saved GTT setup */
 	u32 pge_ctl;
 
-- 
2.35.1

[PATCH v2 08/12] drm/gma500: Split GTT init/resume/fini into GTT and GEM functions

[Thomas Zimmermann]

The GTT init, fini and resume functions contain both, GTT and GEM,
code. Split each into a separate GTT and a GEM function. The GEM
code is responsible for mmap_mutex and the stolen memory area. The
rest of the functionality is left in GTT functions.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gtt.c     | 118 ++++++++++++++++++++-----------
 drivers/gpu/drm/gma500/gtt.h     |   3 +
 drivers/gpu/drm/gma500/psb_drv.c |   4 ++
 3 files changed, 83 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 99c644a5c5cb..0e99774c7e59 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -125,19 +125,26 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *r
 	mutex_unlock(&pdev->gtt_mutex);
 }
 
+void psb_gem_mm_fini(struct drm_device *dev)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+
+	iounmap(dev_priv->vram_addr);
+
+	mutex_destroy(&dev_priv->mmap_mutex);
+}
+
 void psb_gtt_fini(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 
-	iounmap(dev_priv->vram_addr);
 	iounmap(dev_priv->gtt_map);
 
 	pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
 	PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
 	(void)PSB_RVDC32(PSB_PGETBL_CTL);
 
-	mutex_destroy(&dev_priv->mmap_mutex);
 	mutex_destroy(&dev_priv->gtt_mutex);
 }
 
@@ -211,12 +218,10 @@ int psb_gtt_init(struct drm_device *dev)
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	unsigned gtt_pages;
-	unsigned long stolen_size, vram_stolen_size;
 	struct psb_gtt *pg;
 	int ret = 0;
 
 	mutex_init(&dev_priv->gtt_mutex);
-	mutex_init(&dev_priv->mmap_mutex);
 
 	pg = &dev_priv->gtt;
 
@@ -274,22 +279,8 @@ int psb_gtt_init(struct drm_device *dev)
 		dev_priv->gtt_mem = &fudge;
 	}
 
-	pci_read_config_dword(pdev, PSB_BSM, &dev_priv->stolen_base);
-	vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base
-								- PAGE_SIZE;
-
-	stolen_size = vram_stolen_size;
-
-	dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
-			dev_priv->stolen_base, vram_stolen_size / 1024);
-
 	pg->gtt_pages = gtt_pages;
-	pg->stolen_size = stolen_size;
-	dev_priv->vram_stolen_size = vram_stolen_size;
 
-	/*
-	 *	Map the GTT and the stolen memory area
-	 */
 	dev_priv->gtt_map = ioremap(pg->gtt_phys_start, gtt_pages << PAGE_SHIFT);
 	if (!dev_priv->gtt_map) {
 		dev_err(dev->dev, "Failure to map gtt.\n");
@@ -297,26 +288,54 @@ int psb_gtt_init(struct drm_device *dev)
 		goto err_gtt_disable;
 	}
 
+	psb_gtt_clear(dev_priv);
+
+	return 0;
+
+err_gtt_disable:
+	pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
+	PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+	(void)PSB_RVDC32(PSB_PGETBL_CTL);
+	mutex_destroy(&dev_priv->gtt_mutex);
+	return ret;
+}
+
+int psb_gem_mm_init(struct drm_device *dev)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	unsigned long stolen_size, vram_stolen_size;
+	struct psb_gtt *pg;
+	int ret;
+
+	mutex_init(&dev_priv->mmap_mutex);
+
+	pg = &dev_priv->gtt;
+
+	pci_read_config_dword(pdev, PSB_BSM, &dev_priv->stolen_base);
+	vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base - PAGE_SIZE;
+
+	stolen_size = vram_stolen_size;
+
+	dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
+		dev_priv->stolen_base, vram_stolen_size / 1024);
+
+	pg->stolen_size = stolen_size;
+	dev_priv->vram_stolen_size = vram_stolen_size;
+
 	dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
 	if (!dev_priv->vram_addr) {
 		dev_err(dev->dev, "Failure to map stolen base.\n");
 		ret = -ENOMEM;
-		goto err_iounmap;
+		goto err_mutex_destroy;
 	}
 
-	psb_gtt_clear(dev_priv);
 	psb_gtt_populate_stolen(dev_priv);
 
 	return 0;
 
-err_iounmap:
-	iounmap(dev_priv->gtt_map);
-err_gtt_disable:
-	pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
-	PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
-	(void)PSB_RVDC32(PSB_PGETBL_CTL);
+err_mutex_destroy:
 	mutex_destroy(&dev_priv->mmap_mutex);
-	mutex_destroy(&dev_priv->gtt_mutex);
 	return ret;
 }
 
@@ -325,9 +344,8 @@ static int psb_gtt_resume(struct drm_device *dev)
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	unsigned int gtt_pages;
-	unsigned long stolen_size, vram_stolen_size;
 	struct psb_gtt *pg;
-	int ret = 0;
+	int ret;
 
 	pg = &dev_priv->gtt;
 
@@ -387,27 +405,15 @@ static int psb_gtt_resume(struct drm_device *dev)
 		dev_priv->gtt_mem = &fudge;
 	}
 
-	pci_read_config_dword(pdev, PSB_BSM, &dev_priv->stolen_base);
-	vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base - PAGE_SIZE;
-	stolen_size = vram_stolen_size;
-
-	dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
-			dev_priv->stolen_base, vram_stolen_size / 1024);
-
-	if ((gtt_pages != pg->gtt_pages) && (stolen_size != pg->stolen_size)) {
+	if (gtt_pages != pg->gtt_pages) {
 		dev_err(dev->dev, "GTT resume error.\n");
 		ret = -EINVAL;
 		goto err_gtt_disable;
 	}
 
 	pg->gtt_pages = gtt_pages;
-	pg->stolen_size = stolen_size;
-	dev_priv->vram_stolen_size = vram_stolen_size;
 
 	psb_gtt_clear(dev_priv);
-	psb_gtt_populate_stolen(dev_priv);
-
-	return 0;
 
 err_gtt_disable:
 	pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
@@ -416,11 +422,39 @@ static int psb_gtt_resume(struct drm_device *dev)
 	return ret;
 }
 
+static int psb_gem_mm_resume(struct drm_device *dev)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	unsigned long stolen_size, vram_stolen_size;
+	struct psb_gtt *pg;
+
+	pg = &dev_priv->gtt;
+
+	pci_read_config_dword(pdev, PSB_BSM, &dev_priv->stolen_base);
+	vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base - PAGE_SIZE;
+
+	stolen_size = vram_stolen_size;
+
+	dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n", dev_priv->stolen_base,
+		vram_stolen_size / 1024);
+
+	if (stolen_size != pg->stolen_size) {
+		dev_err(dev->dev, "GTT resume error.\n");
+		return -EINVAL;
+	}
+
+	psb_gtt_populate_stolen(dev_priv);
+
+	return 0;
+}
+
 int psb_gtt_restore(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 
 	psb_gtt_resume(dev);
+	psb_gem_mm_resume(dev);
 
 	psb_gtt_populate_resources(dev_priv);
 
diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h
index 5839d5d06adc..45e1926dfce1 100644
--- a/drivers/gpu/drm/gma500/gtt.h
+++ b/drivers/gpu/drm/gma500/gtt.h
@@ -37,4 +37,7 @@ void psb_gtt_insert_pages(struct drm_psb_private *pdev, const struct resource *r
 			  struct page **pages);
 void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *res);
 
+int psb_gem_mm_init(struct drm_device *dev);
+void psb_gem_mm_fini(struct drm_device *dev);
+
 #endif
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 41be6e1ac7f9..6547967b51e1 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -192,6 +192,7 @@ static void psb_driver_unload(struct drm_device *dev)
 		psb_mmu_driver_takedown(dev_priv->mmu);
 		dev_priv->mmu = NULL;
 	}
+	psb_gem_mm_fini(dev);
 	psb_gtt_fini(dev);
 	if (dev_priv->scratch_page) {
 		set_pages_wb(dev_priv->scratch_page, 1);
@@ -325,6 +326,9 @@ static int psb_driver_load(struct drm_device *dev, unsigned long flags)
 	set_pages_uc(dev_priv->scratch_page, 1);
 
 	ret = psb_gtt_init(dev);
+	if (ret)
+		goto out_err;
+	ret = psb_gem_mm_init(dev);
 	if (ret)
 		goto out_err;
 
-- 
2.35.1

[PATCH v2 09/12] drm/gma500: Inline psb_gtt_restore()

[Thomas Zimmermann]

Inline psb_gtt_restore() into its only caller in power.c.

Perform the GTT restoration in psb_gem_mm_resume(). The restoration
step is part of GEM anyway and will be moved over at some point.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gtt.c   | 15 ++-------------
 drivers/gpu/drm/gma500/gtt.h   |  3 ++-
 drivers/gpu/drm/gma500/power.c |  4 +++-
 3 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 0e99774c7e59..9e1b19fcea28 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -339,7 +339,7 @@ int psb_gem_mm_init(struct drm_device *dev)
 	return ret;
 }
 
-static int psb_gtt_resume(struct drm_device *dev)
+int psb_gtt_resume(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
@@ -422,7 +422,7 @@ static int psb_gtt_resume(struct drm_device *dev)
 	return ret;
 }
 
-static int psb_gem_mm_resume(struct drm_device *dev)
+int psb_gem_mm_resume(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
@@ -445,17 +445,6 @@ static int psb_gem_mm_resume(struct drm_device *dev)
 	}
 
 	psb_gtt_populate_stolen(dev_priv);
-
-	return 0;
-}
-
-int psb_gtt_restore(struct drm_device *dev)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-
-	psb_gtt_resume(dev);
-	psb_gem_mm_resume(dev);
-
 	psb_gtt_populate_resources(dev_priv);
 
 	return 0;
diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h
index 45e1926dfce1..9a6d79200dce 100644
--- a/drivers/gpu/drm/gma500/gtt.h
+++ b/drivers/gpu/drm/gma500/gtt.h
@@ -27,7 +27,7 @@ struct psb_gtt {
 /* Exported functions */
 int psb_gtt_init(struct drm_device *dev);
 void psb_gtt_fini(struct drm_device *dev);
-extern int psb_gtt_restore(struct drm_device *dev);
+int psb_gtt_resume(struct drm_device *dev);
 
 int psb_gtt_allocate_resource(struct drm_psb_private *pdev, struct resource *res,
 			      const char *name, resource_size_t size, resource_size_t align,
@@ -39,5 +39,6 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *r
 
 int psb_gem_mm_init(struct drm_device *dev);
 void psb_gem_mm_fini(struct drm_device *dev);
+int psb_gem_mm_resume(struct drm_device *dev);
 
 #endif
diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c
index d2a46d96e746..28e472b31698 100644
--- a/drivers/gpu/drm/gma500/power.c
+++ b/drivers/gpu/drm/gma500/power.c
@@ -112,7 +112,9 @@ static void gma_resume_display(struct pci_dev *pdev)
 	pci_write_config_word(pdev, PSB_GMCH_CTRL,
 			dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
 
-	psb_gtt_restore(dev); /* Rebuild our GTT mappings */
+	/* Rebuild our GTT mappings */
+	psb_gtt_resume(dev);
+	psb_gem_mm_resume(dev);
 	dev_priv->ops->restore_regs(dev);
 }
 
-- 
2.35.1

[PATCH v2 10/12] drm/gma500: Move GEM memory management functions to gem.c

[Thomas Zimmermann]

Move GEM functions from gtt.c to gem.c. Adapt some names. No
functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/gma500/gem.c     | 133 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/gma500/gem.h     |  12 +++
 drivers/gpu/drm/gma500/gtt.c     | 127 +----------------------------
 drivers/gpu/drm/gma500/gtt.h     |   5 +-
 drivers/gpu/drm/gma500/power.c   |   1 +
 drivers/gpu/drm/gma500/psb_drv.c |   1 +
 6 files changed, 149 insertions(+), 130 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c
index 2f44535f58e7..dffe37490206 100644
--- a/drivers/gpu/drm/gma500/gem.c
+++ b/drivers/gpu/drm/gma500/gem.c
@@ -21,6 +21,10 @@
 #include "gem.h"
 #include "psb_drv.h"
 
+/*
+ * PSB GEM object
+ */
+
 int psb_gem_pin(struct psb_gem_object *pobj)
 {
 	struct drm_gem_object *obj = &pobj->base;
@@ -296,3 +300,132 @@ static vm_fault_t psb_gem_fault(struct vm_fault *vmf)
 
 	return ret;
 }
+
+/*
+ * Memory management
+ */
+
+/* Insert vram stolen pages into the GTT. */
+static void psb_gem_mm_populate_stolen(struct drm_psb_private *pdev)
+{
+	struct drm_device *dev = &pdev->dev;
+	unsigned int pfn_base;
+	unsigned int i, num_pages;
+	uint32_t pte;
+
+	pfn_base = pdev->stolen_base >> PAGE_SHIFT;
+	num_pages = pdev->vram_stolen_size >> PAGE_SHIFT;
+
+	drm_dbg(dev, "Set up %u stolen pages starting at 0x%08x, GTT offset %dK\n",
+		num_pages, pfn_base << PAGE_SHIFT, 0);
+
+	for (i = 0; i < num_pages; ++i) {
+		pte = psb_gtt_mask_pte(pfn_base + i, PSB_MMU_CACHED_MEMORY);
+		iowrite32(pte, pdev->gtt_map + i);
+	}
+
+	(void)ioread32(pdev->gtt_map + i - 1);
+}
+
+int psb_gem_mm_init(struct drm_device *dev)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	unsigned long stolen_size, vram_stolen_size;
+	struct psb_gtt *pg;
+	int ret;
+
+	mutex_init(&dev_priv->mmap_mutex);
+
+	pg = &dev_priv->gtt;
+
+	pci_read_config_dword(pdev, PSB_BSM, &dev_priv->stolen_base);
+	vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base - PAGE_SIZE;
+
+	stolen_size = vram_stolen_size;
+
+	dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
+		dev_priv->stolen_base, vram_stolen_size / 1024);
+
+	pg->stolen_size = stolen_size;
+	dev_priv->vram_stolen_size = vram_stolen_size;
+
+	dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
+	if (!dev_priv->vram_addr) {
+		dev_err(dev->dev, "Failure to map stolen base.\n");
+		ret = -ENOMEM;
+		goto err_mutex_destroy;
+	}
+
+	psb_gem_mm_populate_stolen(dev_priv);
+
+	return 0;
+
+err_mutex_destroy:
+	mutex_destroy(&dev_priv->mmap_mutex);
+	return ret;
+}
+
+void psb_gem_mm_fini(struct drm_device *dev)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+
+	iounmap(dev_priv->vram_addr);
+
+	mutex_destroy(&dev_priv->mmap_mutex);
+}
+
+/* Re-insert all pinned GEM objects into GTT. */
+static void psb_gem_mm_populate_resources(struct drm_psb_private *pdev)
+{
+	unsigned int restored = 0, total = 0, size = 0;
+	struct resource *r = pdev->gtt_mem->child;
+	struct drm_device *dev = &pdev->dev;
+	struct psb_gem_object *pobj;
+
+	while (r) {
+		/*
+		 * TODO: GTT restoration needs a refactoring, so that we don't have to touch
+		 *       struct psb_gem_object here. The type represents a GEM object and is
+		 *       not related to the GTT itself.
+		 */
+		pobj = container_of(r, struct psb_gem_object, resource);
+		if (pobj->pages) {
+			psb_gtt_insert_pages(pdev, &pobj->resource, pobj->pages);
+			size += resource_size(&pobj->resource);
+			++restored;
+		}
+		r = r->sibling;
+		++total;
+	}
+
+	drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total, (size / 1024));
+}
+
+int psb_gem_mm_resume(struct drm_device *dev)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	unsigned long stolen_size, vram_stolen_size;
+	struct psb_gtt *pg;
+
+	pg = &dev_priv->gtt;
+
+	pci_read_config_dword(pdev, PSB_BSM, &dev_priv->stolen_base);
+	vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base - PAGE_SIZE;
+
+	stolen_size = vram_stolen_size;
+
+	dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n", dev_priv->stolen_base,
+		vram_stolen_size / 1024);
+
+	if (stolen_size != pg->stolen_size) {
+		dev_err(dev->dev, "GTT resume error.\n");
+		return -EINVAL;
+	}
+
+	psb_gem_mm_populate_stolen(dev_priv);
+	psb_gem_mm_populate_resources(dev_priv);
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/gma500/gem.h b/drivers/gpu/drm/gma500/gem.h
index 96ef864b4bf1..27df6ff4ed37 100644
--- a/drivers/gpu/drm/gma500/gem.h
+++ b/drivers/gpu/drm/gma500/gem.h
@@ -14,6 +14,10 @@
 
 struct drm_device;
 
+/*
+ * PSB GEM object
+ */
+
 struct psb_gem_object {
 	struct drm_gem_object base;
 
@@ -36,4 +40,12 @@ psb_gem_create(struct drm_device *dev, u64 size, const char *name, bool stolen,
 int psb_gem_pin(struct psb_gem_object *pobj);
 void psb_gem_unpin(struct psb_gem_object *pobj);
 
+/*
+ * Memory management
+ */
+
+int psb_gem_mm_init(struct drm_device *dev);
+void psb_gem_mm_fini(struct drm_device *dev);
+int psb_gem_mm_resume(struct drm_device *dev);
+
 #endif
diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 9e1b19fcea28..b03feec64f01 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -49,7 +49,7 @@ int psb_gtt_allocate_resource(struct drm_psb_private *pdev, struct resource *res
  *
  *	Set the GTT entry for the appropriate memory type.
  */
-static inline uint32_t psb_gtt_mask_pte(uint32_t pfn, int type)
+uint32_t psb_gtt_mask_pte(uint32_t pfn, int type)
 {
 	uint32_t mask = PSB_PTE_VALID;
 
@@ -125,15 +125,6 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *r
 	mutex_unlock(&pdev->gtt_mutex);
 }
 
-void psb_gem_mm_fini(struct drm_device *dev)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-
-	iounmap(dev_priv->vram_addr);
-
-	mutex_destroy(&dev_priv->mmap_mutex);
-}
-
 void psb_gtt_fini(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
@@ -164,55 +155,6 @@ static void psb_gtt_clear(struct drm_psb_private *pdev)
 	(void)ioread32(pdev->gtt_map + i - 1);
 }
 
-/* Insert vram stolen pages into the GTT. */
-static void psb_gtt_populate_stolen(struct drm_psb_private *pdev)
-{
-	struct drm_device *dev = &pdev->dev;
-	unsigned int pfn_base;
-	unsigned int i, num_pages;
-	uint32_t pte;
-
-	pfn_base = pdev->stolen_base >> PAGE_SHIFT;
-	num_pages = pdev->vram_stolen_size >> PAGE_SHIFT;
-
-	drm_dbg(dev, "Set up %u stolen pages starting at 0x%08x, GTT offset %dK\n",
-		num_pages, pfn_base << PAGE_SHIFT, 0);
-
-	for (i = 0; i < num_pages; ++i) {
-		pte = psb_gtt_mask_pte(pfn_base + i, PSB_MMU_CACHED_MEMORY);
-		iowrite32(pte, pdev->gtt_map + i);
-	}
-
-	(void)ioread32(pdev->gtt_map + i - 1);
-}
-
-/* Re-insert all pinned GEM objects into GTT. */
-static void psb_gtt_populate_resources(struct drm_psb_private *pdev)
-{
-	unsigned int restored = 0, total = 0, size = 0;
-	struct resource *r = pdev->gtt_mem->child;
-	struct drm_device *dev = &pdev->dev;
-	struct psb_gem_object *pobj;
-
-	while (r) {
-		/*
-		 * TODO: GTT restoration needs a refactoring, so that we don't have to touch
-		 *       struct psb_gem_object here. The type represents a GEM object and is
-		 *       not related to the GTT itself.
-		 */
-		pobj = container_of(r, struct psb_gem_object, resource);
-		if (pobj->pages) {
-			psb_gtt_insert_pages(pdev, &pobj->resource, pobj->pages);
-			size += resource_size(&pobj->resource);
-			++restored;
-		}
-		r = r->sibling;
-		++total;
-	}
-
-	drm_dbg(dev, "Restored %u of %u gtt ranges (%u KB)", restored, total, (size / 1024));
-}
-
 int psb_gtt_init(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
@@ -300,45 +242,6 @@ int psb_gtt_init(struct drm_device *dev)
 	return ret;
 }
 
-int psb_gem_mm_init(struct drm_device *dev)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct pci_dev *pdev = to_pci_dev(dev->dev);
-	unsigned long stolen_size, vram_stolen_size;
-	struct psb_gtt *pg;
-	int ret;
-
-	mutex_init(&dev_priv->mmap_mutex);
-
-	pg = &dev_priv->gtt;
-
-	pci_read_config_dword(pdev, PSB_BSM, &dev_priv->stolen_base);
-	vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base - PAGE_SIZE;
-
-	stolen_size = vram_stolen_size;
-
-	dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n",
-		dev_priv->stolen_base, vram_stolen_size / 1024);
-
-	pg->stolen_size = stolen_size;
-	dev_priv->vram_stolen_size = vram_stolen_size;
-
-	dev_priv->vram_addr = ioremap_wc(dev_priv->stolen_base, stolen_size);
-	if (!dev_priv->vram_addr) {
-		dev_err(dev->dev, "Failure to map stolen base.\n");
-		ret = -ENOMEM;
-		goto err_mutex_destroy;
-	}
-
-	psb_gtt_populate_stolen(dev_priv);
-
-	return 0;
-
-err_mutex_destroy:
-	mutex_destroy(&dev_priv->mmap_mutex);
-	return ret;
-}
-
 int psb_gtt_resume(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
@@ -421,31 +324,3 @@ int psb_gtt_resume(struct drm_device *dev)
 	(void)PSB_RVDC32(PSB_PGETBL_CTL);
 	return ret;
 }
-
-int psb_gem_mm_resume(struct drm_device *dev)
-{
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct pci_dev *pdev = to_pci_dev(dev->dev);
-	unsigned long stolen_size, vram_stolen_size;
-	struct psb_gtt *pg;
-
-	pg = &dev_priv->gtt;
-
-	pci_read_config_dword(pdev, PSB_BSM, &dev_priv->stolen_base);
-	vram_stolen_size = pg->gtt_phys_start - dev_priv->stolen_base - PAGE_SIZE;
-
-	stolen_size = vram_stolen_size;
-
-	dev_dbg(dev->dev, "Stolen memory base 0x%x, size %luK\n", dev_priv->stolen_base,
-		vram_stolen_size / 1024);
-
-	if (stolen_size != pg->stolen_size) {
-		dev_err(dev->dev, "GTT resume error.\n");
-		return -EINVAL;
-	}
-
-	psb_gtt_populate_stolen(dev_priv);
-	psb_gtt_populate_resources(dev_priv);
-
-	return 0;
-}
diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h
index 9a6d79200dce..d5a5fd4466c1 100644
--- a/drivers/gpu/drm/gma500/gtt.h
+++ b/drivers/gpu/drm/gma500/gtt.h
@@ -33,12 +33,9 @@ int psb_gtt_allocate_resource(struct drm_psb_private *pdev, struct resource *res
 			      const char *name, resource_size_t size, resource_size_t align,
 			      bool stolen, u32 *offset);
 
+uint32_t psb_gtt_mask_pte(uint32_t pfn, int type);
 void psb_gtt_insert_pages(struct drm_psb_private *pdev, const struct resource *res,
 			  struct page **pages);
 void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *res);
 
-int psb_gem_mm_init(struct drm_device *dev);
-void psb_gem_mm_fini(struct drm_device *dev);
-int psb_gem_mm_resume(struct drm_device *dev);
-
 #endif
diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c
index 28e472b31698..6f917cfef65b 100644
--- a/drivers/gpu/drm/gma500/power.c
+++ b/drivers/gpu/drm/gma500/power.c
@@ -28,6 +28,7 @@
  *    Alan Cox <alan@linux.intel.com>
  */
 
+#include "gem.h"
 #include "power.h"
 #include "psb_drv.h"
 #include "psb_reg.h"
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/gma500/psb_drv.c
index 6547967b51e1..e30b58184156 100644
--- a/drivers/gpu/drm/gma500/psb_drv.c
+++ b/drivers/gpu/drm/gma500/psb_drv.c
@@ -28,6 +28,7 @@
 #include <drm/drm_vblank.h>
 
 #include "framebuffer.h"
+#include "gem.h"
 #include "intel_bios.h"
 #include "mid_bios.h"
 #include "power.h"
-- 
2.35.1

[PATCH v2 11/12] drm/gma500: Move GTT enable and disable code into helpers

[Thomas Zimmermann]

Move the code for enabling and disabling the GTT into helpers and call
the functions in psb_gtt_init(), psb_gtt_fini() and psb_gtt_resume().
Removes code duplication.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/gma500/gtt.c | 81 ++++++++++++++++++++----------------
 1 file changed, 46 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index b03feec64f01..83d9a9f7c73c 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -125,17 +125,44 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *r
 	mutex_unlock(&pdev->gtt_mutex);
 }
 
-void psb_gtt_fini(struct drm_device *dev)
+static int psb_gtt_enable(struct drm_psb_private *dev_priv)
 {
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct drm_device *dev = &dev_priv->dev;
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	int ret;
 
-	iounmap(dev_priv->gtt_map);
+	ret = pci_read_config_word(pdev, PSB_GMCH_CTRL, &dev_priv->gmch_ctrl);
+	if (ret)
+		return pcibios_err_to_errno(ret);
+	ret = pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
+	if (ret)
+		return pcibios_err_to_errno(ret);
+
+	dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
+	PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
+
+	(void)PSB_RVDC32(PSB_PGETBL_CTL);
+
+	return 0;
+}
+
+static void psb_gtt_disable(struct drm_psb_private *dev_priv)
+{
+	struct drm_device *dev = &dev_priv->dev;
+	struct pci_dev *pdev = to_pci_dev(dev->dev);
 
 	pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
 	PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
+
 	(void)PSB_RVDC32(PSB_PGETBL_CTL);
+}
 
+void psb_gtt_fini(struct drm_device *dev)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+
+	iounmap(dev_priv->gtt_map);
+	psb_gtt_disable(dev_priv);
 	mutex_destroy(&dev_priv->gtt_mutex);
 }
 
@@ -159,22 +186,15 @@ int psb_gtt_init(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	struct psb_gtt *pg = &dev_priv->gtt;
 	unsigned gtt_pages;
-	struct psb_gtt *pg;
-	int ret = 0;
+	int ret;
 
 	mutex_init(&dev_priv->gtt_mutex);
 
-	pg = &dev_priv->gtt;
-
-	/* Enable the GTT */
-	pci_read_config_word(pdev, PSB_GMCH_CTRL, &dev_priv->gmch_ctrl);
-	pci_write_config_word(pdev, PSB_GMCH_CTRL,
-			      dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
-
-	dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
-	PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
-	(void) PSB_RVDC32(PSB_PGETBL_CTL);
+	ret = psb_gtt_enable(dev_priv);
+	if (ret)
+		goto err_mutex_destroy;
 
 	/* The root resource we allocate address space from */
 	pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
@@ -227,17 +247,16 @@ int psb_gtt_init(struct drm_device *dev)
 	if (!dev_priv->gtt_map) {
 		dev_err(dev->dev, "Failure to map gtt.\n");
 		ret = -ENOMEM;
-		goto err_gtt_disable;
+		goto err_psb_gtt_disable;
 	}
 
 	psb_gtt_clear(dev_priv);
 
 	return 0;
 
-err_gtt_disable:
-	pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
-	PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
-	(void)PSB_RVDC32(PSB_PGETBL_CTL);
+err_psb_gtt_disable:
+	psb_gtt_disable(dev_priv);
+err_mutex_destroy:
 	mutex_destroy(&dev_priv->gtt_mutex);
 	return ret;
 }
@@ -246,20 +265,14 @@ int psb_gtt_resume(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
+	struct psb_gtt *pg = &dev_priv->gtt;
 	unsigned int gtt_pages;
-	struct psb_gtt *pg;
 	int ret;
 
-	pg = &dev_priv->gtt;
-
 	/* Enable the GTT */
-	pci_read_config_word(pdev, PSB_GMCH_CTRL, &dev_priv->gmch_ctrl);
-	pci_write_config_word(pdev, PSB_GMCH_CTRL,
-			      dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
-
-	dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
-	PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
-	(void) PSB_RVDC32(PSB_PGETBL_CTL);
+	ret = psb_gtt_enable(dev_priv);
+	if (ret)
+		return ret;
 
 	/* The root resource we allocate address space from */
 	pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
@@ -311,16 +324,14 @@ int psb_gtt_resume(struct drm_device *dev)
 	if (gtt_pages != pg->gtt_pages) {
 		dev_err(dev->dev, "GTT resume error.\n");
 		ret = -EINVAL;
-		goto err_gtt_disable;
+		goto err_psb_gtt_disable;
 	}
 
 	pg->gtt_pages = gtt_pages;
 
 	psb_gtt_clear(dev_priv);
 
-err_gtt_disable:
-	pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
-	PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
-	(void)PSB_RVDC32(PSB_PGETBL_CTL);
+err_psb_gtt_disable:
+	psb_gtt_disable(dev_priv);
 	return ret;
 }
-- 
2.35.1

[PATCH v2 12/12] drm/gma500: Move GTT memory-range setup into helper

[Thomas Zimmermann]

Move the setup code for GTT/GATT memory ranges into a new helper and
call the function from psb_gtt_init() and psb_gtt_resume(). Removes
code duplication.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/gma500/gtt.c | 153 +++++++++++++++--------------------
 1 file changed, 64 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
index 83d9a9f7c73c..379bc218aa6b 100644
--- a/drivers/gpu/drm/gma500/gtt.c
+++ b/drivers/gpu/drm/gma500/gtt.c
@@ -182,68 +182,91 @@ static void psb_gtt_clear(struct drm_psb_private *pdev)
 	(void)ioread32(pdev->gtt_map + i - 1);
 }
 
-int psb_gtt_init(struct drm_device *dev)
+static void psb_gtt_init_ranges(struct drm_psb_private *dev_priv)
 {
-	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct drm_device *dev = &dev_priv->dev;
 	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	struct psb_gtt *pg = &dev_priv->gtt;
-	unsigned gtt_pages;
-	int ret;
-
-	mutex_init(&dev_priv->gtt_mutex);
-
-	ret = psb_gtt_enable(dev_priv);
-	if (ret)
-		goto err_mutex_destroy;
+	resource_size_t gtt_phys_start, mmu_gatt_start, gtt_start, gtt_pages,
+			gatt_start, gatt_pages;
+	struct resource *gtt_mem;
 
 	/* The root resource we allocate address space from */
-	pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
+	gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
 
 	/*
-	 *	The video mmu has a hw bug when accessing 0x0D0000000.
-	 *	Make gatt start at 0x0e000,0000. This doesn't actually
-	 *	matter for us but may do if the video acceleration ever
-	 *	gets opened up.
+	 * The video MMU has a HW bug when accessing 0x0d0000000. Make
+	 * GATT start at 0x0e0000000. This doesn't actually matter for
+	 * us now, but maybe will if the video acceleration ever gets
+	 * opened up.
 	 */
-	pg->mmu_gatt_start = 0xE0000000;
+	mmu_gatt_start = 0xe0000000;
+
+	gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
+	gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE) >> PAGE_SHIFT;
 
-	pg->gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
-	gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE)
-								>> PAGE_SHIFT;
 	/* CDV doesn't report this. In which case the system has 64 gtt pages */
-	if (pg->gtt_start == 0 || gtt_pages == 0) {
+	if (!gtt_start || !gtt_pages) {
 		dev_dbg(dev->dev, "GTT PCI BAR not initialized.\n");
 		gtt_pages = 64;
-		pg->gtt_start = dev_priv->pge_ctl;
+		gtt_start = dev_priv->pge_ctl;
 	}
 
-	pg->gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
-	pg->gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE)
-								>> PAGE_SHIFT;
-	dev_priv->gtt_mem = &pdev->resource[PSB_GATT_RESOURCE];
+	gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
+	gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE) >> PAGE_SHIFT;
 
-	if (pg->gatt_pages == 0 || pg->gatt_start == 0) {
+	if (!gatt_pages || !gatt_start) {
 		static struct resource fudge;	/* Preferably peppermint */
-		/* This can occur on CDV systems. Fudge it in this case.
-		   We really don't care what imaginary space is being allocated
-		   at this point */
+
+		/*
+		 * This can occur on CDV systems. Fudge it in this case. We
+		 * really don't care what imaginary space is being allocated
+		 * at this point.
+		 */
 		dev_dbg(dev->dev, "GATT PCI BAR not initialized.\n");
-		pg->gatt_start = 0x40000000;
-		pg->gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
-		/* This is a little confusing but in fact the GTT is providing
-		   a view from the GPU into memory and not vice versa. As such
-		   this is really allocating space that is not the same as the
-		   CPU address space on CDV */
+		gatt_start = 0x40000000;
+		gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
+
+		/*
+		 * This is a little confusing but in fact the GTT is providing
+		 * a view from the GPU into memory and not vice versa. As such
+		 * this is really allocating space that is not the same as the
+		 * CPU address space on CDV.
+		 */
 		fudge.start = 0x40000000;
 		fudge.end = 0x40000000 + 128 * 1024 * 1024 - 1;
 		fudge.name = "fudge";
 		fudge.flags = IORESOURCE_MEM;
-		dev_priv->gtt_mem = &fudge;
+
+		gtt_mem = &fudge;
+	} else {
+		gtt_mem = &pdev->resource[PSB_GATT_RESOURCE];
 	}
 
+	pg->gtt_phys_start = gtt_phys_start;
+	pg->mmu_gatt_start = mmu_gatt_start;
+	pg->gtt_start = gtt_start;
 	pg->gtt_pages = gtt_pages;
+	pg->gatt_start = gatt_start;
+	pg->gatt_pages = gatt_pages;
+	dev_priv->gtt_mem = gtt_mem;
+}
+
+int psb_gtt_init(struct drm_device *dev)
+{
+	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
+	struct psb_gtt *pg = &dev_priv->gtt;
+	int ret;
+
+	mutex_init(&dev_priv->gtt_mutex);
+
+	ret = psb_gtt_enable(dev_priv);
+	if (ret)
+		goto err_mutex_destroy;
 
-	dev_priv->gtt_map = ioremap(pg->gtt_phys_start, gtt_pages << PAGE_SHIFT);
+	psb_gtt_init_ranges(dev_priv);
+
+	dev_priv->gtt_map = ioremap(pg->gtt_phys_start, pg->gtt_pages << PAGE_SHIFT);
 	if (!dev_priv->gtt_map) {
 		dev_err(dev->dev, "Failure to map gtt.\n");
 		ret = -ENOMEM;
@@ -264,9 +287,8 @@ int psb_gtt_init(struct drm_device *dev)
 int psb_gtt_resume(struct drm_device *dev)
 {
 	struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
-	struct pci_dev *pdev = to_pci_dev(dev->dev);
 	struct psb_gtt *pg = &dev_priv->gtt;
-	unsigned int gtt_pages;
+	unsigned int old_gtt_pages = pg->gtt_pages;
 	int ret;
 
 	/* Enable the GTT */
@@ -274,61 +296,14 @@ int psb_gtt_resume(struct drm_device *dev)
 	if (ret)
 		return ret;
 
-	/* The root resource we allocate address space from */
-	pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
-
-	/*
-	 *	The video mmu has a hw bug when accessing 0x0D0000000.
-	 *	Make gatt start at 0x0e000,0000. This doesn't actually
-	 *	matter for us but may do if the video acceleration ever
-	 *	gets opened up.
-	 */
-	pg->mmu_gatt_start = 0xE0000000;
-
-	pg->gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
-	gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE) >> PAGE_SHIFT;
-	/* CDV doesn't report this. In which case the system has 64 gtt pages */
-	if (pg->gtt_start == 0 || gtt_pages == 0) {
-		dev_dbg(dev->dev, "GTT PCI BAR not initialized.\n");
-		gtt_pages = 64;
-		pg->gtt_start = dev_priv->pge_ctl;
-	}
-
-	pg->gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
-	pg->gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE) >> PAGE_SHIFT;
-	dev_priv->gtt_mem = &pdev->resource[PSB_GATT_RESOURCE];
+	psb_gtt_init_ranges(dev_priv);
 
-	if (pg->gatt_pages == 0 || pg->gatt_start == 0) {
-		static struct resource fudge;	/* Preferably peppermint */
-		/*
-		 * This can occur on CDV systems. Fudge it in this case. We
-		 * really don't care what imaginary space is being allocated
-		 * at this point.
-		 */
-		dev_dbg(dev->dev, "GATT PCI BAR not initialized.\n");
-		pg->gatt_start = 0x40000000;
-		pg->gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
-		/*
-		 * This is a little confusing but in fact the GTT is providing
-		 * a view from the GPU into memory and not vice versa. As such
-		 *  this is really allocating space that is not the same as the
-		 *  CPU address space on CDV.
-		 */
-		fudge.start = 0x40000000;
-		fudge.end = 0x40000000 + 128 * 1024 * 1024 - 1;
-		fudge.name = "fudge";
-		fudge.flags = IORESOURCE_MEM;
-		dev_priv->gtt_mem = &fudge;
-	}
-
-	if (gtt_pages != pg->gtt_pages) {
+	if (old_gtt_pages != pg->gtt_pages) {
 		dev_err(dev->dev, "GTT resume error.\n");
-		ret = -EINVAL;
+		ret = -ENODEV;
 		goto err_psb_gtt_disable;
 	}
 
-	pg->gtt_pages = gtt_pages;
-
 	psb_gtt_clear(dev_priv);
 
 err_psb_gtt_disable:
-- 
2.35.1

Re: [PATCH v2 12/12] drm/gma500: Move GTT memory-range setup into helper

[Patrik Jakobsson]

On Tue, Mar 8, 2022 at 8:52 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Move the setup code for GTT/GATT memory ranges into a new helper and
> call the function from psb_gtt_init() and psb_gtt_resume(). Removes
> code duplication.

LGTM
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/gma500/gtt.c | 153 +++++++++++++++--------------------
>  1 file changed, 64 insertions(+), 89 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
> index 83d9a9f7c73c..379bc218aa6b 100644
> --- a/drivers/gpu/drm/gma500/gtt.c
> +++ b/drivers/gpu/drm/gma500/gtt.c
> @@ -182,68 +182,91 @@ static void psb_gtt_clear(struct drm_psb_private *pdev)
>         (void)ioread32(pdev->gtt_map + i - 1);
>  }
>
> -int psb_gtt_init(struct drm_device *dev)
> +static void psb_gtt_init_ranges(struct drm_psb_private *dev_priv)
>  {
> -       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +       struct drm_device *dev = &dev_priv->dev;
>         struct pci_dev *pdev = to_pci_dev(dev->dev);
>         struct psb_gtt *pg = &dev_priv->gtt;
> -       unsigned gtt_pages;
> -       int ret;
> -
> -       mutex_init(&dev_priv->gtt_mutex);
> -
> -       ret = psb_gtt_enable(dev_priv);
> -       if (ret)
> -               goto err_mutex_destroy;
> +       resource_size_t gtt_phys_start, mmu_gatt_start, gtt_start, gtt_pages,
> +                       gatt_start, gatt_pages;
> +       struct resource *gtt_mem;
>
>         /* The root resource we allocate address space from */
> -       pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
> +       gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
>
>         /*
> -        *      The video mmu has a hw bug when accessing 0x0D0000000.
> -        *      Make gatt start at 0x0e000,0000. This doesn't actually
> -        *      matter for us but may do if the video acceleration ever
> -        *      gets opened up.
> +        * The video MMU has a HW bug when accessing 0x0d0000000. Make
> +        * GATT start at 0x0e0000000. This doesn't actually matter for
> +        * us now, but maybe will if the video acceleration ever gets
> +        * opened up.
>          */
> -       pg->mmu_gatt_start = 0xE0000000;
> +       mmu_gatt_start = 0xe0000000;
> +
> +       gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
> +       gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE) >> PAGE_SHIFT;
>
> -       pg->gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
> -       gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE)
> -                                                               >> PAGE_SHIFT;
>         /* CDV doesn't report this. In which case the system has 64 gtt pages */
> -       if (pg->gtt_start == 0 || gtt_pages == 0) {
> +       if (!gtt_start || !gtt_pages) {
>                 dev_dbg(dev->dev, "GTT PCI BAR not initialized.\n");
>                 gtt_pages = 64;
> -               pg->gtt_start = dev_priv->pge_ctl;
> +               gtt_start = dev_priv->pge_ctl;
>         }
>
> -       pg->gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
> -       pg->gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE)
> -                                                               >> PAGE_SHIFT;
> -       dev_priv->gtt_mem = &pdev->resource[PSB_GATT_RESOURCE];
> +       gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
> +       gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE) >> PAGE_SHIFT;
>
> -       if (pg->gatt_pages == 0 || pg->gatt_start == 0) {
> +       if (!gatt_pages || !gatt_start) {
>                 static struct resource fudge;   /* Preferably peppermint */
> -               /* This can occur on CDV systems. Fudge it in this case.
> -                  We really don't care what imaginary space is being allocated
> -                  at this point */
> +
> +               /*
> +                * This can occur on CDV systems. Fudge it in this case. We
> +                * really don't care what imaginary space is being allocated
> +                * at this point.
> +                */
>                 dev_dbg(dev->dev, "GATT PCI BAR not initialized.\n");
> -               pg->gatt_start = 0x40000000;
> -               pg->gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
> -               /* This is a little confusing but in fact the GTT is providing
> -                  a view from the GPU into memory and not vice versa. As such
> -                  this is really allocating space that is not the same as the
> -                  CPU address space on CDV */
> +               gatt_start = 0x40000000;
> +               gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
> +
> +               /*
> +                * This is a little confusing but in fact the GTT is providing
> +                * a view from the GPU into memory and not vice versa. As such
> +                * this is really allocating space that is not the same as the
> +                * CPU address space on CDV.
> +                */
>                 fudge.start = 0x40000000;
>                 fudge.end = 0x40000000 + 128 * 1024 * 1024 - 1;
>                 fudge.name = "fudge";
>                 fudge.flags = IORESOURCE_MEM;
> -               dev_priv->gtt_mem = &fudge;
> +
> +               gtt_mem = &fudge;
> +       } else {
> +               gtt_mem = &pdev->resource[PSB_GATT_RESOURCE];
>         }
>
> +       pg->gtt_phys_start = gtt_phys_start;
> +       pg->mmu_gatt_start = mmu_gatt_start;
> +       pg->gtt_start = gtt_start;
>         pg->gtt_pages = gtt_pages;
> +       pg->gatt_start = gatt_start;
> +       pg->gatt_pages = gatt_pages;
> +       dev_priv->gtt_mem = gtt_mem;
> +}
> +
> +int psb_gtt_init(struct drm_device *dev)
> +{
> +       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +       struct psb_gtt *pg = &dev_priv->gtt;
> +       int ret;
> +
> +       mutex_init(&dev_priv->gtt_mutex);
> +
> +       ret = psb_gtt_enable(dev_priv);
> +       if (ret)
> +               goto err_mutex_destroy;
>
> -       dev_priv->gtt_map = ioremap(pg->gtt_phys_start, gtt_pages << PAGE_SHIFT);
> +       psb_gtt_init_ranges(dev_priv);
> +
> +       dev_priv->gtt_map = ioremap(pg->gtt_phys_start, pg->gtt_pages << PAGE_SHIFT);
>         if (!dev_priv->gtt_map) {
>                 dev_err(dev->dev, "Failure to map gtt.\n");
>                 ret = -ENOMEM;
> @@ -264,9 +287,8 @@ int psb_gtt_init(struct drm_device *dev)
>  int psb_gtt_resume(struct drm_device *dev)
>  {
>         struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> -       struct pci_dev *pdev = to_pci_dev(dev->dev);
>         struct psb_gtt *pg = &dev_priv->gtt;
> -       unsigned int gtt_pages;
> +       unsigned int old_gtt_pages = pg->gtt_pages;
>         int ret;
>
>         /* Enable the GTT */
> @@ -274,61 +296,14 @@ int psb_gtt_resume(struct drm_device *dev)
>         if (ret)
>                 return ret;
>
> -       /* The root resource we allocate address space from */
> -       pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
> -
> -       /*
> -        *      The video mmu has a hw bug when accessing 0x0D0000000.
> -        *      Make gatt start at 0x0e000,0000. This doesn't actually
> -        *      matter for us but may do if the video acceleration ever
> -        *      gets opened up.
> -        */
> -       pg->mmu_gatt_start = 0xE0000000;
> -
> -       pg->gtt_start = pci_resource_start(pdev, PSB_GTT_RESOURCE);
> -       gtt_pages = pci_resource_len(pdev, PSB_GTT_RESOURCE) >> PAGE_SHIFT;
> -       /* CDV doesn't report this. In which case the system has 64 gtt pages */
> -       if (pg->gtt_start == 0 || gtt_pages == 0) {
> -               dev_dbg(dev->dev, "GTT PCI BAR not initialized.\n");
> -               gtt_pages = 64;
> -               pg->gtt_start = dev_priv->pge_ctl;
> -       }
> -
> -       pg->gatt_start = pci_resource_start(pdev, PSB_GATT_RESOURCE);
> -       pg->gatt_pages = pci_resource_len(pdev, PSB_GATT_RESOURCE) >> PAGE_SHIFT;
> -       dev_priv->gtt_mem = &pdev->resource[PSB_GATT_RESOURCE];
> +       psb_gtt_init_ranges(dev_priv);
>
> -       if (pg->gatt_pages == 0 || pg->gatt_start == 0) {
> -               static struct resource fudge;   /* Preferably peppermint */
> -               /*
> -                * This can occur on CDV systems. Fudge it in this case. We
> -                * really don't care what imaginary space is being allocated
> -                * at this point.
> -                */
> -               dev_dbg(dev->dev, "GATT PCI BAR not initialized.\n");
> -               pg->gatt_start = 0x40000000;
> -               pg->gatt_pages = (128 * 1024 * 1024) >> PAGE_SHIFT;
> -               /*
> -                * This is a little confusing but in fact the GTT is providing
> -                * a view from the GPU into memory and not vice versa. As such
> -                *  this is really allocating space that is not the same as the
> -                *  CPU address space on CDV.
> -                */
> -               fudge.start = 0x40000000;
> -               fudge.end = 0x40000000 + 128 * 1024 * 1024 - 1;
> -               fudge.name = "fudge";
> -               fudge.flags = IORESOURCE_MEM;
> -               dev_priv->gtt_mem = &fudge;
> -       }
> -
> -       if (gtt_pages != pg->gtt_pages) {
> +       if (old_gtt_pages != pg->gtt_pages) {
>                 dev_err(dev->dev, "GTT resume error.\n");
> -               ret = -EINVAL;
> +               ret = -ENODEV;
>                 goto err_psb_gtt_disable;
>         }
>
> -       pg->gtt_pages = gtt_pages;
> -
>         psb_gtt_clear(dev_priv);
>
>  err_psb_gtt_disable:
> --
> 2.35.1
>

Re: [PATCH v2 11/12] drm/gma500: Move GTT enable and disable code into helpers

[Patrik Jakobsson]

On Tue, Mar 8, 2022 at 8:52 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Move the code for enabling and disabling the GTT into helpers and call
> the functions in psb_gtt_init(), psb_gtt_fini() and psb_gtt_resume().
> Removes code duplication.

That makes it much more readable. Thanks.

Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>


>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/gma500/gtt.c | 81 ++++++++++++++++++++----------------
>  1 file changed, 46 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c
> index b03feec64f01..83d9a9f7c73c 100644
> --- a/drivers/gpu/drm/gma500/gtt.c
> +++ b/drivers/gpu/drm/gma500/gtt.c
> @@ -125,17 +125,44 @@ void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *r
>         mutex_unlock(&pdev->gtt_mutex);
>  }
>
> -void psb_gtt_fini(struct drm_device *dev)
> +static int psb_gtt_enable(struct drm_psb_private *dev_priv)
>  {
> -       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +       struct drm_device *dev = &dev_priv->dev;
>         struct pci_dev *pdev = to_pci_dev(dev->dev);
> +       int ret;
>
> -       iounmap(dev_priv->gtt_map);
> +       ret = pci_read_config_word(pdev, PSB_GMCH_CTRL, &dev_priv->gmch_ctrl);
> +       if (ret)
> +               return pcibios_err_to_errno(ret);
> +       ret = pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
> +       if (ret)
> +               return pcibios_err_to_errno(ret);
> +
> +       dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
> +       PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
> +
> +       (void)PSB_RVDC32(PSB_PGETBL_CTL);
> +
> +       return 0;
> +}
> +
> +static void psb_gtt_disable(struct drm_psb_private *dev_priv)
> +{
> +       struct drm_device *dev = &dev_priv->dev;
> +       struct pci_dev *pdev = to_pci_dev(dev->dev);
>
>         pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
>         PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
> +
>         (void)PSB_RVDC32(PSB_PGETBL_CTL);
> +}
>
> +void psb_gtt_fini(struct drm_device *dev)
> +{
> +       struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
> +
> +       iounmap(dev_priv->gtt_map);
> +       psb_gtt_disable(dev_priv);
>         mutex_destroy(&dev_priv->gtt_mutex);
>  }
>
> @@ -159,22 +186,15 @@ int psb_gtt_init(struct drm_device *dev)
>  {
>         struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>         struct pci_dev *pdev = to_pci_dev(dev->dev);
> +       struct psb_gtt *pg = &dev_priv->gtt;
>         unsigned gtt_pages;
> -       struct psb_gtt *pg;
> -       int ret = 0;
> +       int ret;
>
>         mutex_init(&dev_priv->gtt_mutex);
>
> -       pg = &dev_priv->gtt;
> -
> -       /* Enable the GTT */
> -       pci_read_config_word(pdev, PSB_GMCH_CTRL, &dev_priv->gmch_ctrl);
> -       pci_write_config_word(pdev, PSB_GMCH_CTRL,
> -                             dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
> -
> -       dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
> -       PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
> -       (void) PSB_RVDC32(PSB_PGETBL_CTL);
> +       ret = psb_gtt_enable(dev_priv);
> +       if (ret)
> +               goto err_mutex_destroy;
>
>         /* The root resource we allocate address space from */
>         pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
> @@ -227,17 +247,16 @@ int psb_gtt_init(struct drm_device *dev)
>         if (!dev_priv->gtt_map) {
>                 dev_err(dev->dev, "Failure to map gtt.\n");
>                 ret = -ENOMEM;
> -               goto err_gtt_disable;
> +               goto err_psb_gtt_disable;
>         }
>
>         psb_gtt_clear(dev_priv);
>
>         return 0;
>
> -err_gtt_disable:
> -       pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
> -       PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
> -       (void)PSB_RVDC32(PSB_PGETBL_CTL);
> +err_psb_gtt_disable:
> +       psb_gtt_disable(dev_priv);
> +err_mutex_destroy:
>         mutex_destroy(&dev_priv->gtt_mutex);
>         return ret;
>  }
> @@ -246,20 +265,14 @@ int psb_gtt_resume(struct drm_device *dev)
>  {
>         struct drm_psb_private *dev_priv = to_drm_psb_private(dev);
>         struct pci_dev *pdev = to_pci_dev(dev->dev);
> +       struct psb_gtt *pg = &dev_priv->gtt;
>         unsigned int gtt_pages;
> -       struct psb_gtt *pg;
>         int ret;
>
> -       pg = &dev_priv->gtt;
> -
>         /* Enable the GTT */
> -       pci_read_config_word(pdev, PSB_GMCH_CTRL, &dev_priv->gmch_ctrl);
> -       pci_write_config_word(pdev, PSB_GMCH_CTRL,
> -                             dev_priv->gmch_ctrl | _PSB_GMCH_ENABLED);
> -
> -       dev_priv->pge_ctl = PSB_RVDC32(PSB_PGETBL_CTL);
> -       PSB_WVDC32(dev_priv->pge_ctl | _PSB_PGETBL_ENABLED, PSB_PGETBL_CTL);
> -       (void) PSB_RVDC32(PSB_PGETBL_CTL);
> +       ret = psb_gtt_enable(dev_priv);
> +       if (ret)
> +               return ret;
>
>         /* The root resource we allocate address space from */
>         pg->gtt_phys_start = dev_priv->pge_ctl & PAGE_MASK;
> @@ -311,16 +324,14 @@ int psb_gtt_resume(struct drm_device *dev)
>         if (gtt_pages != pg->gtt_pages) {
>                 dev_err(dev->dev, "GTT resume error.\n");
>                 ret = -EINVAL;
> -               goto err_gtt_disable;
> +               goto err_psb_gtt_disable;
>         }
>
>         pg->gtt_pages = gtt_pages;
>
>         psb_gtt_clear(dev_priv);
>
> -err_gtt_disable:
> -       pci_write_config_word(pdev, PSB_GMCH_CTRL, dev_priv->gmch_ctrl);
> -       PSB_WVDC32(dev_priv->pge_ctl, PSB_PGETBL_CTL);
> -       (void)PSB_RVDC32(PSB_PGETBL_CTL);
> +err_psb_gtt_disable:
> +       psb_gtt_disable(dev_priv);
>         return ret;
>  }
> --
> 2.35.1
>

Re: [PATCH v2 00/12] drm/gma500: Various cleanups to GEM code

[Patrik Jakobsson]

On Tue, Mar 8, 2022 at 8:52 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>
> Refactor and simplify various parts of the memory management. This
> includes locking, initialization and finalizer functions, and code
> organization.
>
> Tested on Atom N2800 hardware.
>
> v2:
>         * put common code in psb_gtt_{init,fini,resume}() into
>           helpers (Sam, Patrik)
>

The series is pushed to drm-misc-next

Thanks
Patrik

> Thomas Zimmermann (12):
>   drm/gma500: Remove struct psb_gem_object.npage
>   drm/gma500: Acquire reservation lock for GEM objects
>   drm/gma500: Move GTT locking into GTT helpers
>   drm/gma500: Remove struct psb_gtt.sem sempahore
>   drm/gma500: Move GTT setup and restoration into helper funtions
>   drm/gma500: Move GTT resume logic out of psb_gtt_init()
>   drm/gma500: Cleanup GTT uninit and error handling
>   drm/gma500: Split GTT init/resume/fini into GTT and GEM functions
>   drm/gma500: Inline psb_gtt_restore()
>   drm/gma500: Move GEM memory management functions to gem.c
>   drm/gma500: Move GTT enable and disable code into helpers
>   drm/gma500: Move GTT memory-range setup into helper
>
>  drivers/gpu/drm/gma500/gem.c         | 161 ++++++++++++++-
>  drivers/gpu/drm/gma500/gem.h         |  13 +-
>  drivers/gpu/drm/gma500/gma_display.c |   8 +-
>  drivers/gpu/drm/gma500/gtt.c         | 295 +++++++++++++--------------
>  drivers/gpu/drm/gma500/gtt.h         |   8 +-
>  drivers/gpu/drm/gma500/power.c       |   5 +-
>  drivers/gpu/drm/gma500/psb_drv.c     |  13 +-
>  drivers/gpu/drm/gma500/psb_drv.h     |   1 -
>  8 files changed, 317 insertions(+), 187 deletions(-)
>
>
> base-commit: 710a019ad85e96e66f7d75ee7f4733cdd8a2b0d0
> prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
> prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
> prerequisite-patch-id: 6e1032c6302461624f33194c8b8f37103a3fa6ef
> --
> 2.35.1
>

Re: [PATCH v2 00/12] drm/gma500: Various cleanups to GEM code

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 2327 bytes --]

Hi

Am 16.03.22 um 17:45 schrieb Patrik Jakobsson:
> On Tue, Mar 8, 2022 at 8:52 PM Thomas Zimmermann <tzimmermann@suse.de> wrote:
>>
>> Refactor and simplify various parts of the memory management. This
>> includes locking, initialization and finalizer functions, and code
>> organization.
>>
>> Tested on Atom N2800 hardware.
>>
>> v2:
>>          * put common code in psb_gtt_{init,fini,resume}() into
>>            helpers (Sam, Patrik)
>>
> 
> The series is pushed to drm-misc-next

Thanks a lot, Patrik!

Best regards
Thomas

> 
> Thanks
> Patrik
> 
>> Thomas Zimmermann (12):
>>    drm/gma500: Remove struct psb_gem_object.npage
>>    drm/gma500: Acquire reservation lock for GEM objects
>>    drm/gma500: Move GTT locking into GTT helpers
>>    drm/gma500: Remove struct psb_gtt.sem sempahore
>>    drm/gma500: Move GTT setup and restoration into helper funtions
>>    drm/gma500: Move GTT resume logic out of psb_gtt_init()
>>    drm/gma500: Cleanup GTT uninit and error handling
>>    drm/gma500: Split GTT init/resume/fini into GTT and GEM functions
>>    drm/gma500: Inline psb_gtt_restore()
>>    drm/gma500: Move GEM memory management functions to gem.c
>>    drm/gma500: Move GTT enable and disable code into helpers
>>    drm/gma500: Move GTT memory-range setup into helper
>>
>>   drivers/gpu/drm/gma500/gem.c         | 161 ++++++++++++++-
>>   drivers/gpu/drm/gma500/gem.h         |  13 +-
>>   drivers/gpu/drm/gma500/gma_display.c |   8 +-
>>   drivers/gpu/drm/gma500/gtt.c         | 295 +++++++++++++--------------
>>   drivers/gpu/drm/gma500/gtt.h         |   8 +-
>>   drivers/gpu/drm/gma500/power.c       |   5 +-
>>   drivers/gpu/drm/gma500/psb_drv.c     |  13 +-
>>   drivers/gpu/drm/gma500/psb_drv.h     |   1 -
>>   8 files changed, 317 insertions(+), 187 deletions(-)
>>
>>
>> base-commit: 710a019ad85e96e66f7d75ee7f4733cdd8a2b0d0
>> prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d
>> prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24
>> prerequisite-patch-id: 6e1032c6302461624f33194c8b8f37103a3fa6ef
>> --
>> 2.35.1
>>

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

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