[PATCH v9 6/7] drm/panthor: Make MMU cache maintenance use FLUSH_CACHES command

Karunika Choo posted 7 patches 1 month, 4 weeks ago
[PATCH v9 6/7] drm/panthor: Make MMU cache maintenance use FLUSH_CACHES command
Posted by Karunika Choo 1 month, 4 weeks ago
As the FLUSH_MEM and FLUSH_PT MMU_AS commands are deprecated in GPUs
from Mali-Gx20 onwards, this patch adds support for performing cache
maintenance via the FLUSH_CACHES command in GPU_COMMAND in place of
FLUSH_MEM and FLUSH_PT commands.

Mali-Gx10 and Mali-Gx15 GPUs also has support for the FLUSH_CACHES
command and will also use this by default going forward.

Reviewed-by: Steven Price <steven.price@arm.com>
Reviewed-by: Chia-I Wu <olvaffe@gmail.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Karunika Choo <karunika.choo@arm.com>
---
 drivers/gpu/drm/panthor/panthor_mmu.c | 33 +++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/panthor/panthor_mmu.c b/drivers/gpu/drm/panthor/panthor_mmu.c
index 4140f697ba5a..367c89aca558 100644
--- a/drivers/gpu/drm/panthor/panthor_mmu.c
+++ b/drivers/gpu/drm/panthor/panthor_mmu.c
@@ -29,6 +29,7 @@
 
 #include "panthor_device.h"
 #include "panthor_gem.h"
+#include "panthor_gpu.h"
 #include "panthor_heap.h"
 #include "panthor_mmu.h"
 #include "panthor_regs.h"
@@ -568,6 +569,35 @@ static void lock_region(struct panthor_device *ptdev, u32 as_nr,
 	write_cmd(ptdev, as_nr, AS_COMMAND_LOCK);
 }
 
