On Tue, Feb 18, 2025 at 05:27:41PM +0200, Ville Syrjälä wrote:
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@xxxxxxxxx> 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.
yep. See CG_FUNCS in drivers/gpu/drm/i915/intel_clock_gating.c
The callers are the ones calling intel_clock_gating_init(), which is
both on display and gem sides. On the GEM side there's already and
eternal TODO comment:
* FIXME: break up the workarounds and apply them at the right time!
Lucas De Marchi
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
