[Libva] [PATCH 1/2] batch: factor out MI_FLUSH_DW batch buffer construction.

[Gwenole Beauchesne]

Hi,

2014-05-14 10:05 GMT+02:00 Zhao, Yakui <yakui.zhao at intel.com>:
> On Wed, 2014-05-14 at 01:47 -0600, Gwenole Beauchesne wrote:
>> Simplify the construction of the MI_FLUSH_DW command stream. Use ring
>> buffer generic variants of BEGIN, OUT, ADVANCE batch functions.
>
> Hi, Gwenole
>
>     Thanks for your patch.
>     But I don't think that we need factor out of MI_FLUSH_DW batch
> buffer construction.
>    1. Currently it is very lucky that BSD/BLT/VEBOX are using the
> similar MI_FLUSH_DW. But the command definition is different on these
> rings. we are not sure whether we will always use the similar command.
> When some bits are enabled on the corresponding rings, we will have to
> split it again.
>    2. we are considering the redefinition of batch->flag especially for
> the dual VDBOX for BDW.

By clean-up, I meant something that would not cause you to have a
bunch of if()s either. You can use hooks while initializing the batch
buffer for a particular ring. Besides, there are higher order
optimizations possible in the whole intel_batchbuffer.[ch], which
forked off ancient versions that were used in other projects before.

Either way, I don't care but the second patch for behavioural consistency. :)

Thanks,
Gwenole.

>> diff --git a/src/intel_batchbuffer.c b/src/intel_batchbuffer.c
>> index 9dc496d..bc564d5 100644
>> --- a/src/intel_batchbuffer.c
>> +++ b/src/intel_batchbuffer.c
>> @@ -247,29 +247,16 @@ intel_batchbuffer_emit_mi_flush(struct intel_batchbuffer *batch)
>>              }
>>
>>          } else {
>> -            if (batch->flag == I915_EXEC_BLT) {
>> -                BEGIN_BLT_BATCH(batch, 4);
>> -                OUT_BLT_BATCH(batch, MI_FLUSH_DW);
>> -                OUT_BLT_BATCH(batch, 0);
>> -                OUT_BLT_BATCH(batch, 0);
>> -                OUT_BLT_BATCH(batch, 0);
>> -                ADVANCE_BLT_BATCH(batch);
>> -            }else if (batch->flag == I915_EXEC_VEBOX) {
>> -                BEGIN_VEB_BATCH(batch, 4);
>> -                OUT_VEB_BATCH(batch, MI_FLUSH_DW);
>> -                OUT_VEB_BATCH(batch, 0);
>> -                OUT_VEB_BATCH(batch, 0);
>> -                OUT_VEB_BATCH(batch, 0);
>> -                ADVANCE_VEB_BATCH(batch);
>> -            } else {
>> -                assert(batch->flag == I915_EXEC_BSD);
>> -                BEGIN_BCS_BATCH(batch, 4);
>> -                OUT_BCS_BATCH(batch, MI_FLUSH_DW | MI_FLUSH_DW_VIDEO_PIPELINE_CACHE_INVALIDATE);
>> -                OUT_BCS_BATCH(batch, 0);
>> -                OUT_BCS_BATCH(batch, 0);
>> -                OUT_BCS_BATCH(batch, 0);
>> -                ADVANCE_BCS_BATCH(batch);
>> -            }
>> +            uint32_t cmd = MI_FLUSH_DW;
>> +            if (batch->flag == I915_EXEC_BSD)
>> +                cmd |= MI_FLUSH_DW_VIDEO_PIPELINE_CACHE_INVALIDATE;
>> +
>> +            __BEGIN_BATCH(batch, 4, batch->flag);
>> +            __OUT_BATCH(batch, cmd);
>> +            __OUT_BATCH(batch, 0);
>> +            __OUT_BATCH(batch, 0);
>> +            __OUT_BATCH(batch, 0);
>> +            __ADVANCE_BATCH(batch);
>>          }
>>      } else {
>>          if (batch->flag == I915_EXEC_RENDER) {
>
>