Re: [PATCH 13/22] drm/i915: Move the partial and rotated view data into the same union

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Oct 14, 2015 at 07:29:05PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Each gtt view only has a single type, and so don't need to contain data
> for both rotated and partial views. So we can move the data into the
> same union to avoid wasting space.
> 
> Also rename 'rotation_info' to 'rotated' to match the view type exactly,
> this should avoid confusion which union members is valid for each view
> type. Also make the union anonymous to avoid the .params. stuff all
> over.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem.c      | 10 +++++-----
>  drivers/gpu/drm/i915/i915_gem_gtt.c  | 12 ++++++------
>  drivers/gpu/drm/i915/i915_gem_gtt.h  | 11 ++++-------
>  drivers/gpu/drm/i915/intel_display.c |  4 ++--
>  4 files changed, 17 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index e57061a..b93ed18 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1772,12 +1772,12 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  
>  		memset(&view, 0, sizeof(view));
>  		view.type = I915_GGTT_VIEW_PARTIAL;
> -		view.params.partial.offset = rounddown(page_offset, chunk_size);
> -		view.params.partial.size =
> +		view.partial.offset = rounddown(page_offset, chunk_size);
> +		view.partial.size =
>  			min_t(unsigned int,
>  			      chunk_size,
>  			      (vma->vm_end - vma->vm_start)/PAGE_SIZE -
> -			      view.params.partial.offset);
> +			      view.partial.offset);
>  	}
>  
>  	/* Now pin it into the GTT if needed */
> @@ -1805,10 +1805,10 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  		 * having accessed it before (at this partials' range).
>  		 */
>  		unsigned long base = vma->vm_start +
> -				     (view.params.partial.offset << PAGE_SHIFT);
> +				     (view.partial.offset << PAGE_SHIFT);
>  		unsigned int i;
>  
> -		for (i = 0; i < view.params.partial.size; i++) {
> +		for (i = 0; i < view.partial.size; i++) {
>  			ret = vm_insert_pfn(vma, base + i * PAGE_SIZE, pfn + i);
>  			if (ret)
>  				break;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 71acc71..e0baff2 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3282,7 +3282,7 @@ static struct sg_table *
>  intel_rotate_fb_obj_pages(struct i915_ggtt_view *ggtt_view,
>  			  struct drm_i915_gem_object *obj)
>  {
> -	struct intel_rotation_info *rot_info = &ggtt_view->rotation_info;
> +	struct intel_rotation_info *rot_info = &ggtt_view->rotated;
>  	unsigned int size_pages = rot_info->size >> PAGE_SHIFT;
>  	unsigned int size_pages_uv;
>  	struct sg_page_iter sg_iter;
> @@ -3380,16 +3380,16 @@ intel_partial_pages(const struct i915_ggtt_view *view,
>  	if (!st)
>  		goto err_st_alloc;
>  
> -	ret = sg_alloc_table(st, view->params.partial.size, GFP_KERNEL);
> +	ret = sg_alloc_table(st, view->partial.size, GFP_KERNEL);
>  	if (ret)
>  		goto err_sg_alloc;
>  
>  	sg = st->sgl;
>  	st->nents = 0;
>  	for_each_sg_page(obj->pages->sgl, &obj_sg_iter, obj->pages->nents,
> -		view->params.partial.offset)
> +		view->partial.offset)
>  	{
> -		if (st->nents >= view->params.partial.size)
> +		if (st->nents >= view->partial.size)
>  			break;
>  
>  		sg_set_page(sg, NULL, PAGE_SIZE, 0);
> @@ -3514,9 +3514,9 @@ i915_ggtt_view_size(struct drm_i915_gem_object *obj,
>  	if (view->type == I915_GGTT_VIEW_NORMAL) {
>  		return obj->base.size;
>  	} else if (view->type == I915_GGTT_VIEW_ROTATED) {
> -		return view->rotation_info.size;
> +		return view->rotated.size;
>  	} else if (view->type == I915_GGTT_VIEW_PARTIAL) {
> -		return view->params.partial.size << PAGE_SHIFT;
> +		return view->partial.size << PAGE_SHIFT;
>  	} else {
>  		WARN_ONCE(1, "GGTT view %u not implemented!\n", view->type);
>  		return obj->base.size;
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index a216397..caa182f 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -151,17 +151,14 @@ struct intel_rotation_info {
>  struct i915_ggtt_view {
>  	enum i915_ggtt_view_type type;
>  
> +	struct sg_table *pages;
> +
>  	union {
> +		struct intel_rotation_info rotated;
>  		struct {
>  			u64 offset;
>  			unsigned int size;
>  		} partial;
> -	} params;

If you keep the name for this union (to future-proof the next patch):

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

aka I trust your compiler to get this right ;-)

Cheers, Daniel

> -
> -	struct sg_table *pages;
> -
> -	union {
> -		struct intel_rotation_info rotation_info;
>  	};
>  };
>  
> @@ -557,7 +554,7 @@ i915_ggtt_view_equal(const struct i915_ggtt_view *a,
>  	if (a->type != b->type)
>  		return false;
>  	if (a->type == I915_GGTT_VIEW_PARTIAL)
> -		return !memcmp(&a->params, &b->params, sizeof(a->params));
> +		return !memcmp(&a->partial, &b->partial, sizeof(a->partial));
>  	return true;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 80e9f2e..ce346cfe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2280,7 +2280,7 @@ intel_fill_fb_ggtt_view(struct i915_ggtt_view *view,
>  			unsigned int rotation)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(fb->dev);
> -	struct intel_rotation_info *info = &view->rotation_info;
> +	struct intel_rotation_info *info = &view->rotated;
>  	unsigned int tile_size, tile_width, tile_height, cpp;
>  
>  	*view = i915_ggtt_view_normal;
> @@ -3020,7 +3020,7 @@ unsigned long intel_plane_obj_offset(struct intel_plane *intel_plane,
>  	offset = (unsigned char *)vma->node.start;
>  
>  	if (plane == 1) {
> -		offset += vma->ggtt_view.rotation_info.uv_start_page *
> +		offset += vma->ggtt_view.rotated.uv_start_page *
>  			  PAGE_SIZE;
>  	}
>  
> -- 
> 2.4.9
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx