Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit aa064ada authored by Steve Cohen's avatar Steve Cohen
Browse files

disp: msm: sde: cache encoder_mask for vblank_work 



There is a race between disable commit swap-state on display
thread and vblank work on event thread which may skip applying
the vblank vote if the encoder_mask has already been cleared.
To avoid the race, use a cached encoder_mask that is gauranteed
to be valid between the vblank_on and vblank_off calls.

Also, vblank queue work is initializing a variable that's never
used and msm_disable_outputs has 2 instances of pointers to old
CRTC state. Remove this dead code.

Change-Id: I5e4a482b8f067e272a2aef5afa08cc0e1ab89434
Signed-off-by: default avatarSteve Cohen <cohens@codeaurora.org>
parent 7a98aa21

msm/msm_atomic.c

+0 −1
Original line number Diff line number Diff line
@@ -156,7 +156,6 @@ msm_disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state) 
			old_conn_state, i) {

		const struct drm_encoder_helper_funcs *funcs;

		struct drm_encoder *encoder;

		struct drm_crtc_state *old_crtc_state;



		/*

		 * Shut down everything that's in the changeset and currently

msm/msm_drv.c

+0 −3
Original line number Diff line number Diff line
@@ -366,7 +366,6 @@ static int vblank_ctrl_queue_work(struct msm_drm_private *priv, 
					int crtc_id, bool enable)

{

	struct vblank_work *cur_work;

	struct drm_crtc *crtc;

	struct kthread_worker *worker;



	if (!priv || crtc_id >= priv->num_crtcs)
@@ -376,8 +375,6 @@ static int vblank_ctrl_queue_work(struct msm_drm_private *priv, 
	if (!cur_work)

		return -ENOMEM;



	crtc = priv->crtcs[crtc_id];



	kthread_init_work(&cur_work->work, vblank_ctrl_worker);

	cur_work->crtc_id = crtc_id;

	cur_work->enable = enable;

msm/sde/sde_crtc.c

+21 −21
Original line number Diff line number Diff line
@@ -3820,13 +3820,13 @@ void sde_crtc_commit_kickoff(struct drm_crtc *crtc, 
}



/**

 * _sde_crtc_vblank_enable_no_lock - update power resource and vblank request

 * _sde_crtc_vblank_enable - update power resource and vblank request

 * @sde_crtc: Pointer to sde crtc structure

 * @enable: Whether to enable/disable vblanks

 *

 * @Return: error code

 */

static int _sde_crtc_vblank_enable_no_lock(

static int _sde_crtc_vblank_enable(

		struct sde_crtc *sde_crtc, bool enable)

{

	struct drm_crtc *crtc;
@@ -3838,38 +3838,38 @@ static int _sde_crtc_vblank_enable_no_lock( 
	}



	crtc = &sde_crtc->base;

	SDE_EVT32(DRMID(crtc), enable, sde_crtc->enabled,

			crtc->state->encoder_mask,

			sde_crtc->cached_encoder_mask);



	if (enable) {

		int ret;



		/* drop lock since power crtc cb may try to re-acquire lock */

		mutex_unlock(&sde_crtc->crtc_lock);

		ret = pm_runtime_get_sync(crtc->dev->dev);

		mutex_lock(&sde_crtc->crtc_lock);

		if (ret < 0)

			return ret;



		mutex_lock(&sde_crtc->crtc_lock);

		drm_for_each_encoder_mask(enc, crtc->dev,

				crtc->state->encoder_mask) {

			SDE_EVT32(DRMID(&sde_crtc->base), DRMID(enc), enable,

					sde_crtc->enabled);

				sde_crtc->cached_encoder_mask) {

			SDE_EVT32(DRMID(crtc), DRMID(enc));



			sde_encoder_register_vblank_callback(enc,

					sde_crtc_vblank_cb, (void *)crtc);

		}



		mutex_unlock(&sde_crtc->crtc_lock);

	} else {

		mutex_lock(&sde_crtc->crtc_lock);

		drm_for_each_encoder_mask(enc, crtc->dev,

				crtc->state->encoder_mask) {

			SDE_EVT32(DRMID(&sde_crtc->base), DRMID(enc), enable,

					sde_crtc->enabled);

				sde_crtc->cached_encoder_mask) {

			SDE_EVT32(DRMID(crtc), DRMID(enc));



			sde_encoder_register_vblank_callback(enc, NULL, NULL);

		}



		/* drop lock since power crtc cb may try to re-acquire lock */

		mutex_unlock(&sde_crtc->crtc_lock);

		pm_runtime_put_sync(crtc->dev->dev);

		mutex_lock(&sde_crtc->crtc_lock);

	}



	return 0;
@@ -4165,9 +4165,10 @@ static void sde_crtc_disable(struct drm_crtc *crtc) 
	kthread_cancel_delayed_work_sync(&sde_crtc->static_cache_read_work);

	kthread_cancel_delayed_work_sync(&sde_crtc->idle_notify_work);



	SDE_EVT32(DRMID(crtc), sde_crtc->enabled,

			crtc->state->active, crtc->state->enable);

	SDE_EVT32(DRMID(crtc), sde_crtc->enabled, crtc->state->active,

			crtc->state->enable, sde_crtc->cached_encoder_mask);

	sde_crtc->enabled = false;

	sde_crtc->cached_encoder_mask = 0;



	/* Try to disable uidle */

	sde_core_perf_crtc_update_uidle(crtc, false);
@@ -4276,8 +4277,11 @@ static void sde_crtc_enable(struct drm_crtc *crtc, 
	 * Avoid drm_crtc_vblank_on during seamless DMS case

	 * when CRTC is already in enabled state

	 */

	if (!sde_crtc->enabled)

	if (!sde_crtc->enabled) {

		/* cache the encoder mask now for vblank work */

		sde_crtc->cached_encoder_mask = crtc->state->encoder_mask;

		drm_crtc_vblank_on(crtc);

	}



	mutex_lock(&sde_crtc->crtc_lock);

	SDE_EVT32(DRMID(crtc), sde_crtc->enabled);
@@ -5116,15 +5120,11 @@ int sde_crtc_vblank(struct drm_crtc *crtc, bool en) 
	}

	sde_crtc = to_sde_crtc(crtc);



	mutex_lock(&sde_crtc->crtc_lock);

	SDE_EVT32(DRMID(&sde_crtc->base), en, sde_crtc->enabled);

	ret = _sde_crtc_vblank_enable_no_lock(sde_crtc, en);

	ret = _sde_crtc_vblank_enable(sde_crtc, en);

	if (ret)

		SDE_ERROR("%s vblank enable failed: %d\n",

				sde_crtc->name, ret);



	mutex_unlock(&sde_crtc->crtc_lock);



	return 0;

}

msm/sde/sde_crtc.h

+2 −0
Original line number Diff line number Diff line
@@ -291,6 +291,7 @@ struct sde_crtc_misr_info { 
 * @static_cache_read_work: delayed worker to transition cache state to read

 * @cache_state     : Current static image cache state

 * @dspp_blob_info  : blob containing dspp hw capability information

 * @cached_encoder_mask : cached encoder_mask for vblank work

 */

struct sde_crtc {

	struct drm_crtc base;
@@ -381,6 +382,7 @@ struct sde_crtc { 
	enum sde_crtc_cache_state cache_state;



	struct drm_property_blob *dspp_blob_info;

	u32 cached_encoder_mask;

};



enum sde_crtc_dirty_flags {