From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Nabendu Maiti <nabendu.bikash.maiti@intel.com>,
dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm: Deal with rotation in drm_plane_helper_check_update()
Date: Mon, 18 Jan 2016 20:00:06 +0200 [thread overview]
Message-ID: <20160118180005.GQ23290@intel.com> (raw)
In-Reply-To: <20160118171432.GL19130@phenom.ffwll.local>
On Mon, Jan 18, 2016 at 06:14:32PM +0100, Daniel Vetter wrote:
> On Fri, Jan 15, 2016 at 08:51:06PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > drm_plane_helper_check_update() needs to account for the plane rotation
> > for correct clipping/scaling calculations. Do so.
> >
> > There was an earlier attempt [1] to add this into
> > intel_check_primary_plane() but I requested that it'd be put into the
> > helper instead. An updated patch never materialized AFAICS, so I went
> > ahead and cooked one up myself.
> >
> > [1] https://patchwork.freedesktop.org/patch/65177/
> > Cc: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> > drivers/gpu/drm/armada/armada_overlay.c | 1 +
> > drivers/gpu/drm/drm_plane_helper.c | 28 ++++++++++++++++++----------
> > drivers/gpu/drm/i915/intel_display.c | 2 ++
> > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 1 +
> > include/drm/drm_plane_helper.h | 1 +
> > 5 files changed, 23 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/armada/armada_overlay.c b/drivers/gpu/drm/armada/armada_overlay.c
> > index 148e8a42b2c6..1ee707ef6b8d 100644
> > --- a/drivers/gpu/drm/armada/armada_overlay.c
> > +++ b/drivers/gpu/drm/armada/armada_overlay.c
> > @@ -121,6 +121,7 @@ armada_ovl_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
> > int ret;
> >
> > ret = drm_plane_helper_check_update(plane, crtc, fb, &src, &dest, &clip,
> > + BIT(DRM_ROTATE_0),
> > 0, INT_MAX, true, false, &visible);
> > if (ret)
> > return ret;
> > diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> > index 369d2898ff9e..0c12fa1a696f 100644
> > --- a/drivers/gpu/drm/drm_plane_helper.c
> > +++ b/drivers/gpu/drm/drm_plane_helper.c
> > @@ -115,6 +115,7 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
> > * @src: source coordinates in 16.16 fixed point
> > * @dest: integer destination coordinates
> > * @clip: integer clipping coordinates
> > + * @rotation: plane rotation
> > * @min_scale: minimum @src:@dest scaling factor in 16.16 fixed point
> > * @max_scale: maximum @src:@dest scaling factor in 16.16 fixed point
> > * @can_position: is it legal to position the plane such that it
> > @@ -134,16 +135,17 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
> > * Zero if update appears valid, error code on failure
> > */
> > int drm_plane_helper_check_update(struct drm_plane *plane,
> > - struct drm_crtc *crtc,
> > - struct drm_framebuffer *fb,
> > - struct drm_rect *src,
> > - struct drm_rect *dest,
> > - const struct drm_rect *clip,
> > - int min_scale,
> > - int max_scale,
> > - bool can_position,
> > - bool can_update_disabled,
> > - bool *visible)
> > + struct drm_crtc *crtc,
> > + struct drm_framebuffer *fb,
> > + struct drm_rect *src,
> > + struct drm_rect *dest,
> > + const struct drm_rect *clip,
> > + unsigned int rotation,
> > + int min_scale,
> > + int max_scale,
> > + bool can_position,
> > + bool can_update_disabled,
> > + bool *visible)
> > {
> > int hscale, vscale;
> >
> > @@ -163,6 +165,8 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
> > return -EINVAL;
> > }
> >
> > + drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> > +
> > /* Check scaling */
> > hscale = drm_rect_calc_hscale(src, dest, min_scale, max_scale);
> > vscale = drm_rect_calc_vscale(src, dest, min_scale, max_scale);
> > @@ -174,6 +178,9 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
> > }
> >
> > *visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale);
> > +
> > + drm_rect_rotate_inv(src, fb->width << 16, fb->height << 16, rotation);
> > +
> > if (!*visible)
> > /*
> > * Plane isn't visible; some drivers can handle this
> > @@ -265,6 +272,7 @@ int drm_primary_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
> >
> > ret = drm_plane_helper_check_update(plane, crtc, fb,
> > &src, &dest, &clip,
> > + BIT(DRM_ROTATE_0),
> > DRM_PLANE_HELPER_NO_SCALING,
> > DRM_PLANE_HELPER_NO_SCALING,
> > false, false, &visible);
>
> With this rotation is essentially unsupported on legacy setups. Shouldn't
> we instead wrap this up into an atomic helper which just takes state
> structs and fishes out everything it needs on its own? Then there's no
> need to change existing drivers all that much, and atomic ones would be
> simpler ...
>
> Or do I miss something?
We'd need to move the various drm_rects to live under drm_plane_state.
Right now they live in intel_plane_state. No objection to doing that
from me, but requires someone to actually write the patch.
> -Daniel
>
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index c4cd9c94faa6..a726c5e91220 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -14029,6 +14029,7 @@ intel_check_primary_plane(struct drm_plane *plane,
> >
> > return drm_plane_helper_check_update(plane, crtc, fb, &state->src,
> > &state->dst, &state->clip,
> > + state->base.rotation,
> > min_scale, max_scale,
> > can_position, true,
> > &state->visible);
> > @@ -14192,6 +14193,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
> >
> > ret = drm_plane_helper_check_update(plane, crtc, fb, &state->src,
> > &state->dst, &state->clip,
> > + state->base.rotation,
> > DRM_PLANE_HELPER_NO_SCALING,
> > DRM_PLANE_HELPER_NO_SCALING,
> > true, true, &state->visible);
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > index 46c2a8dfd8aa..532df0a8656a 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> > @@ -583,6 +583,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
> >
> > ret = drm_plane_helper_check_update(plane, crtc, state->fb,
> > src, dest, &clip,
> > + state->rotation,
> > min_scale,
> > max_scale,
> > true, true, &visible);
> > diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> > index 4421f3f4ca8d..0e0c3573cce0 100644
> > --- a/include/drm/drm_plane_helper.h
> > +++ b/include/drm/drm_plane_helper.h
> > @@ -46,6 +46,7 @@ int drm_plane_helper_check_update(struct drm_plane *plane,
> > struct drm_rect *src,
> > struct drm_rect *dest,
> > const struct drm_rect *clip,
> > + unsigned int rotation,
> > int min_scale,
> > int max_scale,
> > bool can_position,
> > --
> > 2.4.10
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2016-01-18 18:00 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-01-15 18:51 [PATCH] drm: Deal with rotation in drm_plane_helper_check_update() ville.syrjala
2016-01-18 17:14 ` Daniel Vetter
2016-01-18 18:00 ` Ville Syrjälä [this message]
2016-05-11 20:05 ` Ville Syrjälä
2016-06-01 22:17 ` Patrik Jakobsson
2016-06-17 14:13 ` [PATCH v2] " ville.syrjala
2016-06-17 14:41 ` Daniel Vetter
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=20160118180005.GQ23290@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=nabendu.bikash.maiti@intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.