[PATCH 1/3] drm/i915: don't deactivate FBC at skylake_disable_primary

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 1/3] drm/i915: don't deactivate FBC at skylake_disable_primary_plane
@ 2016-01-29 20:57 Paulo Zanoni
  2016-01-29 20:57 ` [PATCH 2/3] drm/i915/fbc: unexport the HW level activation functions Paulo Zanoni
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Paulo Zanoni @ 2016-01-29 20:57 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

FBC is already deactivated at this point.

Besides, nothing should be calling these lower-level function
pointers. A few months ago, the only caller of
dev_priv->fbc.deactivate was intel_pipe_set_base_atomic(), which was
the kgdboc function. But the following commit added it to the SKL
function:

    commit a8d201af68506b375b701d0d8dbe8487034256f2
    Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
    Date:   Thu Jan 7 11:54:11 2016 +0100
        drm/i915: Use plane state for primary plane updates.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a66220a..186d6ca 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3156,9 +3156,6 @@ static void skylake_disable_primary_plane(struct drm_plane *primary,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	int pipe = to_intel_crtc(crtc)->pipe;
 
-	if (dev_priv->fbc.deactivate)
-		dev_priv->fbc.deactivate(dev_priv);
-
 	I915_WRITE(PLANE_CTL(pipe, 0), 0);
 	I915_WRITE(PLANE_SURF(pipe, 0), 0);
 	POSTING_READ(PLANE_SURF(pipe, 0));
-- 
2.7.0.rc3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] drm/i915/fbc: unexport the HW level activation functions
  2016-01-29 20:57 [PATCH 1/3] drm/i915: don't deactivate FBC at skylake_disable_primary_plane Paulo Zanoni
@ 2016-01-29 20:57 ` Paulo Zanoni
  2016-01-29 20:57 ` [PATCH 3/3] drm/i915/fbc: set fbc->active from the new " Paulo Zanoni
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Paulo Zanoni @ 2016-01-29 20:57 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

The recent introduction of a new caller of dev_priv->fbc.deactivate()
is a good example of why we need unexport those functions. Anything
outside intel_fbc.c should only call the functions exported by
intel_fbc.c, so in order to enforce that, kill the function pointers
stored inside dev_priv->fbc and replace them with functions that can't
be called from outside intel_fbc.c.

This should make it much harder for new code to call these functions
from outside intel_fbc.c.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  4 ---
 drivers/gpu/drm/i915/intel_fbc.c | 65 ++++++++++++++++++++++++----------------
 2 files changed, 40 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c921ad8..1cd4431 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -964,10 +964,6 @@ struct intel_fbc {
 	} work;
 
 	const char *no_fbc_reason;
-
-	bool (*is_active)(struct drm_i915_private *dev_priv);
-	void (*activate)(struct drm_i915_private *dev_priv);
-	void (*deactivate)(struct drm_i915_private *dev_priv);
 };
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 5adf6d7..8d3caf2 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -43,7 +43,7 @@
 
 static inline bool fbc_supported(struct drm_i915_private *dev_priv)
 {
-	return dev_priv->fbc.activate != NULL;
+	return HAS_FBC(dev_priv);
 }
 
 static inline bool fbc_on_pipe_a_only(struct drm_i915_private *dev_priv)
@@ -343,6 +343,38 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
 	intel_fbc_recompress(dev_priv);
 }
 
+static bool intel_fbc_hw_is_active(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_INFO(dev_priv)->gen >= 5)
+		return ilk_fbc_is_active(dev_priv);
+	else if (IS_GM45(dev_priv))
+		return g4x_fbc_is_active(dev_priv);
+	else
+		return i8xx_fbc_is_active(dev_priv);
+}
+
+static void intel_fbc_hw_activate(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_INFO(dev_priv)->gen >= 7)
+		gen7_fbc_activate(dev_priv);
+	else if (INTEL_INFO(dev_priv)->gen >= 5)
+		ilk_fbc_activate(dev_priv);
+	else if (IS_GM45(dev_priv))
+		g4x_fbc_activate(dev_priv);
+	else
+		i8xx_fbc_activate(dev_priv);
+}
+
+static void intel_fbc_hw_deactivate(struct drm_i915_private *dev_priv)
+{
+	if (INTEL_INFO(dev_priv)->gen >= 5)
+		ilk_fbc_deactivate(dev_priv);
+	else if (IS_GM45(dev_priv))
+		g4x_fbc_deactivate(dev_priv);
+	else
+		i8xx_fbc_deactivate(dev_priv);
+}
+
 /**
  * intel_fbc_is_active - Is FBC active?
  * @dev_priv: i915 device instance
@@ -405,7 +437,7 @@ retry:
 		goto retry;
 	}
 
-	fbc->activate(dev_priv);
+	intel_fbc_hw_activate(dev_priv);
 
 	work->scheduled = false;
 
@@ -451,7 +483,7 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv)
 	fbc->work.scheduled = false;
 
 	if (fbc->active)
-		fbc->deactivate(dev_priv);
+		intel_fbc_hw_deactivate(dev_priv);
 }
 
 static bool multiple_pipes_ok(struct intel_crtc *crtc)
@@ -713,7 +745,7 @@ static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
 
 	/* FIXME: We lack the proper locking here, so only run this on the
 	 * platforms that need. */
-	if (dev_priv->fbc.activate == ilk_fbc_activate)
+	if (INTEL_INFO(dev_priv)->gen >= 5 && INTEL_INFO(dev_priv)->gen < 7)
 		cache->fb.ilk_ggtt_offset = i915_gem_obj_ggtt_offset(obj);
 	cache->fb.pixel_format = fb->pixel_format;
 	cache->fb.stride = fb->pitches[0];
@@ -1223,30 +1255,13 @@ void intel_fbc_init(struct drm_i915_private *dev_priv)
 			break;
 	}
 
-	if (INTEL_INFO(dev_priv)->gen >= 7) {
-		fbc->is_active = ilk_fbc_is_active;
-		fbc->activate = gen7_fbc_activate;
-		fbc->deactivate = ilk_fbc_deactivate;
-	} else if (INTEL_INFO(dev_priv)->gen >= 5) {
-		fbc->is_active = ilk_fbc_is_active;
-		fbc->activate = ilk_fbc_activate;
-		fbc->deactivate = ilk_fbc_deactivate;
-	} else if (IS_GM45(dev_priv)) {
-		fbc->is_active = g4x_fbc_is_active;
-		fbc->activate = g4x_fbc_activate;
-		fbc->deactivate = g4x_fbc_deactivate;
-	} else {
-		fbc->is_active = i8xx_fbc_is_active;
-		fbc->activate = i8xx_fbc_activate;
-		fbc->deactivate = i8xx_fbc_deactivate;
-
-		/* This value was pulled out of someone's hat */
+	/* This value was pulled out of someone's hat */
+	if (INTEL_INFO(dev_priv)->gen <= 4 && !IS_GM45(dev_priv))
 		I915_WRITE(FBC_CONTROL, 500 << FBC_CTL_INTERVAL_SHIFT);
-	}
 
 	/* 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 (fbc->is_active(dev_priv))
-		fbc->deactivate(dev_priv);
+	if (intel_fbc_hw_is_active(dev_priv))
+		intel_fbc_hw_deactivate(dev_priv);
 }
-- 
2.7.0.rc3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] drm/i915/fbc: set fbc->active from the new activation functions
  2016-01-29 20:57 [PATCH 1/3] drm/i915: don't deactivate FBC at skylake_disable_primary_plane Paulo Zanoni
  2016-01-29 20:57 ` [PATCH 2/3] drm/i915/fbc: unexport the HW level activation functions Paulo Zanoni
@ 2016-01-29 20:57 ` Paulo Zanoni
  2016-02-01  9:11 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: don't deactivate FBC at skylake_disable_primary_plane Patchwork
  2016-02-02  9:27 ` [PATCH 1/3] " Maarten Lankhorst
  3 siblings, 0 replies; 5+ messages in thread
From: Paulo Zanoni @ 2016-01-29 20:57 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni

Now that we have top-level gen-independent hw_activate and
hw_deactivate functions, set fbc->active directly from them, removing
the duplicated code.

Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
 drivers/gpu/drm/i915/intel_fbc.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 8d3caf2..3614a95 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -115,8 +115,6 @@ static void i8xx_fbc_deactivate(struct drm_i915_private *dev_priv)
 {
 	u32 fbc_ctl;
 
-	dev_priv->fbc.active = false;
-
 	/* Disable compression */
 	fbc_ctl = I915_READ(FBC_CONTROL);
 	if ((fbc_ctl & FBC_CTL_EN) == 0)
@@ -139,8 +137,6 @@ static void i8xx_fbc_activate(struct drm_i915_private *dev_priv)
 	int i;
 	u32 fbc_ctl;
 
-	dev_priv->fbc.active = true;
-
 	/* Note: fbc.threshold == 1 for i8xx */
 	cfb_pitch = params->cfb_size / FBC_LL_SIZE;
 	if (params->fb.stride < cfb_pitch)
@@ -187,8 +183,6 @@ static void g4x_fbc_activate(struct drm_i915_private *dev_priv)
 	struct intel_fbc_reg_params *params = &dev_priv->fbc.params;
 	u32 dpfc_ctl;
 
-	dev_priv->fbc.active = true;
-
 	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane) | DPFC_SR_EN;
 	if (drm_format_plane_cpp(params->fb.pixel_format, 0) == 2)
 		dpfc_ctl |= DPFC_CTL_LIMIT_2X;
