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.syrj...@linux.intel.com>
> Cc: dri-de...@lists.freedesktop.org
> Signed-off-by: Matt Roper <matthew.d.ro...@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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
