On Tue, 24 Sep 2024, Lucas De Marchi <lucas.demarchi@xxxxxxxxx> wrote: > On Tue, Sep 24, 2024 at 12:49:25PM GMT, Jani Nikula wrote: >>On Thu, 29 Aug 2024, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: >>> On Wed, Aug 28, 2024 at 04:41:24PM -0400, Rodrigo Vivi wrote: >>>> On Mon, Aug 19, 2024 at 09:44:27PM +0300, Jani Nikula wrote: >>>> > v2 of [1]. Please read the cover letter there. >>>> > >>>> > This addresses review comments and adds a few more commits on top, in particular >>>> > the last one showcasing the approach. >>>> > >>>> > The main question remains, is this what we want? >>>> >>>> I don't know why, but the 'is' thing is still strange. >>>> >>>> I know I know... I'm bad with naming myself. >>>> >>>> I think about 'platform' but that get too big >>>> >>>> if (display->platform.BROADWELL) >>>> >>>> I think about 'gen' but then it is overloaded.... >>>> >>>> then I think about 'ip' is worse... >>>> >>>> 'version'? >>>> >>>> 'name'? >>>> >>>> if (display->name.HASWELL)... >>>> >>>> .... >>>> >>>> But well, I like the overall simplification here in general. >>>> Without a better name to suggest, I guess let's just move ahead... >>> >>> One slight concern with the is.foo is whether it complicates finding >>> things with eg. cscope. But I suppose for platforms that doesn't matter >>> all that much. For the has_foo stuff it'd be much more relevant. >> >>It does make finding things harder with cscope and gnu global, but git >>grep for is.FOO is pretty accurate. >> >>> Anyways, can't think of anything particularly elegant myself either, >>> so go ahead I guess. >> >>So I haven't yet. I just still have that slightly uneasy feeling about >>whether this is a good thing or not. That doesn't usually make me shy >>away from things, because you can fix stuff later, but getting this >>wrong causes so much churn everywhere. >> >>The fact that it's not a macro makes it less flexible for future >>changes. The display->is.FOO is somewhat legible, but could be >>better. Would all lowercase make it better? I don't know. >> >>More alternatives? Not elegant for sure, but just alternatives: >> >>- Lowercase names: >> >> if (display->is.rocketlake) > > what I really dislike is a struct named "is". Going full mesa-way would > be slightly better IMO: > > if (display->is_rockelake) > > or > > if (display->platform_rocketlake) > > or > > if (display->platform.rocketlake) Fair enough.
