* [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it @ 2016-12-20 16:51 ` ville.syrjala 0 siblings, 0 replies; 11+ messages in thread From: ville.syrjala @ 2016-12-20 16:51 UTC (permalink / raw) To: intel-gfx; +Cc: stable, Matwey V . Kornilov From: Ville Syrjälä <ville.syrjala@linux.intel.com> Apparently some VLV BIOSen like to leave the VDD force bit enabled even for power seqeuncers that aren't properly hooked up to any port. That will result in a imbalance in the AUX power domain refcount when we stat to use said power sequencer as edp_panel_vdd_on() will not grab the power domain reference if it sees that the VDD is already on. To fix this let's make sure we turn off the VDD force bit when we initialize the power sequencer registers. That is, unless it's being done from the init path since there we are actually initializing the registers for the current power sequencer and we don't want to turn VDD off needlessly as that would require waiting for the power cycle delay before we turn it back on. This fixes the following kind of warnings: WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455 intel_display_power_put+0x13a/0x170 [i915]() WARN_ON(!power_domains->domain_use_count[domain]) ... Cc: stable@vger.kernel.org Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 42 ++++++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 66b5bc80b781..4139122704b3 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, struct intel_dp *intel_dp); static void intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, - struct intel_dp *intel_dp); + struct intel_dp *intel_dp, + bool force_disable_vdd); static void intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp); @@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) /* init power sequencer on this pipe and port */ intel_dp_init_panel_power_sequencer(dev, intel_dp); - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); /* * Even vdd force doesn't work until we've made @@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) * Only the HW needs to be reprogrammed, the SW state is fixed and * has been setup during connector init. */ - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); return 0; } @@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) port_name(port), pipe_name(intel_dp->pps_pipe)); intel_dp_init_panel_power_sequencer(dev, intel_dp); - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); } void intel_power_sequencer_reset(struct drm_i915_private *dev_priv) @@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp) /* init power sequencer on this pipe and port */ intel_dp_init_panel_power_sequencer(dev, intel_dp); - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); } static void vlv_pre_enable_dp(struct intel_encoder *encoder, @@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, static void intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, - struct intel_dp *intel_dp) + struct intel_dp *intel_dp, + bool force_disable_vdd) { struct drm_i915_private *dev_priv = to_i915(dev); u32 pp_on, pp_off, pp_div, port_sel = 0; @@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, intel_pps_get_registers(dev_priv, intel_dp, ®s); + /* + * One some VLV machines the BIOS can leave the VDD + * enabled even on power seqeuencers which aren't + * even hooked up to any port. This would mess up + * the power domain tracking the first time we pick + * one of these power sequencers for use since + * edp_panel_vdd_on() would notice that the VDD was + * already on and therefore wouldn't even grab the + * power domain reference. Disable VDD first to avoid + * this. This also avoids spuriously turning the VDD + * on as soon as the new power seqeuencer gets + * initialized. + */ + if (force_disable_vdd) { + u32 pp = ironlake_get_pp_control(intel_dp); + + WARN(pp & PANEL_POWER_ON, "Panel power already on\n"); + + if (pp & EDP_FORCE_VDD) + DRM_DEBUG_KMS("VDD already on, disabling first\n"); + + pp &= ~EDP_FORCE_VDD; + + I915_WRITE(regs.pp_ctrl, pp); + } + pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT); pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | @@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev, vlv_initial_power_sequencer_setup(intel_dp); } else { intel_dp_init_panel_power_sequencer(dev, intel_dp); - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); } } -- 2.10.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it @ 2016-12-20 16:51 ` ville.syrjala 0 siblings, 0 replies; 11+ messages in thread From: ville.syrjala @ 2016-12-20 16:51 UTC (permalink / raw) To: intel-gfx; +Cc: Matwey V . Kornilov, stable From: Ville Syrjälä <ville.syrjala@linux.intel.com> Apparently some VLV BIOSen like to leave the VDD force bit enabled even for power seqeuncers that aren't properly hooked up to any port. That will result in a imbalance in the AUX power domain refcount when we stat to use said power sequencer as edp_panel_vdd_on() will not grab the power domain reference if it sees that the VDD is already on. To fix this let's make sure we turn off the VDD force bit when we initialize the power sequencer registers. That is, unless it's being done from the init path since there we are actually initializing the registers for the current power sequencer and we don't want to turn VDD off needlessly as that would require waiting for the power cycle delay before we turn it back on. This fixes the following kind of warnings: WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455 intel_display_power_put+0x13a/0x170 [i915]() WARN_ON(!power_domains->domain_use_count[domain]) ... Cc: stable@vger.kernel.org Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695 Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_dp.c | 42 ++++++++++++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 66b5bc80b781..4139122704b3 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, struct intel_dp *intel_dp); static void intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, - struct intel_dp *intel_dp); + struct intel_dp *intel_dp, + bool force_disable_vdd); static void intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp); @@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) /* init power sequencer on this pipe and port */ intel_dp_init_panel_power_sequencer(dev, intel_dp); - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); /* * Even vdd force doesn't work until we've made @@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) * Only the HW needs to be reprogrammed, the SW state is fixed and * has been setup during connector init. */ - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); return 0; } @@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) port_name(port), pipe_name(intel_dp->pps_pipe)); intel_dp_init_panel_power_sequencer(dev, intel_dp); - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); } void intel_power_sequencer_reset(struct drm_i915_private *dev_priv) @@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp) /* init power sequencer on this pipe and port */ intel_dp_init_panel_power_sequencer(dev, intel_dp); - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); } static void vlv_pre_enable_dp(struct intel_encoder *encoder, @@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, static void intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, - struct intel_dp *intel_dp) + struct intel_dp *intel_dp, + bool force_disable_vdd) { struct drm_i915_private *dev_priv = to_i915(dev); u32 pp_on, pp_off, pp_div, port_sel = 0; @@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, intel_pps_get_registers(dev_priv, intel_dp, ®s); + /* + * One some VLV machines the BIOS can leave the VDD + * enabled even on power seqeuencers which aren't + * even hooked up to any port. This would mess up + * the power domain tracking the first time we pick + * one of these power sequencers for use since + * edp_panel_vdd_on() would notice that the VDD was + * already on and therefore wouldn't even grab the + * power domain reference. Disable VDD first to avoid + * this. This also avoids spuriously turning the VDD + * on as soon as the new power seqeuencer gets + * initialized. + */ + if (force_disable_vdd) { + u32 pp = ironlake_get_pp_control(intel_dp); + + WARN(pp & PANEL_POWER_ON, "Panel power already on\n"); + + if (pp & EDP_FORCE_VDD) + DRM_DEBUG_KMS("VDD already on, disabling first\n"); + + pp &= ~EDP_FORCE_VDD; + + I915_WRITE(regs.pp_ctrl, pp); + } + pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT); pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | @@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev, vlv_initial_power_sequencer_setup(intel_dp); } else { intel_dp_init_panel_power_sequencer(dev, intel_dp); - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); } } -- 2.10.2 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 11+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Force VDD off on the new power seqeuencer before starting to use it 2016-12-20 16:51 ` ville.syrjala (?) @ 2016-12-20 18:45 ` Patchwork -1 siblings, 0 replies; 11+ messages in thread From: Patchwork @ 2016-12-20 18:45 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx == Series Details == Series: drm/i915: Force VDD off on the new power seqeuencer before starting to use it URL : https://patchwork.freedesktop.org/series/17068/ State : success == Summary == Series 17068v1 drm/i915: Force VDD off on the new power seqeuencer before starting to use it https://patchwork.freedesktop.org/api/1.0/series/17068/revisions/1/mbox/ fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:247 pass:225 dwarn:1 dfail:0 fail:0 skip:21 fi-bxt-t5700 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-ilk-650 total:247 pass:195 dwarn:0 dfail:0 fail:0 skip:52 fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hq total:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:247 pass:224 dwarn:3 dfail:0 fail:0 skip:20 fi-skl-6770hq total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32 6096aee14ea52e3163729129ee7362e56ff3efb9 drm-tip: 2016y-12m-20d-16h-33m-17s UTC integration manifest 42e3f84 drm/i915: Force VDD off on the new power seqeuencer before starting to use it == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3347/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it 2016-12-20 16:51 ` ville.syrjala @ 2016-12-22 9:39 ` David Weinehall -1 siblings, 0 replies; 11+ messages in thread From: David Weinehall @ 2016-12-22 9:39 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx, Matwey V . Kornilov, stable On Tue, Dec 20, 2016 at 06:51:17PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > Apparently some VLV BIOSen like to leave the VDD force bit enabled > even for power seqeuncers that aren't properly hooked up to any > port. That will result in a imbalance in the AUX power domain > refcount when we stat to use said power sequencer as edp_panel_vdd_on() > will not grab the power domain reference if it sees that the VDD is > already on. > > To fix this let's make sure we turn off the VDD force bit when we > initialize the power sequencer registers. That is, unless it's > being done from the init path since there we are actually > initializing the registers for the current power sequencer and > we don't want to turn VDD off needlessly as that would require > waiting for the power cycle delay before we turn it back on. > > This fixes the following kind of warnings: > WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455 intel_display_power_put+0x13a/0x170 [i915]() > WARN_ON(!power_domains->domain_use_count[domain]) > ... > > Cc: stable@vger.kernel.org > Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> > Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695 > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 42 ++++++++++++++++++++++++++++++++++------- > 1 file changed, 35 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 66b5bc80b781..4139122704b3 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > struct intel_dp *intel_dp); > static void > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > - struct intel_dp *intel_dp); > + struct intel_dp *intel_dp, > + bool force_disable_vdd); > static void > intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp); > > @@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) > > /* init power sequencer on this pipe and port */ > intel_dp_init_panel_power_sequencer(dev, intel_dp); > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > /* > * Even vdd force doesn't work until we've made > @@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > * Only the HW needs to be reprogrammed, the SW state is fixed and > * has been setup during connector init. > */ > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > return 0; > } > @@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) > port_name(port), pipe_name(intel_dp->pps_pipe)); > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > } > > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv) > @@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp) > > /* init power sequencer on this pipe and port */ > intel_dp_init_panel_power_sequencer(dev, intel_dp); > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > } > > static void vlv_pre_enable_dp(struct intel_encoder *encoder, > @@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > static void > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > - struct intel_dp *intel_dp) > + struct intel_dp *intel_dp, > + bool force_disable_vdd) > { > struct drm_i915_private *dev_priv = to_i915(dev); > u32 pp_on, pp_off, pp_div, port_sel = 0; > @@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > intel_pps_get_registers(dev_priv, intel_dp, ®s); > > + /* > + * One some VLV machines the BIOS can leave the VDD On > + * enabled even on power seqeuencers which aren't > + * even hooked up to any port. This would mess up Remove even here > + * the power domain tracking the first time we pick > + * one of these power sequencers for use since > + * edp_panel_vdd_on() would notice that the VDD was > + * already on and therefore wouldn't even grab the And here. > + * power domain reference. Disable VDD first to avoid > + * this. This also avoids spuriously turning the VDD > + * on as soon as the new power seqeuencer gets > + * initialized. > + */ > + if (force_disable_vdd) { > + u32 pp = ironlake_get_pp_control(intel_dp); > + > + WARN(pp & PANEL_POWER_ON, "Panel power already on\n"); Wouldn't this just replace one warning with another? Or is this a subset of the other warning? > + > + if (pp & EDP_FORCE_VDD) > + DRM_DEBUG_KMS("VDD already on, disabling first\n"); > + > + pp &= ~EDP_FORCE_VDD; > + > + I915_WRITE(regs.pp_ctrl, pp); > + } > + > pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | > (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT); > pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | > @@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev, > vlv_initial_power_sequencer_setup(intel_dp); > } else { > intel_dp_init_panel_power_sequencer(dev, intel_dp); > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > } > } > > -- > 2.10.2 Kind regards, David ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it @ 2016-12-22 9:39 ` David Weinehall 0 siblings, 0 replies; 11+ messages in thread From: David Weinehall @ 2016-12-22 9:39 UTC (permalink / raw) To: ville.syrjala; +Cc: intel-gfx, Matwey V . Kornilov, stable On Tue, Dec 20, 2016 at 06:51:17PM +0200, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Apparently some VLV BIOSen like to leave the VDD force bit enabled > even for power seqeuncers that aren't properly hooked up to any > port. That will result in a imbalance in the AUX power domain > refcount when we stat to use said power sequencer as edp_panel_vdd_on() > will not grab the power domain reference if it sees that the VDD is > already on. > > To fix this let's make sure we turn off the VDD force bit when we > initialize the power sequencer registers. That is, unless it's > being done from the init path since there we are actually > initializing the registers for the current power sequencer and > we don't want to turn VDD off needlessly as that would require > waiting for the power cycle delay before we turn it back on. > > This fixes the following kind of warnings: > WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455 intel_display_power_put+0x13a/0x170 [i915]() > WARN_ON(!power_domains->domain_use_count[domain]) > ... > > Cc: stable@vger.kernel.org > Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> > Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695 > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_dp.c | 42 ++++++++++++++++++++++++++++++++++------- > 1 file changed, 35 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 66b5bc80b781..4139122704b3 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > struct intel_dp *intel_dp); > static void > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > - struct intel_dp *intel_dp); > + struct intel_dp *intel_dp, > + bool force_disable_vdd); > static void > intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp); > > @@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) > > /* init power sequencer on this pipe and port */ > intel_dp_init_panel_power_sequencer(dev, intel_dp); > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > /* > * Even vdd force doesn't work until we've made > @@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > * Only the HW needs to be reprogrammed, the SW state is fixed and > * has been setup during connector init. > */ > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > return 0; > } > @@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) > port_name(port), pipe_name(intel_dp->pps_pipe)); > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > } > > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv) > @@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp) > > /* init power sequencer on this pipe and port */ > intel_dp_init_panel_power_sequencer(dev, intel_dp); > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > } > > static void vlv_pre_enable_dp(struct intel_encoder *encoder, > @@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > static void > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > - struct intel_dp *intel_dp) > + struct intel_dp *intel_dp, > + bool force_disable_vdd) > { > struct drm_i915_private *dev_priv = to_i915(dev); > u32 pp_on, pp_off, pp_div, port_sel = 0; > @@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > intel_pps_get_registers(dev_priv, intel_dp, ®s); > > + /* > + * One some VLV machines the BIOS can leave the VDD On > + * enabled even on power seqeuencers which aren't > + * even hooked up to any port. This would mess up Remove even here > + * the power domain tracking the first time we pick > + * one of these power sequencers for use since > + * edp_panel_vdd_on() would notice that the VDD was > + * already on and therefore wouldn't even grab the And here. > + * power domain reference. Disable VDD first to avoid > + * this. This also avoids spuriously turning the VDD > + * on as soon as the new power seqeuencer gets > + * initialized. > + */ > + if (force_disable_vdd) { > + u32 pp = ironlake_get_pp_control(intel_dp); > + > + WARN(pp & PANEL_POWER_ON, "Panel power already on\n"); Wouldn't this just replace one warning with another? Or is this a subset of the other warning? > + > + if (pp & EDP_FORCE_VDD) > + DRM_DEBUG_KMS("VDD already on, disabling first\n"); > + > + pp &= ~EDP_FORCE_VDD; > + > + I915_WRITE(regs.pp_ctrl, pp); > + } > + > pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | > (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT); > pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | > @@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev, > vlv_initial_power_sequencer_setup(intel_dp); > } else { > intel_dp_init_panel_power_sequencer(dev, intel_dp); > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > } > } > > -- > 2.10.2 Kind regards, David ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it 2016-12-22 9:39 ` David Weinehall @ 2016-12-22 11:49 ` Ville Syrjälä -1 siblings, 0 replies; 11+ messages in thread From: Ville Syrjälä @ 2016-12-22 11:49 UTC (permalink / raw) To: intel-gfx, Matwey V . Kornilov, stable On Thu, Dec 22, 2016 at 11:39:37AM +0200, David Weinehall wrote: > On Tue, Dec 20, 2016 at 06:51:17PM +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > > > Apparently some VLV BIOSen like to leave the VDD force bit enabled > > even for power seqeuncers that aren't properly hooked up to any > > port. That will result in a imbalance in the AUX power domain > > refcount when we stat to use said power sequencer as edp_panel_vdd_on() > > will not grab the power domain reference if it sees that the VDD is > > already on. > > > > To fix this let's make sure we turn off the VDD force bit when we > > initialize the power sequencer registers. That is, unless it's > > being done from the init path since there we are actually > > initializing the registers for the current power sequencer and > > we don't want to turn VDD off needlessly as that would require > > waiting for the power cycle delay before we turn it back on. > > > > This fixes the following kind of warnings: > > WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455 intel_display_power_put+0x13a/0x170 [i915]() > > WARN_ON(!power_domains->domain_use_count[domain]) > > ... > > > > Cc: stable@vger.kernel.org > > Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695 > > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 42 ++++++++++++++++++++++++++++++++++------- > > 1 file changed, 35 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 66b5bc80b781..4139122704b3 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > struct intel_dp *intel_dp); > > static void > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > - struct intel_dp *intel_dp); > > + struct intel_dp *intel_dp, > > + bool force_disable_vdd); > > static void > > intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp); > > > > @@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) > > > > /* init power sequencer on this pipe and port */ > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > > > /* > > * Even vdd force doesn't work until we've made > > @@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > > * Only the HW needs to be reprogrammed, the SW state is fixed and > > * has been setup during connector init. > > */ > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > > return 0; > > } > > @@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) > > port_name(port), pipe_name(intel_dp->pps_pipe)); > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > } > > > > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv) > > @@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp) > > > > /* init power sequencer on this pipe and port */ > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > } > > > > static void vlv_pre_enable_dp(struct intel_encoder *encoder, > > @@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > > > static void > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > - struct intel_dp *intel_dp) > > + struct intel_dp *intel_dp, > > + bool force_disable_vdd) > > { > > struct drm_i915_private *dev_priv = to_i915(dev); > > u32 pp_on, pp_off, pp_div, port_sel = 0; > > @@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > > intel_pps_get_registers(dev_priv, intel_dp, ®s); > > > > + /* > > + * One some VLV machines the BIOS can leave the VDD > > On > > > + * enabled even on power seqeuencers which aren't > > + * even hooked up to any port. This would mess up > > Remove even here > > > + * the power domain tracking the first time we pick > > + * one of these power sequencers for use since > > + * edp_panel_vdd_on() would notice that the VDD was > > + * already on and therefore wouldn't even grab the > > And here. > > > + * power domain reference. Disable VDD first to avoid > > + * this. This also avoids spuriously turning the VDD > > + * on as soon as the new power seqeuencer gets > > + * initialized. > > + */ > > + if (force_disable_vdd) { > > + u32 pp = ironlake_get_pp_control(intel_dp); > > + > > + WARN(pp & PANEL_POWER_ON, "Panel power already on\n"); > > Wouldn't this just replace one warning with another? Or is this > a subset of the other warning? This should never happen. But I'd rather we get a bug report if it ever does occur. > > > + > > + if (pp & EDP_FORCE_VDD) > > + DRM_DEBUG_KMS("VDD already on, disabling first\n"); > > + > > + pp &= ~EDP_FORCE_VDD; > > + > > + I915_WRITE(regs.pp_ctrl, pp); > > + } > > + > > pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | > > (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT); > > pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | > > @@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev, > > vlv_initial_power_sequencer_setup(intel_dp); > > } else { > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > } > > } > > > > -- > > 2.10.2 > > Kind regards, David -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it @ 2016-12-22 11:49 ` Ville Syrjälä 0 siblings, 0 replies; 11+ messages in thread From: Ville Syrjälä @ 2016-12-22 11:49 UTC (permalink / raw) To: intel-gfx, Matwey V . Kornilov, stable On Thu, Dec 22, 2016 at 11:39:37AM +0200, David Weinehall wrote: > On Tue, Dec 20, 2016 at 06:51:17PM +0200, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Apparently some VLV BIOSen like to leave the VDD force bit enabled > > even for power seqeuncers that aren't properly hooked up to any > > port. That will result in a imbalance in the AUX power domain > > refcount when we stat to use said power sequencer as edp_panel_vdd_on() > > will not grab the power domain reference if it sees that the VDD is > > already on. > > > > To fix this let's make sure we turn off the VDD force bit when we > > initialize the power sequencer registers. That is, unless it's > > being done from the init path since there we are actually > > initializing the registers for the current power sequencer and > > we don't want to turn VDD off needlessly as that would require > > waiting for the power cycle delay before we turn it back on. > > > > This fixes the following kind of warnings: > > WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455 intel_display_power_put+0x13a/0x170 [i915]() > > WARN_ON(!power_domains->domain_use_count[domain]) > > ... > > > > Cc: stable@vger.kernel.org > > Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695 > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_dp.c | 42 ++++++++++++++++++++++++++++++++++------- > > 1 file changed, 35 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 66b5bc80b781..4139122704b3 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > struct intel_dp *intel_dp); > > static void > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > - struct intel_dp *intel_dp); > > + struct intel_dp *intel_dp, > > + bool force_disable_vdd); > > static void > > intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp); > > > > @@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) > > > > /* init power sequencer on this pipe and port */ > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > > > /* > > * Even vdd force doesn't work until we've made > > @@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > > * Only the HW needs to be reprogrammed, the SW state is fixed and > > * has been setup during connector init. > > */ > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > > return 0; > > } > > @@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) > > port_name(port), pipe_name(intel_dp->pps_pipe)); > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > } > > > > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv) > > @@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp) > > > > /* init power sequencer on this pipe and port */ > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > } > > > > static void vlv_pre_enable_dp(struct intel_encoder *encoder, > > @@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > > > static void > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > - struct intel_dp *intel_dp) > > + struct intel_dp *intel_dp, > > + bool force_disable_vdd) > > { > > struct drm_i915_private *dev_priv = to_i915(dev); > > u32 pp_on, pp_off, pp_div, port_sel = 0; > > @@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > > intel_pps_get_registers(dev_priv, intel_dp, ®s); > > > > + /* > > + * One some VLV machines the BIOS can leave the VDD > > On > > > + * enabled even on power seqeuencers which aren't > > + * even hooked up to any port. This would mess up > > Remove even here > > > + * the power domain tracking the first time we pick > > + * one of these power sequencers for use since > > + * edp_panel_vdd_on() would notice that the VDD was > > + * already on and therefore wouldn't even grab the > > And here. > > > + * power domain reference. Disable VDD first to avoid > > + * this. This also avoids spuriously turning the VDD > > + * on as soon as the new power seqeuencer gets > > + * initialized. > > + */ > > + if (force_disable_vdd) { > > + u32 pp = ironlake_get_pp_control(intel_dp); > > + > > + WARN(pp & PANEL_POWER_ON, "Panel power already on\n"); > > Wouldn't this just replace one warning with another? Or is this > a subset of the other warning? This should never happen. But I'd rather we get a bug report if it ever does occur. > > > + > > + if (pp & EDP_FORCE_VDD) > > + DRM_DEBUG_KMS("VDD already on, disabling first\n"); > > + > > + pp &= ~EDP_FORCE_VDD; > > + > > + I915_WRITE(regs.pp_ctrl, pp); > > + } > > + > > pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | > > (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT); > > pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | > > @@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev, > > vlv_initial_power_sequencer_setup(intel_dp); > > } else { > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > } > > } > > > > -- > > 2.10.2 > > Kind regards, David -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it 2016-12-22 11:49 ` Ville Syrjälä @ 2016-12-22 11:57 ` David Weinehall -1 siblings, 0 replies; 11+ messages in thread From: David Weinehall @ 2016-12-22 11:57 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx, Matwey V . Kornilov, stable On Thu, Dec 22, 2016 at 01:49:39PM +0200, Ville Syrj�l� wrote: > On Thu, Dec 22, 2016 at 11:39:37AM +0200, David Weinehall wrote: > > On Tue, Dec 20, 2016 at 06:51:17PM +0200, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > > > > > Apparently some VLV BIOSen like to leave the VDD force bit enabled > > > even for power seqeuncers that aren't properly hooked up to any > > > port. That will result in a imbalance in the AUX power domain > > > refcount when we stat to use said power sequencer as edp_panel_vdd_on() > > > will not grab the power domain reference if it sees that the VDD is > > > already on. > > > > > > To fix this let's make sure we turn off the VDD force bit when we > > > initialize the power sequencer registers. That is, unless it's > > > being done from the init path since there we are actually > > > initializing the registers for the current power sequencer and > > > we don't want to turn VDD off needlessly as that would require > > > waiting for the power cycle delay before we turn it back on. > > > > > > This fixes the following kind of warnings: > > > WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455 intel_display_power_put+0x13a/0x170 [i915]() > > > WARN_ON(!power_domains->domain_use_count[domain]) > > > ... > > > > > > Cc: stable@vger.kernel.org > > > Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > > Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695 > > > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_dp.c | 42 ++++++++++++++++++++++++++++++++++------- > > > 1 file changed, 35 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > index 66b5bc80b781..4139122704b3 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > > struct intel_dp *intel_dp); > > > static void > > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > - struct intel_dp *intel_dp); > > > + struct intel_dp *intel_dp, > > > + bool force_disable_vdd); > > > static void > > > intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp); > > > > > > @@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) > > > > > > /* init power sequencer on this pipe and port */ > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > > > > > /* > > > * Even vdd force doesn't work until we've made > > > @@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > > > * Only the HW needs to be reprogrammed, the SW state is fixed and > > > * has been setup during connector init. > > > */ > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > > > > return 0; > > > } > > > @@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) > > > port_name(port), pipe_name(intel_dp->pps_pipe)); > > > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > } > > > > > > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv) > > > @@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp) > > > > > > /* init power sequencer on this pipe and port */ > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > > } > > > > > > static void vlv_pre_enable_dp(struct intel_encoder *encoder, > > > @@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > > > > > static void > > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > - struct intel_dp *intel_dp) > > > + struct intel_dp *intel_dp, > > > + bool force_disable_vdd) > > > { > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > u32 pp_on, pp_off, pp_div, port_sel = 0; > > > @@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > > > > intel_pps_get_registers(dev_priv, intel_dp, ®s); > > > > > > + /* > > > + * One some VLV machines the BIOS can leave the VDD > > > > On > > > > > + * enabled even on power seqeuencers which aren't > > > + * even hooked up to any port. This would mess up > > > > Remove even here > > > > > + * the power domain tracking the first time we pick > > > + * one of these power sequencers for use since > > > + * edp_panel_vdd_on() would notice that the VDD was > > > + * already on and therefore wouldn't even grab the > > > > And here. > > > > > + * power domain reference. Disable VDD first to avoid > > > + * this. This also avoids spuriously turning the VDD > > > + * on as soon as the new power seqeuencer gets > > > + * initialized. > > > + */ > > > + if (force_disable_vdd) { > > > + u32 pp = ironlake_get_pp_control(intel_dp); > > > + > > > + WARN(pp & PANEL_POWER_ON, "Panel power already on\n"); > > > > Wouldn't this just replace one warning with another? Or is this > > a subset of the other warning? > > This should never happen. But I'd rather we get a bug report if it ever > does occur. Fair enough! > > > > > > + > > > + if (pp & EDP_FORCE_VDD) > > > + DRM_DEBUG_KMS("VDD already on, disabling first\n"); > > > + > > > + pp &= ~EDP_FORCE_VDD; > > > + > > > + I915_WRITE(regs.pp_ctrl, pp); > > > + } > > > + > > > pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | > > > (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT); > > > pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | > > > @@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev, > > > vlv_initial_power_sequencer_setup(intel_dp); > > > } else { > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > } > > > } > > > > > > -- > > > 2.10.2 > > > > Kind regards, David > > -- > Ville Syrj�l� > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it @ 2016-12-22 11:57 ` David Weinehall 0 siblings, 0 replies; 11+ messages in thread From: David Weinehall @ 2016-12-22 11:57 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx, Matwey V . Kornilov, stable On Thu, Dec 22, 2016 at 01:49:39PM +0200, Ville Syrjälä wrote: > On Thu, Dec 22, 2016 at 11:39:37AM +0200, David Weinehall wrote: > > On Tue, Dec 20, 2016 at 06:51:17PM +0200, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Apparently some VLV BIOSen like to leave the VDD force bit enabled > > > even for power seqeuncers that aren't properly hooked up to any > > > port. That will result in a imbalance in the AUX power domain > > > refcount when we stat to use said power sequencer as edp_panel_vdd_on() > > > will not grab the power domain reference if it sees that the VDD is > > > already on. > > > > > > To fix this let's make sure we turn off the VDD force bit when we > > > initialize the power sequencer registers. That is, unless it's > > > being done from the init path since there we are actually > > > initializing the registers for the current power sequencer and > > > we don't want to turn VDD off needlessly as that would require > > > waiting for the power cycle delay before we turn it back on. > > > > > > This fixes the following kind of warnings: > > > WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455 intel_display_power_put+0x13a/0x170 [i915]() > > > WARN_ON(!power_domains->domain_use_count[domain]) > > > ... > > > > > > Cc: stable@vger.kernel.org > > > Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > > Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695 > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_dp.c | 42 ++++++++++++++++++++++++++++++++++------- > > > 1 file changed, 35 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > index 66b5bc80b781..4139122704b3 100644 > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > @@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > > struct intel_dp *intel_dp); > > > static void > > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > - struct intel_dp *intel_dp); > > > + struct intel_dp *intel_dp, > > > + bool force_disable_vdd); > > > static void > > > intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp); > > > > > > @@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) > > > > > > /* init power sequencer on this pipe and port */ > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > > > > > /* > > > * Even vdd force doesn't work until we've made > > > @@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > > > * Only the HW needs to be reprogrammed, the SW state is fixed and > > > * has been setup during connector init. > > > */ > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > > > > return 0; > > > } > > > @@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) > > > port_name(port), pipe_name(intel_dp->pps_pipe)); > > > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > } > > > > > > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv) > > > @@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp) > > > > > > /* init power sequencer on this pipe and port */ > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > > } > > > > > > static void vlv_pre_enable_dp(struct intel_encoder *encoder, > > > @@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > > > > > static void > > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > - struct intel_dp *intel_dp) > > > + struct intel_dp *intel_dp, > > > + bool force_disable_vdd) > > > { > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > u32 pp_on, pp_off, pp_div, port_sel = 0; > > > @@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > > > > intel_pps_get_registers(dev_priv, intel_dp, ®s); > > > > > > + /* > > > + * One some VLV machines the BIOS can leave the VDD > > > > On > > > > > + * enabled even on power seqeuencers which aren't > > > + * even hooked up to any port. This would mess up > > > > Remove even here > > > > > + * the power domain tracking the first time we pick > > > + * one of these power sequencers for use since > > > + * edp_panel_vdd_on() would notice that the VDD was > > > + * already on and therefore wouldn't even grab the > > > > And here. > > > > > + * power domain reference. Disable VDD first to avoid > > > + * this. This also avoids spuriously turning the VDD > > > + * on as soon as the new power seqeuencer gets > > > + * initialized. > > > + */ > > > + if (force_disable_vdd) { > > > + u32 pp = ironlake_get_pp_control(intel_dp); > > > + > > > + WARN(pp & PANEL_POWER_ON, "Panel power already on\n"); > > > > Wouldn't this just replace one warning with another? Or is this > > a subset of the other warning? > > This should never happen. But I'd rather we get a bug report if it ever > does occur. Fair enough! > > > > > > + > > > + if (pp & EDP_FORCE_VDD) > > > + DRM_DEBUG_KMS("VDD already on, disabling first\n"); > > > + > > > + pp &= ~EDP_FORCE_VDD; > > > + > > > + I915_WRITE(regs.pp_ctrl, pp); > > > + } > > > + > > > pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | > > > (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT); > > > pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | > > > @@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev, > > > vlv_initial_power_sequencer_setup(intel_dp); > > > } else { > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > } > > > } > > > > > > -- > > > 2.10.2 > > > > Kind regards, David > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it 2016-12-22 11:57 ` David Weinehall @ 2016-12-22 13:57 ` Ville Syrjälä -1 siblings, 0 replies; 11+ messages in thread From: Ville Syrjälä @ 2016-12-22 13:57 UTC (permalink / raw) To: intel-gfx, Matwey V . Kornilov, stable On Thu, Dec 22, 2016 at 01:57:26PM +0200, David Weinehall wrote: > On Thu, Dec 22, 2016 at 01:49:39PM +0200, Ville Syrj�l� wrote: > > On Thu, Dec 22, 2016 at 11:39:37AM +0200, David Weinehall wrote: > > > On Tue, Dec 20, 2016 at 06:51:17PM +0200, ville.syrjala@linux.intel.com wrote: > > > > From: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > > > > > > > Apparently some VLV BIOSen like to leave the VDD force bit enabled > > > > even for power seqeuncers that aren't properly hooked up to any > > > > port. That will result in a imbalance in the AUX power domain > > > > refcount when we stat to use said power sequencer as edp_panel_vdd_on() > > > > will not grab the power domain reference if it sees that the VDD is > > > > already on. > > > > > > > > To fix this let's make sure we turn off the VDD force bit when we > > > > initialize the power sequencer registers. That is, unless it's > > > > being done from the init path since there we are actually > > > > initializing the registers for the current power sequencer and > > > > we don't want to turn VDD off needlessly as that would require > > > > waiting for the power cycle delay before we turn it back on. > > > > > > > > This fixes the following kind of warnings: > > > > WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455 intel_display_power_put+0x13a/0x170 [i915]() > > > > WARN_ON(!power_domains->domain_use_count[domain]) > > > > ... > > > > > > > > Cc: stable@vger.kernel.org > > > > Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > > > Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695 > > > > Signed-off-by: Ville Syrj�l� <ville.syrjala@linux.intel.com> > > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> Typos and whatnot fixed and patch pushed to dinq. Thanks for the review. > > > > > --- > > > > drivers/gpu/drm/i915/intel_dp.c | 42 ++++++++++++++++++++++++++++++++++------- > > > > 1 file changed, 35 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > > index 66b5bc80b781..4139122704b3 100644 > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > @@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > > > struct intel_dp *intel_dp); > > > > static void > > > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > > - struct intel_dp *intel_dp); > > > > + struct intel_dp *intel_dp, > > > > + bool force_disable_vdd); > > > > static void > > > > intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp); > > > > > > > > @@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) > > > > > > > > /* init power sequencer on this pipe and port */ > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > > > > > > > /* > > > > * Even vdd force doesn't work until we've made > > > > @@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > > > > * Only the HW needs to be reprogrammed, the SW state is fixed and > > > > * has been setup during connector init. > > > > */ > > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > > > > > > return 0; > > > > } > > > > @@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) > > > > port_name(port), pipe_name(intel_dp->pps_pipe)); > > > > > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > > } > > > > > > > > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv) > > > > @@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp) > > > > > > > > /* init power sequencer on this pipe and port */ > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > > > } > > > > > > > > static void vlv_pre_enable_dp(struct intel_encoder *encoder, > > > > @@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > > > > > > > static void > > > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > > - struct intel_dp *intel_dp) > > > > + struct intel_dp *intel_dp, > > > > + bool force_disable_vdd) > > > > { > > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > > u32 pp_on, pp_off, pp_div, port_sel = 0; > > > > @@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > > > > > > intel_pps_get_registers(dev_priv, intel_dp, ®s); > > > > > > > > + /* > > > > + * One some VLV machines the BIOS can leave the VDD > > > > > > On > > > > > > > + * enabled even on power seqeuencers which aren't > > > > + * even hooked up to any port. This would mess up > > > > > > Remove even here > > > > > > > + * the power domain tracking the first time we pick > > > > + * one of these power sequencers for use since > > > > + * edp_panel_vdd_on() would notice that the VDD was > > > > + * already on and therefore wouldn't even grab the > > > > > > And here. > > > > > > > + * power domain reference. Disable VDD first to avoid > > > > + * this. This also avoids spuriously turning the VDD > > > > + * on as soon as the new power seqeuencer gets > > > > + * initialized. > > > > + */ > > > > + if (force_disable_vdd) { > > > > + u32 pp = ironlake_get_pp_control(intel_dp); > > > > + > > > > + WARN(pp & PANEL_POWER_ON, "Panel power already on\n"); > > > > > > Wouldn't this just replace one warning with another? Or is this > > > a subset of the other warning? > > > > This should never happen. But I'd rather we get a bug report if it ever > > does occur. > > Fair enough! > > > > > > > > > > + > > > > + if (pp & EDP_FORCE_VDD) > > > > + DRM_DEBUG_KMS("VDD already on, disabling first\n"); > > > > + > > > > + pp &= ~EDP_FORCE_VDD; > > > > + > > > > + I915_WRITE(regs.pp_ctrl, pp); > > > > + } > > > > + > > > > pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | > > > > (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT); > > > > pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | > > > > @@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev, > > > > vlv_initial_power_sequencer_setup(intel_dp); > > > > } else { > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > > } > > > > } > > > > > > > > -- > > > > 2.10.2 > > > > > > Kind regards, David > > > > -- > > Ville Syrj�l� > > Intel OTC > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it @ 2016-12-22 13:57 ` Ville Syrjälä 0 siblings, 0 replies; 11+ messages in thread From: Ville Syrjälä @ 2016-12-22 13:57 UTC (permalink / raw) To: intel-gfx, Matwey V . Kornilov, stable On Thu, Dec 22, 2016 at 01:57:26PM +0200, David Weinehall wrote: > On Thu, Dec 22, 2016 at 01:49:39PM +0200, Ville Syrjälä wrote: > > On Thu, Dec 22, 2016 at 11:39:37AM +0200, David Weinehall wrote: > > > On Tue, Dec 20, 2016 at 06:51:17PM +0200, ville.syrjala@linux.intel.com wrote: > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > Apparently some VLV BIOSen like to leave the VDD force bit enabled > > > > even for power seqeuncers that aren't properly hooked up to any > > > > port. That will result in a imbalance in the AUX power domain > > > > refcount when we stat to use said power sequencer as edp_panel_vdd_on() > > > > will not grab the power domain reference if it sees that the VDD is > > > > already on. > > > > > > > > To fix this let's make sure we turn off the VDD force bit when we > > > > initialize the power sequencer registers. That is, unless it's > > > > being done from the init path since there we are actually > > > > initializing the registers for the current power sequencer and > > > > we don't want to turn VDD off needlessly as that would require > > > > waiting for the power cycle delay before we turn it back on. > > > > > > > > This fixes the following kind of warnings: > > > > WARNING: CPU: 0 PID: 123 at ../drivers/gpu/drm/i915/intel_runtime_pm.c:1455 intel_display_power_put+0x13a/0x170 [i915]() > > > > WARN_ON(!power_domains->domain_use_count[domain]) > > > > ... > > > > > > > > Cc: stable@vger.kernel.org > > > > Cc: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > > > Tested-by: Matwey V. Kornilov <matwey.kornilov@gmail.com> > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98695 > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> Typos and whatnot fixed and patch pushed to dinq. Thanks for the review. > > > > > --- > > > > drivers/gpu/drm/i915/intel_dp.c | 42 ++++++++++++++++++++++++++++++++++------- > > > > 1 file changed, 35 insertions(+), 7 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > > > index 66b5bc80b781..4139122704b3 100644 > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > @@ -383,7 +383,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > > > struct intel_dp *intel_dp); > > > > static void > > > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > > - struct intel_dp *intel_dp); > > > > + struct intel_dp *intel_dp, > > > > + bool force_disable_vdd); > > > > static void > > > > intel_dp_pps_init(struct drm_device *dev, struct intel_dp *intel_dp); > > > > > > > > @@ -567,7 +568,7 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) > > > > > > > > /* init power sequencer on this pipe and port */ > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > > > > > > > /* > > > > * Even vdd force doesn't work until we've made > > > > @@ -604,7 +605,7 @@ bxt_power_sequencer_idx(struct intel_dp *intel_dp) > > > > * Only the HW needs to be reprogrammed, the SW state is fixed and > > > > * has been setup during connector init. > > > > */ > > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > > > > > > return 0; > > > > } > > > > @@ -687,7 +688,7 @@ vlv_initial_power_sequencer_setup(struct intel_dp *intel_dp) > > > > port_name(port), pipe_name(intel_dp->pps_pipe)); > > > > > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > > } > > > > > > > > void intel_power_sequencer_reset(struct drm_i915_private *dev_priv) > > > > @@ -2981,7 +2982,7 @@ static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp) > > > > > > > > /* init power sequencer on this pipe and port */ > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, true); > > > > } > > > > > > > > static void vlv_pre_enable_dp(struct intel_encoder *encoder, > > > > @@ -5152,7 +5153,8 @@ intel_dp_init_panel_power_sequencer(struct drm_device *dev, > > > > > > > > static void > > > > intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > > - struct intel_dp *intel_dp) > > > > + struct intel_dp *intel_dp, > > > > + bool force_disable_vdd) > > > > { > > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > > u32 pp_on, pp_off, pp_div, port_sel = 0; > > > > @@ -5165,6 +5167,32 @@ intel_dp_init_panel_power_sequencer_registers(struct drm_device *dev, > > > > > > > > intel_pps_get_registers(dev_priv, intel_dp, ®s); > > > > > > > > + /* > > > > + * One some VLV machines the BIOS can leave the VDD > > > > > > On > > > > > > > + * enabled even on power seqeuencers which aren't > > > > + * even hooked up to any port. This would mess up > > > > > > Remove even here > > > > > > > + * the power domain tracking the first time we pick > > > > + * one of these power sequencers for use since > > > > + * edp_panel_vdd_on() would notice that the VDD was > > > > + * already on and therefore wouldn't even grab the > > > > > > And here. > > > > > > > + * power domain reference. Disable VDD first to avoid > > > > + * this. This also avoids spuriously turning the VDD > > > > + * on as soon as the new power seqeuencer gets > > > > + * initialized. > > > > + */ > > > > + if (force_disable_vdd) { > > > > + u32 pp = ironlake_get_pp_control(intel_dp); > > > > + > > > > + WARN(pp & PANEL_POWER_ON, "Panel power already on\n"); > > > > > > Wouldn't this just replace one warning with another? Or is this > > > a subset of the other warning? > > > > This should never happen. But I'd rather we get a bug report if it ever > > does occur. > > Fair enough! > > > > > > > > > > + > > > > + if (pp & EDP_FORCE_VDD) > > > > + DRM_DEBUG_KMS("VDD already on, disabling first\n"); > > > > + > > > > + pp &= ~EDP_FORCE_VDD; > > > > + > > > > + I915_WRITE(regs.pp_ctrl, pp); > > > > + } > > > > + > > > > pp_on = (seq->t1_t3 << PANEL_POWER_UP_DELAY_SHIFT) | > > > > (seq->t8 << PANEL_LIGHT_ON_DELAY_SHIFT); > > > > pp_off = (seq->t9 << PANEL_LIGHT_OFF_DELAY_SHIFT) | > > > > @@ -5219,7 +5247,7 @@ static void intel_dp_pps_init(struct drm_device *dev, > > > > vlv_initial_power_sequencer_setup(intel_dp); > > > > } else { > > > > intel_dp_init_panel_power_sequencer(dev, intel_dp); > > > > - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); > > > > + intel_dp_init_panel_power_sequencer_registers(dev, intel_dp, false); > > > > } > > > > } > > > > > > > > -- > > > > 2.10.2 > > > > > > Kind regards, David > > > > -- > > Ville Syrjälä > > Intel OTC > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-12-22 13:57 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-20 16:51 [PATCH] drm/i915: Force VDD off on the new power seqeuencer before starting to use it ville.syrjala 2016-12-20 16:51 ` ville.syrjala 2016-12-20 18:45 ` ✓ Fi.CI.BAT: success for " Patchwork 2016-12-22 9:39 ` [Intel-gfx] [PATCH] " David Weinehall 2016-12-22 9:39 ` David Weinehall 2016-12-22 11:49 ` Ville Syrjälä 2016-12-22 11:49 ` Ville Syrjälä 2016-12-22 11:57 ` David Weinehall 2016-12-22 11:57 ` David Weinehall 2016-12-22 13:57 ` [Intel-gfx] " Ville Syrjälä 2016-12-22 13:57 ` Ville Syrjälä
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.