From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 30/58] drm/i915: read out the modeset hw state at load and resume time Date: Wed, 5 Sep 2012 09:14:06 -0700 Message-ID: <20120905091406.1ade3baa@jbarnes-desktop> References: <1345403595-9678-1-git-send-email-daniel.vetter@ffwll.ch> <1345403595-9678-31-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy6-pub.bluehost.com (oproxy6-pub.bluehost.com [67.222.54.6]) by gabe.freedesktop.org (Postfix) with SMTP id 24A429E93A for ; Wed, 5 Sep 2012 09:14:09 -0700 (PDT) In-Reply-To: <1345403595-9678-31-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Sun, 19 Aug 2012 21:12:47 +0200 Daniel Vetter wrote: > +static void > +intel_connector_break_all_links(struct intel_connector *connector) > +{ > + connector->base.dpms = DRM_MODE_DPMS_OFF; > + connector->base.encoder = NULL; > + connector->encoder->connectors_active = false; > + connector->encoder->base.crtc = NULL; > +} Maybe connector_clear_state() or init_state()? instead? > + > +static void intel_sanitize_crtc(struct intel_crtc *crtc) > +{ > + struct drm_device *dev = crtc->base.dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + u32 reg, val; > + > + /* Clear the dpms state for compatibility with code still using that > + * deprecated state variable. */ > + crtc->dpms_mode = -1; > + > + /* Clear any frame start delays used for debugging left by the BIOS */ > + reg = PIPECONF(crtc->pipe); > + I915_WRITE(reg, I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK); > + > + /* We need to sanitize the plane -> pipe mapping first because this will > + * disable the crtc (and hence change the state) if it is wrong. */ > + if (!HAS_PCH_SPLIT(dev)) { > + struct intel_connector *connector; > + bool plane; > + > + reg = DSPCNTR(crtc->plane); > + val = I915_READ(reg); > + > + if ((val & DISPLAY_PLANE_ENABLE) == 0 && > + (!!(val & DISPPLANE_SEL_PIPE_MASK) == crtc->pipe)) > + goto ok; > + > + DRM_DEBUG_KMS("[CRTC:%d] wrong plane connection detected!\n", > + crtc->base.base.id); > + > + /* Pipe has the wrong plane attached and the plane is active. > + * Temporarily change the plane mapping and disable everything > + * ... */ > + plane = crtc->plane; > + crtc->plane = !plane; > + dev_priv->display.crtc_disable(&crtc->base); > + crtc->plane = plane; > + > + /* ... and break all links. */ > + list_for_each_entry(connector, &dev->mode_config.connector_list, > + base.head) { > + if (connector->encoder->base.crtc != &crtc->base) > + continue; > + > + intel_connector_break_all_links(connector); > + } > + > + WARN_ON(crtc->active); > + crtc->base.enabled = false; > + } > +ok: > + > + /* Adjust the state of the output pipe according to whether we > + * have active connectors/encoders. */ > + intel_crtc_update_dpms(&crtc->base); > + > + if (crtc->active != crtc->base.enabled) { > + struct intel_encoder *encoder; > + > + /* This can happen either due to bugs in the get_hw_state > + * functions or because the pipe is force-enabled due to the > + * pipe A quirk. */ > + DRM_DEBUG_KMS("[CRTC:%d] hw state adjusted, was %s, now %s\n", > + crtc->base.base.id, > + crtc->base.enabled ? "enabled" : "disabled", > + crtc->active ? "enabled" : "disabled"); > + > + crtc->base.enabled = crtc->active; > + > + /* Because we only establish the connector -> encoder -> > + * crtc links if something is active, this means the > + * crtc is now deactivated. Break the links. connector > + * -> encoder links are only establish when things are > + * actually up, hence no need to break them. */ > + WARN_ON(crtc->active); > + > + for_each_encoder_on_crtc(dev, &crtc->base, encoder) { > + WARN_ON(encoder->connectors_active); > + encoder->base.crtc = NULL; > + } > + } > +} > + > +static void intel_sanitize_encoder(struct intel_encoder *encoder) > +{ > + struct intel_connector *connector; > + struct drm_device *dev = encoder->base.dev; > + > + /* We need to check both for a crtc link (meaning that the > + * encoder is active and trying to read from a pipe) and the > + * pipe itself being active. */ > + bool has_active_crtc = encoder->base.crtc && > + to_intel_crtc(encoder->base.crtc)->active; > + > + if (encoder->connectors_active && !has_active_crtc) { > + DRM_DEBUG_KMS("[ENCODER:%d:%s] has active connectors but no active pipe!\n", > + encoder->base.base.id, > + drm_get_encoder_name(&encoder->base)); > + > + /* Connector is active, but has no active pipe. This is > + * fallout from our resume register restoring. Disable > + * the encoder manually again. */ > + if (encoder->base.crtc) { > + DRM_DEBUG_KMS("[ENCODER:%d:%s] manually disabled\n", > + encoder->base.base.id, > + drm_get_encoder_name(&encoder->base)); > + encoder->disable(encoder); > + } > + > + /* Inconsisten output/port/pipe state happens presumably due to > + * a bug in one of the get_hw_state functions. Or someplace else > + * in our code, like the register restore mess on resume. Clamp > + * things to off as a safer default. */ > + list_for_each_entry(connector, > + &dev->mode_config.connector_list, > + base.head) { > + if (connector->encoder != encoder) > + continue; > + > + intel_connector_break_all_links(connector); > + } > + } > + /* Enabled encoders without active connectors will be fixed in > + * the crtc fixup. */ > +} > + > +/* Scan out the current hw modeset state, sanitizes it and maps it into the drm > + * and i915 state tracking structures. */ > +void intel_modeset_setup_hw_state(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv = dev->dev_private; > + enum pipe pipe; > + u32 tmp; > + struct intel_crtc *crtc; > + struct intel_encoder *encoder; > + struct intel_connector *connector; > + > + for_each_pipe(pipe) { > + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > + > + tmp = I915_READ(PIPECONF(pipe)); > + if (tmp & PIPECONF_ENABLE) > + crtc->active = true; > + else > + crtc->active = false; > + > + crtc->base.enabled = crtc->active; > + > + DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n", > + crtc->base.base.id, > + crtc->active ? "enabled" : "disabled"); > + } > + > + list_for_each_entry(encoder, &dev->mode_config.encoder_list, > + base.head) { > + pipe = 0; > + > + if (encoder->get_hw_state(encoder, &pipe)) { > + encoder->base.crtc = > + dev_priv->pipe_to_crtc_mapping[pipe]; > + } else { > + encoder->base.crtc = NULL; > + } > + > + encoder->connectors_active = false; > + DRM_DEBUG_KMS("[ENCODER:%d:%s] hw state readout: %s, pipe=%i\n", > + encoder->base.base.id, > + drm_get_encoder_name(&encoder->base), > + encoder->base.crtc ? "enabled" : "disabled", > + pipe); > + } > + > + list_for_each_entry(connector, &dev->mode_config.connector_list, > + base.head) { > + if (connector->get_hw_state(connector)) { > + connector->base.dpms = DRM_MODE_DPMS_ON; > + connector->encoder->connectors_active = true; > + connector->base.encoder = &connector->encoder->base; > + } else { > + connector->base.dpms = DRM_MODE_DPMS_OFF; > + connector->base.encoder = NULL; > + } > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] hw state readout: %s\n", > + connector->base.base.id, > + drm_get_connector_name(&connector->base), > + connector->base.encoder ? "enabled" : "disabled"); > + } > + > + /* HW state is read out, now we need to sanitize this mess. */ > + list_for_each_entry(encoder, &dev->mode_config.encoder_list, > + base.head) { > + intel_sanitize_encoder(encoder); > + } > + > + for_each_pipe(pipe) { > + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > + intel_sanitize_crtc(crtc); > + } > +} I was going to suggest calling this fetch_hw_state or something, but it really does do some sanitizing, so maybe setup_hw_state is ok... Reviewed-by: Jesse Barnes -- Jesse Barnes, Intel Open Source Technology Center