[Mesa-dev] [PATCH 2/2] nvc0: add support for bindless on maxwell+
Ilia Mirkin
imirkin at alum.mit.edu
Sat Feb 10 17:41:39 UTC 2018
On Sat, Feb 10, 2018 at 12:37 PM, Samuel Pitoiset
<samuel.pitoiset at gmail.com> wrote:
>
>
> On 02/10/2018 06:12 AM, Ilia Mirkin wrote:
>>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>
>> Tested on GM107 with DOW3 and DiRT Rally (in bindless mode). Seems to
>> work about as well as on Kepler, except I got a few ltc errors in DiRT
>> Rally. Unclear if it's related to this patch though. Could be a missing
>> flush somewhere.
>>
>> docs/relnotes/18.1.0.html | 1 +
>> .../nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 32 ++++++--
>> src/gallium/drivers/nouveau/nvc0/nvc0_screen.c | 3 +-
>> src/gallium/drivers/nouveau/nvc0/nvc0_tex.c | 96
>> +++++++++++++++++++++-
>> 4 files changed, 118 insertions(+), 14 deletions(-)
>>
>> diff --git a/docs/relnotes/18.1.0.html b/docs/relnotes/18.1.0.html
>> index b8a0cd0d02c..93cf1f2b4ff 100644
>> --- a/docs/relnotes/18.1.0.html
>> +++ b/docs/relnotes/18.1.0.html
>> @@ -44,6 +44,7 @@ Note: some of the new features are only available with
>> certain drivers.
>> </p>
>> <ul>
>> +<li>GL_ARB_bindless_texture on nvc0/maxwell+</li>
>> <li>GL_EXT_semaphore on radeonsi</li>
>> <li>GL_EXT_semaphore_fd on radeonsi</li>
>> </ul>
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> index ddcae1e5a51..65c5bd7810b 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> @@ -2400,6 +2400,7 @@
>> NVC0LoweringPass::processSurfaceCoordsGM107(TexInstruction *su)
>> const int dim = su->tex.target.getDim();
>> const int arg = dim + (su->tex.target.isArray() ||
>> su->tex.target.isCube());
>> Value *ind = su->getIndirectR();
>> + Value *handle;
>> int pos = 0;
>> bld.setPosition(su, false);
>> @@ -2416,7 +2417,16 @@
>> NVC0LoweringPass::processSurfaceCoordsGM107(TexInstruction *su)
>> assert(pos == 0);
>> break;
>> }
>> - su->setSrc(arg + pos, loadTexHandle(ind, slot + 32));
>> + if (su->tex.bindless)
>> + handle = ind;
>> + else
>> + handle = loadTexHandle(ind, slot + 32);
>> + su->setSrc(arg + pos, handle);
>> +
>> + // The address check doesn't make sense here. The format check could
>> make
>> + // sense but it's a bit of a pain.
>> + if (su->tex.bindless)
>> + return;
>> // prevent read fault when the image is not actually bound
>> CmpInstruction *pred =
>> @@ -2450,18 +2460,22 @@
>> NVC0LoweringPass::handleSurfaceOpGM107(TexInstruction *su)
>> Value *def = su->getDef(0);
>> su->op = OP_SUREDB;
>> - su->setDef(0, bld.getSSA());
>> - bld.setPosition(su, true);
>> + // There may not be a predicate in the bindless case.
>> + if (su->getPredicate()) {
>> + su->setDef(0, bld.getSSA());
>> - // make sure to initialize dst value when the atomic operation is
>> not
>> - // performed
>> - Instruction *mov = bld.mkMov(bld.getSSA(), bld.loadImm(NULL, 0));
>> + bld.setPosition(su, true);
>> - assert(su->cc == CC_NOT_P);
>> - mov->setPredicate(CC_P, su->getPredicate());
>> + // make sure to initialize dst value when the atomic operation
>> is not
>> + // performed
>> + Instruction *mov = bld.mkMov(bld.getSSA(), bld.loadImm(NULL,
>> 0));
>> +
>> + assert(su->cc == CC_NOT_P);
>> + mov->setPredicate(CC_P, su->getPredicate());
>> - bld.mkOp2(OP_UNION, TYPE_U32, def, su->getDef(0),
>> mov->getDef(0));
>> + bld.mkOp2(OP_UNION, TYPE_U32, def, su->getDef(0),
>> mov->getDef(0));
>> + }
>> }
>> }
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>> b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>> index 7ba8086a533..b3d7768aed7 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
>> @@ -266,9 +266,8 @@ nvc0_screen_get_param(struct pipe_screen *pscreen,
>> enum pipe_cap param)
>> return class_3d >= GM200_3D_CLASS;
>> case PIPE_CAP_SEAMLESS_CUBE_MAP_PER_TEXTURE:
>> case PIPE_CAP_TGSI_BALLOT:
>> - return class_3d >= NVE4_3D_CLASS;
>> case PIPE_CAP_BINDLESS_TEXTURE:
>> - return class_3d >= NVE4_3D_CLASS && class_3d < GM107_3D_CLASS;
>> + return class_3d >= NVE4_3D_CLASS;
>> /* unsupported caps */
>> case PIPE_CAP_TGSI_FS_COORD_ORIGIN_LOWER_LEFT:
>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
>> b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
>> index 9e391fe1acf..ce58cced916 100644
>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
>> @@ -1386,15 +1386,105 @@ nve4_make_image_handle_resident(struct
>> pipe_context *pipe, uint64_t handle,
>> }
>> }
>> +static uint64_t
>> +gm107_create_image_handle(struct pipe_context *pipe,
>> + const struct pipe_image_view *view)
>> +{
>> + /* GM107+ use TIC handles to reference images. As such, image handles
>> are
>> + * just the TIC id.
>> + */
>> + struct nvc0_context *nvc0 = nvc0_context(pipe);
>> + struct nouveau_pushbuf *push = nvc0->base.pushbuf;
>> + struct pipe_sampler_view *sview =
>> + gm107_create_texture_view_from_image(pipe, view);
>> + struct nv50_tic_entry *tic = nv50_tic_entry(sview);
>> +
>> + if (tic == NULL)
>> + goto fail;
>> +
>> + tic->bindless = 1;
>> + tic->id = nvc0_screen_tic_alloc(nvc0->screen, tic);
>> + if (tic->id < 0)
>> + goto fail;
>> +
>> + nve4_p2mf_push_linear(&nvc0->base, nvc0->screen->txc, tic->id * 32,
>> + NV_VRAM_DOMAIN(&nvc0->screen->base), 32,
>> + tic->tic);
>> +
>> + IMMED_NVC0(push, NVC0_3D(TIC_FLUSH), 0);
>> +
>> + nvc0->screen->tic.lock[tic->id / 32] |= 1 << (tic->id % 32);
>> +
>> + return 0x100000000ULL | tic->id;
>> +
>> +fail:
>> + if (tic)
>> + FREE(tic);
>
>
> Do we really need the conditional here, I don't think but your call.
FREE(null)? I guess that's OK. I can drop the conditional.
>
>
>> + return 0;
>> +}
>> +
>> +static void
>> +gm107_delete_image_handle(struct pipe_context *pipe, uint64_t handle)
>> +{
>> + struct nvc0_context *nvc0 = nvc0_context(pipe);
>> + int tic = handle & NVE4_TIC_ENTRY_INVALID;
>> + struct nv50_tic_entry *entry = nvc0->screen->tic.entries[tic];
>> + struct pipe_sampler_view *view = &entry->pipe;
>> + assert(entry->bindless == 1);
>> + assert(!view_bound(nvc0, view));
>> + entry->bindless = 0;
>> + nvc0_screen_tic_unlock(nvc0->screen, entry);
>> + pipe_sampler_view_reference(&view, NULL);
>> +}
>> +
>> +static void
>> +gm107_make_image_handle_resident(struct pipe_context *pipe, uint64_t
>> handle,
>> + unsigned access, bool resident)
>> +{
>> + struct nvc0_context *nvc0 = nvc0_context(pipe);
>> +
>> + if (resident) {
>> + struct nvc0_resident *res = calloc(1, sizeof(struct
>> nvc0_resident));
>> + struct nv50_tic_entry *tic =
>> + nvc0->screen->tic.entries[handle & NVE4_TIC_ENTRY_INVALID];
>> + assert(tic);
>> + assert(tic->bindless);
>> +
>> + res->handle = handle;
>> + res->buf = nv04_resource(tic->pipe.texture);
>> + res->flags = (access & 3) << 8;
>
>
> (access & 3) looks useless, gallium should correctly set that flag, maybe
> add an assertion instead?
Yeah, but tomorrow someone will add another flag we don't care about
but that will mess this up. It's converting from one set of flags to
another, so I want to be conservative.
>
> Nice work, looks good to me!
Thanks!
>
>
>> + if (res->buf->base.target == PIPE_BUFFER &&
>> + access & PIPE_IMAGE_ACCESS_WRITE)
>> + util_range_add(&res->buf->valid_buffer_range,
>> + tic->pipe.u.buf.offset,
>> + tic->pipe.u.buf.offset + tic->pipe.u.buf.size);
>> + list_add(&res->list, &nvc0->img_head);
>> + } else {
>> + list_for_each_entry_safe(struct nvc0_resident, pos,
>> &nvc0->img_head, list) {
>> + if (pos->handle == handle) {
>> + list_del(&pos->list);
>> + free(pos);
>> + break;
>> + }
>> + }
>> + }
>> +}
>> +
>> void
>> nvc0_init_bindless_functions(struct pipe_context *pipe) {
>> pipe->create_texture_handle = nve4_create_texture_handle;
>> pipe->delete_texture_handle = nve4_delete_texture_handle;
>> pipe->make_texture_handle_resident =
>> nve4_make_texture_handle_resident;
>> - pipe->create_image_handle = nve4_create_image_handle;
>> - pipe->delete_image_handle = nve4_delete_image_handle;
>> - pipe->make_image_handle_resident = nve4_make_image_handle_resident;
>> + if (nvc0_context(pipe)->screen->base.class_3d < GM107_3D_CLASS) {
>> + pipe->create_image_handle = nve4_create_image_handle;
>> + pipe->delete_image_handle = nve4_delete_image_handle;
>> + pipe->make_image_handle_resident = nve4_make_image_handle_resident;
>> + } else {
>> + pipe->create_image_handle = gm107_create_image_handle;
>> + pipe->delete_image_handle = gm107_delete_image_handle;
>> + pipe->make_image_handle_resident =
>> gm107_make_image_handle_resident;
>> + }
>> }
>>
More information about the mesa-dev
mailing list