* [PATCH v2] drm/i915: Disable DRRS when PSR is enabled
@ 2017-08-31 21:17 Radhakrishna Sripada
2017-08-31 21:38 ` ✓ Fi.CI.BAT: success for drm/i915: Disable DRRS when PSR is enabled (rev2) Patchwork
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Radhakrishna Sripada @ 2017-08-31 21:17 UTC (permalink / raw)
To: intel-gfx
Cc: Jani Nikula, Nicholas Stommel, Dhinakaran Pandiyan, Rodrigo Vivi
Some platforms donot support PSR and DRRS simultaneously.
Visual artifacts and flickering were reported on BDW HP Spectre
x360 Convertible. Deferring to PSR when both PSR and DRRS are
supported by the panel.
V2: Minor code-style changes suggested by Rodrigo
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101111
Cc: Nicholas Stommel <nicholas.stommel@gmail.com>
Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Clinton Taylor <clinton.a.taylor@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
drivers/gpu/drm/i915/intel_dp.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 887953c0f495..aa5a69301257 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5467,11 +5467,6 @@ static void intel_dp_set_drrs_state(struct drm_i915_private *dev_priv,
return;
}
- /*
- * FIXME: This needs proper synchronization with psr state for some
- * platforms that cannot have PSR and DRRS enabled at the same time.
- */
-
dig_port = dp_to_dig_port(intel_dp);
encoder = &dig_port->base;
intel_crtc = to_intel_crtc(encoder->base.crtc);
@@ -5555,6 +5550,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp,
return;
}
+ if (dev_priv->psr.enabled) {
+ DRM_DEBUG_KMS("PSR enabled. Disabling DRRS.\n");
+ return;
+ }
+
mutex_lock(&dev_priv->drrs.mutex);
if (WARN_ON(dev_priv->drrs.dp)) {
DRM_ERROR("DRRS already enabled\n");
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* ✓ Fi.CI.BAT: success for drm/i915: Disable DRRS when PSR is enabled (rev2) 2017-08-31 21:17 [PATCH v2] drm/i915: Disable DRRS when PSR is enabled Radhakrishna Sripada @ 2017-08-31 21:38 ` Patchwork 2017-09-01 0:27 ` ✗ Fi.CI.IGT: warning " Patchwork 2017-09-08 22:02 ` [PATCH v2] drm/i915: Disable DRRS when PSR is enabled Pandiyan, Dhinakaran 2 siblings, 0 replies; 7+ messages in thread From: Patchwork @ 2017-08-31 21:38 UTC (permalink / raw) To: Radhakrishna Sripada; +Cc: intel-gfx == Series Details == Series: drm/i915: Disable DRRS when PSR is enabled (rev2) URL : https://patchwork.freedesktop.org/series/29577/ State : success == Summary == Series 29577v2 drm/i915: Disable DRRS when PSR is enabled https://patchwork.freedesktop.org/api/1.0/series/29577/revisions/2/mbox/ Test gem_exec_flush: Subgroup basic-batch-kernel-default-uc: pass -> FAIL (fi-snb-2600) fdo#100007 Test kms_cursor_legacy: Subgroup basic-flip-after-cursor-varying-size: fail -> PASS (fi-hsw-4770) fdo#102402 +1 fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007 fdo#102402 https://bugs.freedesktop.org/show_bug.cgi?id=102402 fi-bdw-5557u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:453s fi-bdw-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:443s fi-blb-e6850 total:288 pass:224 dwarn:1 dfail:0 fail:0 skip:63 time:364s fi-bsw-n3050 total:288 pass:243 dwarn:0 dfail:0 fail:0 skip:45 time:556s fi-bwr-2160 total:288 pass:184 dwarn:0 dfail:0 fail:0 skip:104 time:253s fi-bxt-j4205 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:525s fi-byt-j1900 total:288 pass:254 dwarn:1 dfail:0 fail:0 skip:33 time:522s fi-byt-n2820 total:288 pass:250 dwarn:1 dfail:0 fail:0 skip:37 time:518s fi-elk-e7500 total:288 pass:230 dwarn:0 dfail:0 fail:0 skip:58 time:437s fi-glk-2a total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:619s fi-hsw-4770 total:288 pass:263 dwarn:0 dfail:0 fail:0 skip:25 time:456s fi-hsw-4770r total:288 pass:263 dwarn:0 dfail:0 fail:0 skip:25 time:423s fi-ilk-650 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:430s fi-ivb-3520m total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:508s fi-ivb-3770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:476s fi-kbl-7500u total:288 pass:264 dwarn:1 dfail:0 fail:0 skip:23 time:517s fi-kbl-7560u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:599s fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:598s fi-pnv-d510 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:526s fi-skl-6260u total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:469s fi-skl-6700k total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:535s fi-skl-6770hq total:288 pass:269 dwarn:0 dfail:0 fail:0 skip:19 time:486s fi-skl-gvtdvm total:288 pass:266 dwarn:0 dfail:0 fail:0 skip:22 time:446s fi-skl-x1585l total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:489s fi-snb-2520m total:288 pass:251 dwarn:0 dfail:0 fail:0 skip:37 time:552s fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:2 skip:38 time:407s ccf4ca2d93383fe1a234aba83df9c21400216433 drm-tip: 2017y-08m-31d-18h-37m-46s UTC integration manifest cba2ebe39662 drm/i915: Disable DRRS when PSR is enabled == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5553/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* ✗ Fi.CI.IGT: warning for drm/i915: Disable DRRS when PSR is enabled (rev2) 2017-08-31 21:17 [PATCH v2] drm/i915: Disable DRRS when PSR is enabled Radhakrishna Sripada 2017-08-31 21:38 ` ✓ Fi.CI.BAT: success for drm/i915: Disable DRRS when PSR is enabled (rev2) Patchwork @ 2017-09-01 0:27 ` Patchwork 2017-09-08 22:02 ` [PATCH v2] drm/i915: Disable DRRS when PSR is enabled Pandiyan, Dhinakaran 2 siblings, 0 replies; 7+ messages in thread From: Patchwork @ 2017-09-01 0:27 UTC (permalink / raw) To: Radhakrishna Sripada; +Cc: intel-gfx == Series Details == Series: drm/i915: Disable DRRS when PSR is enabled (rev2) URL : https://patchwork.freedesktop.org/series/29577/ State : warning == Summary == Test kms_setmode: Subgroup basic: fail -> PASS (shard-hsw) fdo#99912 Test perf: Subgroup polling: fail -> PASS (shard-hsw) fdo#102252 +1 Test kms_frontbuffer_tracking: Subgroup fbc-1p-primscrn-spr-indfb-move: pass -> SKIP (shard-hsw) Test kms_chv_cursor_fail: Subgroup pipe-C-256x256-right-edge: pass -> SKIP (shard-hsw) Subgroup pipe-C-128x128-top-edge: pass -> SKIP (shard-hsw) Test pm_rpm: Subgroup legacy-planes: pass -> SKIP (shard-hsw) Test kms_plane: Subgroup plane-position-hole-dpms-pipe-A-planes: pass -> SKIP (shard-hsw) Test kms_cursor_legacy: Subgroup basic-flip-after-cursor-legacy: pass -> SKIP (shard-hsw) fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912 fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252 shard-hsw total:2265 pass:1229 dwarn:0 dfail:0 fail:14 skip:1022 time:9618s == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5553/shards.html _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/i915: Disable DRRS when PSR is enabled 2017-08-31 21:17 [PATCH v2] drm/i915: Disable DRRS when PSR is enabled Radhakrishna Sripada 2017-08-31 21:38 ` ✓ Fi.CI.BAT: success for drm/i915: Disable DRRS when PSR is enabled (rev2) Patchwork 2017-09-01 0:27 ` ✗ Fi.CI.IGT: warning " Patchwork @ 2017-09-08 22:02 ` Pandiyan, Dhinakaran 2017-09-12 14:16 ` Jani Nikula 2 siblings, 1 reply; 7+ messages in thread From: Pandiyan, Dhinakaran @ 2017-09-08 22:02 UTC (permalink / raw) To: Sripada, Radhakrishna Cc: Nikula, Jani, intel-gfx@lists.freedesktop.org, nicholas.stommel@gmail.com, Vivi, Rodrigo On Thu, 2017-08-31 at 14:17 -0700, Radhakrishna Sripada wrote: > Some platforms donot support PSR and DRRS simultaneously. I could not verify which platforms support PSR + DRRS and which don't. But, seems safe to have DRRS disabled for all platforms when PSR is enabled. > Visual artifacts and flickering were reported on BDW HP Spectre > x360 Convertible. Deferring to PSR when both PSR and DRRS are > supported by the panel. > > V2: Minor code-style changes suggested by Rodrigo > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101111 > Cc: Nicholas Stommel <nicholas.stommel@gmail.com> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > Cc: Clinton Taylor <clinton.a.taylor@intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 887953c0f495..aa5a69301257 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -5467,11 +5467,6 @@ static void intel_dp_set_drrs_state(struct drm_i915_private *dev_priv, > return; > } > > - /* > - * FIXME: This needs proper synchronization with psr state for some > - * platforms that cannot have PSR and DRRS enabled at the same time. > - */ > - > dig_port = dp_to_dig_port(intel_dp); > encoder = &dig_port->base; > intel_crtc = to_intel_crtc(encoder->base.crtc); > @@ -5555,6 +5550,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp, > return; > } > > + if (dev_priv->psr.enabled) { > + DRM_DEBUG_KMS("PSR enabled. Disabling DRRS.\n"); > + return; > + } So every time a flush/invalidate happens, we end up taking the drrs.mutex and then returning because dev_priv->drrs.dp is NULL. That seems unnecessary. Have your considered drrs.type = DRRS_NOT_SUPPORTED? And this solution relies on the ordering that psr_enable() is done before drrs_enable(), we need a comment in enable_ddi to make a note of that. A WARN_ON in psr_enable() if drrs is already enabled might work too. > + > mutex_lock(&dev_priv->drrs.mutex); > if (WARN_ON(dev_priv->drrs.dp)) { > DRM_ERROR("DRRS already enabled\n"); _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/i915: Disable DRRS when PSR is enabled 2017-09-08 22:02 ` [PATCH v2] drm/i915: Disable DRRS when PSR is enabled Pandiyan, Dhinakaran @ 2017-09-12 14:16 ` Jani Nikula 2017-09-12 14:17 ` Jani Nikula 0 siblings, 1 reply; 7+ messages in thread From: Jani Nikula @ 2017-09-12 14:16 UTC (permalink / raw) To: Pandiyan, Dhinakaran, Sripada, Radhakrishna Cc: intel-gfx@lists.freedesktop.org, nicholas.stommel@gmail.com, Vivi, Rodrigo On Sat, 09 Sep 2017, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote: > On Thu, 2017-08-31 at 14:17 -0700, Radhakrishna Sripada wrote: >> Some platforms donot support PSR and DRRS simultaneously. > > I could not verify which platforms support PSR + DRRS and which don't. > But, seems safe to have DRRS disabled for all platforms when PSR is > enabled. > > > >> Visual artifacts and flickering were reported on BDW HP Spectre >> x360 Convertible. Deferring to PSR when both PSR and DRRS are >> supported by the panel. >> >> V2: Minor code-style changes suggested by Rodrigo >> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101111 >> Cc: Nicholas Stommel <nicholas.stommel@gmail.com> >> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> >> Cc: Jani Nikula <jani.nikula@intel.com> >> Cc: Clinton Taylor <clinton.a.taylor@intel.com> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index 887953c0f495..aa5a69301257 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -5467,11 +5467,6 @@ static void intel_dp_set_drrs_state(struct drm_i915_private *dev_priv, >> return; >> } >> >> - /* >> - * FIXME: This needs proper synchronization with psr state for some >> - * platforms that cannot have PSR and DRRS enabled at the same time. >> - */ >> - >> dig_port = dp_to_dig_port(intel_dp); >> encoder = &dig_port->base; >> intel_crtc = to_intel_crtc(encoder->base.crtc); >> @@ -5555,6 +5550,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp, >> return; >> } >> >> + if (dev_priv->psr.enabled) { >> + DRM_DEBUG_KMS("PSR enabled. Disabling DRRS.\n"); >> + return; >> + } > > So every time a flush/invalidate happens, we end up taking the > drrs.mutex and then returning because dev_priv->drrs.dp is NULL. That > seems unnecessary. Have your considered drrs.type = DRRS_NOT_SUPPORTED? That would prevent DRRS testing by disabling PSR via module parameter. I think this is fine. Although the debug message is misleading; it's "not enabling DRRS", not "disabling DRRS". There's a difference. Side note, dev_priv->drrs.type is redundant and could be replaced with direct use of dev_priv->vbt.drrs_type. > And this solution relies on the ordering that psr_enable() is done > before drrs_enable(), we need a comment in enable_ddi to make a note of > that. A WARN_ON in psr_enable() if drrs is already enabled might work > too. I think a WARN_ON would be fine. BR, Jani. > > >> + >> mutex_lock(&dev_priv->drrs.mutex); >> if (WARN_ON(dev_priv->drrs.dp)) { >> DRM_ERROR("DRRS already enabled\n"); -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/i915: Disable DRRS when PSR is enabled 2017-09-12 14:16 ` Jani Nikula @ 2017-09-12 14:17 ` Jani Nikula 2017-09-13 23:32 ` Sripada, Radhakrishna 0 siblings, 1 reply; 7+ messages in thread From: Jani Nikula @ 2017-09-12 14:17 UTC (permalink / raw) To: Pandiyan, Dhinakaran, Sripada, Radhakrishna Cc: intel-gfx@lists.freedesktop.org, nicholas.stommel@gmail.com, Vivi, Rodrigo On Tue, 12 Sep 2017, Jani Nikula <jani.nikula@intel.com> wrote: > On Sat, 09 Sep 2017, "Pandiyan, Dhinakaran" <dhinakaran.pandiyan@intel.com> wrote: >> On Thu, 2017-08-31 at 14:17 -0700, Radhakrishna Sripada wrote: >>> Some platforms donot support PSR and DRRS simultaneously. >> >> I could not verify which platforms support PSR + DRRS and which don't. >> But, seems safe to have DRRS disabled for all platforms when PSR is >> enabled. >> >> >> >>> Visual artifacts and flickering were reported on BDW HP Spectre >>> x360 Convertible. Deferring to PSR when both PSR and DRRS are >>> supported by the panel. >>> >>> V2: Minor code-style changes suggested by Rodrigo >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101111 >>> Cc: Nicholas Stommel <nicholas.stommel@gmail.com> >>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> >>> Cc: Jani Nikula <jani.nikula@intel.com> >>> Cc: Clinton Taylor <clinton.a.taylor@intel.com> >>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >>> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> >>> --- >>> drivers/gpu/drm/i915/intel_dp.c | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >>> index 887953c0f495..aa5a69301257 100644 >>> --- a/drivers/gpu/drm/i915/intel_dp.c >>> +++ b/drivers/gpu/drm/i915/intel_dp.c >>> @@ -5467,11 +5467,6 @@ static void intel_dp_set_drrs_state(struct drm_i915_private *dev_priv, >>> return; >>> } >>> >>> - /* >>> - * FIXME: This needs proper synchronization with psr state for some >>> - * platforms that cannot have PSR and DRRS enabled at the same time. >>> - */ >>> - >>> dig_port = dp_to_dig_port(intel_dp); >>> encoder = &dig_port->base; >>> intel_crtc = to_intel_crtc(encoder->base.crtc); >>> @@ -5555,6 +5550,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp, >>> return; >>> } >>> >>> + if (dev_priv->psr.enabled) { >>> + DRM_DEBUG_KMS("PSR enabled. Disabling DRRS.\n"); >>> + return; >>> + } >> >> So every time a flush/invalidate happens, we end up taking the >> drrs.mutex and then returning because dev_priv->drrs.dp is NULL. That >> seems unnecessary. Have your considered drrs.type = DRRS_NOT_SUPPORTED? > > That would prevent DRRS testing by disabling PSR via module parameter. I > think this is fine. I mean, I think the change in this patch is fine, preventing DRRS testing is not fine. > Although the debug message is misleading; it's "not > enabling DRRS", not "disabling DRRS". There's a difference. > > Side note, dev_priv->drrs.type is redundant and could be replaced with > direct use of dev_priv->vbt.drrs_type. > >> And this solution relies on the ordering that psr_enable() is done >> before drrs_enable(), we need a comment in enable_ddi to make a note of >> that. A WARN_ON in psr_enable() if drrs is already enabled might work >> too. > > I think a WARN_ON would be fine. > > BR, > Jani. > >> >> >>> + >>> mutex_lock(&dev_priv->drrs.mutex); >>> if (WARN_ON(dev_priv->drrs.dp)) { >>> DRM_ERROR("DRRS already enabled\n"); -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] drm/i915: Disable DRRS when PSR is enabled 2017-09-12 14:17 ` Jani Nikula @ 2017-09-13 23:32 ` Sripada, Radhakrishna 0 siblings, 0 replies; 7+ messages in thread From: Sripada, Radhakrishna @ 2017-09-13 23:32 UTC (permalink / raw) To: Nikula, Jani, Pandiyan, Dhinakaran Cc: intel-gfx@lists.freedesktop.org, nicholas.stommel@gmail.com, Vivi, Rodrigo > -----Original Message----- > From: Nikula, Jani > Sent: Tuesday, September 12, 2017 7:18 AM > To: Pandiyan, Dhinakaran <dhinakaran.pandiyan@intel.com>; Sripada, > Radhakrishna <radhakrishna.sripada@intel.com> > Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo <rodrigo.vivi@intel.com>; > nicholas.stommel@gmail.com > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Disable DRRS when PSR is > enabled > > On Tue, 12 Sep 2017, Jani Nikula <jani.nikula@intel.com> wrote: > > On Sat, 09 Sep 2017, "Pandiyan, Dhinakaran" > <dhinakaran.pandiyan@intel.com> wrote: > >> On Thu, 2017-08-31 at 14:17 -0700, Radhakrishna Sripada wrote: > >>> Some platforms donot support PSR and DRRS simultaneously. > >> > >> I could not verify which platforms support PSR + DRRS and which don't. > >> But, seems safe to have DRRS disabled for all platforms when PSR is > >> enabled. > >> > >> > >> > >>> Visual artifacts and flickering were reported on BDW HP Spectre > >>> x360 Convertible. Deferring to PSR when both PSR and DRRS are > >>> supported by the panel. > >>> > >>> V2: Minor code-style changes suggested by Rodrigo > >>> > >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101111 > >>> Cc: Nicholas Stommel <nicholas.stommel@gmail.com> > >>> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > >>> Cc: Jani Nikula <jani.nikula@intel.com> > >>> Cc: Clinton Taylor <clinton.a.taylor@intel.com> > >>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > >>> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com> > >>> --- > >>> drivers/gpu/drm/i915/intel_dp.c | 10 +++++----- > >>> 1 file changed, 5 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c > >>> b/drivers/gpu/drm/i915/intel_dp.c index 887953c0f495..aa5a69301257 > >>> 100644 > >>> --- a/drivers/gpu/drm/i915/intel_dp.c > >>> +++ b/drivers/gpu/drm/i915/intel_dp.c > >>> @@ -5467,11 +5467,6 @@ static void intel_dp_set_drrs_state(struct > drm_i915_private *dev_priv, > >>> return; > >>> } > >>> > >>> - /* > >>> - * FIXME: This needs proper synchronization with psr state for some > >>> - * platforms that cannot have PSR and DRRS enabled at the same > time. > >>> - */ > >>> - > >>> dig_port = dp_to_dig_port(intel_dp); > >>> encoder = &dig_port->base; > >>> intel_crtc = to_intel_crtc(encoder->base.crtc); > >>> @@ -5555,6 +5550,11 @@ void intel_edp_drrs_enable(struct intel_dp > *intel_dp, > >>> return; > >>> } > >>> > >>> + if (dev_priv->psr.enabled) { > >>> + DRM_DEBUG_KMS("PSR enabled. Disabling DRRS.\n"); > >>> + return; > >>> + } > >> > >> So every time a flush/invalidate happens, we end up taking the > >> drrs.mutex and then returning because dev_priv->drrs.dp is NULL. That > >> seems unnecessary. Have your considered drrs.type = > DRRS_NOT_SUPPORTED? > > > > That would prevent DRRS testing by disabling PSR via module parameter. > > I think this is fine. > > I mean, I think the change in this patch is fine, preventing DRRS testing is not > fine. > > > > Although the debug message is misleading; it's "not enabling DRRS", > > not "disabling DRRS". There's a difference. > > > > Side note, dev_priv->drrs.type is redundant and could be replaced with > > direct use of dev_priv->vbt.drrs_type. > > > >> And this solution relies on the ordering that psr_enable() is done > >> before drrs_enable(), we need a comment in enable_ddi to make a note > >> of that. A WARN_ON in psr_enable() if drrs is already enabled might > >> work too. > > > > I think a WARN_ON would be fine. I will add a WARN_ON() and send a v3 of the patch. -Radhakrishna > > > > BR, > > Jani. > > > >> > >> > >>> + > >>> mutex_lock(&dev_priv->drrs.mutex); > >>> if (WARN_ON(dev_priv->drrs.dp)) { > >>> DRM_ERROR("DRRS already enabled\n"); > > -- > Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-09-13 23:32 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-31 21:17 [PATCH v2] drm/i915: Disable DRRS when PSR is enabled Radhakrishna Sripada 2017-08-31 21:38 ` ✓ Fi.CI.BAT: success for drm/i915: Disable DRRS when PSR is enabled (rev2) Patchwork 2017-09-01 0:27 ` ✗ Fi.CI.IGT: warning " Patchwork 2017-09-08 22:02 ` [PATCH v2] drm/i915: Disable DRRS when PSR is enabled Pandiyan, Dhinakaran 2017-09-12 14:16 ` Jani Nikula 2017-09-12 14:17 ` Jani Nikula 2017-09-13 23:32 ` Sripada, Radhakrishna
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox