[PATCH 16/87] drm/i915: Convert i915_ggtt_view to use an anonymous union
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Fri Jan 6 13:40:57 UTC 2017
On 05/01/2017 23:19, Chris Wilson wrote:
> Save a lot of characters by making the union anonymous, with the
> side-effect of ignoring unset bits when comparing views.
>
> v2: Roll up the memcmps back into one.
> v3: And split again as Ville points out we can't trust the compiler.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 11 +++++------
> drivers/gpu/drm/i915/i915_gem_gtt.c | 7 ++++---
> drivers/gpu/drm/i915/i915_gem_gtt.h | 16 ++++++++--------
> drivers/gpu/drm/i915/i915_vma.c | 9 ++++-----
> drivers/gpu/drm/i915/i915_vma.h | 16 ++++++++++------
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> 6 files changed, 32 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 2c1631190a60..43233069ccc8 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1841,13 +1841,12 @@ int i915_gem_fault(struct vm_area_struct *area, struct vm_fault *vmf)
> if (i915_gem_object_is_tiled(obj))
> chunk = roundup(chunk, tile_row_pages(obj));
>
> - memset(&view, 0, sizeof(view));
> view.type = I915_GGTT_VIEW_PARTIAL;
> - view.parrams.partial.offset_size = ounddown(page_offset, chunk);
> - view.params.partial.offset_size =
> - (view.params.partial.offset_size << INTEL_PARTIAL_SIZE_BITS) |
> + view.partial.offset_size = rounddown(page_offset, chunk);
> + view.partial.offset_size =
> + (view.partial.offset_size << INTEL_PARTIAL_SIZE_BITS) |
> (min_t(unsigned int, chunk,
> - vma_pages(area) - view.params.partial.offset_size) - 1);
> + vma_pages(area) - view.partial.offset_size) - 1);
>
> /* If the partial covers the entire object, just create a
> * normal VMA.
> @@ -1882,7 +1881,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 + intel_partial_get_offset(&vma->ggtt_view.params.partial),
> + area->vm_start + intel_partial_get_offset(&vma->ggtt_view.partial),
> (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 4e77baf7d652..f863f2f98c7e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3492,7 +3492,7 @@ intel_partial_pages(const struct i915_ggtt_view *view,
> {
> struct sg_table *st;
> struct scatterlist *sg, *iter;
> - unsigned int count = intel_partial_get_pages(&view->params.partial);
> + unsigned int count = intel_partial_get_pages(&view->partial);
> unsigned int offset;
> int ret = -ENOMEM;
>
> @@ -3505,7 +3505,7 @@ intel_partial_pages(const struct i915_ggtt_view *view,
> goto err_sg_alloc;
>
> iter = i915_gem_object_get_sg(obj,
> - intel_partial_get_page_offset(&view->params.partial),
> + intel_partial_get_page_offset(&view->partial),
> &offset);
> GEM_BUG_ON(!iter);
>
> @@ -3558,7 +3558,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 023bf6ac3dc7..b2daca712df7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -142,12 +142,6 @@ typedef uint64_t gen8_ppgtt_pml4e_t;
>
> struct sg_table;
>
> -enum i915_ggtt_view_type {
> - I915_GGTT_VIEW_NORMAL = 0,
> - I915_GGTT_VIEW_ROTATED,
> - I915_GGTT_VIEW_PARTIAL,
> -};
> -
> struct intel_rotation_info {
> struct intel_rotation_plane_info {
> /* tiles */
> @@ -181,13 +175,19 @@ static inline u64 intel_partial_get_page_offset(const struct intel_partial_info
> return pi->offset_size >> INTEL_PARTIAL_SIZE_BITS;
> }
>
> +enum i915_ggtt_view_type {
> + I915_GGTT_VIEW_NORMAL = 0,
> + I915_GGTT_VIEW_ROTATED = sizeof(struct intel_rotation_info),
> + I915_GGTT_VIEW_PARTIAL = sizeof(struct intel_partial_info),
I would split this out so people can decide if it is too much hackery or
not. :)
Ack on the anonymizing.
Regards,
Tvrtko
> +};
> +
> struct i915_ggtt_view {
> enum i915_ggtt_view_type type;
> -
> union {
> + /* 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 65770b7109c0..284273056ea2 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -96,14 +96,13 @@ __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,
> - intel_partial_get_offset(&view->params.partial),
> - intel_partial_get_size(&view->params.partial),
> + intel_partial_get_offset(&view->partial),
> + intel_partial_get_size(&view->partial),
> obj->base.size));
> - vma->size = intel_partial_get_size(&view->params.partial);
> + vma->size = intel_partial_get_size(&view->partial);
> 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 e3b2b3b1e056..6d936009f919 100644
> --- a/drivers/gpu/drm/i915/i915_vma.h
> +++ b/drivers/gpu/drm/i915/i915_vma.h
> @@ -196,15 +196,19 @@ i915_vma_compare(struct i915_vma *vma,
> if (cmp)
> return cmp;
>
> + cmp = vma->ggtt_view.type;
> if (!view)
> - return vma->ggtt_view.type;
> + return cmp;
> +
> + cmp -= view->type;
> + if (cmp)
> + return cmp;
>
> - if (vma->ggtt_view.type != view->type)
> - return vma->ggtt_view.type - view->type;
> + BUILD_BUG_ON(I915_GGTT_VIEW_PARTIAL == I915_GGTT_VIEW_ROTATED);
> + BUILD_BUG_ON(offsetof(typeof(*view), rotated) !=
> + offsetof(typeof(*view), partial));
>
> - return memcmp(&vma->ggtt_view.params,
> - &view->params,
> - sizeof(view->params));
> + 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 e2150a64860c..f5718f99d0d4 100644
> --- 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;
> }
>
More information about the Intel-gfx-trybot
mailing list