Re: [PATCH v3] drm/i915:move and rename reg_in_range_table

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Oct 10, 2025 at 12:55:02PM +0300, Jani Nikula wrote:
> On Thu, 09 Oct 2025, Matt Atwood <matthew.s.atwood@intel.com> wrote:
> > reg_in_range_table is a useful function that is used in multiple places,
> > and will be needed for WA_BB implementation later.
> >
> > Let's move this function and i915_range struct to its own file, as we are
> > trying to move away from i915_utils files.
> >
> > v2: move functions to their own file
> > v3: use correct naming convention
> 
> Okay, Message from the Department of Bikeshedding and Nitpicking.
> 
> There's really nothing mmio specific about the functionality being
> abstracted. You have a range represented by two u32's in a struct, and a
> function to check if another u32 is within that range.
> 
> The struct could just as well remain i915_range, the files could be
> i915_range.[ch], and the function could be, say,
> i915_range_table_contains(). IMO "mmio" makes it unnecessarily specific.

hmm, I'm really sorry about that... That is my bad. I'm so bad with naming.

I suggested mmio in the name because i915_range is way to generic.
The other extreme side.

Perhaps i915_addr_range ?

But I would be okay if the consensus is simply i915_range.

> 
> > +bool i915_mmio_range_table_contains(u32 addr, const struct i915_mmio_range *table)
> 
> Usually, the "context" parameter goes first. I get that this wasn't the
> case before, but I'd use the opportunity to swap these around.

I just had the same feeling while reading this patch again.
Specially because it phrases like table contain ... table first contain last...

Sorry for not noticing it before as well.

But I was on the fence on this one because it was already like that addr,range
and the other range infra that we consider also uses the style addr,range.

> 
> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel