| Message ID | 20180625054741.3919-2-dhinakaran.pandiyan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Jun 24, 2018 at 10:47:41PM -0700, Dhinakaran Pandiyan wrote: > Depending whether PSR1 or PSR2 was configured, we print a warning if the > corresponding control mmio indicated PSR was erroneously enabled. As > Chris pointed out, it makes more sense to check for both the mmio's > since we expect neither PSR1 nor PSR2 to be enabled when psr_activate() is > called. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > --- > drivers/gpu/drm/i915/intel_psr.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c > index 7aa324f0d1f7..970b8ced46a3 100644 > --- a/drivers/gpu/drm/i915/intel_psr.c > +++ b/drivers/gpu/drm/i915/intel_psr.c > @@ -576,10 +576,8 @@ static void intel_psr_activate(struct intel_dp *intel_dp) > struct drm_device *dev = intel_dig_port->base.base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > > - if (dev_priv->psr.psr2_enabled) > - WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); > - else > - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > + WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); now you have to check for platform which really has this register to avoid unclaimed registers accesses... > + WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > WARN_ON(dev_priv->psr.active); > lockdep_assert_held(&dev_priv->psr.lock); > > -- > 2.14.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, 2018-06-25 at 17:09 -0700, Rodrigo Vivi wrote: > On Sun, Jun 24, 2018 at 10:47:41PM -0700, Dhinakaran Pandiyan wrote: > > > > Depending whether PSR1 or PSR2 was configured, we print a warning > > if the > > corresponding control mmio indicated PSR was erroneously enabled. > > As > > Chris pointed out, it makes more sense to check for both the mmio's > > since we expect neither PSR1 nor PSR2 to be enabled when > > psr_activate() is > > called. > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> > > --- > > drivers/gpu/drm/i915/intel_psr.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_psr.c > > b/drivers/gpu/drm/i915/intel_psr.c > > index 7aa324f0d1f7..970b8ced46a3 100644 > > --- a/drivers/gpu/drm/i915/intel_psr.c > > +++ b/drivers/gpu/drm/i915/intel_psr.c > > @@ -576,10 +576,8 @@ static void intel_psr_activate(struct intel_dp > > *intel_dp) > > struct drm_device *dev = intel_dig_port->base.base.dev; > > struct drm_i915_private *dev_priv = to_i915(dev); > > > > - if (dev_priv->psr.psr2_enabled) > > - WARN_ON(I915_READ(EDP_PSR2_CTL) & > > EDP_PSR2_ENABLE); > > - else > > - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > + WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); > now you have to check for platform which really has this register > to avoid unclaimed registers accesses... > I'll add this? if (INTEL_GEN(dev_priv) > 9) WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); > > > > + WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); > > WARN_ON(dev_priv->psr.active); > > lockdep_assert_held(&dev_priv->psr.lock); > >
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index 7aa324f0d1f7..970b8ced46a3 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -576,10 +576,8 @@ static void intel_psr_activate(struct intel_dp *intel_dp) struct drm_device *dev = intel_dig_port->base.base.dev; struct drm_i915_private *dev_priv = to_i915(dev); - if (dev_priv->psr.psr2_enabled) - WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); - else - WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); + WARN_ON(I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE); + WARN_ON(I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE); WARN_ON(dev_priv->psr.active); lockdep_assert_held(&dev_priv->psr.lock);
Depending whether PSR1 or PSR2 was configured, we print a warning if the corresponding control mmio indicated PSR was erroneously enabled. As Chris pointed out, it makes more sense to check for both the mmio's since we expect neither PSR1 nor PSR2 to be enabled when psr_activate() is called. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> --- drivers/gpu/drm/i915/intel_psr.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)