@@ -206,8 +200,6 @@ static void g4x_fbc_deactivate(struct drm_i915_private *dev_priv)
 {
 	u32 dpfc_ctl;
 
-	dev_priv->fbc.active = false;
-
 	/* Disable compression */
 	dpfc_ctl = I915_READ(DPFC_CONTROL);
 	if (dpfc_ctl & DPFC_CTL_EN) {
@@ -234,8 +226,6 @@ static void ilk_fbc_activate(struct drm_i915_private *dev_priv)
 	u32 dpfc_ctl;
 	int threshold = dev_priv->fbc.threshold;
 
-	dev_priv->fbc.active = true;
-
 	dpfc_ctl = DPFC_CTL_PLANE(params->crtc.plane);
 	if (drm_format_plane_cpp(params->fb.pixel_format, 0) == 2)
 		threshold++;
@@ -274,8 +264,6 @@ static void ilk_fbc_deactivate(struct drm_i915_private *dev_priv)
 {
 	u32 dpfc_ctl;
 
-	dev_priv->fbc.active = false;
-
 	/* Disable compression */
 	dpfc_ctl = I915_READ(ILK_DPFC_CONTROL);
 	if (dpfc_ctl & DPFC_CTL_EN) {
@@ -295,8 +283,6 @@ static void gen7_fbc_activate(struct drm_i915_private *dev_priv)
 	u32 dpfc_ctl;
 	int threshold = dev_priv->fbc.threshold;
 
-	dev_priv->fbc.active = true;
-
 	dpfc_ctl = 0;
 	if (IS_IVYBRIDGE(dev_priv))
 		dpfc_ctl |= IVB_DPFC_CTL_PLANE(params->crtc.plane);
@@ -355,6 +341,10 @@ static bool intel_fbc_hw_is_active(struct drm_i915_private *dev_priv)
 
 static void intel_fbc_hw_activate(struct drm_i915_private *dev_priv)
 {
+	struct intel_fbc *fbc = &dev_priv->fbc;
+
+	fbc->active = true;
+
 	if (INTEL_INFO(dev_priv)->gen >= 7)
 		gen7_fbc_activate(dev_priv);
 	else if (INTEL_INFO(dev_priv)->gen >= 5)
@@ -367,6 +357,10 @@ static void intel_fbc_hw_activate(struct drm_i915_private *dev_priv)
 
 static void intel_fbc_hw_deactivate(struct drm_i915_private *dev_priv)
 {
+	struct intel_fbc *fbc = &dev_priv->fbc;
+
+	fbc->active = false;
+
 	if (INTEL_INFO(dev_priv)->gen >= 5)
 		ilk_fbc_deactivate(dev_priv);
 	else if (IS_GM45(dev_priv))
-- 
2.7.0.rc3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: don't deactivate FBC at skylake_disable_primary_plane
  2016-01-29 20:57 [PATCH 1/3] drm/i915: don't deactivate FBC at skylake_disable_primary_plane Paulo Zanoni
  2016-01-29 20:57 ` [PATCH 2/3] drm/i915/fbc: unexport the HW level activation functions Paulo Zanoni
  2016-01-29 20:57 ` [PATCH 3/3] drm/i915/fbc: set fbc->active from the new " Paulo Zanoni
@ 2016-02-01  9:11 ` Patchwork
  2016-02-02  9:27 ` [PATCH 1/3] " Maarten Lankhorst
  3 siblings, 0 replies; 5+ messages in thread
From: Patchwork @ 2016-02-01  9:11 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx

== Summary ==

Series 2935v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/2935/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> PASS       (hsw-gt2)
        Subgroup suspend-read-crc-pipe-c:
                dmesg-warn -> PASS       (bsw-nuc-2)

bdw-nuci7        total:156  pass:147  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:159  pass:147  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2        total:159  pass:129  dwarn:0   dfail:0   fail:0   skip:30 
byt-nuc          total:159  pass:136  dwarn:0   dfail:0   fail:0   skip:23 
hsw-brixbox      total:159  pass:146  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2          total:159  pass:149  dwarn:0   dfail:0   fail:0   skip:10 
ilk-hp8440p      total:159  pass:110  dwarn:1   dfail:0   fail:0   skip:48 
ivb-t430s        total:159  pass:145  dwarn:0   dfail:0   fail:0   skip:14 
skl-i5k-2        total:159  pass:144  dwarn:1   dfail:0   fail:0   skip:14 
snb-dellxps      total:159  pass:137  dwarn:0   dfail:0   fail:0   skip:22 

Results at /archive/results/CI_IGT_test/Patchwork_1332/

6b1049b84dcd979f631d15b2ada325d8e5b2c4e1 drm-intel-nightly: 2016y-01m-29d-22h-50m-57s UTC integration manifest
b798faddcc5ece7ee7e46cdf47a685b7b2700cec drm/i915/fbc: set fbc->active from the new activation functions
8e30f9788a34bca43a06666a2d9b7f72669b684e drm/i915/fbc: unexport the HW level activation functions
bed92771e300c17b2e91587e9ed6b200f7893740 drm/i915: don't deactivate FBC at skylake_disable_primary_plane

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] drm/i915: don't deactivate FBC at skylake_disable_primary_plane
  2016-01-29 20:57 [PATCH 1/3] drm/i915: don't deactivate FBC at skylake_disable_primary_plane Paulo Zanoni
                   ` (2 preceding siblings ...)
  2016-02-01  9:11 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: don't deactivate FBC at skylake_disable_primary_plane Patchwork
@ 2016-02-02  9:27 ` Maarten Lankhorst
  3 siblings, 0 replies; 5+ messages in thread
From: Maarten Lankhorst @ 2016-02-02  9:27 UTC (permalink / raw)
  To: Paulo Zanoni, intel-gfx

Hey,

Op 29-01-16 om 21:57 schreef Paulo Zanoni:
> FBC is already deactivated at this point.
>
> Besides, nothing should be calling these lower-level function
> pointers. A few months ago, the only caller of
> dev_priv->fbc.deactivate was intel_pipe_set_base_atomic(), which was
> the kgdboc function. But the following commit added it to the SKL
> function:
>
>     commit a8d201af68506b375b701d0d8dbe8487034256f2
>     Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>     Date:   Thu Jan 7 11:54:11 2016 +0100
>         drm/i915: Use plane state for primary plane updates.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
Whole series looks good to me.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-02-02  9:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-29 20:57 [PATCH 1/3] drm/i915: don't deactivate FBC at skylake_disable_primary_plane Paulo Zanoni
2016-01-29 20:57 ` [PATCH 2/3] drm/i915/fbc: unexport the HW level activation functions Paulo Zanoni
2016-01-29 20:57 ` [PATCH 3/3] drm/i915/fbc: set fbc->active from the new " Paulo Zanoni
2016-02-01  9:11 ` ✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: don't deactivate FBC at skylake_disable_primary_plane Patchwork
2016-02-02  9:27 ` [PATCH 1/3] " Maarten Lankhorst

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.