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@xxxxxxxxxxxxxxx> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx > Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel