[PATCH 0/8] drm/msm/dpu: various dpu resource allocation/release mismatch fix

[PATCH 0/8] drm/msm/dpu: various dpu resource allocation/release mismatch fix

[Xi Zhang via B4 Relay]

This series fixes various resource de-allocation issues in DPU
resource cleanup code. Aim is more reliable mode set, in particular
when multiple displays/encoders are connected, to avoid blue screen
or snowy dsc displays.

Tested with v7.3 + msm-next, on Ayaneo Pocket DS (SM8550 Kalama).
Changes are not device/chip specific.

Some of the changes has lived in ROCKNIX distro for a few months.
https://github.com/ROCKNIX/distribution/pull/2716

Note: The first patch in that PR was problematic: uninitialized pointer
variable field access (since fixed), and I did not understand drm
atomic state tracking, so it wasn't doing anything at all and
is not included in this series.

The third patch linked above has more changes than what's included in
this series. The extra change did not do anything that's needed.

The following 2 commits are new and are not in ROCKNIX:

[Patch 6/8] ("drm/msm/dpu: workaround res re-assignment issues by pairing enc/crtc")
[Patch 7/8] ("drm/msm: force mode_changed on color_mgmt_changed, not just ctm")

[Patch 7/8] would fix, from my testing:
https://lore.kernel.org/all/33424a9d-10a6-4479-bba6-12f8ce60da1a@packett.cool/
See original report by Stephan Gerhold:
https://gitlab.freedesktop.org/drm/msm/-/work_items/102

Note: I'm not part of the ROCKNIX team, just contributed a few patches
there.

Signed-off-by: Xi Zhang <xz.ca.cn@gmail.com>
---
Xi Zhang (8):
      drm/msm/dpu: reset unassigned resource slots to NULL on topology update
      drm/msm/dpu: save LMs in encoder mode_set for cleanup use later
      drm/msm/dpu: use common helper cleanup function in cmd mode disable
      drm/msm/dpu: Match setup_dither cleanup to the way it was set up
      drm/msm/dpu: use cached PINGPONGs from mode set in cwb setup
      drm/msm/dpu: workaround res re-assignment issues by pairing enc/crtc
      drm/msm: force mode_changed on color_mgmt_changed, not just ctm
      drm/msm/dpu: reset DSC_CMN_MAIN_CNF in dsc disable

 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c        | 115 ++++++++++-----------
 .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c   |  10 +-
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c     |   1 +
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c            |  15 ++-
 drivers/gpu/drm/msm/msm_atomic.c                   |   3 +-
 5 files changed, 70 insertions(+), 74 deletions(-)
---
base-commit: d024361c631b3f020a26c07d683e5103404b0fbf
change-id: 20261026-msm-dpu-res-clean-f1704106453f

Best regards,
--  
Xi Zhang <xz.ca.cn@gmail.com>

[PATCH 1/8] drm/msm/dpu: reset unassigned resource slots to NULL on topology update

[Xi Zhang via B4 Relay]

From: Xi Zhang <xz.ca.cn@gmail.com>

While this is not currently the case, it may be possible that topology
for cwb and dsc resource requirements change between mode set in the
future.

e.g. when dsc support is added to DisplayPort, dsc may be optionally
enabled and disabled between different modes due to
bandwidth requirements.

Set unassigned resource slots to NULL on topology update to avoid
issues in the future. It's good practice to do so anyway.

Signed-off-by: Xi Zhang <xz.ca.cn@gmail.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0a6be69a3e83..8d2b4d2726f2 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1205,9 +1205,13 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
 						       ARRAY_SIZE(hw_pp));
 	}
 
-	for (i = 0; i < num_cwb; i++) {
-		dpu_enc->hw_cwb[i] = to_dpu_hw_cwb(hw_cwb[i]);
-		cwb_mask |= BIT(dpu_enc->hw_cwb[i]->idx - CWB_0);
+	for (i = 0; i < ARRAY_SIZE(dpu_enc->hw_cwb); i++) {
+		if (i < num_cwb) {
+			dpu_enc->hw_cwb[i] = to_dpu_hw_cwb(hw_cwb[i]);
+			cwb_mask |= BIT(dpu_enc->hw_cwb[i]->idx - CWB_0);
+		} else {
+			dpu_enc->hw_cwb[i] = NULL;
+		}
 	}
 
 	dpu_enc->cwb_mask = cwb_mask;
@@ -1215,16 +1219,20 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
 	num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
 		drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
 
-	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
+	for (i = 0; i < ARRAY_SIZE(dpu_enc->hw_pp); i++)
 		dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
 						: NULL;
 
 	num_dsc = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
 						drm_enc->crtc, DPU_HW_BLK_DSC,
 						hw_dsc, ARRAY_SIZE(hw_dsc));
-	for (i = 0; i < num_dsc; i++) {
-		dpu_enc->hw_dsc[i] = to_dpu_hw_dsc(hw_dsc[i]);
-		dsc_mask |= BIT(dpu_enc->hw_dsc[i]->idx - DSC_0);
+	for (i = 0; i < ARRAY_SIZE(dpu_enc->hw_dsc); i++) {
+		if (i < num_dsc) {
+			dpu_enc->hw_dsc[i] = to_dpu_hw_dsc(hw_dsc[i]);
+			dsc_mask |= BIT(dpu_enc->hw_dsc[i]->idx - DSC_0);
+		} else {
+			dpu_enc->hw_dsc[i] = NULL;
+		}
 	}
 
 	dpu_enc->dsc_mask = dsc_mask;

-- 
2.55.0

[PATCH 2/8] drm/msm/dpu: save LMs in encoder mode_set for cleanup use later

[Xi Zhang via B4 Relay]

From: Xi Zhang <xz.ca.cn@gmail.com>

dpu_encoder_helper_reset_mixers, which is called during atomic_commit
disable phase, previously called
dpu_kms_get_existing_global_state, which returns the global_state
after the state swap. So the cleanup was performed on the newly
allocated resources instead of the ones that actually need reset.

Cache LMs in dpu_enc similar to other resources during mode set, and
perform the cleanup on cached LMs instead.

Also removed redundant ctl->ops.setup_blendstage during cleanup since
ctl->ops.clear_all_blendstages covers it already.

Signed-off-by: Xi Zhang <xz.ca.cn@gmail.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 57 +++++++++++++++--------------
 1 file changed, 30 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 8d2b4d2726f2..09afc0681248 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -133,6 +133,7 @@ enum dpu_enc_rc_states {
  * @cur_master:		Pointer to the current master in this mode. Optimization
  *			Only valid after enable. Cleared as disable.
  * @cur_slave:		As above but for the slave encoder.
+ * @hw_mixer:		Handle to the layer mixers used for the display.
  * @hw_pp:		Handle to the pingpong blocks used for the display. No.
  *			pingpong blocks can be different than num_phys_encs.
  * @hw_cwb:		Handle to the CWB muxes used for concurrent writeback
@@ -182,6 +183,7 @@ struct dpu_encoder_virt {
 	struct dpu_encoder_phys *phys_encs[MAX_PHYS_ENCODERS_PER_VIRTUAL];
 	struct dpu_encoder_phys *cur_master;
 	struct dpu_encoder_phys *cur_slave;
+	struct dpu_hw_mixer *hw_mixer[MAX_CHANNELS_PER_ENC];
 	struct dpu_hw_pingpong *hw_pp[MAX_CHANNELS_PER_ENC];
 	struct dpu_hw_cwb *hw_cwb[MAX_CWB_PER_ENC];
 	struct dpu_hw_dsc *hw_dsc[MAX_CHANNELS_PER_ENC];
@@ -1156,11 +1158,12 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
 	struct msm_drm_private *priv;
 	struct dpu_kms *dpu_kms;
 	struct dpu_global_state *global_state;
+	struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
 	struct dpu_hw_blk *hw_pp[MAX_CHANNELS_PER_ENC];
 	struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
 	struct dpu_hw_blk *hw_dsc[MAX_CHANNELS_PER_ENC];
 	struct dpu_hw_blk *hw_cwb[MAX_CHANNELS_PER_ENC];
-	int num_ctl, num_pp, num_dsc, num_pp_per_intf;
+	int num_ctl, num_lm, num_pp, num_dsc, num_pp_per_intf;
 	int num_cwb = 0;
 	bool is_cwb_encoder;
 	unsigned int dsc_mask = 0;
@@ -1189,6 +1192,14 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
 	trace_dpu_enc_mode_set(DRMID(drm_enc));
 
 	/* Query resource that have been reserved in atomic check step. */
+
+	num_lm = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
+		drm_enc->crtc, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm));
+
+	for (i = 0; i < ARRAY_SIZE(dpu_enc->hw_mixer); i++)
+		dpu_enc->hw_mixer[i] = i < num_lm ? to_dpu_hw_mixer(hw_lm[i])
+						: NULL;
+
 	if (is_cwb_encoder) {
 		num_pp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
 						       drm_enc->crtc,
@@ -2183,42 +2194,34 @@ void dpu_encoder_kickoff(struct drm_encoder *drm_enc)
 
 static void dpu_encoder_helper_reset_mixers(struct dpu_encoder_phys *phys_enc)
 {
-	int i, num_lm;
-	struct dpu_global_state *global_state;
-	struct dpu_hw_blk *hw_lm[2];
-	struct dpu_hw_mixer *hw_mixer[2];
+	int i;
 	struct dpu_hw_ctl *ctl = phys_enc->hw_ctl;
+	struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(phys_enc->parent);
 
 	/* reset all mixers for this encoder */
 	if (ctl->ops.clear_all_blendstages)
 		ctl->ops.clear_all_blendstages(ctl);
 
-	global_state = dpu_kms_get_existing_global_state(phys_enc->dpu_kms);
+	for (i = 0; i < ARRAY_SIZE(dpu_enc->hw_mixer); i++) {
+		if (dpu_enc->hw_mixer[i]) {
+			if (ctl->ops.update_pending_flush_mixer)
+				ctl->ops.update_pending_flush_mixer(ctl, dpu_enc->hw_mixer[i]->idx);
 
-	num_lm = dpu_rm_get_assigned_resources(&phys_enc->dpu_kms->rm, global_state,
-		phys_enc->parent->crtc, DPU_HW_BLK_LM, hw_lm, ARRAY_SIZE(hw_lm));
-
-	for (i = 0; i < num_lm; i++) {
-		hw_mixer[i] = to_dpu_hw_mixer(hw_lm[i]);
-		if (ctl->ops.update_pending_flush_mixer)
-			ctl->ops.update_pending_flush_mixer(ctl, hw_mixer[i]->idx);
-
-		/* clear all blendstages */
-		if (ctl->ops.setup_blendstage)
-			ctl->ops.setup_blendstage(ctl, hw_mixer[i]->idx, NULL);
-
-		if (hw_mixer[i]->ops.clear_all_blendstages)
-			hw_mixer[i]->ops.clear_all_blendstages(hw_mixer[i]);
+			/* clear all blendstages */
+			if (dpu_enc->hw_mixer[i]->ops.clear_all_blendstages)
+				dpu_enc->hw_mixer[i]->ops.clear_all_blendstages(
+					dpu_enc->hw_mixer[i]);
+		}
+	}
 
-		if (ctl->ops.set_active_lms)
-			ctl->ops.set_active_lms(ctl, NULL);
+	if (ctl->ops.set_active_lms)
+		ctl->ops.set_active_lms(ctl, NULL);
 
-		if (ctl->ops.set_active_fetch_pipes)
-			ctl->ops.set_active_fetch_pipes(ctl, NULL);
+	if (ctl->ops.set_active_fetch_pipes)
+		ctl->ops.set_active_fetch_pipes(ctl, NULL);
 
-		if (ctl->ops.set_active_pipes)
-			ctl->ops.set_active_pipes(ctl, NULL);
-	}
+	if (ctl->ops.set_active_pipes)
+		ctl->ops.set_active_pipes(ctl, NULL);
 }
 
 static void dpu_encoder_dsc_pipe_clr(struct dpu_hw_ctl *ctl,

-- 
2.55.0

[PATCH 3/8] drm/msm/dpu: use common helper cleanup function in cmd mode disable

[Xi Zhang via B4 Relay]

From: Xi Zhang <xz.ca.cn@gmail.com>

cmd mode encoder previously did not clean up dsc, or LMs during
disable.

Change to use the common dpu_encoder_helper_phys_cleanup function
for cmd mode disable.

Also do not set intf_cfg.mode_3d during disable:
dpu_encoder_helper_get_3d_blend_mode accesses drm_encoder->crtc->state
directly so intf_cfg.mode_3d weren't set to expected value for disable.
intf_cfg.mode_3d is also ignored by ctl->ops.reset_intf_cfg.

Signed-off-by: Xi Zhang <xz.ca.cn@gmail.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c          |  4 ++--
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 10 +---------
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 09afc0681248..0ab151369c99 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2332,8 +2332,8 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
 		dpu_enc->dsc = NULL;
 	}
 
-	intf_cfg.stream_sel = 0; /* Don't care value for video mode */
-	intf_cfg.mode_3d = dpu_encoder_helper_get_3d_blend_mode(phys_enc);
+	intf_cfg.stream_sel = 0; /* Ignored by reset_intf_cfg */
+	intf_cfg.mode_3d = 0; /* Ignored by reset_intf_cfg */
 	intf_cfg.dsc = dpu_encoder_helper_get_dsc(phys_enc);
 	intf_cfg.cwb = dpu_enc->cwb_mask;
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 45079ee59cf6..111f9dd5e053 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -538,7 +538,6 @@ static void dpu_encoder_phys_cmd_disable(struct dpu_encoder_phys *phys_enc)
 {
 	struct dpu_encoder_phys_cmd *cmd_enc =
 		to_dpu_encoder_phys_cmd(phys_enc);
-	struct dpu_hw_ctl *ctl;
 
 	if (phys_enc->enable_state == DPU_ENC_DISABLED) {
 		DPU_ERROR_CMDENC(cmd_enc, "already disabled\n");
@@ -566,14 +565,7 @@ static void dpu_encoder_phys_cmd_disable(struct dpu_encoder_phys *phys_enc)
 			phys_enc->hw_pp->ops.disable_tearcheck(phys_enc->hw_pp);
 	}
 
-	if (phys_enc->hw_intf->ops.bind_pingpong_blk) {
-		phys_enc->hw_intf->ops.bind_pingpong_blk(
-				phys_enc->hw_intf,
-				PINGPONG_NONE);
-
-		ctl = phys_enc->hw_ctl;
-		ctl->ops.update_pending_flush_intf(ctl, phys_enc->hw_intf->idx);
-	}
+	dpu_encoder_helper_phys_cleanup(phys_enc);
 
 	phys_enc->enable_state = DPU_ENC_DISABLED;
 }

-- 
2.55.0

[PATCH 4/8] drm/msm/dpu: Match setup_dither cleanup to the way it was set up

[Xi Zhang via B4 Relay]

From: Xi Zhang <xz.ca.cn@gmail.com>

Cleanup hw_pp->ops.setup_dither with NULL for all PINGPONGs,
matching the way it was setup during enable.

Signed-off-by: Xi Zhang <xz.ca.cn@gmail.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 0ab151369c99..7d092398fc99 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1320,7 +1320,7 @@ static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
 	if (dpu_enc->disp_info.intf_type == INTF_DSI &&
 			!WARN_ON(dpu_enc->num_phys_encs == 0)) {
 		unsigned bpc = dpu_enc->connector->display_info.bpc;
-		for (i = 0; i < MAX_CHANNELS_PER_ENC; i++) {
+		for (i = 0; i < ARRAY_SIZE(dpu_enc->hw_pp); i++) {
 			if (!dpu_enc->hw_pp[i])
 				continue;
 			_dpu_encoder_setup_dither(dpu_enc->hw_pp[i], bpc);
@@ -2303,8 +2303,10 @@ void dpu_encoder_helper_phys_cleanup(struct dpu_encoder_phys *phys_enc)
 		}
 	}
 
-	if (phys_enc->hw_pp && phys_enc->hw_pp->ops.setup_dither)
-		phys_enc->hw_pp->ops.setup_dither(phys_enc->hw_pp, NULL);
+	if (dpu_enc->disp_info.intf_type == INTF_DSI)
+		for (i = 0; i < ARRAY_SIZE(dpu_enc->hw_pp); i++)
+			if (dpu_enc->hw_pp[i] && dpu_enc->hw_pp[i]->ops.setup_dither)
+				dpu_enc->hw_pp[i]->ops.setup_dither(dpu_enc->hw_pp[i], NULL);
 
 	if (dpu_enc->cwb_mask)
 		dpu_encoder_helper_phys_setup_cwb(phys_enc, false);

-- 
2.55.0

[PATCH 5/8] drm/msm/dpu: use cached PINGPONGs from mode set in cwb setup

[Xi Zhang via B4 Relay]

From: Xi Zhang <xz.ca.cn@gmail.com>

No functional change intended. Clean up only.

The cached hw_pp should be the same as the ones from
dpu_kms_get_existing_global_state during
dpu_encoder_helper_phys_setup_cwb.

Signed-off-by: Xi Zhang <xz.ca.cn@gmail.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 7d092398fc99..f1d2482b787b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2363,11 +2363,6 @@ void dpu_encoder_helper_phys_setup_cwb(struct dpu_encoder_phys *phys_enc,
 	struct dpu_hw_ctl *hw_ctl;
 	struct dpu_hw_cwb_setup_cfg cwb_cfg;
 
-	struct dpu_kms *dpu_kms;
-	struct dpu_global_state *global_state;
-	struct dpu_hw_blk *rt_pp_list[MAX_CHANNELS_PER_ENC];
-	int num_pp;
-
 	if (!phys_enc->hw_wb)
 		return;
 
@@ -2379,33 +2374,20 @@ void dpu_encoder_helper_phys_setup_cwb(struct dpu_encoder_phys *phys_enc,
 		return;
 	}
 
-	dpu_kms = phys_enc->dpu_kms;
-	global_state = dpu_kms_get_existing_global_state(dpu_kms);
-	num_pp = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
-					       phys_enc->parent->crtc,
-					       DPU_HW_BLK_PINGPONG, rt_pp_list,
-					       ARRAY_SIZE(rt_pp_list));
-
-	if (num_pp == 0 || num_pp > MAX_CHANNELS_PER_ENC) {
-		DPU_DEBUG_ENC(dpu_enc, "invalid num_pp %d\n", num_pp);
-		return;
-	}
-
 	/*
 	 * The CWB mux supports using LM or DSPP as tap points. For now,
 	 * always use LM tap point
 	 */
 	cwb_cfg.input = INPUT_MODE_LM_OUT;
 
-	for (int i = 0; i < MAX_CWB_PER_ENC; i++) {
+	for (int i = 0; i < ARRAY_SIZE(dpu_enc->hw_cwb); i++) {
 		hw_cwb = dpu_enc->hw_cwb[i];
 		if (!hw_cwb)
 			continue;
 
 		if (enable) {
-			struct dpu_hw_pingpong *hw_pp =
-					to_dpu_hw_pingpong(rt_pp_list[i]);
-			cwb_cfg.pp_idx = hw_pp->idx;
+			cwb_cfg.pp_idx = i < ARRAY_SIZE(dpu_enc->hw_pp) && dpu_enc->hw_pp[i] ?
+					dpu_enc->hw_pp[i]->idx : PINGPONG_NONE;
 		} else {
 			cwb_cfg.pp_idx = PINGPONG_NONE;
 		}

-- 
2.55.0

[PATCH 6/8] drm/msm/dpu: workaround res re-assignment issues by pairing enc/crtc

[Xi Zhang via B4 Relay]

From: Xi Zhang <xz.ca.cn@gmail.com>

Consider the scenario with 4 crtcs, 3 encoders/displays are
connected. DPU resources is nearly maxed-out, and mode set is
performed on a display which has acquired many dpu resources.
A new crtc is selected by kernel drm for the mode set,
and during atomic_check, the previous crtc won't have a chance
to release its old resources from global_state before the new
crtc tries to allocate new ones. Leading to atomic_check failure.

By having the same crtc for encoder when possible, during
atomic_check each individual crtc will always be able to release
old resources before allocating new ones.

Signed-off-by: Xi Zhang <xz.ca.cn@gmail.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index da3556eb6ecc..13099d6e3ac9 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -879,9 +879,18 @@ static int _dpu_kms_drm_obj_init(struct dpu_kms *dpu_kms)
 		}
 	}
 
-	/* All CRTCs are compatible with all encoders */
-	drm_for_each_encoder(encoder, dev)
-		encoder->possible_crtcs = (1 << dev->mode_config.num_crtc) - 1;
+	/* All CRTCs are compatible with all encoders.
+	 * Nevertheless, if enough CRTCs are available, pair up crtc/encoder
+	 * for better resource re-assignments during atomic_check
+	 * from userspace mode set.
+	 */
+	i = 0;
+	drm_for_each_encoder(encoder, dev) {
+		if (dev->mode_config.num_crtc >= num_encoders)
+			encoder->possible_crtcs = 1 << i++;
+		else
+			encoder->possible_crtcs = (1 << dev->mode_config.num_crtc) - 1;
+	}
 
 	return 0;
 }

-- 
2.55.0

[PATCH 7/8] drm/msm: force mode_changed on color_mgmt_changed, not just ctm

[Xi Zhang via B4 Relay]

From: Xi Zhang <xz.ca.cn@gmail.com>

dpu_crtc_atomic_check calls dpu_crtc_assign_resources when
color_mgmt_changed is set, which is unsafe outside of full modeset.

Set allow_modeset and mode_changed when color_mgmt_changed is set,
not just when ctm is changed.

Also would fix
commit 42f62cd79578 ("drm/msm/dpu: try reserving the DSPP-less LM first")
which was reverted.

Closes: https://gitlab.freedesktop.org/drm/msm/-/work_items/102

Signed-off-by: Xi Zhang <xz.ca.cn@gmail.com>
---
 drivers/gpu/drm/msm/msm_atomic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index a8babf1dbe0d..6f0f30b0728a 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -195,8 +195,7 @@ int msm_atomic_check(struct drm_device *dev, struct drm_atomic_commit *state)
 	 */
 	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,
 				      new_crtc_state, i) {
-		if ((old_crtc_state->ctm && !new_crtc_state->ctm) ||
-		    (!old_crtc_state->ctm && new_crtc_state->ctm)) {
+		if (new_crtc_state->color_mgmt_changed) {
 			new_crtc_state->mode_changed = true;
 			state->allow_modeset = true;
 		}

-- 
2.55.0

[PATCH 8/8] drm/msm/dpu: reset DSC_CMN_MAIN_CNF in dsc disable

[Xi Zhang via B4 Relay]

From: Xi Zhang <xz.ca.cn@gmail.com>

From observation, DSC_CMN_MAIN_CNF has hardware restriction on
differing values between paired even/odd dsc.

Reset it to 0 when dsc disable to avoid issues when a dsc pair
is later used individually for each of its dsc.

Note that currently dpu always assigns dsc in pairs if possible
for power saving. In the future, it may be desired to assign
singular dsc if possible to support high display count. e.g.
when dsc for DisplayPort support is added.

Tested on Ayaneo Pocket DS (SM8550 Kalama)

Signed-off-by: Xi Zhang <xz.ca.cn@gmail.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
index b3395e9c34a1..b8a330d813e6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dsc_1_2.c
@@ -78,6 +78,7 @@ static void dpu_hw_dsc_disable_1_2(struct dpu_hw_dsc *hw_dsc)
 
 	hw = &hw_dsc->hw;
 	sblk = hw_dsc->caps->sblk;
+	DPU_REG_WRITE(hw, DSC_CMN_MAIN_CNF, 0);
 	DPU_REG_WRITE(hw, sblk->ctl.base + DSC_CFG, 0);
 
 	DPU_REG_WRITE(hw, sblk->enc.base + ENC_DF_CTRL, 0);

-- 
2.55.0

Re: [PATCH 1/8] drm/msm/dpu: reset unassigned resource slots to NULL on topology update

[Val Packett]

On 10/26/26 4:16 PM, Xi Zhang wrote:
> While this is not currently the case, it may be possible that topology
> for cwb and dsc resource requirements change between mode set in the
> future.
>
> e.g. when dsc support is added to DisplayPort, dsc may be optionally
> enabled and disabled between different modes due to
> bandwidth requirements.
>
> Set unassigned resource slots to NULL on topology update to avoid
> issues in the future. It's good practice to do so anyway.
>
> Signed-off-by: Xi Zhang <xz.ca.cn@gmail.com>
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 22 +++++++++++++++-------
>   1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0a6be69a3e83..8d2b4d2726f2 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -1205,9 +1205,13 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>   						       ARRAY_SIZE(hw_pp));
>   	}
>   
> -	for (i = 0; i < num_cwb; i++) {
> -		dpu_enc->hw_cwb[i] = to_dpu_hw_cwb(hw_cwb[i]);
> -		cwb_mask |= BIT(dpu_enc->hw_cwb[i]->idx - CWB_0);
> +	for (i = 0; i < ARRAY_SIZE(dpu_enc->hw_cwb); i++) {
> +		if (i < num_cwb) {
> +			dpu_enc->hw_cwb[i] = to_dpu_hw_cwb(hw_cwb[i]);
> +			cwb_mask |= BIT(dpu_enc->hw_cwb[i]->idx - CWB_0);
> +		} else {
> +			dpu_enc->hw_cwb[i] = NULL;
> +		}
>   	}
>   
>   	dpu_enc->cwb_mask = cwb_mask;
> @@ -1215,16 +1219,20 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>   	num_ctl = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>   		drm_enc->crtc, DPU_HW_BLK_CTL, hw_ctl, ARRAY_SIZE(hw_ctl));
>   
> -	for (i = 0; i < MAX_CHANNELS_PER_ENC; i++)
> +	for (i = 0; i < ARRAY_SIZE(dpu_enc->hw_pp); i++)
>   		dpu_enc->hw_pp[i] = i < num_pp ? to_dpu_hw_pingpong(hw_pp[i])
>   						: NULL;
>   
>   	num_dsc = dpu_rm_get_assigned_resources(&dpu_kms->rm, global_state,
>   						drm_enc->crtc, DPU_HW_BLK_DSC,
>   						hw_dsc, ARRAY_SIZE(hw_dsc));
> -	for (i = 0; i < num_dsc; i++) {
> -		dpu_enc->hw_dsc[i] = to_dpu_hw_dsc(hw_dsc[i]);
> -		dsc_mask |= BIT(dpu_enc->hw_dsc[i]->idx - DSC_0);
> +	for (i = 0; i < ARRAY_SIZE(dpu_enc->hw_dsc); i++) {
> +		if (i < num_dsc) {
> +			dpu_enc->hw_dsc[i] = to_dpu_hw_dsc(hw_dsc[i]);
> +			dsc_mask |= BIT(dpu_enc->hw_dsc[i]->idx - DSC_0);
> +		} else {
> +			dpu_enc->hw_dsc[i] = NULL;
> +		}
>   	}
>   
>   	dpu_enc->dsc_mask = dsc_mask;

The radxa 7.0.11 tree which has some DP DSC patches indeed touches this 
code with "drm/msm/dpu: Clear stale DSC resources during modesets":

https://github.com/radxa/kernel/commit/d0a8968dca16965395f4089a28d48ab6acde565e

(but it only clears hw_dsc, but in the hw_pp loop, for the assigned 
resources..)

[+cc sophon]

~val