Re: [Intel-gfx] [PATCH v2 5/7] drm/i915: Convert i915_ggtt_view to use an anonymous union

[Chris Wilson]

On Fri, Jan 13, 2017 at 09:04:46AM +, Tvrtko Ursulin wrote:
> 
> On 12/01/2017 21:35, Chris Wilson wrote:
> >Save a lot of characters by making the union anonymous, with the
> >side-effect of ignoring unset bits when comparing views.
> 
> Side-effect is not happening in this patch. Always tricky what to do
> with commit messages for split patches. :) Maybe just rewrite the
> commit message.
> 
> >
> >v2: Roll up the memcmps back into one.
> >v3: And split again as Ville points out we can't trust the compiler.
> 
> Not sure what is split, memcmps? But there is only one. Needs v4? Or
> I am missing something?

The changelog just needs rewriting so that the preconditions for this
patch (wanting to able to use a single memcmp and so no uninitialised
bits within the memcmp) are clear. In a sense, it's why I liked having
this in one patch, since the patches are not really standalone and only
make sense together :| Revert one, we have to revert them all.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v2 5/7] drm/i915: Convert i915_ggtt_view to use an anonymous union

[Tvrtko Ursulin]

On 12/01/2017 21:35, Chris Wilson wrote:

Save a lot of characters by making the union anonymous, with the
side-effect of ignoring unset bits when comparing views.


Side-effect is not happening in this patch. Always tricky what to do 
with commit messages for split patches. :) Maybe just rewrite the commit 
message.




v2: Roll up the memcmps back into one.
v3: And split again as Ville points out we can't trust the compiler.


Not sure what is split, memcmps? But there is only one. Needs v4? Or I 
am missing something?



Signed-off-by: Chris Wilson 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/i915/i915_debugfs.c  | 20 ++--
 drivers/gpu/drm/i915/i915_gem.c  |  8 
 drivers/gpu/drm/i915/i915_gem_gtt.c  |  9 -
 drivers/gpu/drm/i915/i915_gem_gtt.h  |  2 +-
 drivers/gpu/drm/i915/i915_vma.c  |  9 -
 drivers/gpu/drm/i915/i915_vma.h  |  4 +++-
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 7 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index e367f06f5883..da13c4c3aa6b 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -167,20 +167,20 @@ describe_obj(struct seq_file *m, struct 
drm_i915_gem_object *obj)

case I915_GGTT_VIEW_PARTIAL:
seq_printf(m, ", partial [%08llx+%x]",
-  vma->ggtt_view.params.partial.offset 
<< PAGE_SHIFT,
-  vma->ggtt_view.params.partial.size 
<< PAGE_SHIFT);
+  vma->ggtt_view.partial.offset << 
PAGE_SHIFT,
+  vma->ggtt_view.partial.size << 
PAGE_SHIFT);
break;

case I915_GGTT_VIEW_ROTATED:
seq_printf(m, ", rotated [(%ux%u, stride=%u, 
offset=%u), (%ux%u, stride=%u, offset=%u)]",
-  
vma->ggtt_view.params.rotated.plane[0].width,
-  
vma->ggtt_view.params.rotated.plane[0].height,
-  
vma->ggtt_view.params.rotated.plane[0].stride,
-  
vma->ggtt_view.params.rotated.plane[0].offset,
-  
vma->ggtt_view.params.rotated.plane[1].width,
-  
vma->ggtt_view.params.rotated.plane[1].height,
-  
vma->ggtt_view.params.rotated.plane[1].stride,
-  
vma->ggtt_view.params.rotated.plane[1].offset);
+  
vma->ggtt_view.rotated.plane[0].width,
+  
vma->ggtt_view.rotated.plane[0].height,
+  
vma->ggtt_view.rotated.plane[0].stride,
+  
vma->ggtt_view.rotated.plane[0].offset,
+  
vma->ggtt_view.rotated.plane[1].width,
+  
vma->ggtt_view.rotated.plane[1].height,
+  
vma->ggtt_view.rotated.plane[1].stride,
+  
vma->ggtt_view.rotated.plane[1].offset);
break;

default:
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f034d8d2dd4c..d8622fd23f5d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1760,10 +1760,10 @@ compute_partial_view(struct drm_i915_gem_object *obj,
chunk = roundup(chunk, tile_row_pages(obj));

view.type = I915_GGTT_VIEW_PARTIAL;
-   view.params.partial.offset = rounddown(page_offset, chunk);
-   view.params.partial.size =
+   view.partial.offset = rounddown(page_offset, chunk);
+   view.partial.size =
min_t(unsigned int, chunk,
- (obj->base.size >> PAGE_SHIFT) - 
view.params.partial.offset);
+ (obj->base.size >> PAGE_SHIFT) - view.partial.offset);

/* If the partial covers the entire object, just create a normal VMA. */
if (chunk >= obj->base.size >> PAGE_SHIFT)
@@ -1879,7 +1879,7 @@ int i915_gem_fault(struct vm_area_struct *area, struct 
vm_fault *vmf)

/* Finally, remap it using the new GTT offset */
ret = remap_io_mapping(area,
-  area->vm_start + (vma->ggtt_view.params.partial.offset 
<< PAGE_SHIFT),
+  area->vm_start + (vma->ggtt_view.partial.offset 
<< PAGE_SHIFT),
   (ggtt->mappable_base + vma->node.start) >> 
PAGE_SHIFT,
   min_t(u64, vma->size, area->vm_end - 
area->vm_start),