[PATCH 0/3] PSR lag fixes

[PATCH 0/3] PSR lag fixes

[Dhinakaran Pandiyan]

PSR currently when enabled results in semi-permanent freezes or noticeable
cursor lags. 

https://patchwork.freedesktop.org/series/37598/ will fix long freezes due
to frame counter resets.

This series has three more fixes -
Patch 1 eliminates PSR exit for flips and makes us rely on the HW to do it.
Patch 2 fixes cusor move lag by relying on HW to exit PSR.
Patch 3 fixes temporary freeze seen with fbdev.

With both the series applied, PSR on my SKL ThinkPad feels pretty good.

Dhinakaran Pandiyan (2):
  drm/i915/psr: HW tracking for cursor moves to fix lags.
  drm/i915/psr: Wait for PSR transition to complete before exiting.

Rodrigo Vivi (1):
  drm/i915/psr: Use more PSR HW tracking.

 drivers/gpu/drm/i915/i915_drv.h          |  2 ++
 drivers/gpu/drm/i915/i915_gem.c          |  2 +-
 drivers/gpu/drm/i915/intel_drv.h         |  3 ++-
 drivers/gpu/drm/i915/intel_frontbuffer.c |  2 +-
 drivers/gpu/drm/i915/intel_psr.c         | 24 +++++++++++++++++++++++-
 5 files changed, 29 insertions(+), 4 deletions(-)

-- 
2.14.1

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

[PATCH 1/3] drm/i915/psr: Use more PSR HW tracking.

[Dhinakaran Pandiyan]

From: Rodrigo Vivi <rodrigo.vivi@intel.com>

So far we are using frontbuffer tracking for everything
and ignoring that PSR has a HW capable HW tracking for many
modern usages of GPU on Core platforms and newer Atom ones.

One reason for that is that we were trying to keep same
infrastructure in place for VLV/CHV than the rest of platforms.
But also because when this infrastructure was created
the front-buffer-tracking origin wasn't that good and stable
how it is today after Paulo reworked it to attend FBC cases.

However this PSR implementation without HW tracking died
on gen8LP. And newer platforms are starting to demand more HW
tracking specially with PSR2 cases in mind.

By disabling and re-enabling PSR totally every time we believe
someone is going to change the front buffer content we don't
allow PSR HW tracking to do this job and specially compromising
the whole idea of PSR2 case where the HW tracking detect only
the damaged area and do a partial screen update.

So, from now on, on the platforms that has hw_tracking let's
rely more on HW tracking.

This also is the case in used by other drivers and more validated
by SV teams. So I hope that this will lead us to less misterious
bugs.

v2: Only do this for platform that actually has hw tracking.

v3 from DK
Do this only for flips, small gradual changes are better.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jim Bride <jim.bride@linux.intel.com>
Cc: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          |  1 +
 drivers/gpu/drm/i915/intel_drv.h         |  3 ++-
 drivers/gpu/drm/i915/intel_frontbuffer.c |  2 +-
 drivers/gpu/drm/i915/intel_psr.c         | 10 +++++++++-
 4 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 70cf289855af..19d5ac4921e5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -770,6 +770,7 @@ struct i915_psr {
 	bool y_cord_support;
 	bool colorimetry_support;
 	bool alpm;
+	bool has_hw_tracking;
 
 	void (*enable_source)(struct intel_dp *,
 			      const struct intel_crtc_state *);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4b1d357a43ae..6f9a7a81005f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1860,7 +1860,8 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 void intel_psr_disable(struct intel_dp *intel_dp,
 		      const struct intel_crtc_state *old_crtc_state);
 void intel_psr_invalidate(struct drm_i915_private *dev_priv,
-			  unsigned frontbuffer_bits);
+			  unsigned frontbuffer_bits,
+			  enum fb_op_origin origin);
 void intel_psr_flush(struct drm_i915_private *dev_priv,
 		     unsigned frontbuffer_bits,
 		     enum fb_op_origin origin);
diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index fcfc217e754e..efda1af9a5b3 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -79,7 +79,7 @@ void __intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
 		spin_unlock(&dev_priv->fb_tracking.lock);
 	}
 
-	intel_psr_invalidate(dev_priv, frontbuffer_bits);
+	intel_psr_invalidate(dev_priv, frontbuffer_bits, origin);
 	intel_edp_drrs_invalidate(dev_priv, frontbuffer_bits);
 	intel_fbc_invalidate(dev_priv, frontbuffer_bits, origin);
 }
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2ef374f936b9..2a31c7cbdb41 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -824,6 +824,7 @@ void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
  * intel_psr_invalidate - Invalidade PSR
  * @dev_priv: i915 device
  * @frontbuffer_bits: frontbuffer plane tracking bits
+ * @origin: which operation caused the invalidate
  *
  * Since the hardware frontbuffer tracking has gaps we need to integrate
  * with the software frontbuffer tracking. This function gets called every
@@ -833,7 +834,7 @@ void intel_psr_single_frame_update(struct drm_i915_private *dev_priv,
  * Dirty frontbuffers relevant to PSR are tracked in busy_frontbuffer_bits."
  */
 void intel_psr_invalidate(struct drm_i915_private *dev_priv,
-			  unsigned frontbuffer_bits)
+			  unsigned frontbuffer_bits, enum fb_op_origin origin)
 {
 	struct drm_crtc *crtc;
 	enum pipe pipe;
@@ -841,6 +842,9 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
 	if (!CAN_PSR(dev_priv))
 		return;
 
+	if (dev_priv->psr.has_hw_tracking && origin == ORIGIN_FLIP)
+		return;
+
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
 		mutex_unlock(&dev_priv->psr.lock);
@@ -881,6 +885,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 	if (!CAN_PSR(dev_priv))
 		return;
 
+	if (dev_priv->psr.has_hw_tracking && origin == ORIGIN_FLIP)
+		return;
+
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
 		mutex_unlock(&dev_priv->psr.lock);
@@ -957,6 +964,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
 		dev_priv->psr.activate = vlv_psr_activate;
 		dev_priv->psr.setup_vsc = vlv_psr_setup_vsc;
 	} else {
+		dev_priv->psr.has_hw_tracking = true;
 		dev_priv->psr.enable_source = hsw_psr_enable_source;
 		dev_priv->psr.disable_source = hsw_psr_disable;
 		dev_priv->psr.enable_sink = hsw_psr_enable_sink;
-- 
2.14.1

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

[PATCH 2/3] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Dhinakaran Pandiyan]

DRM_IOCTL_MODE_CURSOR results in a frontbuffer flush before the cursor
plane MMIOs are written to. But this flush is not necessary for PSR as
hardware tracking takes care of exiting PSR when the MMIO's are written.

Introduce a new fb_op_origin enum to differentiate these flushes from
those originating due to a dirty fbdev buffer and ignore this enum in
psr_flush and psr_invalidate.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  | 1 +
 drivers/gpu/drm/i915/i915_gem.c  | 2 +-
 drivers/gpu/drm/i915/intel_psr.c | 6 ++++--
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 19d5ac4921e5..158e774ed2e6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -637,6 +637,7 @@ enum fb_op_origin {
 	ORIGIN_CS,
 	ORIGIN_FLIP,
 	ORIGIN_DIRTYFB,
+	ORIGIN_PINNEDFB,
 };
 
 struct intel_fbc {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fc68b35854df..43146699c497 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4141,7 +4141,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 
 	/* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
 	__i915_gem_object_flush_for_display(obj);
-	intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
+	intel_fb_obj_flush(obj, ORIGIN_PINNEDFB);
 
 	/* It should now be out of any other write domains, and we can update
 	 * the domain values for our changes.
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2a31c7cbdb41..ddfabdff3dea 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -842,7 +842,8 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
 	if (!CAN_PSR(dev_priv))
 		return;
 
-	if (dev_priv->psr.has_hw_tracking && origin == ORIGIN_FLIP)
+	if (dev_priv->psr.has_hw_tracking &&
+	    (origin == ORIGIN_FLIP || origin == ORIGIN_PINNEDFB))
 		return;
 
 	mutex_lock(&dev_priv->psr.lock);
@@ -885,7 +886,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 	if (!CAN_PSR(dev_priv))
 		return;
 
-	if (dev_priv->psr.has_hw_tracking && origin == ORIGIN_FLIP)
+	if (dev_priv->psr.has_hw_tracking &&
+	    (origin == ORIGIN_FLIP || origin == ORIGIN_PINNEDFB))
 		return;
 
 	mutex_lock(&dev_priv->psr.lock);
-- 
2.14.1

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

[PATCH 3/3] drm/i915/psr: Wait for PSR transition to complete before exiting.

[Dhinakaran Pandiyan]

With fbdev, screen freezes after a few continuous PSR exit->enter cycles.
Printing out the PSR status register clearly showed this freeze coincided
with exiting when the hardware is in a transitory state. So wait for 100 ms
(~6 frames) for PSR to become active and then exit.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index ddfabdff3dea..7ef6bd01014d 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -738,6 +738,18 @@ static void intel_psr_exit(struct drm_i915_private *dev_priv)
 			WARN_ON(!(val & EDP_PSR2_ENABLE));
 			I915_WRITE(EDP_PSR2_CTL, val & ~EDP_PSR2_ENABLE);
 		} else {
+
+			/* Wait for about 6 frames in case we just enabled PSR,
+			 * this prevents the screen from freezing as the HW does
+			 * not seem to be able to back off cleanly it is already
+			 * trying to enter PSR.
+			 */
+			intel_wait_for_register(dev_priv,
+						EDP_PSR_STATUS,
+						EDP_PSR_STATUS_STATE_MASK,
+						EDP_PSR_STATUS_STATE_SRDENT,
+						100);
+
 			val = I915_READ(EDP_PSR_CTL);
 			WARN_ON(!(val & EDP_PSR_ENABLE));
 			I915_WRITE(EDP_PSR_CTL, val & ~EDP_PSR_ENABLE);
-- 
2.14.1

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

✓ Fi.CI.BAT: success for PSR lag fixes

[Patchwork]

== Series Details ==

Series: PSR lag fixes
URL   : https://patchwork.freedesktop.org/series/38067/
State : success

== Summary ==

Series 38067v1 PSR lag fixes
https://patchwork.freedesktop.org/api/1.0/series/38067/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-cnl-y3) fdo#103191

fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:418s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:423s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:375s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:481s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:286s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:482s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:483s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:469s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:456s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:565s
fi-cnl-y3        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:581s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:411s
fi-gdg-551       total:288  pass:180  dwarn:0   dfail:0   fail:0   skip:108 time:285s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:508s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:387s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:419s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:446s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:410s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:462s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:496s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:501s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:599s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:432s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:504s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:527s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:488s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:472s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:413s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:431s
fi-snb-2520m     total:3    pass:2    dwarn:0   dfail:0   fail:0   skip:0  
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:394s
Blacklisted hosts:
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:466s

4d059b70de402af68016510ae19a70c750806159 drm-tip: 2018y-02m-11d-17h-16m-33s UTC integration manifest
949c0e7a3444 drm/i915/psr: Wait for PSR transition to complete before exiting.
a084e969804e drm/i915/psr: HW tracking for cursor moves to fix lags.
6c0f580e8e39 drm/i915/psr: Use more PSR HW tracking.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7975/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for PSR lag fixes

[Patchwork]

== Series Details ==

Series: PSR lag fixes
URL   : https://patchwork.freedesktop.org/series/38067/
State : warning

== Summary ==

Test perf:
        Subgroup oa-exponents:
                pass       -> FAIL       (shard-apl) fdo#102254
        Subgroup polling:
                fail       -> PASS       (shard-hsw) fdo#102252
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (shard-snb) fdo#103375
Test kms_frontbuffer_tracking:
        Subgroup fbc-rgb565-draw-mmap-cpu:
                pass       -> SKIP       (shard-snb)
Test kms_flip:
        Subgroup 2x-flip-vs-absolute-wf_vblank-interruptible:
                fail       -> PASS       (shard-hsw) fdo#100368

fdo#102254 https://bugs.freedesktop.org/show_bug.cgi?id=102254
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368

shard-apl        total:3422 pass:1773 dwarn:1   dfail:0   fail:21  skip:1626 time:12416s
shard-hsw        total:3444 pass:1761 dwarn:1   dfail:0   fail:10  skip:1671 time:11970s
shard-snb        total:3444 pass:1349 dwarn:2   dfail:0   fail:10  skip:2083 time:6604s
Blacklisted hosts:
shard-kbl        total:3444 pass:1899 dwarn:13  dfail:0   fail:25  skip:1507 time:9794s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7975/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] PSR lag fixes

[Hans de Goede]

Hi,

On 12-02-18 07:08, Dhinakaran Pandiyan wrote:
> PSR currently when enabled results in semi-permanent freezes or noticeable
> cursor lags.
> 
> https://patchwork.freedesktop.org/series/37598/ will fix long freezes due
> to frame counter resets.
> 
> This series has three more fixes -
> Patch 1 eliminates PSR exit for flips and makes us rely on the HW to do it.
> Patch 2 fixes cusor move lag by relying on HW to exit PSR.
> Patch 3 fixes temporary freeze seen with fbdev.
> 
> With both the series applied, PSR on my SKL ThinkPad feels pretty good.

Thank you for your great work on this.

Are there any more PSR fixes in the pipeline? If not I think I should do
a custom Fedora kernel build based on 4.15 + recent fixes and ask all my
testers to retest with that.

I do have some questions before I do this:

1) I believe that only testers with skylake (normal or LP) or newer should
re-test, correct?

2) I know there are 2 series (including this one), can someone provide a link
to the latest patchwork version of those 2 series, or even better a git
branch with 4.15 + those patches? Any patches I'm missing if I pick up these
2 series?

3) I'm thinking 4.15 atm, but I could also do a 4.16-rc1 test kernel instead
if that would be better, would that be better ?

Regards,

Hans

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

Re: [PATCH 2/3] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Chris Wilson]

Quoting Dhinakaran Pandiyan (2018-02-12 06:08:04)
> DRM_IOCTL_MODE_CURSOR results in a frontbuffer flush before the cursor
> plane MMIOs are written to. But this flush is not necessary for PSR as
> hardware tracking takes care of exiting PSR when the MMIO's are written.
> 
> Introduce a new fb_op_origin enum to differentiate these flushes from
> those originating due to a dirty fbdev buffer and ignore this enum in
> psr_flush and psr_invalidate.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  | 1 +
>  drivers/gpu/drm/i915/i915_gem.c  | 2 +-
>  drivers/gpu/drm/i915/intel_psr.c | 6 ++++--
>  3 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 19d5ac4921e5..158e774ed2e6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -637,6 +637,7 @@ enum fb_op_origin {
>         ORIGIN_CS,
>         ORIGIN_FLIP,
>         ORIGIN_DIRTYFB,
> +       ORIGIN_PINNEDFB,
>  };
>  
>  struct intel_fbc {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index fc68b35854df..43146699c497 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4141,7 +4141,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  
>         /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */

Well this comment is bogus then. Please do explain what is going on
here.

>         __i915_gem_object_flush_for_display(obj);
> -       intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
> +       intel_fb_obj_flush(obj, ORIGIN_PINNEDFB);

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

Re: [PATCH 0/3] PSR lag fixes

[Pandiyan, Dhinakaran]

On Mon, 2018-02-12 at 09:45 +0100, Hans de Goede wrote:
> Hi,
> 
> On 12-02-18 07:08, Dhinakaran Pandiyan wrote:
> > PSR currently when enabled results in semi-permanent freezes or noticeable
> > cursor lags.
> > 
> > https://patchwork.freedesktop.org/series/37598/ will fix long freezes due
> > to frame counter resets.
> > 
> > This series has three more fixes -
> > Patch 1 eliminates PSR exit for flips and makes us rely on the HW to do it.
> > Patch 2 fixes cusor move lag by relying on HW to exit PSR.
> > Patch 3 fixes temporary freeze seen with fbdev.
> > 
> > With both the series applied, PSR on my SKL ThinkPad feels pretty good.
> 
> Thank you for your great work on this.
> 
> Are there any more PSR fixes in the pipeline?

Yeah, there are a few more fixes that I hope will appear on the list in
the next two weeks or so.

> If not I think I should do
> a custom Fedora kernel build based on 4.15 + recent fixes and ask all my
> testers to retest with that.
> 
> I do have some questions before I do this:
> 
> 1) I believe that only testers with skylake (normal or LP) or newer should
> re-test, correct?

