From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v3 3/3] drm/i915: move encoder->enable callback later in VLV crtc enable Date: Tue, 30 Jul 2013 17:24:16 +0300 Message-ID: <20130730142416.GS5004@intel.com> References: <81a654a82b03fc6eb4e7d4cc40199a662e60839d.1375175821.git.jani.nikula@intel.com> <3eb5007d000302144b9e77c25536feba3168f134.1375175822.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 mga03.intel.com (mga03.intel.com [143.182.124.21]) by gabe.freedesktop.org (Postfix) with ESMTP id 0BEC9E6CD9 for ; Tue, 30 Jul 2013 07:24:19 -0700 (PDT) Content-Disposition: inline In-Reply-To: <3eb5007d000302144b9e77c25536feba3168f134.1375175822.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, Jul 30, 2013 at 12:20:32PM +0300, Jani Nikula wrote: > VLV wants encoder enabling before the pipe is up. With the previously > rearranged VLV DP and HDMI ->pre_enable and ->enable callbacks in place, > this no longer depends on the early ->enable hook call. Move the > ->enable call at the end of the sequence, similar to the crtc enable on > other platforms. This will be needed e.g. for moving the eDP backlight > enabling to the right place in the sequence, currently done too early on > VLV. > = > There should be no functional changes. > = > v2: Rebase. > = > v3: Explain why this is needed in the commit message (Chris). > = > Signed-off-by: Jani Nikula Looks sane to me as well. For the series: Reviewed-by: Ville Syrj=E4l=E4 Note that I just lost my VLV board so I wasn't able to test this. I'm assuming there will eventually be some followon patch to move the backlight stuff to right place for VLV? Also should we make encoder->enable optional to avoid the stubs for VLV? And it might make sense to rename all the VLV specific dp/hdmi functions to vlv_foo instead of the intel_foo that they are called now. > --- > drivers/gpu/drm/i915/intel_display.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index 7d63d8d..c3c0bf1 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3663,10 +3663,6 @@ static void valleyview_crtc_enable(struct drm_crtc= *crtc) > if (encoder->pre_enable) > encoder->pre_enable(encoder); > = > - /* VLV wants encoder enabling _before_ the pipe is up. */ > - for_each_encoder_on_crtc(dev, crtc, encoder) > - encoder->enable(encoder); > - > i9xx_pfit_enable(intel_crtc); > = > intel_crtc_load_lut(crtc); > @@ -3677,6 +3673,9 @@ static void valleyview_crtc_enable(struct drm_crtc = *crtc) > intel_crtc_update_cursor(crtc, true); > = > intel_update_fbc(dev); > + > + for_each_encoder_on_crtc(dev, crtc, encoder) > + encoder->enable(encoder); > } > = > static void i9xx_crtc_enable(struct drm_crtc *crtc) > -- = > 1.7.10.4 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC