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

[Lucas De Marchi]

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

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