| Message ID | 1390504821-15243-1-git-send-email-ramalingam.c@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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);
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(+)