[Intel-gfx] Some questions about dri_bo_emit_reloc

[Eric Anholt]

On Mon, 2008-10-27 at 15:51 -0700, Carl Worth wrote:
> These questions are mostly for Eric, but I missed my chance to ask him
> when he was sitting next to me today, so I figure I might as well ask on
> a public mailing list.
> 
> There's a patch below, but I'm not proposing it for review/commit yet,
> (note that it's just git-diff output---no commit message yet, etc.). I'm
> sure it's wrong in multiple ways, and it's guaranteed to do nothing
> better than wedge your GPU. I'm including it here only for context for
> some of the questions below.
> 
> 1. How do I determine the correct values for the read_domains argument?
> I found the list in i915_drm.h but when I'm emitting the binding_table
> and surface_state objects is that RENDER or SAMPLER? How do I know?

Generally the chipset documentation tells you what caching domain
something is read through.  In the cases that it doesn't, the Mesa
driver should be noting that it's just a guess.

> 2. Similarly, for write_domains I assume I want just 0 as I'm not asking
> the GPU to write anything here?

You'll have I915_GEM_DOMAIN_RENDER on your destination pixmap relocation
(since 3D destination writes go through the render cache just like 2D
reads/writes), but nothing else.

> 3. The order of the arguments to dri_bo_emit_reloc is really confusing.
> The names "delta" and "offset" are confusing too. Amd I reading things
> correctly that what we have here is basically (using "..." in place of
> the read_domains and write_domains flags):
> 
> 	source_bo, ..., target_offset, source_offset, target_bo
> 
> If so, that's hard to remember to get right. Could we consider moving to
> something with ordered pairs of (buffer_object, offset for that bo)?
> Another confusing part of this is that intel_batch_emit_reloc puts the
> target_bo as the first argument while dri_bo_emit_reloc has it as the
> last.

Yeah, I don't like the arguments to this function either.  We're kind of
stuck with dri_bo_emit_reloc at this point since the library's released,
but I'd be interested in any cleanups we could come up with taking that
into account.

> 4. My patch below adds buffer objects for binding_table and
> surface_state but doesn't add them to my check_aperture_space call yet.
> Am I correct in assuming that if I setup the binding table with relocs
> to the surface_state that I only need to add the binding_table buffer
> object to check_aperture_space and not the surface_state buffer object
> as well?

dri_bufmgr_check_aperture_space looks at the space that would be used by
the union of all buffers referenced by what you hand it.  If you hand it
your batch plus what you're going to OUT_RELOC into the batch next, that
should cover everything.

> 5. Since after emitting the batch header I never again need the
> binding_table and surface_state buffer objects, (unlike the
> vertex_buffer), I'm not holding on to them. Instead I'm just using local
> variables with dri_bo_unreference after emitting the binding_table's
> reloc into the batch. Does that seem reasonable/correct?

Seems good to me.

> +	i965_set_picture_surface_state(&ss[2].state,
> +				       pMaskPicture, pMask,
> +				       FALSE);
> +	binding_table[2] = 2 * sizeof (brw_surface_state_padded);
> +	dri_bo_emit_reloc (binding_table_bo, I915_GEM_DOMAIN_RENDER, 0,
> +			   2 * sizeof (brw_surface_state_padded),
> +			   2 * sizeof (uint32_t),
> +			   surface_state_bo);

The fact that you're grouping together

bo_map[i] = delta;
dri_bo_emit_reloc(bo, ..., delta, i * sizeof(uint32_t), target_bo)

might make a nice opportunity for doing an emit_reloc api cleanup.

-- 
Eric Anholt
eric at anholt.net                         eric.anholt at intel.com


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20081028/2dca7319/attachment.sig>