diff mbox series

[2/2] drm/i915/psr: Add debugfs support to force toggling PSR1/2 mode.

Message ID 20180731133559.6892-2-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [1/2] drm/i915: Allow control of PSR at runtime through debugfs, v4. | expand

Commit Message

Maarten Lankhorst July 31, 2018, 1:35 p.m. UTC 
This will make it easier to test PSR1 on PSR2 capable eDP machines.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  2 ++
 drivers/gpu/drm/i915/intel_psr.c | 27 ++++++++++++++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

Comments

Dhinakaran Pandiyan Aug. 8, 2018, 1:35 a.m. UTC | #1 
On Tue, 2018-07-31 at 15:35 +0200, Maarten Lankhorst wrote:
> This will make it easier to test PSR1 on PSR2 capable eDP machines.
Thanks for writing this patch, we can make use of this to fix failures
on PSR2 machines in CI.

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  2 ++
>  drivers/gpu/drm/i915/intel_psr.c | 27 ++++++++++++++++++++++++---
>  2 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index ef04b6cd7863..07783b9e7960 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -617,6 +617,8 @@ struct i915_psr {
>  #define I915_PSR_DEBUG_DEFAULT	(0 << 1)
>  #define I915_PSR_DEBUG_DISABLE	(1 << 1)
>  #define I915_PSR_DEBUG_ENABLE	(2 << 1)
> +#define I915_PSR_DEBUG_FORCE_PSR1 (3 << 1)
> +#define I915_PSR_DEBUG_FORCE_PSR2 (4 << 1)
>  
>  	u32 debug;
>  	bool sink_support;
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 7848829094ca..4cf4ac7068d0 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -813,19 +813,38 @@ static bool __psr_wait_for_idle_locked(struct
> drm_i915_private *dev_priv)
>  	return err == 0 && dev_priv->psr.enabled;
>  }
>  
> +static bool switching_psr(struct drm_i915_private *dev_priv,
> +			  struct intel_crtc_state *crtc_state,
> +			  u32 mode)
> +{
> +	/* Can't switch psr state anyway if PSR2 is not supported.
> */
> +	if (!crtc_state || !crtc_state->has_psr2)
> +		return false;
> +
> +	if (dev_priv->psr.psr2_enabled && mode ==
> I915_PSR_DEBUG_FORCE_PSR1)
> +		return true;
> +
> +	if (!dev_priv->psr.psr2_enabled && mode !=
> I915_PSR_DEBUG_FORCE_PSR1)
> +		return true;
> +
> +	return false;
> +}
> +
>  int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
>  			       struct drm_modeset_acquire_ctx *ctx,
>  			       u64 val)
>  {
>  	struct drm_device *dev = &dev_priv->drm;
>  	struct drm_connector_state *conn_state;
> +	struct intel_crtc_state *crtc_state = NULL;
>  	struct drm_crtc *crtc;
>  	struct intel_dp *dp;
>  	int ret;
>  	bool enable;
> +	u32 mode = val & I915_PSR_DEBUG_MODE_MASK;
>  
>  	if (val & ~(I915_PSR_DEBUG_IRQ | I915_PSR_DEBUG_MODE_MASK)
> ||
> -	    (val & I915_PSR_DEBUG_MODE_MASK) >
> I915_PSR_DEBUG_ENABLE) {
> +	    mode > I915_PSR_DEBUG_FORCE_PSR2) {
>  		DRM_DEBUG_KMS("Invalid debug mask %llx\n", val);
>  		return -EINVAL;
>  	}
> @@ -843,7 +862,8 @@ int intel_psr_set_debugfs_mode(struct
> drm_i915_private *dev_priv,
>  		if (ret)
>  			return ret;
>  
> -		ret = wait_for_completion_interruptible(&crtc-
> >state->commit->hw_done);
> +		crtc_state = to_intel_crtc_state(crtc->state);
> +		ret = wait_for_completion_interruptible(&crtc_state-
> >base.commit->hw_done);
>  	} else
>  		ret = wait_for_completion_interruptible(&conn_state-
> >commit->hw_done);
>  
> @@ -856,10 +876,11 @@ int intel_psr_set_debugfs_mode(struct
> drm_i915_private *dev_priv,
>  
>  	enable = psr_global_enabled(val);
>  
> -	if (!enable)
> +	if (!enable || switching_psr(dev_priv, crtc_state, mode))
>  		intel_psr_disable_locked(dev_priv->psr.dp);
>  
>  	dev_priv->psr.debug = val;
> +	dev_priv->psr.psr2_enabled = mode !=
> I915_PSR_DEBUG_FORCE_PSR1 && crtc_state->has_psr2;
crtc_state can be NULL.

>  	intel_psr_irq_control(dev_priv, dev_priv->psr.debug &
> I915_PSR_DEBUG_IRQ);
>  
>  	if (dev_priv->psr.prepared && enable)


If a modeset was done after writing I915_PSR_DEBUG_FORCE_PSR1, we'll
still end up enabling PSR2. Check for debug flags in intel_psr_enable()
?

-DK
Maarten Lankhorst Aug. 8, 2018, 9:01 a.m. UTC | #2 
Op 08-08-18 om 03:35 schreef Dhinakaran Pandiyan:
> On Tue, 2018-07-31 at 15:35 +0200, Maarten Lankhorst wrote:
>> This will make it easier to test PSR1 on PSR2 capable eDP machines.
> Thanks for writing this patch, we can make use of this to fix failures
> on PSR2 machines in CI.
>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |  2 ++
>>  drivers/gpu/drm/i915/intel_psr.c | 27 ++++++++++++++++++++++++---
>>  2 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index ef04b6cd7863..07783b9e7960 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -617,6 +617,8 @@ struct i915_psr {
>>  #define I915_PSR_DEBUG_DEFAULT	(0 << 1)
>>  #define I915_PSR_DEBUG_DISABLE	(1 << 1)
>>  #define I915_PSR_DEBUG_ENABLE	(2 << 1)
>> +#define I915_PSR_DEBUG_FORCE_PSR1 (3 << 1)
>> +#define I915_PSR_DEBUG_FORCE_PSR2 (4 << 1)
>>  
>>  	u32 debug;
>>  	bool sink_support;
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c
>> b/drivers/gpu/drm/i915/intel_psr.c
>> index 7848829094ca..4cf4ac7068d0 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -813,19 +813,38 @@ static bool __psr_wait_for_idle_locked(struct
>> drm_i915_private *dev_priv)
>>  	return err == 0 && dev_priv->psr.enabled;
>>  }
>>  
>> +static bool switching_psr(struct drm_i915_private *dev_priv,
>> +			  struct intel_crtc_state *crtc_state,
>> +			  u32 mode)
>> +{
>> +	/* Can't switch psr state anyway if PSR2 is not supported.
>> */
>> +	if (!crtc_state || !crtc_state->has_psr2)
>> +		return false;
>> +
>> +	if (dev_priv->psr.psr2_enabled && mode ==
>> I915_PSR_DEBUG_FORCE_PSR1)
>> +		return true;
>> +
>> +	if (!dev_priv->psr.psr2_enabled && mode !=
>> I915_PSR_DEBUG_FORCE_PSR1)
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>>  int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
>>  			       struct drm_modeset_acquire_ctx *ctx,
>>  			       u64 val)
>>  {
>>  	struct drm_device *dev = &dev_priv->drm;
>>  	struct drm_connector_state *conn_state;
>> +	struct intel_crtc_state *crtc_state = NULL;
>>  	struct drm_crtc *crtc;
>>  	struct intel_dp *dp;
>>  	int ret;
>>  	bool enable;
>> +	u32 mode = val & I915_PSR_DEBUG_MODE_MASK;
>>  
>>  	if (val & ~(I915_PSR_DEBUG_IRQ | I915_PSR_DEBUG_MODE_MASK)
>> ||
>> -	    (val & I915_PSR_DEBUG_MODE_MASK) >
>> I915_PSR_DEBUG_ENABLE) {
>> +	    mode > I915_PSR_DEBUG_FORCE_PSR2) {
>>  		DRM_DEBUG_KMS("Invalid debug mask %llx\n", val);
>>  		return -EINVAL;
>>  	}
>> @@ -843,7 +862,8 @@ int intel_psr_set_debugfs_mode(struct
>> drm_i915_private *dev_priv,
>>  		if (ret)
>>  			return ret;
>>  
>> -		ret = wait_for_completion_interruptible(&crtc-
>>> state->commit->hw_done);
>> +		crtc_state = to_intel_crtc_state(crtc->state);
>> +		ret = wait_for_completion_interruptible(&crtc_state-
>>> base.commit->hw_done);
>>  	} else
>>  		ret = wait_for_completion_interruptible(&conn_state-
>>> commit->hw_done);
>>  
>> @@ -856,10 +876,11 @@ int intel_psr_set_debugfs_mode(struct
>> drm_i915_private *dev_priv,
>>  
>>  	enable = psr_global_enabled(val);
>>  
>> -	if (!enable)
>> +	if (!enable || switching_psr(dev_priv, crtc_state, mode))
>>  		intel_psr_disable_locked(dev_priv->psr.dp);
>>  
>>  	dev_priv->psr.debug = val;
>> +	dev_priv->psr.psr2_enabled = mode !=
>> I915_PSR_DEBUG_FORCE_PSR1 && crtc_state->has_psr2;
> crtc_state can be NULL.
Right, will fix.
>
>>  	intel_psr_irq_control(dev_priv, dev_priv->psr.debug &
>> I915_PSR_DEBUG_IRQ);
>>  
>>  	if (dev_priv->psr.prepared && enable)
>
> If a modeset was done after writing I915_PSR_DEBUG_FORCE_PSR1, we'll
> still end up enabling PSR2. Check for debug flags in intel_psr_enable()
> ?
Hmm.. good point, will add the same check there.

~Maarten
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ef04b6cd7863..07783b9e7960 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -617,6 +617,8 @@  struct i915_psr {
 #define I915_PSR_DEBUG_DEFAULT	(0 << 1)
 #define I915_PSR_DEBUG_DISABLE	(1 << 1)
 #define I915_PSR_DEBUG_ENABLE	(2 << 1)
+#define I915_PSR_DEBUG_FORCE_PSR1 (3 << 1)
+#define I915_PSR_DEBUG_FORCE_PSR2 (4 << 1)
 
 	u32 debug;
 	bool sink_support;
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 7848829094ca..4cf4ac7068d0 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -813,19 +813,38 @@  static bool __psr_wait_for_idle_locked(struct drm_i915_private *dev_priv)
 	return err == 0 && dev_priv->psr.enabled;
 }
 
+static bool switching_psr(struct drm_i915_private *dev_priv,
+			  struct intel_crtc_state *crtc_state,
+			  u32 mode)
+{
+	/* Can't switch psr state anyway if PSR2 is not supported. */
+	if (!crtc_state || !crtc_state->has_psr2)
+		return false;
+
+	if (dev_priv->psr.psr2_enabled && mode == I915_PSR_DEBUG_FORCE_PSR1)
+		return true;
+
+	if (!dev_priv->psr.psr2_enabled && mode != I915_PSR_DEBUG_FORCE_PSR1)
+		return true;
+
+	return false;
+}
+
 int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
 			       struct drm_modeset_acquire_ctx *ctx,
 			       u64 val)
 {
 	struct drm_device *dev = &dev_priv->drm;
 	struct drm_connector_state *conn_state;
+	struct intel_crtc_state *crtc_state = NULL;
 	struct drm_crtc *crtc;
 	struct intel_dp *dp;
 	int ret;
 	bool enable;
+	u32 mode = val & I915_PSR_DEBUG_MODE_MASK;
 
 	if (val & ~(I915_PSR_DEBUG_IRQ | I915_PSR_DEBUG_MODE_MASK) ||
-	    (val & I915_PSR_DEBUG_MODE_MASK) > I915_PSR_DEBUG_ENABLE) {
+	    mode > I915_PSR_DEBUG_FORCE_PSR2) {
 		DRM_DEBUG_KMS("Invalid debug mask %llx\n", val);
 		return -EINVAL;
 	}
@@ -843,7 +862,8 @@  int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
 		if (ret)
 			return ret;
 
-		ret = wait_for_completion_interruptible(&crtc->state->commit->hw_done);
+		crtc_state = to_intel_crtc_state(crtc->state);
+		ret = wait_for_completion_interruptible(&crtc_state->base.commit->hw_done);
 	} else
 		ret = wait_for_completion_interruptible(&conn_state->commit->hw_done);
 
@@ -856,10 +876,11 @@  int intel_psr_set_debugfs_mode(struct drm_i915_private *dev_priv,
 
 	enable = psr_global_enabled(val);
 
-	if (!enable)
+	if (!enable || switching_psr(dev_priv, crtc_state, mode))
 		intel_psr_disable_locked(dev_priv->psr.dp);
 
 	dev_priv->psr.debug = val;
+	dev_priv->psr.psr2_enabled = mode != I915_PSR_DEBUG_FORCE_PSR1 && crtc_state->has_psr2;
 	intel_psr_irq_control(dev_priv, dev_priv->psr.debug & I915_PSR_DEBUG_IRQ);
 
 	if (dev_priv->psr.prepared && enable)