On Tue, 26 Feb 2013 14:48:33 +0200
Ville Syrj?l? <ville.syrjala at linux.intel.com> wrote:
> On Mon, Feb 25, 2013 at 07:55:20PM -0300, Rodrigo Vivi wrote:
> > Adding Enable and Disable PSR functionalities. This includes setting the
> > PSR configuration over AUX, sending SDP VSC DIP over the eDP PIPE config,
> > enabling PSR in the sink via DPCD register and finally enabling PSR on
> > the host.
> >
> > This patch is heavily based on initial PSR code by Sateesh Kavuri and
> > Kumar Shobhit but in a different implementation.
> >
> > Credits-by: Sateesh Kavuri <sateesh.kavuri at intel.com>
> > Credits-by: Shobhit Kumar <shobhit.kumar at intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi at gmail.com>
> > ---
> > drivers/gpu/drm/i915/i915_reg.h | 40 +++++++++
> > drivers/gpu/drm/i915/intel_dp.c | 183 +++++++++++++++++++++++++++++++++++++++
> > drivers/gpu/drm/i915/intel_drv.h | 3 +
> > 3 files changed, 226 insertions(+)
> >
> <snip>
> > +void intel_edp_write_vsc_psr(struct intel_dp* intel_dp,
> > + struct edp_vsc_psr *vsc_psr)
> > +{
> > + struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > + struct drm_i915_private *dev_priv = dev->dev_private;
> > + struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp_to_crtc(intel_dp));
> > + u32 ctl_reg = HSW_TVIDEO_DIP_CTL(intel_crtc->cpu_transcoder);
> > + u32 data_reg = HSW_TVIDEO_DIP_VSC_DATA(intel_crtc->cpu_transcoder);
> > + uint32_t *data = (uint32_t *)vsc_psr;
> > + unsigned int i;
> > + u32 val = I915_READ(ctl_reg);
> > +
> > + if (data_reg == 0)
> > + return;
> > +
> > + /* As per eDP spec, wait for vblank to send SDP VSC packet */
> > + intel_wait_for_vblank(dev, intel_crtc->pipe);
> > +
> > + mmiowb();
>
> I was curious about these mmiowb()s and apparently they were added to
> all infoframe writes "just in case". But AFAICS on x86 mmiowb() ends
> up as a compiler barrier, so this stuff seems to be a nop.
>
> And if these writes can get reordered somewhere, why not everything
> else too? I'm sure we have places where we write a bunch of registers,
> and the final write enables something which requires the earlier
> writes to have landed beforehand.
Yeah we shouldn't need mmiowb() anywhere in our driver (until our
on-chip topology gets really complicated anyway).
--
Jesse Barnes, Intel Open Source Technology Center
