[PATCH] drm: Deal with rotation in drm_plane_helper_check_update()
Ville Syrjälä
ville.syrjala at linux.intel.com
Mon Jan 18 10:00:06 PST 2016
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 at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at 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 at intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala at 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 at 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
More information about the dri-devel
mailing list