| Message ID | 20180913212251.22283-2-jose.souza@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/i915/runtime_pm: Share code to enable/disable PCH reset handshake | expand |
On Thu, Sep 13, 2018 at 02:22:49PM -0700, José Roberto de Souza wrote: > Right now RESET_PCH_HANDSHAKE_ENABLE is enabled all the times inside > of intel_power_domains_init_hw() and if PCH is NOP it is unsed in > i915_gem_init_hw(). > So making skl_pch_reset_handshake() handle both cases and calling > it for the missing gens in intel_power_domains_init_hw(). > Ivybridge have a different register and bits but with the same > objective so moving it too. > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 12 ------------ > drivers/gpu/drm/i915/intel_runtime_pm.c | 12 +++++++++++- > 2 files changed, 11 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 89834ce19acd..b389e084c8c6 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -5296,18 +5296,6 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv) > I915_WRITE(MI_PREDICATE_RESULT_2, IS_HSW_GT3(dev_priv) ? > LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED); > > - if (HAS_PCH_NOP(dev_priv)) { > - if (IS_IVYBRIDGE(dev_priv)) { > - u32 temp = I915_READ(GEN7_MSG_CTL); > - temp &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK); > - I915_WRITE(GEN7_MSG_CTL, temp); > - } else if (INTEL_GEN(dev_priv) >= 7) { > - u32 temp = I915_READ(HSW_NDE_RSTWRN_OPT); > - temp &= ~RESET_PCH_HANDSHAKE_ENABLE; > - I915_WRITE(HSW_NDE_RSTWRN_OPT, temp); > - } > - } > - > intel_gt_workarounds_apply(dev_priv); > > i915_gem_init_swizzling(dev_priv); > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 8bcb33367d0d..369a292cafac 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -3239,11 +3239,12 @@ static void icl_mbus_init(struct drm_i915_private *dev_priv) > I915_WRITE(MBUS_ABOX_CTL, val); > } > > +/* Actually it is hsw+ but until skl it was not required to set it */ > static void skl_pch_reset_handshake(struct drm_i915_private *dev_priv) > { > u32 val = I915_READ(HSW_NDE_RSTWRN_OPT); > > - if (HAS_PCH_SPLIT(dev_priv)) > + if (HAS_PCH_SPLIT(dev_priv) && !HAS_PCH_NOP(dev_priv)) now I saw why you wanted the unset bit here as well ;) > val |= RESET_PCH_HANDSHAKE_ENABLE; > else > val &= ~RESET_PCH_HANDSHAKE_ENABLE; > @@ -3758,6 +3759,15 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume) > mutex_lock(&power_domains->lock); > vlv_cmnlane_wa(dev_priv); > mutex_unlock(&power_domains->lock); > + } else if (IS_IVYBRIDGE(dev_priv)) { > + if (HAS_PCH_NOP(dev_priv)) { > + u32 val = I915_READ(GEN7_MSG_CTL); > + > + val &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK); > + I915_WRITE(GEN7_MSG_CTL, val); > + } > + } else if (INTEL_GEN(dev_priv) >= 7) { > + skl_pch_reset_handshake(dev_priv); well... skl_ doesn't make sense anymore... maybe s/skl/hsw ? but what about the remaining block for Ivybridge? I feel it got little unbalanced here... maybe we move everything inside to a intel_pch_reset_handshake(dev_priv, bool set) ? > } > > /* > -- > 2.19.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Thu, 2018-09-13 at 14:51 -0700, Rodrigo Vivi wrote: > On Thu, Sep 13, 2018 at 02:22:49PM -0700, José Roberto de Souza > wrote: > > Right now RESET_PCH_HANDSHAKE_ENABLE is enabled all the times > > inside > > of intel_power_domains_init_hw() and if PCH is NOP it is unsed in > > i915_gem_init_hw(). > > So making skl_pch_reset_handshake() handle both cases and calling > > it for the missing gens in intel_power_domains_init_hw(). > > Ivybridge have a different register and bits but with the same > > objective so moving it too. > > > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > --- > > drivers/gpu/drm/i915/i915_gem.c | 12 ------------ > > drivers/gpu/drm/i915/intel_runtime_pm.c | 12 +++++++++++- > > 2 files changed, 11 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c > > index 89834ce19acd..b389e084c8c6 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -5296,18 +5296,6 @@ int i915_gem_init_hw(struct drm_i915_private > > *dev_priv) > > I915_WRITE(MI_PREDICATE_RESULT_2, IS_HSW_GT3(dev_priv) > > ? > > LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED); > > > > - if (HAS_PCH_NOP(dev_priv)) { > > - if (IS_IVYBRIDGE(dev_priv)) { > > - u32 temp = I915_READ(GEN7_MSG_CTL); > > - temp &= ~(WAIT_FOR_PCH_FLR_ACK | > > WAIT_FOR_PCH_RESET_ACK); > > - I915_WRITE(GEN7_MSG_CTL, temp); > > - } else if (INTEL_GEN(dev_priv) >= 7) { > > - u32 temp = I915_READ(HSW_NDE_RSTWRN_OPT); > > - temp &= ~RESET_PCH_HANDSHAKE_ENABLE; > > - I915_WRITE(HSW_NDE_RSTWRN_OPT, temp); > > - } > > - } > > - > > intel_gt_workarounds_apply(dev_priv); > > > > i915_gem_init_swizzling(dev_priv); > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c > > b/drivers/gpu/drm/i915/intel_runtime_pm.c > > index 8bcb33367d0d..369a292cafac 100644 > > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > > @@ -3239,11 +3239,12 @@ static void icl_mbus_init(struct > > drm_i915_private *dev_priv) > > I915_WRITE(MBUS_ABOX_CTL, val); > > } > > > > +/* Actually it is hsw+ but until skl it was not required to set it > > */ > > static void skl_pch_reset_handshake(struct drm_i915_private > > *dev_priv) > > { > > u32 val = I915_READ(HSW_NDE_RSTWRN_OPT); > > > > - if (HAS_PCH_SPLIT(dev_priv)) > > + if (HAS_PCH_SPLIT(dev_priv) && !HAS_PCH_NOP(dev_priv)) > > now I saw why you wanted the unset bit here as well ;) > > > val |= RESET_PCH_HANDSHAKE_ENABLE; > > else > > val &= ~RESET_PCH_HANDSHAKE_ENABLE; > > @@ -3758,6 +3759,15 @@ void intel_power_domains_init_hw(struct > > drm_i915_private *dev_priv, bool resume) > > mutex_lock(&power_domains->lock); > > vlv_cmnlane_wa(dev_priv); > > mutex_unlock(&power_domains->lock); > > + } else if (IS_IVYBRIDGE(dev_priv)) { > > + if (HAS_PCH_NOP(dev_priv)) { > > + u32 val = I915_READ(GEN7_MSG_CTL); > > + > > + val &= ~(WAIT_FOR_PCH_FLR_ACK | > > WAIT_FOR_PCH_RESET_ACK); > > + I915_WRITE(GEN7_MSG_CTL, val); > > + } > > + } else if (INTEL_GEN(dev_priv) >= 7) { > > + skl_pch_reset_handshake(dev_priv); > > well... skl_ doesn't make sense anymore... > maybe s/skl/hsw ? > > but what about the remaining block for Ivybridge? > I feel it got little unbalanced here... > > maybe we move everything inside to a > intel_pch_reset_handshake(dev_priv, bool set) ? I guess move everything to intel_pch_reset_handshake() sounds better but without the bool set, as the HAS_PCH_NOP can be set to all those platforms depending if display is on or off. > > > } > > > > /* > > -- > > 2.19.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 89834ce19acd..b389e084c8c6 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -5296,18 +5296,6 @@ int i915_gem_init_hw(struct drm_i915_private *dev_priv) I915_WRITE(MI_PREDICATE_RESULT_2, IS_HSW_GT3(dev_priv) ? LOWER_SLICE_ENABLED : LOWER_SLICE_DISABLED); - if (HAS_PCH_NOP(dev_priv)) { - if (IS_IVYBRIDGE(dev_priv)) { - u32 temp = I915_READ(GEN7_MSG_CTL); - temp &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK); - I915_WRITE(GEN7_MSG_CTL, temp); - } else if (INTEL_GEN(dev_priv) >= 7) { - u32 temp = I915_READ(HSW_NDE_RSTWRN_OPT); - temp &= ~RESET_PCH_HANDSHAKE_ENABLE; - I915_WRITE(HSW_NDE_RSTWRN_OPT, temp); - } - } - intel_gt_workarounds_apply(dev_priv); i915_gem_init_swizzling(dev_priv); diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index 8bcb33367d0d..369a292cafac 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -3239,11 +3239,12 @@ static void icl_mbus_init(struct drm_i915_private *dev_priv) I915_WRITE(MBUS_ABOX_CTL, val); } +/* Actually it is hsw+ but until skl it was not required to set it */ static void skl_pch_reset_handshake(struct drm_i915_private *dev_priv) { u32 val = I915_READ(HSW_NDE_RSTWRN_OPT); - if (HAS_PCH_SPLIT(dev_priv)) + if (HAS_PCH_SPLIT(dev_priv) && !HAS_PCH_NOP(dev_priv)) val |= RESET_PCH_HANDSHAKE_ENABLE; else val &= ~RESET_PCH_HANDSHAKE_ENABLE; @@ -3758,6 +3759,15 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume) mutex_lock(&power_domains->lock); vlv_cmnlane_wa(dev_priv); mutex_unlock(&power_domains->lock); + } else if (IS_IVYBRIDGE(dev_priv)) { + if (HAS_PCH_NOP(dev_priv)) { + u32 val = I915_READ(GEN7_MSG_CTL); + + val &= ~(WAIT_FOR_PCH_FLR_ACK | WAIT_FOR_PCH_RESET_ACK); + I915_WRITE(GEN7_MSG_CTL, val); + } + } else if (INTEL_GEN(dev_priv) >= 7) { + skl_pch_reset_handshake(dev_priv); } /*
Right now RESET_PCH_HANDSHAKE_ENABLE is enabled all the times inside of intel_power_domains_init_hw() and if PCH is NOP it is unsed in i915_gem_init_hw(). So making skl_pch_reset_handshake() handle both cases and calling it for the missing gens in intel_power_domains_init_hw(). Ivybridge have a different register and bits but with the same objective so moving it too. Signed-off-by: José Roberto de Souza <jose.souza@intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 12 ------------ drivers/gpu/drm/i915/intel_runtime_pm.c | 12 +++++++++++- 2 files changed, 11 insertions(+), 13 deletions(-)