Am 23.11.2014 22:04, schrieb Siarhei Siamashka:
On Fri,  7 Nov 2014 04:54:10 -0500
Andreas Baierl <l...@imkreisrum.de> wrote:

From: Andreas Baierl <ich...@imkreisrum.de>

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)) ||

There has no following calculation or if-then-else be done at all if src_rect.w/h and dst_rect.w/h are zero, because no action has to be performed on zero sized src or dst areas? So catching the zero values at the top with returning 0 would be the better solution, as you mentioned further on.
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?


The above cases should also be catched to avoid doing mixer_blt this areas and return 0?
Signed-off-by: Michal Suchanek <hramr...@gmail.com>
Signed-off-by: Andreas Baierl <ich...@imkreisrum.de>
---
  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.
I agree.
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;
Yes, as mentioned above. What are valid and invalid parameters? In which case we should return 0 or -1?

First of all, we have to ensure, that mixer_blt (and the others) are not called with a para.src_rect.w(h) == 0 because this may cause problems here: https://github.com/linux-sunxi/linux-sunxi/blob/sunxi-3.4/drivers/char/sunxi_g2d/g2d_bsp.c#L759 ?
Correct me, if I'm wrong :p

Regards
Andreas
+                       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;



--
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 linux-sunxi+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to