[Mesa-dev] [Nouveau] [PATCH] nv50, nvc0: don't base decisions on available pushbuf space
Samuel Pitoiset
samuel.pitoiset at gmail.com
Sat Oct 10 12:55:33 PDT 2015
On 10/10/2015 09:42 PM, Ilia Mirkin wrote:
> On Sat, Oct 10, 2015 at 3:41 PM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> This patch looks fine except that it should be a bit more normalized. I
>> mean, sometimes you break when PUSH_SPACE fails, sometimes not. Same for
>> PUSH_SPACE calls, sometimes you add it sometimes not.
> Meh. We need to get our error checking situation straight, but this
> isn't the patch to do it in.
Yeah, but this needs to be clarified.
>
>> Did you run a full piglit test this time ? :)
> Nope, but I ran a full piglit before this patch. Almost took down my
> box. Probably won't be running it again for this patch.
Ok, I'll run a full piglit this night then.
>
>> See my comment below.
>>
>>
>> On 10/10/2015 11:09 AM, Ilia Mirkin wrote:
>>> We still have to push everything out, might as well kick earlier and
>>> flip pushbufs when we know we'll need it. This resolves some issues with
>>> the new policy of making sure that we always leave a bit of room at the
>>> end for fences.
>>>
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> Cc: mesa-stable at lists.freedesktop.org
>>> ---
>>> src/gallium/drivers/nouveau/nv50/nv50_shader_state.c | 9 ++-------
>>> src/gallium/drivers/nouveau/nv50/nv50_transfer.c | 16
>>> +++-------------
>>> src/gallium/drivers/nouveau/nvc0/nvc0_transfer.c | 20
>>> +++++---------------
>>> 3 files changed, 10 insertions(+), 35 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
>>> b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
>>> index fdde11f..941555f 100644
>>> --- a/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
>>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_shader_state.c
>>> @@ -65,14 +65,9 @@ nv50_constbufs_validate(struct nv50_context *nv50)
>>> PUSH_DATA (push, (b << 12) | (i << 8) | p | 1);
>>> }
>>> while (words) {
>>> - unsigned nr;
>>> -
>>> - if (!PUSH_SPACE(push, 16))
>>> - break;
>>> - nr = PUSH_AVAIL(push);
>>> - assert(nr >= 16);
>>> - nr = MIN2(MIN2(nr - 3, words), NV04_PFIFO_MAX_PACKET_LEN);
>>> + unsigned nr = MIN2(words, NV04_PFIFO_MAX_PACKET_LEN);
>>> + PUSH_SPACE(push, nr + 3);
>>
>> This PUSH_SPACE call doesn't seem to be needed for me because
>> NV50_PUSH_EXPLICIT_SPACE_CHECKING is not set and the following BEGIN_XXX
>> calls will allocate space.
> I want to ensure that both of the below commands are in the same
> batch. Not sure if it's necessary, but... don't want to find out. They
> were in the same batch before. And this batch stuff is what was
> causing the M2MF errors I was seeing earlier.
>
>>
>>> BEGIN_NV04(push, NV50_3D(CB_ADDR), 1);
>>> PUSH_DATA (push, (start << 8) | b);
>>> BEGIN_NI04(push, NV50_3D(CB_DATA(0)), nr);
>>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_transfer.c
>>> b/src/gallium/drivers/nouveau/nv50/nv50_transfer.c
>>> index be51407..9a3fd1e 100644
>>> --- a/src/gallium/drivers/nouveau/nv50/nv50_transfer.c
>>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_transfer.c
>>> @@ -187,14 +187,7 @@ nv50_sifc_linear_u8(struct nouveau_context *nv,
>>> PUSH_DATA (push, 0);
>>> while (count) {
>>> - unsigned nr;
>>> -
>>> - if (!PUSH_SPACE(push, 16))
>>> - break;
>>> - nr = PUSH_AVAIL(push);
>>> - assert(nr >= 16);
>>> - nr = MIN2(count, nr - 1);
>>> - nr = MIN2(nr, NV04_PFIFO_MAX_PACKET_LEN);
>>> + unsigned nr = MIN2(count, NV04_PFIFO_MAX_PACKET_LEN);
>>> BEGIN_NI04(push, NV50_2D(SIFC_DATA), nr);
>>> PUSH_DATAp(push, src, nr);
>>> @@ -395,12 +388,9 @@ nv50_cb_push(struct nouveau_context *nv,
>>> nouveau_pushbuf_validate(push);
>>> while (words) {
>>> - unsigned nr;
>>> -
>>> - nr = PUSH_AVAIL(push);
>>> - nr = MIN2(nr - 7, words);
>>> - nr = MIN2(nr, NV04_PFIFO_MAX_PACKET_LEN - 1);
>>> + unsigned nr = MIN2(words, NV04_PFIFO_MAX_PACKET_LEN);
>>> + PUSH_SPACE(push, nr + 7);
>>> BEGIN_NV04(push, NV50_3D(CB_DEF_ADDRESS_HIGH), 3);
>>> PUSH_DATAh(push, bo->offset + base);
>>> PUSH_DATA (push, bo->offset + base);
>>> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_transfer.c
>>> b/src/gallium/drivers/nouveau/nvc0/nvc0_transfer.c
>>> index aaec60a..d459dd6 100644
>>> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_transfer.c
>>> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_transfer.c
>>> @@ -188,14 +188,10 @@ nvc0_m2mf_push_linear(struct nouveau_context *nv,
>>> nouveau_pushbuf_validate(push);
>>> while (count) {
>>> - unsigned nr;
>>> + unsigned nr = MIN2(count, NV04_PFIFO_MAX_PACKET_LEN);
>>> - if (!PUSH_SPACE(push, 16))
>>> + if (!PUSH_SPACE(push, nr + 9))
>>> break;
>>> - nr = PUSH_AVAIL(push);
>>> - assert(nr >= 16);
>>> - nr = MIN2(count, nr - 9);
>>> - nr = MIN2(nr, NV04_PFIFO_MAX_PACKET_LEN);
>>> BEGIN_NVC0(push, NVC0_M2MF(OFFSET_OUT_HIGH), 2);
>>> PUSH_DATAh(push, dst->offset + offset);
>>> @@ -234,14 +230,10 @@ nve4_p2mf_push_linear(struct nouveau_context *nv,
>>> nouveau_pushbuf_validate(push);
>>> while (count) {
>>> - unsigned nr;
>>> + unsigned nr = MIN2(count, (NV04_PFIFO_MAX_PACKET_LEN - 1));
>>> - if (!PUSH_SPACE(push, 16))
>>> + if (!PUSH_SPACE(push, nr + 10))
>>> break;
>>> - nr = PUSH_AVAIL(push);
>>> - assert(nr >= 16);
>>> - nr = MIN2(count, nr - 8);
>>> - nr = MIN2(nr, (NV04_PFIFO_MAX_PACKET_LEN - 1));
>>> BEGIN_NVC0(push, NVE4_P2MF(UPLOAD_DST_ADDRESS_HIGH), 2);
>>> PUSH_DATAh(push, dst->offset + offset);
>>> @@ -571,9 +563,7 @@ nvc0_cb_bo_push(struct nouveau_context *nv,
>>> PUSH_DATA (push, bo->offset + base);
>>> while (words) {
>>> - unsigned nr = PUSH_AVAIL(push);
>>> - nr = MIN2(nr, words);
>>> - nr = MIN2(nr, NV04_PFIFO_MAX_PACKET_LEN - 1);
>>> + unsigned nr = MIN2(words, NV04_PFIFO_MAX_PACKET_LEN - 1);
>>> PUSH_SPACE(push, nr + 2);
>>> PUSH_REFN (push, bo, NOUVEAU_BO_WR | domain);
>>
More information about the mesa-dev
mailing list