diff mbox

drm: Deal with rotation in drm_plane_helper_check_update()

Message ID 1452883866-28832-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Jan. 15, 2016, 6:51 p.m. UTC
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(-)

Comments

Daniel Vetter Jan. 18, 2016, 5:14 p.m. UTC | #1
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
Ville Syrjälä Jan. 18, 2016, 6 p.m. UTC | #2
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ä May 11, 2016, 8:05 p.m. UTC | #3
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
Patrik Jakobsson June 1, 2016, 10:17 p.m. UTC | #4
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
diff mbox

Patch

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,