On Thu, Apr 26, 2018 at 10:28:20AM +0200, Maarten Lankhorst wrote:
> No matter how you perform the clip adjustments, a small
> error may push the scaling factor to the other side of
> 0x10000. Solve this with a macro that will fixup the
> scale to 0x10000 if we accidentally wrap to the other side.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
I think this and the previous patch are perfect candidates for in-kernel
selftests. Can I volunteer you to get those started for the kms side? We
already have a drm_mm selftest, but I think splitting things a bit might
be useful.
Or we rename that one and just stuff all the kms tests in there, dunno.
-Daniel
> ---
> drivers/gpu/drm/drm_rect.c | 65 +++++++++++++++++++++++++++-----------
> 1 file changed, 47 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_rect.c b/drivers/gpu/drm/drm_rect.c
> index b179c7c73cc5..71b6b7f5d58f 100644
> --- a/drivers/gpu/drm/drm_rect.c
> +++ b/drivers/gpu/drm/drm_rect.c
> @@ -50,6 +50,24 @@ bool drm_rect_intersect(struct drm_rect *r1, const struct drm_rect *r2)
> }
> EXPORT_SYMBOL(drm_rect_intersect);
>
> +static int drm_calc_scale(int src, int dst)
> +{
> + int scale = 0;
> +
> + if (WARN_ON(src < 0 || dst < 0))
> + return -EINVAL;
> +
> + if (dst == 0)
> + return 0;
> +
> + if (src > (dst << 16))
> + return DIV_ROUND_UP(src, dst);
> + else
> + scale = src / dst;
> +
> + return scale;
> +}
> +
> /**
> * drm_rect_clip_scaled - perform a scaled clip operation
> * @src: source window rectangle
> @@ -71,49 +89,60 @@ bool drm_rect_clip_scaled(struct drm_rect *src, struct drm_rect *dst,
> {
> int diff;
>
> + /*
> + * When scale is near 0x10000 rounding errors may cause the scaling
> + * factor to the other side. Some hardware may support
> + * upsampling, but not downsampling, and that would break when
> + * rounding.
> + */
> +#define FIXUP(oldscale, fn, m, second) do { \
> + if (oldscale != 1 << 16) { \
> + int newscale = drm_calc_scale(fn(src), fn(dst)); \
> + \
> + if (newscale < 0) \
> + return false; \
> + \
> + if ((oldscale < 0x10000) != (newscale < 0x10000)) { \
> + if (!second) \
> + src->m##1 = src->m##2 - ((fn(dst) - diff) << 16); \
> + else \
> + src->m##2 = src->m##1 + ((fn(dst) - diff) << 16); \
> + } \
> + } \
> + } while (0)
> +
> diff = clip->x1 - dst->x1;
> if (diff > 0) {
> int64_t tmp = src->x1 + (int64_t) diff * hscale;
> src->x1 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX);
> + FIXUP(hscale, drm_rect_width, x, 0);
> }
> +
> diff = clip->y1 - dst->y1;
> if (diff > 0) {
> int64_t tmp = src->y1 + (int64_t) diff * vscale;
> src->y1 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX);
> + FIXUP(vscale, drm_rect_height, y, 0);
> }
> +
> diff = dst->x2 - clip->x2;
> if (diff > 0) {
> int64_t tmp = src->x2 - (int64_t) diff * hscale;
> src->x2 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX);
> + FIXUP(hscale, drm_rect_width, x, 1);
> }
> diff = dst->y2 - clip->y2;
> if (diff > 0) {
> int64_t tmp = src->y2 - (int64_t) diff * vscale;
> src->y2 = clamp_t(int64_t, tmp, INT_MIN, INT_MAX);
> + FIXUP(vscale, drm_rect_height, y, 1);
> }
> +#undef FIXUP
>
> return drm_rect_intersect(dst, clip);
> }
> EXPORT_SYMBOL(drm_rect_clip_scaled);
>
> -static int drm_calc_scale(int src, int dst)
> -{
> - int scale = 0;
> -
> - if (WARN_ON(src < 0 || dst < 0))
> - return -EINVAL;
> -
> - if (dst == 0)
> - return 0;
> -
> - if (src > (dst << 16))
> - return DIV_ROUND_UP(src, dst);
> - else
> - scale = src / dst;
> -
> - return scale;
> -}
> -
> /**
> * drm_rect_calc_hscale - calculate the horizontal scaling factor
> * @src: source window rectangle
> --
> 2.17.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
