[igt-dev] [PATCH i-g-t v31 05/32] lib/intel_batchbuffer: add new functions to support rendercopy

[Zbigniew Kempczyński]

On Wed, Aug 26, 2020 at 04:49:52PM +0100, Chris Wilson wrote:
> Quoting Chris Wilson (2020-08-26 16:48:41)
> > Quoting Zbigniew Kempczyński (2020-08-20 07:30:03)
> > > @@ -1364,7 +1367,7 @@ void intel_bb_destroy(struct intel_bb *ibb)
> > >  */
> > >  void intel_bb_reset(struct intel_bb *ibb, bool purge_objects_cache)
> > >  {
> > > -       uint64_t bb_address;
> > > +       uint32_t i;
> > >  
> > >         if (purge_objects_cache && ibb->refcount > 1)
> > >                 igt_warn("Cannot purge objects cache on bb, refcount > 1!");
> > > @@ -1383,10 +1386,20 @@ void intel_bb_reset(struct intel_bb *ibb, bool purge_objects_cache)
> > >         gem_close(ibb->i915, ibb->handle);
> > >         ibb->handle = gem_create(ibb->i915, ibb->size);
> > >  
> > > -       bb_address = __intel_bb_propose_offset(ibb);
> > > -       intel_bb_add_object(ibb, ibb->handle, bb_address, false);
> > > +       ibb->batch_offset = __intel_bb_propose_offset(ibb);
> > > +       intel_bb_add_object(ibb, ibb->handle, ibb->batch_offset, false);
> > >         ibb->ptr = ibb->batch;
> > >         memset(ibb->batch, 0, ibb->size);
> > > +
> > > +       if (purge_objects_cache)
> > > +               return;
> > > +
> > > +       /*
> > > +        * For reset with no purging cache ensure we keep only supports 48bit
> > (Missing noun)
> > > +        * address flag (we don't want to relocate).
> > > +        */
> > > +       for (i = 0; i < ibb->num_objects; i++)
> > > +               ibb->objects[i].flags &= EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> > >  }
> > 
> > This is confusing me.
> > 
> > We keep only the 48b flag, "because we don't want to relocate".
> > 
> > So, what I think you mean is that if the object supports a large address
> > and we have assigned a high virtual address, then we want to make sure
> > we can reuse that high virtual address next time. If we lose the flag,
> > the kernel will pick a 32b address for us.
> > 
> > Ok, that makes sense. Comment could do with a second pass :)
> 
> A second issue we don't take the 48b flag into account with
> propose_offset. :(
> -Chris

We truncate randomized 64-bit value to gtt size. Am I missed something?
--
Zbigniew