Re: [PATCH 04/16] drm/amd/display: Fix Panel Replay not update screen correctly

[Mario Limonciello <mario.limonciello@xxxxxxx>]

On 11/6/2024 14:25, Mario Limonciello wrote:
> On 11/5/2024 14:22, Hamza Mahfooz wrote:
> > From: Tom Chung <chiahsuan.chung@xxxxxxx>
> >
> > [Why]
> > In certain use case such as KDE login screen, there will be no atomic
> > commit while do the frame update.
> > If the Panel Replay enabled, it will cause the screen not updated and
> > looks like system hang.
> >
> > [How]
> > Delay few atomic commits before enabled the Panel Replay just like PSR.
> >
> > Reviewed-by: Leo Li <sunpeng.li@xxxxxxx>
> > Signed-off-by: Tom Chung <chiahsuan.chung@xxxxxxx>
> > Signed-off-by: Hamza Mahfooz <hamza.mahfooz@xxxxxxx>
>
> As this is an issue reported in the drm/amd tracker and we have panel 
> replay in 6.11.y with active reports of this issue a few other tags that 
> this should close and associate with.
>
> Cc: stable@xxxxxxxxxxxxxxx # 6.11+
> Fixes: be64336307a6c ("drm/amd/display: Re-enable panel replay feature")
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3686
> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/3682

And one of the reporterers for those bugs tested it helps their issue,
so here is a tag for them.

Tested-By: Corey Hickey <bugfood-c@xxxxxxxxxx>

> > ---
> >   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 111 +++++++++---------
> >   .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c    |   5 +-
> >   2 files changed, 59 insertions(+), 57 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 570640c25b4b..fdc1d2d1afd9 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -8863,6 +8863,56 @@ static void amdgpu_dm_update_cursor(struct 
> > drm_plane *plane,
> >       }
> >   }
> > +static void amdgpu_dm_enable_self_refresh(struct amdgpu_crtc 
> > *acrtc_attach,
> > +                      const struct dm_crtc_state *acrtc_state,
> > +                      const u64 current_ts)
> > +{
> > +    struct psr_settings *psr = &acrtc_state->stream->link->psr_settings;
> > +    struct replay_settings *pr = &acrtc_state->stream->link- 
> > >replay_settings;
> > +    struct amdgpu_dm_connector *aconn =
> > +        (struct amdgpu_dm_connector *)acrtc_state->stream- 
> > >dm_stream_context;
> > +
> > +    if (acrtc_state->update_type > UPDATE_TYPE_FAST) {
> > +        if (pr->config.replay_supported && !pr->replay_feature_enabled)
> > +            amdgpu_dm_link_setup_replay(acrtc_state->stream->link, 
> > aconn);
> > +        else if (psr->psr_version != DC_PSR_VERSION_UNSUPPORTED &&
> > +                 !psr->psr_feature_enabled)
> > +            if (!aconn->disallow_edp_enter_psr)
> > +                amdgpu_dm_link_setup_psr(acrtc_state->stream);
> > +    }
> > +
> > +    /* Decrement skip count when SR is enabled and we're doing fast 
> > updates. */
> > +    if (acrtc_state->update_type == UPDATE_TYPE_FAST &&
> > +        (psr->psr_feature_enabled || pr->config.replay_supported)) {
> > +        if (aconn->sr_skip_count > 0)
> > +            aconn->sr_skip_count--;
> > +
> > +        /* Allow SR when skip count is 0. */
> > +        acrtc_attach->dm_irq_params.allow_sr_entry = !aconn- 
> > >sr_skip_count;
> > +
> > +        /*
> > +         * If sink supports PSR SU/Panel Replay, there is no need to 
> > rely on
> > +         * a vblank event disable request to enable PSR/RP. PSR SU/RP
> > +         * can be enabled immediately once OS demonstrates an
> > +         * adequate number of fast atomic commits to notify KMD
> > +         * of update events. See `vblank_control_worker()`.
> > +         */
> > +        if (acrtc_attach->dm_irq_params.allow_sr_entry &&
> > +#ifdef CONFIG_DRM_AMD_SECURE_DISPLAY
> > +            !amdgpu_dm_crc_window_is_activated(acrtc_state- 
> > >base.crtc) &&
> > +#endif
> > +            (current_ts - psr->psr_dirty_rects_change_timestamp_ns) > 
> > 500000000) {
> > +            if (pr->replay_feature_enabled && !pr->replay_allow_active)
> > +                amdgpu_dm_replay_enable(acrtc_state->stream, true);
> > +            if (psr->psr_version >= DC_PSR_VERSION_SU_1 &&
> > +                !psr->psr_allow_active && !aconn- 
> > >disallow_edp_enter_psr)
> > +                amdgpu_dm_psr_enable(acrtc_state->stream);
> > +        }
> > +    } else {
> > +        acrtc_attach->dm_irq_params.allow_sr_entry = false;
> > +    }
> > +}
> > +
> >   static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> >                       struct drm_device *dev,
> >                       struct amdgpu_display_manager *dm,
> > @@ -9191,9 +9241,12 @@ static void amdgpu_dm_commit_planes(struct 
> > drm_atomic_state *state,
> >               bundle->stream_update.abm_level = &acrtc_state->abm_level;
> >           mutex_lock(&dm->dc_lock);
> > -        if ((acrtc_state->update_type > UPDATE_TYPE_FAST) &&
> > -                acrtc_state->stream->link- 
> > >psr_settings.psr_allow_active)
> > -            amdgpu_dm_psr_disable(acrtc_state->stream);
> > +        if (acrtc_state->update_type > UPDATE_TYPE_FAST) {
> > +            if (acrtc_state->stream->link- 
> > >replay_settings.replay_allow_active)
> > +                amdgpu_dm_replay_disable(acrtc_state->stream);
> > +            if (acrtc_state->stream->link- 
> > >psr_settings.psr_allow_active)
> > +                amdgpu_dm_psr_disable(acrtc_state->stream);
> > +        }
> >           mutex_unlock(&dm->dc_lock);
> >           /*
> > @@ -9234,57 +9287,7 @@ static void amdgpu_dm_commit_planes(struct 
> > drm_atomic_state *state,
> >               dm_update_pflip_irq_state(drm_to_adev(dev),
> >                             acrtc_attach);
> > -        if (acrtc_state->update_type > UPDATE_TYPE_FAST) {
> > -            if (acrtc_state->stream->link- 
> > >replay_settings.config.replay_supported &&
> > -                    !acrtc_state->stream->link- 
> > >replay_settings.replay_feature_enabled) {
> > -                struct amdgpu_dm_connector *aconn =
> > -                    (struct amdgpu_dm_connector *)acrtc_state- 
> > >stream->dm_stream_context;
> > -                amdgpu_dm_link_setup_replay(acrtc_state->stream- 
> > >link, aconn);
> > -            } else if (acrtc_state->stream->link- 
> > >psr_settings.psr_version != DC_PSR_VERSION_UNSUPPORTED &&
> > -                    !acrtc_state->stream->link- 
> > >psr_settings.psr_feature_enabled) {
> > -
> > -                struct amdgpu_dm_connector *aconn = (struct 
> > amdgpu_dm_connector *)
> > -                    acrtc_state->stream->dm_stream_context;
> > -
> > -                if (!aconn->disallow_edp_enter_psr)
> > -                    amdgpu_dm_link_setup_psr(acrtc_state->stream);
> > -            }
> > -        }
> > -
> > -        /* Decrement skip count when SR is enabled and we're doing 
> > fast updates. */
> > -        if (acrtc_state->update_type == UPDATE_TYPE_FAST &&
> > -            acrtc_state->stream->link- 
> > >psr_settings.psr_feature_enabled) {
> > -            struct amdgpu_dm_connector *aconn =
> > -                (struct amdgpu_dm_connector *)acrtc_state->stream- 
> > >dm_stream_context;
> > -
> > -            if (aconn->sr_skip_count > 0)
> > -                aconn->sr_skip_count--;
> > -
> > -            /* Allow SR when skip count is 0. */
> > -            acrtc_attach->dm_irq_params.allow_sr_entry = !aconn- 
> > >sr_skip_count;
> > -
> > -            /*
> > -             * If sink supports PSR SU/Panel Replay, there is no need 
> > to rely on
> > -             * a vblank event disable request to enable PSR/RP. PSR 
> > SU/RP
> > -             * can be enabled immediately once OS demonstrates an
> > -             * adequate number of fast atomic commits to notify KMD
> > -             * of update events. See `vblank_control_worker()`.
> > -             */
> > -            if (acrtc_state->stream->link->psr_settings.psr_version 
> > >= DC_PSR_VERSION_SU_1 &&
> > -                acrtc_attach->dm_irq_params.allow_sr_entry &&
> > -#ifdef CONFIG_DRM_AMD_SECURE_DISPLAY
> > -                !amdgpu_dm_crc_window_is_activated(acrtc_state- 
> > >base.crtc) &&
> > -#endif
> > -                !acrtc_state->stream->link- 
> > >psr_settings.psr_allow_active &&
> > -                !aconn->disallow_edp_enter_psr &&
> > -                (timestamp_ns -
> > -                acrtc_state->stream->link- 
> > >psr_settings.psr_dirty_rects_change_timestamp_ns) >
> > -                500000000)
> > -                amdgpu_dm_psr_enable(acrtc_state->stream);
> > -        } else {
> > -            acrtc_attach->dm_irq_params.allow_sr_entry = false;
> > -        }
> > -
> > +        amdgpu_dm_enable_self_refresh(acrtc_attach, acrtc_state, 
> > timestamp_ns);
> >           mutex_unlock(&dm->dc_lock);
> >       }
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/ 
> > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> > index 31441f0ec4b0..64a041c2af05 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> > @@ -265,11 +265,10 @@ static void 
> > amdgpu_dm_crtc_vblank_control_worker(struct work_struct *work)
> >        * where the SU region is the full hactive*vactive region. See
> >        * fill_dc_dirty_rects().
> >        */
> > -    if (vblank_work->stream && vblank_work->stream->link) {
> > +    if (vblank_work->stream && vblank_work->stream->link && 
> > vblank_work->acrtc) {
> >           amdgpu_dm_crtc_set_panel_sr_feature(
> >               vblank_work, vblank_work->enable,
> > -            vblank_work->acrtc->dm_irq_params.allow_sr_entry ||
> > -            vblank_work->stream->link- 
> > >replay_settings.replay_feature_enabled);
> > +            vblank_work->acrtc->dm_irq_params.allow_sr_entry);
> >       }
> >       if (dm->active_vblank_irq_count == 0)