Re: [Intel-gfx] â Fi.CI.BAT: failure for drm/i915/display: Check GGTT to determine phys_base (rev2)

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

On Fri, Dec 08, 2023 at 09:32:00AM +0100, Andrzej Hajda wrote:
> On 07.12.2023 12:26, Andrzej Hajda wrote:
> > On 07.12.2023 11:10, Andrzej Hajda wrote:
> > 
> > After taking a look on panic log [1], I have found:
> > [drm:i915_init_ggtt [i915]] Failed to reserve top of GGTT for GuC
> > 
> > I don't know why it is only debug level? It seems serious failure, as a 
> > result i915_init_ggtt fails and probe fails.
> > 
> > The cause is that initial framebuffer is located at the end of GGTT and 
> > it overlaps with reserved area (see ggtt_reserve_guc_top).
> > 
> > I am not sure how it can be properly fixed, I guess dirty fix could be
> > just relocation of vma (hopefully into free area), sth like:
> > new_gte = gsm + (ggtt->vm.total - GUC_TOP_RESERVE_SIZE - size) / 
> > I915_GTT_PAGE_SIZE;
> > memmove(new_gte, gte, size / I915_GTT_PAGE_SIZE);
> > 
> > but I have no idea of possible side effects :)
> 
> I looked once more into the code and maybe you can just pin the buffer 
> to earlier address (not overlapping with GuC reservation and current vma 
> of the fb):
> @@ -143,6 +143,9 @@ initial_plane_vma(struct drm_i915_private *i915,
>          if (IS_ERR(vma))
>                  goto err_obj;
> 
> +       if (base + size > GUC_GGTT_TOP)
> +               base = min(base, GUC_GGTT_TOP) - size;
> +
>          pinctl = PIN_GLOBAL | PIN_OFFSET_FIXED | base;
>          if (HAS_GMCH(i915))
>                  pinctl |= PIN_MAPPABLE;

This is not a solution. The correct fix is either:
1. fix the guc code to not insist on a fixed chunk of the ggtt
   and instead allocate it normally like a good boy. It could
   still try to allocate as high as possible to ideally
   land in the GUC_GGTT_TOP range
2. relocate the display vma to a different part of the ggtt

1. should be far simpler by the looks of it, as 2. would involve:
a) pinning the same object into two places in the ggtt simultanously
   I don't think we have support for that except for special ggtt views,
   and xe doesn't have even that (should we need this trick there as well)
b) flip the plane(s) over to the relocated vma
c) wait for vblank(s)
d) discard the original vma
e) all of that would need to happen pretty early so we may not have
   a lot of the required normal machinery fully up and running yet

Anyways, I ran into much of the same issues when debugging an MTL
machine. This is the result of my efforts (partially overlaps with
the proposed patch here):
https://patchwork.freedesktop.org/series/127721/

-- 
Ville Syrjälä
Intel