Commit 2dde2aad authored by Dmitry Baryshkov's avatar Dmitry Baryshkov
Browse files

drm/msm/dpu: don't set crtc_state->mode_changed from atomic_check() 



The MSM driver uses drm_atomic_helper_check() which mandates that none
of the atomic_check() callbacks toggles crtc_state->mode_changed.
Perform corresponding check before calling the drm_atomic_helper_check()
function.

Fixes: 8b45a26f ("drm/msm/dpu: reserve cdm blocks for writeback in case of YUV output")
Reported-by: default avatarSimona Vetter <simona.vetter@ffwll.ch>
Closes: https://lore.kernel.org/dri-devel/ZtW_S0j5AEr4g0QW@phenom.ffwll.local/


Reviewed-by: default avatarAbhinav Kumar <quic_abhinavk@quicinc.com>
[DB: dropped the WARN_ON]
Signed-off-by: default avatarDmitry Baryshkov <dmitry.baryshkov@linaro.org>
Patchwork: https://patchwork.freedesktop.org/patch/633400/
Link: https://lore.kernel.org/r/20250123-drm-dirty-modeset-v2-4-bbfd3a6cd1a4@linaro.org
parent 78e70fa0

drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c

+28 −4
Original line number Diff line number Diff line
@@ -760,6 +760,34 @@ static void dpu_encoder_assign_crtc_resources(struct dpu_kms *dpu_kms, 
	cstate->num_mixers = num_lm;

}



/**

 * dpu_encoder_virt_check_mode_changed: check if full modeset is required

 * @drm_enc:    Pointer to drm encoder structure

 * @crtc_state:	Corresponding CRTC state to be checked

 * @conn_state: Corresponding Connector's state to be checked

 *

 * Check if the changes in the object properties demand full mode set.

 */

int dpu_encoder_virt_check_mode_changed(struct drm_encoder *drm_enc,

					struct drm_crtc_state *crtc_state,

					struct drm_connector_state *conn_state)

{

	struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);

	struct msm_display_topology topology;



	DPU_DEBUG_ENC(dpu_enc, "\n");



	/* Using mode instead of adjusted_mode as it wasn't computed yet */

	topology = dpu_encoder_get_topology(dpu_enc, &crtc_state->mode, crtc_state, conn_state);



	if (topology.needs_cdm && !dpu_enc->cur_master->hw_cdm)

		crtc_state->mode_changed = true;

	else if (!topology.needs_cdm && dpu_enc->cur_master->hw_cdm)

		crtc_state->mode_changed = true;



	return 0;

}



static int dpu_encoder_virt_atomic_check(

		struct drm_encoder *drm_enc,

		struct drm_crtc_state *crtc_state,
@@ -793,10 +821,6 @@ static int dpu_encoder_virt_atomic_check( 


	topology = dpu_encoder_get_topology(dpu_enc, adj_mode, crtc_state, conn_state);



	if (topology.needs_cdm && !dpu_enc->cur_master->hw_cdm)

		crtc_state->mode_changed = true;

	else if (!topology.needs_cdm && dpu_enc->cur_master->hw_cdm)

		crtc_state->mode_changed = true;

	/*

	 * Release and Allocate resources on every modeset

	 */

drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h

+4 −0
Original line number Diff line number Diff line
@@ -88,4 +88,8 @@ void dpu_encoder_cleanup_wb_job(struct drm_encoder *drm_enc, 


bool dpu_encoder_is_valid_for_commit(struct drm_encoder *drm_enc);



int dpu_encoder_virt_check_mode_changed(struct drm_encoder *drm_enc,

					struct drm_crtc_state *crtc_state,

					struct drm_connector_state *conn_state);



#endif /* __DPU_ENCODER_H__ */

drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c

+24 −0
Original line number Diff line number Diff line
@@ -446,6 +446,29 @@ static void dpu_kms_disable_commit(struct msm_kms *kms) 
	pm_runtime_put_sync(&dpu_kms->pdev->dev);

}



static int dpu_kms_check_mode_changed(struct msm_kms *kms, struct drm_atomic_state *state)

{

	struct drm_crtc_state *new_crtc_state;

	struct drm_connector *connector;

	struct drm_connector_state *new_conn_state;

	int i;



	for_each_new_connector_in_state(state, connector, new_conn_state, i) {

		struct drm_encoder *encoder;



		if (!new_conn_state->crtc || !new_conn_state->best_encoder)

			continue;



		new_crtc_state = drm_atomic_get_new_crtc_state(state, new_conn_state->crtc);



		encoder = new_conn_state->best_encoder;



		dpu_encoder_virt_check_mode_changed(encoder, new_crtc_state, new_conn_state);

	}



	return 0;

}



static void dpu_kms_flush_commit(struct msm_kms *kms, unsigned crtc_mask)

{

	struct dpu_kms *dpu_kms = to_dpu_kms(kms);
@@ -1062,6 +1085,7 @@ static const struct msm_kms_funcs kms_funcs = { 
	.irq             = dpu_core_irq,

	.enable_commit   = dpu_kms_enable_commit,

	.disable_commit  = dpu_kms_disable_commit,

	.check_mode_changed = dpu_kms_check_mode_changed,

	.flush_commit    = dpu_kms_flush_commit,

	.wait_flush      = dpu_kms_wait_flush,

	.complete_commit = dpu_kms_complete_commit,

drivers/gpu/drm/msm/msm_atomic.c

+12 −1
Original line number Diff line number Diff line
@@ -183,10 +183,16 @@ static unsigned get_crtc_mask(struct drm_atomic_state *state) 


int msm_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)

{

	struct msm_drm_private *priv = dev->dev_private;

	struct msm_kms *kms = priv->kms;

	struct drm_crtc_state *old_crtc_state, *new_crtc_state;

	struct drm_crtc *crtc;

	int i;

	int i, ret = 0;



	/*

	 * FIXME: stop setting allow_modeset and move this check to the DPU

	 * driver.

	 */

	for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state,

				      new_crtc_state, i) {

		if ((old_crtc_state->ctm && !new_crtc_state->ctm) ||
@@ -196,6 +202,11 @@ int msm_atomic_check(struct drm_device *dev, struct drm_atomic_state *state) 
		}

	}



	if (kms && kms->funcs && kms->funcs->check_mode_changed)

		ret = kms->funcs->check_mode_changed(kms, state);

	if (ret)

		return ret;



	return drm_atomic_helper_check(dev, state);

}

drivers/gpu/drm/msm/msm_kms.h

+7 −0
Original line number Diff line number Diff line
@@ -59,6 +59,13 @@ struct msm_kms_funcs { 
	void (*enable_commit)(struct msm_kms *kms);

	void (*disable_commit)(struct msm_kms *kms);



	/**

	 * @check_mode_changed:

	 *

	 * Verify if the commit requires a full modeset on one of CRTCs.

	 */

	int (*check_mode_changed)(struct msm_kms *kms, struct drm_atomic_state *state);



	/**

	 * Prepare for atomic commit.  This is called after any previous

	 * (async or otherwise) commit has completed.