[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <20140211115952.GJ17001@phenom.ffwll.local>
Date: Tue, 11 Feb 2014 12:59:52 +0100
From: Daniel Vetter <daniel@...ll.ch>
To: Sagar Arun Kamble <sagar.a.kamble@...el.com>
Cc: Ville Syrjälä
<ville.syrjala@...ux.intel.com>, intel-gfx@...ts.freedesktop.org,
David Airlie <airlied@...ux.ie>,
Daniel Vetter <daniel.vetter@...ll.ch>,
linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH v5 11/11] drm/i915: Calling rotate and
inverse rotate transformations after clipping
On Tue, Feb 11, 2014 at 05:02:31PM +0530, Sagar Arun Kamble wrote:
> On Mon, 2014-02-10 at 15:32 +0200, Ville Syrjälä wrote:
> > On Mon, Feb 10, 2014 at 01:01:18PM +0530, sagar.a.kamble@...el.com wrote:
> > > From: Sagar Kamble <sagar.a.kamble@...el.com>
> > >
> > > With clipped sprites these transformations are not working. these
> > > functions transform complete sprite irrespective of clipping present.
> > > This leads to invisible portion of sprite show up when rotate 180 if
> > > it was out of visible area before.
> > >
> > > v4: Moved rotate transform for source rectangle after clipping.
> > > Added rotate and inverse rotate transform for destination rect.
> >
> > Still NAK.
> >
> > I just pushed rotation support to my glplane test app [1], and with
> > with that my rotated clipping code works exactly as intended.
> >
> > [1] git://gitorious.org/vsyrjala/glplane.git
> I tried this app. I think I am considering 180 degree rotation of
> clipped sprite plane differently. I have captured output with these
> rotate transforms moved before(clip-rotated) and after(rotate-clipped)
> clipping code.
>
> Which is valid? Rotating entire sprite and then clipping or Rotating
> clipped portion?
>
> Reference and Rotated output is attached FYI.
>
> If rotating entire sprite is correct then this patch 11/11 is not needed
> and can be abandoned.
I think doing the clipping first (as a viewport) and then rotating the
viewport makes more sense to me. Dunno what other people think, but I
guess we should document that somewhere.
-Daniel
>
> thanks,
> Sagar
> > >
> > > Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> > > Cc: Jani Nikula <jani.nikula@...ux.intel.com>
> > > Cc: David Airlie <airlied@...ux.ie>
> > > Cc: dri-devel@...ts.freedesktop.org
> > > Cc: linux-kernel@...r.kernel.org
> > > Cc: vijay.a.purushothaman@...el.com
> > > Signed-off-by: Sagar Kamble <sagar.a.kamble@...el.com>
> > > ---
> > > drivers/gpu/drm/i915/intel_sprite.c | 16 ++++++++++++----
> > > 1 file changed, 12 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > > index 62b9f84..799f6a9 100644
> > > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > > @@ -769,9 +769,6 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > > max_scale = intel_plane->max_downscale << 16;
> > > min_scale = intel_plane->can_scale ? 1 : (1 << 16);
> > >
> > > - drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> > > - intel_plane->rotation);
> > > -
> > > hscale = drm_rect_calc_hscale_relaxed(&src, &dst, min_scale, max_scale);
> > > BUG_ON(hscale < 0);
> > >
> > > @@ -785,6 +782,13 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > > crtc_w = drm_rect_width(&dst);
> > > crtc_h = drm_rect_height(&dst);
> > >
> > > + drm_rect_rotate(&src, fb->width << 16, fb->height << 16,
> > > + intel_plane->rotation);
> > > +
> > > + drm_rect_rotate(&dst, intel_crtc->config.pipe_src_w,
> > > + intel_crtc->config.pipe_src_h,
> > > + intel_plane->rotation);
> > > +
> > > if (visible) {
> > > /* check again in case clipping clamped the results */
> > > hscale = drm_rect_calc_hscale(&src, &dst, min_scale, max_scale);
> > > @@ -811,7 +815,11 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
> > > drm_rect_height(&dst) * vscale - drm_rect_height(&src));
> > >
> > > drm_rect_rotate_inv(&src, fb->width << 16, fb->height << 16,
> > > - intel_plane->rotation);
> > > + intel_plane->rotation);
> > > +
> > > + drm_rect_rotate_inv(&dst, intel_crtc->config.pipe_src_w,
> > > + intel_crtc->config.pipe_src_h,
> > > + intel_plane->rotation);
> > >
> > > /* sanity check to make sure the src viewport wasn't enlarged */
> > > WARN_ON(src.x1 < (int) src_x ||
> > > --
> > > 1.8.5
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@...ts.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists