[Mesa-dev] [PATCH v3 06/13] intel/isl: Add support to emit clear value address.

[Jordan Justen]

On 2018-02-26 21:35:42, Jason Ekstrand wrote:
> On Mon, Feb 26, 2018 at 8:12 PM, Jordan Justen <jordan.l.justen at intel.com>
> wrote:
> 
> > On 2018-02-26 17:08:12, Jason Ekstrand wrote:
> > > On Mon, Feb 26, 2018 at 1:14 PM, Jordan Justen <
> > jordan.l.justen at intel.com>
> > > wrote:
> > >
> > > > On 2018-02-21 13:45:15, Rafael Antognolli wrote:
> > > > > +   bool use_clear_address;
> > > >
> > > > I'm still wondering about this field. I think at the end we can just a
> > > > assume that if gen >= 10 and aux_usage != ISL_AUX_USAGE_NONE, then
> > > > we'll use the address.
> > > >
> > >
> > > That's not going to work if we want to turn this on for blorp, anv, and
> > > i965 separately.
> >
> > I guess this goes to the point I mentioned below. Maybe it make it
> > easier to break it up for enabling it. (I'm not certain that we
> > couldn't slice it up another way, but the argument seems fine.)
> >
> > But, after that, is it needed? If it's alway enabled when gen >= 10
> > and aux_usage != ISL_AUX_USAGE_NONE, then once everything is in place,
> > then isl can easily check for that condition, and there's no purpose
> > for use_clear_address. Correct?
> >
> 
> I suppose.  Once everything's moved over, there's really no reason to keep
> it around on gen10.
> 
> > I also wonder if clear_address is needed in the info struct. It did
> > not look like blorp set it by the end of the series, yet blorp was
> > enabling the feature. (I'm guessing that the reloc must be handing the
> > aux buf offset for blorp.)
> >
> 
> Yes and no.  It's not really used today but it is needed the moment we get
> rid of relocations.

Ah. Good point.

I guess the pinned address could still be written by the 'reloc'
function, without emitting the reloc. That would also mean
clear_address wouldn't be needed in the isl info struct. The name
'reloc' starts adding confusion in this case, and maybe something like
emit_bo_address might make more sense at that point.

Or, we could make the reloc functions total no-ops with pinned
addresses, in which case clear_address would be needed.

Based on this, we might want to decide if patch 7 should be doing
anything with clear_address... (Right now it doesn't set it.)

-Jordan

> >
> > > > I think you mentioned that it could be tough implement the support in
> > > > steps if we had an all or nothing enaling of the address usage. But,
> > > > does that mean that at the end of your series you could add a patch to
> > > > remove this `use_clear_address` field?
> > > >
> > > > Maybe as a test in jenkins, you could add a patch that asserts that if
> > > > gen >= 10 and there is an aux_buffer, then use_clear_address==true in
> > > > your current series.
> > > >
> > > > -Jordan
> >