Re: [PATCH 13/26] drm/amd/display: Clear update flags after update has been applied

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

 






On 03/10/2024 20:33, Rodrigo Siqueira wrote:
From: Josip Pavic <Josip.Pavic@xxxxxxx>

[Why]
Since the surface/stream update flags aren't cleared after applying
updates, those same updates may be applied again in a future call to
update surfaces/streams for surfaces/streams that aren't actually part
of that update (i.e. applying an update for one surface/stream can
trigger unintended programming on a different surface/stream).

For example, when an update results in a call to
program_front_end_for_ctx, that function may call program_pipe on all
pipes. If there are surface update flags that were never cleared on the
surface some pipe is attached to, then the same update will be
programmed again.

[How]
Clear the surface and stream update flags after applying the updates.
Hi,

Just to let you know: this patch fixes artifacts when transitioning from 2 to 3 planes with dynamic pipe split policy on DCN301, as reported here:

https://gitlab.freedesktop.org/drm/amd/-/issues/3441

The problem was first seen in kernel 6.5, when multiple features were enabled (plane color mgmt and zpos properties) and minimal transition state was reworked.

Should it be sent to stable too?

Thanks,

Melissa

Reviewed-by: Aric Cyr <aric.cyr@xxxxxxx>
Signed-off-by: Josip Pavic <Josip.Pavic@xxxxxxx>
Signed-off-by: Rodrigo Siqueira <rodrigo.siqueira@xxxxxxx>
---
  drivers/gpu/drm/amd/display/dc/core/dc.c | 45 ++++++++++++++++++------
  1 file changed, 34 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
index 981d9a327daf..7b239cbfbb4a 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -5129,11 +5129,26 @@ static bool update_planes_and_stream_v3(struct dc *dc,
  	return true;
  }
+static void clear_update_flags(struct dc_surface_update *srf_updates,
+	int surface_count, struct dc_stream_state *stream)
+{
+	int i;
+
+	if (stream)
+		stream->update_flags.raw = 0;
+
+	for (i = 0; i < surface_count; i++)
+		if (srf_updates[i].surface)
+			srf_updates[i].surface->update_flags.raw = 0;
+}
+
  bool dc_update_planes_and_stream(struct dc *dc,
  		struct dc_surface_update *srf_updates, int surface_count,
  		struct dc_stream_state *stream,
  		struct dc_stream_update *stream_update)
  {
+	bool ret = false;
+
  	dc_exit_ips_for_hw_access(dc);
  	/*
  	 * update planes and stream version 3 separates FULL and FAST updates
@@ -5150,10 +5165,16 @@ bool dc_update_planes_and_stream(struct dc *dc,
  	 * features as they are now transparent to the new sequence.
  	 */
  	if (dc->ctx->dce_version >= DCN_VERSION_4_01)
-		return update_planes_and_stream_v3(dc, srf_updates,
+		ret = update_planes_and_stream_v3(dc, srf_updates,
  				surface_count, stream, stream_update);
-	return update_planes_and_stream_v2(dc, srf_updates,
+	else
+		ret = update_planes_and_stream_v2(dc, srf_updates,
  			surface_count, stream, stream_update);
+
+	if (ret)
+		clear_update_flags(srf_updates, surface_count, stream);
+
+	return ret;
  }
void dc_commit_updates_for_stream(struct dc *dc,
@@ -5163,6 +5184,8 @@ void dc_commit_updates_for_stream(struct dc *dc,
  		struct dc_stream_update *stream_update,
  		struct dc_state *state)
  {
+	bool ret = false;
+
  	dc_exit_ips_for_hw_access(dc);
  	/* TODO: Since change commit sequence can have a huge impact,
  	 * we decided to only enable it for DCN3x. However, as soon as
@@ -5170,17 +5193,17 @@ void dc_commit_updates_for_stream(struct dc *dc,
  	 * the new sequence for all ASICs.
  	 */
  	if (dc->ctx->dce_version >= DCN_VERSION_4_01) {
-		update_planes_and_stream_v3(dc, srf_updates, surface_count,
+		ret = update_planes_and_stream_v3(dc, srf_updates, surface_count,
  				stream, stream_update);
-		return;
-	}
-	if (dc->ctx->dce_version >= DCN_VERSION_3_2) {
-		update_planes_and_stream_v2(dc, srf_updates, surface_count,
+	} else if (dc->ctx->dce_version >= DCN_VERSION_3_2) {
+		ret = update_planes_and_stream_v2(dc, srf_updates, surface_count,
  				stream, stream_update);
-		return;
-	}
-	update_planes_and_stream_v1(dc, srf_updates, surface_count, stream,
-			stream_update, state);
+	} else
+		ret = update_planes_and_stream_v1(dc, srf_updates, surface_count, stream,
+				stream_update, state);
+
+	if (ret)
+		clear_update_flags(srf_updates, surface_count, stream);
  }
uint8_t dc_get_current_stream_count(struct dc *dc)




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

  Powered by Linux