diff mbox

drm/i915: add update function to disable/enable-back PSR

Message ID 1372441468-3125-1-git-send-email-rodrigo.vivi@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rodrigo Vivi June 28, 2013, 5:44 p.m. UTC
Required function to disable PSR when going to console mode.
But also can be used whenever PSR mode entry conditions changed.

v2: Add it before PSR Hook. Update function not really been called yet.
v3: Fix coding style detected by checkpatch by Paulo Zanoni.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 37 ++++++++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 2 files changed, 31 insertions(+), 7 deletions(-)

Comments

Paulo Zanoni July 5, 2013, 10:48 p.m. UTC | #1
2013/6/28 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
> Required function to disable PSR when going to console mode.
> But also can be used whenever PSR mode entry conditions changed.
>
> v2: Add it before PSR Hook. Update function not really been called yet.
> v3: Fix coding style detected by checkpatch by Paulo Zanoni.
>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 37 ++++++++++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  2 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 9730d6b..86c1a7d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1536,14 +1536,8 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
>         return true;
>  }
>
> -void intel_edp_psr_enable(struct intel_dp *intel_dp)
> +void intel_edp_psr_do_enable(struct intel_dp *intel_dp)

Then this function should become static.

So, how often do we disable/enable or update after a mode set? I see
that on enable_sink we reset the VSC header, rewrite the AUX
registers, also reset EDP_PSR_DEBUG_CTL and reupdate everything inside
EDP_PSR_CTL. Do we need to redo all this? Don't we just need to redo
the aux_native_write to DP_PSR_EN_CFG and re-enable EDP_PSR_ENABLE
since everything else might still be the same on the same mode-set?

>  {
> -       struct drm_device *dev = intel_dp_to_dev(intel_dp);
> -
> -       if (!intel_edp_psr_match_conditions(intel_dp) ||
> -           intel_edp_is_psr_enabled(dev))
> -               return;
> -
>         /* Enable PSR on the panel */
>         intel_edp_psr_enable_sink(intel_dp);
>
> @@ -1551,6 +1545,15 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp)
>         intel_edp_psr_enable_source(intel_dp);
>  }
>
> +void intel_edp_psr_enable(struct intel_dp *intel_dp)
> +{
> +       struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +
> +       if (intel_edp_psr_match_conditions(intel_dp) &&
> +           !intel_edp_is_psr_enabled(dev))
> +               intel_edp_psr_do_enable(intel_dp);
> +}
> +
>  void intel_edp_psr_disable(struct intel_dp *intel_dp)
>  {
>         struct drm_device *dev = intel_dp_to_dev(intel_dp);
> @@ -1573,6 +1576,26 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
>         intel_wait_for_vblank(dev, intel_crtc->pipe);
>  }
>
> +void intel_edp_psr_update(struct drm_device *dev)
> +{
> +       struct intel_encoder *encoder;
> +       struct intel_dp *intel_dp = NULL;
> +
> +       list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head)
> +               if (encoder->type == INTEL_OUTPUT_EDP) {
> +                       intel_dp = enc_to_intel_dp(&encoder->base);
> +
> +                       if (!is_edp_psr(intel_dp))
> +                               return;
> +
> +                       if (!intel_edp_psr_match_conditions(intel_dp))
> +                               intel_edp_psr_disable(intel_dp);
> +                       else
> +                               if (!intel_edp_is_psr_enabled(dev))
> +                                       intel_edp_psr_do_enable(intel_dp);
> +               }
> +}
> +
>  static void intel_disable_dp(struct intel_encoder *encoder)
>  {
>         struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ff09c4c..1f638cf 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -842,5 +842,6 @@ extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
>
>  extern void intel_edp_psr_enable(struct intel_dp *intel_dp);
>  extern void intel_edp_psr_disable(struct intel_dp *intel_dp);
> +extern void intel_edp_psr_update(struct drm_device *dev);
>
>  #endif /* __INTEL_DRV_H__ */
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rodrigo Vivi July 8, 2013, 9:52 p.m. UTC | #2
On Fri, Jul 5, 2013 at 7:48 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
> 2013/6/28 Rodrigo Vivi <rodrigo.vivi@gmail.com>:
>> Required function to disable PSR when going to console mode.
>> But also can be used whenever PSR mode entry conditions changed.
>>
>> v2: Add it before PSR Hook. Update function not really been called yet.
>> v3: Fix coding style detected by checkpatch by Paulo Zanoni.
>>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c  | 37 ++++++++++++++++++++++++++++++-------
>>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>>  2 files changed, 31 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 9730d6b..86c1a7d 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1536,14 +1536,8 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
>>         return true;
>>  }
>>
>> -void intel_edp_psr_enable(struct intel_dp *intel_dp)
>> +void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
>
> Then this function should become static.
>
> So, how often do we disable/enable or update after a mode set? I see
> that on enable_sink we reset the VSC header, rewrite the AUX
> registers, also reset EDP_PSR_DEBUG_CTL and reupdate everything inside
> EDP_PSR_CTL. Do we need to redo all this? Don't we just need to redo
> the aux_native_write to DP_PSR_EN_CFG and re-enable EDP_PSR_ENABLE
> since everything else might still be the same on the same mode-set?

Agree, I'll try to move vsc header, aux sets, etc to setup function
that is executed only once
or something like this..
Thanks for the suggestion.

>
>>  {
>> -       struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> -
>> -       if (!intel_edp_psr_match_conditions(intel_dp) ||
>> -           intel_edp_is_psr_enabled(dev))
>> -               return;
>> -
>>         /* Enable PSR on the panel */
>>         intel_edp_psr_enable_sink(intel_dp);
>>
>> @@ -1551,6 +1545,15 @@ void intel_edp_psr_enable(struct intel_dp *intel_dp)
>>         intel_edp_psr_enable_source(intel_dp);
>>  }
>>
>> +void intel_edp_psr_enable(struct intel_dp *intel_dp)
>> +{
>> +       struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> +
>> +       if (intel_edp_psr_match_conditions(intel_dp) &&
>> +           !intel_edp_is_psr_enabled(dev))
>> +               intel_edp_psr_do_enable(intel_dp);
>> +}
>> +
>>  void intel_edp_psr_disable(struct intel_dp *intel_dp)
>>  {
>>         struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> @@ -1573,6 +1576,26 @@ void intel_edp_psr_disable(struct intel_dp *intel_dp)
>>         intel_wait_for_vblank(dev, intel_crtc->pipe);
>>  }
>>
>> +void intel_edp_psr_update(struct drm_device *dev)
>> +{
>> +       struct intel_encoder *encoder;
>> +       struct intel_dp *intel_dp = NULL;
>> +
>> +       list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head)
>> +               if (encoder->type == INTEL_OUTPUT_EDP) {
>> +                       intel_dp = enc_to_intel_dp(&encoder->base);
>> +
>> +                       if (!is_edp_psr(intel_dp))
>> +                               return;
>> +
>> +                       if (!intel_edp_psr_match_conditions(intel_dp))
>> +                               intel_edp_psr_disable(intel_dp);
>> +                       else
>> +                               if (!intel_edp_is_psr_enabled(dev))
>> +                                       intel_edp_psr_do_enable(intel_dp);
>> +               }
>> +}
>> +
>>  static void intel_disable_dp(struct intel_encoder *encoder)
>>  {
>>         struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index ff09c4c..1f638cf 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -842,5 +842,6 @@ extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
>>
>>  extern void intel_edp_psr_enable(struct intel_dp *intel_dp);
>>  extern void intel_edp_psr_disable(struct intel_dp *intel_dp);
>> +extern void intel_edp_psr_update(struct drm_device *dev);
>>
>>  #endif /* __INTEL_DRV_H__ */
>> --
>> 1.8.1.4
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni



--
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 9730d6b..86c1a7d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1536,14 +1536,8 @@  static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp)
 	return true;
 }
 
-void intel_edp_psr_enable(struct intel_dp *intel_dp)
+void intel_edp_psr_do_enable(struct intel_dp *intel_dp)
 {
-	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-
-	if (!intel_edp_psr_match_conditions(intel_dp) ||
-	    intel_edp_is_psr_enabled(dev))
-		return;
-
 	/* Enable PSR on the panel */
 	intel_edp_psr_enable_sink(intel_dp);
 
@@ -1551,6 +1545,15 @@  void intel_edp_psr_enable(struct intel_dp *intel_dp)
 	intel_edp_psr_enable_source(intel_dp);
 }
 
+void intel_edp_psr_enable(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+
+	if (intel_edp_psr_match_conditions(intel_dp) &&
+	    !intel_edp_is_psr_enabled(dev))
+		intel_edp_psr_do_enable(intel_dp);
+}
+
 void intel_edp_psr_disable(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -1573,6 +1576,26 @@  void intel_edp_psr_disable(struct intel_dp *intel_dp)
 	intel_wait_for_vblank(dev, intel_crtc->pipe);
 }
 
+void intel_edp_psr_update(struct drm_device *dev)
+{
+	struct intel_encoder *encoder;
+	struct intel_dp *intel_dp = NULL;
+
+	list_for_each_entry(encoder, &dev->mode_config.encoder_list, base.head)
+		if (encoder->type == INTEL_OUTPUT_EDP) {
+			intel_dp = enc_to_intel_dp(&encoder->base);
+
+			if (!is_edp_psr(intel_dp))
+				return;
+
+			if (!intel_edp_psr_match_conditions(intel_dp))
+				intel_edp_psr_disable(intel_dp);
+			else
+				if (!intel_edp_is_psr_enabled(dev))
+					intel_edp_psr_do_enable(intel_dp);
+		}
+}
+
 static void intel_disable_dp(struct intel_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ff09c4c..1f638cf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -842,5 +842,6 @@  extern bool intel_set_pch_fifo_underrun_reporting(struct drm_device *dev,
 
 extern void intel_edp_psr_enable(struct intel_dp *intel_dp);
 extern void intel_edp_psr_disable(struct intel_dp *intel_dp);
+extern void intel_edp_psr_update(struct drm_device *dev);
 
 #endif /* __INTEL_DRV_H__ */