[Libva] [PATCH intel-driver 01/13] vebox: silence compilation warning.

[Zhao, Yakui]

On Thu, 2014-08-28 at 02:35 -0600, Gwenole Beauchesne wrote:
> Hi Yakui,
> 
> 2014-08-28 3:16 GMT+02:00 Zhao, Yakui <yakui.zhao at intel.com>:
> > On Wed, 2014-08-27 at 05:50 -0600, Gwenole Beauchesne wrote:
> >> Silence the following compilation warning:
> >>   CC     i965_drv_video_la-gen75_vpp_vebox.lo
> >> gen75_vpp_vebox.c: In function 'bdw_veb_dndi_iecp_command':
> >> gen75_vpp_vebox.c:1537:5: warning: suggest parentheses around arithmetic in operand of '|' [-Wparentheses]
> >>
> >> Also simplify the calculation of the VEB_DI_IECP::endingX variable
> >> with existing helper macros.
> >>
> >> Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne at intel.com>
> >> ---
> >>  src/gen75_vpp_vebox.c |   16 ++++++----------
> >>  1 file changed, 6 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/src/gen75_vpp_vebox.c b/src/gen75_vpp_vebox.c
> >> index 1113c90..f452e67 100644
> >> --- a/src/gen75_vpp_vebox.c
> >> +++ b/src/gen75_vpp_vebox.c
> >> @@ -805,8 +805,8 @@ void hsw_veb_dndi_iecp_command(VADriverContextP ctx, struct intel_vebox_context
> >>  {
> >>      struct intel_batchbuffer *batch = proc_ctx->batch;
> >>      unsigned char frame_ctrl_bits = 0;
> >> -    unsigned int startingX = 0;
> >> -    unsigned int endingX = (proc_ctx->width_input + 63 ) / 64 * 64;
> >> +    const unsigned int startingX = 0;
> >> +    const unsigned int endingX = ALIGN(proc_ctx->width_input, 64) - 1;
> >>
> >>      /* s1:update the previous and current input */
> >>  /*    tempFrame = proc_ctx->frame_store[FRAME_IN_PREVIOUS];
> >> @@ -829,9 +829,7 @@ void hsw_veb_dndi_iecp_command(VADriverContextP ctx, struct intel_vebox_context
> >>      /*s3:set reloc buffer address */
> >>      BEGIN_VEB_BATCH(batch, 10);
> >>      OUT_VEB_BATCH(batch, VEB_DNDI_IECP_STATE | (10 - 2));
> >> -    OUT_VEB_BATCH(batch,
> >> -                  startingX << 16 |
> >> -                  (endingX-1));
> >> +    OUT_VEB_BATCH(batch, (startingX << 16) | endingX);
> >
> > Hi, Gwenole
> >
> >       I would like to add the parentheses around arithmetic in operand
> > of '|' to fix the silence compilation warning.
> >       In fact the (endingX -1 ) in OUT_VEB_BATCH command can easily
> > illustrate the hardware setting.
> 
> Well, the HW field is named endingX. If I create a variable endingX
> and it turns out to be expected_endingX + 1, this is not going to be
> consistent. :)
> 
> So, either (i) we keep the proposed patch as is, i.e. endingX that
> exactly represents the expected value to match the field of same name,
> or (ii) we rename that to "something" and use ("something" - 1) as you
> mention. I don't mind either way, but definitely "something" should
> not be endingX. Thanks
> 
> How about aligned_width, width64, etc.?

I prefer the second proposal which uses the ("something" -1 ).
The aligned_width/width64 is OK to me.

Thanks.
   Yakui
> 
> Regards,
> Gwenole.
> 
> >>      OUT_RELOC(batch,
> >>                proc_ctx->frame_store[FRAME_IN_CURRENT].obj_surface->bo,
> >>                I915_GEM_DOMAIN_RENDER, 0, frame_ctrl_bits);
> >> @@ -1532,14 +1530,12 @@ void bdw_veb_dndi_iecp_command(VADriverContextP ctx, struct intel_vebox_context
> >>  {
> >>      struct intel_batchbuffer *batch = proc_ctx->batch;
> >>      unsigned char frame_ctrl_bits = 0;
> >> -    unsigned int startingX = 0;
> >> -    unsigned int endingX = (proc_ctx->width_input + 63 ) / 64 * 64;
> >> +    const unsigned int startingX = 0;
> >> +    const unsigned int endingX = ALIGN(proc_ctx->width_input, 64) - 1;
> >>
> >>      BEGIN_VEB_BATCH(batch, 0x14);
> >>      OUT_VEB_BATCH(batch, VEB_DNDI_IECP_STATE | (0x14 - 2));//DWord 0
> >> -    OUT_VEB_BATCH(batch,
> >> -                  startingX << 16 |
> >> -                  endingX -1);//DWord 1
> >> +    OUT_VEB_BATCH(batch, (startingX << 16) | endingX);
> >>
> >>      OUT_RELOC(batch,
> >>                proc_ctx->frame_store[FRAME_IN_CURRENT].obj_surface->bo,
> >
> >
> 
> 
>