* [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better @ 2018-03-07 16:39 Maarten Lankhorst 2018-03-07 16:59 ` ✓ Fi.CI.BAT: success for " Patchwork ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Maarten Lankhorst @ 2018-03-07 16:39 UTC (permalink / raw) To: intel-gfx; +Cc: Benjamin Berg 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Handle changing enable_psr parameter at runtime better 2018-03-07 16:39 [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better Maarten Lankhorst @ 2018-03-07 16:59 ` Patchwork 2018-03-07 19:02 ` ✓ Fi.CI.IGT: " Patchwork 2018-03-07 22:22 ` [PATCH] " Pandiyan, Dhinakaran 2 siblings, 0 replies; 9+ messages in thread From: Patchwork @ 2018-03-07 16:59 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx == 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* ✓ Fi.CI.IGT: success for drm/i915: Handle changing enable_psr parameter at runtime better 2018-03-07 16:39 [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better Maarten Lankhorst 2018-03-07 16:59 ` ✓ Fi.CI.BAT: success for " Patchwork @ 2018-03-07 19:02 ` Patchwork 2018-03-07 22:22 ` [PATCH] " Pandiyan, Dhinakaran 2 siblings, 0 replies; 9+ messages in thread From: Patchwork @ 2018-03-07 19:02 UTC (permalink / raw) To: Maarten Lankhorst; +Cc: intel-gfx == 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better 2018-03-07 16:39 [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better Maarten Lankhorst 2018-03-07 16:59 ` ✓ Fi.CI.BAT: success for " Patchwork 2018-03-07 19:02 ` ✓ Fi.CI.IGT: " Patchwork @ 2018-03-07 22:22 ` Pandiyan, Dhinakaran 2018-03-08 7:07 ` Maarten Lankhorst 2 siblings, 1 reply; 9+ messages in thread From: Pandiyan, Dhinakaran @ 2018-03-07 22:22 UTC (permalink / raw) To: maarten.lankhorst; +Cc: intel-gfx, bberg, Vivi, Rodrigo 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better 2018-03-07 22:22 ` [PATCH] " Pandiyan, Dhinakaran @ 2018-03-08 7:07 ` Maarten Lankhorst 2018-03-08 17:43 ` Pandiyan, Dhinakaran 0 siblings, 1 reply; 9+ messages in thread From: Maarten Lankhorst @ 2018-03-08 7:07 UTC (permalink / raw) To: Pandiyan, Dhinakaran; +Cc: intel-gfx, bberg, Vivi, Rodrigo 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better 2018-03-08 7:07 ` Maarten Lankhorst @ 2018-03-08 17:43 ` Pandiyan, Dhinakaran 2018-03-08 17:52 ` Maarten Lankhorst 0 siblings, 1 reply; 9+ messages in thread From: Pandiyan, Dhinakaran @ 2018-03-08 17:43 UTC (permalink / raw) To: maarten.lankhorst; +Cc: intel-gfx, bberg, Vivi, Rodrigo 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better 2018-03-08 17:43 ` Pandiyan, Dhinakaran @ 2018-03-08 17:52 ` Maarten Lankhorst 2018-03-08 18:07 ` Pandiyan, Dhinakaran 0 siblings, 1 reply; 9+ messages in thread From: Maarten Lankhorst @ 2018-03-08 17:52 UTC (permalink / raw) To: Pandiyan, Dhinakaran; +Cc: intel-gfx, bberg, Vivi, Rodrigo 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better 2018-03-08 17:52 ` Maarten Lankhorst @ 2018-03-08 18:07 ` Pandiyan, Dhinakaran 2018-03-08 19:08 ` Rodrigo Vivi 0 siblings, 1 reply; 9+ messages in thread From: Pandiyan, Dhinakaran @ 2018-03-08 18:07 UTC (permalink / raw) To: maarten.lankhorst; +Cc: intel-gfx, bberg, Vivi, Rodrigo 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better 2018-03-08 18:07 ` Pandiyan, Dhinakaran @ 2018-03-08 19:08 ` Rodrigo Vivi 0 siblings, 0 replies; 9+ messages in thread From: Rodrigo Vivi @ 2018-03-08 19:08 UTC (permalink / raw) To: Pandiyan, Dhinakaran; +Cc: intel-gfx, bberg 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-03-08 19:09 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-07 16:39 [PATCH] drm/i915: Handle changing enable_psr parameter at runtime better Maarten Lankhorst 2018-03-07 16:59 ` ✓ Fi.CI.BAT: success for " Patchwork 2018-03-07 19:02 ` ✓ Fi.CI.IGT: " Patchwork 2018-03-07 22:22 ` [PATCH] " Pandiyan, Dhinakaran 2018-03-08 7:07 ` Maarten Lankhorst 2018-03-08 17:43 ` Pandiyan, Dhinakaran 2018-03-08 17:52 ` Maarten Lankhorst 2018-03-08 18:07 ` Pandiyan, Dhinakaran 2018-03-08 19:08 ` Rodrigo Vivi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.