<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 21, 2024 at 10:08 AM Tomi Valkeinen <<a href="mailto:tomi.valkeinen@ideasonboard.com">tomi.valkeinen@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">It has been observed that sometimes DSS will trigger an interrupt and<br>
the top level interrupt (DISPC_IRQSTATUS) is not zero, but the VP and<br>
VID level interrupt-statuses are zero.<br>
<br>
As the top level irqstatus is supposed to tell whether we have VP/VID<br>
interrupts, the thinking of the driver authors was that this particular<br>
case could never happen. Thus the driver only clears the DISPC_IRQSTATUS<br>
bits which has corresponding interrupts in VP/VID status. So when this<br>
issue happens, the driver will not clear DISPC_IRQSTATUS, and we get an<br>
interrupt flood.<br>
<br>
It is unclear why the issue happens. It could be a race issue in the<br>
driver, but no such race has been found. It could also be an issue with<br>
the HW. However a similar case can be easily triggered by manually<br>
writing to DISPC_IRQSTATUS_RAW. This will forcibly set a bit in the<br>
DISPC_IRQSTATUS and trigger an interrupt, and as the driver never clears<br>
the bit, we get an interrupt flood.<br>
<br>
To fix the issue, always clear DISPC_IRQSTATUS. The concern with this<br>
solution is that if the top level irqstatus is the one that triggers the<br>
interrupt, always clearing DISPC_IRQSTATUS might leave some interrupts<br>
unhandled if VP/VID interrupt statuses have bits set. However, testing<br>
shows that if any of the irqstatuses is set (i.e. even if<br>
DISPC_IRQSTATUS == 0, but a VID irqstatus has a bit set), we will get an<br>
interrupt.<br>
<br>
Signed-off-by: Tomi Valkeinen <<a href="mailto:tomi.valkeinen@ideasonboard.com" target="_blank">tomi.valkeinen@ideasonboard.com</a>><br>
Co-developed-by: Bin Liu <<a href="mailto:b-liu@ti.com" target="_blank">b-liu@ti.com</a>><br>
Signed-off-by: Bin Liu <<a href="mailto:b-liu@ti.com" target="_blank">b-liu@ti.com</a>><br>
Co-developed-by: Devarsh Thakkar <<a href="mailto:devarsht@ti.com" target="_blank">devarsht@ti.com</a>><br>
Signed-off-by: Devarsh Thakkar <<a href="mailto:devarsht@ti.com" target="_blank">devarsht@ti.com</a>><br>
Co-developed-by: Jonathan Cormier <<a href="mailto:jcormier@criticallink.com" target="_blank">jcormier@criticallink.com</a>><br>
Signed-off-by: Jonathan Cormier <<a href="mailto:jcormier@criticallink.com" target="_blank">jcormier@criticallink.com</a>><br>
Fixes: 32a1795f57ee ("drm/tidss: New driver for TI Keystone platform Display SubSystem")<br>
Cc: <a href="mailto:stable@vger.kernel.org" target="_blank">stable@vger.kernel.org</a><br>
---<br>
 drivers/gpu/drm/tidss/tidss_dispc.c | 12 ++++--------<br>
 1 file changed, 4 insertions(+), 8 deletions(-)<br></blockquote><div><br></div><div>I assume a reviewed by doesn't make sense since I co-developed this patch but adding my tested by, hopefully that makes sense. </div><div><br></div><div>
Tested an equivalent patch for several weeks.<br>
Tested-by: Jonathan Cormier <<a href="mailto:jcormier@criticallink.com" target="_blank">jcormier@criticallink.com</a>></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c<br>
index 1ad711f8d2a8..f81111067578 100644<br>
--- a/drivers/gpu/drm/tidss/tidss_dispc.c<br>
+++ b/drivers/gpu/drm/tidss/tidss_dispc.c<br>
@@ -780,24 +780,20 @@ static<br>
 void dispc_k3_clear_irqstatus(struct dispc_device *dispc, dispc_irq_t clearmask)<br>
 {<br>
        unsigned int i;<br>
-       u32 top_clear = 0;<br>
<br>
        for (i = 0; i < dispc->feat->num_vps; ++i) {<br>
-               if (clearmask & DSS_IRQ_VP_MASK(i)) {<br>
+               if (clearmask & DSS_IRQ_VP_MASK(i))<br>
                        dispc_k3_vp_write_irqstatus(dispc, i, clearmask);<br>
-                       top_clear |= BIT(i);<br>
-               }<br>
        }<br>
        for (i = 0; i < dispc->feat->num_planes; ++i) {<br>
-               if (clearmask & DSS_IRQ_PLANE_MASK(i)) {<br>
+               if (clearmask & DSS_IRQ_PLANE_MASK(i))<br>
                        dispc_k3_vid_write_irqstatus(dispc, i, clearmask);<br>
-                       top_clear |= BIT(4 + i);<br>
-               }<br>
        }<br>
        if (dispc->feat->subrev == DISPC_K2G)<br>
                return;<br>
<br>
-       dispc_write(dispc, DISPC_IRQSTATUS, top_clear);<br>
+       /* always clear the top level irqstatus */<br>
+       dispc_write(dispc, DISPC_IRQSTATUS, dispc_read(dispc, DISPC_IRQSTATUS));<br>
<br>
        /* Flush posted writes */<br>
        dispc_read(dispc, DISPC_IRQSTATUS);<br>
<br>
-- <br>
2.43.0<br>
<br>
</blockquote></div><br clear="all"><br><span class="gmail_signature_prefix">-- </span><br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div>Jonathan Cormier<br>Software Engineer<div><div><div><div><br></div><div><div><font color="#000000">Voice:  <a value="+13154254045">315.425.4045</a> x222</font></div><div><font color="#000000"><br></font></div><div><font color="#000000"><img src="https://ci3.googleusercontent.com/mail-sig/AIorK4xk-E6kTWNd5AJHtEUAEcL6C7vYjIUeEExuTc6ecDmTEshpkG53r8T5L4geBRI1997uwEiKrqgnopA3"><br></font></div></div></div><div><div><font color="#1155cc"><br></font></div><div><a href="http://www.criticallink.com/" target="_blank"><font color="#1155cc">http://www.CriticalLink.com</font></a><br></div><div>6712 Brooklawn Parkway, Syracuse, NY 13211 </div></div></div><div><div><br></div><div><font color="#1155cc"><font color="#1155cc"><a href="https://www.linkedin.com/company/critical-link-llc" target="_blank"><img src="https://docs.google.com/uc?export=download&id=0B2vNSnu-aYu6OEhHRW9BUFV5WnM&revid=0B2vNSnu-aYu6RHNZUnhNbFpER1l3emNQY2VoaHA0RDdudWlFPQ"></a>  <a href="https://twitter.com/Critical_Link" target="_blank"><img alt="" src="https://docs.google.com/uc?export=download&id=0B2vNSnu-aYu6cU1yWERrLXE0SnM&revid=0B2vNSnu-aYu6b1YrZW1SM0hueVhVS0pPWm1IOXFSc0I3ay9jPQ"></a></font></font></div></div></div></div></div></div></div></div></div></div></div></div></div></div></div>