From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from sender4-op-o11.zoho.com (sender4-op-o11.zoho.com [136.143.188.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EEF36303C8A for ; Tue, 16 Jun 2026 19:48:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=pass smtp.client-ip=136.143.188.11 ARC-Seal:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781639329; cv=pass; b=p5otJbHSgGNo660QIVWG2Glwq7aHWvqcmNLv2s2iM5xnkJOqF8WH3sskiDFrSETFTJoCuyVVenuoRAkxJixVQAT0cTfjeCjdYdtVttpzmRr4S8sQes6OquM3kl888ddpHBNT5MYDTjZSIAy0xf5ZppM5wgUffsbVh+F5Dak2RRE= ARC-Message-Signature:i=2; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781639329; c=relaxed/simple; bh=NoW84pg5n5dgdwr241+fwP4VkJ9sv3DJCe0ZlSJT6NU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=FjpDbDmyKGDwMfm3vW8R+wdCxgqNz76jQJW0kNLXGxqT1DO1g6qchxdUh4Djd/O+t7LIlTehN0qFOfdIFjuq6ZXliqzaRQB4H8h3Wg3ZwrRpRdQ1QyXjPNDVJb+bl9cRua32bAHmaia4FJzUzFId/VM+X3arOxsRmJyqaU5s1Bw= ARC-Authentication-Results:i=2; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (1024-bit key) header.d=collabora.com header.i=adrian.larumbe@collabora.com header.b=GeNCVNxJ; arc=pass smtp.client-ip=136.143.188.11 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=collabora.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=collabora.com header.i=adrian.larumbe@collabora.com header.b="GeNCVNxJ" ARC-Seal: i=1; a=rsa-sha256; t=1781639305; cv=none; d=zohomail.com; s=zohoarc; b=OwQmF6a0MBQulcy9vsdQV2YD+kMRaIVrNR12C1u/kVUl+XnalMOYATNh4nr7KlYsxU7GizDfV11L2/YOOSSx99VhJg3aLah8y/5OezWdWoRiOrCr1qR/L+irJIzZXfx48H3FwQUT2cR+Z/PCaPCiVB+KCDv95wscDyJBG+wGt2c= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1781639305; h=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=P1zqWBMiSEg6eraAaC2x1b5nQhRS3dgZDM0hA0HtxG4=; b=oC3kp1DQP9zt+D3UF12BRE/cF8R0CR4G6e3zdR3GqzVRd9ImiHfDISqAz//BNZ14+kATB/IJLyyJ7aoKwVTJl0itjIIlt+zFptz8O4ByQ4cMVdMcasx2z2O2QKJOI0XdBumuaIQKHsHFPYh0gunT6IIv7a43vfc05IGHyrPXbiM= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass header.i=collabora.com; spf=pass smtp.mailfrom=adrian.larumbe@collabora.com; dmarc=pass header.from= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; t=1781639305; s=zohomail; d=collabora.com; i=adrian.larumbe@collabora.com; h=Date:Date:From:From:To:To:Cc:Cc:Subject:Subject:Message-ID:References:MIME-Version:Content-Type:Content-Transfer-Encoding:In-Reply-To:Message-Id:Reply-To; bh=P1zqWBMiSEg6eraAaC2x1b5nQhRS3dgZDM0hA0HtxG4=; b=GeNCVNxJGhNFUq+DyjVjlauK6xYcx+SPN/Ez/D/O1HOW22g9Sq8HnB7RH0dmaLa2 OXr/WaysOCMG009+b1tjciFnbCcRH0KCGnn/mtbtAXYqFCIhzA6fLCG0RnFsED4zpLS 7M35XDT+nrH5wiM4LFiho4ZxLzrweaOXENXaIMFs= Received: by mx.zohomail.com with SMTPS id 1781639304959241.30848604580262; Tue, 16 Jun 2026 12:48:24 -0700 (PDT) Date: Tue, 16 Jun 2026 20:48:19 +0100 From: =?utf-8?Q?Adri=C3=A1n?= Larumbe To: Steven Price Cc: 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 , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Collabora Kernel Team , Neil Armstrong Subject: Re: [PATCH v2 6/7] drm/panfrost: Fix PM usage_count mishandling Message-ID: References: <20260604-claude-fixes-v2-0-57c6bd4c1655@collabora.com> <20260604-claude-fixes-v2-6-57c6bd4c1655@collabora.com> <57571cae-dac7-4550-a634-c2889a961c7b@arm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <57571cae-dac7-4550-a634-c2889a961c7b@arm.com> On 05.06.2026 11:48, Steven Price wrote: > 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? I think this could be an issue if the device were already registered and someone could drive the pm resume and then suspend sequence through an ioctl, but because this is an error path and yet the device was never made available, I can't imagine how this could happen. Maybe if the panfrost device had any children devices, and when one of them did a put autosuspend, the refcnt would be propagdated back to Panfrost and then trigger the scenario described by shashiko. However, I've just realised it's alright to call pm_runtime_put_noidle() even after pm_runtime_disable(). Seems that the latter just prevents any further suspends or resumes on the PM device, but we're still in control of the refcnt, so moving pm_runtime_put_noidle() right after panfrost_device_fini() should be fine. > 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 I'll look into all the pre-existing issues and write fixes for the next patch series revision. > 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); > > } > > > > Adrian Larumbe