[PATCH 07/10] drm/amdgpu/ttm: handle tt moves properly.

[Dave Airlie]

On Thu, 24 Sep 2020 at 10:46, Dave Airlie <airlied at gmail.com> wrote:
>
> On Thu, 24 Sep 2020 at 00:46, Christian König <christian.koenig at amd.com> wrote:
> >
> > Am 23.09.20 um 05:04 schrieb Dave Airlie:
> > > From: Dave Airlie <airlied at redhat.com>
> > >
> > > The core move code currently handles use_tt moves, for amdgpu
> > > this was being handled also in the driver, but not using the same
> > > paths.
> > >
> > > If moving between TT/SYSTEM (all the use_tt paths on amdgpu) use
> > > the core move function.
> > >
> > > Eventually the core will be flipped over to calling the driver.
> > >
> > > Signed-off-by: Dave Airlie <airlied at redhat.com>
> > > ---
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 12 +++++++-----
> > >   1 file changed, 7 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > index db5f761f37ec..d3bd2fd448be 100644
> > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > > @@ -671,14 +671,16 @@ static int amdgpu_bo_move(struct ttm_buffer_object *bo, bool evict,
> > >               ttm_bo_move_null(bo, new_mem);
> > >               return 0;
> > >       }
> > > -     if ((old_mem->mem_type == TTM_PL_TT &&
> > > -          new_mem->mem_type == TTM_PL_SYSTEM) ||
> > > -         (old_mem->mem_type == TTM_PL_SYSTEM &&
> > > -          new_mem->mem_type == TTM_PL_TT)) {
> > > -             /* bind is enough */
> > > +     if (old_mem->mem_type == TTM_PL_SYSTEM &&
> > > +         new_mem->mem_type == TTM_PL_TT) {
> > >               ttm_bo_move_null(bo, new_mem);
> >
> > I would feel better if we nuke ttm_bo_move_null() and always use
> > ttm_bo_move_ttm().
>
> Any reason? The code path in the current move code pretty much
> is this.
>
> The current move paths are
>
> if (new_tt && old_tt)
>   if old is system
>      move null
>   else
>     move ttm
> else
>   call driver move.
>
> So I wanted to maintain that order. calling the move ttm will just
> make a pointless caching, populate, bind step.
>
> But I'm going to think about it a bit more.

I've looked into moving the code over to move_ttm to see if I could
combine things better, but it doesn't really fall out that nicely.

I might try again on top of the refactoring once I'm gone further.

Dave.