[igt-dev] [PATCH i-g-t 7/7] benchmarks/gem_exec_fault: Add softpin mode to support gens with ppgtt
Zbigniew Kempczyński
zbigniew.kempczynski at intel.com
Fri Oct 15 03:49:48 UTC 2021
On Thu, Oct 14, 2021 at 08:41:39PM -0700, Dixit, Ashutosh wrote:
> On Thu, 14 Oct 2021 20:31:03 -0700, Zbigniew Kempczyński wrote:
> >
> > On Thu, Oct 14, 2021 at 08:18:23PM -0700, Dixit, Ashutosh wrote:
> > > On Thu, 14 Oct 2021 19:49:05 -0700, Zbigniew Kempczyński wrote:
> > > >
> > > > On Thu, Oct 14, 2021 at 01:07:37PM -0700, Dixit, Ashutosh wrote:
> > > > > On Thu, 14 Oct 2021 01:19:17 -0700, Zbigniew Kempczyński wrote:
> > > > > >
> > > > > > @@ -127,9 +151,14 @@ static int loop(uint64_t size, unsigned ring, int reps, int ncpus,
> > > > > > obj.alignment = 0;
> > > > > > gem_execbuf(fd, &execbuf);
> > > > > >
> > > > > > - /* fault out */
> > > > > > - obj.alignment = 1ull << 63;
> > > > > > - __gem_execbuf(fd, &execbuf);
> > > > > > + if (ahnd) {
> > > > > > + obj.offset = get_offset(ahnd, obj.handle, size, 0);
> > > > > > + obj.flags |= EXEC_OBJECT_PINNED | EXEC_OBJECT_SUPPORTS_48B_ADDRESS;
> > > > > > + } else {
> > > > > > + /* fault out */
> > > > > > + obj.alignment = 1ull << 63;
> > > > > > + __gem_execbuf(fd, &execbuf);
> > > > > > + }
> > > > >
> > > > > Bug above, __gem_execbuf should be moved out of the else {}.
> > > >
> > > > No, it shouldn't. Normal execbuf will lead to unbind/bind with new offset
> > > > and no 'alignment' fault-out execbuf is necessary.
> > >
> > > Ah, you are right. Though in that case I think, if the loop has N
> > > iterations, the number of binds is N and the number of unbinds will be (N -
> > > 1). Is it worth fixing that? Basically I think we might need to add a bind
> > > outside the first iteration of the loop so that we have an unbind in the
> > > first iteration itself. Then we will have N binds and N unbinds I think.
> >
> > You're right, for softpin case we got N-1. But I don't think we want to
> > compare results between alignment / softpin paths but for dedicated changes
> > in the kernel. So then that missing unbind doesn't matter.
>
> I thought it is benchmarking the time taken for N binds and N unbinds so
> not too sure how much difference N binds and N-1 unbinds make. Anyway
> please go ahead and merge if you think it's fine.
>
> Reviewed-by: Ashutosh Dixit <ashutosh.dixit at intel.com>
Thank you for review. I think for benchmarking purposes last unbind
is not so important - if we change kernel we will compare same number
of binds and unbinds and this imo does matter.
--
Zbigniew
More information about the igt-dev
mailing list