Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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 <tzimmermann@suse.de>
Acked-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20211015084053.13708-10-tzimmermann@suse.de

+45 -67
+4 -8
drivers/gpu/drm/gma500/gem.c
··· 23 23 24 24 int psb_gem_pin(struct gtt_range *gt) 25 25 { 26 - int ret = 0; 27 26 struct drm_device *dev = gt->gem.dev; 28 27 struct drm_psb_private *dev_priv = to_drm_psb_private(dev); 29 28 u32 gpu_base = dev_priv->gtt.gatt_start; 30 29 struct page **pages; 31 30 unsigned int npages; 31 + int ret; 32 32 33 33 mutex_lock(&dev_priv->gtt_mutex); 34 34 ··· 45 45 46 46 set_pages_array_wc(pages, npages); 47 47 48 - ret = psb_gtt_insert(dev, gt); 49 - if (ret) 50 - goto err_drm_gem_put_pages; 51 - 48 + psb_gtt_insert_pages(dev_priv, &gt->resource, pages); 52 49 psb_mmu_insert_pages(psb_mmu_get_default_pd(dev_priv->mmu), pages, 53 50 (gpu_base + gt->offset), npages, 0, 0, 54 51 PSB_MMU_CACHED_MEMORY); ··· 59 62 60 63 return 0; 61 64 62 - err_drm_gem_put_pages: 63 - drm_gem_put_pages(&gt->gem, pages, true, false); 64 65 err_mutex_unlock: 65 66 mutex_unlock(&dev_priv->gtt_mutex); 66 67 return ret; ··· 81 86 82 87 psb_mmu_remove_pages(psb_mmu_get_default_pd(dev_priv->mmu), 83 88 (gpu_base + gt->offset), gt->npage, 0, 0); 84 - psb_gtt_remove(dev, gt); 89 + psb_gtt_remove_pages(dev_priv, &gt->resource); 85 90 86 91 /* Reset caching flags */ 87 92 set_pages_array_wb(gt->pages, gt->npage); 88 93 89 94 drm_gem_put_pages(&gt->gem, gt->pages, true, false); 90 95 gt->pages = NULL; 96 + gt->npage = 0; 91 97 92 98 out: 93 99 mutex_unlock(&dev_priv->gtt_mutex);
+38 -57
drivers/gpu/drm/gma500/gtt.c
··· 66 66 return (pfn << PAGE_SHIFT) | mask; 67 67 } 68 68 69 - /** 70 - * psb_gtt_entry - find the GTT entries for a gtt_range 71 - * @dev: our DRM device 72 - * @r: our GTT range 73 - * 74 - * Given a gtt_range object return the GTT offset of the page table 75 - * entries for this gtt_range 76 - */ 77 - static u32 __iomem *psb_gtt_entry(struct drm_device *dev, struct gtt_range *r) 69 + static u32 __iomem *psb_gtt_entry(struct drm_psb_private *pdev, const struct resource *res) 78 70 { 79 - struct drm_psb_private *dev_priv = to_drm_psb_private(dev); 80 - unsigned long offset; 71 + unsigned long offset = res->start - pdev->gtt_mem->start; 81 72 82 - offset = r->resource.start - dev_priv->gtt_mem->start; 83 - 84 - return dev_priv->gtt_map + (offset >> PAGE_SHIFT); 73 + return pdev->gtt_map + (offset >> PAGE_SHIFT); 85 74 } 86 75 87 - /** 88 - * psb_gtt_insert - put an object into the GTT 89 - * @dev: our DRM device 90 - * @r: our GTT range 91 - * 92 - * Take our preallocated GTT range and insert the GEM object into 93 - * the GTT. This is protected via the gtt mutex which the caller 94 - * must hold. 76 + /* 77 + * Take our preallocated GTT range and insert the GEM object into 78 + * the GTT. This is protected via the gtt mutex which the caller 79 + * must hold. 95 80 */ 96 - int psb_gtt_insert(struct drm_device *dev, struct gtt_range *r) 81 + void psb_gtt_insert_pages(struct drm_psb_private *pdev, const struct resource *res, 82 + struct page **pages) 97 83 { 84 + resource_size_t npages, i; 98 85 u32 __iomem *gtt_slot; 99 86 u32 pte; 100 - int i; 101 - 102 - if (r->pages == NULL) { 103 - WARN_ON(1); 104 - return -EINVAL; 105 - } 106 - 107 - WARN_ON(r->stolen); /* refcount these maybe ? */ 108 - 109 - gtt_slot = psb_gtt_entry(dev, r); 110 87 111 88 /* Write our page entries into the GTT itself */ 112 - for (i = 0; i < r->npage; i++) { 113 - pte = psb_gtt_mask_pte(page_to_pfn(r->pages[i]), 114 - PSB_MMU_CACHED_MEMORY); 115 - iowrite32(pte, gtt_slot++); 89 + 90 + npages = resource_size(res) >> PAGE_SHIFT; 91 + gtt_slot = psb_gtt_entry(pdev, res); 92 + 93 + for (i = 0; i < npages; ++i, ++gtt_slot) { 94 + pte = psb_gtt_mask_pte(page_to_pfn(pages[i]), PSB_MMU_CACHED_MEMORY); 95 + iowrite32(pte, gtt_slot); 116 96 } 117 97 118 98 /* Make sure all the entries are set before we return */ 119 99 ioread32(gtt_slot - 1); 120 - 121 - return 0; 122 100 } 123 101 124 - /** 125 - * psb_gtt_remove - remove an object from the GTT 126 - * @dev: our DRM device 127 - * @r: our GTT range 128 - * 129 - * Remove a preallocated GTT range from the GTT. Overwrite all the 130 - * page table entries with the dummy page. This is protected via the gtt 131 - * mutex which the caller must hold. 102 + /* 103 + * Remove a preallocated GTT range from the GTT. Overwrite all the 104 + * page table entries with the dummy page. This is protected via the gtt 105 + * mutex which the caller must hold. 132 106 */ 133 - void psb_gtt_remove(struct drm_device *dev, struct gtt_range *r) 107 + void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *res) 134 108 { 135 - struct drm_psb_private *dev_priv = to_drm_psb_private(dev); 109 + resource_size_t npages, i; 136 110 u32 __iomem *gtt_slot; 137 111 u32 pte; 138 - int i; 139 112 140 - WARN_ON(r->stolen); 113 + /* Install scratch page for the resource */ 141 114 142 - gtt_slot = psb_gtt_entry(dev, r); 143 - pte = psb_gtt_mask_pte(page_to_pfn(dev_priv->scratch_page), 144 - PSB_MMU_CACHED_MEMORY); 115 + pte = psb_gtt_mask_pte(page_to_pfn(pdev->scratch_page), PSB_MMU_CACHED_MEMORY); 145 116 146 - for (i = 0; i < r->npage; i++) 147 - iowrite32(pte, gtt_slot++); 117 + npages = resource_size(res) >> PAGE_SHIFT; 118 + gtt_slot = psb_gtt_entry(pdev, res); 119 + 120 + for (i = 0; i < npages; ++i, ++gtt_slot) 121 + iowrite32(pte, gtt_slot); 122 + 123 + /* Make sure all the entries are set before we return */ 148 124 ioread32(gtt_slot - 1); 149 125 } 150 126 ··· 310 334 psb_gtt_init(dev, 1); 311 335 312 336 while (r != NULL) { 337 + /* 338 + * TODO: GTT restoration needs a refactoring, so that we don't have to touch 339 + * struct gtt_range here. The type represents a GEM object and is not 340 + * related to the GTT itself. 341 + */ 313 342 range = container_of(r, struct gtt_range, resource); 314 343 if (range->pages) { 315 - psb_gtt_insert(dev, range); 344 + psb_gtt_insert_pages(dev_priv, &range->resource, range->pages); 316 345 size += range->resource.end - range->resource.start; 317 346 restored++; 318 347 }
+3 -2
drivers/gpu/drm/gma500/gtt.h
··· 49 49 const char *name, resource_size_t size, resource_size_t align, 50 50 bool stolen, u32 *offset); 51 51 52 - int psb_gtt_insert(struct drm_device *dev, struct gtt_range *r); 53 - void psb_gtt_remove(struct drm_device *dev, struct gtt_range *r); 52 + void psb_gtt_insert_pages(struct drm_psb_private *pdev, const struct resource *res, 53 + struct page **pages); 54 + void psb_gtt_remove_pages(struct drm_psb_private *pdev, const struct resource *res); 54 55 55 56 #endif