Re: [PATCH] drm/i915/skl: Correct other-pipe watermark update condition check (v2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Yes, it doesn't solve all warnings.
Other than fbdev restore, there is one warning which I faced during DPMS/Hot-plug, which is due to calculation of watermarks twice. Once for first CRTC "Pipe-A" (during which it consider/calculates watermark for other pipe "Pipe-B" as well & stores in structure "intel_crtc->wm.active.skl") & Now in same atomic_commit, when it calculate watermark for second CRTC "Pipe-B" , that time watermark for other-pipe (in this case pipe-A) already have calculated values so it gives !wm_changed warning.

Hoping 2-stage watermark programming will take care of this, as in that case, If I'm not misunderstood final results should be stored after calculation for all pipes.

Regards,
-Mahesh

On 10/23/2015 10:11 PM, Matt Roper wrote:
From: "Kumar, Mahesh" <mahesh1.kumar@xxxxxxxxx>

If ddb allocation for planes in current CRTC is changed, that doesn't
lead to ddb allocation change for other CRTCs, because our DDB allocation
is not dynamic according to plane parameters, ddb is allocated according
to number of CRTC enabled, & divided equally among CTRC's.

In current condition check during Watermark calculation, if number of
plane/ddb allocation changes for current CRTC, Watermark for other pipes
are recalculated. But there is no change in DDB allocation of other pipe
so watermark is also not changed, This leads to warning messages.
WARN_ON(!wm_changed)

This patch corrects this and check if DDB allocation for pipes is changed,
then only recalculate watermarks.

v2 (by Matt): Rebased to latest -nightly and fixed a typo

Signed-off-by: Kumar, Mahesh <mahesh1.kumar@xxxxxxxxx>
Reviewed-by(v1): Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
---
I don't think this solves all the !wm_changed warnings (I still see one during
fbdev restore following various igt tests), but it seems like a move in the
right direction so I figured I'd go ahead and rebase Mahesh' patch so it can
get merged.

  drivers/gpu/drm/i915/intel_pm.c | 12 +++++-------
  1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 0fb0459..0467e34 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3064,14 +3064,12 @@ static bool skl_ddb_allocation_changed(const struct skl_ddb_allocation *new_ddb,
  	struct drm_device *dev = intel_crtc->base.dev;
  	struct drm_i915_private *dev_priv = dev->dev_private;
  	const struct skl_ddb_allocation *cur_ddb = &dev_priv->wm.skl_hw.ddb;
-	enum pipe pipe = intel_crtc->pipe;
-
-	if (memcmp(new_ddb->plane[pipe], cur_ddb->plane[pipe],
-		   sizeof(new_ddb->plane[pipe])))
-		return true;
- if (memcmp(&new_ddb->plane[pipe][PLANE_CURSOR], &cur_ddb->plane[pipe][PLANE_CURSOR],
-		    sizeof(new_ddb->plane[pipe][PLANE_CURSOR])))
+	/*
+	 * If ddb allocation of pipes changed, it may require recalculation of
+	 * watermarks
+	 */
+	if (memcmp(new_ddb->pipe, cur_ddb->pipe, sizeof(new_ddb->pipe)))
  		return true;
return false;

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux