Re: [PATCH 3/4] drm/i915: Support NV12 in rotated GGTT mapping

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Sep 21, 2015 at 02:14:47PM +0300, Joonas Lahtinen wrote:
> On ma, 2015-09-21 at 10:45 +0100, Tvrtko Ursulin wrote:
> > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> > Just adding the rotated UV plane at the end of the rotated Y plane.
> > 
> > v2: Rebase.
> > 
> > 
> > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > 
> 
> One comment below, otherwise.
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c  | 37
> > ++++++++++++++++++++++++++++++------
> >  drivers/gpu/drm/i915/i915_gem_gtt.h  |  3 +++
> >  drivers/gpu/drm/i915/intel_display.c | 12 ++++++++++++
> >  3 files changed, 46 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 59c934fb9230..2df9d16dcefd 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -3272,10 +3272,13 @@ intel_rotate_fb_obj_pages(struct
> > i915_ggtt_view *ggtt_view,
> >  {
> >  > 	> struct intel_rotation_info *rot_info = &ggtt_view
> > ->rotation_info;
> >  > 	> unsigned int size_pages = rot_info->size >> PAGE_SHIFT;
> > +> 	> unsigned int size_pages_uv;
> 
> Could be initialized to zero here already as majority of the time it'll
> be unchanged.

Personally (and this is really pretty clearly in bikeshed territory) I
prefer it like Tvrtko has done since if you ever rework the code for new
platform support and forget one case gcc will complain about potentially
uninitialized variable. But if you initialize it at first that will paper
over such a bug. And gcc will optimize this anyway for you (besides that
generally performance in modeset code just doesn't really matter all that
much).

And something funny is going on with your mail replies, you insert piles
of spurious > even though it's just one reply level ...
-Daniel
-- 
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