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

[Gwenole Beauchesne]

Hi,

2014-08-29 2:44 GMT+02:00 Zhao, Yakui <yakui.zhao at intel.com>:
> 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.

Done locally.

    v2: renamed endingX to width64, which represents the width aligned
      on to 64 columns, dropped obsolete startingX [Yakui]

Any more comments for other patches?

Thanks,
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,
>> >
>> >
>>
>>
>>
>
>



-- 
Gwenole Beauchesne
Intel Corporation SAS / 2 rue de Paris, 92196 Meudon Cedex, France
Registration Number (RCS): Nanterre B 302 456 199