diff mbox series

[RFC] drm/msm/dpu: Force modeset if new CTLs have been reserved

Message ID 20250303-force-modeset-hw-ctl-v1-1-9cbf6d4fbf8e@quicinc.com (mailing list archive)
State New
Headers show
Series [RFC] drm/msm/dpu: Force modeset if new CTLs have been reserved | expand

Commit Message

Jessica Zhang March 3, 2025, 6:28 p.m. UTC 
If new CTLs are reserved by CRTC but atomic_enable() is skipped, the
encoders will configure the stale CTL instead of the newly reserved one.

Avoid this by setting mode_changed to true if new CTLs have been
reserved by CRTC.

Note: This patch only adds tracking for the CTL reservation, but eventually
all HW blocks used by encoders (i.e. DSC, PINGPONG, CWB) should have a
similar check to avoid the same issue.

Suggested-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Closes: https://lists.freedesktop.org/archives/freedreno/2025-February/036719.html
Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 13 +++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 ++++++++++++
 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  1 +
 3 files changed, 26 insertions(+)


---
base-commit: 866e43b945bf98f8e807dfa45eca92f931f3a032
change-id: 20250228-force-modeset-hw-ctl-d02b80a2bb4c
prerequisite-change-id: 20241222-drm-dirty-modeset-88079bd27ae6:v2
prerequisite-patch-id: 0c61aabfcd13651203f476985380cbf4d3c299e6
prerequisite-patch-id: c6026f08011c288fd301676e9fa6f46d0cc1dab7
prerequisite-patch-id: b0cb06d5c88791d6e4755d879ced0d5050aa3cbf
prerequisite-patch-id: fd72ddde9dba0df053113bc505c213961a9760da
prerequisite-change-id: 20250209-dpu-c3fac78fc617:v2
prerequisite-patch-id: c84d2b4b06be06384968429085d1e8ebae23a583
prerequisite-patch-id: fb8ea7b9e7c85fabd27589c6551108382a235002
prerequisite-change-id: 20250211-dither-disable-b77b1e31977f:v1
prerequisite-patch-id: 079e04296212b4b83d51394b5a9b5eea6870d98a
prerequisite-change-id: 20240618-concurrent-wb-97d62387f952:v6
prerequisite-patch-id: b52034179741dc182aea9411fd446e270fdc69d1
prerequisite-patch-id: bc472765a7d5214691f3d92696cc8b0119f3252e
prerequisite-patch-id: c959bc480e96b04297ebaf30fea3a68bbac69da6
prerequisite-patch-id: f7db8449b241a41faac357d9257f8c7cb16503ec
prerequisite-patch-id: 7beb73131d0ab100f266fcd3c1f67c818a3263f4
prerequisite-patch-id: c08cbb5cf4e67e308afd61fdad6684b89429d3b6
prerequisite-patch-id: a4e343143b8fbe98ae4aa068cc459c750105eb9d
prerequisite-patch-id: 1d09edcf12ef7e7ab43547eefacae5b604b698e9
prerequisite-patch-id: 0008f9802bfd3c5877267666cceb7608203e5830
prerequisite-patch-id: 49402eb767c97915faf2378c5f5d05ced2dcfdac
prerequisite-patch-id: 522be2a6b5fe4e3a2d609526bb1539f9bc6f828f
prerequisite-patch-id: 031da00d0fffd522f74d682a551362f3ecda0c71
prerequisite-patch-id: 9454cec22231a8f3f01c33d52a5df3e26dd88287
prerequisite-patch-id: 7edbeaace3549332e581bee3183a76b0e4d18163

Best regards,

Comments

Dmitry Baryshkov March 3, 2025, 11:49 p.m. UTC | #1 
On Mon, Mar 03, 2025 at 10:28:00AM -0800, Jessica Zhang wrote:
> If new CTLs are reserved by CRTC but atomic_enable() is skipped, the
> encoders will configure the stale CTL instead of the newly reserved one.

The CTLs are propagates in .atomic_mode_set(), not in .atomic_enable().

> 
> Avoid this by setting mode_changed to true if new CTLs have been
> reserved by CRTC.

This looks very strange. First we reserve new CTLs when there is a
modeset requested. Then on one of the next commits we detect that
encoder has stale CTLs and try to upgrade the commit to full modeset
(while the user might not have .allow_modeset set to true for whatever
reason, e.g. because only ACTIVE is changed).

Could you please check if the following change fixes the issue: in
crtc_set_mode() replace the raw !new_crtc_state->mode_changed check with
the drm_atomic_crtc_needs_modeset() call?

> 
> Note: This patch only adds tracking for the CTL reservation, but eventually
> all HW blocks used by encoders (i.e. DSC, PINGPONG, CWB) should have a
> similar check to avoid the same issue.
> 
> Suggested-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Closes: https://lists.freedesktop.org/archives/freedreno/2025-February/036719.html
> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 13 +++++++++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 ++++++++++++
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  1 +
>  3 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 4073d821158c0..a1a8be8f5ab9f 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1406,19 +1406,32 @@ int dpu_crtc_check_mode_changed(struct drm_crtc_state *old_crtc_state,
>  	struct drm_crtc *crtc = new_crtc_state->crtc;
>  	bool clone_mode_enabled = drm_crtc_in_clone_mode(old_crtc_state);
>  	bool clone_mode_requested = drm_crtc_in_clone_mode(new_crtc_state);
> +	struct dpu_crtc_state *cstate = to_dpu_crtc_state(new_crtc_state);
> +	uint32_t enc_ctl_mask = 0;
> +	uint32_t crtc_ctl_mask = 0;
> +	struct dpu_crtc_mixer *m;
>  
>  	DRM_DEBUG_ATOMIC("%d\n", crtc->base.id);
>  
> +	for (int i = 0; i < cstate->num_mixers; i++) {
> +		m = &cstate->mixers[i];
> +		crtc_ctl_mask |= BIT(m->lm_ctl->idx - CTL_0);
> +	}
> +
>  	/* there might be cases where encoder needs a modeset too */
>  	drm_for_each_encoder_mask(drm_enc, crtc->dev, new_crtc_state->encoder_mask) {
>  		if (dpu_encoder_needs_modeset(drm_enc, new_crtc_state->state))
>  			new_crtc_state->mode_changed = true;
> +		enc_ctl_mask |= dpu_encoder_get_ctls(drm_enc);
>  	}
>  
>  	if ((clone_mode_requested && !clone_mode_enabled) ||
>  	    (!clone_mode_requested && clone_mode_enabled))
>  		new_crtc_state->mode_changed = true;
>  
> +	if (crtc_ctl_mask != enc_ctl_mask)
> +		new_crtc_state->mode_changed = true;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index a61598710acda..2f3101caeba91 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -188,6 +188,7 @@ struct dpu_encoder_virt {
>  
>  	unsigned int dsc_mask;
>  	unsigned int cwb_mask;
> +	unsigned int ctl_mask;
>  
>  	bool intfs_swapped;
>  
> @@ -707,6 +708,13 @@ void dpu_encoder_update_topology(struct drm_encoder *drm_enc,
>  	}
>  }
>  
> +uint32_t dpu_encoder_get_ctls(struct drm_encoder *drm_enc)
> +{
> +	struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
> +
> +	return dpu_enc->ctl_mask;
> +}
> +
>  bool dpu_encoder_needs_modeset(struct drm_encoder *drm_enc, struct drm_atomic_state *state)
>  {
>  	struct drm_connector *connector;
> @@ -1155,6 +1163,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>  	bool is_cwb_encoder;
>  	unsigned int dsc_mask = 0;
>  	unsigned int cwb_mask = 0;
> +	unsigned int ctl_mask = 0;
>  	int i;
>  
>  	if (!drm_enc) {
> @@ -1245,11 +1254,14 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>  				"no ctl block assigned at idx: %d\n", i);
>  			return;
>  		}
> +		ctl_mask |= BIT(phys->hw_ctl->idx - CTL_0);
>  
>  		phys->cached_mode = crtc_state->adjusted_mode;
>  		if (phys->ops.atomic_mode_set)
>  			phys->ops.atomic_mode_set(phys, crtc_state, conn_state);
>  	}
> +
> +	dpu_enc->ctl_mask = ctl_mask;
>  }
>  
>  static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> index ca1ca2e51d7ea..70b03743dc346 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
> @@ -91,6 +91,7 @@ bool dpu_encoder_needs_modeset(struct drm_encoder *drm_enc, struct drm_atomic_st
>  
>  void dpu_encoder_prepare_wb_job(struct drm_encoder *drm_enc,
>  		struct drm_writeback_job *job);
> +uint32_t dpu_encoder_get_ctls(struct drm_encoder *drm_enc);
>  
>  void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc,
>  		struct drm_writeback_job *job);
> 
> ---
> base-commit: 866e43b945bf98f8e807dfa45eca92f931f3a032
> change-id: 20250228-force-modeset-hw-ctl-d02b80a2bb4c
> prerequisite-change-id: 20241222-drm-dirty-modeset-88079bd27ae6:v2
> prerequisite-patch-id: 0c61aabfcd13651203f476985380cbf4d3c299e6
> prerequisite-patch-id: c6026f08011c288fd301676e9fa6f46d0cc1dab7
> prerequisite-patch-id: b0cb06d5c88791d6e4755d879ced0d5050aa3cbf
> prerequisite-patch-id: fd72ddde9dba0df053113bc505c213961a9760da
> prerequisite-change-id: 20250209-dpu-c3fac78fc617:v2
> prerequisite-patch-id: c84d2b4b06be06384968429085d1e8ebae23a583
> prerequisite-patch-id: fb8ea7b9e7c85fabd27589c6551108382a235002
> prerequisite-change-id: 20250211-dither-disable-b77b1e31977f:v1
> prerequisite-patch-id: 079e04296212b4b83d51394b5a9b5eea6870d98a
> prerequisite-change-id: 20240618-concurrent-wb-97d62387f952:v6
> prerequisite-patch-id: b52034179741dc182aea9411fd446e270fdc69d1
> prerequisite-patch-id: bc472765a7d5214691f3d92696cc8b0119f3252e
> prerequisite-patch-id: c959bc480e96b04297ebaf30fea3a68bbac69da6
> prerequisite-patch-id: f7db8449b241a41faac357d9257f8c7cb16503ec
> prerequisite-patch-id: 7beb73131d0ab100f266fcd3c1f67c818a3263f4
> prerequisite-patch-id: c08cbb5cf4e67e308afd61fdad6684b89429d3b6
> prerequisite-patch-id: a4e343143b8fbe98ae4aa068cc459c750105eb9d
> prerequisite-patch-id: 1d09edcf12ef7e7ab43547eefacae5b604b698e9
> prerequisite-patch-id: 0008f9802bfd3c5877267666cceb7608203e5830
> prerequisite-patch-id: 49402eb767c97915faf2378c5f5d05ced2dcfdac
> prerequisite-patch-id: 522be2a6b5fe4e3a2d609526bb1539f9bc6f828f
> prerequisite-patch-id: 031da00d0fffd522f74d682a551362f3ecda0c71
> prerequisite-patch-id: 9454cec22231a8f3f01c33d52a5df3e26dd88287
> prerequisite-patch-id: 7edbeaace3549332e581bee3183a76b0e4d18163
> 
> Best regards,
> -- 
> Jessica Zhang <quic_jesszhan@quicinc.com>
>
Jessica Zhang March 4, 2025, 2:44 a.m. UTC | #2 
On 3/3/2025 3:49 PM, Dmitry Baryshkov wrote:
> On Mon, Mar 03, 2025 at 10:28:00AM -0800, Jessica Zhang wrote:
>> If new CTLs are reserved by CRTC but atomic_enable() is skipped, the
>> encoders will configure the stale CTL instead of the newly reserved one.
> 
> The CTLs are propagates in .atomic_mode_set(), not in .atomic_enable().

Hi Dmitry,

Yes, sorry mixed up the two function ops here and in my reply in the CWB 
thread.

> 
>>
>> Avoid this by setting mode_changed to true if new CTLs have been
>> reserved by CRTC.
> 
> This looks very strange. First we reserve new CTLs when there is a
> modeset requested. Then on one of the next commits we detect that
> encoder has stale CTLs and try to upgrade the commit to full modeset
> (while the user might not have .allow_modeset set to true for whatever
> reason, e.g. because only ACTIVE is changed).

Ah I see what you mean. I think this is an issue with how/when we're 
calling dpu_rm_reserve(). Since RM reservation is tied to 
atomic_check(), we aren't able to force a modeset based on HW block 
reservation. The only reason we were able to avoid this issue with 
needs_cdm is because needs_cdm didn't depend on the CDM HW block index.

I think there's not really a good way to avoid this other than flipping 
the order of the msm_atomic_check to drm_helper_atomic_check -> 
dpu_kms.check_mode_changed -> drm_atomic_helper_check_modeset().

What do you think? It seems to be valid given the examples in the DRM 
docs [1]

[1] 
https://elixir.bootlin.com/linux/v6.13.5/source/drivers/gpu/drm/drm_atomic_helper.c#L610

> 
> Could you please check if the following change fixes the issue: in
> crtc_set_mode() replace the raw !new_crtc_state->mode_changed check with
> the drm_atomic_crtc_needs_modeset() call?

This also fixes the DPMS failures. IIRC Abhinav had suggested a similar 
change to fix a different issue [2] and you gave some feedback on 
avoiding mode_set() for enable/disable calls which don't have mode_changed.

Also, while this may fix the CWB CI failures, wouldn't the issue still 
remain regarding how to force modeset for changes in HW block reservation?

[2] https://gitlab.freedesktop.org/drm/msm/-/issues/59

Thanks,

Jessica Zhang

> 
>>
>> Note: This patch only adds tracking for the CTL reservation, but eventually
>> all HW blocks used by encoders (i.e. DSC, PINGPONG, CWB) should have a
>> similar check to avoid the same issue.
>>
>> Suggested-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
>> Closes: https://lists.freedesktop.org/archives/freedreno/2025-February/036719.html
>> Signed-off-by: Jessica Zhang <quic_jesszhan@quicinc.com>
>> ---
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c    | 13 +++++++++++++
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 ++++++++++++
>>   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h |  1 +
>>   3 files changed, 26 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> index 4073d821158c0..a1a8be8f5ab9f 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> @@ -1406,19 +1406,32 @@ int dpu_crtc_check_mode_changed(struct drm_crtc_state *old_crtc_state,
>>   	struct drm_crtc *crtc = new_crtc_state->crtc;
>>   	bool clone_mode_enabled = drm_crtc_in_clone_mode(old_crtc_state);
>>   	bool clone_mode_requested = drm_crtc_in_clone_mode(new_crtc_state);
>> +	struct dpu_crtc_state *cstate = to_dpu_crtc_state(new_crtc_state);
>> +	uint32_t enc_ctl_mask = 0;
>> +	uint32_t crtc_ctl_mask = 0;
>> +	struct dpu_crtc_mixer *m;
>>   
>>   	DRM_DEBUG_ATOMIC("%d\n", crtc->base.id);
>>   
>> +	for (int i = 0; i < cstate->num_mixers; i++) {
>> +		m = &cstate->mixers[i];
>> +		crtc_ctl_mask |= BIT(m->lm_ctl->idx - CTL_0);
>> +	}
>> +
>>   	/* there might be cases where encoder needs a modeset too */
>>   	drm_for_each_encoder_mask(drm_enc, crtc->dev, new_crtc_state->encoder_mask) {
>>   		if (dpu_encoder_needs_modeset(drm_enc, new_crtc_state->state))
>>   			new_crtc_state->mode_changed = true;
>> +		enc_ctl_mask |= dpu_encoder_get_ctls(drm_enc);
>>   	}
>>   
>>   	if ((clone_mode_requested && !clone_mode_enabled) ||
>>   	    (!clone_mode_requested && clone_mode_enabled))
>>   		new_crtc_state->mode_changed = true;
>>   
>> +	if (crtc_ctl_mask != enc_ctl_mask)
>> +		new_crtc_state->mode_changed = true;
>> +
>>   	return 0;
>>   }
>>   
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> index a61598710acda..2f3101caeba91 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
>> @@ -188,6 +188,7 @@ struct dpu_encoder_virt {
>>   
>>   	unsigned int dsc_mask;
>>   	unsigned int cwb_mask;
>> +	unsigned int ctl_mask;
>>   
>>   	bool intfs_swapped;
>>   
>> @@ -707,6 +708,13 @@ void dpu_encoder_update_topology(struct drm_encoder *drm_enc,
>>   	}
>>   }
>>   
>> +uint32_t dpu_encoder_get_ctls(struct drm_encoder *drm_enc)
>> +{
>> +	struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
>> +
>> +	return dpu_enc->ctl_mask;
>> +}
>> +
>>   bool dpu_encoder_needs_modeset(struct drm_encoder *drm_enc, struct drm_atomic_state *state)
>>   {
>>   	struct drm_connector *connector;
>> @@ -1155,6 +1163,7 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>>   	bool is_cwb_encoder;
>>   	unsigned int dsc_mask = 0;
>>   	unsigned int cwb_mask = 0;
>> +	unsigned int ctl_mask = 0;
>>   	int i;
>>   
>>   	if (!drm_enc) {
>> @@ -1245,11 +1254,14 @@ static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
>>   				"no ctl block assigned at idx: %d\n", i);
>>   			return;
>>   		}
>> +		ctl_mask |= BIT(phys->hw_ctl->idx - CTL_0);
>>   
>>   		phys->cached_mode = crtc_state->adjusted_mode;
>>   		if (phys->ops.atomic_mode_set)
>>   			phys->ops.atomic_mode_set(phys, crtc_state, conn_state);
>>   	}
>> +
>> +	dpu_enc->ctl_mask = ctl_mask;
>>   }
>>   
>>   static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
>> index ca1ca2e51d7ea..70b03743dc346 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
>> @@ -91,6 +91,7 @@ bool dpu_encoder_needs_modeset(struct drm_encoder *drm_enc, struct drm_atomic_st
>>   
>>   void dpu_encoder_prepare_wb_job(struct drm_encoder *drm_enc,
>>   		struct drm_writeback_job *job);
>> +uint32_t dpu_encoder_get_ctls(struct drm_encoder *drm_enc);
>>   
>>   void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc,
>>   		struct drm_writeback_job *job);
>>
>> ---
>> base-commit: 866e43b945bf98f8e807dfa45eca92f931f3a032
>> change-id: 20250228-force-modeset-hw-ctl-d02b80a2bb4c
>> prerequisite-change-id: 20241222-drm-dirty-modeset-88079bd27ae6:v2
>> prerequisite-patch-id: 0c61aabfcd13651203f476985380cbf4d3c299e6
>> prerequisite-patch-id: c6026f08011c288fd301676e9fa6f46d0cc1dab7
>> prerequisite-patch-id: b0cb06d5c88791d6e4755d879ced0d5050aa3cbf
>> prerequisite-patch-id: fd72ddde9dba0df053113bc505c213961a9760da
>> prerequisite-change-id: 20250209-dpu-c3fac78fc617:v2
>> prerequisite-patch-id: c84d2b4b06be06384968429085d1e8ebae23a583
>> prerequisite-patch-id: fb8ea7b9e7c85fabd27589c6551108382a235002
>> prerequisite-change-id: 20250211-dither-disable-b77b1e31977f:v1
>> prerequisite-patch-id: 079e04296212b4b83d51394b5a9b5eea6870d98a
>> prerequisite-change-id: 20240618-concurrent-wb-97d62387f952:v6
>> prerequisite-patch-id: b52034179741dc182aea9411fd446e270fdc69d1
>> prerequisite-patch-id: bc472765a7d5214691f3d92696cc8b0119f3252e
>> prerequisite-patch-id: c959bc480e96b04297ebaf30fea3a68bbac69da6
>> prerequisite-patch-id: f7db8449b241a41faac357d9257f8c7cb16503ec
>> prerequisite-patch-id: 7beb73131d0ab100f266fcd3c1f67c818a3263f4
>> prerequisite-patch-id: c08cbb5cf4e67e308afd61fdad6684b89429d3b6
>> prerequisite-patch-id: a4e343143b8fbe98ae4aa068cc459c750105eb9d
>> prerequisite-patch-id: 1d09edcf12ef7e7ab43547eefacae5b604b698e9
>> prerequisite-patch-id: 0008f9802bfd3c5877267666cceb7608203e5830
>> prerequisite-patch-id: 49402eb767c97915faf2378c5f5d05ced2dcfdac
>> prerequisite-patch-id: 522be2a6b5fe4e3a2d609526bb1539f9bc6f828f
>> prerequisite-patch-id: 031da00d0fffd522f74d682a551362f3ecda0c71
>> prerequisite-patch-id: 9454cec22231a8f3f01c33d52a5df3e26dd88287
>> prerequisite-patch-id: 7edbeaace3549332e581bee3183a76b0e4d18163
>>
>> Best regards,
>> -- 
>> Jessica Zhang <quic_jesszhan@quicinc.com>
>>
> 
> -- 
> With best wishes
> Dmitry
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 4073d821158c0..a1a8be8f5ab9f 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1406,19 +1406,32 @@  int dpu_crtc_check_mode_changed(struct drm_crtc_state *old_crtc_state,
 	struct drm_crtc *crtc = new_crtc_state->crtc;
 	bool clone_mode_enabled = drm_crtc_in_clone_mode(old_crtc_state);
 	bool clone_mode_requested = drm_crtc_in_clone_mode(new_crtc_state);
+	struct dpu_crtc_state *cstate = to_dpu_crtc_state(new_crtc_state);
+	uint32_t enc_ctl_mask = 0;
+	uint32_t crtc_ctl_mask = 0;
+	struct dpu_crtc_mixer *m;
 
 	DRM_DEBUG_ATOMIC("%d\n", crtc->base.id);
 
+	for (int i = 0; i < cstate->num_mixers; i++) {
+		m = &cstate->mixers[i];
+		crtc_ctl_mask |= BIT(m->lm_ctl->idx - CTL_0);
+	}
+
 	/* there might be cases where encoder needs a modeset too */
 	drm_for_each_encoder_mask(drm_enc, crtc->dev, new_crtc_state->encoder_mask) {
 		if (dpu_encoder_needs_modeset(drm_enc, new_crtc_state->state))
 			new_crtc_state->mode_changed = true;
+		enc_ctl_mask |= dpu_encoder_get_ctls(drm_enc);
 	}
 
 	if ((clone_mode_requested && !clone_mode_enabled) ||
 	    (!clone_mode_requested && clone_mode_enabled))
 		new_crtc_state->mode_changed = true;
 
+	if (crtc_ctl_mask != enc_ctl_mask)
+		new_crtc_state->mode_changed = true;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index a61598710acda..2f3101caeba91 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -188,6 +188,7 @@  struct dpu_encoder_virt {
 
 	unsigned int dsc_mask;
 	unsigned int cwb_mask;
+	unsigned int ctl_mask;
 
 	bool intfs_swapped;
 
@@ -707,6 +708,13 @@  void dpu_encoder_update_topology(struct drm_encoder *drm_enc,
 	}
 }
 
+uint32_t dpu_encoder_get_ctls(struct drm_encoder *drm_enc)
+{
+	struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
+
+	return dpu_enc->ctl_mask;
+}
+
 bool dpu_encoder_needs_modeset(struct drm_encoder *drm_enc, struct drm_atomic_state *state)
 {
 	struct drm_connector *connector;
@@ -1155,6 +1163,7 @@  static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
 	bool is_cwb_encoder;
 	unsigned int dsc_mask = 0;
 	unsigned int cwb_mask = 0;
+	unsigned int ctl_mask = 0;
 	int i;
 
 	if (!drm_enc) {
@@ -1245,11 +1254,14 @@  static void dpu_encoder_virt_atomic_mode_set(struct drm_encoder *drm_enc,
 				"no ctl block assigned at idx: %d\n", i);
 			return;
 		}
+		ctl_mask |= BIT(phys->hw_ctl->idx - CTL_0);
 
 		phys->cached_mode = crtc_state->adjusted_mode;
 		if (phys->ops.atomic_mode_set)
 			phys->ops.atomic_mode_set(phys, crtc_state, conn_state);
 	}
+
+	dpu_enc->ctl_mask = ctl_mask;
 }
 
 static void _dpu_encoder_virt_enable_helper(struct drm_encoder *drm_enc)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index ca1ca2e51d7ea..70b03743dc346 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -91,6 +91,7 @@  bool dpu_encoder_needs_modeset(struct drm_encoder *drm_enc, struct drm_atomic_st
 
 void dpu_encoder_prepare_wb_job(struct drm_encoder *drm_enc,
 		struct drm_writeback_job *job);
+uint32_t dpu_encoder_get_ctls(struct drm_encoder *drm_enc);
 
 void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc,
 		struct drm_writeback_job *job);