Re: [Mesa-dev] [PATCH] nvc0: make use of NVC0_CB_AUX_XXX_SIZE as much as possible

Samuel Pitoiset Sat, 28 May 2016 07:15:48 -0700


On 05/27/2016 08:26 PM, Ilia Mirkin wrote:

TBH I don't like this. The way it is now, there's an obvious
correlation between the numbers uploaded, and the for loops/etc which
actually stick the data into the pushbuf. After your change, it's not
at all clear, and should those numbers become disconnected it'll be
difficult to track down.

I'm a bit surprised because it seems like removing such magic numbers isalways a good practice. However, I get the idea of the correlation thingbut I don't think this patch decreases readability. :)


Anyway, I'm going to try to convince you for a cosmetic patch like that.


If you REALLY want to do this, please throw STATIC_ASSERT's all over
the place ensuring that the values are what they are now. But I'd just
as soon leave things as they are now.

  -ilia


On Fri, May 27, 2016 at 4:42 AM, Samuel Pitoiset
<[email protected]> wrote:

This avoids using magic numbers for the driver constant buffer areas
and might also prevent using wrong sizes and offsets.

Signed-off-by: Samuel Pitoiset <[email protected]>
---
 src/gallium/drivers/nouveau/nvc0/nvc0_compute.c        |  2 +-
 src/gallium/drivers/nouveau/nvc0/nvc0_query_hw_sm.c    |  4 ++--
 src/gallium/drivers/nouveau/nvc0/nvc0_screen.c         |  2 +-
 src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c |  6 +++---
 src/gallium/drivers/nouveau/nvc0/nvc0_tex.c            |  2 +-
 src/gallium/drivers/nouveau/nvc0/nve4_compute.c        | 14 +++++++-------
 6 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
index 832c085..7574a95 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_compute.c
@@ -235,7 +235,7 @@ nvc0_compute_validate_buffers(struct nvc0_context *nvc0)
    PUSH_DATA (push, 2048);
    PUSH_DATAh(push, screen->uniform_bo->offset + NVC0_CB_AUX_INFO(s));
    PUSH_DATA (push, screen->uniform_bo->offset + NVC0_CB_AUX_INFO(s));
-   BEGIN_1IC0(push, NVC0_CP(CB_POS), 1 + 4 * NVC0_MAX_BUFFERS);
+   BEGIN_1IC0(push, NVC0_CP(CB_POS), 1 + (NVC0_CB_AUX_BUF_SIZE / 4));
    PUSH_DATA (push, NVC0_CB_AUX_BUF_INFO(0));

    for (i = 0; i < NVC0_MAX_BUFFERS; i++) {
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw_sm.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw_sm.c
index 27cbbc4..bb7fa7f 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw_sm.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_query_hw_sm.c
@@ -1830,7 +1830,7 @@ nvc0_hw_sm_upload_input(struct nvc0_context *nvc0, struct 
nvc0_hw_query *hq)
       PUSH_DATAh(push, address + NVC0_CB_AUX_MP_INFO);
       PUSH_DATA (push, address + NVC0_CB_AUX_MP_INFO);
       BEGIN_NVC0(push, NVE4_CP(UPLOAD_LINE_LENGTH_IN), 2);
-      PUSH_DATA (push, 3 * 4);
+      PUSH_DATA (push, NVC0_CB_AUX_MP_SIZE);
       PUSH_DATA (push, 0x1);
       BEGIN_1IC0(push, NVE4_CP(UPLOAD_EXEC), 1 + 3);
       PUSH_DATA (push, NVE4_COMPUTE_UPLOAD_EXEC_LINEAR | (0x20 << 1));
@@ -1839,7 +1839,7 @@ nvc0_hw_sm_upload_input(struct nvc0_context *nvc0, struct 
nvc0_hw_query *hq)
       PUSH_DATA (push, 2048);
       PUSH_DATAh(push, address);
       PUSH_DATA (push, address);
-      BEGIN_1IC0(push, NVC0_CP(CB_POS), 1 + 3);
+      BEGIN_1IC0(push, NVC0_CP(CB_POS), 1 + (NVC0_CB_AUX_MP_SIZE / 4));
       PUSH_DATA (push, NVC0_CB_AUX_MP_INFO);
    }
    PUSH_DATA (push, (hq->bo->offset + hq->base_offset));
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
index 6541241..de28fb0 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_screen.c
@@ -964,7 +964,7 @@ nvc0_screen_create(struct nouveau_device *dev)
       PUSH_DATA (push, (15 << 4) | 1);
       if (screen->eng3d->oclass >= NVE4_3D_CLASS) {
          unsigned j;
-         BEGIN_1IC0(push, NVC0_3D(CB_POS), 9);
+         BEGIN_1IC0(push, NVC0_3D(CB_POS), 1 + (NVC0_CB_AUX_UNK_SIZE / 4));
          PUSH_DATA (push, NVC0_CB_AUX_UNK_INFO);
          for (j = 0; j < 8; ++j)
             PUSH_DATA(push, j);
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
index a77486d..09f0862 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_state_validate.c
@@ -336,9 +336,9 @@ nvc0_upload_uclip_planes(struct nvc0_context *nvc0, 
unsigned s)
    PUSH_DATA (push, 2048);
    PUSH_DATAh(push, screen->uniform_bo->offset + NVC0_CB_AUX_INFO(s));
    PUSH_DATA (push, screen->uniform_bo->offset + NVC0_CB_AUX_INFO(s));
-   BEGIN_1IC0(push, NVC0_3D(CB_POS), PIPE_MAX_CLIP_PLANES * 4 + 1);
+   BEGIN_1IC0(push, NVC0_3D(CB_POS), 1 + (NVC0_CB_AUX_UCP_SIZE / 4));
    PUSH_DATA (push, NVC0_CB_AUX_UCP_INFO);
-   PUSH_DATAp(push, &nvc0->clip.ucp[0][0], PIPE_MAX_CLIP_PLANES * 4);
+   PUSH_DATAp(push, &nvc0->clip.ucp[0][0], (NVC0_CB_AUX_UCP_SIZE / 4));
 }

 static inline void
