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

[Gwenole Beauchesne]

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.?

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



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