[PATCH 1/2] drm/i915: Add psr_ready on pipe_config

[PATCH 1/2] drm/i915: Add psr_ready on pipe_config

[Rodrigo Vivi]

Let's know beforehand if PSR is ready and will be enabled so we can
prevent DRRS to get enabled.

v2: Removing is_edp_psr func that is not used after this patch.
    Rename match_conditions and document it since it is now external.
    Moving to a propper place as pointed out by Sivakumar.
    Use a better name as pointed out by Ram.

v3: Don't dereferrence drm_encoder->crtc and intel_crtc->config on psr_ready check.
    Fix a opps caused with previous versions.

Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Cc: Ramalingam C <ramalingam.c@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: Ramalingam C <ramalingam.c@intel.com> (v2)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  1 +
 drivers/gpu/drm/i915/intel_dp.c      |  2 ++
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++
 drivers/gpu/drm/i915/intel_psr.c     | 57 ++++++++++++++++++++----------------
 4 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 75afa6e..50f2db7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10895,6 +10895,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 		      pipe_config->pch_pfit.pos,
 		      pipe_config->pch_pfit.size,
 		      pipe_config->pch_pfit.enabled ? "enabled" : "disabled");
+	DRM_DEBUG_KMS("psr ready: %i\n", pipe_config->psr_ready);
 	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
 	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 14cdd00..94bbdf4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1394,6 +1394,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		 */
 		min_lane_count = max_lane_count;
 		min_clock = max_clock;
+
+		pipe_config->psr_ready = intel_psr_ready(intel_dp, pipe_config);
 	}
 
 	for (; bpp >= 6*3; bpp -= 2*3) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 082be71..9895772 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -448,6 +448,7 @@ struct intel_crtc_state {
 	int fdi_lanes;
 	struct intel_link_m_n fdi_m_n;
 
+	bool psr_ready;
 	bool ips_enabled;
 
 	bool double_wide;
@@ -1287,6 +1288,8 @@ void intel_backlight_unregister(struct drm_device *dev);
 
 
 /* intel_psr.c */
+bool intel_psr_ready(struct intel_dp *intel_dp,
+		     struct intel_crtc_state *pipe_config);
 void intel_psr_enable(struct intel_dp *intel_dp);
 void intel_psr_disable(struct intel_dp *intel_dp);
 void intel_psr_invalidate(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 5ee0fa5..61d582b 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,11 +56,6 @@
 #include "intel_drv.h"
 #include "i915_drv.h"
 
-static bool is_edp_psr(struct intel_dp *intel_dp)
-{
-	return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
-}
-
 static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -282,19 +277,32 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
 }
 
-static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
+/**
+ * intel_psr_ready - PSR ready
+ * @intel_dp: Intel DP
+ * @pipe_config: Pipe Config
+ *
+ * This function Checks if PSR is supported by Hardware/Source and
+ * Panel/Sink and if all conditions to be enabled are fulfilled.
+ *
+ * It is used to know beforehand if PSR is going to be enabled.
+ *
+ * Returns:
+ * True when PSR is ready to be enabled, false otherwise.
+ */
+bool intel_psr_ready(struct intel_dp *intel_dp,
+		     struct intel_crtc_state *pipe_config)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc *crtc = dig_port->base.base.crtc;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	lockdep_assert_held(&dev_priv->psr.lock);
-	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+	if (!HAS_PSR(dev)) {
+		DRM_DEBUG_KMS("PSR not supported on this platform\n");
+		return false;
+	}
 
-	dev_priv->psr.source_ok = false;
+	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 
 	if (IS_HASWELL(dev) && dig_port->port != PORT_A) {
 		DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
@@ -307,14 +315,14 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 	}
 
 	if (IS_HASWELL(dev) &&
-	    I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config->cpu_transcoder)) &
+	    I915_READ(HSW_STEREO_3D_CTL(pipe_config->cpu_transcoder)) &
 		      S3D_ENABLE) {
 		DRM_DEBUG_KMS("PSR condition failed: Stereo 3D is Enabled\n");
 		return false;
 	}
 
 	if (IS_HASWELL(dev) &&
-	    intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
+	    pipe_config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
 		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
 		return false;
 	}
@@ -325,10 +333,19 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 		return false;
 	}
 
+	/* At this point we can tell HW/Source supports PSR */
 	dev_priv->psr.source_ok = true;
+
+	/* Now check if Panel/Sink supports it */
+	if (!dev_priv->psr.sink_support) {
+		DRM_DEBUG_KMS("PSR not supported by this panel\n");
+		return false;
+	}
+
 	return true;
 }
 
+
 static void intel_psr_activate(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -365,15 +382,8 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
 
-	if (!HAS_PSR(dev)) {
-		DRM_DEBUG_KMS("PSR not supported on this platform\n");
-		return;
-	}
-
-	if (!is_edp_psr(intel_dp)) {
-		DRM_DEBUG_KMS("PSR not supported by this panel\n");
+	if (!crtc->config->psr_ready)
 		return;
-	}
 
 	mutex_lock(&dev_priv->psr.lock);
 	if (dev_priv->psr.enabled) {
@@ -381,9 +391,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		goto unlock;
 	}
 
-	if (!intel_psr_match_conditions(intel_dp))
-		goto unlock;
-
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
 	if (HAS_DDI(dev)) {
-- 
2.1.0

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

[PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Rodrigo Vivi]

With PSR enabled being pre computed on pipe_config we can now
prevent DRRS to be enabled along with PSR.

v2: Rebase after changing previous patch

Cc: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed-by: Ramalingam C <ramalingam.c@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 94bbdf4..bf4b0cf 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -5011,11 +5011,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 		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);
@@ -5101,6 +5096,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
 		return;
 	}
 
+	if (intel_crtc->config->psr_ready) {
+		DRM_DEBUG_KMS("DRRS: PSR will be enabled on this crtc\n");
+		return;
+	}
+
 	mutex_lock(&dev_priv->drrs.mutex);
 	if (WARN_ON(dev_priv->drrs.dp)) {
 		DRM_ERROR("DRRS already enabled\n");
-- 
2.1.0

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

[PATCH] drm/i915: Add psr_ready on pipe_config

[Rodrigo Vivi]

Let's know beforehand if PSR is ready and will be enabled so we can
prevent DRRS to get enabled.

WARN_ON(!drm_modeset_is_locked(&crtc->mutex)) on intel_psr_ready()
has been removed on v3. We don't dereferrence crtc here anymore so
we don't need this check. All configs are now checked from received
pipe config.

v2: Removing is_edp_psr func that is not used after this patch.
    Rename match_conditions and document it since it is now external.
    Moving to a propper place as pointed out by Sivakumar.
    Use a better name as pointed out by Ram.

v3: Don't dereferrence drm_encoder->crtc and intel_crtc->config on psr_ready
    check. Fix a opps caused with previous versions.

v4: Mention and explain on commit message the crtc->mutex check removal that
    happened on v3.

Cc: Sivakumar Thulasimani <sivakumar.thulasimani@intel.com>
Cc: Ramalingam C <ramalingam.c@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel@ffwll.ch>
Reviewed-by: Ramalingam C <ramalingam.c@intel.com> (v2)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  1 +
 drivers/gpu/drm/i915/intel_dp.c      |  2 ++
 drivers/gpu/drm/i915/intel_drv.h     |  3 ++
 drivers/gpu/drm/i915/intel_psr.c     | 57 ++++++++++++++++++++----------------
 4 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 75afa6e..50f2db7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10895,6 +10895,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 		      pipe_config->pch_pfit.pos,
 		      pipe_config->pch_pfit.size,
 		      pipe_config->pch_pfit.enabled ? "enabled" : "disabled");
+	DRM_DEBUG_KMS("psr ready: %i\n", pipe_config->psr_ready);
 	DRM_DEBUG_KMS("ips: %i\n", pipe_config->ips_enabled);
 	DRM_DEBUG_KMS("double wide: %i\n", pipe_config->double_wide);
 
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 14cdd00..94bbdf4 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1394,6 +1394,8 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		 */
 		min_lane_count = max_lane_count;
 		min_clock = max_clock;
+
+		pipe_config->psr_ready = intel_psr_ready(intel_dp, pipe_config);
 	}
 
 	for (; bpp >= 6*3; bpp -= 2*3) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 082be71..9895772 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -448,6 +448,7 @@ struct intel_crtc_state {
 	int fdi_lanes;
 	struct intel_link_m_n fdi_m_n;
 
+	bool psr_ready;
 	bool ips_enabled;
 
 	bool double_wide;
@@ -1287,6 +1288,8 @@ void intel_backlight_unregister(struct drm_device *dev);
 
 
 /* intel_psr.c */
+bool intel_psr_ready(struct intel_dp *intel_dp,
+		     struct intel_crtc_state *pipe_config);
 void intel_psr_enable(struct intel_dp *intel_dp);
 void intel_psr_disable(struct intel_dp *intel_dp);
 void intel_psr_invalidate(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 5ee0fa5..61d582b 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,11 +56,6 @@
 #include "intel_drv.h"
 #include "i915_drv.h"
 
-static bool is_edp_psr(struct intel_dp *intel_dp)
-{
-	return intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED;
-}
-
 static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -282,19 +277,32 @@ static void hsw_psr_enable_source(struct intel_dp *intel_dp)
 				EDP_SU_TRACK_ENABLE | EDP_PSR2_TP2_TIME_100);
 }
 
-static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
+/**
+ * intel_psr_ready - PSR ready
+ * @intel_dp: Intel DP
+ * @pipe_config: Pipe Config
+ *
+ * This function Checks if PSR is supported by Hardware/Source and
+ * Panel/Sink and if all conditions to be enabled are fulfilled.
+ *
+ * It is used to know beforehand if PSR is going to be enabled.
+ *
+ * Returns:
+ * True when PSR is ready to be enabled, false otherwise.
+ */
+bool intel_psr_ready(struct intel_dp *intel_dp,
+		     struct intel_crtc_state *pipe_config)
 {
 	struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc *crtc = dig_port->base.base.crtc;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	lockdep_assert_held(&dev_priv->psr.lock);
-	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+	if (!HAS_PSR(dev)) {
+		DRM_DEBUG_KMS("PSR not supported on this platform\n");
+		return false;
+	}
 
-	dev_priv->psr.source_ok = false;
+	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 
 	if (IS_HASWELL(dev) && dig_port->port != PORT_A) {
 		DRM_DEBUG_KMS("HSW ties PSR to DDI A (eDP)\n");
@@ -307,14 +315,14 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 	}
 
 	if (IS_HASWELL(dev) &&
-	    I915_READ(HSW_STEREO_3D_CTL(intel_crtc->config->cpu_transcoder)) &
+	    I915_READ(HSW_STEREO_3D_CTL(pipe_config->cpu_transcoder)) &
 		      S3D_ENABLE) {
 		DRM_DEBUG_KMS("PSR condition failed: Stereo 3D is Enabled\n");
 		return false;
 	}
 
 	if (IS_HASWELL(dev) &&
-	    intel_crtc->config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
+	    pipe_config->base.adjusted_mode.flags & DRM_MODE_FLAG_INTERLACE) {
 		DRM_DEBUG_KMS("PSR condition failed: Interlaced is Enabled\n");
 		return false;
 	}
@@ -325,10 +333,19 @@ static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
 		return false;
 	}
 
+	/* At this point we can tell HW/Source supports PSR */
 	dev_priv->psr.source_ok = true;
+
+	/* Now check if Panel/Sink supports it */
+	if (!dev_priv->psr.sink_support) {
+		DRM_DEBUG_KMS("PSR not supported by this panel\n");
+		return false;
+	}
+
 	return true;
 }
 
