On Fri,  7 Nov 2014 04:54:10 -0500
Andreas Baierl <[email protected]> wrote:

> From: Andreas Baierl <[email protected]>
> 
> Zero area blits are technically valid noops and are requested bay
> libvdpau. Return 0 when blit area is zero without performing bogus
> calculations.
> 
> This reverts commit 3303e27 but also catches the zero values
> which were leading to failed calculations.

What kind of failed calculations? Do you mean the suspicious
checks like this:

> +    if(((para->src_rect.x < 0)&&((-para->src_rect.x) > para->src_rect.w)) ||

Which do not catch a special case with negative para->src_rect.x where
(-para->src_rect.x == para->src_rect.w) and this causes troubles
further in the function? Or something else?


> Signed-off-by: Michal Suchanek <[email protected]>
> Signed-off-by: Andreas Baierl <[email protected]>
> ---
>  drivers/char/sunxi_g2d/g2d.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/char/sunxi_g2d/g2d.c b/drivers/char/sunxi_g2d/g2d.c
> index 288685a..085ace3 100644
> --- a/drivers/char/sunxi_g2d/g2d.c
> +++ b/drivers/char/sunxi_g2d/g2d.c
> @@ -138,8 +138,7 @@ int g2d_blit(g2d_blt * para)
>       __s32 err = 0;
>  
>       /* check the parameter valid */
> -    if(para->src_rect.w == 0 || para->src_rect.h == 0 ||
> -       ((para->src_rect.x < 0)&&((-para->src_rect.x) > para->src_rect.w)) ||
> +    if(((para->src_rect.x < 0)&&((-para->src_rect.x) > para->src_rect.w)) ||
>         ((para->src_rect.y < 0)&&((-para->src_rect.y) > para->src_rect.h)) ||
>         ((para->dst_x < 0)&&((-para->dst_x) > para->src_rect.w)) ||
>         ((para->dst_y < 0)&&((-para->dst_y) > para->src_rect.h)) ||
> @@ -153,6 +152,12 @@ int g2d_blit(g2d_blt * para)
>       }
>       else
>       {
> +             if((para->dst_rect.w == 0) || (para->dst_rect.h == 0) ||
> +                (para->src_rect.w == 0) || (para->src_rect.h == 0))
> +             {
> +                     printk(KERN_DEBUG "User requested g2d blit on zero 
> region\n");

If zero area blits are technically valid and really used, then spamming
the dmesg log is not a really great idea. It may lead to a severe
performance problems.

Wouldn't an early check and return 0 (success) be a much better fix?
Maybe something like this:

        if (para->src_rect.w == 0 || para->src_rect.h == 0)
                return 0;

> +                     return err;
> +             }
>               if(((para->src_rect.x < 0)&&((-para->src_rect.x) < 
> para->src_rect.w)))
>               {
>                       para->src_rect.w = para->src_rect.w + para->src_rect.x;
> @@ -205,8 +210,7 @@ int g2d_fill(g2d_fillrect * para)
>       __s32 err = 0;
>  
>       /* check the parameter valid */
> -     if(para->dst_rect.w == 0 || para->dst_rect.h == 0 ||
> -        ((para->dst_rect.x < 0)&&((-para->dst_rect.x)>para->dst_rect.w)) ||
> +     if(((para->dst_rect.x < 0)&&((-para->dst_rect.x)>para->dst_rect.w)) ||
>          ((para->dst_rect.y < 0)&&((-para->dst_rect.y)>para->dst_rect.h)) ||
>          ((para->dst_rect.x > 0)&&(para->dst_rect.x > para->dst_image.w - 1)) 
> ||
>          ((para->dst_rect.y > 0)&&(para->dst_rect.y > para->dst_image.h - 1)))
> @@ -216,6 +220,11 @@ int g2d_fill(g2d_fillrect * para)
>       }
>       else
>       {
> +             if((para->dst_rect.w == 0) || (para->dst_rect.h == 0))
> +             {
> +                     printk(KERN_DEBUG "User requested g2d fill on zero 
> region\n");
> +                     return err;
> +             }
>               if(((para->dst_rect.x < 0)&&((-para->dst_rect.x) < 
> para->dst_rect.w)))
>               {
>                       para->dst_rect.w = para->dst_rect.w + para->dst_rect.x;
> @@ -247,9 +256,7 @@ int g2d_stretchblit(g2d_stretchblt * para)
>       __s32 err = 0;
>  
>       /* check the parameter valid */
> -    if(para->src_rect.w == 0 || para->src_rect.h == 0 ||
> -       para->dst_rect.w == 0 || para->dst_rect.h == 0 ||
> -       ((para->src_rect.x < 0)&&((-para->src_rect.x) > para->src_rect.w)) ||
> +    if(((para->src_rect.x < 0)&&((-para->src_rect.x) > para->src_rect.w)) ||
>         ((para->src_rect.y < 0)&&((-para->src_rect.y) > para->src_rect.h)) ||
>         ((para->dst_rect.x < 0)&&((-para->dst_rect.x) > para->dst_rect.w)) ||
>         ((para->dst_rect.y < 0)&&((-para->dst_rect.y) > para->dst_rect.h)) ||
> @@ -263,6 +270,12 @@ int g2d_stretchblit(g2d_stretchblt * para)
>       }
>       else
>       {
> +             if((para->dst_rect.w == 0) || (para->dst_rect.h == 0) ||
> +                (para->src_rect.w == 0) || (para->src_rect.h == 0))
> +             {
> +                     printk(KERN_DEBUG "User requested g2d stretchblit on 
> zero region\n");
> +                     return err;
> +             }
>               if(((para->src_rect.x < 0)&&((-para->src_rect.x) < 
> para->src_rect.w)))
>               {
>                       para->src_rect.w = para->src_rect.w + para->src_rect.x;



-- 
Best regards,
Siarhei Siamashka

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to