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

[Matt Atwood <matthew.s.atwood@intel.com>]

On Fri, Oct 10, 2025 at 09:19:24AM -0400, Rodrigo Vivi wrote:
> 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.
I think this would best fwiw.
> 
> 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...
Ack.
> 
> 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