• Lyude's avatar
    drm/i915/skl: Update plane watermarks atomically during plane updates · 62e0fb88
    Lyude authored
    Thanks to Ville for suggesting this as a potential solution to pipe
    underruns on Skylake.
    
    On Skylake all of the registers for configuring planes, including the
    registers for configuring their watermarks, are double buffered. New
    values written to them won't take effect until said registers are
    "armed", which is done by writing to the PLANE_SURF (or in the case of
    cursor planes, the CURBASE register) register.
    
    With this in mind, up until now we've been updating watermarks on skl
    like this:
    
      non-modeset {
       - calculate (during atomic check phase)
       - finish_atomic_commit:
         - intel_pre_plane_update:
            - intel_update_watermarks()
         - {vblank happens; new watermarks + old plane values => underrun }
         - drm_atomic_helper_commit_planes_on_crtc:
            - start vblank evasion
            - write new plane registers
            - end vblank evasion
      }
    
      or
    
      modeset {
       - calculate (during atomic check phase)
       - finish_atomic_commit:
         - crtc_enable:
            - intel_update_watermarks()
         - {vblank happens; new watermarks + old plane values => underrun }
         - drm_atomic_helper_commit_planes_on_crtc:
            - start vblank evasion
            - write new plane registers
            - end vblank evasion
      }
    
    Now we update watermarks atomically like this:
    
      non-modeset {
       - calculate (during atomic check phase)
       - finish_atomic_commit:
         - intel_pre_plane_update:
            - intel_update_watermarks() (wm values aren't written yet)
         - drm_atomic_helper_commit_planes_on_crtc:
            - start vblank evasion
            - write new plane registers
            - write new wm values
            - end vblank evasion
      }
    
      modeset {
       - calculate (during atomic check phase)
       - finish_atomic_commit:
         - crtc_enable:
            - intel_update_watermarks() (actual wm values aren't written
              yet)
         - drm_atomic_helper_commit_planes_on_crtc:
            - start vblank evasion
            - write new plane registers
    	- write new wm values
            - end vblank evasion
      }
    
    So this patch moves all of the watermark writes into the right place;
    inside of the vblank evasion where we update all of the registers for
    each plane. While this patch doesn't fix everything, it does allow us to
    update the watermark values in the way the hardware expects us to.
    
    Changes since original patch series:
     - Remove mutex_lock/mutex_unlock since they don't do anything and we're
       not touching global state
     - Move skl_write_cursor_wm/skl_write_plane_wm functions into
       intel_pm.c, make externally visible
     - Add skl_write_plane_wm calls to skl_update_plane
     - Fix conditional for for loop in skl_write_plane_wm (level < max_level
       should be level <= max_level)
     - Make diagram in commit more accurate to what's actually happening
     - Add Fixes:
    
    Changes since v1:
     - Use IS_GEN9() instead of IS_SKYLAKE() since these fixes apply to more
       then just Skylake
     - Update description to make it clear this patch doesn't fix everything
     - Check if pipes were actually changed before writing watermarks
    
    Changes since v2:
     - Write PIPE_WM_LINETIME during vblank evasion
    
    Changes since v3:
     - Rebase against new SAGV patch changes
    
    Changes since v4:
     - Add a parameter to choose what skl_wm_values struct to use when
       writing new plane watermarks
    
    Changes since v5:
     - Remove cursor ddb entry write in skl_write_cursor_wm(), defer until
       patch 6
     - Write WM_LINETIME in intel_begin_crtc_commit()
    
    Changes since v6:
     - Remove redundant dirty_pipes check in skl_write_plane_wm (we check
       this in all places where we call this function, and it was supposed
       to have been removed earlier anyway)
     - In i9xx_update_cursor(), use dev_priv->info.gen >= 9 instead of
       IS_GEN9(dev_priv). We do this everywhere else and I'd imagine this
       needs to be done for gen10 as well
    
    Changes since v7:
     - Fix rebase fail (unused variable obj)
     - Make struct skl_wm_values *wm const
     - Fix indenting
     - Use INTEL_GEN() instead of dev_priv->info.gen
    
    Changes since v8:
     - Don't forget calls to skl_write_plane_wm() when disabling planes
     - Use INTEL_GEN(), not INTEL_INFO()->gen in intel_begin_crtc_commit()
    
    Fixes: 2d41c0b5 ("drm/i915/skl: SKL Watermark Computation")
    Signed-off-by: default avatarLyude <cpaul@redhat.com>
    Reviewed-by: default avatarMatt Roper <matthew.d.roper@intel.com>
    Cc: stable@vger.kernel.org
    Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
    Cc: Daniel Vetter <daniel.vetter@intel.com>
    Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
    Cc: Hans de Goede <hdegoede@redhat.com>
    Signed-off-by: default avatarMaarten Lankhorst <maarten.lankhorst@linux.intel.com>
    Link: http://patchwork.freedesktop.org/patch/msgid/1471884608-10671-1-git-send-email-cpaul@redhat.com
    Link: http://patchwork.freedesktop.org/patch/msgid/1471884608-10671-1-git-send-email-cpaul@redhat.com
    62e0fb88
intel_sprite.c 32.7 KB