From 8eb0e6421958e9777db98448a4030d8ae940c9a0 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Fri, 27 Jul 2018 13:19:45 -0400 Subject: media: v4l: vsp1: Fix deadlock in VSPDL DRM pipelines The VSP uses a lock to protect the BRU and BRS assignment when configuring pipelines. The lock is taken in vsp1_du_atomic_begin() and released in vsp1_du_atomic_flush(), as well as taken and released in vsp1_du_setup_lif(). This guards against multiple pipelines trying to assign the same BRU and BRS at the same time. The DRM framework calls the .atomic_begin() operations in a loop over all CRTCs included in an atomic commit. On a VSPDL (the only VSP type where this matters), a single VSP instance handles two CRTCs, with a single lock. This results in a deadlock when the .atomic_begin() operation is called on the second CRTC. The DRM framework serializes atomic commits that affect the same CRTCs, but doesn't know about two CRTCs sharing the same VSPDL. Two commits affecting the VSPDL LIF0 and LIF1 respectively can thus race each other, hence the need for a lock. This could be fixed on the DRM side by forcing serialization of commits affecting CRTCs backed by the same VSPDL, but that would negatively affect performances, as the locking is only needed when the BRU and BRS need to be reassigned, which is an uncommon case. The lock protects the whole .atomic_begin() to .atomic_flush() sequence. The only operation that can occur in-between is vsp1_du_atomic_update(), which doesn't touch the BRU and BRS, and thus doesn't need to be protected by the lock. We can thus only take the lock around the pipeline setup calls in vsp1_du_atomic_flush(), which fixes the deadlock. Fixes: f81f9adc4ee1 ("media: v4l: vsp1: Assign BRU and BRS to pipelines dynamically") Signed-off-by: Laurent Pinchart Reviewed-by: Kieran Bingham Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/vsp1/vsp1_drm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index edb35a5c57ea..a99fc0ced7a7 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -728,9 +728,6 @@ EXPORT_SYMBOL_GPL(vsp1_du_setup_lif); */ void vsp1_du_atomic_begin(struct device *dev, unsigned int pipe_index) { - struct vsp1_device *vsp1 = dev_get_drvdata(dev); - - mutex_lock(&vsp1->drm->lock); } EXPORT_SYMBOL_GPL(vsp1_du_atomic_begin); @@ -846,6 +843,7 @@ void vsp1_du_atomic_flush(struct device *dev, unsigned int pipe_index, drm_pipe->crc = cfg->crc; + mutex_lock(&vsp1->drm->lock); vsp1_du_pipeline_setup_inputs(vsp1, pipe); vsp1_du_pipeline_configure(pipe); mutex_unlock(&vsp1->drm->lock); -- cgit v1.2.3