Re: [PATCH 4/5] drm/i915: PSR VLV: Add single frame update.

Vivi, Rodrigo
2015-04-14T18:09:28Z

On Tue, 2015-04-14 at 20:04 +0200, Daniel Vetter wrote:
> On Fri, Apr 10, 2015 at 11:15:10AM -0700, Rodrigo Vivi wrote:
> > According to spec: "In PSR HW or SW mode, SW set this bit before writing
> > registers for a flip. It will be self-clear when it gets to the PSR
> > active state."
> >
> > Some versions of spec mention that this is needed when in
> > "Persistent mode" but define it as same as "SW mode". Since this
> > fix the page flip case let's assume this is exactly what we need.
> >
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Merged up to this one to dinq, will wait with the final enable until the
> bug Matthew reported is a bit clearer.

Thank you! I agree it is safe to wait,
but I believe the fbcon roling screen is the ips issue with resolution
and clock.
and the slowness on X typping is something else that is triggered with
some set that powertop --auto-tune uses, because I'm also facing this
even with PSR disabled. I'll investigate that anyway.

So I'm confident Matthew's issues aren't PSR related.

Thanks again,
Rodrigo.

> -Daniel
>
> > ---
> > drivers/gpu/drm/i915/intel_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_frontbuffer.c | 2 ++
> > drivers/gpu/drm/i915/intel_psr.c | 42 ++++++++++++++++++++++++++++++++
> > 3 files changed, 45 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 7a0aa24..9c5d1cd 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1220,6 +1220,7 @@ void intel_psr_invalidate(struct drm_device *dev,
> > void intel_psr_flush(struct drm_device *dev,
> > unsigned frontbuffer_bits);
> > void intel_psr_init(struct drm_device *dev);
> > +void intel_psr_single_frame_update(struct drm_device *dev);
> >
> > /* intel_runtime_pm.c */
> > int intel_power_domains_init(struct drm_i915_private *);
> > diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
> > index a20cffb..57095f5 100644
> > --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> > @@ -243,6 +243,8 @@ void intel_frontbuffer_flip_prepare(struct drm_device *dev,
> > /* Remove stale busy bits due to the old buffer. */
> > dev_priv->fb_tracking.busy_bits &= ~frontbuffer_bits;
> > mutex_unlock(&dev_priv->fb_tracking.lock);
> > +
> > + intel_psr_single_frame_update(dev);
> > }
> >
> > /**
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index 5cd374b..5ee0fa5 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -594,6 +594,48 @@ static void intel_psr_exit(struct drm_device *dev)
> > }
> >
> > /**
> > + * intel_psr_single_frame_update - Single Frame Update
> > + * @dev: DRM device
> > + *
> > + * Some platforms support a single frame update feature that is used to
> > + * send and update only one frame on Remote Frame Buffer.
> > + * So far it is only implemented for Valleyview and Cherryview because
> > + * hardware requires this to be done before a page flip.
> > + */
> > +void intel_psr_single_frame_update(struct drm_device *dev)
> > +{
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + struct drm_crtc *crtc;
> > + enum pipe pipe;
> > + u32 val;
> > +
> > + /*
> > + * Single frame update is already supported on BDW+ but it requires
> > + * many W/A and it isn't really needed.
> > + */
> > + if (!IS_VALLEYVIEW(dev))
> > + return;
> > +
> > + mutex_lock(&dev_priv->psr.lock);
> > + if (!dev_priv->psr.enabled) {
> > + mutex_unlock(&dev_priv->psr.lock);
> > + return;
> > + }
> > +
> > + crtc = dp_to_dig_port(dev_priv->psr.enabled)->base.base.crtc;
> > + pipe = to_intel_crtc(crtc)->pipe;
> > + val = I915_READ(VLV_PSRCTL(pipe));
> > +
> > + /*
> > + * We need to set this bit before writing registers for a flip.
> > + * This bit will be self-clear when it gets to the PSR active state.
> > + */
> > + I915_WRITE(VLV_PSRCTL(pipe), val | VLV_EDP_PSR_SINGLE_FRAME_UPDATE);
> > +
> > + mutex_unlock(&dev_priv->psr.lock);
> > +}
> > +
> > +/**
> > * intel_psr_invalidate - Invalidade PSR
> > * @dev: DRM device
> > * @frontbuffer_bits: frontbuffer plane tracking bits
> > --
> > 2.1.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx