[Intel-gfx] [PATCH 2/4] drm/plane-helper: Add drm_primary_helper_check_update() (v2)

Lee, Chon Ming chon.ming.lee at intel.com
Fri May 16 04:51:44 CEST 2014


On 04/30 10:07, Matt Roper wrote:
> Pull the parameter checking from drm_primary_helper_update() out into
> its own function; drivers that provide their own setplane()
> implementations rather than using the helper may still want to share
> this parameter checking logic.
> 
> A few of the checks here were also updated based on suggestions by
> Ville Syrjälä.
> 
> v2:
>   - Pass src/dest/clip rects and min/max scaling down to helper to avoid
>     duplication of effort between helper and drivers (suggested by
>     Ville).
>   - Allow caller to specify whether the primary plane should be
>     updatable while the crtc is disabled.
> 
> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
>  drivers/gpu/drm/drm_plane_helper.c | 123 ++++++++++++++++++++++++++++---------
>  include/drm/drm_plane_helper.h     |  24 +++++++-
>  2 files changed, 116 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
> index b601233..11e8b82 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -26,6 +26,7 @@
>  #include <linux/list.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_rect.h>
> +#include <drm/drm_plane_helper.h>
>  
>  #define SUBPIXEL_MASK 0xffff
>  
> @@ -66,6 +67,77 @@ static int get_connectors_for_crtc(struct drm_crtc *crtc,
>  }
>  
>  /**
> + * drm_primary_helper_check_update() - Check primary 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
> + * @clip: integer clipping coordinates
> + * @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 primary plane such that it
> + *                doesn't cover the entire crtc?
> + * @can_update_disabled: can the primary plane be updated while the crtc
> + *                       is disabled?
> + * @visible: output parameter indicating whether plane is still visible after
> + *           clipping
> + *
> + * Checks that a desired primary plane update is valid.  Drivers that provide
> + * their own primary plane handling may still wish to call this function to
> + * avoid duplication of error checking code.
> + *
> + * RETURNS:
> + * Zero if update appears valid, error code on failure
> + */
> +int drm_primary_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)
> +{
> +	int hscale, vscale;
> +
> +	if (!crtc->enabled && !can_update_disabled) {
> +		DRM_DEBUG_KMS("Cannot update primary plane of a disabled CRTC.\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Check scaling */
> +	hscale = drm_rect_calc_hscale(src, dest, min_scale, max_scale);
> +	vscale = drm_rect_calc_vscale(src, dest, min_scale, max_scale);
> +	if (hscale < 0 || vscale < 0) {
> +		DRM_DEBUG_KMS("Invalid scaling of primary plane\n");
> +		return -ERANGE;
> +	}
> +
> +	*visible = drm_rect_clip_scaled(src, dest, clip, hscale, vscale);
> +	if (!visible)
> +		/*
> +		 * Primary plane isn't visible; some drivers can handle this
> +		 * so we just return success here.  Drivers that can't
> +		 * (including those that use the primary plane helper's
> +		 * update function) will return an error from their
> +		 * update_plane handler.
> +		 */
> +		return 0;
> +
> +	if (!can_position && !drm_rect_equals(dest, clip)) {
> +		DRM_DEBUG_KMS("Primary plane must cover entire CRTC\n");
> +		return -EINVAL;
> +	}

Cherryview display allow the primary plane to be position at any location
similiar to sprite plane for certain port. So, this shouldn't need to check here.  

And the width/height doesn't need to cover the whole screen.

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_primary_helper_check_update);
> +
> +/**
>   * drm_primary_helper_update() - Helper for primary plane update
>   * @plane: plane object to update
>   * @crtc: owning CRTC of owning plane
> @@ -113,51 +185,42 @@ int drm_primary_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
>  		.x = src_x >> 16,
>  		.y = src_y >> 16,
>  	};
> +	struct drm_rect src = {
> +		.x1 = src_x,
> +		.y1 = src_y,
> +		.x2 = src_x + src_w,
> +		.y2 = src_y + src_h,
> +	};
>  	struct drm_rect dest = {
>  		.x1 = crtc_x,
>  		.y1 = crtc_y,
>  		.x2 = crtc_x + crtc_w,
>  		.y2 = crtc_y + crtc_h,
>  	};
> -	struct drm_rect clip = {
> +	const struct drm_rect clip = {
>  		.x2 = crtc->mode.hdisplay,
>  		.y2 = crtc->mode.vdisplay,
>  	};
>  	struct drm_connector **connector_list;
>  	int num_connectors, ret;
> +	bool visible;
>  
> -	if (!crtc->enabled) {
> -		DRM_DEBUG_KMS("Cannot update primary plane of a disabled CRTC.\n");
> -		return -EINVAL;
> -	}
> -
> -	/* Disallow subpixel positioning */
> -	if ((src_x | src_y | src_w | src_h) & SUBPIXEL_MASK) {
> -		DRM_DEBUG_KMS("Primary plane does not support subpixel positioning\n");
> -		return -EINVAL;
> -	}
> -
> -	/* Disallow scaling */
> -	src_w >>= 16;
> -	src_h >>= 16;
> -	if (crtc_w != src_w || crtc_h != src_h) {
> -		DRM_DEBUG_KMS("Can't scale primary plane\n");
> -		return -EINVAL;
> -	}
> -
> -	/* Make sure primary plane covers entire CRTC */
> -	drm_rect_intersect(&dest, &clip);
> -	if (dest.x1 != 0 || dest.y1 != 0 ||
> -	    dest.x2 != crtc->mode.hdisplay || dest.y2 != crtc->mode.vdisplay) {
> -		DRM_DEBUG_KMS("Primary plane must cover entire CRTC\n");
> -		return -EINVAL;
> -	}
> -
> -	/* Framebuffer must be big enough to cover entire plane */
> -	ret = drm_crtc_check_viewport(crtc, crtc_x, crtc_y, &crtc->mode, fb);
> +	ret = drm_primary_helper_check_update(plane, crtc, fb,
> +					      &src, &dest, &clip,
> +					      DRM_PLANE_HELPER_NO_SCALING,
> +					      DRM_PLANE_HELPER_NO_SCALING,
> +					      false, false, &visible);
>  	if (ret)
>  		return ret;
>  
> +	if (!visible)
> +		/*
> +		 * Primary plane isn't visible.  Note that unless a driver
> +		 * provides their own disable function, this will just
> +		 * wind up returning -EINVAL to userspace.
> +		 */
> +		return plane->funcs->disable_plane(plane);
> +
>  	/* Find current connectors for CRTC */
>  	num_connectors = get_connectors_for_crtc(crtc, NULL, 0);
>  	BUG_ON(num_connectors == 0);
> diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
> index 09824be..05e1357 100644
> --- a/include/drm/drm_plane_helper.h
> +++ b/include/drm/drm_plane_helper.h
> @@ -24,6 +24,17 @@
>  #ifndef DRM_PLANE_HELPER_H
>  #define DRM_PLANE_HELPER_H
>  
> +#include <drm/drm_rect.h>
> +
> +/*
> + * Drivers that don't allow primary plane scaling may pass this macro in place
> + * of the min/max scale parameters of the update checker function.
> + *
> + * Due to src being in 16.16 fixed point and dest being in integer pixels,
> + * 1<<16 represents no scaling.
> + */
> +#define DRM_PLANE_HELPER_NO_SCALING (1<<16)
> +
>  /**
>   * DOC: plane helpers
>   *
> @@ -31,6 +42,17 @@
>   * planes.
>   */
>  
> +extern int drm_primary_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);
>  extern int drm_primary_helper_update(struct drm_plane *plane,
>  				     struct drm_crtc *crtc,
>  				     struct drm_framebuffer *fb,
> @@ -42,7 +64,7 @@ extern int drm_primary_helper_disable(struct drm_plane *plane);
>  extern void drm_primary_helper_destroy(struct drm_plane *plane);
>  extern const struct drm_plane_funcs drm_primary_helper_funcs;
>  extern struct drm_plane *drm_primary_helper_create_plane(struct drm_device *dev,
> -							 uint32_t *formats,
> +							 const uint32_t *formats,
>  							 int num_formats);
>  
>  
> -- 
> 1.8.5.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



More information about the Intel-gfx mailing list