[PATCH] drm/i915: Redoing the PSR setup on resume

[PATCH] drm/i915: Redoing the PSR setup on resume

[Ramalingam C]

Due to switch between console and graphics modes multiple psr_enable
call will be made. On such occasions, to avoid repeated psr_setup,
a flag called psr_setup_done is used.

On suspend-resume, panel goes for a power cycle. Hence PSR setup
should be redone to enable the PSR after suspend-resume.
So this patch resets the corresponding flag, if it is set during the
panel powerup process.

Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 74749c6..686f8f6 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1201,6 +1201,11 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
 	if (type == INTEL_OUTPUT_EDP) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 		ironlake_edp_panel_on(intel_dp);
+
+		/* Resetting the flag, to redo the PSR setup after
+		 * panel powercycle */
+		if (intel_dp->psr_setup_done)
+			intel_dp->psr_setup_done = false;
 	}
 
 	WARN_ON(intel_crtc->ddi_pll_sel == PORT_CLK_SEL_NONE);
-- 
1.7.9.5

Re: [PATCH] drm/i915: Redoing the PSR setup on resume

[Daniel Vetter]

On Fri, Jan 24, 2014 at 12:50:17AM +0530, Ramalingam C wrote:
> Due to switch between console and graphics modes multiple psr_enable
> call will be made. On such occasions, to avoid repeated psr_setup,
> a flag called psr_setup_done is used.
> 
> On suspend-resume, panel goes for a power cycle. Hence PSR setup
> should be redone to enable the PSR after suspend-resume.
> So this patch resets the corresponding flag, if it is set during the
> panel powerup process.
> 
> Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 74749c6..686f8f6 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1201,6 +1201,11 @@ static void intel_ddi_pre_enable(struct intel_encoder *intel_encoder)
>  	if (type == INTEL_OUTPUT_EDP) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
>  		ironlake_edp_panel_on(intel_dp);
> +
> +		/* Resetting the flag, to redo the PSR setup after
> +		 * panel powercycle */
> +		if (intel_dp->psr_setup_done)
> +			intel_dp->psr_setup_done = false;

Imo it's better to add this as an encoder->reset callback. That one is
only called on resume (at least in -nightly), so fits exactly. Resetting
this flag in every pre_enable is a bit much imo.

Also it sounds like we need to extend Rodrigo's psr testcase with a
suspend/resume subtest.
-Daniel

>  	}
>  
>  	WARN_ON(intel_crtc->ddi_pll_sel == PORT_CLK_SEL_NONE);
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Redoing the PSR setup on resume

[C, Ramalingam]

Hi Daniel,

Thanks for your valuable review. I found that Rodrigo has submitted a solution for the same issue just before mine. I am seeing that solution is aligning to the expectations.
http://lists.freedesktop.org/archives/intel-gfx/2014-January/038749.html

So may be we can ignore this submission.

Thanks and Regards,
Ramalingam C.

> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Sunday, January 26, 2014 1:41 AM
> To: C, Ramalingam
> Cc: Intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Redoing the PSR setup on resume
> 
> On Fri, Jan 24, 2014 at 12:50:17AM +0530, Ramalingam C wrote:
> > Due to switch between console and graphics modes multiple psr_enable
> > call will be made. On such occasions, to avoid repeated psr_setup, a
> > flag called psr_setup_done is used.
> >
> > On suspend-resume, panel goes for a power cycle. Hence PSR setup
> > should be redone to enable the PSR after suspend-resume.
> > So this patch resets the corresponding flag, if it is set during the
> > panel powerup process.
> >
> > Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c |    5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index 74749c6..686f8f6 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1201,6 +1201,11 @@ static void intel_ddi_pre_enable(struct
> intel_encoder *intel_encoder)
> >  	if (type == INTEL_OUTPUT_EDP) {
> >  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >  		ironlake_edp_panel_on(intel_dp);
> > +
> > +		/* Resetting the flag, to redo the PSR setup after
> > +		 * panel powercycle */
> > +		if (intel_dp->psr_setup_done)
> > +			intel_dp->psr_setup_done = false;
> 
> Imo it's better to add this as an encoder->reset callback. That one is only called
> on resume (at least in -nightly), so fits exactly. Resetting this flag in every
> pre_enable is a bit much imo.
> 
> Also it sounds like we need to extend Rodrigo's psr testcase with a
> suspend/resume subtest.
> -Daniel
> 
> >  	}
> >
> >  	WARN_ON(intel_crtc->ddi_pll_sel == PORT_CLK_SEL_NONE);
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] drm/i915: Redoing the PSR setup on resume

[Daniel Vetter]

On Tue, Jan 28, 2014 at 03:57:13PM +0000, C, Ramalingam wrote:
> Hi Daniel,
> 
> Thanks for your valuable review. I found that Rodrigo has submitted a solution for the same issue just before mine. I am seeing that solution is aligning to the expectations.
> http://lists.freedesktop.org/archives/intel-gfx/2014-January/038749.html
> 
> So may be we can ignore this submission.

Since you're working on psr, too: Can you please review Rodrigo's patch
series and maybe also supply a tested by if it really fixes your issue?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch