On Thu, 09 Oct 2025, Rodrigo Vivi <[email protected]> wrote: > 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.
Agreed. I'll note that there's also include/linux/range.h, which could be expanded to our use case, but it deals with u64 offsets. BR, Jani. -- Jani Nikula, Intel
