[Mesa-dev] [PATCH 6/7] i965/cs: Implement brw_emit_gpgpu_walker

[Jordan Justen]

On 2015-04-27 19:02:38, Kenneth Graunke wrote:
> On Friday, April 24, 2015 04:33:43 PM Jordan Justen wrote:
> > +   BEGIN_BATCH(dwords);
> > +   OUT_BATCH(GPGPU_WALKER << 16 | (dwords - 2));
> 
> I was going to suggest splitting this into separate Gen8+ and Gen7
> blocks, but now that I look at the code...these two are slightly
> different indirect handling, and the later one is just a DWord of MBZ,
> so...it's not really that different.  I think what you have is fine :)

Hmm. Maybe time to press my luck. :)

In my other 20 patch series
"[PATCH v2 19/20] i965/cs: Upload brw_cs_state"

We discussed this somewhat ugly code:
> +   int dw = 0;
> +   desc[dw++] = brw->cs.base.prog_offset;
> +   if (brw->gen >= 8)
> +      dw++; /* Kernel Start Pointer High */
> +   dw++;
> +   dw++;
> +   desc[dw++] = stage_state->bind_bo_offset;

It turns out it eventually doesn't look quite so pointless to use the
dw var. Later, it would look like:

http://cgit.freedesktop.org/~jljusten/mesa/tree/src/mesa/drivers/dri/i965/brw_cs.cpp?h=cs-27#n392

Basically, the structure is pretty similar, but an extra dword appears
for the high address in gen8.

If it seems cleaner, I wouldn't mind splitting either or both of these
to be initialized in separate paths based on the gen.

Does the link above change your opinion on the other patch?

Thanks for your time,

-Jordan

> > +   uint32_t dwords = brw->gen < 8 ? 11 : 15;
> > +   OUT_BATCH(0);
> > +   if (brw->gen >= 8) {
> > +      OUT_BATCH(0);
> > +      OUT_BATCH(0);
> > +   }
> > +   assert(thread_width_max <= brw->max_cs_threads);
> > +   OUT_BATCH(((simd_size == 8) ? 0 : 1) << 30 |