On Fri, 10 Oct 2025 15:22:50 +0100 Steven Price <[email protected]> wrote:
> On 10/10/2025 11:11, Boris Brezillon wrote: > > If we want to be able to skip CPU cache maintenance operations on > > CPU-cached mappings, the UMD needs to know the kind of coherency > > in place. Add a field to drm_panthor_gpu_info to do that. We can re-use > > a padding field for that since this object is write-only from the > > KMD perspective, and the UMD should just ignore it. > > > > v2: > > - New commit > > > > Signed-off-by: Boris Brezillon <[email protected]> > > --- > > drivers/gpu/drm/panthor/panthor_device.c | 6 +++- > > drivers/gpu/drm/panthor/panthor_gpu.c | 2 +- > > include/uapi/drm/panthor_drm.h | 39 ++++++++++++++++++++++-- > > 3 files changed, 42 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/panthor/panthor_device.c > > b/drivers/gpu/drm/panthor/panthor_device.c > > index c7033d82cef5..afe24a03a71c 100644 > > --- a/drivers/gpu/drm/panthor/panthor_device.c > > +++ b/drivers/gpu/drm/panthor/panthor_device.c > > @@ -25,6 +25,8 @@ > > > > static int panthor_gpu_coherency_init(struct panthor_device *ptdev) > > { > > + /* Start with no coherency, and update it if the device is flagged > > coherent. */ > > + ptdev->gpu_info.selected_coherency = GPU_COHERENCY_NONE; > > ptdev->coherent = device_get_dma_attr(ptdev->base.dev) == > > DEV_DMA_COHERENT; > > > > if (!ptdev->coherent) > > @@ -34,8 +36,10 @@ static int panthor_gpu_coherency_init(struct > > panthor_device *ptdev) > > * ACE protocol has never been supported for command stream frontend > > GPUs. > > */ > > if ((gpu_read(ptdev, GPU_COHERENCY_FEATURES) & > > - GPU_COHERENCY_PROT_BIT(ACE_LITE))) > > + GPU_COHERENCY_PROT_BIT(ACE_LITE))) { > > + ptdev->gpu_info.selected_coherency = > > GPU_COHERENCY_PROT_BIT(ACE_LITE); > > return 0; > > + } > > > > drm_err(&ptdev->base, "Coherency not supported by the device"); > > return -ENOTSUPP; > > diff --git a/drivers/gpu/drm/panthor/panthor_gpu.c > > b/drivers/gpu/drm/panthor/panthor_gpu.c > > index 9d98720ce03f..a95c0b94ef58 100644 > > --- a/drivers/gpu/drm/panthor/panthor_gpu.c > > +++ b/drivers/gpu/drm/panthor/panthor_gpu.c > > @@ -49,7 +49,7 @@ struct panthor_gpu { > > static void panthor_gpu_coherency_set(struct panthor_device *ptdev) > > { > > gpu_write(ptdev, GPU_COHERENCY_PROTOCOL, > > - ptdev->coherent ? GPU_COHERENCY_PROT_BIT(ACE_LITE) : > > GPU_COHERENCY_NONE); > > + ptdev->gpu_info.selected_coherency); > > It looks like an existing bug, but GPU_COHERENCY_PROTOCOL doesn't take a > bit mask. So we should be just writing GPU_COHERENCY_ACE_LITE not > GPU_COHERENCY_PROT_BIT(ACE_LITE). Oops. Should I prepare a fix, or does someone at Arm intend to send a fix for this one? > > > } > > > > static void panthor_gpu_l2_config_set(struct panthor_device *ptdev) > > diff --git a/include/uapi/drm/panthor_drm.h b/include/uapi/drm/panthor_drm.h > > index 467d365ed7ba..b9e483ff5e21 100644 > > --- a/include/uapi/drm/panthor_drm.h > > +++ b/include/uapi/drm/panthor_drm.h > > @@ -245,6 +245,26 @@ enum drm_panthor_dev_query_type { > > DRM_PANTHOR_DEV_QUERY_GROUP_PRIORITIES_INFO, > > }; > > > > +/** > > + * enum drm_panthor_gpu_coherency: Type of GPU coherency > > + */ > > +enum drm_panthor_gpu_coherency { > > + /** > > + * @DRM_PANTHOR_GPU_COHERENCY_ACE_LITE: ACE Lite coherency. > > + */ > > + DRM_PANTHOR_GPU_COHERENCY_ACE_LITE = 1 << 0, > > + > > + /** > > + * @DRM_PANTHOR_GPU_COHERENCY_ACE_LITE: ACE coherency. > > Copy/paste mistake ^^^^^ Will fix. > > > + */ > > + DRM_PANTHOR_GPU_COHERENCY_ACE = 1 << 1, > > + > > + /** > > + * @DRM_PANTHOR_GPU_COHERENCY_NONE: No coherency. > > + */ > > + DRM_PANTHOR_GPU_COHERENCY_NONE = 31, > > +}; > > This is a mix of bit mask and non-bit mask. I'm assuming this was > intended for the newly added selected_coherency field, in which case it > shouldn't be shifting - the values are 0 and 1 for ace_lite and ace. Yeah, I think I went back and forth on this, and just picked the worst option in the end. I'll make it a real enum with the mapping you suggested.
