From e1f80341e312088f0e6c46107db7098e30e6d764 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Fri, 15 Oct 2021 10:40:52 +0200 Subject: [PATCH] drm/gma500: Rewrite GTT page insert/remove without struct gtt_range struct gtt_range represents a GEM object and should not be used for GTT setup. Change psb_gtt_insert() and psb_gtt_remove() to receive all necessary parameters from their caller. This also eliminates possible failure from psb_gtt_insert(). There's one exception in psb_gtt_restore(), which requires an upcast from struct resource to struct gtt_range when restoring the GTT after hibernation. A possible solution would track the GEM objects that need restoration separately from the GTT resource. Rename the functions to psb_gtt_insert_pages() and psb_gtt_remove_pages() to reflect their similarity to MMU interfaces. v3: * restore the comments about locking rules (Patrik) Signed-off-by: Thomas Zimmermann Acked-by: Patrik Jakobsson Link: https://patchwork.freedesktop.org/patch/msgid/20211015084053.13708-10-tzimmermann@suse.de --- drivers/gpu/drm/gma500/gem.c | 12 ++---- drivers/gpu/drm/gma500/gtt.c | 93 ++++++++++++++++++-------------------------- drivers/gpu/drm/gma500/gtt.h | 5 ++- 3 files changed, 44 insertions(+), 66 deletions(-) diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c index def26d9ce89d..c93d09e1921e 100644 --- a/drivers/gpu/drm/gma500/gem.c +++ b/drivers/gpu/drm/gma500/gem.c @@ -23,12 +23,12 @@ int psb_gem_pin(struct gtt_range *gt) { - int ret = 0; struct drm_device *dev = gt->gem.dev; struct drm_psb_private *dev_priv = to_drm_psb_private(dev); u32 gpu_base = dev_priv->gtt.gatt_start; struct page **pages; unsigned int npages; + int ret; mutex_lock(&dev_priv->gtt_mutex); @@ -45,10 +45,7 @@ int psb_gem_pin(struct gtt_range *gt) set_pages_array_wc(pages, npages); - ret = psb_gtt_insert(dev, gt); - if (ret) - goto err_drm_gem_put_pages; - + psb_gtt_insert_pages(dev_priv, >->resource, pages); psb_mmu_insert_pages(psb_mmu_get_default_pd(dev_priv->mmu), pages, (gpu_base + gt->offset), npages, 0, 0, PSB_MMU_CACHED_MEMORY); @@ -62,8 +59,6 @@ out: return 0; -err_drm_gem_put_pages: - drm_gem_put_pages(>->gem, pages, true, false); err_mutex_unlock: mutex_unlock(&dev_priv->gtt_mutex); return ret; @@ -86,13 +81,14 @@ void psb_gem_unpin(struct gtt_range *gt) psb_mmu_remove_pages(psb_mmu_get_default_pd(dev_priv->mmu), (gpu_base + gt->offset), gt->npage, 0, 0); - psb_gtt_remove(dev, gt); + psb_gtt_remove_pages(dev_priv, >->resource); /* Reset caching flags */ set_pages_array_wb(gt->pages, gt->npage); drm_gem_put_pages(>->gem, gt->pages, true, false); gt->pages = NULL; + gt->npage = 0; out: mutex_unlock(&dev_priv->gtt_mutex); diff --git a/drivers/gpu/drm/gma500/gtt.c b/drivers/gpu/drm/gma500/gtt.c index 3a716a970a8a..0d70f63c3267 100644 --- a/drivers/gpu/drm/gma500/gtt.c +++ b/drivers/gpu/drm/gma500/gtt.c @@ -66,85 +66,61 @@ static inline uint32_t psb_gtt_mask_pte(uint32_t pfn, int type) return (pfn << PAGE_SHIFT) | mask; } -/** - * psb_gtt_entry - find the GTT entries for a gtt_range - * @dev: our DRM device - * @r: our GTT range - * - * Given a gtt_range object return the GTT offset of the page table - * entries for this gtt_range - */ -static u32 __iomem *psb_gtt_entry(struct drm_device *dev, struct gtt_range *r) +static u32 __iomem *psb_gtt_entry(struct drm_psb_private *pdev, const struct resource *res) { - struct drm_psb_private *dev_priv = to_drm_psb_private(dev); - unsigned long offset; - - offset = r->resource.start - dev_priv->gtt_mem->start; + unsigned long offset = res->start - pdev->gtt_mem->start; - return dev_priv->gtt_map + (offset >> PAGE_SHIFT); + return pdev->gtt_map + (offset >> PAGE_SHIFT); } -/** - * psb_gtt_insert - put an object into the GTT - * @dev: our DRM device - * @r: our GTT range - * - * 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. +/* + * 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. */ -int psb_gtt_insert(struct drm_device *dev, struct gtt_range *r) +void psb_gtt_insert_pages(struct drm_psb_private *pdev, const struct resource *res, + struct page **pages) { + resource_size_t npages, i; u32 __iomem *gtt_slot; u32 pte; - int i; - if (r->pages == NULL) { - WARN_ON(1); - return -EINVAL; - } - - WARN_ON(r->stolen); /* refcount these maybe ? */ + /* Write our page entries into the GTT itself */ - gtt_slot = psb_gtt_entry(dev, r); + npages = resource_size(res) >> PAGE_SHIFT; + gtt_slot = psb_gtt_entry(pdev, res); - /* Write our page entries into the GTT itself */ - for (i = 0; i < r->npage; i++) { - pte = psb_gtt_mask_pte(page_to_pfn(r->pages[i]), - PSB_MMU_CACHED_MEMORY); - iowrite32(pte, gtt_slot++); + for (i = 0; i < npages; ++i, ++gtt_slot) { + pte = psb_gtt_mask_pte(page_to_pfn(pages[i]), PSB_MMU_CACHED_MEMORY); + iowrite32(pte, gtt_slot); } /* Make sure all the entries are set before we return */ ioread32(gtt_slot - 1); - - return 0; } -/** - * psb_gtt_remove - remove an object from the GTT - * @dev: our DRM device - * @r: our GTT range - * - * 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. +/* + * 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. */ -void psb_gtt_remove(struct drm_device *dev, struct gtt_range *r) +void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *res) { - struct drm_psb_private *dev_priv = to_drm_psb_private(dev); + resource_size_t npages, i; u32 __iomem *gtt_slot; u32 pte; - int i; - WARN_ON(r->stolen); + /* Install scratch page for the resource */ + + pte = psb_gtt_mask_pte(page_to_pfn(pdev->scratch_page), PSB_MMU_CACHED_MEMORY); - gtt_slot = psb_gtt_entry(dev, r); - pte = psb_gtt_mask_pte(page_to_pfn(dev_priv->scratch_page), - PSB_MMU_CACHED_MEMORY); + npages = resource_size(res) >> PAGE_SHIFT; + gtt_slot = psb_gtt_entry(pdev, res); - for (i = 0; i < r->npage; i++) - iowrite32(pte, gtt_slot++); + for (i = 0; i < npages; ++i, ++gtt_slot) + iowrite32(pte, gtt_slot); + + /* Make sure all the entries are set before we return */ ioread32(gtt_slot - 1); } @@ -334,9 +310,14 @@ int psb_gtt_restore(struct drm_device *dev) psb_gtt_init(dev, 1); while (r != NULL) { + /* + * TODO: GTT restoration needs a refactoring, so that we don't have to touch + * struct gtt_range here. The type represents a GEM object and is not + * related to the GTT itself. + */ range = container_of(r, struct gtt_range, resource); if (range->pages) { - psb_gtt_insert(dev, range); + psb_gtt_insert_pages(dev_priv, &range->resource, range->pages); size += range->resource.end - range->resource.start; restored++; } diff --git a/drivers/gpu/drm/gma500/gtt.h b/drivers/gpu/drm/gma500/gtt.h index ddb4f3af93f7..b967dcf6bef1 100644 --- a/drivers/gpu/drm/gma500/gtt.h +++ b/drivers/gpu/drm/gma500/gtt.h @@ -49,7 +49,8 @@ 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); -int psb_gtt_insert(struct drm_device *dev, struct gtt_range *r); -void psb_gtt_remove(struct drm_device *dev, struct gtt_range *r); +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); #endif -- 2.11.0