On Tue, Jul 09, 2024 at 03:28:15PM -0500, Lucas De Marchi wrote:
> On Fri, Jul 05, 2024 at 05:52:50PM GMT, Ville Syrjälä wrote:
> >From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> >Consolidate the "should we allocate fbdev fb in stolen?"
> >check into a helper function. Makes it easier to change the
> >heuristics without having to change so many places.
> >
> >Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >---
> > drivers/gpu/drm/i915/display/intel_fbdev_fb.c | 24 ++++++++++++-------
> > drivers/gpu/drm/i915/display/intel_fbdev_fb.h | 5 +++-
> > .../drm/i915/display/intel_plane_initial.c | 10 +++-----
> > 3 files changed, 23 insertions(+), 16 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/display/intel_fbdev_fb.c b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> >index 497525ef9668..0a6445acb100 100644
> >--- a/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> >+++ b/drivers/gpu/drm/i915/display/intel_fbdev_fb.c
> >@@ -11,6 +11,19 @@
> > #include "intel_display_types.h"
> > #include "intel_fbdev_fb.h"
> >
> >+bool intel_fbdev_fb_prefer_stolen(struct intel_display *display,
> >+ unsigned int size)
> >+{
> >+ struct drm_i915_private *i915 = to_i915(display->drm);
> >+
> >+ /*
> >+ * If the FB is too big, just don't use it since fbdev is not very
> >+ * important and we should probably use that space with FBC or other
> >+ * features.
> >+ */
> >+ return i915->dsm.usable_size >= size * 2;
> >+}
> >+
> > struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
> > struct drm_fb_helper_surface_size *sizes)
> > {
> >@@ -42,14 +55,9 @@ struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
> > I915_BO_ALLOC_CONTIGUOUS |
> > I915_BO_ALLOC_USER);
> > } else {
> >- /*
> >- * If the FB is too big, just don't use it since fbdev is not very
> >- * important and we should probably use that space with FBC or other
> >- * features.
> >- *
> >- * Also skip stolen on MTL as Wa_22018444074 mitigation.
> >- */
> >- if (!(IS_METEORLAKE(dev_priv)) && size * 2 < dev_priv->dsm.usable_size)
> >+ /* skip stolen on MTL as Wa_22018444074 mitigation */
> >+ if (!IS_METEORLAKE(dev_priv) &&
>
> shouldn't this be inside intel_fbdev_fb_prefer_stolen()?
That would also apply it to the BIOS fb takeover, so change the
behaviour. The correct answer is likely just removing the MTL check
entirely, but I left that out for now to avoid too many functional
changes.
>
> And also pull the same logic on the xe side a few patches after this.
>
> Lucas De Marchi
>
> >+ intel_fbdev_fb_prefer_stolen(&dev_priv->display, size))
> > obj = i915_gem_object_create_stolen(dev_priv, size);
> > if (IS_ERR(obj))
> > obj = i915_gem_object_create_shmem(dev_priv, size);
> >diff --git a/drivers/gpu/drm/i915/display/intel_fbdev_fb.h b/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> >index 4832fe688fbf..3b9033bd2160 100644
> >--- a/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> >+++ b/drivers/gpu/drm/i915/display/intel_fbdev_fb.h
> >@@ -6,16 +6,19 @@
> > #ifndef __INTEL_FBDEV_FB_H__
> > #define __INTEL_FBDEV_FB_H__
> >
> >+#include <linux/types.h>
> >+
> > struct drm_fb_helper;
> > struct drm_fb_helper_surface_size;
> > struct drm_i915_gem_object;
> > struct drm_i915_private;
> > struct fb_info;
> > struct i915_vma;
> >+struct intel_display;
> >
> > struct intel_framebuffer *intel_fbdev_fb_alloc(struct drm_fb_helper *helper,
> > struct drm_fb_helper_surface_size *sizes);
> > int intel_fbdev_fb_fill_info(struct drm_i915_private *i915, struct fb_info *info,
> > struct drm_i915_gem_object *obj, struct i915_vma *vma);
> >-
> >+bool intel_fbdev_fb_prefer_stolen(struct intel_display *display, unsigned int size);
> > #endif
> >diff --git a/drivers/gpu/drm/i915/display/intel_plane_initial.c b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> >index ada1792df5b3..4622bb5f3426 100644
> >--- a/drivers/gpu/drm/i915/display/intel_plane_initial.c
> >+++ b/drivers/gpu/drm/i915/display/intel_plane_initial.c
> >@@ -11,6 +11,7 @@
> > #include "intel_display.h"
> > #include "intel_display_types.h"
> > #include "intel_fb.h"
> >+#include "intel_fbdev_fb.h"
> > #include "intel_frontbuffer.h"
> > #include "intel_plane_initial.h"
> >
> >@@ -160,15 +161,10 @@ initial_plane_vma(struct drm_i915_private *i915,
> > mem->min_page_size);
> > size -= base;
> >
> >- /*
> >- * If the FB is too big, just don't use it since fbdev is not very
> >- * important and we should probably use that space with FBC or other
> >- * features.
> >- */
> > if (IS_ENABLED(CONFIG_FRAMEBUFFER_CONSOLE) &&
> > mem == i915->mm.stolen_region &&
> >- size * 2 > i915->dsm.usable_size) {
> >- drm_dbg_kms(&i915->drm, "Initial FB size exceeds half of stolen, discarding\n");
> >+ !intel_fbdev_fb_prefer_stolen(&i915->display, size)) {
> >+ drm_dbg_kms(&i915->drm, "Initial FB size uses too much stolen, discarding\n");
> > return NULL;
> > }
> >
> >--
> >2.44.2
> >
--
Ville Syrjälä
Intel
