From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from bali.collaboradmins.com (bali.collaboradmins.com [148.251.105.195]) (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 149903314D9 for ; Thu, 18 Dec 2025 12:38:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=148.251.105.195 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766061514; cv=none; b=qQPh5HP6G2qSxig6PZg6ttbrsaFWcWi+ykbcoKFLGTygGtEtV5LTVlVBYC5MMBvIZXn9shCMOqOXDmCbnUZMbdtTVmu8qtZf8gKYqHoXmjbiAVVYalPINHocBfh7ucyarCyCGDeQh+vdfsIh4ks/y6Rg46jD2PoTz1RlOnQCdac= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1766061514; c=relaxed/simple; bh=PbmbSGRTKNSlpkIMIcAeqY7irz7C8Mi8TAsByNoZXMM=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=Ogtjbrr0RhO9lDN0hY3MmdjQ2Yqf29n03e3nwB8kh0Gp4YFNJVk9USlpHktx1gCDi5dkqDjGZ8eY8MP7E3S0g5sWs9qhsWRVqmnX6Zukik1ip8yWd+8mIHjSY9hcHfwWHDhBekgHoy9gaHKtH45TnuBqixibC+uV+1SfiqNZXXg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=collabora.com; spf=pass smtp.mailfrom=collabora.com; dkim=pass (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b=ETaaGTSL; arc=none smtp.client-ip=148.251.105.195 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 (2048-bit key) header.d=collabora.com header.i=@collabora.com header.b="ETaaGTSL" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1766061510; bh=PbmbSGRTKNSlpkIMIcAeqY7irz7C8Mi8TAsByNoZXMM=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=ETaaGTSL6qR2F592lIRCNEvXi5wNXkJNBmV30CMka//yOkrC5lsKCu80cq0ZK2JRs daoXV8L59oPoiZrYjNLGu+uH1BdtZFJjHTzCdnvm/n2D8tw1KcMqN+lVNMq3vS88im 6zjlSfrX2WM57TZpofluyheazYdEGzLqt1MRQrvR09z0/3EIMelsg/TC9aao8NycZN 5LSFbTOlpqBzPIPoY+zrqFmNZnbSwmoUpNaI0Z9w34UGO7houQ1iyXvsqlNmXxb/As zO+H+QD2xwExMG4tkkbXUNIDr/uCfUDD28C9lZEuFrBhzGUD9uw+zCFtjqKNPttyw0 djk8iRjFf4CmA== Received: from fedora (unknown [IPv6:2a01:e0a:2c:6930:d919:a6e:5ea1:8a9f]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (prime256v1) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: bbrezillon) by bali.collaboradmins.com (Postfix) with ESMTPSA id 9E57B17E0184; Thu, 18 Dec 2025 13:38:29 +0100 (CET) Date: Thu, 18 Dec 2025 13:38:25 +0100 From: Boris Brezillon To: Nicolas Frattaroli Cc: Steven Price , Liviu Dudau , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , Chia-I Wu , Karunika Choo , kernel@collabora.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH v4 1/3] drm/panthor: Add panthor_*_irq_mask_set helper Message-ID: <20251218133825.64177ebf@fedora> In-Reply-To: <20251217-panthor-tracepoints-v4-1-916186cb8d03@collabora.com> References: <20251217-panthor-tracepoints-v4-0-916186cb8d03@collabora.com> <20251217-panthor-tracepoints-v4-1-916186cb8d03@collabora.com> Organization: Collabora X-Mailer: Claws Mail 4.3.1 (GTK 3.24.51; x86_64-redhat-linux-gnu) 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=US-ASCII Content-Transfer-Encoding: 7bit On Wed, 17 Dec 2025 15:29:38 +0100 Nicolas Frattaroli wrote: > Add a function to modify an IRQ's mask. If the IRQ is currently active, > it will write to the register, otherwise it will only set the struct > member. > > There's no locking done to guarantee exclusion with the other two > functions that touch the IRQ mask, and it should only be called from a > context where the circumstances guarantee no concurrent access is > performed. > > Signed-off-by: Nicolas Frattaroli > --- > drivers/gpu/drm/panthor/panthor_device.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/panthor/panthor_device.h b/drivers/gpu/drm/panthor/panthor_device.h > index f35e52b9546a..894d28b3eb02 100644 > --- a/drivers/gpu/drm/panthor/panthor_device.h > +++ b/drivers/gpu/drm/panthor/panthor_device.h > @@ -470,6 +470,13 @@ static int panthor_request_ ## __name ## _irq(struct panthor_device *ptdev, \ > panthor_ ## __name ## _irq_threaded_handler, \ > IRQF_SHARED, KBUILD_MODNAME "-" # __name, \ > pirq); \ > +} \ > + \ > +static inline void panthor_ ## __name ## _irq_mask_set(struct panthor_irq *pirq, u32 mask) \ > +{ \ > + pirq->mask = mask; \ > + if (!atomic_read(&pirq->suspended)) \ > + gpu_write(pirq->ptdev, __reg_prefix ## _INT_MASK, mask); \ This is racy if called outside the (threaded) IRQ handler, which I believe is the case since its called when a trace point is enabled: 1. _irq_raw_handler() sets INT_MASK to zero to avoid receiving interrupts from this IRQ line until the threaded handler has processed events 2. _irq_mask_set() sets INT_MASK to something non-zero, meaning the interrupt will be re-enabled before events have been processed this leads to at least one spurious interrupt being received before we set INT_MASK to zero again. Probably not the end of the world, but if we can avoid it, that'd be better. Also, I'd like to see if we could re-purpose panthor_irq::mask to be the mask of events the user wants to monitor instead of a pure proxy of INT_MASK. If we do that, and we make panthor_irq::mask an atomic_t, we can add panthor_xxx_irq_{enable,disable}_event() helpers that would do the atomic_{or,and} on panthor_irq::mask, and write the new value to _INT_MASK if: - we're processing events in the threaded handler (we would need another field, or we'd need to turn suspended into a state that can encode more than just "suspended or not") - the device is not suspended (that test you already have) > } > > extern struct workqueue_struct *panthor_cleanup_wq; >