On Fri, Nov 01, 2013 at 10:50:21AM +0100, Daniel Vetter wrote:
> + /* DP port CRC needs slight spec mis-compliance to get stable CRCs. */
We could add a slighty better explanation. How about:
/*
* When the pipe CRC tap point is after the transcoders we need
* to tweak symbol-level features to produce a deterministic series of
* symbols for a given frame. We need to reset those features only once
* a frame (instead of every nth symbol):
* - DC-balance: used to ensure a better clock recovery from the data
* link (SDVO)
* - DisplayPort scrambling: used for EMI reduction
*/
> + if (need_scramble_reset) {
so then maybe rename this with 'need_stable_symbols'
> + uint32_t tmp = I915_READ(PORT_DFT2_G4X);
> +
> + WARN_ON(!IS_G4X(dev));
> +
> + I915_WRITE(PORT_DFT_I9XX,
> + I915_READ(PORT_DFT_I9XX) | DC_BALANCE_RESET);
> +
> + if (pipe == PIPE_A)
> + tmp |= PIPE_A_SCRAMBLE_RESET;
> + else
> + tmp |= PIPE_B_SCRAMBLE_RESET;
> +
> + I915_WRITE(PORT_DFT2_G4X, tmp);
> + }
> +
> return 0;
> }
>
> +static void g4x_undo_pipe_scramble_reset(struct drm_device *dev,
> + enum pipe pipe)
> +{
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + uint32_t tmp = I915_READ(PORT_DFT2_G4X);
> +
> + if (pipe == PIPE_A)
> + tmp &= ~PIPE_A_SCRAMBLE_RESET;
> + else
> + tmp &= ~PIPE_B_SCRAMBLE_RESET;
> + I915_WRITE(PORT_DFT2_G4X, tmp);
> +
> + if (!(tmp & PIPE_SCRAMBLE_RESET_MASK)) {
> + I915_WRITE(PORT_DFT_I9XX,
> + I915_READ(PORT_DFT_I9XX) & ~DC_BALANCE_RESET);
> + }
> +}
It took me a bit of time to realize that you were only clearing the
"DC-Balance reset once a frame" bit when you're sure that we have no
pipe CRC needing stable symbols running at all on either pipe. And this
by using the SCRAMBLER_RESET bits in DFT2 as the state tracker.
Maybe add a small comment here to avoid those 20s of head scratching
when reading that code again in a bit.
--
Damien
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
