From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 2/4] drm/i915: IBX+ doesn't have separate vsync/hsync controls on the VGA DAC Date: Wed, 11 Apr 2012 20:36:31 +0200 Message-ID: <20120411183631.GP4296@phenom.ffwll.local> References: <1334161416-11775-1-git-send-email-jbarnes@virtuousgeek.org> <1334161416-11775-2-git-send-email-jbarnes@virtuousgeek.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wg0-f43.google.com (mail-wg0-f43.google.com [74.125.82.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 000769E819 for ; Wed, 11 Apr 2012 11:35:39 -0700 (PDT) Received: by wgbdr12 with SMTP id dr12so958058wgb.12 for ; Wed, 11 Apr 2012 11:35:39 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1334161416-11775-2-git-send-email-jbarnes@virtuousgeek.org> 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: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Apr 11, 2012 at 09:23:34AM -0700, Jesse Barnes wrote: > When the PCH split occurred, we dropped support for separate hsync and > vsync disable in the VGA DAC. So add a PCH specific DPMS function that > just uses the port enable bit for controlling DPMS states. > > Before this fix, when anything other than a full DPMS off occurred, > the VGA port would be left enabled and scanning out while all the other > heads would turn off as expected. > > Signed-off-by: Jesse Barnes > --- > drivers/gpu/drm/i915/intel_crt.c | 42 ++++++++++++++++++++++++++++--------- > 1 files changed, 32 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c > index 70b0f1a..bdaefff 100644 > --- a/drivers/gpu/drm/i915/intel_crt.c > +++ b/drivers/gpu/drm/i915/intel_crt.c > @@ -55,18 +55,36 @@ static struct intel_crt *intel_attached_crt(struct drm_connector *connector) > struct intel_crt, base); > } > > -static void intel_crt_dpms(struct drm_encoder *encoder, int mode) > +static void pch_crt_dpms(struct drm_encoder *encoder, int mode) > { > struct drm_device *dev = encoder->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - u32 temp, reg; > + u32 temp; > > - if (HAS_PCH_SPLIT(dev)) > - reg = PCH_ADPA; > - else > - reg = ADPA; > + temp = I915_READ(PCH_ADPA); > + temp &= ~ADPA_DAC_ENABLE; > + > + switch (mode) { > + case DRM_MODE_DPMS_ON: > + temp |= ADPA_DAC_ENABLE; > + break; > + case DRM_MODE_DPMS_STANDBY: > + case DRM_MODE_DPMS_SUSPEND: > + case DRM_MODE_DPMS_OFF: > + /* Just leave port enable cleared */ > + break; > + } > + > + I915_WRITE(PCH_ADPA, temp); > +} > + > +static void intel_crt_dpms(struct drm_encoder *encoder, int mode) > +{ > + struct drm_device *dev = encoder->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + u32 temp; > > - temp = I915_READ(reg); > + temp = I915_READ(ADPA); > temp &= ~(ADPA_HSYNC_CNTL_DISABLE | ADPA_VSYNC_CNTL_DISABLE); > temp &= ~ADPA_DAC_ENABLE; > > @@ -85,7 +103,7 @@ static void intel_crt_dpms(struct drm_encoder *encoder, int mode) > break; > } > > - I915_WRITE(reg, temp); > + I915_WRITE(ADPA, temp); > } > > static int intel_crt_mode_valid(struct drm_connector *connector, > @@ -516,8 +534,7 @@ static void intel_crt_reset(struct drm_connector *connector) > * Routines for controlling stuff on the analog port > */ > > -static const struct drm_encoder_helper_funcs intel_crt_helper_funcs = { > - .dpms = intel_crt_dpms, > +static struct drm_encoder_helper_funcs intel_crt_helper_funcs = { > .mode_fixup = intel_crt_mode_fixup, > .prepare = intel_encoder_prepare, > .commit = intel_encoder_commit, > @@ -602,6 +619,11 @@ void intel_crt_init(struct drm_device *dev) > connector->interlace_allowed = 1; > connector->doublescan_allowed = 0; > > + if (HAS_PCH_SPLIT(dev)) > + intel_crt_helper_funcs.dpms = pch_crt_dpms; > + else > + intel_crt_helper_funcs.dpms = intel_crt_dpms; I like the clean split of this, but the static struct frobbing really itches me. Can you either split up intel_crt_helper_funcs into a gmch and pch_split version or add a dpms function pointer to struct intel_crt? Patch 1&3 of this series are queued for -next, thanks. -Daniel > + > drm_encoder_helper_add(&crt->base.base, &intel_crt_helper_funcs); > drm_connector_helper_add(connector, &intel_crt_connector_helper_funcs); > > -- > 1.7.4.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48