These fixes do apply for HSW/BDW, so essentially all the big cores
supporting PSR. But, HSW/BDW need fixes for AUX channel-PSR interaction
also. I haven't looked into CHV/VLV.

> 
> 2) I know there are 2 series (including this one), can someone provide a link
> to the latest patchwork version of those 2 series, or even better a git
> branch with 4.15 + those patches? Any patches I'm missing if I pick up these
> 2 series?

https://patchwork.freedesktop.org/series/37598/
https://patchwork.freedesktop.org/series/38067/


> 3) I'm thinking 4.15 atm, but I could also do a 4.16-rc1 test kernel instead
> if that would be better, would that be better ?

I can't think of any diff that would affect PSR, but the latest is
better I suppose.


> 
> Regards,
> 
> Hans
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Dhinakaran Pandiyan]

DRM_IOCTL_MODE_CURSOR results in a frontbuffer flush before the cursor
plane MMIOs are written to. But this flush is not necessary for PSR as
hardware tracking takes care of exiting PSR when the MMIO's are written.

Introduce a new fb_op_origin enum to differentiate flushes due to a BO
being pinned from those originating due to a dirty fbdev buffer. Now, this
enum can be ignored in psr_flush and psr_invalidate.

v2: Update comment in i915_gem_object_pin_to_display_plane. (Chris)

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/i915_gem.c  | 11 +++++++++--
 drivers/gpu/drm/i915/intel_psr.c |  6 ++++--
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 81886b74c750..3bf6c6ec0509 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -637,6 +637,7 @@ enum fb_op_origin {
 	ORIGIN_CS,
 	ORIGIN_FLIP,
 	ORIGIN_DIRTYFB,
+	ORIGIN_PINNEDFB,
 };
 
 struct intel_fbc {
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fc68b35854df..405acf3562de 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4139,9 +4139,16 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
 
 	vma->display_alignment = max_t(u64, vma->display_alignment, alignment);
 
-	/* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
+	/* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() to
+	 * flush the caches.
+	 */
 	__i915_gem_object_flush_for_display(obj);
-	intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
+
+	/* Features like PSR might want to rely on HW to do the frontbuffer
+	 * flush, pass origin as ORIGIN_PINNEDFB rather than ORIGIN_DIRTYFB
+	 * so that their flush implementations can handle it accordingly.
+	 */
+	intel_fb_obj_flush(obj, ORIGIN_PINNEDFB);
 
 	/* It should now be out of any other write domains, and we can update
 	 * the domain values for our changes.
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 2a31c7cbdb41..ddfabdff3dea 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -842,7 +842,8 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
 	if (!CAN_PSR(dev_priv))
 		return;
 
-	if (dev_priv->psr.has_hw_tracking && origin == ORIGIN_FLIP)
+	if (dev_priv->psr.has_hw_tracking &&
+	    (origin == ORIGIN_FLIP || origin == ORIGIN_PINNEDFB))
 		return;
 
 	mutex_lock(&dev_priv->psr.lock);
@@ -885,7 +886,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 	if (!CAN_PSR(dev_priv))
 		return;
 
-	if (dev_priv->psr.has_hw_tracking && origin == ORIGIN_FLIP)
+	if (dev_priv->psr.has_hw_tracking &&
+	    (origin == ORIGIN_FLIP || origin == ORIGIN_PINNEDFB))
 		return;
 
 	mutex_lock(&dev_priv->psr.lock);
-- 
2.14.1

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

Re: [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Chris Wilson]

Quoting Dhinakaran Pandiyan (2018-02-13 21:46:13)
> DRM_IOCTL_MODE_CURSOR results in a frontbuffer flush before the cursor
> plane MMIOs are written to. But this flush is not necessary for PSR as
> hardware tracking takes care of exiting PSR when the MMIO's are written.
> 
> Introduce a new fb_op_origin enum to differentiate flushes due to a BO
> being pinned from those originating due to a dirty fbdev buffer. Now, this
> enum can be ignored in psr_flush and psr_invalidate.
> 
> v2: Update comment in i915_gem_object_pin_to_display_plane. (Chris)
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/i915_gem.c  | 11 +++++++++--
>  drivers/gpu/drm/i915/intel_psr.c |  6 ++++--
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 81886b74c750..3bf6c6ec0509 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -637,6 +637,7 @@ enum fb_op_origin {
>         ORIGIN_CS,
>         ORIGIN_FLIP,
>         ORIGIN_DIRTYFB,
> +       ORIGIN_PINNEDFB,
>  };
>  
>  struct intel_fbc {
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index fc68b35854df..405acf3562de 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4139,9 +4139,16 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
>  
>         vma->display_alignment = max_t(u64, vma->display_alignment, alignment);
>  
> -       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
> +       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() to
> +        * flush the caches.
> +        */
>         __i915_gem_object_flush_for_display(obj);
> -       intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
> +
> +       /* Features like PSR might want to rely on HW to do the frontbuffer
> +        * flush, pass origin as ORIGIN_PINNEDFB rather than ORIGIN_DIRTYFB
> +        * so that their flush implementations can handle it accordingly.
> +        */

So why it is different? Why can't the dirtyfb ioctl benefit from HW, which the
application is meant to call every frame to *all* dirty framebuffers
(which include cursors in atomic)?

> +       intel_fb_obj_flush(obj, ORIGIN_PINNEDFB);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Pandiyan, Dhinakaran]

On Tue, 2018-02-13 at 21:54 +0000, Chris Wilson wrote:
> Quoting Dhinakaran Pandiyan (2018-02-13 21:46:13)
> > DRM_IOCTL_MODE_CURSOR results in a frontbuffer flush before the cursor
> > plane MMIOs are written to. But this flush is not necessary for PSR as
> > hardware tracking takes care of exiting PSR when the MMIO's are written.
> > 
> > Introduce a new fb_op_origin enum to differentiate flushes due to a BO
> > being pinned from those originating due to a dirty fbdev buffer. Now, this
> > enum can be ignored in psr_flush and psr_invalidate.
> > 
> > v2: Update comment in i915_gem_object_pin_to_display_plane. (Chris)
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> >  drivers/gpu/drm/i915/i915_gem.c  | 11 +++++++++--
> >  drivers/gpu/drm/i915/intel_psr.c |  6 ++++--
> >  3 files changed, 14 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 81886b74c750..3bf6c6ec0509 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -637,6 +637,7 @@ enum fb_op_origin {
> >         ORIGIN_CS,
> >         ORIGIN_FLIP,
> >         ORIGIN_DIRTYFB,
> > +       ORIGIN_PINNEDFB,
> >  };
> >  
> >  struct intel_fbc {
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index fc68b35854df..405acf3562de 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4139,9 +4139,16 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> >  
> >         vma->display_alignment = max_t(u64, vma->display_alignment, alignment);
> >  
> > -       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
> > +       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() to
> > +        * flush the caches.
> > +        */
> >         __i915_gem_object_flush_for_display(obj);
> > -       intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
> > +
> > +       /* Features like PSR might want to rely on HW to do the frontbuffer
> > +        * flush, pass origin as ORIGIN_PINNEDFB rather than ORIGIN_DIRTYFB
> > +        * so that their flush implementations can handle it accordingly.
> > +        */
> 
> So why it is different? Why can't the dirtyfb ioctl benefit from HW, which the
> application is meant to call every frame to *all* dirty framebuffers
> (which include cursors in atomic)?
> 

Because the hardware requires a write to one of the pipe registers. When
applications write to the buffer via fbdev, it doesn't lead to pipe MMIO
write and hence does not benefit from HW triggered PSR exit.




> > +       intel_fb_obj_flush(obj, ORIGIN_PINNEDFB);
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Chris Wilson]

