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

Commit dca8eeb8 authored by Xu Yang's avatar Xu Yang
Browse files

drm/msm/sde: fix deadlock between IRQ lock and node state lock 



Deadlock is found when histogram interrupt is being disabled and
at the same time histogram callback function is being called. The
histogram IRQ lock is hold by interrupt thread, and node state
lock is hold by main thread. And each thread is waiting for the
other lock. Change fixes the race condition and add one more state
in node to avoid the interrupt be disabled twice.

And the patch also fix another issue, when Driver receives the request
from userspace to disable histogram irq, and the irq node has already
been deleted from user event list, it will fail to disable the irq. The
change fixes the issue by getting the irq node by container_of instead
of user event list.

Change-Id: If1158e1d916fce4d79248b082a15b7590b54bab6
Signed-off-by: default avatarXu Yang <yangxu@codeaurora.org>
parent ddbece73

drivers/gpu/drm/msm/sde/sde_color_processing.c

+40 −68
Original line number Diff line number Diff line
@@ -1799,10 +1799,9 @@ int sde_cp_ad_interrupt(struct drm_crtc *crtc_drm, bool en, 
		goto exit;

	}



	node = _sde_cp_get_intr_node(DRM_EVENT_AD_BACKLIGHT, crtc);

	node = container_of(ad_irq, struct sde_crtc_irq_info, irq);



	if (!en) {

		if (node) {

		spin_lock_irqsave(&node->state_lock, flags);

		if (node->state == IRQ_ENABLED) {

			ret = sde_core_irq_disable(kms, &irq_idx, 1);
@@ -1815,9 +1814,6 @@ int sde_cp_ad_interrupt(struct drm_crtc *crtc_drm, bool en, 
			node->state = IRQ_NOINIT;

		}

		spin_unlock_irqrestore(&node->state_lock, flags);

		} else {

			DRM_ERROR("failed to get node from crtc event list\n");

		}

		sde_core_irq_unregister_callback(kms, irq_idx, ad_irq);

		ret = 0;

		goto exit;
@@ -1831,32 +1827,18 @@ int sde_cp_ad_interrupt(struct drm_crtc *crtc_drm, bool en, 
		goto exit;

	}



	if (node) {

		/* device resume or resume from IPC cases */

	spin_lock_irqsave(&node->state_lock, flags);

	if (node->state == IRQ_DISABLED || node->state == IRQ_NOINIT) {

		ret = sde_core_irq_enable(kms, &irq_idx, 1);

		if (ret) {

				DRM_ERROR("enable irq %d error %d\n",

					irq_idx, ret);

				sde_core_irq_unregister_callback(kms,

					irq_idx, ad_irq);

			DRM_ERROR("enable irq %d error %d\n", irq_idx, ret);

			sde_core_irq_unregister_callback(kms, irq_idx, ad_irq);

		} else {

			node->state = IRQ_ENABLED;

		}

	}

	spin_unlock_irqrestore(&node->state_lock, flags);

	} else {

		/* request from userspace to register the event

		 * in this case, node has not been added into the event list

		 */

		ret = sde_core_irq_enable(kms, &irq_idx, 1);

		if (ret) {

			DRM_ERROR("failed to enable irq ret %d\n", ret);

			sde_core_irq_unregister_callback(kms,

				irq_idx, ad_irq);

		}

	}



exit:

	return ret;

}
@@ -1937,7 +1919,7 @@ static void sde_cp_hist_interrupt_cb(void *arg, int irq_idx) 
	spin_unlock_irqrestore(&crtc->spin_lock, flags);



	if (!node) {

		DRM_ERROR("cannot find histogram event node in crtc\n");

		DRM_DEBUG_DRIVER("cannot find histogram event node in crtc\n");

		return;

	}
@@ -2082,26 +2064,29 @@ int sde_cp_hist_interrupt(struct drm_crtc *crtc_drm, bool en, 
		goto exit;

	}



	node = _sde_cp_get_intr_node(DRM_EVENT_HISTOGRAM, crtc);

	node = container_of(hist_irq, struct sde_crtc_irq_info, irq);



	/* deregister histogram irq */

	if (!en) {

		if (node) {

			/* device suspend case or suspend to IPC cases */

		spin_lock_irqsave(&node->state_lock, flags);

		if (node->state == IRQ_ENABLED) {

			node->state = IRQ_DISABLING;

			spin_unlock_irqrestore(&node->state_lock, flags);

			ret = sde_core_irq_disable(kms, &irq_idx, 1);

				if (ret)

			spin_lock_irqsave(&node->state_lock, flags);

			if (ret) {

				DRM_ERROR("disable irq %d error %d\n",

					irq_idx, ret);

				else

					node->state = IRQ_NOINIT;

				node->state = IRQ_ENABLED;

			} else {

				node->state = IRQ_NOINIT;

			}

			spin_unlock_irqrestore(&node->state_lock, flags);

		} else if (node->state == IRQ_DISABLED) {

			node->state = IRQ_NOINIT;

			spin_unlock_irqrestore(&node->state_lock, flags);

		} else {

			DRM_ERROR("failed to get node from crtc event list\n");

			spin_unlock_irqrestore(&node->state_lock, flags);

		}



		sde_core_irq_unregister_callback(kms, irq_idx, hist_irq);
@@ -2117,14 +2102,11 @@ int sde_cp_hist_interrupt(struct drm_crtc *crtc_drm, bool en, 
		goto exit;

	}



	if (node) {

		/* device resume or resume from IPC cases */

	spin_lock_irqsave(&node->state_lock, flags);

	if (node->state == IRQ_DISABLED || node->state == IRQ_NOINIT) {

		ret = sde_core_irq_enable(kms, &irq_idx, 1);

		if (ret) {

				DRM_ERROR("enable irq %d error %d\n",

					irq_idx, ret);

			DRM_ERROR("enable irq %d error %d\n", irq_idx, ret);

			sde_core_irq_unregister_callback(kms,

				irq_idx, hist_irq);

		} else {
@@ -2132,17 +2114,7 @@ int sde_cp_hist_interrupt(struct drm_crtc *crtc_drm, bool en, 
		}

	}

	spin_unlock_irqrestore(&node->state_lock, flags);

	} else {

		/* request from userspace to register the event

		 * in this case, node has not been added into the event list

		 */

		ret = sde_core_irq_enable(kms, &irq_idx, 1);

		if (ret) {

			DRM_ERROR("failed to enable irq ret %d\n", ret);

			sde_core_irq_unregister_callback(kms,

				irq_idx, hist_irq);

		}

	}



exit:

	return ret;

}

drivers/gpu/drm/msm/sde/sde_crtc.c

+1 −3
Original line number Diff line number Diff line
@@ -6032,10 +6032,10 @@ static int _sde_crtc_event_enable(struct sde_kms *kms, 
			node = kzalloc(sizeof(*node), GFP_KERNEL);

			if (!node)

				return -ENOMEM;

			node->event = event;

			INIT_LIST_HEAD(&node->list);

			node->func = custom_events[i].func;

			node->event = event;

			node->state = IRQ_NOINIT;

			spin_lock_init(&node->state_lock);

			break;

		}
@@ -6066,8 +6066,6 @@ static int _sde_crtc_event_enable(struct sde_kms *kms, 


	if (!ret) {

		spin_lock_irqsave(&crtc->spin_lock, flags);

		/* irq is regiestered and enabled and set the state */

		node->state = IRQ_ENABLED;

		list_add_tail(&node->list, &crtc->user_event_list);

		spin_unlock_irqrestore(&crtc->spin_lock, flags);

	} else {

drivers/gpu/drm/msm/sde/sde_crtc.h

+1 −0
Original line number Diff line number Diff line
@@ -390,6 +390,7 @@ struct sde_crtc_state { 
enum sde_crtc_irq_state {

	IRQ_NOINIT,

	IRQ_ENABLED,

	IRQ_DISABLING,

	IRQ_DISABLED,

};