[igt-dev] [PATCH i-g-t v14 1/4] lib/intel_batchbuffer: Extend intel_bb

[Chris Wilson]

Quoting Zbigniew Kempczyński (2020-07-02 06:42:40)
> @@ -1361,7 +1487,9 @@ intel_bb_add_object(struct intel_bb *ibb, uint32_t handle,
>                 object = *found;
>  
>         /* Assign address once */
> -       if (object->offset == 0) {
> +       if (ibb->enforce_relocs)
> +               object->offset = 0;
> +       else if (object->offset == 0) {
>                 if (offset) {
>                         object->offset = offset;
>                 } else {

Hmm, I didn't spot this last time, but offset == 0 is a valid address.
Safer to use #define INVALID_OFFSET -1ull

> @@ -1606,17 +1785,32 @@ int __intel_bb_exec(struct intel_bb *ibb, uint32_t end_offset,
>         execbuf.buffer_count = ibb->num_objects;
>         execbuf.batch_len = end_offset;
>         execbuf.rsvd1 = ibb->ctx = ctx;
> -       execbuf.flags = flags | I915_EXEC_BATCH_FIRST;
> +       execbuf.flags = flags | I915_EXEC_BATCH_FIRST | I915_EXEC_FENCE_OUT;
> +       execbuf.rsvd2 = 0;
>  
> -       ret = __gem_execbuf(ibb->i915, &execbuf);
> -       if (ret)
> +       ret = __gem_execbuf_wr(ibb->i915, &execbuf);
> +       if (ret) {
> +               intel_bb_dump_execbuf(ibb, &execbuf);
>                 return ret;
> +       }
> +
> +       /* Save/merge fences */
> +       fence = execbuf.rsvd2 >> 32;
> +
> +       if (ibb->fence < 0) {
> +               ibb->fence = fence;
> +       } else {
> +               new_fence = sync_fence_merge(ibb->fence, fence);
> +               close(ibb->fence);
> +               close(fence);
> +               ibb->fence = new_fence;
> +       }

One day, we'll have a not so messy version :)
Fencing looks correct.

> +void intel_bb_blit_start(struct intel_bb *ibb, uint32_t flags)
> +{
> +       intel_bb_out(ibb, XY_SRC_COPY_BLT_CMD |
> +                    XY_SRC_COPY_BLT_WRITE_ALPHA |
> +                    XY_SRC_COPY_BLT_WRITE_RGB |
> +                    (flags) |
> +                    (6 + 2*(ibb->gen >= 8)));
> +}
> +
> +void intel_bb_emit_blt_copy(struct intel_bb *ibb,
> +                           struct intel_buf *src,
> +                           int src_x1, int src_y1, int src_pitch,
> +                           struct intel_buf *dst,
> +                           int dst_x1, int dst_y1, int dst_pitch,
> +                           int width, int height, int bpp)
> +{
> +       const int gen = ibb->gen;
> +       uint32_t cmd_bits = 0;
> +       uint32_t br13_bits;
> +
> +       igt_assert(bpp*(src_x1 + width) <= 8*src_pitch);
> +       igt_assert(bpp*(dst_x1 + width) <= 8*dst_pitch);
> +       igt_assert(src_pitch * (src_y1 + height) <= src->size);
> +       igt_assert(dst_pitch * (dst_y1 + height) <= dst->size);
> +
> +       if (gen >= 4 && src->tiling != I915_TILING_NONE) {
> +               src_pitch /= 4;
> +               cmd_bits |= XY_SRC_COPY_BLT_SRC_TILED;
> +       }
> +
> +       if (gen >= 4 && dst->tiling != I915_TILING_NONE) {
> +               dst_pitch /= 4;
> +               cmd_bits |= XY_SRC_COPY_BLT_DST_TILED;
> +       }
> +
> +       CHECK_RANGE(src_x1); CHECK_RANGE(src_y1);
> +       CHECK_RANGE(dst_x1); CHECK_RANGE(dst_y1);
> +       CHECK_RANGE(width); CHECK_RANGE(height);
> +       CHECK_RANGE(src_x1 + width); CHECK_RANGE(src_y1 + height);
> +       CHECK_RANGE(dst_x1 + width); CHECK_RANGE(dst_y1 + height);
> +       CHECK_RANGE(src_pitch); CHECK_RANGE(dst_pitch);
> +
> +       br13_bits = 0;
> +       switch (bpp) {
> +       case 8:
> +               break;
> +       case 16:                /* supporting only RGB565, not ARGB1555 */
> +               br13_bits |= 1 << 24;
> +               break;
> +       case 32:
> +               br13_bits |= 3 << 24;
> +               cmd_bits |= XY_SRC_COPY_BLT_WRITE_ALPHA |
> +                           XY_SRC_COPY_BLT_WRITE_RGB;
> +               break;
> +       default:
> +               igt_fail(IGT_EXIT_FAILURE);
> +       }

Hmm, I thought this version was going to the BCS_SWCTL for gen6+?

> +
> +       intel_bb_blit_start(ibb, cmd_bits);
> +       intel_bb_out(ibb, (br13_bits) |
> +                 (0xcc << 16) | /* copy ROP */
> +                 dst_pitch);
> +       intel_bb_out(ibb, (dst_y1 << 16) | dst_x1); /* dst x1,y1 */
> +       intel_bb_out(ibb, ((dst_y1 + height) << 16) | (dst_x1 + width)); /* dst x2,y2 */
> +       intel_bb_emit_reloc_fenced(ibb, dst->handle,
> +                                  I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
> +                                  0, dst->addr.offset);
> +       intel_bb_out(ibb, (src_y1 << 16) | src_x1); /* src x1,y1 */
> +       intel_bb_out(ibb, src_pitch);
> +       intel_bb_emit_reloc_fenced(ibb, src->handle,
> +                                  I915_GEM_DOMAIN_RENDER, 0,
> +                                  0, src->addr.offset);
> +
> +       if (gen >= 6 && src->handle == dst->handle) {
> +               intel_bb_out(ibb, XY_SETUP_CLIP_BLT_CMD);
> +               intel_bb_out(ibb, 0);
> +               intel_bb_out(ibb, 0);
> +       }
> +
> +}

-Chris