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 6FD004028E8 for ; Tue, 19 May 2026 14:19:51 +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=1779200392; cv=none; b=RNYug8bueWgmC17f/rtO3fQCsh55BGbHBRgD4PxJM1s5TvMEtS40kYUpZ+nyz5klg8kIJVwdjPoHIf6B3Q3duwmWVhB6eaGNUGVgGzEse17R2JSd8aRuc7XoW6pKiFog+6oEO20ISoEqiNk0Imrt0S8tzmM3LCMpZHCvNwSkmxk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779200392; c=relaxed/simple; bh=mdBbzjNI/cEnhs+9shTezmkc6qwa3DXnypjDBGjWzV8=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=qt6FA8A35pyaRxQGIoELu2Q4F5JVOYM0myo7tWjrE+3VfZ8xDEF7IYgqenNcpM45WNjCGxMmJ0UYDxPhg9M9MaKrnszSz4F+fL89pvEyWnISBZCMVHwhYs5qZ0iSENyBURIH4h2LaYr2u7gKwhza4exSCjqB26ulH0LhXuujmd0= 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=qn+BcsoS; 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="qn+BcsoS" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1779200389; bh=mdBbzjNI/cEnhs+9shTezmkc6qwa3DXnypjDBGjWzV8=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=qn+BcsoSl1ORad3e/LA/M9iuf8rP/z/n9c8SoKAYpF8ow35GLCwHnUqlJ6EAKQjvU LPB2QI1CVFfQleH+S6b8OS5hs+jyU7RphdbbwOOEVq5tVdiLIxRuJlK2BGeKVLbc/B ogLY/ivPvgUVFGYctsEojYFt14vaYpYy2lBsNNYlcu/rMU1oG0RpD7aNYR4T7a2T31 PUSuNqfAlJXzkKseTuZNHXEIR+ZBVoUcNmYXoHn6PfIXQzznkbBtlL+PIUUIS3OQH+ Qip63sSooZrH2VqEsHN3QjITbR7xlT1IMVSyPmNPH+jDlyOMWbX8PtK4Mn7La/NEBj h+NqvfZkifJww== Received: from fedora (unknown [100.64.0.11]) (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 2BD3717E02A3; Tue, 19 May 2026 16:19:49 +0200 (CEST) Date: Tue, 19 May 2026 16:19:44 +0200 From: Boris Brezillon To: Steven Price Cc: Liviu Dudau , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , David Airlie , Simona Vetter , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 08/11] drm/panthor: Automatically enable interrupts in panthor_fw_wait_acks() Message-ID: <20260519161944.285178c6@fedora> In-Reply-To: <20260518101656.6426fb18@fedora> References: <20260512-panthor-signal-from-irq-v2-0-95c614a739cb@collabora.com> <20260512-panthor-signal-from-irq-v2-8-95c614a739cb@collabora.com> <44d8b158-76bd-4ad5-9fd5-616afd432155@arm.com> <20260518101656.6426fb18@fedora> Organization: Collabora X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; 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 Mon, 18 May 2026 10:16:56 +0200 Boris Brezillon wrote: > On Thu, 14 May 2026 15:25:07 +0100 > Steven Price wrote: > > > On 12/05/2026 12:37, Boris Brezillon wrote: > > > Rather than assuming an interrupt is always expected for request > > > acks, temporarily enable the relevant interrupts when the polling-wait > > > failed. This should hopefully reduce the number of interrupts the CPU > > > has to process. > > > > > > Signed-off-by: Boris Brezillon > > > > As mentioned in the other thread[1] it turns out this won't work with > > the current firmware. > > > > The firmware checks the interrupt mask before signalling the ACK - so > > enabling the bit in the mask just before waiting for it is problematic - > > the firmware may not see the addition in the mask and will not trigger > > the interrupt. > > Is it a problem though? wait_event_timeout() will evaluate the > condition before going to sleep, so, if the FW raced with the > input->ack_irq_mask update, I assume the condition will evaluate to > true and wait_event_timeout() would return immediately. The only issue > is if the FW updates the output->ack register after reading > input->ack_irq_mask, but that would be weird, since the output->ack > update doesn't depend on input->ack_irq_mask, and raising an interrupt > before updating output->ack would be racy anyway. > > Am I missing something? > Quoting your reply to v1, for the context, because I missed it when replying here: > So I've looked at the firmware implementation and I can say that there's > a race condition on the firmware side if we change the mask after > sending the START/RESUME request. The firmware currently samples the > mask before triggering the update to CSG_ACK and then uses the sampled > value to decide whether to trigger an interrupt. So, the race is: CPU MCU CSG_REQ.STATE = START atomic_poll(CSG_ACK & STATE_MASK) process_state_change() ... mask = CSG_ACK_IRQ_MASK ... // done CSG_ACK_IRQ_MASK |= STATE_MASK wait(CSG_ACK & STATE_MASK) CSG_ACK = (CSG_ACK & STATE_MASK) | CSG_REQ.STATE if (mask & STATE_MASK) // evaluates to false raise_int() It's a bit unfortunate, because conditionally enabling IRQs only after the polling period expires reduces the number of interrupts directly caused by requests initiated by the CPU since it's assumed those requests will complete relatively quickly and avoid a sleep+interrupt+wake-up round-trip. We could of course update CSG_ACK_IRQ_MASK at the time we update CSG_REQ, but that means we'll still get an interrupt, so I guess we should stick to what exists now if there's no other option. Oh well. > > This is all fine if the mask is set before the request (so nothing > broken with the current code), but we'd need a firmware change before we > can safely do what this patch was proposing. And of course we'd have to > get our heads round the barriers needed! ;) :-(