[Intel-gfx] [PATCH 3/9] drm/plane-helper: Add drm_plane_helper_check_state()

[Chris Wilson]

On Tue, Jul 26, 2016 at 07:06:58PM +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Add a version of drm_plane_helper_check_update() which takes a plane
> state instead of having the caller pass in everything.
> 
> And to reduce code duplication, let's reimplement
> drm_plane_helper_check_update() in terms of the new function, by
> having a tempororary plane state on the stack.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/drm_plane_helper.c | 136 ++++++++++++++++++++++++++++---------
>  include/drm/drm_plane_helper.h     |   5 ++
>  2 files changed, 110 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index 16c4a7bd7465..1124eb827671 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -108,14 +108,9 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
>  }
>  
>  /**
> - * drm_plane_helper_check_update() - Check plane update for validity
> - * @plane: plane object to update
> - * @crtc: owning CRTC of owning plane
> - * @fb: framebuffer to flip onto plane
> - * @src: source coordinates in 16.16 fixed point
> - * @dest: integer destination coordinates
> + * drm_plane_helper_check_state() - Check plane state for validity
> + * @state: plane state to check
>   * @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
> @@ -123,8 +118,6 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
>   *                only be false for primary planes.
>   * @can_update_disabled: can the plane be updated while the crtc
>   *                       is disabled?
> - * @visible: output parameter indicating whether plane is still visible after
> - *           clipping
>   *
>   * Checks that a desired plane update is valid.  Drivers that provide
>   * their own plane handling rather than helper-provided implementations may
> @@ -134,29 +127,38 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
>   * RETURNS:
>   * 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,
> -				  unsigned int rotation,
> -				  int min_scale,
> -				  int max_scale,
> -				  bool can_position,
> -				  bool can_update_disabled,
> -				  bool *visible)
> +int drm_plane_helper_check_state(struct drm_plane_state *state,
> +				 const struct drm_rect *clip,
> +				 int min_scale,
> +				 int max_scale,
> +				 bool can_position,
> +				 bool can_update_disabled)
>  {
> +	struct drm_crtc *crtc = state->crtc;
> +	struct drm_framebuffer *fb = state->fb;
> +	struct drm_rect *src = &state->src;
> +	struct drm_rect *dst = &state->dst;
> +	unsigned int rotation = state->rotation;
>  	int hscale, vscale;
>  
> +	src->x1 = state->src_x;
> +	src->y1 = state->src_y;
> +	src->x2 = state->src_x + state->src_w;
> +	src->y2 = state->src_y + state->src_h;
> +
> +	dst->x1 = state->crtc_x;
> +	dst->y1 = state->crtc_y;
> +	dst->x2 = state->crtc_x + state->crtc_w;
> +	dst->y2 = state->crtc_y + state->crtc_h;

It's a little surprising to me that the check writes fields into the
drm_plane_state. Care to at least mention the side-effects in the kernel
doc?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre