Re: [PATCH] drm/amdgpu/display: drop the reduction loop when setting the sync groups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020-05-29 2:04 p.m., Alex Deucher wrote:
On Fri, May 29, 2020 at 9:56 AM Kazlauskas, Nicholas
<nicholas.kazlauskas@xxxxxxx> wrote:

On 2020-05-28 10:06 a.m., Alex Deucher wrote:
The logic for blanked is not the same as having a plane_state. Technically
you can drive an OTG without anything connected in the front end and it'll
just draw out the back color which is distinct from having the OTG be blanked.
If we add planes or unblank the OTG later then we'll still want the
synchronization.

Bug: https://gitlab.freedesktop.org/drm/amd/issues/781
Fixes: 5fc0cbfad45648 ("drm/amd/display: determine if a pipe is synced by plane state")
Cc: nicholas.kazlauskas@xxxxxxx
Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx> > ---
   drivers/gpu/drm/amd/display/dc/core/dc.c | 8 --------
   1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 04c3d9f7e323..6279520f7873 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -1040,14 +1040,6 @@ static void program_timing_sync(
                               status->timing_sync_info.master = false;

               }
-             /* remove any other pipes with plane as they have already been synced */
-             for (j = j + 1; j < group_size; j++) {
-                     if (pipe_set[j]->plane_state) {
-                             group_size--;
-                             pipe_set[j] = pipe_set[group_size];
-                             j--;
-                     }
-             }


Looking at this again, I think I may understand the issue this was
trying to work around.

If we try to force timing synchronization on displays that are currently
active then this is going to force reset the vertical position,
resulting in screen corruption.

So what this logic was attempting to do was ensure that timing
synchronization only happens when committing two streams at a time
without any image on the screen.

Maybe it'd be best to just blank these streams out first, but for now,
let's actually go back to fixing this by applying the actual dpg/tg
check that Wenjing suggests, something like:

     if (pool->opps[i]->funcs->dpg_is_blanked)
                  s.blank_enabled =
pool->opps[i]->funcs->dpg_is_blanked(pool->opps[i]);
             else
                  s.blank_enabled = tg->funcs->is_blanked(tg);


Hmm, it's not clear to me where this code needs to go.  Can you point
me in the right direction or provide a quick patch?

Thanks,

Alex

The old code used to check !tg->funcs->is_blanked(tg) ie. to drop the pipe from the group if it's currently active.

The issue was that on newer ASIC it's now the DPG that's the indicator from the hardware side, so we should replace the !plane_state check with a check first for !dpg_is_blanked and then !is_blanked if the DPG doesn't exist.

Regards,
Nicholas Kazlauskas




The reason why we have this issue in the first place is because
amdgpu_dm usually commits a dc_state with the planes already in it
instead of committing them later, so plane_state not being NULL is
typically true.

Regards,
Nicholas Kazlauskas


               if (group_size > 1) {
                       dc->hwss.enable_timing_synchronization(



_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux