On Wed, 12 Nov 2025, Jani Nikula <[email protected]> wrote: > On Tue, 11 Nov 2025, Ville Syrjälä <[email protected]> wrote: >> On Tue, Nov 11, 2025 at 09:34:04AM +0200, Jani Nikula wrote: >>> Add an irq parent driver interface for the .enabled and .synchronize >>> calls. This lets us drop the dependency on i915_drv.h and i915_irq.h in >>> multiple places, and subsequently remove the compat i915_irq.h and >>> i915_irq.c files along with the display/ext directory from xe >>> altogether. >>> >>> Use intel_irqs_enabled() and intel_synchronize_irq() static wrappers for >>> parent interface calls in intel_display_irq.c while chasing the function >>> pointers everywhere else. It's still a bit uncertain if we should >>> universally have wrappers for the parent interface calls or not. >> >> Why would we want those ugly pointer chains anywhere (except perhaps >> for single use cases)? > > I guess my main point is, I'm not yet sure how those wrappers should be > organized, and I'm simply postponing the decision. But let's discuss. > > I'm leaning towards putting them in files that are separate from the > implementation, i.e. intel_irqs_enabled() and intel_synchronize_irq() > would *not* be in intel_display_irq.[ch] but rather in > something_parent_something.[ch]. Because it's not so much about the > functionality, it's all about calling the parent interface. > > Maybe just one file grouping *all* parent interface access, similar to > how include/drm/intel/display_parent_interface.h defines the interface. > > And then should the naming indicate it's calling the parent? I think > there's value in knowing where the call ends up when reading the > code. But then do the function names end up being unweildy? > > If you simply translate display->parent->irq->enabled to a function > name, it would be display_parent_irq_enabled(). Or perhaps > intel_parent_irq_enabled(). And you'd have the naming scheme right > there, always 1:1 without further thinking. > > intel_display_rpm.[ch] has it all in a dedicated file now. But there's > really nothing runtime pm specific there anymore, it's just mechanical > calling of the interface and relaying parameters and return values. > > I think we have too much "intel_display_something_something.[ch]" so > maybe intel_parent.[ch] and intel_parent_<SUBSTRUCT>_<FUNCTION>()?
Typed it up, I like it, sent as v3. BR, Jani. -- Jani Nikula, Intel