+static int mmu_hw_do_flush_on_gpu_ctrl(struct panthor_device *ptdev, int as_nr,
+				       u32 op)
+{
+	const u32 l2_flush_op = CACHE_CLEAN | CACHE_INV;
+	u32 lsc_flush_op = 0;
+	int ret;
+
+	if (op == AS_COMMAND_FLUSH_MEM)
+		lsc_flush_op = CACHE_CLEAN | CACHE_INV;
+
+	ret = wait_ready(ptdev, as_nr);
+	if (ret)
+		return ret;
+
+	ret = panthor_gpu_flush_caches(ptdev, l2_flush_op, lsc_flush_op, 0);
+	if (ret)
+		return ret;
+
+	/*
+	 * Explicitly unlock the region as the AS is not unlocked automatically
+	 * at the end of the GPU_CONTROL cache flush command, unlike
+	 * AS_COMMAND_FLUSH_MEM or AS_COMMAND_FLUSH_PT.
+	 */
+	write_cmd(ptdev, as_nr, AS_COMMAND_UNLOCK);
+
+	/* Wait for the unlock command to complete */
+	return wait_ready(ptdev, as_nr);
+}
+
 static int mmu_hw_do_operation_locked(struct panthor_device *ptdev, int as_nr,
 				      u64 iova, u64 size, u32 op)
 {
@@ -585,6 +615,9 @@ static int mmu_hw_do_operation_locked(struct panthor_device *ptdev, int as_nr,
 	if (op != AS_COMMAND_UNLOCK)
 		lock_region(ptdev, as_nr, iova, size);
 
+	if (op == AS_COMMAND_FLUSH_MEM || op == AS_COMMAND_FLUSH_PT)
+		return mmu_hw_do_flush_on_gpu_ctrl(ptdev, as_nr, op);
+
 	/* Run the MMU operation */
 	write_cmd(ptdev, as_nr, op);
 
-- 
2.49.0
Re: [PATCH v9 6/7] drm/panthor: Make MMU cache maintenance use FLUSH_CACHES command
Posted by Daniel Stone 1 month, 3 weeks ago
Hi Karunika,


On Thu, 7 Aug 2025 at 17:27, Karunika Choo <karunika.choo@arm.com> wrote:
> @@ -585,6 +615,9 @@ static int mmu_hw_do_operation_locked(struct panthor_device *ptdev, int as_nr,
>         if (op != AS_COMMAND_UNLOCK)
>                 lock_region(ptdev, as_nr, iova, size);
>
> +       if (op == AS_COMMAND_FLUSH_MEM || op == AS_COMMAND_FLUSH_PT)
> +               return mmu_hw_do_flush_on_gpu_ctrl(ptdev, as_nr, op);

Given that FLUSH_MEM and FLUSH_PT are the only ops which are ever
used, the below becomes dead code. Could you please just inline these,
so it's more clear what's actually going on? The (op !=
AS_COMMAND_UNLOCK) branch can also become unconditional, perhaps with
a WARN_ON() around unknown ops.

Cheers,
Daniel
Re: [PATCH v9 6/7] drm/panthor: Make MMU cache maintenance use FLUSH_CACHES command
Posted by Liviu Dudau 1 month, 3 weeks ago
On Fri, Aug 08, 2025 at 11:50:27AM +0100, Daniel Stone wrote:
> Hi Karunika,
> 
> 
> On Thu, 7 Aug 2025 at 17:27, Karunika Choo <karunika.choo@arm.com> wrote:
> > @@ -585,6 +615,9 @@ static int mmu_hw_do_operation_locked(struct panthor_device *ptdev, int as_nr,
> >         if (op != AS_COMMAND_UNLOCK)
> >                 lock_region(ptdev, as_nr, iova, size);
> >
> > +       if (op == AS_COMMAND_FLUSH_MEM || op == AS_COMMAND_FLUSH_PT)
> > +               return mmu_hw_do_flush_on_gpu_ctrl(ptdev, as_nr, op);
> 
> Given that FLUSH_MEM and FLUSH_PT are the only ops which are ever
> used, the below becomes dead code. Could you please just inline these,
> so it's more clear what's actually going on? The (op !=
> AS_COMMAND_UNLOCK) branch can also become unconditional, perhaps with
> a WARN_ON() around unknown ops.

Hmm, the commit message says that FLUSH_MEM and FLUSH_PT are going to be
deprecated and replaced with FLUSH_CACHES so the first are clearly not the
only ones ever used (at least not in the future). I'm not sure why you
think this code is not correct.

Best regards,
Liviu

> 
> Cheers,
> Daniel

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯
Re: [PATCH v9 6/7] drm/panthor: Make MMU cache maintenance use FLUSH_CACHES command
Posted by Steven Price 1 month, 2 weeks ago
On 14/08/2025 23:58, Liviu Dudau wrote:
> On Fri, Aug 08, 2025 at 11:50:27AM +0100, Daniel Stone wrote:
>> Hi Karunika,
>>
>>
>> On Thu, 7 Aug 2025 at 17:27, Karunika Choo <karunika.choo@arm.com> wrote:
>>> @@ -585,6 +615,9 @@ static int mmu_hw_do_operation_locked(struct panthor_device *ptdev, int as_nr,
>>>         if (op != AS_COMMAND_UNLOCK)
>>>                 lock_region(ptdev, as_nr, iova, size);
>>>
>>> +       if (op == AS_COMMAND_FLUSH_MEM || op == AS_COMMAND_FLUSH_PT)
>>> +               return mmu_hw_do_flush_on_gpu_ctrl(ptdev, as_nr, op);
>>
>> Given that FLUSH_MEM and FLUSH_PT are the only ops which are ever
>> used, the below becomes dead code. Could you please just inline these,
>> so it's more clear what's actually going on? The (op !=
>> AS_COMMAND_UNLOCK) branch can also become unconditional, perhaps with
>> a WARN_ON() around unknown ops.
> 
> Hmm, the commit message says that FLUSH_MEM and FLUSH_PT are going to be
> deprecated and replaced with FLUSH_CACHES so the first are clearly not the
> only ones ever used (at least not in the future). I'm not sure why you
> think this code is not correct.

The code is "correct" (it does the right thing), but Daniel is also
correct that as things stand the code below that "return" is all dead.
The only code paths call with op either AS_COMMAND_FLUSH_MEM or
AS_COMMAND_FLUSH_PT.

But I think this patch set has been through enough revisions already,
and we've now got the firmware upstream[1]. So rather than do another
wrong, I'm going to merge this to drm-misc-next and I'll post a separate
patch cleaning up the dead code.

Thanks,
Steve

[1]
https://gitlab.com/kernel-firmware/linux-firmware/-/commit/930ef9046e3848df688d98b7d5e68154b031dc66

> Best regards,
> Liviu
> 
>>
>> Cheers,
>> Daniel
>