Re: [PATCH] drm/i915: restrict PP save/restore to platforms with lvds

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/9/2016 11:54 AM, Jani Nikula wrote:
On Tue, 09 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@xxxxxxxxx> wrote:
On 2/8/2016 3:25 PM, Daniel Vetter wrote:
eDP already restores PP state completely on it's own, we only need
this code for LVDS. Since it's more work to move this into the lvds
encoder properly just limit it to affected pch chips for now
(ibx&cpt/ppt).

Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
Acked-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
---
   drivers/gpu/drm/i915/i915_suspend.c | 8 ++++----
   1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_suspend.c b/drivers/gpu/drm/i915/i915_suspend.c
index a2aa09ce3202..7f6b050266a7 100644
--- a/drivers/gpu/drm/i915/i915_suspend.c
+++ b/drivers/gpu/drm/i915/i915_suspend.c
@@ -43,8 +43,8 @@ static void i915_save_display(struct drm_device *dev)
   	else if (INTEL_INFO(dev)->gen <= 4 && IS_MOBILE(dev) && !IS_I830(dev))
   		dev_priv->regfile.saveLVDS = I915_READ(LVDS);
- /* Panel power sequencer */
-	if (HAS_PCH_SPLIT(dev)) {
+	/* Panel power sequencer, only needed for LVDS */
+	if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
   		dev_priv->regfile.savePP_CONTROL = I915_READ(PCH_PP_CONTROL);
   		dev_priv->regfile.savePP_ON_DELAYS = I915_READ(PCH_PP_ON_DELAYS);
   		dev_priv->regfile.savePP_OFF_DELAYS = I915_READ(PCH_PP_OFF_DELAYS);
won't this code execute for eDP too ? Also it seems incorrect we are
saving and restoring PP_CONTROL register too when we should have
followed proper sequence delays for each bit in PP_CONTROL.  This will
end up bring up the panel before modeset in best case or damage the
panel in the worst case.
Exactly the reasons why we are trying to limit this code to fewer
platforms! This is very fragile stuff though, I fear we might regress
something (even if that something is currently working by coincidence)
if we limit this further, in one go. I'd like to do this little by
little, and see what breaks, if anything.

With this patch, we'd already limit all of this hackery to cougarpoint
and older, instead of all PCH split up to and including Skylake. If
anyone wants to dig into LVDS power sequencing after that, I'm not
opposed...

BR,
Jani.
missed the part where this patch is reducing the platforms :).
can we reduce this to systems with LVDS alone ? i am sure eDP will blankout if
pps is not followed but have observed some LVDS panels working even without
pps delays.

Sivakumar

Sivakumar

@@ -78,8 +78,8 @@ static void i915_restore_display(struct drm_device *dev)
   	else if (INTEL_INFO(dev)->gen <= 4 && IS_MOBILE(dev) && !IS_I830(dev))
   		I915_WRITE(LVDS, dev_priv->regfile.saveLVDS & mask);
- /* Panel power sequencer */
-	if (HAS_PCH_SPLIT(dev)) {
+	/* Panel power sequencer, only needed for LVDS */
+	if (HAS_PCH_IBX(dev) || HAS_PCH_CPT(dev)) {
   		I915_WRITE(PCH_PP_ON_DELAYS, dev_priv->regfile.savePP_ON_DELAYS);
   		I915_WRITE(PCH_PP_OFF_DELAYS, dev_priv->regfile.savePP_OFF_DELAYS);
   		I915_WRITE(PCH_PP_DIVISOR, dev_priv->regfile.savePP_DIVISOR);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux