Re: [PATCH v3 15/16] drm/i915: Try to relocate the BIOS fb to the start of ggtt

From: Paz Zcharya <pazz@chromium.org>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, Andrzej Hajda <andrzej.hajda@intel.com>
Subject: Re: [PATCH v3 15/16] drm/i915: Try to relocate the BIOS fb to the start of ggtt
Date: Tue, 30 Jan 2024 23:27:24 +0000	[thread overview]
Message-ID: <ZbmF3IeyAyb-QERO@google.com> (raw)
In-Reply-To: <20240116075636.6121-16-ville.syrjala@linux.intel.com>

On Tue, Jan 16, 2024 at 09:56:35AM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> On MTL the GOP (for whatever reason) likes to bind its framebuffer
> high up in the ggtt address space. This can conflict with whatever
> ggtt_reserve_guc_top() is trying to do, and the result is that
> ggtt_reserve_guc_top() fails and then we proceed to explode when
> trying to tear down the driver. Thus far I haven't analyzed what
> causes the actual fireworks, but it's not super important as even
> if it didn't explode we'd still fail the driver load and the user
> would be left with an unusable GPU.
> 
> To remedy this (without having to figure out exactly what
> ggtt_reserve_guc_top() is trying to achieve) we can attempt to
> relocate the BIOS framebuffer to a lower ggtt address. We can do
> this at this early point in driver init because nothing else is
> supposed to be clobbering the ggtt yet. So we simply change where
> in the ggtt we pin the vma, the original PTEs will be left as is,
> and the new PTEs will get written with the same dma addresses.
> The plane will keep on scanning out from the original PTEs until
> we are done with the whole process, and at that point we rewrite
> the plane's surface address register to point at the new ggtt
> address.
> 
> Since we don't need a specific ggtt address for the plane
> (apart from needing it to land in the mappable region for
> normal stolen objects) we'll just try to pin it without a fixed
> offset first. It should end up at the lowest available address
> (which really should be 0 at this point in the driver init).
> If that fails we'll fall back to just pinning it exactly to the
> origianal address.
> 
> To make sure we don't accidentlally pin it partially over the
> original ggtt range (as that would corrupt the original PTEs)
> we reserve the original range temporarily during this process.
> 
> v2: Try to pin explicitly to ggtt offset 0 as otherwise DG2 puts it
>     even higher (atm we have no PIN_LOW flag to force it low)
> 
> Cc: Paz Zcharya <pazz@chromium.org>
> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Hi Ville,

Thank you so much for this incredible series.
It solves the issue regarding MTL initial plane readout
that Andrzej Hajda and I worked on in
https://patchwork.freedesktop.org/patch/570811/?series=127130&rev=2
In addition, it solved the issue with the new GOP.

I tested it on two different devices with Meteor Lake and it worked perfectly:
no i915 errors, no flickers or observable issues.

Tested-by: Paz Zcharya <pazz@chromium.org>