lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CADnq5_MRMjr3byW_qO==Ps+X9grYQ9FLYGFEnd_Jsu3FLQBzCw@mail.gmail.com>
Date:   Mon, 17 Apr 2023 13:34:33 -0400
From:   Alex Deucher <alexdeucher@...il.com>
To:     Christian König <christian.koenig@....com>
Cc:     Hamza Mahfooz <hamza.mahfooz@....com>,
        amd-gfx@...ts.freedesktop.org, Stylon Wang <stylon.wang@....com>,
        Luben Tuikov <luben.tuikov@....com>,
        Leo Li <sunpeng.li@....com>,
        Qingqing Zhuo <qingqing.zhuo@....com>,
        "Pan, Xinhui" <Xinhui.Pan@....com>,
        Rodrigo Siqueira <Rodrigo.Siqueira@....com>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Hans de Goede <hdegoede@...hat.com>,
        Aurabindo Pillai <aurabindo.pillai@....com>,
        Hersen Wu <hersenxs.wu@....com>,
        dri-devel@...ts.freedesktop.org,
        Alex Deucher <alexander.deucher@....com>
Subject: Re: [PATCH] drm/amd/display: fix flickering caused by S/G mode

On Mon, Apr 17, 2023 at 1:59 AM Christian König
<christian.koenig@....com> wrote:
>
> Am 14.04.23 um 21:33 schrieb Hamza Mahfooz:
> > Currently, we allow the framebuffer for a given plane to move between
> > memory domains, however when that happens it causes the screen to
> > flicker, it is even possible for the framebuffer to change memory
> > domains on every plane update (causing a continuous flicker effect). So,
> > to fix this, don't perform an immediate flip in the aforementioned case.
>
> That sounds strongly like you just forget to wait for the move to finish!

It doesn't exhibit when we allow only gtt or only vram, only when
switches between pools does it flicker.

Alex

>
> What is the order of things done here? E.g. who calls amdgpu_bo_pin()
> and who waits for fences for finish signaling? Is that maybe just in the
> wrong order?
>
> Regards,
> Christian.
>
> >
> > Cc: stable@...r.kernel.org
> > Fixes: 81d0bcf99009 ("drm/amdgpu: make display pinning more flexible (v2)")
> > Signed-off-by: Hamza Mahfooz <hamza.mahfooz@....com>
> > ---
> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 41 ++++++++++++++++++-
> >   1 file changed, 39 insertions(+), 2 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 da3045fdcb6d..9a4e7408384a 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -7897,6 +7897,34 @@ static void amdgpu_dm_commit_cursors(struct drm_atomic_state *state)
> >                       amdgpu_dm_plane_handle_cursor_update(plane, old_plane_state);
> >   }
> >
> > +static inline uint32_t get_mem_type(struct amdgpu_device *adev,
> > +                                 struct drm_gem_object *obj,
> > +                                 bool check_domain)
> > +{
> > +     struct amdgpu_bo *abo = gem_to_amdgpu_bo(obj);
> > +     uint32_t mem_type;
> > +
> > +     if (unlikely(amdgpu_bo_reserve(abo, true)))
> > +             return 0;
> > +
> > +     if (unlikely(dma_resv_reserve_fences(abo->tbo.base.resv, 1)))
> > +             goto err;
> > +
> > +     if (check_domain &&
> > +         amdgpu_display_supported_domains(adev, abo->flags) !=
> > +         (AMDGPU_GEM_DOMAIN_VRAM | AMDGPU_GEM_DOMAIN_GTT))
> > +             goto err;
> > +
> > +     mem_type = abo->tbo.resource->mem_type;
> > +     amdgpu_bo_unreserve(abo);
> > +
> > +     return mem_type;
> > +
> > +err:
> > +     amdgpu_bo_unreserve(abo);
> > +     return 0;
> > +}
> > +
> >   static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> >                                   struct dc_state *dc_state,
> >                                   struct drm_device *dev,
> > @@ -7916,6 +7944,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> >                       to_dm_crtc_state(drm_atomic_get_old_crtc_state(state, pcrtc));
> >       int planes_count = 0, vpos, hpos;
> >       unsigned long flags;
> > +     uint32_t mem_type;
> >       u32 target_vblank, last_flip_vblank;
> >       bool vrr_active = amdgpu_dm_crtc_vrr_active(acrtc_state);
> >       bool cursor_update = false;
> > @@ -8035,13 +8064,21 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> >                       }
> >               }
> >
> > +             mem_type = get_mem_type(dm->adev, old_plane_state->fb->obj[0],
> > +                                     true);
> > +
> >               /*
> >                * Only allow immediate flips for fast updates that don't
> > -              * change FB pitch, DCC state, rotation or mirroing.
> > +              * change memory domain, FB pitch, DCC state, rotation or
> > +              * mirroring.
> >                */
> >               bundle->flip_addrs[planes_count].flip_immediate =
> >                       crtc->state->async_flip &&
> > -                     acrtc_state->update_type == UPDATE_TYPE_FAST;
> > +                     acrtc_state->update_type == UPDATE_TYPE_FAST &&
> > +                     (!mem_type || (mem_type && get_mem_type(dm->adev,
> > +                                                             fb->obj[0],
> > +                                                             false) ==
> > +                                    mem_type));
> >
> >               timestamp_ns = ktime_get_ns();
> >               bundle->flip_addrs[planes_count].flip_timestamp_in_us = div_u64(timestamp_ns, 1000);
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