From patchwork Thu Nov 23 12:05:21 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: AngeloGioacchino Del Regno X-Patchwork-Id: 168901 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp394796vqx; Thu, 23 Nov 2023 04:07:43 -0800 (PST) X-Google-Smtp-Source: AGHT+IG+/qBJGtK9JtwtnUBqzpfpg5vtxSNZAt/wM/IJLHIfxogJ0WMSTkmFq2jWdDYQnspSGnGh X-Received: by 2002:a05:6e02:1345:b0:359:8ddf:5319 with SMTP id k5-20020a056e02134500b003598ddf5319mr5280364ilr.26.1700741263618; Thu, 23 Nov 2023 04:07:43 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700741263; cv=none; d=google.com; s=arc-20160816; b=uY8YI4PGa6EmvRXzaNkTgv+7j4OPnp/I82rfal5ySpnHsHW8OKSoUN3tXFwTQQlUu9 ul0ZJc8+ptM5dw0qbD9dWwlNRTsm8xhoQIG4shGnsje0MGb0K82ZFjk5f0AhtkbNhZyx +5IQSTqFgr25bZ2twR75GYYJaM33aISivVAvK+4pZMI++AHKfvMvHqovDp0nY85JGHMy z8avK8cs5RzQQPwdbLnqhKIfan2tv6EPNDRjv0yTDBGgwEyECLyCqzXLiBtTUB9hHryT aYR7F6F6Rshts1g/6ychCLxz5dTO3ZbQigqCiBWItTA9P2IsDEPY8bFRo/vFkx0dCLff 5Nfw== 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=FdaxanrrtbXvJ8LUXHU1WL7CWUKwPRRfUnHgWVzm9A8=; fh=K65JxhdzjVRH8Fw/+Xsss/BzR8g3wU1drRz29wc70AU=; b=G/683OEBNl2B5MdS0j9w8DuILa+cFkw1FhvKqrhhO0W7o8mANBk9GffAqPQf4WUF+J DgEAWspa8OGV2GnQeL87xW1XXUOh7GmXubXu0dc52zC8MjYPz6aFH8AyaZZ8fcmo6ywW bCRnRqEv2VsO4rV0ALpDDktY0HHq5zATY7ntPjXbnoO6yMQa/XC6Px/UVCJDxiu2Fv7p Qus9k491hPtxvRqx03hXGNDqhFQwK+MoCXK0FxWnJ+nKcz2d722+Df8tRS4qJgE0asDy crJwqClMb5DnGdcJytTFZ+9hcQOj8GGW0ZEtCpqBKQfackApgnaQjmw1Rrzqbt0Jnpry YWCg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=cuv31xir; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Received: from groat.vger.email (groat.vger.email. [2620:137:e000::3:5]) by mx.google.com with ESMTPS id 10-20020a63144a000000b00584b6af3b9fsi1168614pgu.524.2023.11.23.04.07.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Nov 2023 04:07:43 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5; Authentication-Results: mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=cuv31xir; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=collabora.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by groat.vger.email (Postfix) with ESMTP id AE6CE80740F6; Thu, 23 Nov 2023 04:05:34 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at groat.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345206AbjKWMFZ (ORCPT + 99 others); Thu, 23 Nov 2023 07:05:25 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35996 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345151AbjKWMFX (ORCPT ); Thu, 23 Nov 2023 07:05:23 -0500 Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 73947110 for ; Thu, 23 Nov 2023 04:05:29 -0800 (PST) Received: from IcarusMOD.eternityproject.eu (cola.collaboradmins.com [195.201.22.229]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kholk11) by madras.collabora.co.uk (Postfix) with ESMTPSA id 7F99C66073A3; Thu, 23 Nov 2023 12:05:27 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1700741128; bh=Ztfry6hm4G915tlRPUEks10lc95x+AKs0SuSoQiVF5k=; h=From:To:Cc:Subject:Date:From; b=cuv31xir6qEzG+dcpVpNptwAd8CvR8n/kBjMtbxbF/dCfBbE+WMF5zKg+SA3Hom5L bGCyRvTP5bNi437ZiWmV1GPM8NyaCsIRZI0uvk2L1b6NVRfhkzjHcVIZej6wiuISjw rNejLv+l0SIbpsFrT2j473fXlc/5zEMgkOqeIboler5LkRdicW56NE4TJKo8cP4+F0 Qx20DR0DwnCd+Qc/KDtCDY2kit5BARlRsN7NplGWogLfOqcr082sjq5Ue19xTSw0Bp U07uGp2fxmG221ZB6jp2kbW4ngL2KlppSH0Zju6iPKxRR4tGDTfyUb6iQxkjldZZwx MxEPt/7/fpKdg== From: AngeloGioacchino Del Regno To: steven.price@arm.com Cc: boris.brezillon@collabora.com, robh@kernel.org, maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@gmail.com, daniel@ffwll.ch, angelogioacchino.delregno@collabora.com, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, krzysztof.kozlowski@linaro.org Subject: [PATCH v3] drm/panfrost: Ignore core_mask for poweroff and sync interrupts Date: Thu, 23 Nov 2023 13:05:21 +0100 Message-ID: <20231123120521.147695-1-angelogioacchino.delregno@collabora.com> X-Mailer: git-send-email 2.42.0 MIME-Version: 1.0 X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (groat.vger.email [0.0.0.0]); Thu, 23 Nov 2023 04:05:34 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783348061402553696 X-GMAIL-MSGID: 1783356471358225177 Some SoCs may be equipped with a GPU containing two core groups and this is exactly the case of Samsung's Exynos 5422 featuring an ARM Mali-T628 MP6 GPU: the support for this GPU in Panfrost is partial, as this driver currently supports using only one core group and that's reflected on all parts of it, including the power on (and power off, previously to this patch) function. The issue with this is that even though executing the soft reset operation should power off all cores unconditionally, on at least one platform we're seeing a crash that seems to be happening due to an interrupt firing which may be because we are calling power transition only on the first core group, leaving the second one unchanged, or because ISR execution was pending before entering the panfrost_gpu_power_off() function and executed after powering off the GPU cores, or all of the above. Finally, solve this by introducing a new panfrost_gpu_suspend_irq() helper function and changing the panfrost_device_suspend() flow to 1. Mask and clear all interrupts: we don't need nor want any, as for power_off() we are polling PWRTRANS, but we anyway don't want GPU IRQs to fire while it is suspended/powered off; 2. Call synchronize_irq() after that to make sure that any pending ISR is executed before powering off the GPU Shaders/Tilers/L2 hence avoiding unpowered registers R/W; and 3. Ignore the core_mask and ask the GPU to poweroff both core groups Of course it was also necessary to add a `irq` variable to `struct panfrost_device` as we need to get that in panfrost_gpu_power_off() for calling synchronize_irq() on it. Fixes: 22aa1a209018 ("drm/panfrost: Really power off GPU cores in panfrost_gpu_power_off()") [Regression detected on Odroid HC1, Exynos5422, Mali-T628 MP6] Reported-by: Krzysztof Kozlowski Signed-off-by: AngeloGioacchino Del Regno Tested-by: Krzysztof Kozlowski --- Changes in v3: - Fix compile issue Changes in v2: - Fixed the commit hash of "Really power off [...]" - Actually based on a clean next-20231121 - Renamed "irq" to "gpu_irq" as per Boris' suggestion - Moved the IRQ mask/clear/sync to a helper function and added a call to that in panfrost_device.c instead of doing that in panfrost_gpu_power_off(). NOTE: I didn't split 1+2 from 3 as suggested by Boris, and I'm sending this one without waiting for feedback on my reasons for that which I explained as a reply to v1 because the former couldn't be applied to linux-next, and I want to unblock Krzysztof ASAP to get this tested. drivers/gpu/drm/panfrost/panfrost_device.c | 1 + drivers/gpu/drm/panfrost/panfrost_device.h | 1 + drivers/gpu/drm/panfrost/panfrost_gpu.c | 28 +++++++++++++++------- drivers/gpu/drm/panfrost/panfrost_gpu.h | 1 + 4 files changed, 23 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index c90ad5ee34e7..b0a4f3e513f4 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -421,6 +421,7 @@ static int panfrost_device_runtime_suspend(struct device *dev) return -EBUSY; panfrost_devfreq_suspend(pfdev); + panfrost_gpu_suspend_irq(pfdev); panfrost_gpu_power_off(pfdev); return 0; diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index 0fc558db6bfd..f2b1d4afb9e9 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -94,6 +94,7 @@ struct panfrost_device { struct device *dev; struct drm_device *ddev; struct platform_device *pdev; + int gpu_irq; void __iomem *iomem; struct clk *clock; diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index 09f5e1563ebd..2c09aede0945 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -425,11 +425,10 @@ void panfrost_gpu_power_on(struct panfrost_device *pfdev) void panfrost_gpu_power_off(struct panfrost_device *pfdev) { - u64 core_mask = panfrost_get_core_mask(pfdev); int ret; u32 val; - gpu_write(pfdev, SHADER_PWROFF_LO, pfdev->features.shader_present & core_mask); + gpu_write(pfdev, SHADER_PWROFF_LO, pfdev->features.shader_present); ret = readl_relaxed_poll_timeout(pfdev->iomem + SHADER_PWRTRANS_LO, val, !val, 1, 1000); if (ret) @@ -441,16 +440,29 @@ void panfrost_gpu_power_off(struct panfrost_device *pfdev) if (ret) dev_err(pfdev->dev, "tiler power transition timeout"); - gpu_write(pfdev, L2_PWROFF_LO, pfdev->features.l2_present & core_mask); + gpu_write(pfdev, L2_PWROFF_LO, pfdev->features.l2_present); ret = readl_poll_timeout(pfdev->iomem + L2_PWRTRANS_LO, val, !val, 0, 1000); if (ret) dev_err(pfdev->dev, "l2 power transition timeout"); } +void panfrost_gpu_suspend_irq(struct panfrost_device *pfdev) +{ + /* Disable all interrupts before suspending the GPU */ + gpu_write(pfdev, GPU_INT_MASK, 0); + gpu_write(pfdev, GPU_INT_CLEAR, GPU_IRQ_MASK_ALL); + + /* + * Make sure that we don't have pending ISRs, otherwise we'll be + * reading and/or writing registers while the GPU is powered off + */ + synchronize_irq(pfdev->gpu_irq); +} + int panfrost_gpu_init(struct panfrost_device *pfdev) { - int err, irq; + int err; err = panfrost_gpu_soft_reset(pfdev); if (err) @@ -465,11 +477,11 @@ int panfrost_gpu_init(struct panfrost_device *pfdev) dma_set_max_seg_size(pfdev->dev, UINT_MAX); - irq = platform_get_irq_byname(to_platform_device(pfdev->dev), "gpu"); - if (irq < 0) - return irq; + pfdev->gpu_irq = platform_get_irq_byname(to_platform_device(pfdev->dev), "gpu"); + if (pfdev->gpu_irq < 0) + return pfdev->gpu_irq; - err = devm_request_irq(pfdev->dev, irq, panfrost_gpu_irq_handler, + err = devm_request_irq(pfdev->dev, pfdev->gpu_irq, panfrost_gpu_irq_handler, IRQF_SHARED, KBUILD_MODNAME "-gpu", pfdev); if (err) { dev_err(pfdev->dev, "failed to request gpu irq"); diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.h b/drivers/gpu/drm/panfrost/panfrost_gpu.h index 876fdad9f721..d841b86504ea 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.h +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.h @@ -15,6 +15,7 @@ u32 panfrost_gpu_get_latest_flush_id(struct panfrost_device *pfdev); int panfrost_gpu_soft_reset(struct panfrost_device *pfdev); void panfrost_gpu_power_on(struct panfrost_device *pfdev); void panfrost_gpu_power_off(struct panfrost_device *pfdev); +void panfrost_gpu_suspend_irq(struct panfrost_device *pfdev); void panfrost_cycle_counter_get(struct panfrost_device *pfdev); void panfrost_cycle_counter_put(struct panfrost_device *pfdev);