Re: [PATCH v2 6/8] drm/panthor: Support GLB_REQ.STATE field for Mali-G1 GPUs

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

On Mon, 27 Oct 2025 10:43:42 +0000
Karunika Choo <karunika.choo@arm.com> wrote:

> On 26/10/2025 08:27, Boris Brezillon wrote:
> > On Fri, 24 Oct 2025 21:21:15 +0100
> > Karunika Choo <karunika.choo@arm.com> wrote:
> >   
> >> +static bool panthor_fw_has_glb_state(struct panthor_device *ptdev)
> >> +{
> >> +	struct panthor_fw_global_iface *glb_iface = panthor_fw_get_glb_iface(ptdev);
> >> +
> >> +	return glb_iface->control->version >= CSF_IFACE_VERSION(4, 1, 0);
> >> +}  
> > 
> > It's okay for now, but if we start having more of these, we probably
> > want to automate the generation of those has_<featurex>() helpers with
> > something like:
> > 
> > #define FW_FEATURE(feat_name, major, minor)						\
> > static bool panthor_fw_has_ ## feat_name(struct panthor_device *ptdev) 			\
> > {											\
> > 	struct panthor_fw_global_iface *glb_iface = panthor_fw_get_glb_iface(ptdev); 	\
> > 											\
> > 	return glb_iface->control->version >= CSF_IFACE_VERSION(major, minor, 0);	\
> > }
> > 
> > Same goes for the HW features BTW.
> >   
> 
> I wonder if at that point, would a bitmask as previously proposed be a
> cleaner solution? I have a minor bone to pick with MACROs that generate
> functions as they make finding its definition unnecessarily complicated
> and obtuse. Not to mention if the conditions for a feature changes and
> is disjoint from all the others, updating the macro to handle these
> would cause additional churn that I would really hope to avoid.

Let's do without the macros for now.