All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: Deal with rotation in drm_plane_helper_check_update()
@ 2016-01-15 18:51 ville.syrjala
  2016-01-18 17:14 ` Daniel Vetter
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: ville.syrjala @ 2016-01-15 18:51 UTC (permalink / raw)
  To: dri-devel; +Cc: Nabendu Maiti

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);
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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm: Deal with rotation in drm_plane_helper_check_update()
  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ä
  2016-05-11 20:05 ` Ville Syrjälä
  2016-06-17 14:13 ` [PATCH v2] " ville.syrjala
  2 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2016-01-18 17:14 UTC (permalink / raw)
  To: ville.syrjala; +Cc: Nabendu Maiti, dri-devel

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?
-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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm: Deal with rotation in drm_plane_helper_check_update()
  2016-01-18 17:14 ` Daniel Vetter
@ 2016-01-18 18:00   ` Ville Syrjälä
  0 siblings, 0 replies; 7+ messages in thread
From: Ville Syrjälä @ 2016-01-18 18:00 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Nabendu Maiti, dri-devel

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm: Deal with rotation in drm_plane_helper_check_update()
  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-05-11 20:05 ` Ville Syrjälä
  2016-06-01 22:17   ` Patrik Jakobsson
  2016-06-17 14:13 ` [PATCH v2] " ville.syrjala
  2 siblings, 1 reply; 7+ messages in thread
From: Ville Syrjälä @ 2016-05-11 20:05 UTC (permalink / raw)
  To: dri-devel; +Cc: Nabendu Maiti

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.

Ping. Anyone interested in rotation actually working with
drm_plane_helper_check_update() ?

> 
> [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);
> 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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] drm: Deal with rotation in drm_plane_helper_check_update()
  2016-05-11 20:05 ` Ville Syrjälä
@ 2016-06-01 22:17   ` Patrik Jakobsson
  0 siblings, 0 replies; 7+ messages in thread
From: Patrik Jakobsson @ 2016-06-01 22:17 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Nabendu Maiti, dri-devel

On Wed, May 11, 2016 at 10:05 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> 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.
>
> Ping. Anyone interested in rotation actually working with
> drm_plane_helper_check_update() ?
>

Passing state structs sounds better but since that patch doesn't exist
I'd go for this one.

There are now more users of drm_plane_helper_check_update() so needs fixing.

With that done;
Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

>>
>> [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);
>> 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
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] drm: Deal with rotation in drm_plane_helper_check_update()
  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-05-11 20:05 ` Ville Syrjälä
@ 2016-06-17 14:13 ` ville.syrjala
  2016-06-17 14:41   ` Daniel Vetter
  2 siblings, 1 reply; 7+ messages in thread
From: ville.syrjala @ 2016-06-17 14:13 UTC (permalink / raw)
  To: dri-devel; +Cc: Russell King, Nabendu Maiti

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.

v2: Deal with new drm_plane_helper_check_update() callers

[1] https://patchwork.freedesktop.org/patch/65177/
Cc: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
Cc: Noralf Trønnes <noralf@tronnes.org>
Cc: CK Hu <ck.hu@mediatek.com>
Cc: Mark Yao <mark.yao@rock-chips.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
---
 drivers/gpu/drm/armada/armada_overlay.c     |  1 +
 drivers/gpu/drm/drm_plane_helper.c          | 28 ++++++++++++++++++----------
 drivers/gpu/drm/drm_simple_kms_helper.c     |  1 +
 drivers/gpu/drm/i915/intel_display.c        |  2 ++
 drivers/gpu/drm/mediatek/mtk_drm_plane.c    |  1 +
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  1 +
 include/drm/drm_plane_helper.h              |  1 +
 7 files changed, 25 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 fc51306fe365..16c4a7bd7465 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
@@ -267,6 +274,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);
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
index b2071d495ada..0db36d27e90b 100644
--- a/drivers/gpu/drm/drm_simple_kms_helper.c
+++ b/drivers/gpu/drm/drm_simple_kms_helper.c
@@ -105,6 +105,7 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
 	ret = drm_plane_helper_check_update(plane, &pipe->crtc,
 					    plane_state->fb,
 					    &src, &dest, &clip,
+					    plane_state->rotation,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    DRM_PLANE_HELPER_NO_SCALING,
 					    false, true, &visible);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8c6f4e2a2cb8..029456cfc57e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -14184,6 +14184,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);
@@ -14375,6 +14376,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/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
index 51bc8988fc26..3995765a90dc 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
@@ -170,6 +170,7 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
 
 	return drm_plane_helper_check_update(plane, state->crtc, fb,
 					     &src, &dest, &clip,
+					     state->rotation,
 					     DRM_PLANE_HELPER_NO_SCALING,
 					     DRM_PLANE_HELPER_NO_SCALING,
 					     true, true, &visible);
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 8cd840f602b7..6255e5bcd954 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -626,6 +626,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.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] drm: Deal with rotation in drm_plane_helper_check_update()
  2016-06-17 14:13 ` [PATCH v2] " ville.syrjala
