[PATCH] drm/i915: Handle changing enable_psr parameter at runtime better

[PATCH] drm/i915: Handle changing enable_psr parameter at runtime better

[Maarten Lankhorst]

Similar to enable_fbc, enable_psr was ignored at runtime if it was
changed. The easiest fix is to pretend enable_psr is ignored at
configure time, and never activate it for !enable_psr, so both cases
are handled without modesets.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Tested-by: Benjamin Berg <bberg@redhat.com>
Cc: Benjamin Berg <bberg@redhat.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 23175c5c4a50..ac3ce7a1c2a7 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -502,11 +502,6 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 	if (!CAN_PSR(dev_priv))
 		return;
 
-	if (!i915_modparams.enable_psr) {
-		DRM_DEBUG_KMS("PSR disable by flag\n");
-		return;
-	}
-
 	/*
 	 * HSW spec explicitly says PSR is tied to port A.
 	 * BDW+ platforms with DDI implementation of PSR have different
@@ -559,7 +554,10 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 
 	crtc_state->has_psr = true;
 	crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
-	DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
+	if (i915_modparams.enable_psr)
+		DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
+	else
+		DRM_DEBUG_KMS("PSR disable by flag\n");
 }
 
 static void intel_psr_activate(struct intel_dp *intel_dp)
@@ -652,7 +650,9 @@ void intel_psr_enable(struct intel_dp *intel_dp,
 	dev_priv->psr.enable_source(intel_dp, crtc_state);
 	dev_priv->psr.enabled = intel_dp;
 
-	if (INTEL_GEN(dev_priv) >= 9) {
+	if (!i915_modparams.enable_psr) {
+		DRM_DEBUG_KMS("PSR disable by flag\n");
+	} else if (INTEL_GEN(dev_priv) >= 9) {
 		intel_psr_activate(intel_dp);
 	} else {
 		/*
@@ -843,7 +843,7 @@ static void intel_psr_work(struct work_struct *work)
 	 * recheck. Since psr_flush first clears this and then reschedules we
 	 * won't ever miss a flush when bailing out here.
 	 */
-	if (dev_priv->psr.busy_frontbuffer_bits)
+	if (dev_priv->psr.busy_frontbuffer_bits || !i915_modparams.enable_psr)
 		goto unlock;
 
 	intel_psr_activate(intel_dp);
@@ -1015,7 +1015,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 		return;
 
 	mutex_lock(&dev_priv->psr.lock);
-	if (!dev_priv->psr.enabled) {
+	if (!dev_priv->psr.enabled || !i915_modparams.enable_psr) {
+		intel_psr_exit(dev_priv);
 		mutex_unlock(&dev_priv->psr.lock);
 		return;
 	}
-- 
2.16.2

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

✓ Fi.CI.BAT: success for drm/i915: Handle changing enable_psr parameter at runtime better

[Patchwork]

== Series Details ==

Series: drm/i915: Handle changing enable_psr parameter at runtime better
URL   : https://patchwork.freedesktop.org/series/39545/
State : success

== Summary ==

Series 39545v1 drm/i915: Handle changing enable_psr parameter at runtime better
https://patchwork.freedesktop.org/api/1.0/series/39545/revisions/1/mbox/

---- Known issues:

Test debugfs_test:
        Subgroup read_all_entries:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713

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

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:425s
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:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:495s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:277s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:487s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:491s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:483s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:467s
fi-cfl-8700k     total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:406s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:577s
fi-cfl-u         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:504s
fi-elk-e7500     total:288  pass:229  dwarn:0   dfail:0   fail:0   skip:59  time:418s
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:292s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:521s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:396s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:412s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:454s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:417s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:465s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:459s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:515s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:589s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:431s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:517s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:535s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:497s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:479s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:424s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:428s
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:390s

c362488c07f3f59581a479b2f3b82219fb5bfee3 drm-tip: 2018y-03m-07d-15h-23m-59s UTC integration manifest
7b2597370ab9 drm/i915: Handle changing enable_psr parameter at runtime better

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915: Handle changing enable_psr parameter at runtime better

[Patchwork]

== Series Details ==

Series: drm/i915: Handle changing enable_psr parameter at runtime better
URL   : https://patchwork.freedesktop.org/series/39545/
State : success

== Summary ==

---- Known issues:

Test gem_eio:
        Subgroup in-flight-external:
                incomplete -> PASS       (shard-apl) fdo#105341
Test kms_chv_cursor_fail:
        Subgroup pipe-b-256x256-left-edge:
                dmesg-warn -> PASS       (shard-snb) fdo#105185 +7
Test kms_flip:
        Subgroup flip-vs-expired-vblank:
                pass       -> FAIL       (shard-hsw) fdo#102887
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-c-planes:
                pass       -> INCOMPLETE (shard-hsw) fdo#103375 +1
Test kms_rotation_crc:
        Subgroup primary-rotation-180:
                pass       -> FAIL       (shard-snb) fdo#103925
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-apl) fdo#100047

fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341
fdo#105185 https://bugs.freedesktop.org/show_bug.cgi?id=105185
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-apl        total:3467 pass:1824 dwarn:2   dfail:0   fail:8   skip:1632 time:12308s
shard-hsw        total:3455 pass:1763 dwarn:2   dfail:0   fail:3   skip:1685 time:11400s
shard-snb        total:3467 pass:1360 dwarn:4   dfail:0   fail:2   skip:2101 time:7032s
Blacklisted hosts:
shard-kbl        total:3467 pass:1951 dwarn:2   dfail:0   fail:7   skip:1507 time:9549s

== Logs ==

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

Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better

[Pandiyan, Dhinakaran]

On Wed, 2018-03-07 at 17:39 +0100, Maarten Lankhorst wrote:
> Similar to enable_fbc, enable_psr was ignored at runtime if it was
> changed. The easiest fix is to pretend enable_psr is ignored at
> configure time, and never activate it for !enable_psr, so both cases
> are handled without modesets.

What about cases where psr_flush() is not called and consequently the
module parameter is not checked? With HW tracking, PSR is
enabled/disabled during modeset and the hardware is expected to exit and
activate PSR without driver intervention.


> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Tested-by: Benjamin Berg <bberg@redhat.com>
> Cc: Benjamin Berg <bberg@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 23175c5c4a50..ac3ce7a1c2a7 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -502,11 +502,6 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>  	if (!CAN_PSR(dev_priv))
>  		return;
>  
> -	if (!i915_modparams.enable_psr) {
> -		DRM_DEBUG_KMS("PSR disable by flag\n");
> -		return;
> -	}
> -
>  	/*
>  	 * HSW spec explicitly says PSR is tied to port A.
>  	 * BDW+ platforms with DDI implementation of PSR have different
> @@ -559,7 +554,10 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>  
>  	crtc_state->has_psr = true;
>  	crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
> -	DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
> +	if (i915_modparams.enable_psr)
> +		DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
> +	else
> +		DRM_DEBUG_KMS("PSR disable by flag\n");
>  }
>  
>  static void intel_psr_activate(struct intel_dp *intel_dp)
> @@ -652,7 +650,9 @@ void intel_psr_enable(struct intel_dp *intel_dp,
>  	dev_priv->psr.enable_source(intel_dp, crtc_state);
>  	dev_priv->psr.enabled = intel_dp;
>  
> -	if (INTEL_GEN(dev_priv) >= 9) {
> +	if (!i915_modparams.enable_psr) {
> +		DRM_DEBUG_KMS("PSR disable by flag\n");
> +	} else if (INTEL_GEN(dev_priv) >= 9) {
>  		intel_psr_activate(intel_dp);
>  	} else {
>  		/*
> @@ -843,7 +843,7 @@ static void intel_psr_work(struct work_struct *work)
>  	 * recheck. Since psr_flush first clears this and then reschedules we
>  	 * won't ever miss a flush when bailing out here.
>  	 */
> -	if (dev_priv->psr.busy_frontbuffer_bits)
> +	if (dev_priv->psr.busy_frontbuffer_bits || !i915_modparams.enable_psr)
>  		goto unlock;
>  
>  	intel_psr_activate(intel_dp);
> @@ -1015,7 +1015,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>  		return;
>  
>  	mutex_lock(&dev_priv->psr.lock);
> -	if (!dev_priv->psr.enabled) {
> +	if (!dev_priv->psr.enabled || !i915_modparams.enable_psr) {
> +		intel_psr_exit(dev_priv);
>  		mutex_unlock(&dev_priv->psr.lock);
>  		return;
>  	}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better

[Maarten Lankhorst]

Op 07-03-18 om 23:22 schreef Pandiyan, Dhinakaran:
> On Wed, 2018-03-07 at 17:39 +0100, Maarten Lankhorst wrote:
>> Similar to enable_fbc, enable_psr was ignored at runtime if it was
>> changed. The easiest fix is to pretend enable_psr is ignored at
>> configure time, and never activate it for !enable_psr, so both cases
>> are handled without modesets.
> What about cases where psr_flush() is not called and consequently the
> module parameter is not checked? With HW tracking, PSR is
> enabled/disabled during modeset and the hardware is expected to exit and
> activate PSR without driver intervention.
It looks like intel_frontbuffer_flush always calls intel_psr_flush,
so we at least get a PSR toggle after every atomic commit?

~Maarten

>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Tested-by: Benjamin Berg <bberg@redhat.com>
>> Cc: Benjamin Berg <bberg@redhat.com>
>> ---
>>  drivers/gpu/drm/i915/intel_psr.c | 19 ++++++++++---------
>>  1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>> index 23175c5c4a50..ac3ce7a1c2a7 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -502,11 +502,6 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>>  	if (!CAN_PSR(dev_priv))
>>  		return;
>>  
>> -	if (!i915_modparams.enable_psr) {
>> -		DRM_DEBUG_KMS("PSR disable by flag\n");
>> -		return;
>> -	}
>> -
>>  	/*
>>  	 * HSW spec explicitly says PSR is tied to port A.
>>  	 * BDW+ platforms with DDI implementation of PSR have different
>> @@ -559,7 +554,10 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>>  
>>  	crtc_state->has_psr = true;
>>  	crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
>> -	DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
>> +	if (i915_modparams.enable_psr)
>> +		DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
>> +	else
>> +		DRM_DEBUG_KMS("PSR disable by flag\n");
>>  }
>>  
>>  static void intel_psr_activate(struct intel_dp *intel_dp)
>> @@ -652,7 +650,9 @@ void intel_psr_enable(struct intel_dp *intel_dp,
>>  	dev_priv->psr.enable_source(intel_dp, crtc_state);
>>  	dev_priv->psr.enabled = intel_dp;
>>  
>> -	if (INTEL_GEN(dev_priv) >= 9) {
>> +	if (!i915_modparams.enable_psr) {
>> +		DRM_DEBUG_KMS("PSR disable by flag\n");
>> +	} else if (INTEL_GEN(dev_priv) >= 9) {
>>  		intel_psr_activate(intel_dp);
>>  	} else {
>>  		/*
>> @@ -843,7 +843,7 @@ static void intel_psr_work(struct work_struct *work)
>>  	 * recheck. Since psr_flush first clears this and then reschedules we
>>  	 * won't ever miss a flush when bailing out here.
>>  	 */
>> -	if (dev_priv->psr.busy_frontbuffer_bits)
>> +	if (dev_priv->psr.busy_frontbuffer_bits || !i915_modparams.enable_psr)
>>  		goto unlock;
>>  
>>  	intel_psr_activate(intel_dp);
>> @@ -1015,7 +1015,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>>  		return;
>>  
>>  	mutex_lock(&dev_priv->psr.lock);
>> -	if (!dev_priv->psr.enabled) {
>> +	if (!dev_priv->psr.enabled || !i915_modparams.enable_psr) {
>> +		intel_psr_exit(dev_priv);
>>  		mutex_unlock(&dev_priv->psr.lock);
>>  		return;
>>  	}


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

Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better

[Pandiyan, Dhinakaran]

On Thu, 2018-03-08 at 08:07 +0100, Maarten Lankhorst wrote:
> Op 07-03-18 om 23:22 schreef Pandiyan, Dhinakaran:
> > On Wed, 2018-03-07 at 17:39 +0100, Maarten Lankhorst wrote:
> >> Similar to enable_fbc, enable_psr was ignored at runtime if it was
> >> changed. The easiest fix is to pretend enable_psr is ignored at
> >> configure time, and never activate it for !enable_psr, so both cases
> >> are handled without modesets.
> > What about cases where psr_flush() is not called and consequently the
> > module parameter is not checked? With HW tracking, PSR is
> > enabled/disabled during modeset and the hardware is expected to exit and
> > activate PSR without driver intervention.
> It looks like intel_frontbuffer_flush always calls intel_psr_flush,
> so we at least get a PSR toggle after every atomic commit?

I have a patch to remove flush() from legacy_cursor_update(). We end up
with an inconsistent behavior when that patch gets merged,
cursor moves -> trigger psr exit but don't read module parameter
commits -> trigger psr exit but read module parameter

Eventually, when we get to removing flush() from commits, then this
patch won't really be useful. And tests disabling/enabling PSR at
runtime will probably fail.


> 
> ~Maarten
> 
> >> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >> Tested-by: Benjamin Berg <bberg@redhat.com>
> >> Cc: Benjamin Berg <bberg@redhat.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_psr.c | 19 ++++++++++---------
> >>  1 file changed, 10 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> >> index 23175c5c4a50..ac3ce7a1c2a7 100644
> >> --- a/drivers/gpu/drm/i915/intel_psr.c
> >> +++ b/drivers/gpu/drm/i915/intel_psr.c
> >> @@ -502,11 +502,6 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> >>  	if (!CAN_PSR(dev_priv))
> >>  		return;
> >>  
> >> -	if (!i915_modparams.enable_psr) {
> >> -		DRM_DEBUG_KMS("PSR disable by flag\n");
> >> -		return;
> >> -	}
> >> -
> >>  	/*
> >>  	 * HSW spec explicitly says PSR is tied to port A.
> >>  	 * BDW+ platforms with DDI implementation of PSR have different
> >> @@ -559,7 +554,10 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> >>  
> >>  	crtc_state->has_psr = true;
> >>  	crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
> >> -	DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
> >> +	if (i915_modparams.enable_psr)
> >> +		DRM_DEBUG_KMS("Enabling PSR%s\n", crtc_state->has_psr2 ? "2" : "");
> >> +	else
> >> +		DRM_DEBUG_KMS("PSR disable by flag\n");
> >>  }
> >>  
> >>  static void intel_psr_activate(struct intel_dp *intel_dp)
> >> @@ -652,7 +650,9 @@ void intel_psr_enable(struct intel_dp *intel_dp,
> >>  	dev_priv->psr.enable_source(intel_dp, crtc_state);
> >>  	dev_priv->psr.enabled = intel_dp;
> >>  
> >> -	if (INTEL_GEN(dev_priv) >= 9) {
> >> +	if (!i915_modparams.enable_psr) {
> >> +		DRM_DEBUG_KMS("PSR disable by flag\n");
> >> +	} else if (INTEL_GEN(dev_priv) >= 9) {
> >>  		intel_psr_activate(intel_dp);
> >>  	} else {
> >>  		/*
> >> @@ -843,7 +843,7 @@ static void intel_psr_work(struct work_struct *work)
> >>  	 * recheck. Since psr_flush first clears this and then reschedules we
> >>  	 * won't ever miss a flush when bailing out here.
> >>  	 */
> >> -	if (dev_priv->psr.busy_frontbuffer_bits)
> >> +	if (dev_priv->psr.busy_frontbuffer_bits || !i915_modparams.enable_psr)
> >>  		goto unlock;
> >>  
> >>  	intel_psr_activate(intel_dp);
> >> @@ -1015,7 +1015,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> >>  		return;
> >>  
> >>  	mutex_lock(&dev_priv->psr.lock);
> >> -	if (!dev_priv->psr.enabled) {
> >> +	if (!dev_priv->psr.enabled || !i915_modparams.enable_psr) {
> >> +		intel_psr_exit(dev_priv);
> >>  		mutex_unlock(&dev_priv->psr.lock);
> >>  		return;
> >>  	}
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better

[Maarten Lankhorst]

Op 08-03-18 om 18:43 schreef Pandiyan, Dhinakaran:
>
>
> On Thu, 2018-03-08 at 08:07 +0100, Maarten Lankhorst wrote:
>> Op 07-03-18 om 23:22 schreef Pandiyan, Dhinakaran:
>>> On Wed, 2018-03-07 at 17:39 +0100, Maarten Lankhorst wrote:
>>>> Similar to enable_fbc, enable_psr was ignored at runtime if it was
>>>> changed. The easiest fix is to pretend enable_psr is ignored at
>>>> configure time, and never activate it for !enable_psr, so both cases
>>>> are handled without modesets.
>>> What about cases where psr_flush() is not called and consequently the
>>> module parameter is not checked? With HW tracking, PSR is
>>> enabled/disabled during modeset and the hardware is expected to exit and
>>> activate PSR without driver intervention.
>> It looks like intel_frontbuffer_flush always calls intel_psr_flush,
>> so we at least get a PSR toggle after every atomic commit?
> I have a patch to remove flush() from legacy_cursor_update(). We end up
> with an inconsistent behavior when that patch gets merged,
> cursor moves -> trigger psr exit but don't read module parameter
> commits -> trigger psr exit but read module parameter
Legacy cursor updates are special, I don't mind them not changing PSR.
> Eventually, when we get to removing flush() from commits, then this
> patch won't really be useful. And tests disabling/enabling PSR at
> runtime will probably fail.
Could we transition to debugfs for changing it at runtime?

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

Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better

[Pandiyan, Dhinakaran]

On Thu, 2018-03-08 at 18:52 +0100, Maarten Lankhorst wrote:
> Op 08-03-18 om 18:43 schreef Pandiyan, Dhinakaran:
> >
> >
> > On Thu, 2018-03-08 at 08:07 +0100, Maarten Lankhorst wrote:
> >> Op 07-03-18 om 23:22 schreef Pandiyan, Dhinakaran:
> >>> On Wed, 2018-03-07 at 17:39 +0100, Maarten Lankhorst wrote:
> >>>> Similar to enable_fbc, enable_psr was ignored at runtime if it was
> >>>> changed. The easiest fix is to pretend enable_psr is ignored at
> >>>> configure time, and never activate it for !enable_psr, so both cases
> >>>> are handled without modesets.
> >>> What about cases where psr_flush() is not called and consequently the
> >>> module parameter is not checked? With HW tracking, PSR is
> >>> enabled/disabled during modeset and the hardware is expected to exit and
> >>> activate PSR without driver intervention.
> >> It looks like intel_frontbuffer_flush always calls intel_psr_flush,
> >> so we at least get a PSR toggle after every atomic commit?
> > I have a patch to remove flush() from legacy_cursor_update(). We end up
> > with an inconsistent behavior when that patch gets merged,
> > cursor moves -> trigger psr exit but don't read module parameter
> > commits -> trigger psr exit but read module parameter
> Legacy cursor updates are special, I don't mind them not changing PSR.
> > Eventually, when we get to removing flush() from commits, then this
> > patch won't really be useful. And tests disabling/enabling PSR at
> > runtime will probably fail.
> Could we transition to debugfs for changing it at runtime?

That does sound like a better idea.

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

Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better

[Rodrigo Vivi]

On Thu, Mar 08, 2018 at 10:07:05AM -0800, Pandiyan, Dhinakaran wrote:
> 
> 
> 
> On Thu, 2018-03-08 at 18:52 +0100, Maarten Lankhorst wrote:
> > Op 08-03-18 om 18:43 schreef Pandiyan, Dhinakaran:
> > >
> > >
> > > On Thu, 2018-03-08 at 08:07 +0100, Maarten Lankhorst wrote:
> > >> Op 07-03-18 om 23:22 schreef Pandiyan, Dhinakaran:
> > >>> On Wed, 2018-03-07 at 17:39 +0100, Maarten Lankhorst wrote:
> > >>>> Similar to enable_fbc, enable_psr was ignored at runtime if it was
> > >>>> changed. The easiest fix is to pretend enable_psr is ignored at
> > >>>> configure time, and never activate it for !enable_psr, so both cases
> > >>>> are handled without modesets.
> > >>> What about cases where psr_flush() is not called and consequently the
> > >>> module parameter is not checked? With HW tracking, PSR is
> > >>> enabled/disabled during modeset and the hardware is expected to exit and
> > >>> activate PSR without driver intervention.
> > >> It looks like intel_frontbuffer_flush always calls intel_psr_flush,
> > >> so we at least get a PSR toggle after every atomic commit?
> > > I have a patch to remove flush() from legacy_cursor_update(). We end up
> > > with an inconsistent behavior when that patch gets merged,
> > > cursor moves -> trigger psr exit but don't read module parameter
> > > commits -> trigger psr exit but read module parameter
> > Legacy cursor updates are special, I don't mind them not changing PSR.
> > > Eventually, when we get to removing flush() from commits, then this
> > > patch won't really be useful. And tests disabling/enabling PSR at
> > > runtime will probably fail.
> > Could we transition to debugfs for changing it at runtime?
> 
> That does sound like a better idea.

+1

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