From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/4] drm/i915: add intel_dp_power_get/put Date: Wed, 23 Apr 2014 12:45:04 +0300 Message-ID: <20140423094504.GF18465@intel.com> References: <20140411132123.GH9262@phenom.ffwll.local> <1398207345-1865-1-git-send-email-przanoni@gmail.com> <1398207345-1865-2-git-send-email-przanoni@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id BC5C76E641 for ; Wed, 23 Apr 2014 02:45:09 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1398207345-1865-2-git-send-email-przanoni@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Paulo Zanoni Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Tue, Apr 22, 2014 at 07:55:43PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni > = > This was suggested by Chris on his review to the first version of > "drm/i915: get power domain in case the BIOS enabled eDP VDD". Well, > at least that's what I understood from his comment :) > = > The downside of this patch is that in a few places we will call > intel_display_port_power_domain() twice instead of once. > = > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/intel_dp.c | 61 ++++++++++++++++++++---------------= ------ > 1 file changed, 29 insertions(+), 32 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel= _dp.c > index 44df493..b385b03 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1063,13 +1063,30 @@ static u32 ironlake_get_pp_control(struct intel_= dp *intel_dp) > return control; > } > = > +static void intel_dp_power_get(struct intel_encoder *encoder) > +{ > + struct drm_i915_private *dev_priv =3D encoder->base.dev->dev_private; > + enum intel_display_power_domain power_domain; > + > + power_domain =3D intel_display_port_power_domain(encoder); > + intel_display_power_get(dev_priv, power_domain); > +} > + > +static void intel_dp_power_put(struct intel_encoder *encoder) > +{ > + struct drm_i915_private *dev_priv =3D encoder->base.dev->dev_private; > + enum intel_display_power_domain power_domain; > + > + power_domain =3D intel_display_port_power_domain(encoder); > + intel_display_power_put(dev_priv, power_domain); > +} There's nothing DP specific about these functions, so they could be moved somewhere else and used for other output types as well. > + > static bool _edp_panel_vdd_on(struct intel_dp *intel_dp) > { > struct drm_device *dev =3D intel_dp_to_dev(intel_dp); > struct intel_digital_port *intel_dig_port =3D dp_to_dig_port(intel_dp); > struct intel_encoder *intel_encoder =3D &intel_dig_port->base; > struct drm_i915_private *dev_priv =3D dev->dev_private; > - enum intel_display_power_domain power_domain; > u32 pp; > u32 pp_stat_reg, pp_ctrl_reg; > bool need_to_disable =3D !intel_dp->want_panel_vdd; > @@ -1082,8 +1099,7 @@ static bool _edp_panel_vdd_on(struct intel_dp *inte= l_dp) > if (edp_have_panel_vdd(intel_dp)) > return need_to_disable; > = > - power_domain =3D intel_display_port_power_domain(intel_encoder); > - intel_display_power_get(dev_priv, power_domain); > + intel_dp_power_get(intel_encoder); > = > DRM_DEBUG_KMS("Turning eDP VDD on\n"); > = > @@ -1133,7 +1149,6 @@ static void edp_panel_vdd_off_sync(struct intel_dp = *intel_dp) > struct intel_digital_port *intel_dig_port =3D > dp_to_dig_port(intel_dp); > struct intel_encoder *intel_encoder =3D &intel_dig_port->base; > - enum intel_display_power_domain power_domain; > = > DRM_DEBUG_KMS("Turning eDP VDD off\n"); > = > @@ -1153,8 +1168,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp = *intel_dp) > if ((pp & POWER_TARGET_ON) =3D=3D 0) > intel_dp->last_power_cycle =3D jiffies; > = > - power_domain =3D intel_display_port_power_domain(intel_encoder); > - intel_display_power_put(dev_priv, power_domain); > + intel_dp_power_put(intel_encoder); > } > } > = > @@ -1242,7 +1256,6 @@ void intel_edp_panel_off(struct intel_dp *intel_dp) > struct intel_encoder *intel_encoder =3D &intel_dig_port->base; > struct drm_device *dev =3D intel_dp_to_dev(intel_dp); > struct drm_i915_private *dev_priv =3D dev->dev_private; > - enum intel_display_power_domain power_domain; > u32 pp; > u32 pp_ctrl_reg; > = > @@ -1272,8 +1285,7 @@ void intel_edp_panel_off(struct intel_dp *intel_dp) > wait_panel_off(intel_dp); > = > /* We got a reference when we enabled the VDD. */ > - power_domain =3D intel_display_port_power_domain(intel_encoder); > - intel_display_power_put(dev_priv, power_domain); > + intel_dp_power_put(intel_encoder); > } > = > void intel_edp_backlight_on(struct intel_dp *intel_dp) > @@ -3161,13 +3173,10 @@ intel_dp_detect(struct drm_connector *connector, = bool force) > struct drm_device *dev =3D connector->dev; > struct drm_i915_private *dev_priv =3D dev->dev_private; > enum drm_connector_status status; > - enum intel_display_power_domain power_domain; > struct edid *edid =3D NULL; > = > intel_runtime_pm_get(dev_priv); > - > - power_domain =3D intel_display_port_power_domain(intel_encoder); > - intel_display_power_get(dev_priv, power_domain); > + intel_dp_power_get(intel_encoder); > = > DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > connector->base.id, drm_get_connector_name(connector)); > @@ -3199,8 +3208,7 @@ intel_dp_detect(struct drm_connector *connector, bo= ol force) > status =3D connector_status_connected; > = > out: > - intel_display_power_put(dev_priv, power_domain); > - > + intel_dp_power_put(intel_encoder); > intel_runtime_pm_put(dev_priv); > = > return status; > @@ -3213,18 +3221,14 @@ static int intel_dp_get_modes(struct drm_connecto= r *connector) > struct intel_encoder *intel_encoder =3D &intel_dig_port->base; > struct intel_connector *intel_connector =3D to_intel_connector(connecto= r); > struct drm_device *dev =3D connector->dev; > - struct drm_i915_private *dev_priv =3D dev->dev_private; > - enum intel_display_power_domain power_domain; > int ret; > = > /* We should parse the EDID data and find out if it has an audio sink > */ > = > - power_domain =3D intel_display_port_power_domain(intel_encoder); > - intel_display_power_get(dev_priv, power_domain); > - > + intel_dp_power_get(intel_encoder); > ret =3D intel_dp_get_edid_modes(connector, &intel_dp->aux.ddc); > - intel_display_power_put(dev_priv, power_domain); > + intel_dp_power_put(intel_encoder); > if (ret) > return ret; > = > @@ -3247,14 +3251,10 @@ intel_dp_detect_audio(struct drm_connector *conne= ctor) > struct intel_dp *intel_dp =3D intel_attached_dp(connector); > struct intel_digital_port *intel_dig_port =3D dp_to_dig_port(intel_dp); > struct intel_encoder *intel_encoder =3D &intel_dig_port->base; > - struct drm_device *dev =3D connector->dev; > - struct drm_i915_private *dev_priv =3D dev->dev_private; > - enum intel_display_power_domain power_domain; > struct edid *edid; > bool has_audio =3D false; > = > - power_domain =3D intel_display_port_power_domain(intel_encoder); > - intel_display_power_get(dev_priv, power_domain); > + intel_dp_power_get(intel_encoder); > = > edid =3D intel_dp_get_edid(connector, &intel_dp->aux.ddc); > if (edid) { > @@ -3262,7 +3262,7 @@ intel_dp_detect_audio(struct drm_connector *connect= or) > kfree(edid); > } > = > - intel_display_power_put(dev_priv, power_domain); > + intel_dp_power_put(intel_encoder); > = > return has_audio; > } > @@ -3801,11 +3801,8 @@ static bool intel_edp_init_connector(struct intel_= dp *intel_dp, > = > /* The VDD bit needs a power domain reference, so if the bit is already > * enabled when we boot, grab this reference. */ > - if (edp_have_panel_vdd(intel_dp)) { > - enum intel_display_power_domain power_domain; > - power_domain =3D intel_display_port_power_domain(intel_encoder); > - intel_display_power_get(dev_priv, power_domain); > - } > + if (edp_have_panel_vdd(intel_dp)) > + intel_dp_power_get(intel_encoder); > = > /* Cache DPCD and EDID for edp. */ > intel_edp_panel_vdd_on(intel_dp); > -- = > 1.9.0 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC