diff mbox

[v3] drm/i915: Do not enable DRRS when PSR is enabled

Message ID 20170914181641.24393-1-radhakrishna.sripada@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sripada, Radhakrishna Sept. 14, 2017, 6:16 p.m. UTC
Some platforms do not 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
V3: Add a WARN_ON during PSR init suggested by Dhinakaran
    Correct debug message,title suggested by Jani

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 +++++-----
 drivers/gpu/drm/i915/intel_psr.c |  1 +
 2 files changed, 6 insertions(+), 5 deletions(-)

Comments

Rodrigo Vivi Sept. 18, 2017, 7:35 p.m. UTC | #1
On Thu, Sep 14, 2017 at 06:16:41PM +0000, Radhakrishna Sripada wrote:
> Some platforms do not 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
> V3: Add a WARN_ON during PSR init suggested by Dhinakaran
>     Correct debug message,title suggested by Jani
> 
> 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>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 10 +++++-----
>  drivers/gpu/drm/i915/intel_psr.c |  1 +
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 887953c0f495..8db6b11f103f 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. Not enabling DRRS.\n");
> +		return;
> +	}
> +
>  	mutex_lock(&dev_priv->drrs.mutex);
>  	if (WARN_ON(dev_priv->drrs.dp)) {
>  		DRM_ERROR("DRRS already enabled\n");
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 55b4002bbe53..acb50945bfa8 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -530,6 +530,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
>  		return;
>  	}
>  
> +	WARN_ON(dev_priv->drrs.dp);
>  	mutex_lock(&dev_priv->psr.lock);
>  	if (dev_priv->psr.enabled) {
>  		DRM_DEBUG_KMS("PSR already in use\n");
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rodrigo Vivi Sept. 18, 2017, 7:50 p.m. UTC | #2
On Mon, Sep 18, 2017 at 12:35 PM, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Thu, Sep 14, 2017 at 06:16:41PM +0000, Radhakrishna Sripada wrote:
>> Some platforms do not 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
>> V3: Add a WARN_ON during PSR init suggested by Dhinakaran
>>     Correct debug message,title suggested by Jani
>>
>> 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>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

merged to dinq. thanks for the patch.

>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c  | 10 +++++-----
>>  drivers/gpu/drm/i915/intel_psr.c |  1 +
>>  2 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 887953c0f495..8db6b11f103f 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. Not enabling DRRS.\n");
>> +             return;
>> +     }
>> +
>>       mutex_lock(&dev_priv->drrs.mutex);
>>       if (WARN_ON(dev_priv->drrs.dp)) {
>>               DRM_ERROR("DRRS already enabled\n");
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
>> index 55b4002bbe53..acb50945bfa8 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -530,6 +530,7 @@ void intel_psr_enable(struct intel_dp *intel_dp,
>>               return;
>>       }
>>
>> +     WARN_ON(dev_priv->drrs.dp);
>>       mutex_lock(&dev_priv->psr.lock);
>>       if (dev_priv->psr.enabled) {
>>               DRM_DEBUG_KMS("PSR already in use\n");
>> --
>> 2.9.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 887953c0f495..8db6b11f103f 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. Not enabling DRRS.\n");
+		return;
+	}
+
 	mutex_lock(&dev_priv->drrs.mutex);
 	if (WARN_ON(dev_priv->drrs.dp)) {
 		DRM_ERROR("DRRS already enabled\n");
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 55b4002bbe53..acb50945bfa8 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -530,6 +530,7 @@  void intel_psr_enable(struct intel_dp *intel_dp,
 		return;
 	}
 
+	WARN_ON(dev_priv->drrs.dp);
 	mutex_lock(&dev_priv->psr.lock);
 	if (dev_priv->psr.enabled) {
 		DRM_DEBUG_KMS("PSR already in use\n");