On 24/10/2025 07:43, Boris Brezillon wrote: > On Tue, 14 Oct 2025 10:43:30 +0100 > Karunika Choo <[email protected]> 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. >
I see your point — the intent here is mainly to have the feature flag reflect hardware-level changes. In this series, for example, it corresponds to the addition of the new PWR_CONTROL block. Another use case would be arch v11, where a new PRFCNT_FEATURES register was introduced. In that case, we might want to adjust the counters_per_block [1] value depending on that register’s value. I would also expect this mechanism to remain useful for future hardware revisions, as it provides a clean way to describe architectural differences without scattering version-specific checks throughout the code, while still being lighter-weight than function pointers. [1] https://lore.kernel.org/dri-devel/[email protected]/ Kind regards, Karunika Choo >> >> Signed-off-by: Karunika Choo <[email protected]> >> --- >> 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__ */ >
