[Intel-gfx] [PATCH 13/20] drm/i915/fbc: Allocate intel_fbc dynamically

Ville Syrjala ville.syrjala at linux.intel.com
Wed Nov 24 11:36:45 UTC 2021


From: Ville Syrjälä <ville.syrjala at linux.intel.com>

In the future we may have more than one FBC instance on some
platforms. So let's just allocate it dynamically. This also
lets us fully hide the implementation from prying eyes.

Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
---
 drivers/gpu/drm/i915/display/i9xx_plane.c     |   2 +-
 drivers/gpu/drm/i915/display/intel_fbc.c      | 154 +++++++++++++-----
 .../drm/i915/display/skl_universal_plane.c    |   2 +-
 drivers/gpu/drm/i915/i915_drv.h               |  59 +------
 4 files changed, 116 insertions(+), 101 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/i9xx_plane.c b/drivers/gpu/drm/i915/display/i9xx_plane.c
index 84f50c90728f..85950ff67609 100644
--- a/drivers/gpu/drm/i915/display/i9xx_plane.c
+++ b/drivers/gpu/drm/i915/display/i9xx_plane.c
@@ -125,7 +125,7 @@ static struct intel_fbc *i9xx_plane_fbc(struct drm_i915_private *dev_priv,
 					enum i9xx_plane_id i9xx_plane)
 {
 	if (i9xx_plane_has_fbc(dev_priv, i9xx_plane))
-		return &dev_priv->fbc;
+		return dev_priv->fbc;
 	else
 		return NULL;
 }
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index 9be8e7dcaab6..1daf4f7b5d80 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -59,6 +59,63 @@ struct intel_fbc_funcs {
 	void (*set_false_color)(struct intel_fbc *fbc, bool enable);
 };
 
+struct intel_fbc_state {
+	const char *no_fbc_reason;
+	enum i9xx_plane_id i9xx_plane;
+	unsigned int cfb_stride;
+	unsigned int cfb_size;
+	unsigned int fence_y_offset;
+	u16 override_cfb_stride;
+	u16 interval;
+	s8 fence_id;
+};
+
+struct intel_fbc {
+	struct drm_i915_private *i915;
+	const struct intel_fbc_funcs *funcs;
+
+	/*
+	 * This is always the inner lock when overlapping with
+	 * struct_mutex and it's the outer lock when overlapping
+	 * with stolen_lock.
+	 */
+	struct mutex lock;
+	unsigned int possible_framebuffer_bits;
+	unsigned int busy_bits;
+	struct intel_plane *plane;
+
+	struct drm_mm_node compressed_fb;
+	struct drm_mm_node compressed_llb;
+
+	u8 limit;
+
+	bool false_color;
+
+	bool active;
+	bool activated;
+	bool flip_pending;
+
+	bool underrun_detected;
+	struct work_struct underrun_work;
+
+	/*
+	 * Due to the atomic rules we can't access some structures without the
+	 * appropriate locking, so we cache information here in order to avoid
+	 * these problems.
+	 */
+	struct intel_fbc_state state_cache;
+
+	/*
+	 * This structure contains everything that's relevant to program the
+	 * hardware registers. When we want to figure out if we need to disable
+	 * and re-enable FBC for a new configuration we just check if there's
+	 * something different in the struct. The genx_fbc_activate functions
+	 * are supposed to read from it in order to program the registers.
+	 */
+	struct intel_fbc_state params;
+	const char *no_fbc_reason;
+};
+
 /* plane stride in pixels */
 static unsigned int intel_fbc_plane_stride(const struct intel_plane_state *plane_state)
 {
@@ -762,14 +819,16 @@ static void __intel_fbc_cleanup_cfb(struct intel_fbc *fbc)
 
 void intel_fbc_cleanup(struct drm_i915_private *i915)
 {
-	struct intel_fbc *fbc = &i915->fbc;
+	struct intel_fbc *fbc = i915->fbc;
 
-	if (!HAS_FBC(i915))
+	if (!fbc)
 		return;
 
 	mutex_lock(&fbc->lock);
 	__intel_fbc_cleanup_cfb(fbc);
 	mutex_unlock(&fbc->lock);
+
+	kfree(fbc);
 }
 
 static bool stride_is_valid(const struct intel_plane_state *plane_state)
@@ -1319,9 +1378,9 @@ void intel_fbc_invalidate(struct drm_i915_private *i915,
 			  unsigned int frontbuffer_bits,
 			  enum fb_op_origin origin)
 {
-	struct intel_fbc *fbc = &i915->fbc;
+	struct intel_fbc *fbc = i915->fbc;
 
-	if (!HAS_FBC(i915))
+	if (!fbc)
 		return;
 
 	if (origin == ORIGIN_FLIP || origin == ORIGIN_CURSOR_UPDATE)
@@ -1340,9 +1399,9 @@ void intel_fbc_invalidate(struct drm_i915_private *i915,
 void intel_fbc_flush(struct drm_i915_private *i915,
 		     unsigned int frontbuffer_bits, enum fb_op_origin origin)
 {
-	struct intel_fbc *fbc = &i915->fbc;
+	struct intel_fbc *fbc = i915->fbc;
 
-	if (!HAS_FBC(i915))
+	if (!fbc)
 		return;
 
 	mutex_lock(&fbc->lock);
@@ -1484,9 +1543,9 @@ void intel_fbc_update(struct intel_atomic_state *state,
  */
 void intel_fbc_global_disable(struct drm_i915_private *i915)
 {
-	struct intel_fbc *fbc = &i915->fbc;
+	struct intel_fbc *fbc = i915->fbc;
 
-	if (!HAS_FBC(i915))
+	if (!fbc)
 		return;
 
 	mutex_lock(&fbc->lock);
@@ -1497,9 +1556,8 @@ void intel_fbc_global_disable(struct drm_i915_private *i915)
 
 static void intel_fbc_underrun_work_fn(struct work_struct *work)
 {
-	struct drm_i915_private *i915 =
-		container_of(work, struct drm_i915_private, fbc.underrun_work);
-	struct intel_fbc *fbc = &i915->fbc;
+	struct intel_fbc *fbc = container_of(work, typeof(*fbc), underrun_work);
+	struct drm_i915_private *i915 = fbc->i915;
 
 	mutex_lock(&fbc->lock);
 
@@ -1524,9 +1582,9 @@ static void intel_fbc_underrun_work_fn(struct work_struct *work)
  */
 void intel_fbc_reset_underrun(struct drm_i915_private *i915)
 {
-	struct intel_fbc *fbc = &i915->fbc;
+	struct intel_fbc *fbc = i915->fbc;
 
-	if (!HAS_FBC(i915))
+	if (!fbc)
 		return;
 
 	cancel_work_sync(&fbc->underrun_work);
@@ -1559,9 +1617,9 @@ void intel_fbc_reset_underrun(struct drm_i915_private *i915)
  */
 void intel_fbc_handle_fifo_underrun_irq(struct drm_i915_private *i915)
 {
-	struct intel_fbc *fbc = &i915->fbc;
+	struct intel_fbc *fbc = i915->fbc;
 
-	if (!HAS_FBC(i915))
+	if (!fbc)
 		return;
 
 	/* There's no guarantee that underrun_detected won't be set to true
@@ -1621,35 +1679,17 @@ void intel_fbc_add_plane(struct intel_fbc *fbc, struct intel_plane *plane)
 	fbc->possible_framebuffer_bits |= plane->frontbuffer_bit;
 }
 
-/**
- * intel_fbc_init - Initialize FBC
- * @i915: the i915 device
- *
- * This function might be called during PM init process.
- */
-void intel_fbc_init(struct drm_i915_private *i915)
+static struct intel_fbc *intel_fbc_create(struct drm_i915_private *i915)
 {
-	struct intel_fbc *fbc = &i915->fbc;
+	struct intel_fbc *fbc;
+
+	fbc = kzalloc(sizeof(*fbc), GFP_KERNEL);
+	if (!fbc)
+		return NULL;
 
 	fbc->i915 = i915;
 	INIT_WORK(&fbc->underrun_work, intel_fbc_underrun_work_fn);
 	mutex_init(&fbc->lock);
-	fbc->active = false;
-
-	if (!drm_mm_initialized(&i915->mm.stolen))
-		mkwrite_device_info(i915)->display.has_fbc = false;
-
-	if (need_fbc_vtd_wa(i915))
-		mkwrite_device_info(i915)->display.has_fbc = false;
-
-	i915->params.enable_fbc = intel_sanitize_fbc_option(i915);
-	drm_dbg_kms(&i915->drm, "Sanitized enable_fbc value: %d\n",
-		    i915->params.enable_fbc);
-
-	if (!HAS_FBC(i915)) {
-		fbc->no_fbc_reason = "unsupported by this chipset";
-		return;
-	}
 
 	if (DISPLAY_VER(i915) >= 7)
 		fbc->funcs = &ivb_fbc_funcs;
@@ -1664,11 +1704,43 @@ void intel_fbc_init(struct drm_i915_private *i915)
 	else
 		fbc->funcs = &i8xx_fbc_funcs;
 
+	return fbc;
+}
+
+/**
+ * intel_fbc_init - Initialize FBC
+ * @i915: the i915 device
+ *
+ * This function might be called during PM init process.
+ */
+void intel_fbc_init(struct drm_i915_private *i915)
+{
+	struct intel_fbc *fbc;
+
+	if (!drm_mm_initialized(&i915->mm.stolen))
+		mkwrite_device_info(i915)->display.has_fbc = false;
+
+	if (need_fbc_vtd_wa(i915))
+		mkwrite_device_info(i915)->display.has_fbc = false;
+
+	i915->params.enable_fbc = intel_sanitize_fbc_option(i915);
+	drm_dbg_kms(&i915->drm, "Sanitized enable_fbc value: %d\n",
+		    i915->params.enable_fbc);
+
+	if (!HAS_FBC(i915))
+		return;
+
+	fbc = intel_fbc_create(i915);
+	if (!fbc)
+		return;
+
 	/* We still don't have any sort of hardware state readout for FBC, so
 	 * deactivate it in case the BIOS activated it to make sure software
 	 * matches the hardware state. */
 	if (intel_fbc_hw_is_active(fbc))
 		intel_fbc_hw_deactivate(fbc);
+
+	i915->fbc = fbc;
 }
 
 static int intel_fbc_debugfs_status_show(struct seq_file *m, void *unused)
@@ -1743,8 +1815,8 @@ static void intel_fbc_debugfs_add(struct intel_fbc *fbc)
 
 void intel_fbc_debugfs_register(struct drm_i915_private *i915)
 {
-	struct intel_fbc *fbc = &i915->fbc;
+	struct intel_fbc *fbc = i915->fbc;
 
-	if (HAS_FBC(i915))
+	if (fbc)
 		intel_fbc_debugfs_add(fbc);
 }
diff --git a/drivers/gpu/drm/i915/display/skl_universal_plane.c b/drivers/gpu/drm/i915/display/skl_universal_plane.c
index 22ec6901ee30..980f23680842 100644
--- a/drivers/gpu/drm/i915/display/skl_universal_plane.c
+++ b/drivers/gpu/drm/i915/display/skl_universal_plane.c
@@ -1820,7 +1820,7 @@ static struct intel_fbc *skl_plane_fbc(struct drm_i915_private *dev_priv,
 				       enum pipe pipe, enum plane_id plane_id)
 {
 	if (skl_plane_has_fbc(dev_priv, pipe, plane_id))
-		return &dev_priv->fbc;
+		return dev_priv->fbc;
 	else
 		return NULL;
 }
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f632b026ce34..12099f7ff98e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -399,64 +399,8 @@ struct drm_i915_display_funcs {
 	void (*commit_modeset_enables)(struct intel_atomic_state *state);
 };
 
-struct intel_fbc_funcs;
-
 #define I915_COLOR_UNEVICTABLE (-1) /* a non-vma sharing the address space */
 
-struct intel_fbc_state {
-	const char *no_fbc_reason;
-	enum i9xx_plane_id i9xx_plane;
-	unsigned int cfb_stride;
-	unsigned int cfb_size;
-	unsigned int fence_y_offset;
-	u16 override_cfb_stride;
-	u16 interval;
-	s8 fence_id;
-};
-
-struct intel_fbc {
-	struct drm_i915_private *i915;
-	const struct intel_fbc_funcs *funcs;
-
-	/* This is always the inner lock when overlapping with struct_mutex and
-	 * it's the outer lock when overlapping with stolen_lock. */
-	struct mutex lock;
-	unsigned int possible_framebuffer_bits;
-	unsigned int busy_bits;
-	struct intel_plane *plane;
-
-	struct drm_mm_node compressed_fb;
-	struct drm_mm_node compressed_llb;
-
-	u8 limit;
-
-	bool false_color;
-
-	bool active;
-	bool activated;
-	bool flip_pending;
-
-	bool underrun_detected;
-	struct work_struct underrun_work;
-
-	/*
-	 * Due to the atomic rules we can't access some structures without the
-	 * appropriate locking, so we cache information here in order to avoid
-	 * these problems.
-	 */
-	struct intel_fbc_state state_cache;
-
-	/*
-	 * This structure contains everything that's relevant to program the
-	 * hardware registers. When we want to figure out if we need to disable
-	 * and re-enable FBC for a new configuration we just check if there's
-	 * something different in the struct. The genx_fbc_activate functions
-	 * are supposed to read from it in order to program the registers.
-	 */
-	struct intel_fbc_state params;
-	const char *no_fbc_reason;
-};
-
 /*
  * HIGH_RR is the highest eDP panel refresh rate read from EDID
  * LOW_RR is the lowest eDP panel refresh rate found from EDID
@@ -493,7 +437,6 @@ struct i915_drrs {
 #define QUIRK_NO_PPS_BACKLIGHT_POWER_HOOK (1<<8)
 
 struct intel_fbdev;
-struct intel_fbc_work;
 
 struct intel_gmbus {
 	struct i2c_adapter adapter;
@@ -892,7 +835,7 @@ struct drm_i915_private {
 	u32 pipestat_irq_mask[I915_MAX_PIPES];
 
 	struct i915_hotplug hotplug;
-	struct intel_fbc fbc;
+	struct intel_fbc *fbc;
 	struct i915_drrs drrs;
 	struct intel_opregion opregion;
 	struct intel_vbt_data vbt;
-- 
2.32.0



More information about the Intel-gfx mailing list