Quoting Pandiyan, Dhinakaran (2018-02-13 22:10:48)
> 
> 
> 
> On Tue, 2018-02-13 at 21:54 +0000, Chris Wilson wrote:
> > Quoting Dhinakaran Pandiyan (2018-02-13 21:46:13)
> > > DRM_IOCTL_MODE_CURSOR results in a frontbuffer flush before the cursor
> > > plane MMIOs are written to. But this flush is not necessary for PSR as
> > > hardware tracking takes care of exiting PSR when the MMIO's are written.
> > > 
> > > Introduce a new fb_op_origin enum to differentiate flushes due to a BO
> > > being pinned from those originating due to a dirty fbdev buffer. Now, this
> > > enum can be ignored in psr_flush and psr_invalidate.
> > > 
> > > v2: Update comment in i915_gem_object_pin_to_display_plane. (Chris)
> > > 
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> > >  drivers/gpu/drm/i915/i915_gem.c  | 11 +++++++++--
> > >  drivers/gpu/drm/i915/intel_psr.c |  6 ++++--
> > >  3 files changed, 14 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > index 81886b74c750..3bf6c6ec0509 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -637,6 +637,7 @@ enum fb_op_origin {
> > >         ORIGIN_CS,
> > >         ORIGIN_FLIP,
> > >         ORIGIN_DIRTYFB,
> > > +       ORIGIN_PINNEDFB,
> > >  };
> > >  
> > >  struct intel_fbc {
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > index fc68b35854df..405acf3562de 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -4139,9 +4139,16 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> > >  
> > >         vma->display_alignment = max_t(u64, vma->display_alignment, alignment);
> > >  
> > > -       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
> > > +       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() to
> > > +        * flush the caches.
> > > +        */
> > >         __i915_gem_object_flush_for_display(obj);
> > > -       intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
> > > +
> > > +       /* Features like PSR might want to rely on HW to do the frontbuffer
> > > +        * flush, pass origin as ORIGIN_PINNEDFB rather than ORIGIN_DIRTYFB
> > > +        * so that their flush implementations can handle it accordingly.
> > > +        */
> > 
> > So why it is different? Why can't the dirtyfb ioctl benefit from HW, which the
> > application is meant to call every frame to *all* dirty framebuffers
> > (which include cursors in atomic)?
> > 
> 
> Because the hardware requires a write to one of the pipe registers. When
> applications write to the buffer via fbdev, it doesn't lead to pipe MMIO
> write and hence does not benefit from HW triggered PSR exit.

Somewhere you have to have that explanation, that you rely on a
subsequent mmioflip of the framebuffer to trigger the frontbuffer flush.
That probably also deserves lifting out of pin_to_display_plane as
currently there's no requirement that pin_to_display is followed by a
flip.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for PSR lag fixes (rev2)

[Patchwork]

== Series Details ==

Series: PSR lag fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/38067/
State : failure

== Summary ==

Series 38067v2 PSR lag fixes
https://patchwork.freedesktop.org/api/1.0/series/38067/revisions/2/mbox/

Test kms_busy:
        Subgroup basic-flip-b:
                pass       -> INCOMPLETE (fi-cnl-y3)

fi-bdw-5557u     total:288  pass:265  dwarn:0   dfail:0   fail:2   skip:21  time:441s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:430s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:378s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:491s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:293s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:486s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:474s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:461s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:569s
fi-cnl-y3        total:197  pass:185  dwarn:0   dfail:0   fail:0   skip:11 
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:424s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:296s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:513s
fi-hsw-4770      total:288  pass:259  dwarn:0   dfail:0   fail:2   skip:27  time:411s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:413s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:415s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:462s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:496s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:501s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:601s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:430s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:506s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:528s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:495s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:483s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:418s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:433s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:541s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:400s
Blacklisted hosts:
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:469s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:457s

55dfdd18a566b5c632cbb34086b2a03410e810fb drm-tip: 2018y-02m-13d-18h-37m-11s UTC integration manifest
558a92e3776e drm/i915/psr: Wait for PSR transition to complete before exiting.
9d920a6e7906 drm/i915/psr: HW tracking for cursor moves to fix lags.
085f2a67eebd drm/i915/psr: Use more PSR HW tracking.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8000/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Pandiyan, Dhinakaran]

On Tue, 2018-02-13 at 22:15 +0000, Chris Wilson wrote:
> Quoting Pandiyan, Dhinakaran (2018-02-13 22:10:48)
> > 
> > 
> > 
> > On Tue, 2018-02-13 at 21:54 +0000, Chris Wilson wrote:
> > > Quoting Dhinakaran Pandiyan (2018-02-13 21:46:13)
> > > > DRM_IOCTL_MODE_CURSOR results in a frontbuffer flush before the cursor
> > > > plane MMIOs are written to. But this flush is not necessary for PSR as
> > > > hardware tracking takes care of exiting PSR when the MMIO's are written.
> > > > 
> > > > Introduce a new fb_op_origin enum to differentiate flushes due to a BO
> > > > being pinned from those originating due to a dirty fbdev buffer. Now, this
> > > > enum can be ignored in psr_flush and psr_invalidate.
> > > > 
> > > > v2: Update comment in i915_gem_object_pin_to_display_plane. (Chris)
> > > > 
> > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> > > >  drivers/gpu/drm/i915/i915_gem.c  | 11 +++++++++--
> > > >  drivers/gpu/drm/i915/intel_psr.c |  6 ++++--
> > > >  3 files changed, 14 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > index 81886b74c750..3bf6c6ec0509 100644
> > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > @@ -637,6 +637,7 @@ enum fb_op_origin {
> > > >         ORIGIN_CS,
> > > >         ORIGIN_FLIP,
> > > >         ORIGIN_DIRTYFB,
> > > > +       ORIGIN_PINNEDFB,
> > > >  };
> > > >  
> > > >  struct intel_fbc {
> > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > > index fc68b35854df..405acf3562de 100644
> > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > @@ -4139,9 +4139,16 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> > > >  
> > > >         vma->display_alignment = max_t(u64, vma->display_alignment, alignment);
> > > >  
> > > > -       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
> > > > +       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() to
> > > > +        * flush the caches.
> > > > +        */
> > > >         __i915_gem_object_flush_for_display(obj);
> > > > -       intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
> > > > +
> > > > +       /* Features like PSR might want to rely on HW to do the frontbuffer
> > > > +        * flush, pass origin as ORIGIN_PINNEDFB rather than ORIGIN_DIRTYFB
> > > > +        * so that their flush implementations can handle it accordingly.
> > > > +        */
> > > 
> > > So why it is different? Why can't the dirtyfb ioctl benefit from HW, which the
> > > application is meant to call every frame to *all* dirty framebuffers
> > > (which include cursors in atomic)?
> > > 
> > 
> > Because the hardware requires a write to one of the pipe registers. When
> > applications write to the buffer via fbdev, it doesn't lead to pipe MMIO
> > write and hence does not benefit from HW triggered PSR exit.
> 
> Somewhere you have to have that explanation, that you rely on a
> subsequent mmioflip of the framebuffer to trigger the frontbuffer flush.


diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index 405acf3562de..c669fef5d388 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4071,9 +4071,13 @@ int i915_gem_set_caching_ioctl(struct drm_device
*dev, void *data,
 }

 /*
- * Prepare buffer for display plane (scanout, cursors, etc).
- * Can be called from an uninterruptible phase (modesetting) and allows
- * any flushes to be pipelined (for pageflips).
+ * Prepare buffer for display plane (scanout, cursors, etc). Can be
called from
+ * an uninterruptible phase (modesetting) and allows any flushes to be
pipelined
+ * (for pageflips). We only flush the caches while preparing the buffer
for
+ * display and this may not lead to the buffer being scanned out if
+ * intel_fb_obj_flush() consumers ignore ORIGIN_PINNEDFB. This is
useful because
+ * features like PSR can delegate the flush to HW, which gets triggered
when
+ * flip or cursor move MMIO's are written to.
  */
 struct i915_vma *
 i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,


Like this?



> That probably also deserves lifting out of pin_to_display_plane as
> currently there's no requirement that pin_to_display is followed by a
> flip.

Does pin_to_display imply ready to scan out? And I didn't get the part
about "lifting out of pin_to_display_plane"



-DK


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

Re: [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Chris Wilson]

Quoting Pandiyan, Dhinakaran (2018-02-13 22:45:55)
> 
> 
> 
> On Tue, 2018-02-13 at 22:15 +0000, Chris Wilson wrote:
> > Quoting Pandiyan, Dhinakaran (2018-02-13 22:10:48)
> > > 
> > > 
> > > 
> > > On Tue, 2018-02-13 at 21:54 +0000, Chris Wilson wrote:
> > > > Quoting Dhinakaran Pandiyan (2018-02-13 21:46:13)
> > > > > DRM_IOCTL_MODE_CURSOR results in a frontbuffer flush before the cursor
> > > > > plane MMIOs are written to. But this flush is not necessary for PSR as
> > > > > hardware tracking takes care of exiting PSR when the MMIO's are written.
> > > > > 
> > > > > Introduce a new fb_op_origin enum to differentiate flushes due to a BO
> > > > > being pinned from those originating due to a dirty fbdev buffer. Now, this
> > > > > enum can be ignored in psr_flush and psr_invalidate.
> > > > > 
> > > > > v2: Update comment in i915_gem_object_pin_to_display_plane. (Chris)
> > > > > 
> > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> > > > >  drivers/gpu/drm/i915/i915_gem.c  | 11 +++++++++--
> > > > >  drivers/gpu/drm/i915/intel_psr.c |  6 ++++--
> > > > >  3 files changed, 14 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > > index 81886b74c750..3bf6c6ec0509 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > @@ -637,6 +637,7 @@ enum fb_op_origin {
> > > > >         ORIGIN_CS,
> > > > >         ORIGIN_FLIP,
> > > > >         ORIGIN_DIRTYFB,
> > > > > +       ORIGIN_PINNEDFB,
> > > > >  };
> > > > >  
> > > > >  struct intel_fbc {
> > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > > > index fc68b35854df..405acf3562de 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > > @@ -4139,9 +4139,16 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> > > > >  
> > > > >         vma->display_alignment = max_t(u64, vma->display_alignment, alignment);
> > > > >  
> > > > > -       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
> > > > > +       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() to
> > > > > +        * flush the caches.
> > > > > +        */
> > > > >         __i915_gem_object_flush_for_display(obj);
> > > > > -       intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
> > > > > +
> > > > > +       /* Features like PSR might want to rely on HW to do the frontbuffer
> > > > > +        * flush, pass origin as ORIGIN_PINNEDFB rather than ORIGIN_DIRTYFB
> > > > > +        * so that their flush implementations can handle it accordingly.
> > > > > +        */
> > > > 
> > > > So why it is different? Why can't the dirtyfb ioctl benefit from HW, which the
> > > > application is meant to call every frame to *all* dirty framebuffers
> > > > (which include cursors in atomic)?
> > > > 
> > > 
> > > Because the hardware requires a write to one of the pipe registers. When
> > > applications write to the buffer via fbdev, it doesn't lead to pipe MMIO
> > > write and hence does not benefit from HW triggered PSR exit.
> > 
> > Somewhere you have to have that explanation, that you rely on a
> > subsequent mmioflip of the framebuffer to trigger the frontbuffer flush.
> 
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c
> b/drivers/gpu/drm/i915/i915_gem.c
> index 405acf3562de..c669fef5d388 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4071,9 +4071,13 @@ int i915_gem_set_caching_ioctl(struct drm_device
> *dev, void *data,
>  }
> 
>  /*
> - * Prepare buffer for display plane (scanout, cursors, etc).
> - * Can be called from an uninterruptible phase (modesetting) and allows
> - * any flushes to be pipelined (for pageflips).
> + * Prepare buffer for display plane (scanout, cursors, etc). Can be
> called from
> + * an uninterruptible phase (modesetting) and allows any flushes to be
> pipelined
> + * (for pageflips). We only flush the caches while preparing the buffer
> for
> + * display and this may not lead to the buffer being scanned out if
> + * intel_fb_obj_flush() consumers ignore ORIGIN_PINNEDFB. This is
> useful because
> + * features like PSR can delegate the flush to HW, which gets triggered
> when
> + * flip or cursor move MMIO's are written to.
>   */
>  struct i915_vma *
>  i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> 
> 
> Like this?
> 
> 
> 
> > That probably also deserves lifting out of pin_to_display_plane as
> > currently there's no requirement that pin_to_display is followed by a
> > flip.
> 
> Does pin_to_display imply ready to scan out? And I didn't get the part
> about "lifting out of pin_to_display_plane"

Nothing about flushing the GEM caches for coherency with the display 
engine imply how it will be used. Since PINNEDFB only works if the
caller follows it with a mmioflip, it should be moved to the caller or
that information passed in.

So how is PINNEDFB different from FLIP? Once you move it to the callers,
doesn't it just reduce to FLIP in the cases where you want to treat it
as FLIP and DIRTYFB in the others?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2] drm/i915/psr: HW tracking for cursor moves to fix lags.

[Pandiyan, Dhinakaran]

On Tue, 2018-02-13 at 22:59 +0000, Chris Wilson wrote:
> Quoting Pandiyan, Dhinakaran (2018-02-13 22:45:55)
> > 
> > 
> > 
> > On Tue, 2018-02-13 at 22:15 +0000, Chris Wilson wrote:
> > > Quoting Pandiyan, Dhinakaran (2018-02-13 22:10:48)
> > > > 
> > > > 
> > > > 
> > > > On Tue, 2018-02-13 at 21:54 +0000, Chris Wilson wrote:
> > > > > Quoting Dhinakaran Pandiyan (2018-02-13 21:46:13)
> > > > > > DRM_IOCTL_MODE_CURSOR results in a frontbuffer flush before the cursor
> > > > > > plane MMIOs are written to. But this flush is not necessary for PSR as
> > > > > > hardware tracking takes care of exiting PSR when the MMIO's are written.
> > > > > > 
> > > > > > Introduce a new fb_op_origin enum to differentiate flushes due to a BO
> > > > > > being pinned from those originating due to a dirty fbdev buffer. Now, this
> > > > > > enum can be ignored in psr_flush and psr_invalidate.
> > > > > > 
> > > > > > v2: Update comment in i915_gem_object_pin_to_display_plane. (Chris)
> > > > > > 
> > > > > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> > > > > >  drivers/gpu/drm/i915/i915_gem.c  | 11 +++++++++--
> > > > > >  drivers/gpu/drm/i915/intel_psr.c |  6 ++++--
> > > > > >  3 files changed, 14 insertions(+), 4 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > index 81886b74c750..3bf6c6ec0509 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > > > > @@ -637,6 +637,7 @@ enum fb_op_origin {
> > > > > >         ORIGIN_CS,
> > > > > >         ORIGIN_FLIP,
> > > > > >         ORIGIN_DIRTYFB,
> > > > > > +       ORIGIN_PINNEDFB,
> > > > > >  };
> > > > > >  
> > > > > >  struct intel_fbc {
> > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > > > > > index fc68b35854df..405acf3562de 100644
> > > > > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > > > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > > > > @@ -4139,9 +4139,16 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> > > > > >  
> > > > > >         vma->display_alignment = max_t(u64, vma->display_alignment, alignment);
> > > > > >  
> > > > > > -       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() */
> > > > > > +       /* Treat this as an end-of-frame, like intel_user_framebuffer_dirty() to
> > > > > > +        * flush the caches.
> > > > > > +        */
> > > > > >         __i915_gem_object_flush_for_display(obj);
> > > > > > -       intel_fb_obj_flush(obj, ORIGIN_DIRTYFB);
> > > > > > +
> > > > > > +       /* Features like PSR might want to rely on HW to do the frontbuffer
> > > > > > +        * flush, pass origin as ORIGIN_PINNEDFB rather than ORIGIN_DIRTYFB
> > > > > > +        * so that their flush implementations can handle it accordingly.
> > > > > > +        */
> > > > > 
> > > > > So why it is different? Why can't the dirtyfb ioctl benefit from HW, which the
> > > > > application is meant to call every frame to *all* dirty framebuffers
> > > > > (which include cursors in atomic)?
> > > > > 
> > > > 
> > > > Because the hardware requires a write to one of the pipe registers. When
> > > > applications write to the buffer via fbdev, it doesn't lead to pipe MMIO
> > > > write and hence does not benefit from HW triggered PSR exit.
> > > 
> > > Somewhere you have to have that explanation, that you rely on a
> > > subsequent mmioflip of the framebuffer to trigger the frontbuffer flush.
> > 
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 405acf3562de..c669fef5d388 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -4071,9 +4071,13 @@ int i915_gem_set_caching_ioctl(struct drm_device
> > *dev, void *data,
> >  }
> > 
> >  /*
> > - * Prepare buffer for display plane (scanout, cursors, etc).
> > - * Can be called from an uninterruptible phase (modesetting) and allows
> > - * any flushes to be pipelined (for pageflips).
> > + * Prepare buffer for display plane (scanout, cursors, etc). Can be
> > called from
> > + * an uninterruptible phase (modesetting) and allows any flushes to be
> > pipelined
> > + * (for pageflips). We only flush the caches while preparing the buffer
> > for
> > + * display and this may not lead to the buffer being scanned out if
> > + * intel_fb_obj_flush() consumers ignore ORIGIN_PINNEDFB. This is
> > useful because
> > + * features like PSR can delegate the flush to HW, which gets triggered
> > when
> > + * flip or cursor move MMIO's are written to.
> >   */
> >  struct i915_vma *
> >  i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
> > 
> > 
> > Like this?
> > 
> > 
> > 
> > > That probably also deserves lifting out of pin_to_display_plane as
> > > currently there's no requirement that pin_to_display is followed by a
> > > flip.
> > 
> > Does pin_to_display imply ready to scan out? And I didn't get the part
> > about "lifting out of pin_to_display_plane"
> 
> Nothing about flushing the GEM caches for coherency with the display 
> engine imply how it will be used. Since PINNEDFB only works if the
> caller follows it with a mmioflip, it should be moved to the caller or
> that information passed in.
> 
> So how is PINNEDFB different from FLIP? Once you move it to the callers,
> doesn't it just reduce to FLIP in the cases where you want to treat it
> as FLIP and DIRTYFB in the others?

Good point, let me revisit the idea of reusing FLIP now that I have a
slightly better understanding of how this is supposed to work.

-DK



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

Re: [PATCH 0/3] PSR lag fixes

[Hans de Goede]

Hi,

On 12-02-18 18:42, Pandiyan, Dhinakaran wrote:
> On Mon, 2018-02-12 at 09:45 +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 12-02-18 07:08, Dhinakaran Pandiyan wrote:
>>> PSR currently when enabled results in semi-permanent freezes or noticeable
>>> cursor lags.
>>>
>>> https://patchwork.freedesktop.org/series/37598/ will fix long freezes due
>>> to frame counter resets.
>>>
>>> This series has three more fixes -
>>> Patch 1 eliminates PSR exit for flips and makes us rely on the HW to do it.
>>> Patch 2 fixes cusor move lag by relying on HW to exit PSR.
>>> Patch 3 fixes temporary freeze seen with fbdev.
>>>
>>> With both the series applied, PSR on my SKL ThinkPad feels pretty good.
>>
>> Thank you for your great work on this.
>>
>> Are there any more PSR fixes in the pipeline?
> 
> Yeah, there are a few more fixes that I hope will appear on the list in
> the next two weeks or so.

Ok, can you send a mail when you're done (in sofar any software is ever
"done") and you would like me to ask all people who have been kind enough
to test PSR to retest ?

>> If not I think I should do
>> a custom Fedora kernel build based on 4.15 + recent fixes and ask all my
>> testers to retest with that.
>>
>> I do have some questions before I do this:
>>
>> 1) I believe that only testers with skylake (normal or LP) or newer should
>> re-test, correct?
> 
> These fixes do apply for HSW/BDW, so essentially all the big cores
> supporting PSR. But, HSW/BDW need fixes for AUX channel-PSR interaction
> also. I haven't looked into CHV/VLV.
> 
>>
>> 2) I know there are 2 series (including this one), can someone provide a link
>> to the latest patchwork version of those 2 series, or even better a git
>> branch with 4.15 + those patches? Any patches I'm missing if I pick up these
>> 2 series?
> 
> https://patchwork.freedesktop.org/series/37598/
> https://patchwork.freedesktop.org/series/38067/
> 
> 
>> 3) I'm thinking 4.15 atm, but I could also do a 4.16-rc1 test kernel instead
>> if that would be better, would that be better ?
> 
> I can't think of any diff that would affect PSR, but the latest is
> better I suppose.

Ok.

Regards,

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

Re: [PATCH 0/3] PSR lag fixes

[Pandiyan, Dhinakaran]

On Wed, 2018-02-14 at 09:25 +0100, Hans de Goede wrote:
> Hi,
> 
> On 12-02-18 18:42, Pandiyan, Dhinakaran wrote:
> > On Mon, 2018-02-12 at 09:45 +0100, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 12-02-18 07:08, Dhinakaran Pandiyan wrote:
> >>> PSR currently when enabled results in semi-permanent freezes or noticeable
> >>> cursor lags.
> >>>
> >>> https://patchwork.freedesktop.org/series/37598/ will fix long freezes due
> >>> to frame counter resets.
> >>>
> >>> This series has three more fixes -
> >>> Patch 1 eliminates PSR exit for flips and makes us rely on the HW to do it.
> >>> Patch 2 fixes cusor move lag by relying on HW to exit PSR.
> >>> Patch 3 fixes temporary freeze seen with fbdev.
> >>>
> >>> With both the series applied, PSR on my SKL ThinkPad feels pretty good.
> >>
> >> Thank you for your great work on this.
> >>
> >> Are there any more PSR fixes in the pipeline?
> > 
> > Yeah, there are a few more fixes that I hope will appear on the list in
> > the next two weeks or so.
> 
> Ok, can you send a mail when you're done (in sofar any software is ever
> "done") and you would like me to ask all people who have been kind enough
> to test PSR to retest ?
> 

Hi Hans,

Thanks for your patience and help. I believe the current drm-tip is in a
decent shape to retest PSR. Booting with i915.enable_psr = 1 is still
needed. The fixes have been mostly developed/tested on gen-9 hardware
but they apply to other platforms too.

-DK

> >> If not I think I should do
> >> a custom Fedora kernel build based on 4.15 + recent fixes and ask all my
> >> testers to retest with that.
> >>
> >> I do have some questions before I do this:
> >>
> >> 1) I believe that only testers with skylake (normal or LP) or newer should
> >> re-test, correct?
> > 
> > These fixes do apply for HSW/BDW, so essentially all the big cores
> > supporting PSR. But, HSW/BDW need fixes for AUX channel-PSR interaction
> > also. I haven't looked into CHV/VLV.
> > 
> >>
> >> 2) I know there are 2 series (including this one), can someone provide a link
> >> to the latest patchwork version of those 2 series, or even better a git
> >> branch with 4.15 + those patches? Any patches I'm missing if I pick up these
> >> 2 series?
> > 
> > https://patchwork.freedesktop.org/series/37598/
> > https://patchwork.freedesktop.org/series/38067/
> > 
> > 
> >> 3) I'm thinking 4.15 atm, but I could also do a 4.16-rc1 test kernel instead
> >> if that would be better, would that be better ?
> > 
> > I can't think of any diff that would affect PSR, but the latest is
> > better I suppose.
> 
> Ok.
> 
> Regards,
> 
> Hans
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] PSR lag fixes

[Hans de Goede]

