From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 23F4B2F8BEE for ; Fri, 5 Jun 2026 10:48:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780656525; cv=none; b=Dtx9UHhxfER8dO0EKLKdUO6+P17nUo+bTdUTiwXWu/eeLxcotib2h3zCnGPZlTQdjvdw+FFUw8/F+nhbQaqM3pqvWZcZH6foYRJyTOLV+vN6v9cjQf/wjvdc03I9cwqoGgCdWQPTQZCdwuyBdvpW79FOYm0JxWnaJ0GYelfP07c= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780656525; c=relaxed/simple; bh=NKkvKPtBO2Iiyx4BaKJx0azdCEH/SeTZbwaWFffbLsg=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=bjXOdZWjS71KW6wp2LnbCx1z14tajOoJp5/XGtFxGUf53xbpgRvPcmZFUHJ5T98LAQJXn+fumw82o1eWWBYmxQ9J1Hu6NkT6HoZApPvybzZ7uPuLpA/MQYSWFketAztw3sWyU72XamHdGj7DP/HQrPbTCAjp49bl8hnG1VpI5rI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=WMvTMM+R; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="WMvTMM+R" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 953D64AC6; Fri, 5 Jun 2026 03:48:37 -0700 (PDT) Received: from [10.1.31.21] (e122027.cambridge.arm.com [10.1.31.21]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D55D33F7D8; Fri, 5 Jun 2026 03:48:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1780656522; bh=NKkvKPtBO2Iiyx4BaKJx0azdCEH/SeTZbwaWFffbLsg=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=WMvTMM+RXc4Rwgmso6tlmvhAZMPmBRjT1E1NvtYnh8tsxZS5yaINZy/6oVzPi+wW7 8O2M9I/s4ydC1zJEIYCqijNoNBQSk2gTVs3rte3A7QTXnObJnbxWXyQLd+MzvznU2I qW+w+pnFuUx/+O8dLMdcP33XHbIrpgVsfWWXxMms= Message-ID: <57571cae-dac7-4550-a634-c2889a961c7b@arm.com> Date: Fri, 5 Jun 2026 11:48:36 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 6/7] drm/panfrost: Fix PM usage_count mishandling To: =?UTF-8?Q?Adri=C3=A1n_Larumbe?= , Boris Brezillon , Rob Herring , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Faith Ekstrand , "Marty E. Plummer" , Tomeu Vizoso , Eric Anholt , Alyssa Rosenzweig , Robin Murphy Cc: dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Collabora Kernel Team , Neil Armstrong References: <20260604-claude-fixes-v2-0-57c6bd4c1655@collabora.com> <20260604-claude-fixes-v2-6-57c6bd4c1655@collabora.com> From: Steven Price Content-Language: en-GB In-Reply-To: <20260604-claude-fixes-v2-6-57c6bd4c1655@collabora.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 04/06/2026 18:35, Adrián Larumbe wrote: > During device probe(), failure to do a PM get() will leave the usage_count > set to 0, which is the value assigned at device creation time. That means > when the autosuspend delay expires, runtime suspend callback won't be > invoked, so the device will remain powered on forever. > > On top of that, failure to call PM put() during device unplug means > Panfrost device's PM usage_count increases monotonically for every new > module reload. > > The combined outcome of both of the above was that devfreq OPP transition > notifications would be printed all the time, even when no jobs are being > submitted. This quickly fills the kernel ring buffer with junk. > > Even direr than that was the fact MMU interrupts are only enabled when > the device is reset, so after device probe() the very first job targeting > the tiler heap BO would always time out, because the driver's PM runtime > resume callback would not be invoked. > > Signed-off-by: Adrián Larumbe > Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization") > Fixes: 876b15d2c88d ("drm/panfrost: Fix module unload") > --- > drivers/gpu/drm/panfrost/panfrost_drv.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c > index 2d4b6aa95c66..545fbf2c8d0c 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_drv.c > +++ b/drivers/gpu/drm/panfrost/panfrost_drv.c > @@ -989,6 +989,7 @@ static int panfrost_probe(struct platform_device *pdev) > pm_runtime_set_active(pfdev->base.dev); > pm_runtime_mark_last_busy(pfdev->base.dev); > pm_runtime_enable(pfdev->base.dev); > + pm_runtime_get_noresume(pfdev->base.dev); > pm_runtime_set_autosuspend_delay(pfdev->base.dev, 50); /* ~3 frames */ > pm_runtime_use_autosuspend(pfdev->base.dev); > > @@ -1000,10 +1001,12 @@ static int panfrost_probe(struct platform_device *pdev) > if (err < 0) > goto err_out1; > > + pm_runtime_put_autosuspend(pfdev->base.dev); > > return 0; > > err_out1: > + pm_runtime_put_noidle(pfdev->base.dev); > pm_runtime_disable(pfdev->base.dev); > panfrost_device_fini(pfdev); Sashiko is concerned that dropping the usage count before pm_runtime_disable() could cause things to turn off too early, and I have to agree it sounds like it could be a problem: Sashiko wrote: > Does dropping the usage count before pm_runtime_disable() create a race > condition where the suspend callback can run and disable clocks before > hardware shutdown? > Because the usage count is dropped early, a concurrent PM event could trigger > the suspend callback, disabling clocks. Then, panfrost_device_fini() calls > panfrost_gpu_fini() which writes to MMIO registers. Could writing to > unclocked registers on ARM SoCs cause fatal bus errors or panics? Sashiko also suggests we might have some other (partially pre-existing) issues here. https://sashiko.dev/#/patchset/20260604-claude-fixes-v2-0-57c6bd4c1655%40collabora.com Thanks, Steve > pm_runtime_set_suspended(pfdev->base.dev); > @@ -1018,8 +1021,9 @@ static void panfrost_remove(struct platform_device *pdev) > drm_dev_unregister(&pfdev->base); > > pm_runtime_get_sync(pfdev->base.dev); > - pm_runtime_disable(pfdev->base.dev); > panfrost_device_fini(pfdev); > + pm_runtime_put_noidle(pfdev->base.dev); > + pm_runtime_disable(pfdev->base.dev); > pm_runtime_set_suspended(pfdev->base.dev); > } > >