From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 1/2] drm/i915: factor out intel_edp_panel_vdd_sanitize Date: Tue, 29 Jul 2014 17:44:27 +0300 Message-ID: <20140729144427.GY27580@intel.com> References: <1403894114-28007-1-git-send-email-imre.deak@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id C7B836E3AC for ; Tue, 29 Jul 2014 07:45:12 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1403894114-28007-1-git-send-email-imre.deak@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Imre Deak Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Jun 27, 2014 at 09:35:13PM +0300, Imre Deak wrote: > This will be needed by an upcoming patch too that needs to sanitize the > VDD state during resume. The additional async disabling is only needed > for the resume path, here it doesn't make a difference since we enable > VDD right after the sanitize call. > = > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/intel_dp.c | 59 ++++++++++++++++++++++++++++------= ------ > drivers/gpu/drm/i915/intel_drv.h | 1 + > 2 files changed, 43 insertions(+), 17 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel= _dp.c > index b5ec489..a40d914 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1234,6 +1234,19 @@ static void edp_panel_vdd_work(struct work_struct = *__work) > drm_modeset_unlock(&dev->mode_config.connection_mutex); > } > = > +static void edp_panel_vdd_schedule_off(struct intel_dp *intel_dp) > +{ > + unsigned long delay; > + > + /* > + * Queue the timer to fire a long time from now (relative to the power > + * down delay) to keep the panel power up across a sequence of > + * operations. > + */ > + delay =3D msecs_to_jiffies(intel_dp->panel_power_cycle_delay * 5); > + schedule_delayed_work(&intel_dp->panel_vdd_work, delay); > +} > + > static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync) > { > if (!is_edp(intel_dp)) > @@ -1243,17 +1256,10 @@ static void edp_panel_vdd_off(struct intel_dp *in= tel_dp, bool sync) > = > intel_dp->want_panel_vdd =3D false; > = > - if (sync) { > + if (sync) > edp_panel_vdd_off_sync(intel_dp); > - } else { > - /* > - * Queue the timer to fire a long > - * time from now (relative to the power down delay) > - * to keep the panel power up across a sequence of operations > - */ > - schedule_delayed_work(&intel_dp->panel_vdd_work, > - msecs_to_jiffies(intel_dp->panel_power_cycle_delay * 5)); > - } > + else > + edp_panel_vdd_schedule_off(intel_dp); > } > = > void intel_edp_panel_on(struct intel_dp *intel_dp) > @@ -4179,6 +4185,31 @@ intel_dp_drrs_init(struct intel_digital_port *inte= l_dig_port, > return downclock_mode; > } > = > +void intel_edp_panel_vdd_sanitize(struct intel_encoder *intel_encoder) > +{ > + struct intel_dp *intel_dp =3D enc_to_intel_dp(&intel_encoder->base); > + struct drm_device *dev =3D intel_encoder->base.dev; > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + enum intel_display_power_domain power_domain; > + > + if (intel_encoder->type !=3D INTEL_OUTPUT_EDP) > + return; I'd move the intel_dp assignment after the encoder type check to avoid anyone thinking that they're allowed to dereference it before we're sure about the type. Apart from that both patches are: Reviewed-by: Ville Syrj=E4l=E4 > + > + if (!edp_have_panel_vdd(intel_dp)) > + return; > + /* > + * The VDD bit needs a power domain reference, so if the bit is > + * already enabled when we boot or resume, grab this reference and > + * schedule a vdd off, so we don't hold on to the reference > + * indefinitely. > + */ > + DRM_DEBUG_KMS("VDD left on by BIOS, adjusting state tracking\n"); > + power_domain =3D intel_display_port_power_domain(intel_encoder); > + intel_display_power_get(dev_priv, power_domain); > + > + edp_panel_vdd_schedule_off(intel_dp); > +} > + > static bool intel_edp_init_connector(struct intel_dp *intel_dp, > struct intel_connector *intel_connector, > struct edp_power_seq *power_seq) > @@ -4199,13 +4230,7 @@ static bool intel_edp_init_connector(struct intel_= dp *intel_dp, > if (!is_edp(intel_dp)) > return true; > = > - /* 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); > - } > + intel_edp_panel_vdd_sanitize(intel_encoder); > = > /* Cache DPCD and EDID for edp. */ > intel_edp_panel_vdd_on(intel_dp); > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/inte= l_drv.h > index 5f7c7bd..150c09b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -855,6 +855,7 @@ bool intel_dp_is_edp(struct drm_device *dev, enum por= t port); > void intel_edp_backlight_on(struct intel_dp *intel_dp); > void intel_edp_backlight_off(struct intel_dp *intel_dp); > void intel_edp_panel_vdd_on(struct intel_dp *intel_dp); > +void intel_edp_panel_vdd_sanitize(struct intel_encoder *intel_encoder); > void intel_edp_panel_on(struct intel_dp *intel_dp); > void intel_edp_panel_off(struct intel_dp *intel_dp); > void intel_edp_psr_enable(struct intel_dp *intel_dp); > -- = > 1.8.4 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC