drm/i915: Convert i915_ggtt_view to use an anonymous union

Reading the ggtt_views is much more pleasant without the extra
characters from specifying the union (i.e. ggtt_view.partial rather than
ggtt_view.params.partial). To make this work inside i915_vma_compare()
with only a single memcmp requires us to ensure that there are no
uninitialised bytes within each branch of the union (we make sure the
structs are packed) and we need to store the size of each branch.

v4: Rewrite changelog and add comments explaining the assert.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/20170114002827.31315-5-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 32ede342ab94f55af01e85f8c34f894e0b8a2393..01fdbbf0fd43f34f79755b7a658381f824bc98ea 100644 (file)
--- 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 f034d8d2dd4cc485c74619ce7b8052584695cf5c..d8622fd23f5d94291dca20efd1f19266023dbb67 100644 (file)
--- 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),
                               &ggtt->mappable);

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 7d21cdfc6b0e28600c62ef185ff45c565de8386e..e24b961c30c657a827e02f267861aa3a2397aeed 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3511,7 +3511,7 @@ intel_partial_pages(const struct i915_ggtt_view *view,
 {
        struct sg_table *st;
        struct scatterlist *sg, *iter;
-       unsigned int count = view->params.partial.size;
+       unsigned int count = view->partial.size;
        unsigned int offset;
        int ret = -ENOMEM;
 
@@ -3523,9 +3523,7 @@ intel_partial_pages(const struct i915_ggtt_view *view,
        if (ret)
                goto err_sg_alloc;
 
-       iter = i915_gem_object_get_sg(obj,
-                                     view->params.partial.offset,
-                                     &offset);
+       iter = i915_gem_object_get_sg(obj, view->partial.offset, &offset);
        GEM_BUG_ON(!iter);
 
        sg = st->sgl;
@@ -3577,7 +3575,8 @@ i915_get_ggtt_vma_pages(struct i915_vma *vma)
                vma->pages = vma->obj->mm.pages;
        else if (vma->ggtt_view.type == I915_GGTT_VIEW_ROTATED)
                vma->pages =
-                       intel_rotate_fb_obj_pages(&vma->ggtt_view.params.rotated, vma->obj);
+                       intel_rotate_fb_obj_pages(&vma->ggtt_view.rotated,
+                                                 vma->obj);
        else if (vma->ggtt_view.type == I915_GGTT_VIEW_PARTIAL)
                vma->pages = intel_partial_pages(&vma->ggtt_view, vma->obj);
        else

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 35ea4a18dc7764d1d2a99e07f5492fb64731d0ec..71e7e0a7e2b6c74450702e152d597cf95f9204bb 100644 (file)
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -193,7 +193,7 @@ struct i915_ggtt_view {
                /* Members need to contain no holes/padding */
                struct intel_partial_info partial;
                struct intel_rotation_info rotated;
-       } params;
+       };
 };
 
 extern const struct i915_ggtt_view i915_ggtt_view_normal;

diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 379364b8fef9337617560f3fdf9987c27473be1d..fe93ed1e012f343a04cde5bf7c13891876bb257e 100644 (file)
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -97,15 +97,14 @@ __i915_vma_create(struct drm_i915_gem_object *obj,
                vma->ggtt_view = *view;
                if (view->type == I915_GGTT_VIEW_PARTIAL) {
                        GEM_BUG_ON(range_overflows_t(u64,
-                                                    view->params.partial.offset,
-                                                    view->params.partial.size,
+                                                    view->partial.offset,
+                                                    view->partial.size,
                                                     obj->base.size >> PAGE_SHIFT));
-                       vma->size = view->params.partial.size;
+                       vma->size = view->partial.size;
                        vma->size <<= PAGE_SHIFT;
                        GEM_BUG_ON(vma->size >= obj->base.size);
                } else if (view->type == I915_GGTT_VIEW_ROTATED) {
-                       vma->size =
-                               intel_rotation_info_size(&view->params.rotated);
+                       vma->size = intel_rotation_info_size(&view->rotated);
                        vma->size <<= PAGE_SHIFT;
                }
        }

diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h
index fdbacc036080efef6c0cc87b5cbb91f426c75fdf..86b60fb4e95478c71a9be03a488b59092ed0ec81 100644 (file)
--- a/drivers/gpu/drm/i915/i915_vma.h
+++ b/drivers/gpu/drm/i915/i915_vma.h
@@ -212,10 +212,17 @@ i915_vma_compare(struct i915_vma *vma,
         * different views using it as a "type" and also use a compact (no
         * accessing of uninitialised padding bytes) memcmp without storing
         * an extra parameter or adding more code.
+        *
+        * To ensure that the memcmp is valid for all branches of the union,
+        * even though the code looks like it is just comparing one branch,
+        * we assert above that all branches have the same address, and that
+        * each branch has a unique type/size.
         */
        BUILD_BUG_ON(I915_GGTT_VIEW_NORMAL >= I915_GGTT_VIEW_PARTIAL);
        BUILD_BUG_ON(I915_GGTT_VIEW_PARTIAL >= I915_GGTT_VIEW_ROTATED);
-       return memcmp(&vma->ggtt_view.params, &view->params, view->type);
+       BUILD_BUG_ON(offsetof(typeof(*view), rotated) !=
+                    offsetof(typeof(*view), partial));
+       return memcmp(&vma->ggtt_view.partial, &view->partial, view->type);
 }
 
 int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level,

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fd5fbc83c69e6a68b8ec32cf375b37fef8f2229b..f4be20f0198a90002fe69218a374579c16e29619 100644 (file)
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2139,7 +2139,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
 {
        if (drm_rotation_90_or_270(rotation)) {
                *view = i915_ggtt_view_rotated;
-               view->params.rotated = to_intel_framebuffer(fb)->rot_info;
+               view->rotated = to_intel_framebuffer(fb)->rot_info;
        } else {
                *view = i915_ggtt_view_normal;
        }