@@ -506,7 +506,7 @@ nvc0_validate_buffers(struct nvc0_context *nvc0)
       PUSH_DATA (push, 2048);
       PUSH_DATAh(push, screen->uniform_bo->offset + NVC0_CB_AUX_INFO(s));
       PUSH_DATA (push, screen->uniform_bo->offset + NVC0_CB_AUX_INFO(s));
-      BEGIN_1IC0(push, NVC0_3D(CB_POS), 1 + 4 * NVC0_MAX_BUFFERS);
+      BEGIN_1IC0(push, NVC0_3D(CB_POS), 1 + (NVC0_CB_AUX_BUF_SIZE / 4));
       PUSH_DATA (push, NVC0_CB_AUX_BUF_INFO(0));
       for (i = 0; i < NVC0_MAX_BUFFERS; i++) {
          if (nvc0->buffers[s][i].buffer) {
diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c 
b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
index 14a34d2..6d6fcae 100644
--- a/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
+++ b/src/gallium/drivers/nouveau/nvc0/nvc0_tex.c
@@ -1094,7 +1094,7 @@ nve4_update_surface_bindings(struct nvc0_context *nvc0)
       PUSH_DATA (push, 2048);
       PUSH_DATAh(push, screen->uniform_bo->offset + NVC0_CB_AUX_INFO(s));
       PUSH_DATA (push, screen->uniform_bo->offset + NVC0_CB_AUX_INFO(s));
-      BEGIN_1IC0(push, NVC0_3D(CB_POS), 1 + 16 * NVC0_MAX_IMAGES);
+      BEGIN_1IC0(push, NVC0_3D(CB_POS), 1 + (NVC0_CB_AUX_SU_SIZE / 4));
       PUSH_DATA (push, NVC0_CB_AUX_SU_INFO(0));

       for (i = 0; i < NVC0_MAX_IMAGES; ++i) {
diff --git a/src/gallium/drivers/nouveau/nvc0/nve4_compute.c 
b/src/gallium/drivers/nouveau/nvc0/nve4_compute.c
index 1891e4b..8795dde 100644
--- a/src/gallium/drivers/nouveau/nvc0/nve4_compute.c
+++ b/src/gallium/drivers/nouveau/nvc0/nve4_compute.c
@@ -142,9 +142,9 @@ nve4_screen_compute_setup(struct nvc0_screen *screen,
    PUSH_DATAh(push, address + NVC0_CB_AUX_MS_INFO);
    PUSH_DATA (push, address + NVC0_CB_AUX_MS_INFO);
    BEGIN_NVC0(push, NVE4_CP(UPLOAD_LINE_LENGTH_IN), 2);
-   PUSH_DATA (push, 64);
+   PUSH_DATA (push, NVC0_CB_AUX_MS_SIZE);
    PUSH_DATA (push, 1);
-   BEGIN_1IC0(push, NVE4_CP(UPLOAD_EXEC), 17);
+   BEGIN_1IC0(push, NVE4_CP(UPLOAD_EXEC), 1 + (NVC0_CB_AUX_MS_SIZE / 4));
    PUSH_DATA (push, NVE4_COMPUTE_UPLOAD_EXEC_LINEAR | (0x20 << 1));
    PUSH_DATA (push, 0); /* 0 */
    PUSH_DATA (push, 0);
@@ -204,9 +204,9 @@ nve4_compute_validate_surfaces(struct nvc0_context *nvc0)
    PUSH_DATAh(push, address + NVC0_CB_AUX_SU_INFO(0));
    PUSH_DATA (push, address + NVC0_CB_AUX_SU_INFO(0));
    BEGIN_NVC0(push, NVE4_CP(UPLOAD_LINE_LENGTH_IN), 2);
-   PUSH_DATA (push, 16 * NVC0_MAX_IMAGES * 4);
+   PUSH_DATA (push, NVC0_CB_AUX_SU_SIZE);
    PUSH_DATA (push, 0x1);
-   BEGIN_1IC0(push, NVE4_CP(UPLOAD_EXEC), 1 + 16 * NVC0_MAX_IMAGES);
+   BEGIN_1IC0(push, NVE4_CP(UPLOAD_EXEC), 1 + (NVC0_CB_AUX_SU_SIZE / 4));
    PUSH_DATA (push, NVE4_COMPUTE_UPLOAD_EXEC_LINEAR | (0x20 << 1));

    for (i = 0; i < NVC0_MAX_IMAGES; ++i) {
@@ -426,9 +426,9 @@ nve4_compute_validate_buffers(struct nvc0_context *nvc0)
    PUSH_DATAh(push, address + NVC0_CB_AUX_BUF_INFO(0));
    PUSH_DATA (push, address + NVC0_CB_AUX_BUF_INFO(0));
    BEGIN_NVC0(push, NVE4_CP(UPLOAD_LINE_LENGTH_IN), 2);
-   PUSH_DATA (push, 4 * NVC0_MAX_BUFFERS * 4);
+   PUSH_DATA (push, NVC0_CB_AUX_BUF_SIZE);
    PUSH_DATA (push, 0x1);
-   BEGIN_1IC0(push, NVE4_CP(UPLOAD_EXEC), 1 + 4 * NVC0_MAX_BUFFERS);
+   BEGIN_1IC0(push, NVE4_CP(UPLOAD_EXEC), 1 + (NVC0_CB_AUX_BUF_SIZE / 4));
    PUSH_DATA (push, NVE4_COMPUTE_UPLOAD_EXEC_LINEAR | (0x20 << 1));

    for (i = 0; i < NVC0_MAX_BUFFERS; i++) {
@@ -502,7 +502,7 @@ nve4_compute_upload_input(struct nvc0_context *nvc0,
    PUSH_DATAh(push, address + NVC0_CB_AUX_GRID_INFO);
    PUSH_DATA (push, address + NVC0_CB_AUX_GRID_INFO);
    BEGIN_NVC0(push, NVE4_CP(UPLOAD_LINE_LENGTH_IN), 2);
-   PUSH_DATA (push, 7 * 4);
+   PUSH_DATA (push, NVC0_CB_AUX_GRID_SIZE);
    PUSH_DATA (push, 0x1);

    if (unlikely(info->indirect)) {
--
2.8.3

_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] nvc0: make use of NVC0_CB_AUX_XXX_SIZE as much as possible

Reply via email to