+
 static void intel_psr_activate(struct intel_dp *intel_dp)
 {
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
@@ -365,15 +382,8 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
 
-	if (!HAS_PSR(dev)) {
-		DRM_DEBUG_KMS("PSR not supported on this platform\n");
-		return;
-	}
-
-	if (!is_edp_psr(intel_dp)) {
-		DRM_DEBUG_KMS("PSR not supported by this panel\n");
+	if (!crtc->config->psr_ready)
 		return;
-	}
 
 	mutex_lock(&dev_priv->psr.lock);
 	if (dev_priv->psr.enabled) {
@@ -381,9 +391,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
 		goto unlock;
 	}
 
-	if (!intel_psr_match_conditions(intel_dp))
-		goto unlock;
-
 	dev_priv->psr.busy_frontbuffer_bits = 0;
 
 	if (HAS_DDI(dev)) {
-- 
2.1.0

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

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6206
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  318/318              318/318
IVB                                  341/341              341/341
BYT                                  287/287              287/287
HSW                 -2              395/395              393/395
BDW                                  318/318              318/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 HSW  igt@gem_pwrite_pread@snooped-copy-performance      DMESG_WARN(1)PASS(4)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck timer elapsed... blitter ring idle
*HSW  igt@gem_pwrite_pread@snooped-pwrite-blt-cpu_mmap-performance      PASS(6)      DMESG_WARN(2)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck timer elapsed... blitter ring idle
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Add psr_ready on pipe_config

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6208
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  302/302              302/302
SNB                                  318/318              318/318
IVB                                  341/341              341/341
BYT                                  287/287              287/287
HSW                                  395/395              395/395
BDW                                  318/318              318/318
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Daniel Vetter]

On Mon, Mar 30, 2015 at 05:39:54PM +0200, Daniel Vetter wrote:
> On Mon, Mar 30, 2015 at 03:19:29PM +0530, Ramalingam C wrote:
> > Looks good to me..
> > 
> > Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> > 
> > On Friday 27 March 2015 12:51 AM, Rodrigo Vivi wrote:
> > >With PSR enabled being pre computed on pipe_config we can now
> > >prevent DRRS to be enabled along with PSR.
> > >
> > >v2: Rebase after changing previous patch
> > >
> > >Cc: Ramalingam C <ramalingam.c@intel.com>
> > >Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Both patches applied, thanks.

Both dropped again since it blows up - in compute_config functions you are
not allowed to look at the current configuration ever. And you do that in
two places with this code:
- deref drm_encoder->crtc, which results in oopses
- deref intel_crtc->config, which is just semantically wrong.

See Chris patch for how to fix this (although it's slightly incomplete
since it doesn't replace all access to intel_crtc->config with the
explicitly passed-in pipe_config pointer).
-Daniel

> -Daniel
> 
> > >---
> > >  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 e6b1c42..5551b3c 100644
> > >--- a/drivers/gpu/drm/i915/intel_dp.c
> > >+++ b/drivers/gpu/drm/i915/intel_dp.c
> > >@@ -4994,11 +4994,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
> > >  		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 = encoder->new_crtc;
> > >@@ -5084,6 +5079,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
> > >  		return;
> > >  	}
> > >+	if (intel_crtc->config->psr_ready) {
> > >+		DRM_DEBUG_KMS("DRRS: PSR will be enabled on this crtc\n");
> > >+		return;
> > >+	}
> > >+
> > >  	mutex_lock(&dev_priv->drrs.mutex);
> > >  	if (WARN_ON(dev_priv->drrs.dp)) {
> > >  		DRM_ERROR("DRRS already enabled\n");
> > 
> > -- 
> > Thanks,
> > --Ram
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Daniel Vetter]

On Mon, Mar 30, 2015 at 03:19:29PM +0530, Ramalingam C wrote:
> Looks good to me..
> 
> Reviewed-by: Ramalingam C <ramalingam.c@intel.com>
> 
> On Friday 27 March 2015 12:51 AM, Rodrigo Vivi wrote:
> >With PSR enabled being pre computed on pipe_config we can now
> >prevent DRRS to be enabled along with PSR.
> >
> >v2: Rebase after changing previous patch
> >
> >Cc: Ramalingam C <ramalingam.c@intel.com>
> >Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Both patches applied, thanks.
-Daniel

> >---
> >  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 e6b1c42..5551b3c 100644
> >--- a/drivers/gpu/drm/i915/intel_dp.c
> >+++ b/drivers/gpu/drm/i915/intel_dp.c
> >@@ -4994,11 +4994,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
> >  		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 = encoder->new_crtc;
> >@@ -5084,6 +5079,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
> >  		return;
> >  	}
> >+	if (intel_crtc->config->psr_ready) {
> >+		DRM_DEBUG_KMS("DRRS: PSR will be enabled on this crtc\n");
> >+		return;
> >+	}
> >+
> >  	mutex_lock(&dev_priv->drrs.mutex);
> >  	if (WARN_ON(dev_priv->drrs.dp)) {
> >  		DRM_ERROR("DRRS already enabled\n");
> 
> -- 
> Thanks,
> --Ram
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Ramalingam C]

Looks good to me..

Reviewed-by: Ramalingam C <ramalingam.c@intel.com>

On Friday 27 March 2015 12:51 AM, Rodrigo Vivi wrote:
> With PSR enabled being pre computed on pipe_config we can now
> prevent DRRS to be enabled along with PSR.
>
> v2: Rebase after changing previous patch
>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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 e6b1c42..5551b3c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4994,11 +4994,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
>   		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 = encoder->new_crtc;
> @@ -5084,6 +5079,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
>   		return;
>   	}
>   
> +	if (intel_crtc->config->psr_ready) {
> +		DRM_DEBUG_KMS("DRRS: PSR will be enabled on this crtc\n");
> +		return;
> +	}
> +
>   	mutex_lock(&dev_priv->drrs.mutex);
>   	if (WARN_ON(dev_priv->drrs.dp)) {
>   		DRM_ERROR("DRRS already enabled\n");

-- 
Thanks,
--Ram

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

[PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Rodrigo Vivi]

With PSR enabled being pre computed on pipe_config we can now
prevent DRRS to be enabled along with PSR.

v2: Rebase after changing previous patch

Cc: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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 e6b1c42..5551b3c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4994,11 +4994,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 		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 = encoder->new_crtc;
@@ -5084,6 +5079,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
 		return;
 	}
 
+	if (intel_crtc->config->psr_ready) {
+		DRM_DEBUG_KMS("DRRS: PSR will be enabled on this crtc\n");
+		return;
+	}
+
 	mutex_lock(&dev_priv->drrs.mutex);
 	if (WARN_ON(dev_priv->drrs.dp)) {
 		DRM_ERROR("DRRS already enabled\n");
-- 
2.1.0

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

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Ramalingam C]

On Wednesday 25 March 2015 09:11 PM, Vivi, Rodrigo wrote:
> On Wed, 2015-03-25 at 16:08 +0530, Ramalingam C wrote:
>> On Wednesday 25 March 2015 12:42 AM, Rodrigo Vivi wrote:
>>> With PSR enabled being pre computed on pipe_config we can now
>>> prevent DRRS to be enabled along with PSR.
>>>
>>> Cc: Ramalingam C <ramalingam.c@intel.com>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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 637dd53..af28833 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -4992,11 +4992,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
>>>    		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 = encoder->new_crtc;
>>> @@ -5082,6 +5077,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
>>>    		return;
>>>    	}
>>>    
>>> +	if (intel_crtc->config->psr_enabled) {
>>> +		DRM_DEBUG_KMS("DRRS: PSR enabled on this crtc\n");
>>> +		return;
>>> +	}
>>> +
>> But you might want to explicitly disable the DRRS when PSR is supported.
>> Because incase DRRS is enabled before PSR,
>> then blocking the enable call wont help us to stop DRRS.
>> In fact I would have suggested to place this in DRRS init path.  But a
>> feature called media playback DRRS(in RFC stage)
>> needs DRRS active even when PSR is enabled/supported. So dont want to
>> block the DRRS init path because PSR is supported.
>> Disabling alone will be sufficient.
>>
>> So as soon as PSR support is detected you would like to explicitly
>> disable the DRRS(if it is supported)
>> along with blocking further drrs_enable calls.
> Not actually. Please take a look on the other patch on this series.
> If possible please review both.
>
> intel_crtc->config->psr_enabled is pre-computed and comes before DRRS
> enabling. So with this pre-computed psr enable flag we don't need to
> switch other states because DRRS won't be enabled never.

Thanks for the explanation. Gone through the previous patch in this 
series. As this flag psr_enabled is
getting computed well before the PSR and DRRS enable, then this patch 
will be sufficient to
disable the DRRS.

>
>>>    	mutex_lock(&dev_priv->drrs.mutex);
>>>    	if (WARN_ON(dev_priv->drrs.dp)) {
>>>    		DRM_ERROR("DRRS already enabled\n");

Thanks,
--Ram

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

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Vivi, Rodrigo]

On Wed, 2015-03-25 at 16:08 +0530, Ramalingam C wrote:
> On Wednesday 25 March 2015 12:42 AM, Rodrigo Vivi wrote:
> > With PSR enabled being pre computed on pipe_config we can now
> > prevent DRRS to be enabled along with PSR.
> >
> > Cc: Ramalingam C <ramalingam.c@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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 637dd53..af28833 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4992,11 +4992,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
> >   		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 = encoder->new_crtc;
> > @@ -5082,6 +5077,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
> >   		return;
> >   	}
> >   
> > +	if (intel_crtc->config->psr_enabled) {
> > +		DRM_DEBUG_KMS("DRRS: PSR enabled on this crtc\n");
> > +		return;
> > +	}
> > +
> But you might want to explicitly disable the DRRS when PSR is supported. 
> Because incase DRRS is enabled before PSR,
> then blocking the enable call wont help us to stop DRRS.
> In fact I would have suggested to place this in DRRS init path.  But a 
> feature called media playback DRRS(in RFC stage)
> needs DRRS active even when PSR is enabled/supported. So dont want to 
> block the DRRS init path because PSR is supported.
> Disabling alone will be sufficient.
> 
> So as soon as PSR support is detected you would like to explicitly 
> disable the DRRS(if it is supported)
> along with blocking further drrs_enable calls.

Not actually. Please take a look on the other patch on this series.
If possible please review both.

intel_crtc->config->psr_enabled is pre-computed and comes before DRRS
enabling. So with this pre-computed psr enable flag we don't need to
switch other states because DRRS won't be enabled never.

> >   	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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Ramalingam C]

On Wednesday 25 March 2015 12:42 AM, Rodrigo Vivi wrote:
> With PSR enabled being pre computed on pipe_config we can now
> prevent DRRS to be enabled along with PSR.
>
> Cc: Ramalingam C <ramalingam.c@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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 637dd53..af28833 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4992,11 +4992,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
>   		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 = encoder->new_crtc;
> @@ -5082,6 +5077,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
>   		return;
>   	}
>   
> +	if (intel_crtc->config->psr_enabled) {
> +		DRM_DEBUG_KMS("DRRS: PSR enabled on this crtc\n");
> +		return;
> +	}
> +
But you might want to explicitly disable the DRRS when PSR is supported. 
Because incase DRRS is enabled before PSR,
then blocking the enable call wont help us to stop DRRS.
In fact I would have suggested to place this in DRRS init path.  But a 
feature called media playback DRRS(in RFC stage)
needs DRRS active even when PSR is enabled/supported. So dont want to 
block the DRRS init path because PSR is supported.
Disabling alone will be sufficient.

So as soon as PSR support is detected you would like to explicitly 
disable the DRRS(if it is supported)
along with blocking further drrs_enable calls.
>   	mutex_lock(&dev_priv->drrs.mutex);
>   	if (WARN_ON(dev_priv->drrs.dp)) {
>   		DRM_ERROR("DRRS already enabled\n");

-- 
Ram

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

Re: [PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[shuang.he]

Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6043
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                 -1              269/269              268/269
ILK                                  303/303              303/303
SNB                                  304/304              304/304
IVB                                  338/338              338/338
BYT                                  287/287              287/287
HSW                                  362/362              362/362
BDW                                  310/310              310/310
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
 PNV  igt@gem_userptr_blits@coherency-sync      CRASH(1)PASS(3)      CRASH(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915: Only enabled DRRS if PRS won't be enabled on this pipe.

[Rodrigo Vivi]

With PSR enabled being pre computed on pipe_config we can now
prevent DRRS to be enabled along with PSR.

Cc: Ramalingam C <ramalingam.c@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@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 637dd53..af28833 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4992,11 +4992,6 @@ static void intel_dp_set_drrs_state(struct drm_device *dev, int refresh_rate)
 		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 = encoder->new_crtc;
@@ -5082,6 +5077,11 @@ void intel_edp_drrs_enable(struct intel_dp *intel_dp)
 		return;
 	}
 
+	if (intel_crtc->config->psr_enabled) {
+		DRM_DEBUG_KMS("DRRS: PSR enabled on this crtc\n");
+		return;
+	}
+
 	mutex_lock(&dev_priv->drrs.mutex);
 	if (WARN_ON(dev_priv->drrs.dp)) {
 		DRM_ERROR("DRRS already enabled\n");
-- 
2.1.0

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