From patchwork Thu Nov 23 11:50:29 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: 168893 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:ce62:0:b0:403:3b70:6f57 with SMTP id o2csp385002vqx; Thu, 23 Nov 2023 03:50:53 -0800 (PST) X-Google-Smtp-Source: AGHT+IGyccBATn/0TIevdXQv6fF+QQ9qFd+r+4OOS9a8ssUCCyOQV5rtGO2NdswYuz+b4hjcrpN7 X-Received: by 2002:a05:6a21:60a:b0:189:b35b:3f2 with SMTP id ll10-20020a056a21060a00b00189b35b03f2mr4892777pzb.35.1700740252048; Thu, 23 Nov 2023 03:50:52 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1700740252; cv=none; d=google.com; s=arc-20160816; b=pC4wI7ja1F/Kju1JeK6jG3mWl9lWpwEeaJsFLPLvT1+TjrNX2DOLmYQPLXSKY8v6B2 VL4WGkcXfAKqaQa9AhCEONQQ3pjTJZiu0r3qQpj5U376Y6HsPKIPwQ6QDWys+PlZEwut mG+GC7U2yYWPwVfToOghrWOGw6CPJ2H0SpFUwNxG7TK+/6UTVh0nrI53gu/BjZt9QFVL 8YgxGw/P2M1+5pOL3WHN/YAJ3lxwYsMf9lz3POSyjbP8PbOVEx6oAcBn4zc52oD5M9Gc sWG0i1mGJ9fCYcxUEo9iC0csxBzoodowSbGCYwYzBBeiSLhYIj27jr2W3lM9eKkp8rd/ jD5g== 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=RvM+3J3OcY5sAxY9+LiXMZaXgHzyiMFP9LaxmTis88c=; fh=K65JxhdzjVRH8Fw/+Xsss/BzR8g3wU1drRz29wc70AU=; b=Qx4rTl6HWlsyWyIecvih9NvxLv5f2KRhq0YmskeTC0EtMXoQuCNf97OtcE9AnVVXV2 FDzZa4zJ4nVlNuy5Dh+JAMbuvjdzbk+CpAE6LKafSPA222dgD/rzBm+OHstcpJK7rrZ4 q8ar0jsdGnwsZ1nRkSfcvEJz7GyLRrgDt1UrvbldHG5K3AmXvKrU3ZhFeLmfGv2JOBpw IEl7RjBWvwmGzkf2ub9O6ncy/sGpPrfxs2mMzkURSGGEHe903xIAqw9u9RMFhula5Cwh 8dyeCfU450LMs3kW/iAHUwljSK2GfB4wN3w8BFrlH9iABZL4Q+g00O/szW0TP+YZDt+K ZJqw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=AGPkOiF8; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 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 howler.vger.email (howler.vger.email. [23.128.96.34]) by mx.google.com with ESMTPS id s5-20020a656445000000b00565e0624182si1089382pgv.404.2023.11.23.03.50.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Nov 2023 03:50:52 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34; Authentication-Results: mx.google.com; dkim=pass header.i=@collabora.com header.s=mail header.b=AGPkOiF8; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 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 howler.vger.email (Postfix) with ESMTP id 95AC7808BCBF; Thu, 23 Nov 2023 03:50:44 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at howler.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1345118AbjKWLuf (ORCPT + 99 others); Thu, 23 Nov 2023 06:50:35 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46136 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345105AbjKWLuf (ORCPT ); Thu, 23 Nov 2023 06:50:35 -0500 Received: from madras.collabora.co.uk (madras.collabora.co.uk [46.235.227.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1425F98 for ; Thu, 23 Nov 2023 03:50:40 -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 D4E4B660739D; Thu, 23 Nov 2023 11:50:38 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1700740239; bh=dh2u8+ENkzMcCabo+NPjxzRrT5AZKX5LLmWOZX/WdMo=; h=From:To:Cc:Subject:Date:From; b=AGPkOiF8QGU98z6j6hhUuTZWRxGHq6SROI9m1VlPT7DbVaZipMFDKR56F31owqYvT hMFwCjsd2TNF0mrcalG5HCiN05IfPwQAmB9ZSORQfPCg75UewEP6S5rnLR7ApUJyWa h95lDFmeWAwYSj15ykcNj3KVHXjdO/yZi9t9i22qL/eMRdEgAmHMB29YxDYPO/lko/ W9fwEgFoBC/f9bO7SxhhbC1SMZylY/pLzorBw7jkYdo14gWAqiRvKxjQIcibBEPhIR nzH7bVgmLFJRHDCxg3JOCeTw64YV/26GMHHQfzhKl8a1zqzQKcgcUnGO8lsSdbFR5m dkVv/Yb6fIBMw== 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 v2] drm/panfrost: Ignore core_mask for poweroff and sync interrupts Date: Thu, 23 Nov 2023 12:50:29 +0100 Message-ID: <20231123115029.68422-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 howler.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 (howler.vger.email [0.0.0.0]); Thu, 23 Nov 2023 03:50:44 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1783348061402553696 X-GMAIL-MSGID: 1783355410484649343 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 --- 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 | 26 ++++++++++++++++------ drivers/gpu/drm/panfrost/panfrost_gpu.h | 1 + 4 files changed, 22 insertions(+), 7 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..efda085c2b95 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) @@ -448,9 +447,22 @@ void panfrost_gpu_power_off(struct panfrost_device *pfdev) 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);