[Mesa-dev] [PATCH 2/7] intel: Move BCS_SWCTRL setup to a helper function.

[Eric Anholt]

Kenneth Graunke <kenneth at whitecape.org> writes:

> On 06/05/2013 10:14 AM, Eric Anholt wrote:
>> This will get reused shortly.
>> ---
>>   src/mesa/drivers/dri/intel/intel_blit.c | 70 +++++++++++++++++----------------
>>   1 file changed, 36 insertions(+), 34 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/intel/intel_blit.c b/src/mesa/drivers/dri/intel/intel_blit.c
>> index 1f6ad09..70d6dcc 100644
>> --- a/src/mesa/drivers/dri/intel/intel_blit.c
>> +++ b/src/mesa/drivers/dri/intel/intel_blit.c
>> @@ -86,6 +86,33 @@ br13_for_cpp(int cpp)
>>   }
>>
>>   /**
>> + * Emits the packet for switching the blitter from X to Y tiled or back.
>> + *
>> + * The BATCH_BEGIN_BLT must already be started, since the pairs of this
>> + * command emitted need to be in the same batchbuffer as the blit they're
>> + * wrapping (otherwise, our state might leak out into another context, since
>> + * BCS_SWCTRL is saved as part of the power context, not a render context).
>> + */
>> +static void
>> +set_blitter_tiling(struct intel_context *intel,
>> +                   bool dst_y_tiled, bool src_y_tiled)
>> +{
>> +   assert(intel->gen >= 6);
>> +
>> +   /* Idle the blitter before we update how tiling is interpreted. */
>> +   OUT_BATCH(MI_FLUSH_DW);
>> +   OUT_BATCH(0);
>> +   OUT_BATCH(0);
>> +   OUT_BATCH(0);
>> +
>> +   OUT_BATCH(MI_LOAD_REGISTER_IMM | (3 - 2));
>> +   OUT_BATCH(BCS_SWCTRL);
>> +   OUT_BATCH((BCS_SWCTRL_DST_Y | BCS_SWCTRL_SRC_Y) << 16 |
>> +             (dst_y_tiled ? BCS_SWCTRL_DST_Y : 0) |
>> +             (src_y_tiled ? BCS_SWCTRL_SRC_Y : 0));
>> +}
>> +
>> +/**
>>    * Implements a rectangular block transfer (blit) of pixels between two
>>    * miptrees.
>>    *
>> @@ -204,21 +231,20 @@ intelEmitCopyBlit(struct intel_context *intel,
>>      int dst_y2 = dst_y + h;
>>      int dst_x2 = dst_x + w;
>>      drm_intel_bo *aper_array[3];
>> -   uint32_t bcs_swctrl = 0;
>> +   bool dst_y_tiled = dst_tiling == I915_TILING_Y;
>> +   bool src_y_tiled = src_tiling == I915_TILING_Y;
>>      BATCH_LOCALS;
>>
>>      if (dst_tiling != I915_TILING_NONE) {
>>         if (dst_offset & 4095)
>>   	 return false;
>> -      if (dst_tiling == I915_TILING_Y && intel->gen < 6)
>> -	 return false;
>>      }
>>      if (src_tiling != I915_TILING_NONE) {
>>         if (src_offset & 4095)
>>   	 return false;
>> -      if (src_tiling == I915_TILING_Y && intel->gen < 6)
>> -	 return false;
>>      }
>> +   if ((dst_y_tiled || src_y_tiled) && intel->gen < 6)
>> +      return false;
>>
>>      /* do space check before going any further */
>>      do {
>> @@ -284,16 +310,10 @@ intelEmitCopyBlit(struct intel_context *intel,
>>      if (dst_tiling != I915_TILING_NONE) {
>>         CMD |= XY_DST_TILED;
>>         dst_pitch /= 4;
>> -
>> -      if (dst_tiling == I915_TILING_Y)
>> -         bcs_swctrl |= BCS_SWCTRL_DST_Y;
>>      }
>>      if (src_tiling != I915_TILING_NONE) {
>>         CMD |= XY_SRC_TILED;
>>         src_pitch /= 4;
>> -
>> -      if (src_tiling == I915_TILING_Y)
>> -         bcs_swctrl |= BCS_SWCTRL_SRC_Y;
>>      }
>>   #endif
>>
>> @@ -304,20 +324,10 @@ intelEmitCopyBlit(struct intel_context *intel,
>>      assert(dst_x < dst_x2);
>>      assert(dst_y < dst_y2);
>>
>> -   BEGIN_BATCH_BLT(8 + ((bcs_swctrl != 0) ? 14 : 0));
>> +   BEGIN_BATCH_BLT(8 + ((dst_y_tiled || src_y_tiled) ? 14 : 0));
>
> I would much prefer to see set_blitter_tiling() include 
> BEGIN_BATCH_BLT(7)/ADVANCE_BATCH().  Then this could just be 
> BEGIN_BATCH_BLT(8).  That's much clearer: you can statically see that 
> each BEGIN/ADVANCE block has the right number of OUT_BATCH calls.
>
> Everything will still be within a single batch since it doesn't get 
> flushed until you start emitting render ring packets.

Unless you run out of batch space between the setup and rendering, or
rendering and restore.  In that case, you or somebody else will end up
with bad tiling state, which is why I did it this way and commented that
it was intentional in set_blitter_tiling().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130606/e4223b23/attachment.pgp>