| Message ID | 20200604081224.863494-9-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | dma-fence lockdep annotations, round 2 | expand |
Hi Roland & vmwgfx maintainers, Thomas has played around with these annotations on his vmwgfx setup, and found some issues. Apparently in the atomic_commit_tail path when handling the dirty rectangle stuff you acquire a ttm reservation, which is a no-go since it could deadlock with other paths - atomic commits can produce a dma_fence. This patch here highlights that with the new annotations, and apparently causes a lockdep splat if you go through the dirty rect paths (not sure if it also happens otherwise, Thomas can fill you in with the details). Can you pls take a look at this? I'm happy to help out with analyzing any lockdep splats. For actual fixes Thomas is better since I don't understand a lot of how drm/vmwgfx works internally. Cheers, Daniel On Thu, Jun 4, 2020 at 10:12 AM Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > I need a canary in a ttm-based atomic driver to make sure the > dma_fence_begin/end_signalling annotations actually work. > > Cc: linux-media@vger.kernel.org > Cc: linaro-mm-sig@lists.linaro.org > Cc: linux-rdma@vger.kernel.org > Cc: amd-gfx@lists.freedesktop.org > Cc: intel-gfx@lists.freedesktop.org > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Christian König <christian.koenig@amd.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > index bdba0bfd6df1..adabfa929f42 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -57,6 +57,7 @@ > > #include "ivsrcid/ivsrcid_vislands30.h" > > +#include <linux/module.h> > #include <linux/module.h> > #include <linux/moduleparam.h> > #include <linux/version.h> > @@ -7320,6 +7321,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) > struct drm_connector_state *old_con_state, *new_con_state; > struct dm_crtc_state *dm_old_crtc_state, *dm_new_crtc_state; > int crtc_disable_count = 0; > + bool fence_cookie; > + > + fence_cookie = dma_fence_begin_signalling(); > > drm_atomic_helper_update_legacy_modeset_state(dev, state); > > @@ -7600,6 +7604,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) > /* Signal HW programming completion */ > drm_atomic_helper_commit_hw_done(state); > > + dma_fence_end_signalling(fence_cookie); > + > if (wait_for_vblank) > drm_atomic_helper_wait_for_flip_done(dev, state); > > -- > 2.26.2 >
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index bdba0bfd6df1..adabfa929f42 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -57,6 +57,7 @@ #include "ivsrcid/ivsrcid_vislands30.h" +#include <linux/module.h> #include <linux/module.h> #include <linux/moduleparam.h> #include <linux/version.h> @@ -7320,6 +7321,9 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) struct drm_connector_state *old_con_state, *new_con_state; struct dm_crtc_state *dm_old_crtc_state, *dm_new_crtc_state; int crtc_disable_count = 0; + bool fence_cookie; + + fence_cookie = dma_fence_begin_signalling(); drm_atomic_helper_update_legacy_modeset_state(dev, state); @@ -7600,6 +7604,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) /* Signal HW programming completion */ drm_atomic_helper_commit_hw_done(state); + dma_fence_end_signalling(fence_cookie); + if (wait_for_vblank) drm_atomic_helper_wait_for_flip_done(dev, state);
I need a canary in a ttm-based atomic driver to make sure the dma_fence_begin/end_signalling annotations actually work. Cc: linux-media@vger.kernel.org Cc: linaro-mm-sig@lists.linaro.org Cc: linux-rdma@vger.kernel.org Cc: amd-gfx@lists.freedesktop.org Cc: intel-gfx@lists.freedesktop.org Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Christian König <christian.koenig@amd.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 ++++++ 1 file changed, 6 insertions(+)