From patchwork Fri Jan 27 01:09:12 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 49010 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp582785wrn; Thu, 26 Jan 2023 17:22:27 -0800 (PST) X-Google-Smtp-Source: AMrXdXv1HcKNCAwstXAdQWnwE0/jWezxWVqte0x22jRvDzqMNLsrLb86DDVnGXexp6cBLeIKJNzy X-Received: by 2002:a05:6a20:7a89:b0:a2:c45f:f0fc with SMTP id u9-20020a056a207a8900b000a2c45ff0fcmr40461626pzh.27.1674782547664; Thu, 26 Jan 2023 17:22:27 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1674782547; cv=none; d=google.com; s=arc-20160816; b=aqmB4I9z2T47Ef0jVQi+CnpqiRUaFe6fZ4l9twAQYrbNwQuF+PPjoyiWoB0+HaxIQ1 NKcJ0wvrNjlT9TZ4SH11moig376agiafSVBndJBPclymd1s9ZP9NiwYob1WuASeUtTg4 VXk6/DMiL9Nq8ykkGDQdZfX/mG6BQ8vfg75ePsudcitIYe+QG6QnLrIxhZVuAO50SySB jZfmZBbSLF+TN0O055W/9ElnV0Pl0KVFNucZBGFAYqn+QiQyV5tDqvQaBsbt5S3oD5o7 UeUMR5RRciauhsPMWUC/oaTf5ZVDqOj1WuRwA+RhSdufiDvgai+s0HCmCxUGYKeXVKRh XRNQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=uORFmtwf+WIQa4zOQDY675hMQt16yP5oZ3IJbiKasy0=; b=pugNogpe5ERJGc0bZ6w0Nn6ISVXuizmKpvIcgP9gmEl/kQrPyQMt5FT7gRKPoSX3Kq s5M3lmoLwoB3RnJ99TGzTQD3RgqBGlBNGW4M1QqI0fbXfsFAC9yWDXH3Ows8oFQMR+K8 urYMoL7GjzzFPrmKAbJ5+ddv55LW29kHkKKoVmfTM7EEH8Jh45GEklMR8oY31CCdUg5i 11b8dHqBlcvyYPWstjMhpGQR/EmE041hAd1oXIK2ue6XIDvDNwFrIWm6tpleubQilS4P DISA1Dknk+vfyu+F8sELEa39enS2sJLXyLnBaf5zqrd682xOQBYEqD2J347XZ/+yRabh 23tg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=cy4ezLsg; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s1-20020a639e01000000b0049b262ac916si2564166pgd.566.2023.01.26.17.22.15; Thu, 26 Jan 2023 17:22:27 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=cy4ezLsg; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233038AbjA0BJh (ORCPT + 99 others); Thu, 26 Jan 2023 20:09:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47276 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231743AbjA0BJe (ORCPT ); Thu, 26 Jan 2023 20:09:34 -0500 Received: from mail-pj1-x1030.google.com (mail-pj1-x1030.google.com [IPv6:2607:f8b0:4864:20::1030]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C5D7A50840 for ; Thu, 26 Jan 2023 17:09:32 -0800 (PST) Received: by mail-pj1-x1030.google.com with SMTP id m7-20020a17090a71c700b0022c0c070f2eso6406470pjs.4 for ; Thu, 26 Jan 2023 17:09:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=uORFmtwf+WIQa4zOQDY675hMQt16yP5oZ3IJbiKasy0=; b=cy4ezLsgrRklfSsdqOdGdWX5lEWLqYyxBbiuayCxSoCVZxvsNERAEbRDC6sIO8N0SP 5t8NR2mH7YDnmFBcLlXAnK3ACyXGW+i4Xbh4FpIIkeZf4UoWg0RjnTxyciPChZrzfO2i rm3+Ik9KgiFSAolP2dlfyisJFU/KEPyCQcD+8= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=uORFmtwf+WIQa4zOQDY675hMQt16yP5oZ3IJbiKasy0=; b=mk2BPOw70Od8B6CFpyhMDAARsXqWQf/fEXFgt+E19bks+ZGNbRtqbd2ZMIEsg/hFp5 OHtO1jk7+sv3Kdv6uEJdYwuCH+hVe0O9ihyz/eHOmtC22aUpCQ+uhI9nX0eVrsri7Nr3 RKIgiKc0bi2lm/s1L8tgrUWIVi3YTck2zk1qOJ1gfS8fu0NHEZRobA0Y3pS6dGuvPhFm W0Cf6RHops4lPrgn06/qfPfe3kze4DxLj3Bj2Q1TPyASjb+lFh/OXrVjeYu7h3wUokuE 3lNtrcXaNDIMgR14LPUm9e7xKi//2fyBW+6RzZZj2WyQn2lhO+pCzU5i19zKpK+dDs6Q 6rqw== X-Gm-Message-State: AO0yUKXdRowyVw6yKfNq9mTYHdj3mGX0u78KMMCJe/TgBRi84orhDnqZ N+fyV0rd0x+e6Dcb0KPyECduQg== X-Received: by 2002:a17:90b:3806:b0:22c:3052:47dd with SMTP id mq6-20020a17090b380600b0022c305247ddmr3446534pjb.17.1674781772234; Thu, 26 Jan 2023 17:09:32 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:9d:2:d3b5:7433:dc03:ca1f]) by smtp.gmail.com with ESMTPSA id s10-20020a17090a760a00b0022bbbba9801sm3843981pjk.37.2023.01.26.17.09.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Jan 2023 17:09:31 -0800 (PST) From: Douglas Anderson To: Rob Clark , Abhinav Kumar , Dmitry Baryshkov Cc: Stephen Boyd , Kuogee Hsieh , Douglas Anderson , Daniel Vetter , David Airlie , Sankeerth Billakanti , Sean Paul , Thomas Zimmermann , dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2 1/2] drm/msm/dp: Clean up handling of DP AUX interrupts Date: Thu, 26 Jan 2023 17:09:12 -0800 Message-Id: <20230126170745.v2.1.I90ffed3ddd21e818ae534f820cb4d6d8638859ab@changeid> X-Mailer: git-send-email 2.39.1.456.gfc5497dd1b-goog MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1756136784698395075?= X-GMAIL-MSGID: =?utf-8?q?1756136784698395075?= The DP AUX interrupt handling was a bit of a mess. * There were two functions (one for "native" transfers and one for "i2c" transfers) that were quite similar. It was hard to say how many of the differences between the two functions were on purpose and how many of them were just an accident of how they were coded. * Each function sometimes used "else if" to test for error bits and sometimes didn't and again it was hard to say if this was on purpose or just an accident. * The two functions wouldn't notice whether "unknown" bits were set. For instance, there seems to be a bit "DP_INTR_PLL_UNLOCKED" and if it was set there would be no indication. * The two functions wouldn't notice if more than one error was set. Let's fix this by being more consistent / explicit about what we're doing. By design this could cause different handling for AUX transfers, though I'm not actually aware of any bug fixed as a result of this patch (this patch was created because we simply noticed how odd the old code was by code inspection). Specific notes here: 1. In the old native transfer case if we got "done + wrong address" we'd ignore the "wrong address" (because of the "else if"). Now we won't. 2. In the old native transfer case if we got "done + timeout" we'd ignore the "timeout" (because of the "else if"). Now we won't. 3. In the old native transfer case we'd see "nack_defer" and translate it to the error number for "nack". This differed from the i2c transfer case where "nack_defer" was given the error number for "nack_defer". This 100% can't matter because the only user of this error number treats "nack defer" the same as "nack", so it's clear that the difference between the "native" and "i2c" was pointless here. 4. In the old i2c transfer case if we got "done" plus any error besides "nack" or "defer" then we'd ignore the error. Now we don't. 5. If there is more than one error signaled by the hardware it's possible that we'll report a different one than we used to. I don't know if this matters. If someone is aware of a case this matters we should document it and change the code to make it explicit. 6. One quirk we keep (I don't know if this is important) is that in the i2c transfer case if we see "done + defer" we report that as a "nack". That seemed too intentional in the old code to just drop. After this change we will add extra logging, including: * A warning if we see more than one error bit set. * A warning if we see an unexpected interrupt. * A warning if we get an AUX transfer interrupt when shouldn't. It actually turns out that as a result of this change then at boot we sometimes see an error: [drm:dp_aux_isr] *ERROR* Unexpected DP AUX IRQ 0x01000000 when not busy That means that, during init, we are seeing DP_INTR_PLL_UNLOCKED. For now I'm going to say that leaving this error reported in the logs is OK-ish and hopefully it will encourage someone to track down what's going on at init time. One last note here is that this change renames one of the interrupt bits. The bit named "i2c done" clearly was used for native transfers being done too, so I renamed it to indicate this. Signed-off-by: Douglas Anderson Tested-by: Kuogee Hsieh Reviewed-by: Kuogee Hsieh --- I don't have good test coverage for this change and it does have the potential to change behavior. I confirmed that eDP and DP still continue to work OK on one machine. Hopefully folks can test it more. Changes in v2: - Moved DP_INTR_AUX_XFER_DONE to the end of the if else chain. drivers/gpu/drm/msm/dp/dp_aux.c | 80 ++++++++++++----------------- drivers/gpu/drm/msm/dp/dp_catalog.c | 2 +- drivers/gpu/drm/msm/dp/dp_catalog.h | 2 +- 3 files changed, 36 insertions(+), 48 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c index cc3efed593aa..84f9e3e5f964 100644 --- a/drivers/gpu/drm/msm/dp/dp_aux.c +++ b/drivers/gpu/drm/msm/dp/dp_aux.c @@ -162,47 +162,6 @@ static ssize_t dp_aux_cmd_fifo_rx(struct dp_aux_private *aux, return i; } -static void dp_aux_native_handler(struct dp_aux_private *aux, u32 isr) -{ - if (isr & DP_INTR_AUX_I2C_DONE) - aux->aux_error_num = DP_AUX_ERR_NONE; - else if (isr & DP_INTR_WRONG_ADDR) - aux->aux_error_num = DP_AUX_ERR_ADDR; - else if (isr & DP_INTR_TIMEOUT) - aux->aux_error_num = DP_AUX_ERR_TOUT; - if (isr & DP_INTR_NACK_DEFER) - aux->aux_error_num = DP_AUX_ERR_NACK; - if (isr & DP_INTR_AUX_ERROR) { - aux->aux_error_num = DP_AUX_ERR_PHY; - dp_catalog_aux_clear_hw_interrupts(aux->catalog); - } -} - -static void dp_aux_i2c_handler(struct dp_aux_private *aux, u32 isr) -{ - if (isr & DP_INTR_AUX_I2C_DONE) { - if (isr & (DP_INTR_I2C_NACK | DP_INTR_I2C_DEFER)) - aux->aux_error_num = DP_AUX_ERR_NACK; - else - aux->aux_error_num = DP_AUX_ERR_NONE; - } else { - if (isr & DP_INTR_WRONG_ADDR) - aux->aux_error_num = DP_AUX_ERR_ADDR; - else if (isr & DP_INTR_TIMEOUT) - aux->aux_error_num = DP_AUX_ERR_TOUT; - if (isr & DP_INTR_NACK_DEFER) - aux->aux_error_num = DP_AUX_ERR_NACK_DEFER; - if (isr & DP_INTR_I2C_NACK) - aux->aux_error_num = DP_AUX_ERR_NACK; - if (isr & DP_INTR_I2C_DEFER) - aux->aux_error_num = DP_AUX_ERR_DEFER; - if (isr & DP_INTR_AUX_ERROR) { - aux->aux_error_num = DP_AUX_ERR_PHY; - dp_catalog_aux_clear_hw_interrupts(aux->catalog); - } - } -} - static void dp_aux_update_offset_and_segment(struct dp_aux_private *aux, struct drm_dp_aux_msg *input_msg) { @@ -427,13 +386,42 @@ void dp_aux_isr(struct drm_dp_aux *dp_aux) if (!isr) return; - if (!aux->cmd_busy) + if (!aux->cmd_busy) { + DRM_ERROR("Unexpected DP AUX IRQ %#010x when not busy\n", isr); return; + } - if (aux->native) - dp_aux_native_handler(aux, isr); - else - dp_aux_i2c_handler(aux, isr); + /* + * The logic below assumes only one error bit is set (other than "done" + * which can apparently be set at the same time as some of the other + * bits). Warn if more than one get set so we know we need to improve + * the logic. + */ + if (hweight32(isr & ~DP_INTR_AUX_XFER_DONE) > 1) + DRM_WARN("Some DP AUX interrupts unhandled: %#010x\n", isr); + + if (isr & DP_INTR_AUX_ERROR) { + aux->aux_error_num = DP_AUX_ERR_PHY; + dp_catalog_aux_clear_hw_interrupts(aux->catalog); + } else if (isr & DP_INTR_NACK_DEFER) { + aux->aux_error_num = DP_AUX_ERR_NACK_DEFER; + } else if (isr & DP_INTR_WRONG_ADDR) { + aux->aux_error_num = DP_AUX_ERR_ADDR; + } else if (isr & DP_INTR_TIMEOUT) { + aux->aux_error_num = DP_AUX_ERR_TOUT; + } else if (!aux->native && (isr & DP_INTR_I2C_NACK)) { + aux->aux_error_num = DP_AUX_ERR_NACK; + } else if (!aux->native && (isr & DP_INTR_I2C_DEFER)) { + if (isr & DP_INTR_AUX_XFER_DONE) + aux->aux_error_num = DP_AUX_ERR_NACK; + else + aux->aux_error_num = DP_AUX_ERR_DEFER; + } else if (isr & DP_INTR_AUX_XFER_DONE) { + aux->aux_error_num = DP_AUX_ERR_NONE; + } else { + DRM_WARN("Unexpected interrupt: %#010x\n", isr); + return; + } complete(&aux->comp); } diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c b/drivers/gpu/drm/msm/dp/dp_catalog.c index 676279d0ca8d..421391755427 100644 --- a/drivers/gpu/drm/msm/dp/dp_catalog.c +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c @@ -27,7 +27,7 @@ #define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4) #define DP_INTERRUPT_STATUS1 \ - (DP_INTR_AUX_I2C_DONE| \ + (DP_INTR_AUX_XFER_DONE| \ DP_INTR_WRONG_ADDR | DP_INTR_TIMEOUT | \ DP_INTR_NACK_DEFER | DP_INTR_WRONG_DATA_CNT | \ DP_INTR_I2C_NACK | DP_INTR_I2C_DEFER | \ diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.h b/drivers/gpu/drm/msm/dp/dp_catalog.h index 1f717f45c115..f36b7b372a06 100644 --- a/drivers/gpu/drm/msm/dp/dp_catalog.h +++ b/drivers/gpu/drm/msm/dp/dp_catalog.h @@ -13,7 +13,7 @@ /* interrupts */ #define DP_INTR_HPD BIT(0) -#define DP_INTR_AUX_I2C_DONE BIT(3) +#define DP_INTR_AUX_XFER_DONE BIT(3) #define DP_INTR_WRONG_ADDR BIT(6) #define DP_INTR_TIMEOUT BIT(9) #define DP_INTR_NACK_DEFER BIT(12) From patchwork Fri Jan 27 01:09:13 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 49009 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp582605wrn; Thu, 26 Jan 2023 17:21:54 -0800 (PST) X-Google-Smtp-Source: AMrXdXvmyKaLuCJQoh5OAYb2NR2BeXD1zpJig/jOIXWQvzAQ65UaNUzX06iDvThU60i6+Vbbtfd2 X-Received: by 2002:a05:6a20:c916:b0:b9:92c2:7ecd with SMTP id gx22-20020a056a20c91600b000b992c27ecdmr24813598pzb.3.1674782514562; Thu, 26 Jan 2023 17:21:54 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1674782514; cv=none; d=google.com; s=arc-20160816; b=HYGb6ffF7TAhYp0B1fyOHechV4gO3rApRNOurD76G6GcPYPnFk90PTrKBRDh1zIeTJ S6DCDEbKIexTkFclbMsKXabtmADKbbvgBNIz7iR0C6bb/X5ZqGRoJZKhQZ/w17ru27Y5 talZB8h9XU9ojw60ZsHV2aM+1tg5CTssYKzK9sHmMfwNz6yGOiuhgcI6bQoTMBhRvQEo /Yhkqfbvjn9q++qRdJYF4uyXnpEtgPWDhG1sSAalC4w/EKAMEA4px9lVw9irDPgtxwQM Zi29/Z+Eg5W1pQ/KPGQOhgNpgVy/px2VEUx13/f2NdgfV2XNHLrqEs5yKjHFZTtEG89x x4rg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=6S56yI90J6SMsvBPS/eoWABUHdDVHzOi+B+mbBsY/0Y=; b=rDK+MpjimKysgQtl6JoHuP9scrDGM+BKJ4QO67LQ9jqDGEtWW0EdAsJ6OUM7xXaOjY xGCQRzD5QN+VjY1NYE1yzvqhAuaGWN9o/oiFSOUjxDdEC37crQuqe+3M65c5czdvHOJo XixqsNh2GLLeh7ZVTQXAcHZ+nIMMwFbb8DIMDmXInZ8X82+Jl9zxaDCfdnBHiI+nZV8O TenC/34LPWQ7SPsH/OaIWrs9yo9cFpfMu2iJuK4nXmr3IjMZEaDN6odDq5dxSTfvETcC 4kdZQfi8db7QN2xTV9RqXWrBJWdNoLfR3hu44DyoXq3iKpmn7S4V+DL6o9XAYulhlVhP cDBw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=U4D+jpgw; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id h189-20020a636cc6000000b004deb29a8ae2si2550975pgc.422.2023.01.26.17.21.41; Thu, 26 Jan 2023 17:21:54 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=U4D+jpgw; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229759AbjA0BJl (ORCPT + 99 others); Thu, 26 Jan 2023 20:09:41 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47288 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232882AbjA0BJg (ORCPT ); Thu, 26 Jan 2023 20:09:36 -0500 Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C57D354220 for ; Thu, 26 Jan 2023 17:09:34 -0800 (PST) Received: by mail-pj1-x102c.google.com with SMTP id b10so3165697pjo.1 for ; Thu, 26 Jan 2023 17:09:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=6S56yI90J6SMsvBPS/eoWABUHdDVHzOi+B+mbBsY/0Y=; b=U4D+jpgwO1YI68vRVXAddtAN5r/lxcTAdAbsavlwvB++kS12OVLBsU76+rImeY7j4D Vk375gvfV14xmJZ19HwexRQB/nig+uyxyPEKMwWct7WlVK5+ImPlhMo/cNF7BFMDV9m5 re2upayitnBNwEK1ijTEpOEAD/HcRl270AQjI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=6S56yI90J6SMsvBPS/eoWABUHdDVHzOi+B+mbBsY/0Y=; b=Q///geVcMlOjDqJ497pD32K4Kvbb1+u6yrdhVEHpplnN8xPBspN/JBOOPAfAN5dzpt n8B3+BqhiPoTXpJwdUYwlT1hFwKIpuce4Eq0B8DbOiRTr3uWhoQTHgei+TfeetlN5CF7 4EgY5GPikiMyOlNN7Ef27a4yEAsDqhvronR+ocJOJjVhNaaBr6a3NvDzrhu0D+L6g6NQ sQiJuJBv4jc2zD5Y/cULIuRQsOYXhaXKwwg4N/2rMnPdAruGkoYMvEstWliNIX+bXj14 l70uHphC4RsJXGbkUgKyh5XmiP9kG8vUs2VnvIlxhQBBz6m3amnP5pqA59BdO1jEoBgC ecnQ== X-Gm-Message-State: AFqh2kpCphUunEK927Gb3R8IAL6QVO/P9j9b2W0HlXJtjtWUwJ3KY5QF jCdqI9GMPCy6d/3ZpMqLplKLPQ== X-Received: by 2002:a17:90a:7a84:b0:214:132a:2bea with SMTP id q4-20020a17090a7a8400b00214132a2beamr40082120pjf.4.1674781774222; Thu, 26 Jan 2023 17:09:34 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:9d:2:d3b5:7433:dc03:ca1f]) by smtp.gmail.com with ESMTPSA id s10-20020a17090a760a00b0022bbbba9801sm3843981pjk.37.2023.01.26.17.09.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Jan 2023 17:09:33 -0800 (PST) From: Douglas Anderson To: Rob Clark , Abhinav Kumar , Dmitry Baryshkov Cc: Stephen Boyd , Kuogee Hsieh , Douglas Anderson , Alex Deucher , Bjorn Andersson , Daniel Vetter , David Airlie , Javier Martinez Canillas , Johan Hovold , Lyude Paul , Sankeerth Billakanti , Sean Paul , Thomas Zimmermann , dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2 2/2] drm/msm/dp: Return IRQ_NONE for unhandled interrupts Date: Thu, 26 Jan 2023 17:09:13 -0800 Message-Id: <20230126170745.v2.2.I2d7aec2fadb9c237cd0090a47d6a8ba2054bf0f8@changeid> X-Mailer: git-send-email 2.39.1.456.gfc5497dd1b-goog In-Reply-To: <20230126170745.v2.1.I90ffed3ddd21e818ae534f820cb4d6d8638859ab@changeid> References: <20230126170745.v2.1.I90ffed3ddd21e818ae534f820cb4d6d8638859ab@changeid> MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1756136749763673472?= X-GMAIL-MSGID: =?utf-8?q?1756136749763673472?= If our interrupt handler gets called and we don't really handle the interrupt then we should return IRQ_NONE. The current interrupt handler didn't do this, so let's fix it. NOTE: for some of the cases it's clear that we should return IRQ_NONE and some cases it's clear that we should return IRQ_HANDLED. However, there are a few that fall somewhere in between. Specifically, the documentation for when to return IRQ_NONE vs. IRQ_HANDLED is probably best spelled out in the commit message of commit d9e4ad5badf4 ("Document that IRQ_NONE should be returned when IRQ not actually handled"). That commit makes it clear that we should return IRQ_HANDLED if we've done something to make the interrupt stop happening. The case where it's unclear is, for instance, in dp_aux_isr() after we've read the interrupt using dp_catalog_aux_get_irq() and confirmed that "isr" is non-zero. The function dp_catalog_aux_get_irq() not only reads the interrupts but it also "ack"s all the interrupts that are returned. For an "unknown" interrupt this has a very good chance of actually stopping the interrupt from happening. That would mean we've identified that it's our device and done something to stop them from happening and should return IRQ_HANDLED. Specifically, it should be noted that most interrupts that need "ack"ing are ones that are one-time events and doing an "ack" is enough to clear them. However, since these interrupts are unknown then, by definition, it's unknown if "ack"ing them is truly enough to clear them. It's possible that we also need to remove the original source of the interrupt. In this case, IRQ_NONE would be a better choice. Given that returning an occasional IRQ_NONE isn't the absolute end of the world, however, let's choose that course of action. The IRQ framework will forgive a few IRQ_NONE returns now and again (and it won't even log them, which is why we have to log them ourselves). This means that if we _do_ end hitting an interrupt where "ack"ing isn't enough the kernel will eventually detect the problem and shut our device down. Signed-off-by: Douglas Anderson Tested-by: Kuogee Hsieh Reviewed-by: Kuogee Hsieh --- (no changes since v1) drivers/gpu/drm/msm/dp/dp_aux.c | 12 +++++++----- drivers/gpu/drm/msm/dp/dp_aux.h | 2 +- drivers/gpu/drm/msm/dp/dp_ctrl.c | 10 ++++++++-- drivers/gpu/drm/msm/dp/dp_ctrl.h | 2 +- drivers/gpu/drm/msm/dp/dp_display.c | 8 +++++--- 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c index 84f9e3e5f964..8e3b677f35e6 100644 --- a/drivers/gpu/drm/msm/dp/dp_aux.c +++ b/drivers/gpu/drm/msm/dp/dp_aux.c @@ -368,14 +368,14 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux, return ret; } -void dp_aux_isr(struct drm_dp_aux *dp_aux) +irqreturn_t dp_aux_isr(struct drm_dp_aux *dp_aux) { u32 isr; struct dp_aux_private *aux; if (!dp_aux) { DRM_ERROR("invalid input\n"); - return; + return IRQ_NONE; } aux = container_of(dp_aux, struct dp_aux_private, dp_aux); @@ -384,11 +384,11 @@ void dp_aux_isr(struct drm_dp_aux *dp_aux) /* no interrupts pending, return immediately */ if (!isr) - return; + return IRQ_NONE; if (!aux->cmd_busy) { DRM_ERROR("Unexpected DP AUX IRQ %#010x when not busy\n", isr); - return; + return IRQ_NONE; } /* @@ -420,10 +420,12 @@ void dp_aux_isr(struct drm_dp_aux *dp_aux) aux->aux_error_num = DP_AUX_ERR_NONE; } else { DRM_WARN("Unexpected interrupt: %#010x\n", isr); - return; + return IRQ_NONE; } complete(&aux->comp); + + return IRQ_HANDLED; } void dp_aux_reconfig(struct drm_dp_aux *dp_aux) diff --git a/drivers/gpu/drm/msm/dp/dp_aux.h b/drivers/gpu/drm/msm/dp/dp_aux.h index e930974bcb5b..511305da4f66 100644 --- a/drivers/gpu/drm/msm/dp/dp_aux.h +++ b/drivers/gpu/drm/msm/dp/dp_aux.h @@ -11,7 +11,7 @@ int dp_aux_register(struct drm_dp_aux *dp_aux); void dp_aux_unregister(struct drm_dp_aux *dp_aux); -void dp_aux_isr(struct drm_dp_aux *dp_aux); +irqreturn_t dp_aux_isr(struct drm_dp_aux *dp_aux); void dp_aux_init(struct drm_dp_aux *dp_aux); void dp_aux_deinit(struct drm_dp_aux *dp_aux); void dp_aux_reconfig(struct drm_dp_aux *dp_aux); diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c index dd26ca651a05..1a5377ef1967 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.c +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c @@ -1979,27 +1979,33 @@ int dp_ctrl_off(struct dp_ctrl *dp_ctrl) return ret; } -void dp_ctrl_isr(struct dp_ctrl *dp_ctrl) +irqreturn_t dp_ctrl_isr(struct dp_ctrl *dp_ctrl) { struct dp_ctrl_private *ctrl; u32 isr; + irqreturn_t ret = IRQ_NONE; if (!dp_ctrl) - return; + return IRQ_NONE; ctrl = container_of(dp_ctrl, struct dp_ctrl_private, dp_ctrl); isr = dp_catalog_ctrl_get_interrupt(ctrl->catalog); + if (isr & DP_CTRL_INTR_READY_FOR_VIDEO) { drm_dbg_dp(ctrl->drm_dev, "dp_video_ready\n"); complete(&ctrl->video_comp); + ret = IRQ_HANDLED; } if (isr & DP_CTRL_INTR_IDLE_PATTERN_SENT) { drm_dbg_dp(ctrl->drm_dev, "idle_patterns_sent\n"); complete(&ctrl->idle_comp); + ret = IRQ_HANDLED; } + + return ret; } struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h index 9f29734af81c..c3af06dc87b1 100644 --- a/drivers/gpu/drm/msm/dp/dp_ctrl.h +++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h @@ -25,7 +25,7 @@ int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl); int dp_ctrl_off_link(struct dp_ctrl *dp_ctrl); int dp_ctrl_off(struct dp_ctrl *dp_ctrl); void dp_ctrl_push_idle(struct dp_ctrl *dp_ctrl); -void dp_ctrl_isr(struct dp_ctrl *dp_ctrl); +irqreturn_t dp_ctrl_isr(struct dp_ctrl *dp_ctrl); void dp_ctrl_handle_sink_request(struct dp_ctrl *dp_ctrl); struct dp_ctrl *dp_ctrl_get(struct device *dev, struct dp_link *link, struct dp_panel *panel, struct drm_dp_aux *aux, diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index bde1a7ce442f..b5343c9f1c1e 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -1204,7 +1204,7 @@ static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv) static irqreturn_t dp_display_irq_handler(int irq, void *dev_id) { struct dp_display_private *dp = dev_id; - irqreturn_t ret = IRQ_HANDLED; + irqreturn_t ret = IRQ_NONE; u32 hpd_isr_status; if (!dp) { @@ -1232,13 +1232,15 @@ static irqreturn_t dp_display_irq_handler(int irq, void *dev_id) if (hpd_isr_status & DP_DP_HPD_UNPLUG_INT_MASK) dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0); + + ret = IRQ_HANDLED; } /* DP controller isr */ - dp_ctrl_isr(dp->ctrl); + ret |= dp_ctrl_isr(dp->ctrl); /* DP aux isr */ - dp_aux_isr(dp->aux); + ret |= dp_aux_isr(dp->aux); return ret; }