Em Fri, 05 Jun 2015 17:10:53 +0200
Hans Verkuil <hverk...@xs4all.nl> escreveu:

> On 06/05/2015 12:29 PM, Mauro Carvalho Chehab wrote:
> > Remove the following compilation warnings:
> > 
> >     drivers/media/platform/vivid/vivid-tpg.c: In function 'tpg_gen_text':
> >     drivers/media/platform/vivid/vivid-tpg.c:1562:1: warning: the frame 
> > size of 1308 bytes is larger than 1024 bytes [-Wframe-larger-than=]
> >      }
> >      ^
> > 
> > This seems to be due to some bad optimization done by gcc.
> > 
> > Moving the for() loop to happen inside the macro solves the
> > issue.
> > 
> > While here, fix CodingStyle at the switch().
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
> > 
> > diff --git a/drivers/media/platform/vivid/vivid-tpg.c 
> > b/drivers/media/platform/vivid/vivid-tpg.c
> > index b1147f2df26c..7a3ed580626a 100644
> > --- a/drivers/media/platform/vivid/vivid-tpg.c
> > +++ b/drivers/media/platform/vivid/vivid-tpg.c
> > @@ -1492,12 +1492,10 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 
> > *basep[TPG_MAX_PLANES][2],
> >     else if (tpg->field == V4L2_FIELD_SEQ_TB || tpg->field == 
> > V4L2_FIELD_SEQ_BT)
> >             div = 2;
> >  
> > -   for (p = 0; p < tpg->planes; p++) {
> > -           unsigned vdiv = tpg->vdownsampling[p];
> > -           unsigned hdiv = tpg->hdownsampling[p];
> > -
> > -           /* Print text */
> > -#define PRINTSTR(PIXTYPE) do {     \
> > +   /* Print text */
> > +#define PRINTSTR(PIXTYPE) for (p = 0; p < tpg->planes; p++) {      \
> > +   unsigned vdiv = tpg->vdownsampling[p];  \
> > +   unsigned hdiv = tpg->hdownsampling[p];  \
> >     PIXTYPE fg;     \
> >     PIXTYPE bg;     \
> >     memcpy(&fg, tpg->textfg[p], sizeof(PIXTYPE));   \
> > @@ -1548,16 +1546,19 @@ void tpg_gen_text(const struct tpg_data *tpg, u8 
> > *basep[TPG_MAX_PLANES][2],
> >     }       \
> >  } while (0)
> >  
> > -           switch (tpg->twopixelsize[p]) {
> > -           case 2:
> > -                   PRINTSTR(u8); break;
> > -           case 4:
> > -                   PRINTSTR(u16); break;
> > -           case 6:
> > -                   PRINTSTR(x24); break;
> > -           case 8:
> > -                   PRINTSTR(u32); break;
> > -           }
> > +   switch (tpg->twopixelsize[p]) {
> 
> This doesn't work I just discovered. Compiling gives this warning:
> 
> drivers/media/platform/vivid/vivid-tpg.c: In function ‘tpg_gen_text’:
> drivers/media/platform/vivid/vivid-tpg.c:1549:27: warning: ‘p’ may be used 
> uninitialized in this function [-Wmaybe-uninitialized]
>   switch (tpg->twopixelsize[p]) {
>                            ^
> 

Weird. here (gcc 5.1.1) I didn't get any warning.

> And the value for tpg->twopixelsize[p] will actually differ depending on the 
> value of p.
> 
> It's probably best to revert this patch.
> 
> The correct approach is likely to make four functions, one for case,
> with the macro as the function body.

I tried that (without the for). The result was worse.

I'll double check it latter today.

> 
> Regards,
> 
>       Hans
> 
> > +   case 2:
> > +           PRINTSTR(u8);
> > +           break;
> > +   case 4:
> > +           PRINTSTR(u16);
> > +           break;
> > +   case 6:
> > +           PRINTSTR(x24);
> > +           break;
> > +   case 8:
> > +           PRINTSTR(u32);
> > +           break;
> >     }
> >  }
> >  
> > 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to