On Tue, Apr 24, 2018 at 05:11:37PM +0200, Maarten Lankhorst wrote:
> Op 24-04-18 om 17:21 schreef Ville Syrjälä:
> > On Tue, Apr 24, 2018 at 01:36:32PM +0200, Maarten Lankhorst wrote:
> >> When calculating limits we want to be as pessimistic as possible,
> >> so we have to explicitly say whether we want to round up or down
> >> to accurately calculate whether we are below min_scale or above
> >> max_scale.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/drm_atomic_helper.c | 4 +--
> >> drivers/gpu/drm/drm_rect.c | 39 +++++++++++++++++++++--------
> >> drivers/gpu/drm/i915/intel_sprite.c | 8 +++---
> >> include/drm/drm_rect.h | 8 +++---
> >> 4 files changed, 39 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> >> index 9cb2209f6fc8..7643202bfcf7 100644
> >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> @@ -754,8 +754,8 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
> >> drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> >>
> >> /* Check scaling */
> >> - hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >> - vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> >> + hscale = drm_rect_calc_hscale(src, dst, min_scale, 1<<16, max_scale);
> >> + vscale = drm_rect_calc_vscale(src, dst, min_scale, 1<<16, max_scale);
> >> if (hscale < 0 || vscale < 0) {
> >> DRM_DEBUG_KMS("Invalid scaling of plane\n");
> >> drm_rect_debug_print("src: ", &plane_state->src, true);
> >> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> >> index 9817c1445ba9..2093474c268c 100644
> >> --- a/drivers/gpu/drm/drm_rect.c
> >> +++ b/drivers/gpu/drm/drm_rect.c
> >> @@ -96,7 +96,7 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
> >> }
> >> EXPORT_SYMBOL(drm_rect_clip_scaled);
> >>
> >> -static int drm_calc_scale(int src, int dst)
> >> +static int drm_calc_scale(int src, int dst, int scale_center)
> >> {
> >> int scale = 0;
> >>
> >> @@ -106,7 +106,10 @@ static int drm_calc_scale(int src, int dst)
> >> if (dst == 0)
> >> return 0;
> >>
> >> - scale = src / dst;
> >> + if (DIV_ROUND_UP(dst, scale_center) > src)
> > That doesn't look right to me.
> >
> > How about just 'if (src > dst << 16)' ?
> >
> > I guess if we want to keep this code independent of the number of
> > fractional bits we could pass the 16 in from the caller(s). Not sure
> > there's much point in that though.
> pass a bool round_up to the calc_scale functions?
That would mean duplicating the "which way we round?" logic in all
the callers, which doesn't seem ideal. I'd probably just go with the
hardcoded 16 fractional bits assumption. We can always come up
with something else later if the need arises.
> >> + return DIV_ROUND_UP(src, dst);
> >> + else
> >> + scale = src / dst;
> >>
> >> return scale;
> >> }
> >> @@ -116,21 +119,25 @@ static int drm_calc_scale(int src, int dst)
> >> * @src: source window rectangle
> >> * @dst: destination window rectangle
> >> * @min_hscale: minimum allowed horizontal scaling factor
> >> + * @mid_hscale: mid point, below this point round down scaling, above round up.
> >> * @max_hscale: maximum allowed horizontal scaling factor
> >> *
> >> * Calculate the horizontal scaling factor as
> >> * (@src width) / (@dst width).
> >> *
> >> + * If the scale is below @mid_hscale we round down, if above up. This will
> >> + * calculate the hscale with the most pessimistic limit calculation.
> >> + *
> >> * RETURNS:
> >> * The horizontal scaling factor, or errno of out of limits.
> >> */
> >> int drm_rect_calc_hscale(const struct drm_rect *src,
> >> const struct drm_rect *dst,
> >> - int min_hscale, int max_hscale)
> >> + int min_hscale, int mid_hscale, int max_hscale)
> >> {
> >> int src_w = drm_rect_width(src);
> >> int dst_w = drm_rect_width(dst);
> >> - int hscale = drm_calc_scale(src_w, dst_w);
> >> + int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
> >>
> >> if (hscale < 0 || dst_w == 0)
> >> return hscale;
> >> @@ -147,21 +154,25 @@ EXPORT_SYMBOL(drm_rect_calc_hscale);
> >> * @src: source window rectangle
> >> * @dst: destination window rectangle
> >> * @min_vscale: minimum allowed vertical scaling factor
> >> + * @mid_vscale: mid point, below this point round down scaling, above round up.
> >> * @max_vscale: maximum allowed vertical scaling factor
> >> *
> >> * Calculate the vertical scaling factor as
> >> * (@src height) / (@dst height).
> >> *
> >> + * If the scale is below @mid_vscale we round down, if above up. This will
> >> + * calculate the vscale with the most pessimistic limit calculation.
> >> + *
> >> * RETURNS:
> >> * The vertical scaling factor, or errno of out of limits.
> >> */
> >> int drm_rect_calc_vscale(const struct drm_rect *src,
> >> const struct drm_rect *dst,
> >> - int min_vscale, int max_vscale)
> >> + int min_vscale, int mid_vscale, int max_vscale)
> >> {
> >> int src_h = drm_rect_height(src);
> >> int dst_h = drm_rect_height(dst);
> >> - int vscale = drm_calc_scale(src_h, dst_h);
> >> + int vscale = drm_calc_scale(src_h, dst_h, mid_vscale);
> >>
> >> if (vscale < 0 || dst_h == 0)
> >> return vscale;
> >> @@ -178,6 +189,7 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
> >> * @src: source window rectangle
> >> * @dst: destination window rectangle
> >> * @min_hscale: minimum allowed horizontal scaling factor
> >> + * @mid_hscale: mid point, below this point round down scaling, above round up.
> >> * @max_hscale: maximum allowed horizontal scaling factor
> >> *
> >> * Calculate the horizontal scaling factor as
> >> @@ -189,16 +201,19 @@ EXPORT_SYMBOL(drm_rect_calc_vscale);
> >> * If the calculated scaling factor is above @max_vscale,
> >> * decrease the height of rectangle @src to compensate.
> >> *
> >> + * If the scale is below @mid_hscale we round down, if above up. This will
> >> + * calculate the hscale with the most pessimistic limit calculation.
> >> + *
> >> * RETURNS:
> >> * The horizontal scaling factor.
> >> */
> >> int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
> >> struct drm_rect *dst,
> >> - int min_hscale, int max_hscale)
> >> + int min_hscale, int mid_hscale, int max_hscale)
> >> {
> >> int src_w = drm_rect_width(src);
> >> int dst_w = drm_rect_width(dst);
> >> - int hscale = drm_calc_scale(src_w, dst_w);
> >> + int hscale = drm_calc_scale(src_w, dst_w, mid_hscale);
> >>
> >> if (hscale < 0 || dst_w == 0)
> >> return hscale;
> >> @@ -228,6 +243,7 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
> >> * @src: source window rectangle
> >> * @dst: destination window rectangle
> >> * @min_vscale: minimum allowed vertical scaling factor
> >> + * @mid_vscale: mid point, below this point round down scaling, above round up.
> >> * @max_vscale: maximum allowed vertical scaling factor
> >> *
> >> * Calculate the vertical scaling factor as
> >> @@ -239,16 +255,19 @@ EXPORT_SYMBOL(drm_rect_calc_hscale_relaxed);
> >> * If the calculated scaling factor is above @max_vscale,
> >> * decrease the height of rectangle @src to compensate.
> >> *
> >> + * If the scale is below @mid_vscale we round down, if above up. This will
> >> + * calculate the vscale with the most pessimistic limit calculation.
> >> + *
> >> * RETURNS:
> >> * The vertical scaling factor.
> >> */
> >> int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
> >> struct drm_rect *dst,
> >> - int min_vscale, int max_vscale)
> >> + int min_vscale, int mid_vscale, int max_vscale)
> >> {
> >> int src_h = drm_rect_height(src);
> >> int dst_h = drm_rect_height(dst);
> >> - int vscale = drm_calc_scale(src_h, dst_h);
> >> + int vscale = drm_calc_scale(src_h, mid_vscale, dst_h);
> >>
> >> if (vscale < 0 || dst_h == 0)
> >> return vscale;
> >> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> >> index aa1dfaa692b9..f8f7c66fc3ac 100644
> >> --- a/drivers/gpu/drm/i915/intel_sprite.c
> >> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> >> @@ -998,10 +998,10 @@ intel_check_sprite_plane(struct intel_plane *plane,
> >> drm_rect_rotate(src, fb->width << 16, fb->height << 16,
> >> state->base.rotation);
> >>
> >> - hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, max_scale);
> >> + hscale = drm_rect_calc_hscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
> >> BUG_ON(hscale < 0);
> >>
> >> - vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, max_scale);
> >> + vscale = drm_rect_calc_vscale_relaxed(src, dst, min_scale, 1 << 16, max_scale);
> >> BUG_ON(vscale < 0);
> >>
> >> if (crtc_state->base.enable)
> >> @@ -1017,7 +1017,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
> >>
> >> if (state->base.visible) {
> >> /* check again in case clipping clamped the results */
> >> - hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >> + hscale = drm_rect_calc_hscale(src, dst, min_scale, 1 << 16, max_scale);
> >> if (hscale < 0) {
> >> DRM_DEBUG_KMS("Horizontal scaling factor out of limits\n");
> >> drm_rect_debug_print("src: ", src, true);
> >> @@ -1026,7 +1026,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
> >> return hscale;
> >> }
> >>
> >> - vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> >> + vscale = drm_rect_calc_vscale(src, dst, min_scale, 1 << 16, max_scale);
> >> if (vscale < 0) {
> >> DRM_DEBUG_KMS("Vertical scaling factor out of limits\n");
> >> drm_rect_debug_print("src: ", src, true);
> >> diff --git a/include/drm/drm_rect.h b/include/drm/drm_rect.h
> >> index 44bc122b9ee0..dbb8631a9288 100644
> >> --- a/include/drm/drm_rect.h
> >> +++ b/include/drm/drm_rect.h
> >> @@ -179,16 +179,16 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
> >> int hscale, int vscale);
> >> int drm_rect_calc_hscale(const struct drm_rect *src,
> >> const struct drm_rect *dst,
> >> - int min_hscale, int max_hscale);
> >> + int min_hscale, int mid_hscale, int max_hscale);
> >> int drm_rect_calc_vscale(const struct drm_rect *src,
> >> const struct drm_rect *dst,
> >> - int min_vscale, int max_vscale);
> >> + int min_vscale, int mid_vscale, int max_vscale);
> >> int drm_rect_calc_hscale_relaxed(struct drm_rect *src,
> >> struct drm_rect *dst,
> >> - int min_hscale, int max_hscale);
> >> + int min_hscale, int mid_hscale, int max_hscale);
> >> int drm_rect_calc_vscale_relaxed(struct drm_rect *src,
> >> struct drm_rect *dst,
> >> - int min_vscale, int max_vscale);
> >> + int min_vscale, int mid_vscale, int max_vscale);
> >> void drm_rect_debug_print(const char *prefix,
> >> const struct drm_rect *r, bool fixed_point);
> >> void drm_rect_rotate(struct drm_rect *r,
> >> --
> >> 2.17.0
>
--
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
