From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/9] drm/i915: remove intel_update_linetime_watermarks Date: Mon, 20 May 2013 16:48:03 +0300 Message-ID: <20130520134803.GG14974@intel.com> References: <20130506134320.GA1985@cantiga.alporthouse.com> <1368129350-15295-1-git-send-email-przanoni@gmail.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 AFABDE5C87 for ; Mon, 20 May 2013 06:48:07 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1368129350-15295-1-git-send-email-przanoni@gmail.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: Paulo Zanoni Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Thu, May 09, 2013 at 04:55:50PM -0300, Paulo Zanoni wrote: > From: Paulo Zanoni > = > The spec says the linetime watermarks must be programmed before > enabling any display low power watermarks, but we're currently > updating the linetime watermarks after we call intel_update_watermarks > (and only at crtc_mode_set, not at crtc_{enable,disable}). So IMHO the > best way guarantee the linetime watermarks will be updated before the > low power watermarks is inside the update_wm function, because it's > the function that enables low power watermarks. And since Haswell is > the only platform that has linetime watermarks, let's completely kill > the "intel_update_linetime_watermarks" abstraction and just use the > intel_update_watermarks abstraction by creating haswell_update_wm. > = > For now haswell_update_wm is still calling sandybridge_update_wm, but > in the future I plan to implement a function specific to Haswell. > = > v2: - Rename patch > - Disable LP watermarks before changing linetime WMs (Chris) > - Add a comment explaining that this is just temporary code. > = > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/i915_drv.h | 2 -- > drivers/gpu/drm/i915/intel_display.c | 2 -- > drivers/gpu/drm/i915/intel_drv.h | 2 -- > drivers/gpu/drm/i915/intel_pm.c | 43 ++++++++++++++++++++++++----= ------ > 4 files changed, 30 insertions(+), 19 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index c81100c..8606103 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -316,8 +316,6 @@ struct drm_i915_display_funcs { > void (*update_wm)(struct drm_device *dev); > void (*update_sprite_wm)(struct drm_device *dev, int pipe, > uint32_t sprite_width, int pixel_size); > - void (*update_linetime_wm)(struct drm_device *dev, int pipe, > - struct drm_display_mode *mode); > void (*modeset_global_resources)(struct drm_device *dev); > /* Returns the active state of the crtc, and if the crtc is active, > * fills out the pipe-config with the hw state. */ > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index b796752..7bcd60c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6026,8 +6026,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *c= rtc, > = > intel_update_watermarks(dev); > = > - intel_update_linetime_watermarks(dev, pipe, adjusted_mode); > - > return ret; > } > = > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/inte= l_drv.h > index be9ad39..80b417a 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -727,8 +727,6 @@ extern void intel_update_watermarks(struct drm_device= *dev); > extern void intel_update_sprite_watermarks(struct drm_device *dev, int p= ipe, > uint32_t sprite_width, > int pixel_size); > -extern void intel_update_linetime_watermarks(struct drm_device *dev, int= pipe, > - struct drm_display_mode *mode); > = > extern unsigned long intel_gen4_compute_page_offset(int *x, int *y, > unsigned int tiling_mode, > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel= _pm.c > index 93b8f70..236cd99 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -2069,12 +2069,19 @@ static void ivybridge_update_wm(struct drm_device= *dev) > } > = > static void > -haswell_update_linetime_wm(struct drm_device *dev, int pipe, > - struct drm_display_mode *mode) > +haswell_update_linetime_wm(struct drm_device *dev, struct drm_crtc *crtc) > { > struct drm_i915_private *dev_priv =3D dev->dev_private; > + struct intel_crtc *intel_crtc =3D to_intel_crtc(crtc); > + enum pipe pipe =3D intel_crtc->pipe; > + struct drm_display_mode *mode =3D &intel_crtc->config.adjusted_mode; > u32 temp; > = > + if (!intel_crtc_active(crtc)) { > + I915_WRITE(PIPE_WM_LINETIME(pipe), 0); > + return; > + } > + > temp =3D I915_READ(PIPE_WM_LINETIME(pipe)); > temp &=3D ~PIPE_WM_LINETIME_MASK; > = > @@ -2095,6 +2102,26 @@ haswell_update_linetime_wm(struct drm_device *dev,= int pipe, > I915_WRITE(PIPE_WM_LINETIME(pipe), temp); > } > = > +static void haswell_update_wm(struct drm_device *dev) > +{ > + struct drm_i915_private *dev_priv =3D dev->dev_private; > + struct drm_crtc *crtc; > + enum pipe pipe; > + > + /* Disable the LP WMs before changine the linetime registers. This is > + * just a temporary code that will be replaced soon. */ > + I915_WRITE(WM3_LP_ILK, 0); > + I915_WRITE(WM2_LP_ILK, 0); > + I915_WRITE(WM1_LP_ILK, 0); > + > + for_each_pipe(pipe) { > + crtc =3D dev_priv->pipe_to_crtc_mapping[pipe]; list_for_each_entry(crtc, ...) Otherwise: Reviewed-by: Ville Syrj=E4l=E4 > + haswell_update_linetime_wm(dev, crtc); > + } > + > + sandybridge_update_wm(dev); > +} > + > static bool > sandybridge_compute_sprite_wm(struct drm_device *dev, int plane, > uint32_t sprite_width, int pixel_size, > @@ -2290,15 +2317,6 @@ void intel_update_watermarks(struct drm_device *de= v) > dev_priv->display.update_wm(dev); > } > = > -void intel_update_linetime_watermarks(struct drm_device *dev, > - int pipe, struct drm_display_mode *mode) > -{ > - struct drm_i915_private *dev_priv =3D dev->dev_private; > - > - if (dev_priv->display.update_linetime_wm) > - dev_priv->display.update_linetime_wm(dev, pipe, mode); > -} > - > void intel_update_sprite_watermarks(struct drm_device *dev, int pipe, > uint32_t sprite_width, int pixel_size) > { > @@ -4553,9 +4571,8 @@ void intel_init_pm(struct drm_device *dev) > dev_priv->display.init_clock_gating =3D ivybridge_init_clock_gating; > } else if (IS_HASWELL(dev)) { > if (SNB_READ_WM0_LATENCY()) { > - dev_priv->display.update_wm =3D sandybridge_update_wm; > + dev_priv->display.update_wm =3D haswell_update_wm; > dev_priv->display.update_sprite_wm =3D sandybridge_update_sprite_wm; > - dev_priv->display.update_linetime_wm =3D haswell_update_linetime_wm; > } else { > DRM_DEBUG_KMS("Failed to read display plane latency. " > "Disable CxSR\n"); > -- = > 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