On Thu, Oct 09, 2025 at 03:08:28PM +0200, Andi Shyti wrote:
> On Wed, Oct 08, 2025 at 05:34:39PM -0400, Rodrigo Vivi wrote:
> > On Wed, Oct 08, 2025 at 10:37:13AM -0700, Matt Roper wrote:
> > > On Wed, Oct 08, 2025 at 10:22:42AM -0700, Matt Atwood wrote:
> > > > On Wed, Oct 08, 2025 at 09:53:34AM -0700, Matt Roper wrote:
> > > > > On Tue, Oct 07, 2025 at 02:23:36PM -0700, Matt Atwood 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.
> > > > > 
> > > > > It looks like this is a new revision of this patch from a couple years
> > > > > ago, right?
> > > > > 
> > > > >         
> > > > > https://lore.kernel.org/all/[email protected]/
> > > > > 
> > > > > Even though it's been a long time, it would still be a good idea to
> > > > > include a patch changelog so that it's clear what's been changed and
> > > > > what review feedback was/wasn't incorporated.
> > > > Sorry, I will include it if theres another version
> > > > > 
> > > > > I'm also wondering if we should be thinking about moving i915 to use
> > > > > 'struct regmap_range' and existing functions like 
> > > > > regmap_reg_in_ranges()
> > > > > rather than maintaining our own i915-specific versions of the logic.
> > > > > regmap in general does a bunch of other stuff that isn't relevant to
> > > > > i915, but it seems like we might be able to re-use the type 
> > > > > definitions
> > > > > and basic lookups to avoid reinventing the wheel.
> > > > This is doable but just requires a rewrite of the current implementation
> > > > as it's not a 1:1 conversion.
> > > 
> > > The idea is that we'd eliminate 'struct i915_range' and related
> > > functions and just use the regmap types and functions instead.  It looks
> > > like the main difference is that the regmap lists are size-based, while
> > > our lists use a sentinel to mark the end of the table.
> > > 
> > > Although I did just notice that even the basic types and helpers for
> > > regmap rely on CONFIG_REGMAP, so that might be an argument against
> > > switching over since we'd need to add an extra kconfig dependency, and
> > > most of what it brings in isn't useful to us.  But probably more
> > > something for Rodrigo and the other maintainers to weigh in on.
> > 
> > Cc: all other maintainers.
> > 
> > I could easily be convinced either way.
> > 
> > I like the idea of reusing something existing and this helper and struct
> > does fit to our needs.
> > I don't mind having to include another config dependency here.
> > The part that is not good is to bring a lot more than we need :/
> > 
> > Perhaps the really right thing to do there would be to split regmap
> > into a generic map part and the support to the other different bus stuff.
> > Then we start using the generic part.
> 
> It's true that they are similar (regmap_reg_in_ranges() is
> basically a copy paste), but regmap and mmio are two different
> things (although conceptually similar in some cases). Working to
> expose regmap_range so that we can use it as mmio_range looks to
> me a bit of an overkill.

fair enough. Let's go then with this i915-only approach here, but
renaming the functions and structs.

> 
> Andi

Reply via email to