From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesse Barnes Subject: Re: [PATCH 09/76] drm/i915: add crtc->enable/disable vfuncs insted of dpms Date: Wed, 29 Aug 2012 10:49:02 -0700 Message-ID: <20120829104902.73f6f030@jbarnes-x220> References: <1343328581-2324-1-git-send-email-daniel.vetter@ffwll.ch> <1343328581-2324-10-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from oproxy6-pub.bluehost.com (oproxy6-pub.bluehost.com [67.222.54.6]) by gabe.freedesktop.org (Postfix) with SMTP id 3EE4D9E7D1 for ; Wed, 29 Aug 2012 10:49:05 -0700 (PDT) In-Reply-To: <1343328581-2324-10-git-send-email-daniel.vetter@ffwll.ch> 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: Daniel Vetter Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Thu, 26 Jul 2012 20:48:34 +0200 Daniel Vetter wrote: > Because that's what we're essentially calling. This is the first step > in untangling the crtc_helper induced dpms handling mess we have - at > the crtc level we only have 2 states and the magic is just in > selecting which one (and atm there isn't even much magic, but on > recent platforms where not even the crt output has more than 2 states > we could do better). > > Signed-Off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/i915_drv.h | 3 +- > drivers/gpu/drm/i915/intel_display.c | 62 > ++++++++++------------------------ 2 files changed, 20 insertions(+), > 45 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > b/drivers/gpu/drm/i915/i915_drv.h index 1258a74..c81c6e8 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -240,7 +240,6 @@ struct drm_i915_error_state { > }; > > struct drm_i915_display_funcs { > - void (*dpms)(struct drm_crtc *crtc, int mode); > bool (*fbc_enabled)(struct drm_device *dev); > void (*enable_fbc)(struct drm_crtc *crtc, unsigned long > interval); void (*disable_fbc)(struct drm_device *dev); > @@ -256,6 +255,8 @@ struct drm_i915_display_funcs { > struct drm_display_mode *adjusted_mode, > int x, int y, > struct drm_framebuffer *old_fb); > + void (*crtc_enable)(struct drm_crtc *crtc); > + void (*crtc_disable)(struct drm_crtc *crtc); > void (*off)(struct drm_crtc *crtc); > void (*write_eld)(struct drm_connector *connector, > struct drm_crtc *crtc); > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c index 1d5a811..17eb87b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3341,30 +3341,6 @@ static void ironlake_crtc_disable(struct > drm_crtc *crtc) mutex_unlock(&dev->struct_mutex); > } > > -static void ironlake_crtc_dpms(struct drm_crtc *crtc, int mode) > -{ > - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - int pipe = intel_crtc->pipe; > - int plane = intel_crtc->plane; > - > - /* XXX: When our outputs are all unaware of DPMS modes other > than off > - * and on, we should map those modes to DRM_MODE_DPMS_OFF in > the CRTC. > - */ > - switch (mode) { > - case DRM_MODE_DPMS_ON: > - case DRM_MODE_DPMS_STANDBY: > - case DRM_MODE_DPMS_SUSPEND: > - DRM_DEBUG_KMS("crtc %d/%d dpms on\n", pipe, plane); > - ironlake_crtc_enable(crtc); > - break; > - > - case DRM_MODE_DPMS_OFF: > - DRM_DEBUG_KMS("crtc %d/%d dpms off\n", pipe, plane); > - ironlake_crtc_disable(crtc); > - break; > - } > -} > - > static void ironlake_crtc_off(struct drm_crtc *crtc) > { > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > @@ -3444,23 +3420,6 @@ static void i9xx_crtc_disable(struct drm_crtc > *crtc) intel_update_watermarks(dev); > } > > -static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode) > -{ > - /* XXX: When our outputs are all unaware of DPMS modes other > than off > - * and on, we should map those modes to DRM_MODE_DPMS_OFF in > the CRTC. > - */ > - switch (mode) { > - case DRM_MODE_DPMS_ON: > - case DRM_MODE_DPMS_STANDBY: > - case DRM_MODE_DPMS_SUSPEND: > - i9xx_crtc_enable(crtc); > - break; > - case DRM_MODE_DPMS_OFF: > - i9xx_crtc_disable(crtc); > - break; > - } > -} > - > static void i9xx_crtc_off(struct drm_crtc *crtc) > { > } > @@ -3482,7 +3441,20 @@ static void intel_crtc_dpms(struct drm_crtc > *crtc, int mode) > intel_crtc->dpms_mode = mode; > > - dev_priv->display.dpms(crtc, mode); > + /* XXX: When our outputs are all unaware of DPMS modes other > than off > + * and on, we should map those modes to DRM_MODE_DPMS_OFF in > the CRTC. > + */ > + switch (mode) { > + case DRM_MODE_DPMS_ON: > + case DRM_MODE_DPMS_STANDBY: > + case DRM_MODE_DPMS_SUSPEND: > + dev_priv->display.crtc_enable(crtc); > + break; > + > + case DRM_MODE_DPMS_OFF: > + dev_priv->display.crtc_disable(crtc); > + break; > + } > > if (!dev->primary->master) > return; > @@ -6898,13 +6870,15 @@ static void intel_init_display(struct > drm_device *dev) > /* We always want a DPMS function */ > if (HAS_PCH_SPLIT(dev)) { > - dev_priv->display.dpms = ironlake_crtc_dpms; > dev_priv->display.crtc_mode_set = > ironlake_crtc_mode_set; > + dev_priv->display.crtc_enable = ironlake_crtc_enable; > + dev_priv->display.crtc_disable = > ironlake_crtc_disable; dev_priv->display.off = ironlake_crtc_off; > dev_priv->display.update_plane = > ironlake_update_plane; } else { > - dev_priv->display.dpms = i9xx_crtc_dpms; > dev_priv->display.crtc_mode_set = i9xx_crtc_mode_set; > + dev_priv->display.crtc_enable = i9xx_crtc_enable; > + dev_priv->display.crtc_disable = i9xx_crtc_disable; > dev_priv->display.off = i9xx_crtc_off; > dev_priv->display.update_plane = i9xx_update_plane; > } I like it. Reviewed-by: Jesse Barnes