* [PATCH] drm/i915: restrict PP save/restore to platforms with lvds
@ 2016-02-08 9:55 Daniel Vetter
2016-02-09 2:46 ` Thulasimani, Sivakumar
2016-03-10 10:20 ` ✗ Fi.CI.BAT: warning for " Patchwork
0 siblings, 2 replies; 5+ messages in thread
From: Daniel Vetter @ 2016-02-08 9:55 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Jani Nikula, Daniel Vetter, Daniel Vetter
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@intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
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);
@@ -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);
--
2.7.0.rc3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] drm/i915: restrict PP save/restore to platforms with lvds
2016-02-08 9:55 [PATCH] drm/i915: restrict PP save/restore to platforms with lvds Daniel Vetter
@ 2016-02-09 2:46 ` Thulasimani, Sivakumar
2016-02-09 6:24 ` Jani Nikula
2016-03-10 10:20 ` ✗ Fi.CI.BAT: warning for " Patchwork
1 sibling, 1 reply; 5+ messages in thread
From: Thulasimani, Sivakumar @ 2016-02-09 2:46 UTC (permalink / raw)
To: Daniel Vetter, Intel Graphics Development; +Cc: Jani Nikula, Daniel Vetter
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@intel.com>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> 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.
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/i915: restrict PP save/restore to platforms with lvds
2016-02-09 2:46 ` Thulasimani, Sivakumar
@ 2016-02-09 6:24 ` Jani Nikula
2016-02-09 7:35 ` Thulasimani, Sivakumar
0 siblings, 1 reply; 5+ messages in thread
From: Jani Nikula @ 2016-02-09 6:24 UTC (permalink / raw)
To: Thulasimani, Sivakumar, Daniel Vetter, Intel Graphics Development
Cc: Daniel Vetter
On Tue, 09 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> 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@intel.com>
>> Acked-by: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>> 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.
>
> 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);
>
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] drm/i915: restrict PP save/restore to platforms with lvds
2016-02-09 6:24 ` Jani Nikula
@ 2016-02-09 7:35 ` Thulasimani, Sivakumar
0 siblings, 0 replies; 5+ messages in thread
From: Thulasimani, Sivakumar @ 2016-02-09 7:35 UTC (permalink / raw)
To: Jani Nikula, Daniel Vetter, Intel Graphics Development; +Cc: Daniel Vetter
On 2/9/2016 11:54 AM, Jani Nikula wrote:
> On Tue, 09 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> 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@intel.com>
>>> Acked-by: Jani Nikula <jani.nikula@intel.com>
>>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>>> ---
>>> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
* ✗ Fi.CI.BAT: warning for drm/i915: restrict PP save/restore to platforms with lvds
2016-02-08 9:55 [PATCH] drm/i915: restrict PP save/restore to platforms with lvds Daniel Vetter
2016-02-09 2:46 ` Thulasimani, Sivakumar
@ 2016-03-10 10:20 ` Patchwork
1 sibling, 0 replies; 5+ messages in thread
From: Patchwork @ 2016-03-10 10:20 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: restrict PP save/restore to platforms with lvds
URL : https://patchwork.freedesktop.org/series/3176/
State : warning
== Summary ==
Series 3176v1 drm/i915: restrict PP save/restore to platforms with lvds
http://patchwork.freedesktop.org/api/1.0/series/3176/revisions/1/mbox/
Test kms_flip:
Subgroup basic-flip-vs-modeset:
pass -> DMESG-WARN (bdw-ultra)
Test kms_pipe_crc_basic:
Subgroup read-crc-pipe-a-frame-sequence:
pass -> DMESG-WARN (bdw-ultra)
Subgroup read-crc-pipe-b:
pass -> DMESG-WARN (bdw-ultra)
Subgroup read-crc-pipe-c:
pass -> DMESG-WARN (bdw-ultra)
Subgroup suspend-read-crc-pipe-a:
pass -> DMESG-WARN (bdw-ultra)
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (bdw-ultra)
Subgroup suspend-read-crc-pipe-c:
pass -> DMESG-WARN (bdw-ultra)
bdw-nuci7 total:161 pass:152 dwarn:0 dfail:0 fail:0 skip:9
bdw-ultra total:164 pass:145 dwarn:7 dfail:0 fail:0 skip:12
byt-nuc total:164 pass:141 dwarn:0 dfail:0 fail:0 skip:23
hsw-brixbox total:164 pass:151 dwarn:0 dfail:0 fail:0 skip:13
ilk-hp8440p total:164 pass:116 dwarn:0 dfail:0 fail:0 skip:48
ivb-t430s total:164 pass:150 dwarn:0 dfail:0 fail:0 skip:14
skl-i5k-2 total:164 pass:149 dwarn:1 dfail:0 fail:0 skip:14
snb-dellxps total:164 pass:142 dwarn:0 dfail:0 fail:0 skip:22
Results at /archive/results/CI_IGT_test/Patchwork_1378/
9ae71c139227311e70dcc91d16d5acba34ce9a71 drm-intel-nightly: 2016y-02m-08d-09h-39m-29s UTC integration manifest
040fe541cbccc93cbd4333bd30e5950d50fbc19c drm/i915: restrict PP save/restore to platforms with lvds
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-03-10 10:20 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-08 9:55 [PATCH] drm/i915: restrict PP save/restore to platforms with lvds Daniel Vetter
2016-02-09 2:46 ` Thulasimani, Sivakumar
2016-02-09 6:24 ` Jani Nikula
2016-02-09 7:35 ` Thulasimani, Sivakumar
2016-03-10 10:20 ` ✗ Fi.CI.BAT: warning for " Patchwork
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.