| Message ID | 20220429134230.24334-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm: handle kernel fences in drm_gem_plane_helper_prepare_fb v2 | expand |
Just a gentle ping on this. Harry, Nicholas any comments or can I push this through drm-misc-next? Thanks, Christian. Am 29.04.22 um 15:42 schrieb Christian König: > This gives us the standard atomic implicit and explicit fencing rules. > > Signed-off-by: Christian König <christian.koenig@amd.com> > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> > Cc: Roman Li <Roman.Li@amd.com> > Cc: Qingqing Zhuo <qingqing.zhuo@amd.com> > Cc: Jude Shih <shenshih@amd.com> > Cc: Wayne Lin <Wayne.Lin@amd.com> > Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++----------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > 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 2ade82cfb1ac..c5b2417adcc6 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -83,6 +83,7 @@ > #include <drm/drm_edid.h> > #include <drm/drm_vblank.h> > #include <drm/drm_audio_component.h> > +#include <drm/drm_gem_atomic_helper.h> > > #if defined(CONFIG_DRM_AMD_DC_DCN) > #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h" > @@ -7627,6 +7628,10 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane, > goto error_unpin; > } > > + r = drm_gem_plane_helper_prepare_fb(plane, new_state); > + if (unlikely(r != 0)) > + goto error_unpin; > + > amdgpu_bo_unreserve(rbo); > > afb->address = amdgpu_bo_gpu_offset(rbo); > @@ -9160,7 +9165,6 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > struct dm_crtc_state *dm_old_crtc_state = > to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc)); > int planes_count = 0, vpos, hpos; > - long r; > unsigned long flags; > struct amdgpu_bo *abo; > uint32_t target_vblank, last_flip_vblank; > @@ -9173,6 +9177,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > struct dc_flip_addrs flip_addrs[MAX_SURFACES]; > struct dc_stream_update stream_update; > } *bundle; > + int r; > > bundle = kzalloc(sizeof(*bundle), GFP_KERNEL); > > @@ -9181,6 +9186,10 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > goto cleanup; > } > > + r = drm_atomic_helper_wait_for_fences(dev, state, false); > + if (unlikely(r)) > + DRM_ERROR("Waiting for fences timed out!"); > + > /* > * Disable the cursor first if we're disabling all the planes. > * It'll remain on the screen after the planes are re-enabled > @@ -9235,18 +9244,6 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > } > > abo = gem_to_amdgpu_bo(fb->obj[0]); > - > - /* > - * Wait for all fences on this FB. Do limited wait to avoid > - * deadlock during GPU reset when this fence will not signal > - * but we hold reservation lock for the BO. > - */ > - r = dma_resv_wait_timeout(abo->tbo.base.resv, > - DMA_RESV_USAGE_WRITE, false, > - msecs_to_jiffies(5000)); > - if (unlikely(r <= 0)) > - DRM_ERROR("Waiting for fences timed out!"); > - > fill_dc_plane_info_and_addr( > dm->adev, new_plane_state, > afb->tiling_flags,
On Fri, Apr 29, 2022 at 03:42:28PM +0200, Christian König wrote: > This gives us the standard atomic implicit and explicit fencing rules. > > Signed-off-by: Christian König <christian.koenig@amd.com> > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> > Cc: Roman Li <Roman.Li@amd.com> > Cc: Qingqing Zhuo <qingqing.zhuo@amd.com> > Cc: Jude Shih <shenshih@amd.com> > Cc: Wayne Lin <Wayne.Lin@amd.com> > Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++----------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > 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 2ade82cfb1ac..c5b2417adcc6 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -83,6 +83,7 @@ > #include <drm/drm_edid.h> > #include <drm/drm_vblank.h> > #include <drm/drm_audio_component.h> > +#include <drm/drm_gem_atomic_helper.h> > > #if defined(CONFIG_DRM_AMD_DC_DCN) > #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h" > @@ -7627,6 +7628,10 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane, > goto error_unpin; > } > > + r = drm_gem_plane_helper_prepare_fb(plane, new_state); > + if (unlikely(r != 0)) > + goto error_unpin; > + > amdgpu_bo_unreserve(rbo); > > afb->address = amdgpu_bo_gpu_offset(rbo); > @@ -9160,7 +9165,6 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > struct dm_crtc_state *dm_old_crtc_state = > to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc)); > int planes_count = 0, vpos, hpos; > - long r; > unsigned long flags; > struct amdgpu_bo *abo; > uint32_t target_vblank, last_flip_vblank; > @@ -9173,6 +9177,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > struct dc_flip_addrs flip_addrs[MAX_SURFACES]; > struct dc_stream_update stream_update; > } *bundle; > + int r; > > bundle = kzalloc(sizeof(*bundle), GFP_KERNEL); > > @@ -9181,6 +9186,10 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > goto cleanup; > } > > + r = drm_atomic_helper_wait_for_fences(dev, state, false); This already waits for all fences over all planes, I think the call should be put somewhere in amdgpu_dm_atomic_commit_tail() instead. In helpers it's at the top before we start doing anything, but I guess you can also put it right before the plane commit loop. While reviewing I also noticed that the legacy kms path's various mode_set_base implementations don't wait for fences after calling amdgpu_bo_pin(), e.g. in dce_v10_0_crtc_do_set_base(). The page flip path is already updated, but might be good to fix this one here too. That stuff is a bit copypasta gallore (atomic is a lot nicer) but not too tricky to fix. Maybe separate patch. With the wait_for_fences call moved: Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> -Daniel > + if (unlikely(r)) > + DRM_ERROR("Waiting for fences timed out!"); > + > /* > * Disable the cursor first if we're disabling all the planes. > * It'll remain on the screen after the planes are re-enabled > @@ -9235,18 +9244,6 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > } > > abo = gem_to_amdgpu_bo(fb->obj[0]); > - > - /* > - * Wait for all fences on this FB. Do limited wait to avoid > - * deadlock during GPU reset when this fence will not signal > - * but we hold reservation lock for the BO. > - */ > - r = dma_resv_wait_timeout(abo->tbo.base.resv, > - DMA_RESV_USAGE_WRITE, false, > - msecs_to_jiffies(5000)); > - if (unlikely(r <= 0)) > - DRM_ERROR("Waiting for fences timed out!"); > - > fill_dc_plane_info_and_addr( > dm->adev, new_plane_state, > afb->tiling_flags, > -- > 2.25.1 >
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 2ade82cfb1ac..c5b2417adcc6 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -83,6 +83,7 @@ #include <drm/drm_edid.h> #include <drm/drm_vblank.h> #include <drm/drm_audio_component.h> +#include <drm/drm_gem_atomic_helper.h> #if defined(CONFIG_DRM_AMD_DC_DCN) #include "ivsrcid/dcn/irqsrcs_dcn_1_0.h" @@ -7627,6 +7628,10 @@ static int dm_plane_helper_prepare_fb(struct drm_plane *plane, goto error_unpin; } + r = drm_gem_plane_helper_prepare_fb(plane, new_state); + if (unlikely(r != 0)) + goto error_unpin; + amdgpu_bo_unreserve(rbo); afb->address = amdgpu_bo_gpu_offset(rbo); @@ -9160,7 +9165,6 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, struct dm_crtc_state *dm_old_crtc_state = to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc)); int planes_count = 0, vpos, hpos; - long r; unsigned long flags; struct amdgpu_bo *abo; uint32_t target_vblank, last_flip_vblank; @@ -9173,6 +9177,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, struct dc_flip_addrs flip_addrs[MAX_SURFACES]; struct dc_stream_update stream_update; } *bundle; + int r; bundle = kzalloc(sizeof(*bundle), GFP_KERNEL); @@ -9181,6 +9186,10 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, goto cleanup; } + r = drm_atomic_helper_wait_for_fences(dev, state, false); + if (unlikely(r)) + DRM_ERROR("Waiting for fences timed out!"); + /* * Disable the cursor first if we're disabling all the planes. * It'll remain on the screen after the planes are re-enabled @@ -9235,18 +9244,6 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, } abo = gem_to_amdgpu_bo(fb->obj[0]); - - /* - * Wait for all fences on this FB. Do limited wait to avoid - * deadlock during GPU reset when this fence will not signal - * but we hold reservation lock for the BO. - */ - r = dma_resv_wait_timeout(abo->tbo.base.resv, - DMA_RESV_USAGE_WRITE, false, - msecs_to_jiffies(5000)); - if (unlikely(r <= 0)) - DRM_ERROR("Waiting for fences timed out!"); - fill_dc_plane_info_and_addr( dm->adev, new_plane_state, afb->tiling_flags,
This gives us the standard atomic implicit and explicit fencing rules. Signed-off-by: Christian König <christian.koenig@amd.com> Cc: Harry Wentland <harry.wentland@amd.com> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> Cc: Roman Li <Roman.Li@amd.com> Cc: Qingqing Zhuo <qingqing.zhuo@amd.com> Cc: Jude Shih <shenshih@amd.com> Cc: Wayne Lin <Wayne.Lin@amd.com> Cc: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com> --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-)