public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/3] drm/i915: Stuff rotation params into view union
Date: Wed, 14 Oct 2015 19:33:23 +0300	[thread overview]
Message-ID: <20151014163323.GW26517@intel.com> (raw)
In-Reply-To: <1444834266-12689-2-git-send-email-daniel.vetter@ffwll.ch>

On Wed, Oct 14, 2015 at 04:51:05PM +0200, Daniel Vetter wrote:
> We don't need 2 separate unions.
> 
> Note that this was done intentinoally
> 
> Author: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Date:   Wed May 6 14:35:38 2015 +0300
> 
>     drm/i915: Add a partial GGTT view type
> 
> on Tvrtko's request, but without a clear justification. Rotated views
> are also not checking for matching paramters in i915_ggtt_view_equal,
> which seems like a bug. But this patch here doesn't change that.
> 
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c  | 4 ++--
>  drivers/gpu/drm/i915/i915_gem_gtt.h  | 5 +----
>  drivers/gpu/drm/i915/intel_display.c | 4 ++--
>  3 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index bead61e0de1b..15dfefd58d08 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3283,7 +3283,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->params.rotation_info;
>  	unsigned int size_pages = rot_info->size >> PAGE_SHIFT;
>  	unsigned int size_pages_uv;
>  	struct sg_page_iter sg_iter;
> @@ -3515,7 +3515,7 @@ 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->params.rotation_info.size;
>  	} else if (view->type == I915_GGTT_VIEW_PARTIAL) {
>  		return view->params.partial.size << PAGE_SHIFT;
>  	} else {
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 9fbb07d6eaad..2e1f6493c9e7 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -156,13 +156,10 @@ struct i915_ggtt_view {
>  			u64 offset;
>  			unsigned int size;
>  		} partial;
> +		struct intel_rotation_info rotation_info;
>  	} params;
>  
>  	struct sg_table *pages;
> -
> -	union {
> -		struct intel_rotation_info rotation_info;
> -	};

So, I basically just sent the same patch, except I made the union
anonymous, and I renamed 'rotation_info' to 'rotated' to match the type.

>  };
>  
>  extern const struct i915_ggtt_view i915_ggtt_view_normal;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2c9d822b29b6..57459fedf216 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2247,7 +2247,7 @@ static void
>  intel_fill_fb_ggtt_view(struct i915_ggtt_view *view, struct drm_framebuffer *fb,
>  			const struct drm_plane_state *plane_state)
>  {
> -	struct intel_rotation_info *info = &view->rotation_info;
> +	struct intel_rotation_info *info = &view->params.rotation_info;
>  	unsigned int tile_height, tile_pitch;
>  
>  	*view = i915_ggtt_view_normal;
> @@ -2909,7 +2909,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.params.rotation_info.uv_start_page *
>  			  PAGE_SIZE;
>  	}
>  
> -- 
> 2.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2015-10-14 16:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-14 14:51 [PATCH 1/3] drm/i915: Drop return value from intel_fill_fb_ggtt_view Daniel Vetter
2015-10-14 14:51 ` [PATCH 2/3] drm/i915: Stuff rotation params into view union Daniel Vetter
2015-10-14 16:33   ` Ville Syrjälä [this message]
2015-10-14 14:51 ` [PATCH 3/3] drm/i915: Fix i915_ggtt_view_equal to handle rotation correctly Daniel Vetter
2015-10-14 15:08   ` Tvrtko Ursulin
2015-10-14 15:33     ` Chris Wilson
2015-10-14 15:56       ` Tvrtko Ursulin
2015-10-14 16:13         ` Chris Wilson
2015-10-14 15:35     ` Chris Wilson
2015-10-14 15:55       ` Tvrtko Ursulin
2015-10-14 16:10         ` Chris Wilson
2015-11-19 15:42     ` Daniel Vetter
2015-10-14 16:34   ` Ville Syrjälä

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151014163323.GW26517@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=daniel.vetter@intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox