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. Andi