Hi,

On 14-03-18 21:49, Pandiyan, Dhinakaran wrote:
> 
> On Wed, 2018-02-14 at 09:25 +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 12-02-18 18:42, Pandiyan, Dhinakaran wrote:
>>> On Mon, 2018-02-12 at 09:45 +0100, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 12-02-18 07:08, Dhinakaran Pandiyan wrote:
>>>>> PSR currently when enabled results in semi-permanent freezes or noticeable
>>>>> cursor lags.
>>>>>
>>>>> https://patchwork.freedesktop.org/series/37598/ will fix long freezes due
>>>>> to frame counter resets.
>>>>>
>>>>> This series has three more fixes -
>>>>> Patch 1 eliminates PSR exit for flips and makes us rely on the HW to do it.
>>>>> Patch 2 fixes cusor move lag by relying on HW to exit PSR.
>>>>> Patch 3 fixes temporary freeze seen with fbdev.
>>>>>
>>>>> With both the series applied, PSR on my SKL ThinkPad feels pretty good.
>>>>
>>>> Thank you for your great work on this.
>>>>
>>>> Are there any more PSR fixes in the pipeline?
>>>
>>> Yeah, there are a few more fixes that I hope will appear on the list in
>>> the next two weeks or so.
>>
>> Ok, can you send a mail when you're done (in sofar any software is ever
>> "done") and you would like me to ask all people who have been kind enough
>> to test PSR to retest ?
>>
> 
> Hi Hans,
> 
> Thanks for your patience and help. I believe the current drm-tip is in a
> decent shape to retest PSR. Booting with i915.enable_psr = 1 is still
> needed. The fixes have been mostly developed/tested on gen-9 hardware
> but they apply to other platforms too.

Cool, thank you. Current drm-tip will all be merged into 4.17-rc1, right?

Then I think I will just wait for that, most distros already provide rc builds
for testing, so that way it will be easy for people to test.

Regards,

Hans




> 
> -DK
> 
>>>> If not I think I should do
>>>> a custom Fedora kernel build based on 4.15 + recent fixes and ask all my
>>>> testers to retest with that.
>>>>
>>>> I do have some questions before I do this:
>>>>
>>>> 1) I believe that only testers with skylake (normal or LP) or newer should
>>>> re-test, correct?
>>>
>>> These fixes do apply for HSW/BDW, so essentially all the big cores
>>> supporting PSR. But, HSW/BDW need fixes for AUX channel-PSR interaction
>>> also. I haven't looked into CHV/VLV.
>>>
>>>>
>>>> 2) I know there are 2 series (including this one), can someone provide a link
>>>> to the latest patchwork version of those 2 series, or even better a git
>>>> branch with 4.15 + those patches? Any patches I'm missing if I pick up these
>>>> 2 series?
>>>
>>> https://patchwork.freedesktop.org/series/37598/
>>> https://patchwork.freedesktop.org/series/38067/
>>>
>>>
>>>> 3) I'm thinking 4.15 atm, but I could also do a 4.16-rc1 test kernel instead
>>>> if that would be better, would that be better ?
>>>
>>> I can't think of any diff that would affect PSR, but the latest is
>>> better I suppose.
>>
>> Ok.
>>
>> Regards,
>>
>> Hans
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] PSR lag fixes

[Pandiyan, Dhinakaran]

On Wed, 2018-03-14 at 23:09 +0100, Hans de Goede wrote:
> Hi,
> 
> On 14-03-18 21:49, Pandiyan, Dhinakaran wrote:
> > 
> > On Wed, 2018-02-14 at 09:25 +0100, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 12-02-18 18:42, Pandiyan, Dhinakaran wrote:
> >>> On Mon, 2018-02-12 at 09:45 +0100, Hans de Goede wrote:
> >>>> Hi,
> >>>>
> >>>> On 12-02-18 07:08, Dhinakaran Pandiyan wrote:
> >>>>> PSR currently when enabled results in semi-permanent freezes or noticeable
> >>>>> cursor lags.
> >>>>>
> >>>>> https://patchwork.freedesktop.org/series/37598/ will fix long freezes due
> >>>>> to frame counter resets.
> >>>>>
> >>>>> This series has three more fixes -
> >>>>> Patch 1 eliminates PSR exit for flips and makes us rely on the HW to do it.
> >>>>> Patch 2 fixes cusor move lag by relying on HW to exit PSR.
> >>>>> Patch 3 fixes temporary freeze seen with fbdev.
> >>>>>
> >>>>> With both the series applied, PSR on my SKL ThinkPad feels pretty good.
> >>>>
> >>>> Thank you for your great work on this.
> >>>>
> >>>> Are there any more PSR fixes in the pipeline?
> >>>
> >>> Yeah, there are a few more fixes that I hope will appear on the list in
> >>> the next two weeks or so.
> >>
> >> Ok, can you send a mail when you're done (in sofar any software is ever
> >> "done") and you would like me to ask all people who have been kind enough
> >> to test PSR to retest ?
> >>
> > 
> > Hi Hans,
> > 
> > Thanks for your patience and help. I believe the current drm-tip is in a
> > decent shape to retest PSR. Booting with i915.enable_psr = 1 is still
> > needed. The fixes have been mostly developed/tested on gen-9 hardware
> > but they apply to other platforms too.
> 
> Cool, thank you. Current drm-tip will all be merged into 4.17-rc1, right?

Rodrigo,

Can you help me answer that?


> 
> Then I think I will just wait for that, most distros already provide rc builds
> for testing, so that way it will be easy for people to test.
> 
> Regards,
> 
> Hans
> 
> 
> 
> 
> > 
> > -DK
> > 
> >>>> If not I think I should do
> >>>> a custom Fedora kernel build based on 4.15 + recent fixes and ask all my
> >>>> testers to retest with that.
> >>>>
> >>>> I do have some questions before I do this:
> >>>>
> >>>> 1) I believe that only testers with skylake (normal or LP) or newer should
> >>>> re-test, correct?
> >>>
> >>> These fixes do apply for HSW/BDW, so essentially all the big cores
> >>> supporting PSR. But, HSW/BDW need fixes for AUX channel-PSR interaction
> >>> also. I haven't looked into CHV/VLV.
> >>>
> >>>>
> >>>> 2) I know there are 2 series (including this one), can someone provide a link
> >>>> to the latest patchwork version of those 2 series, or even better a git
> >>>> branch with 4.15 + those patches? Any patches I'm missing if I pick up these
> >>>> 2 series?
> >>>
> >>> https://patchwork.freedesktop.org/series/37598/
> >>> https://patchwork.freedesktop.org/series/38067/
> >>>
> >>>
> >>>> 3) I'm thinking 4.15 atm, but I could also do a 4.16-rc1 test kernel instead
> >>>> if that would be better, would that be better ?
> >>>
> >>> I can't think of any diff that would affect PSR, but the latest is
> >>> better I suppose.
> >>
> >> Ok.
> >>
> >> Regards,
> >>
> >> Hans
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] PSR lag fixes

[Rodrigo Vivi]

On Wed, Mar 14, 2018 at 04:35:55PM -0700, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Wed, 2018-03-14 at 23:09 +0100, Hans de Goede wrote:
> > Hi,
> > 
> > On 14-03-18 21:49, Pandiyan, Dhinakaran wrote:
> > > 
> > > On Wed, 2018-02-14 at 09:25 +0100, Hans de Goede wrote:
> > >> Hi,
> > >>
> > >> On 12-02-18 18:42, Pandiyan, Dhinakaran wrote:
> > >>> On Mon, 2018-02-12 at 09:45 +0100, Hans de Goede wrote:
> > >>>> Hi,
> > >>>>
> > >>>> On 12-02-18 07:08, Dhinakaran Pandiyan wrote:
> > >>>>> PSR currently when enabled results in semi-permanent freezes or noticeable
> > >>>>> cursor lags.
> > >>>>>
> > >>>>> https://patchwork.freedesktop.org/series/37598/ will fix long freezes due
> > >>>>> to frame counter resets.
> > >>>>>
> > >>>>> This series has three more fixes -
> > >>>>> Patch 1 eliminates PSR exit for flips and makes us rely on the HW to do it.
> > >>>>> Patch 2 fixes cusor move lag by relying on HW to exit PSR.
> > >>>>> Patch 3 fixes temporary freeze seen with fbdev.
> > >>>>>
> > >>>>> With both the series applied, PSR on my SKL ThinkPad feels pretty good.
> > >>>>
> > >>>> Thank you for your great work on this.
> > >>>>
> > >>>> Are there any more PSR fixes in the pipeline?
> > >>>
> > >>> Yeah, there are a few more fixes that I hope will appear on the list in
> > >>> the next two weeks or so.
> > >>
> > >> Ok, can you send a mail when you're done (in sofar any software is ever
> > >> "done") and you would like me to ask all people who have been kind enough
> > >> to test PSR to retest ?
> > >>
> > > 
> > > Hi Hans,
> > > 
> > > Thanks for your patience and help. I believe the current drm-tip is in a
> > > decent shape to retest PSR. Booting with i915.enable_psr = 1 is still
> > > needed. The fixes have been mostly developed/tested on gen-9 hardware
> > > but they apply to other platforms too.
> > 
> > Cool, thank you. Current drm-tip will all be merged into 4.17-rc1, right?

Unfortunately no. All changes current in dinq and not on drm-intel-next are only
going to 4.18-rc1.

> 
> Rodrigo,
> 
> Can you help me answer that?
> 
> 
> > 
> > Then I think I will just wait for that, most distros already provide rc builds
> > for testing, so that way it will be easy for people to test.
> > 
> > Regards,
> > 
> > Hans
> > 
> > 
> > 
> > 
> > > 
> > > -DK
> > > 
> > >>>> If not I think I should do
> > >>>> a custom Fedora kernel build based on 4.15 + recent fixes and ask all my
> > >>>> testers to retest with that.
> > >>>>
> > >>>> I do have some questions before I do this:
> > >>>>
> > >>>> 1) I believe that only testers with skylake (normal or LP) or newer should
> > >>>> re-test, correct?
> > >>>
> > >>> These fixes do apply for HSW/BDW, so essentially all the big cores
> > >>> supporting PSR. But, HSW/BDW need fixes for AUX channel-PSR interaction
> > >>> also. I haven't looked into CHV/VLV.
> > >>>
> > >>>>
> > >>>> 2) I know there are 2 series (including this one), can someone provide a link
> > >>>> to the latest patchwork version of those 2 series, or even better a git
> > >>>> branch with 4.15 + those patches? Any patches I'm missing if I pick up these
> > >>>> 2 series?
> > >>>
> > >>> https://patchwork.freedesktop.org/series/37598/
> > >>> https://patchwork.freedesktop.org/series/38067/
> > >>>
> > >>>
> > >>>> 3) I'm thinking 4.15 atm, but I could also do a 4.16-rc1 test kernel instead
> > >>>> if that would be better, would that be better ?
> > >>>
> > >>> I can't think of any diff that would affect PSR, but the latest is
> > >>> better I suppose.
> > >>
> > >> Ok.
> > >>
> > >> Regards,
> > >>
> > >> Hans
> > >> _______________________________________________
> > >> Intel-gfx mailing list
> > >> Intel-gfx@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/3] PSR lag fixes

[Hans de Goede]

Hi,

On 16-03-18 01:16, Rodrigo Vivi wrote:
> On Wed, Mar 14, 2018 at 04:35:55PM -0700, Pandiyan, Dhinakaran wrote:
>>
>>
>>
>> On Wed, 2018-03-14 at 23:09 +0100, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 14-03-18 21:49, Pandiyan, Dhinakaran wrote:
>>>>
>>>> On Wed, 2018-02-14 at 09:25 +0100, Hans de Goede wrote:
>>>>> Hi,
>>>>>
>>>>> On 12-02-18 18:42, Pandiyan, Dhinakaran wrote:
>>>>>> On Mon, 2018-02-12 at 09:45 +0100, Hans de Goede wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 12-02-18 07:08, Dhinakaran Pandiyan wrote:
>>>>>>>> PSR currently when enabled results in semi-permanent freezes or noticeable
>>>>>>>> cursor lags.
>>>>>>>>
>>>>>>>> https://patchwork.freedesktop.org/series/37598/ will fix long freezes due
>>>>>>>> to frame counter resets.
>>>>>>>>
>>>>>>>> This series has three more fixes -
>>>>>>>> Patch 1 eliminates PSR exit for flips and makes us rely on the HW to do it.
>>>>>>>> Patch 2 fixes cusor move lag by relying on HW to exit PSR.
>>>>>>>> Patch 3 fixes temporary freeze seen with fbdev.
>>>>>>>>
>>>>>>>> With both the series applied, PSR on my SKL ThinkPad feels pretty good.
>>>>>>>
>>>>>>> Thank you for your great work on this.
>>>>>>>
>>>>>>> Are there any more PSR fixes in the pipeline?
>>>>>>
>>>>>> Yeah, there are a few more fixes that I hope will appear on the list in
>>>>>> the next two weeks or so.
>>>>>
>>>>> Ok, can you send a mail when you're done (in sofar any software is ever
>>>>> "done") and you would like me to ask all people who have been kind enough
>>>>> to test PSR to retest ?
>>>>>
>>>>
>>>> Hi Hans,
>>>>
>>>> Thanks for your patience and help. I believe the current drm-tip is in a
>>>> decent shape to retest PSR. Booting with i915.enable_psr = 1 is still
>>>> needed. The fixes have been mostly developed/tested on gen-9 hardware
>>>> but they apply to other platforms too.
>>>
>>> Cool, thank you. Current drm-tip will all be merged into 4.17-rc1, right?
> 
> Unfortunately no. All changes current in dinq and not on drm-intel-next are only
> going to 4.18-rc1.

Bummer.

Pandiyan, can you make a list of commits or patch-work series links
which contain the PSR related fixes? Then I will try to backport those
to 4.16 (when I find time for this).

Regards,

Hans





> 
>>
>> Rodrigo,
>>
>> Can you help me answer that?
>>
>>
>>>
>>> Then I think I will just wait for that, most distros already provide rc builds
>>> for testing, so that way it will be easy for people to test.
>>>
>>> Regards,
>>>
>>> Hans
>>>
>>>
>>>
>>>
>>>>
>>>> -DK
>>>>
>>>>>>> If not I think I should do
>>>>>>> a custom Fedora kernel build based on 4.15 + recent fixes and ask all my
>>>>>>> testers to retest with that.
>>>>>>>
>>>>>>> I do have some questions before I do this:
>>>>>>>
>>>>>>> 1) I believe that only testers with skylake (normal or LP) or newer should
>>>>>>> re-test, correct?
>>>>>>
>>>>>> These fixes do apply for HSW/BDW, so essentially all the big cores
>>>>>> supporting PSR. But, HSW/BDW need fixes for AUX channel-PSR interaction
>>>>>> also. I haven't looked into CHV/VLV.
>>>>>>
>>>>>>>
>>>>>>> 2) I know there are 2 series (including this one), can someone provide a link
>>>>>>> to the latest patchwork version of those 2 series, or even better a git
>>>>>>> branch with 4.15 + those patches? Any patches I'm missing if I pick up these
>>>>>>> 2 series?
>>>>>>
>>>>>> https://patchwork.freedesktop.org/series/37598/
>>>>>> https://patchwork.freedesktop.org/series/38067/
>>>>>>
>>>>>>
>>>>>>> 3) I'm thinking 4.15 atm, but I could also do a 4.16-rc1 test kernel instead
>>>>>>> if that would be better, would that be better ?
>>>>>>
>>>>>> I can't think of any diff that would affect PSR, but the latest is
>>>>>> better I suppose.
>>>>>
>>>>> Ok.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Hans
>>>>> _______________________________________________
>>>>> Intel-gfx mailing list
>>>>> Intel-gfx@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>> _______________________________________________
>>> Intel-gfx mailing list
>>> Intel-gfx@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx