[PATCH] drm: Deal with rotation in drm_plane_helper_check_update()
Daniel Vetter
daniel at ffwll.ch
Mon Jan 18 09:14:32 PST 2016
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?
-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
More information about the dri-devel
mailing list