[Bug 205915] AMDGPU: Screen flicker after resume from suspend

[bugzilla-daemon@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=205915

Manuel Ullmann (labre@posteo.de) changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |labre@posteo.de

--- Comment #5 from Manuel Ullmann (labre@posteo.de) ---
Confirmed on navi14. Bisected it to 1ea8751bd28d1ec2b36a56ec6bc1ac28903d09b4.
Log is indicating exactly nothing. I’ll attach them nonetheless, if you happen
to see more than me.
It’s just a wild guess, but could it be, that your indicator for a safe
lowering of the clock is not always a good indicator? I mean you know your code
a lot better than me. A clock, that is raised too high while voltage being too
low could however be a reason for the flickering on OpenGL operations, that I’m
seeing. Have not seen those on Vulkan or framebuffer though.
Anyway, have a look for yourself:

commit 1ea8751bd28d1ec2b36a56ec6bc1ac28903d09b4
Author: Noah Abradjian <noah.abradjian@amd.com>
Date:   Fri Sep 27 16:30:57 2019 -0400

    drm/amd/display: Make clk mgr the only dto update point

    [Why]

    * Clk Mgr DTO update point did not cover all needed updates, as it included
a
      check for plane_state which does not exist yet when the updater is called
on
      driver startup
    * This resulted in another update path in the pipe programming sequence,
based
      on a dppclk update flag
    * However, this alternate path allowed for stray DTO updates, some of which
would
      occur in the wrong order during dppclk lowering and cause underflow

    [How]

    * Remove plane_state check and use of plane_res.dpp->inst, getting rid
      of sequence dependencies (this results in extra dto programming for
unused
      pipes but that doesn't cause issues and is a small cost)
    * Allow DTOs to be updated even if global clock is equal, to account for
      edge case exposed by diags tests
    * Remove update_dpp_dto call in pipe programming sequence (leave update to
      dppclk_control there, as that update is necessary and shouldn't occur in
clk
      mgr)
    * Remove call to optimize_bandwidth when committing state, as it is not
needed
      and resulted in sporadic underflows even with other fixes in place

    Signed-off-by: Noah Abradjian <noah.abradjian@amd.com>
    Reviewed-by: Jun Lei <Jun.Lei@amd.com>
    Acked-by: Leo Li <sunpeng.li@amd.com>
    Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

 .../gpu/drm/amd/display/dc/clk_mgr/dcn20/dcn20_clk_mgr.c   | 14 +++++++++-----
 drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c  |  3 ++-
 drivers/gpu/drm/amd/display/dc/core/dc.c                   |  4 ----
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c         |  8 +-------
    4 files changed, 12 insertions(+), 17 deletions(-)

modified   drivers/gpu/drm/amd/display/dc/clk_mgr/dcn20/dcn20_clk_mgr.c
@@ -108,11 +108,12 @@ void dcn20_update_clocks_update_dpp_dto(struct
clk_mgr_internal *clk_mgr,
        for (i = 0; i < clk_mgr->base.ctx->dc->res_pool->pipe_count; i++) {
                int dpp_inst, dppclk_khz;

-               if (!context->res_ctx.pipe_ctx[i].plane_state)
-                       continue;
-
-               dpp_inst = context->res_ctx.pipe_ctx[i].plane_res.dpp->inst;
+               /* Loop index will match dpp->inst if resource exists,
+                * and we want to avoid dependency on dpp object
+                */
+               dpp_inst = i;
                dppclk_khz =
context->res_ctx.pipe_ctx[i].plane_res.bw.dppclk_khz;
+
                clk_mgr->dccg->funcs->update_dpp_dto(
                                clk_mgr->dccg, dpp_inst, dppclk_khz);
        }
@@ -235,6 +236,7 @@ void dcn2_update_clocks(struct clk_mgr *clk_mgr_base,

                update_dispclk = true;
        }
+
        if (dc->config.forced_clocks == false || (force_reset &&
safe_to_lower)) {
                if (dpp_clock_lowered) {
                        // if clock is being lowered, increase DTO before
lowering refclk
@@ -244,10 +246,12 @@ void dcn2_update_clocks(struct clk_mgr *clk_mgr_base,
                        // if clock is being raised, increase refclk before
lowering DTO
                        if (update_dppclk || update_dispclk)
                                dcn20_update_clocks_update_dentist(clk_mgr);
-                       if (update_dppclk)
+                       // always update dtos unless clock is lowered and not
safe to lower
+                       if (new_clocks->dppclk_khz >=
dc->current_state->bw_ctx.bw.dcn.clk.dppclk_khz)
                                dcn20_update_clocks_update_dpp_dto(clk_mgr,
context);
                }
        }
+
        if (update_dispclk &&
                        dmcu && dmcu->funcs->is_dmcu_initialized(dmcu)) {
                /*update dmcu for wait_loop count*/
modified   drivers/gpu/drm/amd/display/dc/clk_mgr/dcn21/rn_clk_mgr.c
@@ -171,7 +171,8 @@ void rn_update_clocks(struct clk_mgr *clk_mgr_base,
                // if clock is being raised, increase refclk before lowering
DTO
                if (update_dppclk || update_dispclk)
                        rn_vbios_smu_set_dppclk(clk_mgr,
clk_mgr_base->clks.dppclk_khz);
-               if (update_dppclk)
+               // always update dtos unless clock is lowered and not safe to
lower
+               if (new_clocks->dppclk_khz >=
dc->current_state->bw_ctx.bw.dcn.clk.dppclk_khz)
                        dcn20_update_clocks_update_dpp_dto(clk_mgr, context);
        }

modified   drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1241,10 +1241,6 @@ static enum dc_status dc_commit_state_no_check(struct dc
*dc, struct dc_state *c

        dc_enable_stereo(dc, context, dc_streams, context->stream_count);

-       if (!dc->optimize_seamless_boot)
-               /* pplib is notified if disp_num changed */
-               dc->hwss.optimize_bandwidth(dc, context);
-
        for (i = 0; i < context->stream_count; i++)
                context->streams[i]->mode_changed = false;

modified   drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
@@ -1202,15 +1202,9 @@ static void dcn20_update_dchubp_dpp(
        struct dpp *dpp = pipe_ctx->plane_res.dpp;
        struct dc_plane_state *plane_state = pipe_ctx->plane_state;

-       if (pipe_ctx->update_flags.bits.dppclk) {
+       if (pipe_ctx->update_flags.bits.dppclk)
                dpp->funcs->dpp_dppclk_control(dpp, false, true);

-               dc->res_pool->dccg->funcs->update_dpp_dto(
-                               dc->res_pool->dccg,
-                               dpp->inst,
-                               pipe_ctx->plane_res.bw.dppclk_khz);
-       }
-
        /* TODO: Need input parameter to tell current DCHUB pipe tie to which
OTG
         * VTG is within DCHUBBUB which is commond block share by each pipe
HUBP.
         * VTG is 1:1 mapping with OTG. Each pipe HUBP will select which VTG

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel