From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [RFC PATCH 1/3] drm/i915: move backlight enable later in vlv enable sequence Date: Wed, 4 Sep 2013 21:30:48 +0300 Message-ID: <20130904183047.GT11428@intel.com> References: <4dc01095562b360832a9b33429862199dd4e73d3.1378208439.git.jani.nikula@intel.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 7E705E7095 for ; Wed, 4 Sep 2013 11:30:51 -0700 (PDT) Content-Disposition: inline In-Reply-To: <4dc01095562b360832a9b33429862199dd4e73d3.1378208439.git.jani.nikula@intel.com> 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: Jani Nikula Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Sep 03, 2013 at 02:43:37PM +0300, Jani Nikula wrote: > Follow-up to > commit 5004945f1d6c0282c0288afa89ad85d7f2bea4d5 > Author: Jani Nikula > Date: Tue Jul 30 12:20:32 2013 +0300 > = > drm/i915: move encoder->enable callback later in VLV crtc enable > = > Reference: http://mid.gmane.org/CAKMK7uFs9EMvMW8BnS24e5UNm1D7JrfVg3SD5SDF= tVEamGfOOg@mail.gmail.com > Signed-off-by: Jani Nikula > --- > drivers/gpu/drm/i915/intel_dp.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > = > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel= _dp.c > index c192dbb..adbe7bc 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1717,11 +1717,17 @@ static void intel_enable_dp(struct intel_encoder = *encoder) > ironlake_edp_panel_vdd_off(intel_dp, true); > intel_dp_complete_link_train(intel_dp); > intel_dp_stop_link_train(intel_dp); > - ironlake_edp_backlight_on(intel_dp); > + > + /* this function is called from vlv_pre_enable_dp() */ > + if (!IS_VALLEYVIEW(dev)) > + ironlake_edp_backlight_on(intel_dp); > } > = > static void vlv_enable_dp(struct intel_encoder *encoder) > { > + struct intel_dp *intel_dp =3D enc_to_intel_dp(&encoder->base); > + > + ironlake_edp_backlight_on(intel_dp); > } This feels a bit off to me since we've already started to use separate functions for VLV vs. others. So how about something a bit more in line with that design: intel_enable_dp() { ... - ironlake_edp_backlight_on() } = + g4x_enable_dp() { + intel_enable_dp() + ironlake_edp_backlight_on() + } = vlv_enable_dp() { + ironlake_edp_backlight_on() } ... - intel_encoder->enable =3D intel_enable_dp; + intel_encoder->enable =3D g4x_enable_dp; And also perhaps s/intel_pre_enable_dp/g4x_pre_enable_dp/ and s/intel_dp_pre_pll_enable/vlv_dp_pre_pll_enable/ just to keep the naming somewhat predictable. Also we could kill the !IS_VLV check from intel_dp_pre_pll_enable() since we don't plug that guy in except on VLV. > = > static void intel_pre_enable_dp(struct intel_encoder *encoder) > -- = > 1.7.9.5 -- = Ville Syrj=E4l=E4 Intel OTC