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.z...@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.beauche...@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,
> >
> >
>
>
>
_______________________________________________
Libva mailing list
Libva@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libva
