From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 09/19] drm/i915: check port power domain when reading the encoder hw state Date: Thu, 20 Feb 2014 11:36:20 -0800 Message-ID: <20140220113620.72cae64f@jbarnes-desktop> References: <1392674540-10915-1-git-send-email-imre.deak@intel.com> <1392674540-10915-10-git-send-email-imre.deak@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy17-pub.mail.unifiedlayer.com (oproxy17-pub.mail.unifiedlayer.com [74.220.201.171]) by gabe.freedesktop.org (Postfix) with SMTP id 2D1E8FB39F for ; Thu, 20 Feb 2014 11:36:09 -0800 (PST) In-Reply-To: <1392674540-10915-10-git-send-email-imre.deak@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Imre Deak Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, 18 Feb 2014 00:02:10 +0200 Imre Deak wrote: > Since the encoder is tied to its port, we need to make sure the power > domain for that port is on before reading out the encoder HW state. > > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/intel_ddi.c | 2 +- > drivers/gpu/drm/i915/intel_display.c | 23 ++++++++++++++++++----- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 3 files changed, 20 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c > index 2643d3b..f95bc3a 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -1110,7 +1110,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector) > enum transcoder cpu_transcoder; > uint32_t tmp; > > - if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) > + if (!intel_encoder_get_hw_state(intel_encoder, &pipe)) > return false; > > if (port == PORT_A) > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 7ef06fa..ce1c00a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4519,7 +4519,7 @@ static void intel_connector_check_state(struct intel_connector *connector) > WARN(!encoder->connectors_active, > "encoder->connectors_active not set\n"); > > - encoder_enabled = encoder->get_hw_state(encoder, &pipe); > + encoder_enabled = intel_encoder_get_hw_state(encoder, &pipe); > WARN(!encoder_enabled, "encoder not enabled\n"); > if (WARN_ON(!encoder->base.crtc)) > return; > @@ -4561,7 +4561,7 @@ bool intel_connector_get_hw_state(struct intel_connector *connector) > enum pipe pipe = 0; > struct intel_encoder *encoder = connector->encoder; > > - return encoder->get_hw_state(encoder, &pipe); > + return intel_encoder_get_hw_state(encoder, &pipe); > } > > static bool ironlake_check_fdi_lanes(struct drm_device *dev, enum pipe pipe, > @@ -9464,6 +9464,19 @@ check_connector_state(struct drm_device *dev) > } > } > > +bool intel_encoder_get_hw_state(struct intel_encoder *intel_encoder, > + enum pipe *pipe) > +{ > + enum intel_display_power_domain power_domain; > + struct drm_i915_private *dev_priv = intel_encoder->base.dev->dev_private; > + > + power_domain = intel_display_port_power_domain(intel_encoder); > + if (!intel_display_power_enabled(dev_priv, power_domain)) > + return false; > + > + return intel_encoder->get_hw_state(intel_encoder, pipe); > +} > + > static void > check_encoder_state(struct drm_device *dev) > { > @@ -9504,7 +9517,7 @@ check_encoder_state(struct drm_device *dev) > "encoder's computed active state doesn't match tracked active state " > "(expected %i, found %i)\n", active, encoder->connectors_active); > > - active = encoder->get_hw_state(encoder, &pipe); > + active = intel_encoder_get_hw_state(encoder, &pipe); > WARN(active != encoder->connectors_active, > "encoder's hw state doesn't match sw tracking " > "(expected %i, found %i)\n", > @@ -9571,7 +9584,7 @@ check_crtc_state(struct drm_device *dev) > enum pipe pipe; > if (encoder->base.crtc != &crtc->base) > continue; > - if (encoder->get_hw_state(encoder, &pipe)) > + if (intel_encoder_get_hw_state(encoder, &pipe)) > encoder->get_config(encoder, &pipe_config); > } > > @@ -11350,7 +11363,7 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > base.head) { > pipe = 0; > > - if (encoder->get_hw_state(encoder, &pipe)) { > + if (intel_encoder_get_hw_state(encoder, &pipe)) { > crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]); > encoder->base.crtc = &crtc->base; > encoder->get_config(encoder, &crtc->config); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index e31eb1e..afc01a4 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -738,6 +738,7 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder); > int valleyview_get_vco(struct drm_i915_private *dev_priv); > void intel_mode_from_pipe_config(struct drm_display_mode *mode, > struct intel_crtc_config *pipe_config); > +bool intel_encoder_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe); > > /* intel_dp.c */ > void intel_dp_init(struct drm_device *dev, int output_reg, enum port port); I think there are a few more places to switch over, e.g. connector_check_state and connector_get_hw_state? Maybe we should rename the fn pointer field to include a __ in the front so the compiler will catch them all and we'll know they're not to be called directly. -- Jesse Barnes, Intel Open Source Technology Center