[Intel-gfx] [PATCH 6/8] drm/i915: Enable/Disable PSR

[Jesse Barnes]

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