drm/i915: prefer vma->size for vma iomap
Subsystems (2)
PATCH 1/1 drm/i915: prefer vma->size for vma iomap
It makes more sense to use vma->size, since this determines the number of entries we inserted into the vm, while the vma->node.size is the size of the vm window we reserved, which may also include padding. At the very least this keeps things consistent with the GTT routines.
--- drivers/gpu/drm/i915/i915_gem_shrinker.c | 2 +- drivers/gpu/drm/i915/i915_vma.c | 2 +- drivers/gpu/drm/i915/intel_fbdev.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/i915_gem_shrinker.c +1 -1
@@ -481,7 +481,7 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr/* We also want to clear any cached iomaps as they wrap vmap */ list_for_each_entry_safe (vma, next, &i915->ggtt.base.inactive_list, vm_link) { -unsigned long count = vma->node.size >>PAGE_SHIFT ; +unsigned long count = vma->size >>PAGE_SHIFT ;if (vma->iomap &&i915_vma_unbind (vma) ==0 ) freed_pages += count; }
drivers/gpu/drm/i915/i915_vma.c +1 -1
@@ -312,7 +312,7 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)if (ptr ==NULL ) { ptr =io_mapping_map_wc (&i915_vm_to_ggtt (vma->vm)->iomap, vma->node.start, - vma->node.size); + vma->size);if (ptr ==NULL ) { err = -ENOMEM ; goto err;
drivers/gpu/drm/i915/intel_fbdev.c +2 -2
@@ -244,7 +244,7 @@ static int intelfb_create(struct drm_fb_helper *helper, info->apertures->ranges[0 ].size = ggtt->mappable_end; info->fix.smem_start = dev->mode_config.fb_base +i915_ggtt_offset (vma); - info->fix.smem_len = vma->node.size; + info->fix.smem_len = vma->size; vaddr =i915_vma_pin_iomap (vma);if (IS_ERR (vaddr)) {
@@ -253,7 +253,7 @@ static int intelfb_create(struct drm_fb_helper *helper, goto out_unpin; } info->screen_base = vaddr; - info->screen_size = vma->node.size; + info->screen_size = vma->size;/* This driver doesn't need a VT switch to restore the mode on resume */ info->skip_vt_switch = true;
Signature · 6 lines
2.14.3 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Quoting Matthew Auld (2018-03-17 17:42:07)
Quoted 4 lines
It makes more sense to use
vma->size, since this determines the number of entries we inserted into the vm, while thevma->node.sizeis the size of the vm window we reserved, which may also include padding. At the very least this keeps things consistent with the GTT routines.
"Being consistent with the other GTT routines"? Name them, we're talking about how many pages does the caller expect to be able to access, is the padding (that they established for the vma) part of the iomap?
So the padding exists to ensure that random tiling beneath the pointer maps to real pages, but it was established during vma pinning by the caller. Being the kernel maybe they do want to access those pages, but equally what can they do? It's just one page that they could access directly if so required. I don't see a reason and can't think of an instance where we do.
Quoted 13 lines
Signed-off-by Matthew Auld <[email protected]>Cc Chris Wilson <[email protected]>--- drivers/gpu/drm/i915/i915_gem_shrinker.c | 2 +- drivers/gpu/drm/i915/i915_vma.c | 2 +- drivers/gpu/drm/i915/intel_fbdev.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-)drivers/gpu/drm/i915/i915_gem_shrinker.c +0
-0
drivers/gpu/drm/i915/i915_gem_shrinker.c i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr · line 487
Quoted 8 lines
drivers/gpu/drm/i915/i915_gem_shrinker.c +1
-1@@ -481,7 +481,7 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr/* We also want to clear any cached iomaps as they wrap vmap */ list_for_each_entry_safe (vma, next, &i915->ggtt.base.inactive_list, vm_link) { -unsigned long count = vma->node.size >>PAGE_SHIFT ; +unsigned long count = vma->size >>PAGE_SHIFT ;if (vma->iomap &&i915_vma_unbind (vma) ==0 ) freed_pages += count; }
Ok, as the size the vmap() is obj->base.size.
drivers/gpu/drm/i915/i915_vma.c void __iomem *i915_vma_pin_iomap(struct i915_vma *vma) · line 315
Quoted 10 lines
drivers/gpu/drm/i915/i915_vma.c +1
-1@@ -312,7 +312,7 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma)if (ptr ==NULL ) { ptr =io_mapping_map_wc (&i915_vm_to_ggtt (vma->vm)->iomap, vma->node.start, - vma->node.size); + vma->size);
Debatable. Even though I can't think of a reason why, I don't see if we need to be overly restrictive either.
drivers/gpu/drm/i915/intel_fbdev.c static int intelfb_create(struct drm_fb_helper *helper, · line 247
Quoted 13 lines
drivers/gpu/drm/i915/i915_vma.c +0
-0if (ptr ==NULL ) { err = -ENOMEM ; goto err;drivers/gpu/drm/i915/intel_fbdev.c +1
-1@@ -244,7 +244,7 @@ static int intelfb_create(struct drm_fb_helper *helper, info->apertures->ranges[0 ].size = ggtt->mappable_end; info->fix.smem_start = dev->mode_config.fb_base +i915_ggtt_offset (vma); - info->fix.smem_len = vma->node.size; + info->fix.smem_len = vma->size;
I think this has to be node.size for the overlaps to be checked.
drivers/gpu/drm/i915/intel_fbdev.c static int intelfb_create(struct drm_fb_helper *helper, · line 256
Quoted 8 lines
drivers/gpu/drm/i915/intel_fbdev.c +1
-1vaddr =i915_vma_pin_iomap (vma);if (IS_ERR (vaddr)) {@@ -253,7 +253,7 @@ static int intelfb_create(struct drm_fb_helper *helper, goto out_unpin; } info->screen_base = vaddr; - info->screen_size = vma->node.size; + info->screen_size = vma->size;
I don't what this is used for; vma->size is probably correct. -Chris
_______________________________________________
Intel-gfx mailing list [email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx