On Thu, 28 Mar 2013 10:42:00 +0100
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> We need to be able to read out the hw state code for a bunch
> of reasons:
> - Correctly disabling boot-up/resume state.
> - Pure paranoia.
>
> Since not all of the pipe configuration is e.g. relevant for
> fastboot (or at least we can allow some wiggle room in some
> parameters, like the clocks), we need to add a strict_checking
> parameter to intel_pipe_config_compare for fastboot.
>
> For now intel_pipe_config_compare should be fully paranoid and
> check everything that the hw state readout code supports. Which
> for this infrastructure code is nothing.
>
> I've gone a bit overboard with adding 3 get_pipe_config functions:
> The ilk version will differ with the next patch, so it's not too
> onerous.
>
> v2: Don't check the hw config if the pipe is off, since an enabled,
> but dpms off crtc will obviously have tons of difference with the hw
> state.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 5 +++
> drivers/gpu/drm/i915/intel_display.c | 77 +++++++++++++++++++++++++++++++-----
> 2 files changed, 72 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 86777c8..99d7f81 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -285,6 +285,7 @@ struct drm_i915_error_state {
> };
>
> struct intel_crtc_config;
> +struct intel_crtc;
>
> struct drm_i915_display_funcs {
> bool (*fbc_enabled)(struct drm_device *dev);
> @@ -298,6 +299,10 @@ struct drm_i915_display_funcs {
> void (*update_linetime_wm)(struct drm_device *dev, int pipe,
> struct drm_display_mode *mode);
> void (*modeset_global_resources)(struct drm_device *dev);
> + /* Returns the active state of the crtc, and if the crtc is active,
> + * fills out the pipe-config with the hw state. */
> + bool (*get_pipe_config)(struct intel_crtc *,
> + struct intel_crtc_config *);
> int (*crtc_mode_set)(struct drm_crtc *crtc,
> int x, int y,
> struct drm_framebuffer *old_fb);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e925efe..a5adaa0 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4695,6 +4695,20 @@ static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> return ret;
> }
>
> +static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
> + struct intel_crtc_config *pipe_config)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + uint32_t tmp;
> +
> + tmp = I915_READ(PIPECONF(crtc->pipe));
> + if (!(tmp & PIPECONF_ENABLE))
> + return false;
> +
> + return true;
> +}
> +
> static void ironlake_init_pch_refclk(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5355,7 +5369,6 @@ static void ironlake_fdi_set_m_n(struct drm_crtc *crtc)
> &intel_crtc->config.adjusted_mode;
> struct intel_link_m_n m_n = {0};
> int target_clock, lane, link_bw;
> - uint32_t bps;
>
> /* FDI is a binary signal running at ~2.7GHz, encoding
> * each output octet as 10 bits. The actual frequency
> @@ -5611,6 +5624,20 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> return fdi_config_ok ? ret : -EINVAL;
> }
>
> +static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
> + struct intel_crtc_config *pipe_config)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + uint32_t tmp;
> +
> + tmp = I915_READ(PIPECONF(crtc->pipe));
> + if (!(tmp & PIPECONF_ENABLE))
> + return false;
> +
> + return true;
> +}
> +
> static void haswell_modeset_global_resources(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -5725,6 +5752,20 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> return ret;
> }
>
> +static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> + struct intel_crtc_config *pipe_config)
> +{
> + struct drm_device *dev = crtc->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + uint32_t tmp;
> +
> + tmp = I915_READ(PIPECONF(crtc->cpu_transcoder));
> + if (!(tmp & PIPECONF_ENABLE))
> + return false;
> +
> + return true;
> +}
> +
> static int intel_crtc_mode_set(struct drm_crtc *crtc,
> int x, int y,
> struct drm_framebuffer *fb)
> @@ -7647,12 +7688,21 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
> base.head) \
> if (mask & (1 <<(intel_crtc)->pipe)) \
>
> +static bool
> +intel_pipe_config_compare(struct intel_crtc_config *current_config,
> + struct intel_crtc_config *pipe_config)
> +{
> + return true;
> +}
> +
> void
> intel_modeset_check_state(struct drm_device *dev)
> {
> + drm_i915_private_t *dev_priv = dev->dev_private;
> struct intel_crtc *crtc;
> struct intel_encoder *encoder;
> struct intel_connector *connector;
> + struct intel_crtc_config pipe_config;
>
> list_for_each_entry(connector, &dev->mode_config.connector_list,
> base.head) {
> @@ -7741,7 +7791,15 @@ intel_modeset_check_state(struct drm_device *dev)
> "crtc's computed enabled state doesn't match tracked enabled state "
> "(expected %i, found %i)\n", enabled, crtc->base.enabled);
>
> - assert_pipe(dev->dev_private, crtc->pipe, crtc->active);
> + active = dev_priv->display.get_pipe_config(crtc,
> + &pipe_config);
> + WARN(crtc->active != active,
> + "crtc active state doesn't match with hw state "
> + "(expected %i, found %i)\n", crtc->active, active);
> +
> + WARN(active &&
> + !intel_pipe_config_compare(&crtc->config, &pipe_config),
> + "pipe state doesn't match!\n");
> }
> }
>
> @@ -8549,18 +8607,21 @@ static void intel_init_display(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> if (HAS_DDI(dev)) {
> + dev_priv->display.get_pipe_config = haswell_get_pipe_config;
> dev_priv->display.crtc_mode_set = haswell_crtc_mode_set;
> dev_priv->display.crtc_enable = haswell_crtc_enable;
> dev_priv->display.crtc_disable = haswell_crtc_disable;
> dev_priv->display.off = haswell_crtc_off;
> dev_priv->display.update_plane = ironlake_update_plane;
> } else if (HAS_PCH_SPLIT(dev)) {
> + dev_priv->display.get_pipe_config = ironlake_get_pipe_config;
> dev_priv->display.crtc_mode_set = ironlake_crtc_mode_set;
> dev_priv->display.crtc_enable = ironlake_crtc_enable;
> dev_priv->display.crtc_disable = ironlake_crtc_disable;
> dev_priv->display.off = ironlake_crtc_off;
> dev_priv->display.update_plane = ironlake_update_plane;
> } else {
> + dev_priv->display.get_pipe_config = i9xx_get_pipe_config;
> dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set;
> dev_priv->display.crtc_enable = i9xx_crtc_enable;
> dev_priv->display.crtc_disable = i9xx_crtc_disable;
> @@ -9092,14 +9153,10 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
> }
>
> setup_pipes:
> - for_each_pipe(pipe) {
> - crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> -
> - tmp = I915_READ(PIPECONF(crtc->cpu_transcoder));
> - if (tmp & PIPECONF_ENABLE)
> - crtc->active = true;
> - else
> - crtc->active = false;
> + list_for_each_entry(crtc, &dev->mode_config.crtc_list,
> + base.head) {
> + crtc->active = dev_priv->display.get_pipe_config(crtc,
> + &crtc->config);
>
> crtc->base.enabled = crtc->active;
>
Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>
--
Jesse Barnes, Intel Open Source Technology Center
