Re: [PATCH 1/2] drm/{i915,xe}: Move intel_pch under display

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Feb 18, 2025 at 09:45:46AM -0500, Rodrigo Vivi wrote:
> On Tue, Feb 18, 2025 at 02:19:38PM +0200, Jani Nikula wrote:
> > On Mon, 17 Feb 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > The only usage of the "PCH" infra is to detect which South Display
> > > Engine we should be using. Move it under display so we can convert
> > > all its callers towards intel_display struct later.
> > 
> > Yeah, PCH is becoming a blocker to finishing the conversions of many
> > files from drm_i915_private to intel_display. We'll need to do something
> > like this.
> 
> First of all, I'm sorry for not sending a cover letter explaining more about
> my thoughts here and also for not tagging this as an RFC. And thank you very
> much for jumping in the discussion.
> 
> I started this, exactly because my consolidation of display pm now is
> blocked in some HPD calls. Then I checked the IRQ code and I have some
> ideas do organize that, but that got blocked on the PCH, then I moved
> towards seeing what could and should be done to PCH and these 2 patches
> is the initial of my conclusion.
> 
> > 
> > I was eyeing the PCH checks outside of display, and frankly many of them
> > are plain wrong (done to check stuff that's unrelated to PCH, but
> > happens to match desired platforms), or should be in display
> > (e.g. gt_record_display_regs()). But there are also legitimate checks, I
> > think in clock gating.
> 
> Yes, this one in GPU hang was one of the most strange ones, but then
> I noticed it is inside this function that should be moved to the display
> side anyway.
> 
> Other cases are:
> drivers/gpu/drm/i915/intel_clock_gating.c:
> 
> This entire file should be in the display side.

Mostly, but it still has bunch of gt workarounds in it.
Those all should be moved into the gt w/a framework.

> But I got super
> scared now because it looks like it is not getting called from nowhere.
> So we might be missing many display workarounds. I will investigate
> this more later.

It's hidden behind some confusing macro stuff.

> 
> drivers/gpu/drm/i915/i915_irq.c:
> This is exactly where I got blocked. All the PCH calls inside it
> are display related that I need to move to the display side in
> order to have a proper split and make the display to stop using
> the irq spin locks directly.
> 
> drivers/gpu/drm/i915/i915_gpu_error.c:
> good idea on moving that entire function to display anyway.

That seems like a random set of stuff no one actually cares about.
Should just nuke the whole thing, apart from DERRMR because that
one is actually relevant for GPU hangs. Though that one doesn't
exist on VLV/CHV so currently some random garbage is being read
into it on those platforms.

-- 
Ville Syrjälä
Intel