On Tue, 26 Feb 2013 14:48:33 +0200 Ville Syrjälä ville.syrjala@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@intel.com Credits-by: Shobhit Kumar shobhit.kumar@intel.com Signed-off-by: Rodrigo Vivi rodrigo.vivi@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).