Re: [RESEND] [PATCH v6 0/8] do not store GPU address in TTM

From: Daniel Vetter <daniel@ffwll.ch>
To: "Christian König" <christian.koenig@amd.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>,
	Sasha Levin <sashal@kernel.org>, Dave Airlie <airlied@linux.ie>,
	"Ho, Kenny" <kenny.ho@amd.com>,
	Brian Welty <brian.welty@intel.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Ben Skeggs <bskeggs@redhat.com>, Nirmoy Das <nirmoy.das@amd.com>,
	VMware Graphics <linux-graphics-maintainer@vmware.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Alex Deucher <alexander.deucher@amd.com>,
	Sean Paul <sean@poorly.run>, Nirmoy Das <nirmoy.aiemd@gmail.com>
Subject: Re: [RESEND] [PATCH v6 0/8] do not store GPU address in TTM
Date: Fri, 26 Jun 2020 15:12:26 +0200	[thread overview]
Message-ID: <CAKMK7uEaudcGU2cpaS4qdAgCb=WzoeOyrTAwARkUkepqOfgCCA@mail.gmail.com> (raw)
In-Reply-To: <779a3a25-723b-6805-debd-a7a59e657536@amd.com>

On Fri, Jun 26, 2020 at 3:04 PM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 25.06.20 um 18:02 schrieb Christian König:
> > Am 25.06.20 um 17:52 schrieb Christian König:
> >> Am 25.06.20 um 17:44 schrieb Daniel Vetter:
> >>> On Thu, Jun 25, 2020 at 11:50 AM Christian König
> >>> <christian.koenig@amd.com> wrote:
> >>>> I've pushed patches #1, #2 and #5-#8 of this series to drm-misc-next.
> >>> I think you left an unresolved conflict behind in drm-tip, please
> >>> resolve per
> >>>
> >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdrm.pages.freedesktop.org%2Fmaintainer-tools%2Fdrm-tip.html%23resolving-conflicts-when-rebuilding-drm-tip&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7Cc1441a81e9dc4fa4507208d8191ea0f6%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637286966631595300&amp;sdata=pfWS4VgDvU8IsR2638MDr7fYWE0nefa3b6XxyPCCsOU%3D&amp;reserved=0
> >>>
> >>>
> >>> The script should have told you that already, so maybe reinvent that
> >>> in whatever thing you're using :-) Jani has reported this on
> >>> #dri-devel, this also holds up CI testing since we're running on top
> >>> of drm-tip.
> >>
> >> Well I used dim push-branch. I even had to rebase once more because I
> >> forgot a signed-of on one of the patches, so I'm pretty sure of that.
> >>
> >> Haven't seen anything problematic except for that while doing so.
> >
> > Ok, anyway that one was trivial to fix.
>
> Ran into the next merge conflict. But no surprise that I missed the
> warning, that needs to be in red or so.

Yeah the trouble is that the merging is done after you pushed, we
don't do a test drm-tip rebuild before you push. Reason is that I've
not gotten around to implementing this since years, it's on the
wishlist.

If we'd do a test-rebuild before pushing (something like use all the
normal branches, excpet for $branch use the local one, or a specific
sha1, with git worktree that's all shared so doable), then it would be
a lot more obvious that people have some work to do before they're
done with their push.

Trouble is only rebuilding in dim push isn't enough, since then you
can't test. So we need the full suite of tools be able to work with a
local branch. And once you have a conflict which needs a fixup patch
that's kinda not possible anymore.

But maybe just doing the test-rebuild and if that fails, pointing it
out and requiring a special --promises-I-fix-it-up-right-afterwards
flag might be good enough.

If you know how to print stuff in red if it's a terminal, then
patching warn_or_fail should catch them all.
-Daniel

>
> Regards,
> Christian.
>
> >
> > Thanks,
> > Christian.
> >
> >>
> >> Regards,
> >> Christian.
> >>
> >>> -Daniel
> >>>
> >>>
> >>>
> >>>> Only VMGFX and Nouveau are missing and I'm pretty close to just push
> >>>> them with my Acked-by since they should not contain any functional
> >>>> change.
> >>>>
> >>>> Any objections?
> >>>>
> >>>> Thanks,
> >>>> Christian.
> >>>>
> >>>> Am 24.06.20 um 20:26 schrieb Nirmoy Das:
> >>>>> With this patch series I am trying to remove GPU address
> >>>>> dependency in
> >>>>> TTM and moving GPU address calculation to individual drm drivers.
> >>>>> This
> >>>>> cleanup will simplify introduction of drm_mem_region/domain work
> >>>>> started
> >>>>> by Brian Welty[1].
> >>>>>
> >>>>>
> >>>>> It would be nice if someone test this for nouveau. Rest of the
> >>>>> drivers
> >>>>> are already tested.
> >>>>>
> >>>>> v2:
> >>>>> * set bo->offset = 0 for drm/nouveau if bo->mem.mm_node == NULL
> >>>>>
> >>>>> v3:
> >>>>> * catch return value of drm_gem_vram_offset() in drm/bochs
> >>>>> * introduce drm_gem_vram_pg_offset() in vram helper
> >>>>> * improve nbo->offset calculation for nouveau
> >>>>>
> >>>>> v4:
> >>>>> * minor coding style fixes in amdgpu and radeon
> >>>>> * remove unnecessary kerneldoc for internal function
> >>>>>
> >>>>> v5:
> >>>>> * rebase on top of drm-misc-next
> >>>>> * fix return value of drm_gem_vram_pg_offset()
> >>>>> * add a comment in drm_gem_vram_pg_offset() to clearify why we
> >>>>> return 0.
> >>>>>
> >>>>> v6:
> >>>>> * rebase to drm-misc-next
> >>>>> * removed acked for vmwgfx as there was a small conflict
> >>>>>
> >>>>> Nirmoy Das (8):
> >>>>>     drm/amdgpu: move ttm bo->offset to amdgpu_bo
> >>>>>     drm/radeon: don't use ttm bo->offset
> >>>>>     drm/vmwgfx: don't use ttm bo->offset
> >>>>>     drm/nouveau: don't use ttm bo->offset v3
> >>>>>     drm/qxl: don't use ttm bo->offset
> >>>>>     drm/vram-helper: don't use ttm bo->offset v4
> >>>>>     drm/bochs: use drm_gem_vram_offset to get bo offset v2
> >>>>>     drm/ttm: do not keep GPU dependent addresses
> >>>>>
> >>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_object.c  | 23 ++++++++++++++--
> >>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_object.h  |  1 +
> >>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 30
> >>>>> ++++++++++++++++-----
> >>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 +
> >>>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c |  4 +--
> >>>>>    drivers/gpu/drm/bochs/bochs_kms.c           |  7 ++++-
> >>>>>    drivers/gpu/drm/drm_gem_vram_helper.c       | 11 +++++++-
> >>>>>    drivers/gpu/drm/nouveau/dispnv04/crtc.c     |  6 ++---
> >>>>>    drivers/gpu/drm/nouveau/dispnv04/disp.c     |  3 ++-
> >>>>>    drivers/gpu/drm/nouveau/dispnv04/overlay.c  |  6 ++---
> >>>>>    drivers/gpu/drm/nouveau/dispnv50/base507c.c |  2 +-
> >>>>>    drivers/gpu/drm/nouveau/dispnv50/core507d.c |  2 +-
> >>>>>    drivers/gpu/drm/nouveau/dispnv50/ovly507e.c |  2 +-
> >>>>>    drivers/gpu/drm/nouveau/dispnv50/wndw.c     |  2 +-
> >>>>>    drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c |  2 +-
> >>>>>    drivers/gpu/drm/nouveau/nouveau_abi16.c     |  8 +++---
> >>>>>    drivers/gpu/drm/nouveau/nouveau_bo.c        |  8 ++++++
> >>>>>    drivers/gpu/drm/nouveau/nouveau_bo.h        |  3 +++
> >>>>>    drivers/gpu/drm/nouveau/nouveau_chan.c      |  2 +-
> >>>>>    drivers/gpu/drm/nouveau/nouveau_dmem.c      |  2 +-
> >>>>>    drivers/gpu/drm/nouveau/nouveau_fbcon.c     |  2 +-
> >>>>>    drivers/gpu/drm/nouveau/nouveau_gem.c       | 10 +++----
> >>>>>    drivers/gpu/drm/qxl/qxl_drv.h               |  6 ++---
> >>>>>    drivers/gpu/drm/qxl/qxl_kms.c               |  5 ++--
> >>>>>    drivers/gpu/drm/qxl/qxl_object.h            |  5 ----
> >>>>>    drivers/gpu/drm/qxl/qxl_ttm.c               |  9 -------
> >>>>>    drivers/gpu/drm/radeon/radeon.h             |  1 +
> >>>>>    drivers/gpu/drm/radeon/radeon_object.h      | 16 ++++++++++-
> >>>>>    drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +--
> >>>>>    drivers/gpu/drm/ttm/ttm_bo.c                |  7 -----
> >>>>>    drivers/gpu/drm/vmwgfx/vmwgfx_bo.c          |  4 +--
> >>>>>    drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c     |  2 +-
> >>>>>    drivers/gpu/drm/vmwgfx/vmwgfx_fifo.c        |  2 +-
> >>>>>    drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c  |  2 --
> >>>>>    include/drm/ttm/ttm_bo_api.h                |  2 --
> >>>>>    include/drm/ttm/ttm_bo_driver.h             |  1 -
> >>>>>    36 files changed, 125 insertions(+), 78 deletions(-)
> >>>>>
> >>>>> --
> >>>>> 2.27.0
> >>>>>
> >>>
> >>
> >
>

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel