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

[Maarten Lankhorst]

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 at 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