• Daniel Vetter's avatar
    Merge the modeset-rework, basic conversion into drm-intel-next · a1ceb677
    Daniel Vetter authored
    As a quick reference I'll detail the motivation and design of the new code a
    bit here (mostly stitched together from patchbomb announcements and commits
    introducing the new concepts).
    
    The crtc helper code has the fundamental assumption that encoders and crtcs can
    be enabled/disabled in any order, as long as we take care of depencies (which
    means that enabled encoders need an enabled crtc to feed them data,
    essentially).
    
    Our hw works differently. We already have tons of ugly cases where crtc code
    enables encoder hw (or encoder->mode_set enables stuff that should only be
    enabled in enocder->commit) to work around these issues. But on the disable
    side we can't pull off similar tricks - there we actually need to rework the
    modeset sequence that controls all this. And this is also the real motivation
    why I've finally undertaken this rewrite: eDP on my shiny new Ivybridge
    Ultrabook is broken, and it's broken due to the wrong disable sequence ...
    
    The new code introduces a few interfaces and concepts:
    
    - Add new encoder->enable/disable functions which are directly called from the
    crtc->enable/disable function. This ensures that the encoder's can be
    enabled/disabled at a very specific in the modeset sequence, controlled by our
    platform specific code (instead of the crtc helper code calling them at a time
    it deems convenient).
    
    - Rework the dpms code - our code has mostly 1:1 connector:encoder mappings and
    does support cloning on only a few encoders, so we can simplify things quite a
    bit.
    
    - Also only ever disable/enable the entire output pipeline. This ensures that
    we obey the right sequence of enabling/disabling things, trying to be clever
    here mostly just complicates the code and results in bugs. For cloneable
    encoders this requires a bit of special handling to ensure that outputs can
    still be disabled individually, but it simplifies the common case.
    
    - Add infrastructure to read out the current hw state. No amount of careful
    ordering will help us if we brick the hw on the initial modeset setup. Which
    could happen if we just randomly disable things, oblivious to the state set up
    by the bios. Hence we need to be able to read that out. As a benefit, we grow a
    few generic functions useful to cross-check our modeset code with actual hw
    state.
    
    With all this in place, we can copy&paste the crtc helper code into the
    drm/i915 driver and start to rework it:
    
    - As detailed above, the new code only disables/enables an entire output pipe.
    As a preparation for global mode-changes (e.g. reassigning shared resources) it
    keeps track of which pipes need to be touched by a set of bitmasks.
    
    - To ensure that we correctly disable the current display pipes, we need to
    know the currently active connector/encoder/crtc linking. The old crtc helper
    simply overwrote these links with the new setup, the new code stages the new
    links in ->new_* pointers. Those get commited to the real linking pointers once
    the old output configuration has been torn down, before the ->mode_set
    callbacks are called.
    
    - Finally the code adds tons of self-consistency checks by employing the new hw
    state readout functions to cross-check the actual hw state with what the
    datastructure think it should be. These checks are done both after every
    modeset and after the hw state has been read out and sanitized at boot/resume
    time. All these checks greatly helped in tracking down regressions and bugs in
    the new code.
    
    With this new basis, a lot of cleanups and improvements to the code are now
    possible (besides the DP fixes that ultimately made me write this), but not yet
    done:
    
    - I think we should create struct intel_mode and use it as the adjusted mode
    everywhere to store little pieces like needs_tvclock, pipe dithering values or
    dp link parameters. That would still be a layering violation, but at least we
    wouldn't need to recompute these kinds of things in intel_display.c. Especially
    the port bpc computation needed for selecting the pipe bpc and dithering
    settings in intel_display.c is rather gross.
    
    - In a related rework we could implement ->mode_valid in terms of ->mode_fixup
    in a generic way - I've hunted down too many bugs where ->mode_valid did the
    right thing, but ->mode_fixup didn't. Or vice versa, resulting in funny bugs
    for user-supplied modes.
    
    - Ditch the idea to rework the hdp handling in the common crtc helper code and
    just move things to i915.ko. Which would rid us of the ->detect crtc helper
    dependencies.
    
    - LVDS wire pair and pll enabling is all done in the crtc->mode_set function
    currently. We should be able to move this to the crtc_enable callbacks (or in
    the case of the LVDS wire pair enabling, into some encoder callback).
    
    Last, but not least, this new code should also help in enabling a few neat
    features: The hw state readout code prepares (but there are still big pieces
    missing) for fastboot, i.e. avoiding the inital modeset at boot-up and just
    taking over the configuration left behind by the bios. We also should be able
    to extend the configuration checks in the beginning of the modeset sequence and
    make better decisions about shared resources (which is the entire point behind
    the atomic/global modeset ioctl).
    Tested-by: default avatarJani Nikula <jani.nikula@intel.com>
    Tested-by: default avatarBen Widawsky <ben@bwidawsk.net>
    Tested-by: default avatarDamien Lespiau <damien.lespiau@intel.com>
    Tested-by: default avatarRodrigo Vivi <rodrigo.vivi@gmail.com>
    Acked-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
    Tested-by: default avatarVijay Purushothaman <vijay.a.purushothaman@intel.com>
    Acked-by: default avatarVijay Purushothaman <vijay.a.purushothaman@intel.com>
    Tested-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
    Acked-by: default avatarPaulo Zanoni <paulo.r.zanoni@intel.com>
    Tested-by: default avatarJesse Barnes <jbarnes@virtuousgeek.org>
    Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
    a1ceb677
intel_display.c 225 KB