[PATCH 3/7] drm/amd/display: Use vblank control events for PSR enable/disable
Mike Lothian
mike at fireburn.co.uk
Sat Sep 4 14:36:11 UTC 2021
Hi
This patch is causing issues on my PRIME system
I've opened https://gitlab.freedesktop.org/drm/amd/-/issues/1700 to track
Cheers
Mike
On Fri, 13 Aug 2021 at 07:35, Wayne Lin <Wayne.Lin at amd.com> wrote:
>
> From: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
>
> [Why]
> PSR can disable the HUBP along with the OTG when PSR is active.
>
> We'll hit a pageflip timeout when the OTG is disable because we're no
> longer updating the CRTC vblank counter and the pflip high IRQ will
> not fire on the flip.
>
> In order to flip the page flip timeout occur we should modify the
> enter/exit conditions to match DRM requirements.
>
> [How]
> Use our deferred handlers for DRM vblank control to notify DMCU(B)
> when it can enable or disable PSR based on whether vblank is disabled or
> enabled respectively.
>
> We'll need to pass along the stream with the notification now because
> we want to access the CRTC state while the CRTC is locked to get the
> stream state prior to the commit.
>
> Retain a reference to the stream so it remains safe to continue to
> access and release that reference once we're done with it.
>
> Enable/disable logic follows what we were previously doing in
> update_planes.
>
> The workqueue has to be flushed before programming streams or planes
> to ensure that we exit out of idle optimizations and PSR before
> these events occur if necessary.
>
> To keep the skip count logic the same to avoid FBCON PSR enablement
> requires copying the allow condition onto the DM IRQ parameters - a
> field that we can actually access from the worker.
>
> Reviewed-by: Roman Li <Roman.Li at amd.com>
> Acked-by: Wayne Lin <wayne.lin at amd.com>
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 48 +++++++++++++++----
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 +
> .../display/amdgpu_dm/amdgpu_dm_irq_params.h | 1 +
> 3 files changed, 43 insertions(+), 8 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 f88b6c5b83cd..cebd663b6708 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1061,7 +1061,22 @@ static void vblank_control_worker(struct work_struct *work)
>
> DRM_DEBUG_KMS("Allow idle optimizations (MALL): %d\n", dm->active_vblank_irq_count == 0);
>
> + /* Control PSR based on vblank requirements from OS */
> + if (vblank_work->stream && vblank_work->stream->link) {
> + if (vblank_work->enable) {
> + if (vblank_work->stream->link->psr_settings.psr_allow_active)
> + amdgpu_dm_psr_disable(vblank_work->stream);
> + } else if (vblank_work->stream->link->psr_settings.psr_feature_enabled &&
> + !vblank_work->stream->link->psr_settings.psr_allow_active &&
> + vblank_work->acrtc->dm_irq_params.allow_psr_entry) {
> + amdgpu_dm_psr_enable(vblank_work->stream);
> + }
> + }
> +
> mutex_unlock(&dm->dc_lock);
> +
> + dc_stream_release(vblank_work->stream);
> +
> kfree(vblank_work);
> }
>
> @@ -6018,6 +6033,11 @@ static inline int dm_set_vblank(struct drm_crtc *crtc, bool enable)
> work->acrtc = acrtc;
> work->enable = enable;
>
> + if (acrtc_state->stream) {
> + dc_stream_retain(acrtc_state->stream);
> + work->stream = acrtc_state->stream;
> + }
> +
> queue_work(dm->vblank_control_workqueue, &work->work);
> #endif
>
> @@ -8623,6 +8643,12 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> /* Update the planes if changed or disable if we don't have any. */
> if ((planes_count || acrtc_state->active_planes == 0) &&
> acrtc_state->stream) {
> + /*
> + * If PSR or idle optimizations are enabled then flush out
> + * any pending work before hardware programming.
> + */
> + flush_workqueue(dm->vblank_control_workqueue);
> +
> bundle->stream_update.stream = acrtc_state->stream;
> if (new_pcrtc_state->mode_changed) {
> bundle->stream_update.src = acrtc_state->stream->src;
> @@ -8691,16 +8717,20 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> acrtc_state->stream->link->psr_settings.psr_version != DC_PSR_VERSION_UNSUPPORTED &&
> !acrtc_state->stream->link->psr_settings.psr_feature_enabled)
> amdgpu_dm_link_setup_psr(acrtc_state->stream);
> - else if ((acrtc_state->update_type == UPDATE_TYPE_FAST) &&
> - acrtc_state->stream->link->psr_settings.psr_feature_enabled &&
> - !acrtc_state->stream->link->psr_settings.psr_allow_active) {
> - struct amdgpu_dm_connector *aconn = (struct amdgpu_dm_connector *)
> - acrtc_state->stream->dm_stream_context;
> +
> + /* Decrement skip count when PSR 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->psr_skip_count > 0)
> aconn->psr_skip_count--;
> - else
> - amdgpu_dm_psr_enable(acrtc_state->stream);
> +
> + /* Allow PSR when skip count is 0. */
> + acrtc_attach->dm_irq_params.allow_psr_entry = !aconn->psr_skip_count;
> + } else {
> + acrtc_attach->dm_irq_params.allow_psr_entry = false;
> }
>
> mutex_unlock(&dm->dc_lock);
> @@ -8949,8 +8979,10 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state)
>
> if (dc_state) {
> /* if there mode set or reset, disable eDP PSR */
> - if (mode_set_reset_required)
> + if (mode_set_reset_required) {
> + flush_workqueue(dm->vblank_control_workqueue);
> amdgpu_dm_psr_disable_all(dm);
> + }
>
> dm_enable_per_frame_crtc_master_sync(dc_state);
> mutex_lock(&dm->dc_lock);
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index c6b8b835b08a..d1d353a7c77d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -91,12 +91,14 @@ struct dm_compressor_info {
> * @work: Kernel work data for the work event
> * @dm: amdgpu display manager device
> * @acrtc: amdgpu CRTC instance for which the event has occurred
> + * @stream: DC stream for which the event has occurred
> * @enable: true if enabling vblank
> */
> struct vblank_control_work {
> struct work_struct work;
> struct amdgpu_display_manager *dm;
> struct amdgpu_crtc *acrtc;
> + struct dc_stream_state *stream;
> bool enable;
> };
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq_params.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq_params.h
> index f3b93ba69a27..79b5f9999fec 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq_params.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq_params.h
> @@ -33,6 +33,7 @@ struct dm_irq_params {
> struct mod_vrr_params vrr_params;
> struct dc_stream_state *stream;
> int active_planes;
> + bool allow_psr_entry;
> struct mod_freesync_config freesync_config;
>
> #ifdef CONFIG_DEBUG_FS
> --
> 2.25.1
>
More information about the amd-gfx
mailing list