[Intel-gfx] [PATCH 1/2] intel: Add the ability to supply annotations for .aub files.

[Paul Berry]

On 9 May 2012 20:07, Eric Anholt <eric at anholt.net> wrote:

> On Mon,  7 May 2012 14:31:51 -0700, Paul Berry <stereotype441 at gmail.com>
> wrote:
> > This patch adds a new function,
> > drm_intel_bufmgr_gem_set_aub_annotations(), which can be used to
> > annotate the type and subtype of data stored in various sections of
> > each buffer.  This data is used to populate type and subtype fields
> > when generating the .aub file, which improves the ability of later
> > debugging tools to analyze the contents of the .aub file.
> >
> > If drm_intel_bufmgr_gem_set_aub_annotations() is not called, then we
> > fall back to the old set of annotations (annotate the portion of the
> > batchbuffer that is executed as AUB_TRACE_TYPE_BATCH, and everything
> > else as AUB_TRACE_TYPE_NOTYPE).
>
> This looks better than the interface I was thinking of.  Only real
> nitpick note is that the style in this file is tab indents, rather than
> 8 spaces, same as linux kernel and 2d driver style.  (I have since been
> convinced that 8 spaces is superior, but a mix is worse).
>

Ok.  Are there any plans to switch drm over to spaces or are we stuck with
tabs forever?


>
> >  /*
> > @@ -1989,23 +2027,31 @@ aub_exec(drm_intel_bo *bo, int ring_flag, int
> used)
> >       drm_intel_bufmgr_gem *bufmgr_gem = (drm_intel_bufmgr_gem *)
> bo->bufmgr;
> >       drm_intel_bo_gem *bo_gem = (drm_intel_bo_gem *) bo;
> >       int i;
> > +        bool batch_buffer_needs_annotations;
> >
> >       if (!bufmgr_gem->aub_file)
> >               return;
> >
> > -     /* Write out all but the batchbuffer to AUB memory */
> > -     for (i = 0; i < bufmgr_gem->exec_count - 1; i++) {
> > -             if (bufmgr_gem->exec_bos[i] != bo)
> > -                     aub_write_bo(bufmgr_gem->exec_bos[i]);
> > +        /* If batch buffer is not annotated, annotate it the best we
> > +         * can.
> > +         */
> > +        batch_buffer_needs_annotations = bo_gem->aub_annotation_count
> == 0;
> > +        if (batch_buffer_needs_annotations) {
> > +                drm_intel_aub_annotation annotations[2] = {
> > +                        { AUB_TRACE_TYPE_BATCH, 0, used },
> > +                        { AUB_TRACE_TYPE_NOTYPE, 0, bo->size }
> > +                };
> > +                drm_intel_bufmgr_gem_set_aub_annotations(bo,
> annotations, 2);
>
> Looks like you don't actually need to be explicit about the range from
> used to bo->size given the "Write out any remaining unannotated data"
> code above, but either way.
>

Fair point.  I think I will leave the code as is just to avoid having to do
another round of testing :)


>
> Assuming tab replacement: Reviewed-by: Eric Anholt <eric at anholt.net>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20120510/24295d83/attachment.html>