[(resend) PATCH 1/2] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.

[(resend) PATCH 1/2] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.

[Maarten Lankhorst]

The watermarks it should calculate against are the old optimal watermarks.
The currently active crtc watermarks are pure fiction, and are invalid in
case of a nonblocking modeset, page flip enabling/disabling planes or any
other reason.

When the crtc is disabled or during a modeset the intermediate watermarks
don't need to be programmed separately, and could be directly assigned
to the optimal watermarks.

CXSR must always be disabled in the intermediate case for modesets, else
we get a WARN for vblank wait timeout.

Also rename crtc_state to new_crtc_state, to distinguish it from the old state.

Changes since v1:
- Use intel_atomic_get_old_crtc_state. (ville)
Changes since v2:
- Always unset cxsr during modeset.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8c69ec9eb6ee..f904bf73dbd6 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -2026,16 +2026,27 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
 
 static int vlv_compute_intermediate_wm(struct drm_device *dev,
 				       struct intel_crtc *crtc,
-				       struct intel_crtc_state *crtc_state)
+				       struct intel_crtc_state *new_crtc_state)
 {
-	struct vlv_wm_state *intermediate = &crtc_state->wm.vlv.intermediate;
-	const struct vlv_wm_state *optimal = &crtc_state->wm.vlv.optimal;
-	const struct vlv_wm_state *active = &crtc->wm.active.vlv;
+	struct vlv_wm_state *intermediate = &new_crtc_state->wm.vlv.intermediate;
+	const struct vlv_wm_state *optimal = &new_crtc_state->wm.vlv.optimal;
+	struct intel_atomic_state *intel_state =
+		to_intel_atomic_state(new_crtc_state->base.state);
+	const struct intel_crtc_state *old_crtc_state =
+		intel_atomic_get_old_crtc_state(intel_state, crtc);
+	const struct vlv_wm_state *active = &old_crtc_state->wm.vlv.optimal;
 	int level;
 
+	if (!new_crtc_state->base.active || drm_atomic_crtc_needs_modeset(&new_crtc_state->base)) {
+		*intermediate = *optimal;
+
+		intermediate->cxsr = false;
+		goto out;
+	}
+
 	intermediate->num_levels = min(optimal->num_levels, active->num_levels);
 	intermediate->cxsr = optimal->cxsr && active->cxsr &&
-		!crtc_state->disable_cxsr;
+		!new_crtc_state->disable_cxsr;
 
 	for (level = 0; level < intermediate->num_levels; level++) {
 		enum plane_id plane_id;
@@ -2054,12 +2065,13 @@ static int vlv_compute_intermediate_wm(struct drm_device *dev,
 
 	vlv_invalidate_wms(crtc, intermediate, level);
 
+out:
 	/*
 	 * If our intermediate WM are identical to the final WM, then we can
 	 * omit the post-vblank programming; only update if it's different.
 	 */
 	if (memcmp(intermediate, optimal, sizeof(*intermediate)) != 0)
-		crtc_state->wm.need_postvbl_update = true;
+		new_crtc_state->wm.need_postvbl_update = true;
 
 	return 0;
 }
-- 
2.15.0

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

[(resend) PATCH 2/2] drm/i915: Calculate g4x intermediate watermarks correctly

[Maarten Lankhorst]

The watermarks it should calculate against are the old optimal watermarks.
The currently active crtc watermarks are pure fiction, and are invalid in
case of a nonblocking modeset, page flip enabling/disabling planes or any
other reason.

When the crtc is disabled or during a modeset the intermediate watermarks
don't need to be programmed separately, and could be directly assigned
to the optimal watermarks.

CXSR must always be disabled in the intermediate case for modesets,
else we get a WARN for vblank wait timeout.

Also rename crtc_state to new_crtc_state, to distinguish it from the old state.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f904bf73dbd6..3c55e4026331 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -1406,17 +1406,29 @@ static int g4x_compute_pipe_wm(struct intel_crtc_state *crtc_state)
 
 static int g4x_compute_intermediate_wm(struct drm_device *dev,
 				       struct intel_crtc *crtc,
-				       struct intel_crtc_state *crtc_state)
+				       struct intel_crtc_state *new_crtc_state)
 {
-	struct g4x_wm_state *intermediate = &crtc_state->wm.g4x.intermediate;
-	const struct g4x_wm_state *optimal = &crtc_state->wm.g4x.optimal;
-	const struct g4x_wm_state *active = &crtc->wm.active.g4x;
+	struct g4x_wm_state *intermediate = &new_crtc_state->wm.g4x.intermediate;
+	const struct g4x_wm_state *optimal = &new_crtc_state->wm.g4x.optimal;
+	struct intel_atomic_state *intel_state =
+		to_intel_atomic_state(new_crtc_state->base.state);
+	const struct intel_crtc_state *old_crtc_state =
+		intel_atomic_get_old_crtc_state(intel_state, crtc);
+	const struct g4x_wm_state *active = &old_crtc_state->wm.g4x.optimal;
 	enum plane_id plane_id;
 
+	if (!new_crtc_state->base.active || drm_atomic_crtc_needs_modeset(&new_crtc_state->base)) {
+		*intermediate = *optimal;
+
+		intermediate->cxsr = false;
+		intermediate->hpll_en = false;
+		goto out;
+	}
+
 	intermediate->cxsr = optimal->cxsr && active->cxsr &&
-		!crtc_state->disable_cxsr;
+		!new_crtc_state->disable_cxsr;
 	intermediate->hpll_en = optimal->hpll_en && active->hpll_en &&
-		!crtc_state->disable_cxsr;
+		!new_crtc_state->disable_cxsr;
 	intermediate->fbc_en = optimal->fbc_en && active->fbc_en;
 
 	for_each_plane_id_on_crtc(crtc, plane_id) {
@@ -1458,12 +1470,13 @@ static int g4x_compute_intermediate_wm(struct drm_device *dev,
 	WARN_ON(intermediate->hpll.fbc > g4x_fbc_fifo_size(2) &&
 		intermediate->fbc_en && intermediate->hpll_en);
 
+out:
 	/*
 	 * If our intermediate WM are identical to the final WM, then we can
 	 * omit the post-vblank programming; only update if it's different.
 	 */
 	if (memcmp(intermediate, optimal, sizeof(*intermediate)) != 0)
-		crtc_state->wm.need_postvbl_update = true;
+		new_crtc_state->wm.need_postvbl_update = true;
 
 	return 0;
 }
-- 
2.15.0

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

✓ Fi.CI.BAT: success for series starting with [(resend),1/2] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.

[Patchwork]

== Series Details ==

Series: series starting with [(resend),1/2] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.
URL   : https://patchwork.freedesktop.org/series/33888/
State : success

== Summary ==

Series 33888v1 series starting with [(resend),1/2] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.
https://patchwork.freedesktop.org/api/1.0/series/33888/revisions/1/mbox/

Test gem_exec_reloc:
        Subgroup basic-cpu-active:
                pass       -> FAIL       (fi-gdg-551) fdo#102582 +1
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

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

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:448s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:458s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:378s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:553s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:277s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:508s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:511s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:497s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:486s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:430s
fi-gdg-551       total:289  pass:176  dwarn:1   dfail:0   fail:3   skip:109 time:264s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:542s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:428s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:436s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:429s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:480s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:465s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:489s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:532s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:474s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:537s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:578s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:452s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:543s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:563s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:518s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:511s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:459s
fi-snb-2520m     total:246  pass:212  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:419s
Blacklisted hosts:
fi-cfl-s         total:289  pass:254  dwarn:3   dfail:0   fail:0   skip:32  time:527s

06718a287f282ad31264560c5dc18a38474cb562 drm-tip: 2017y-11m-15d-11h-36m-39s UTC integration manifest
1c6ba74267d3 drm/i915: Calculate g4x intermediate watermarks correctly
23e9ba662adf drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.

== Logs ==

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

✗ Fi.CI.IGT: failure for series starting with [(resend),1/2] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.

[Patchwork]

== Series Details ==

Series: series starting with [(resend),1/2] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.
URL   : https://patchwork.freedesktop.org/series/33888/
State : failure

== Summary ==

Test drv_module_reload:
        Subgroup basic-reload-inject:
                pass       -> DMESG-WARN (shard-hsw) fdo#102707
Test drv_selftest:
        Subgroup mock_sanitycheck:
                pass       -> DMESG-WARN (shard-hsw) fdo#103719
Test kms_cursor_crc:
        Subgroup cursor-64x64-suspend:
                pass       -> FAIL       (shard-hsw)
Test kms_atomic_transition:
        Subgroup plane-all-modeset-transition-fencing:
                dmesg-warn -> PASS       (shard-hsw) fdo#102614
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-hsw) fdo#100047

fdo#102707 https://bugs.freedesktop.org/show_bug.cgi?id=102707
fdo#103719 https://bugs.freedesktop.org/show_bug.cgi?id=103719
fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-hsw        total:2584 pass:1467 dwarn:5   dfail:1   fail:11  skip:1099 time:9419s
Blacklisted hosts:
shard-apl        total:2584 pass:1617 dwarn:3   dfail:2   fail:22  skip:940 time:13412s
shard-kbl        total:2565 pass:1699 dwarn:6   dfail:1   fail:25  skip:833 time:10599s
shard-snb        total:2584 pass:1211 dwarn:2   dfail:1   fail:12  skip:1358 time:7870s

== Logs ==

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

Re: [(resend) PATCH 1/2] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.

[Ville Syrjälä]

On Wed, Nov 15, 2017 at 05:31:56PM +0100, Maarten Lankhorst wrote:
> The watermarks it should calculate against are the old optimal watermarks.
> The currently active crtc watermarks are pure fiction, and are invalid in
> case of a nonblocking modeset, page flip enabling/disabling planes or any
> other reason.
> 
> When the crtc is disabled or during a modeset the intermediate watermarks
> don't need to be programmed separately, and could be directly assigned
> to the optimal watermarks.
> 
> CXSR must always be disabled in the intermediate case for modesets, else
> we get a WARN for vblank wait timeout.
> 
> Also rename crtc_state to new_crtc_state, to distinguish it from the old state.
> 
> Changes since v1:
> - Use intel_atomic_get_old_crtc_state. (ville)
> Changes since v2:
> - Always unset cxsr during modeset.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

I was going to try and figure out how/if these get rid of the unclaimed
reg warns, but I didn't quite get that far. I did spot a few other
buglets in the wm code though (I'll send fixes for those at some point).

Anyways, these patches make sense to me, so for the series
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 24 ++++++++++++++++++------
>  1 file changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8c69ec9eb6ee..f904bf73dbd6 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2026,16 +2026,27 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
>  
>  static int vlv_compute_intermediate_wm(struct drm_device *dev,
>  				       struct intel_crtc *crtc,
> -				       struct intel_crtc_state *crtc_state)
> +				       struct intel_crtc_state *new_crtc_state)
>  {
> -	struct vlv_wm_state *intermediate = &crtc_state->wm.vlv.intermediate;
> -	const struct vlv_wm_state *optimal = &crtc_state->wm.vlv.optimal;
> -	const struct vlv_wm_state *active = &crtc->wm.active.vlv;
> +	struct vlv_wm_state *intermediate = &new_crtc_state->wm.vlv.intermediate;
> +	const struct vlv_wm_state *optimal = &new_crtc_state->wm.vlv.optimal;
> +	struct intel_atomic_state *intel_state =
> +		to_intel_atomic_state(new_crtc_state->base.state);
> +	const struct intel_crtc_state *old_crtc_state =
> +		intel_atomic_get_old_crtc_state(intel_state, crtc);
> +	const struct vlv_wm_state *active = &old_crtc_state->wm.vlv.optimal;
>  	int level;
>  
> +	if (!new_crtc_state->base.active || drm_atomic_crtc_needs_modeset(&new_crtc_state->base)) {
> +		*intermediate = *optimal;
> +
> +		intermediate->cxsr = false;
> +		goto out;
> +	}
> +
>  	intermediate->num_levels = min(optimal->num_levels, active->num_levels);
>  	intermediate->cxsr = optimal->cxsr && active->cxsr &&
> -		!crtc_state->disable_cxsr;
> +		!new_crtc_state->disable_cxsr;
>  
>  	for (level = 0; level < intermediate->num_levels; level++) {
>  		enum plane_id plane_id;
> @@ -2054,12 +2065,13 @@ static int vlv_compute_intermediate_wm(struct drm_device *dev,
>  
>  	vlv_invalidate_wms(crtc, intermediate, level);
>  
> +out:
>  	/*
>  	 * If our intermediate WM are identical to the final WM, then we can
>  	 * omit the post-vblank programming; only update if it's different.
>  	 */
>  	if (memcmp(intermediate, optimal, sizeof(*intermediate)) != 0)
> -		crtc_state->wm.need_postvbl_update = true;
> +		new_crtc_state->wm.need_postvbl_update = true;
>  
>  	return 0;
>  }
> -- 
> 2.15.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [(resend) PATCH 1/2] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.

[Maarten Lankhorst]

Op 17-11-17 om 14:31 schreef Ville Syrjälä:
> On Wed, Nov 15, 2017 at 05:31:56PM +0100, Maarten Lankhorst wrote:
>> The watermarks it should calculate against are the old optimal watermarks.
>> The currently active crtc watermarks are pure fiction, and are invalid in
>> case of a nonblocking modeset, page flip enabling/disabling planes or any
>> other reason.
>>
>> When the crtc is disabled or during a modeset the intermediate watermarks
>> don't need to be programmed separately, and could be directly assigned
>> to the optimal watermarks.
>>
>> CXSR must always be disabled in the intermediate case for modesets, else
>> we get a WARN for vblank wait timeout.
>>
>> Also rename crtc_state to new_crtc_state, to distinguish it from the old state.
>>
>> Changes since v1:
>> - Use intel_atomic_get_old_crtc_state. (ville)
>> Changes since v2:
>> - Always unset cxsr during modeset.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> I was going to try and figure out how/if these get rid of the unclaimed
> reg warns, but I didn't quite get that far. I did spot a few other
> buglets in the wm code though (I'll send fixes for those at some point).
>
> Anyways, these patches make sense to me, so for the series
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Seems Chris Wilson already beat us to it..

https://intel-gfx-ci.01.org/tree/drm-tip/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html
was gone with

commit 1a1f12872edcd5e425b668a35fb23548cfa918ef
Author: Chris Wilson <chris@chris-wilson.co.uk <mailto:chris@chris-wilson.co.uk>>
Date:   Tue Nov 7 14:03:38 2017 +0000

    drm/i915: Prevent unbounded wm results in g4x_compute_wm()

But I think this patch also fixes it in a slightly different way, never using garbage for intermediaries. :-)

Thanks for review, pushed.

Cheers,
~Maarten

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

Re: [(resend) PATCH 1/2] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.

[Ville Syrjälä]

On Fri, Nov 17, 2017 at 03:47:58PM +0100, Maarten Lankhorst wrote:
> Op 17-11-17 om 14:31 schreef Ville Syrjälä:
> > On Wed, Nov 15, 2017 at 05:31:56PM +0100, Maarten Lankhorst wrote:
> >> The watermarks it should calculate against are the old optimal watermarks.
> >> The currently active crtc watermarks are pure fiction, and are invalid in
> >> case of a nonblocking modeset, page flip enabling/disabling planes or any
> >> other reason.
> >>
> >> When the crtc is disabled or during a modeset the intermediate watermarks
> >> don't need to be programmed separately, and could be directly assigned
> >> to the optimal watermarks.
> >>
> >> CXSR must always be disabled in the intermediate case for modesets, else
> >> we get a WARN for vblank wait timeout.
> >>
> >> Also rename crtc_state to new_crtc_state, to distinguish it from the old state.
> >>
> >> Changes since v1:
> >> - Use intel_atomic_get_old_crtc_state. (ville)
> >> Changes since v2:
> >> - Always unset cxsr during modeset.
> >>
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > I was going to try and figure out how/if these get rid of the unclaimed
> > reg warns, but I didn't quite get that far. I did spot a few other
> > buglets in the wm code though (I'll send fixes for those at some point).
> >
> > Anyways, these patches make sense to me, so for the series
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Seems Chris Wilson already beat us to it..
> 
> https://intel-gfx-ci.01.org/tree/drm-tip/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html
> was gone with
> 
> commit 1a1f12872edcd5e425b668a35fb23548cfa918ef
> Author: Chris Wilson <chris@chris-wilson.co.uk <mailto:chris@chris-wilson.co.uk>>
> Date:   Tue Nov 7 14:03:38 2017 +0000
> 
>     drm/i915: Prevent unbounded wm results in g4x_compute_wm()

That patch should be nop. So this is a very surprising result.

> 
> But I think this patch also fixes it in a slightly different way, never using garbage for intermediaries. :-)
> 
> Thanks for review, pushed.
> 
> Cheers,
> ~Maarten

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [(resend) PATCH 1/2] drm/i915: Calculate vlv/chv intermediate watermarks correctly, v3.

[Maarten Lankhorst]

Op 17-11-17 om 15:53 schreef Ville Syrjälä:
> On Fri, Nov 17, 2017 at 03:47:58PM +0100, Maarten Lankhorst wrote:
>> Op 17-11-17 om 14:31 schreef Ville Syrjälä:
>>> On Wed, Nov 15, 2017 at 05:31:56PM +0100, Maarten Lankhorst wrote:
>>>> The watermarks it should calculate against are the old optimal watermarks.
>>>> The currently active crtc watermarks are pure fiction, and are invalid in
>>>> case of a nonblocking modeset, page flip enabling/disabling planes or any
>>>> other reason.
>>>>
>>>> When the crtc is disabled or during a modeset the intermediate watermarks
>>>> don't need to be programmed separately, and could be directly assigned
>>>> to the optimal watermarks.
>>>>
>>>> CXSR must always be disabled in the intermediate case for modesets, else
>>>> we get a WARN for vblank wait timeout.
>>>>
>>>> Also rename crtc_state to new_crtc_state, to distinguish it from the old state.
>>>>
>>>> Changes since v1:
>>>> - Use intel_atomic_get_old_crtc_state. (ville)
>>>> Changes since v2:
>>>> - Always unset cxsr during modeset.
>>>>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> I was going to try and figure out how/if these get rid of the unclaimed
>>> reg warns, but I didn't quite get that far. I did spot a few other
>>> buglets in the wm code though (I'll send fixes for those at some point).
>>>
>>> Anyways, these patches make sense to me, so for the series
>>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Seems Chris Wilson already beat us to it..
>>
>> https://intel-gfx-ci.01.org/tree/drm-tip/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b.html
>> was gone with
>>
>> commit 1a1f12872edcd5e425b668a35fb23548cfa918ef
>> Author: Chris Wilson <chris@chris-wilson.co.uk <mailto:chris@chris-wilson.co.uk>>
>> Date:   Tue Nov 7 14:03:38 2017 +0000
>>
>>     drm/i915: Prevent unbounded wm results in g4x_compute_wm()
> That patch should be nop. So this is a very surprising result.
>
Oh well maybe the watermarks initially read out were being garbage. Garbage in, garbage out. :-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx