[Mesa-dev] [PATCH 2/2] nvc0: add support for bindless on maxwell+
Samuel Pitoiset
samuel.pitoiset at gmail.com
Sat Feb 10 17:44:09 UTC 2018
On 02/10/2018 06:41 PM, Ilia Mirkin wrote:
> 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.
Good point!
>
>>
>> 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