@ 2016-06-17 14:41   ` Daniel Vetter
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Vetter @ 2016-06-17 14:41 UTC (permalink / raw)
  To: ville.syrjala; +Cc: Russell King, Nabendu Maiti, dri-devel

On Fri, Jun 17, 2016 at 05:13:10PM +0300, 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.
> 
> v2: Deal with new drm_plane_helper_check_update() callers
> 
> [1] https://patchwork.freedesktop.org/patch/65177/
> Cc: Nabendu Maiti <nabendu.bikash.maiti@intel.com>
> Cc: Noralf Trønnes <noralf@tronnes.org>
> Cc: CK Hu <ck.hu@mediatek.com>
> Cc: Mark Yao <mark.yao@rock-chips.com>
> Cc: Russell King <rmk+kernel@armlinux.org.uk>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>

Applied to drm-misc, thanks.
-Daniel

> ---
>  drivers/gpu/drm/armada/armada_overlay.c     |  1 +
>  drivers/gpu/drm/drm_plane_helper.c          | 28 ++++++++++++++++++----------
>  drivers/gpu/drm/drm_simple_kms_helper.c     |  1 +
>  drivers/gpu/drm/i915/intel_display.c        |  2 ++
>  drivers/gpu/drm/mediatek/mtk_drm_plane.c    |  1 +
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c |  1 +
>  include/drm/drm_plane_helper.h              |  1 +
>  7 files changed, 25 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 fc51306fe365..16c4a7bd7465 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
> @@ -267,6 +274,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);
> diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c
> index b2071d495ada..0db36d27e90b 100644
> --- a/drivers/gpu/drm/drm_simple_kms_helper.c
> +++ b/drivers/gpu/drm/drm_simple_kms_helper.c
> @@ -105,6 +105,7 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane,
>  	ret = drm_plane_helper_check_update(plane, &pipe->crtc,
>  					    plane_state->fb,
>  					    &src, &dest, &clip,
> +					    plane_state->rotation,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    DRM_PLANE_HELPER_NO_SCALING,
>  					    false, true, &visible);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8c6f4e2a2cb8..029456cfc57e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -14184,6 +14184,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);
> @@ -14375,6 +14376,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/mediatek/mtk_drm_plane.c b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> index 51bc8988fc26..3995765a90dc 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c
> @@ -170,6 +170,7 @@ static int mtk_plane_atomic_check(struct drm_plane *plane,
>  
>  	return drm_plane_helper_check_update(plane, state->crtc, fb,
>  					     &src, &dest, &clip,
> +					     state->rotation,
>  					     DRM_PLANE_HELPER_NO_SCALING,
>  					     DRM_PLANE_HELPER_NO_SCALING,
>  					     true, true, &visible);
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 8cd840f602b7..6255e5bcd954 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -626,6 +626,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.7.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2016-06-17 14:41 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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ä
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

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.