Re: [PATCH v1 03/10] drm/panthor: Introduce framework for architecture-specific features

[Boris Brezillon <boris.brezillon@collabora.com>]

On Tue, 14 Oct 2025 10:43:30 +0100
Karunika Choo <karunika.choo@arm.com> wrote:

> Add a framework to support architecture-specific features. This allows
> other parts of the driver to adjust their behaviour based on the feature
> bits enabled for a given architecture.

I'm not convinced we need this just yet. AFAICT, the only feature flag
being added in this patchset is PANTHOR_HW_FEATURE_PWR_CONTROL, and
most of this is abstracted away with function pointers already. The
only part that tests this FEATURE_PWR_CONTROL flag is the
initialization, which could very much be abstracted away with a
function pointer (NULL meaning no PWR block present). There might be
other use cases you're planning to use this for, so I'd like to hear
about them to make my final opinion on that.

> 
> Signed-off-by: Karunika Choo <karunika.choo@arm.com>
> ---
>  drivers/gpu/drm/panthor/panthor_hw.c |  5 +++++
>  drivers/gpu/drm/panthor/panthor_hw.h | 18 ++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/panthor/panthor_hw.c b/drivers/gpu/drm/panthor/panthor_hw.c
> index b6e7401327c3..34536526384d 100644
> --- a/drivers/gpu/drm/panthor/panthor_hw.c
> +++ b/drivers/gpu/drm/panthor/panthor_hw.c
> @@ -186,3 +186,8 @@ int panthor_hw_init(struct panthor_device *ptdev)
>  
>  	return 0;
>  }
> +
> +bool panthor_hw_has_feature(struct panthor_device *ptdev, enum panthor_hw_feature feature)
> +{
> +	return test_bit(feature, ptdev->hw->features);
> +}
> diff --git a/drivers/gpu/drm/panthor/panthor_hw.h b/drivers/gpu/drm/panthor/panthor_hw.h
> index 39752de3e7ad..7a191e76aeec 100644
> --- a/drivers/gpu/drm/panthor/panthor_hw.h
> +++ b/drivers/gpu/drm/panthor/panthor_hw.h
> @@ -4,14 +4,32 @@
>  #ifndef __PANTHOR_HW_H__
>  #define __PANTHOR_HW_H__
>  
> +#include <linux/types.h>
> +
>  struct panthor_device;
>  
> +/**
> + * enum panthor_hw_feature - Bit position of each HW feature
> + *
> + * Used to define GPU specific features based on the GPU architecture ID.
> + * New feature flags will be added with support for newer GPU architectures.
> + */
> +enum panthor_hw_feature {
> +	/** @PANTHOR_HW_FEATURES_END: Must be last. */
> +	PANTHOR_HW_FEATURES_END
> +};
> +
> +
>  /**
>   * struct panthor_hw - GPU specific register mapping and functions
>   */
>  struct panthor_hw {
> +	/** @features: Bitmap containing panthor_hw_feature */
> +	DECLARE_BITMAP(features, PANTHOR_HW_FEATURES_END);
>  };
>  
>  int panthor_hw_init(struct panthor_device *ptdev);
>  
> +bool panthor_hw_has_feature(struct panthor_device *ptdev, enum panthor_hw_feature feature);
> +
>  #endif /* __PANTHOR_HW_H__ */