Re: Questions about KMS flip

[Lang Yu <Lang.Yu@amd.com>]

On Tue, Nov 16, 2021 at 08:14:08AM +0100, Christian KKKnig wrote:
> Am 16.11.21 um 04:27 schrieb Lang Yu:
> > On Mon, Nov 15, 2021 at 01:04:15PM +0100, Michel DDDnzer wrote:
> > > [SNIP]
> > > > Though a single call to dce_v*_0_crtc_do_set_base() will
> > > > only pin the BO, I found it will be unpinned in next call to
> > > > dce_v*_0_crtc_do_set_base().
> > > Yeah, that's the normal case when the new BO is different from the old one.
> > > 
> > > To catch the case I described, try something like
> > > 
> > > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > > 
> > > index 18a7b3bd633b..5726bd87a355 100644
> > > 
> > > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > > 
> > > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > > 
> > > @@ -1926,6 +1926,7 @@ static int dce_v11_0_crtc_do_set_base(struct drm_crtc *crtc,
> > > 
> > >                  return r;
> > > 
> > > 
> > > 
> > >          if (!atomic) {
> > > 
> > > +               WARN_ON_ONCE(target_fb == fb);
> > > 
> > >                  r = amdgpu_bo_pin(abo, AMDGPU_GEM_DOMAIN_VRAM);
> > > 
> > >                  if (unlikely(r != 0)) {
> > > 
> > >                          amdgpu_bo_unreserve(abo);
> > > 
> > I did some tests, the warning can be triggered.
> > 
> > pin/unpin operations in *_crtc_do_set_base() and
> > amdgpu_display_crtc_page_flip_target() are mixed.
> 
> Ok sounds like we narrowed down the root cause pretty well.
> 
> Question is now how can we fix this? Just not pin the BO when target_fb ==
> fb?

That worked. I did a few simple tests and didn't observe ttm_bo_release warnings 
any more.

The pin/unpin logic,

1, fist crtc_mode_set, dce_v*_0_crtc_do_set_base() pins crtc->primary->fb(new),
old_fb is NULL.

2, second crtc_mode_set, dce_v*_0_crtc_do_set_base() pins crtc->primary->fb(new),
unpins old fb.

3, amdgpu_display_crtc_page_flip_target() pin/unpin operations.

4, third crtc_mode_set, dce_v*_0_crtc_do_set_base() pins crtc->primary->fb(new),
unpins old fb (it is pinned in last call to amdgpu_display_crtc_page_flip_target)

5, amdgpu_display_crtc_page_flip_target() pin/unpin operations.

.....

x, reboot, amdgpu_display_suspend_helper() is called, the last pinned fb was unpinned.

And I didn't observe amdgpu_bo_unpin() in dce_v*_0_crtc_disable() is called.

If the logic is wrong, please correct me.

Regards,
Lang

> Thanks,
> Christian.
> 
> > 
> > Regards,
> > Lang
> > 
>