Re: [PATCH 1/5] drm/i915: add "enable" argument to intel_update_sprite_watermarks

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, May 24, 2013 at 11:59:17AM -0300, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Because we want to call it from the "sprite disable" paths, since on
> Haswell we need to update the sprite watermarks when we disable
> sprites.
> 
> For now, all this patch does is to add the "enable" argument and call
> intel_update_sprite_watermarks from inside ivb_disable_plane. This
> shouldn't change how the code behaves because on
> sandybridge_update_sprite_wm we just ignore the "!enable" case. The
> patches that implement Haswell watermarks will make use of the changes
> introduced by this patch.
> 
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Looks all right.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h     |  3 ++-
>  drivers/gpu/drm/i915/intel_drv.h    |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c     | 11 ++++++++---
>  drivers/gpu/drm/i915/intel_sprite.c |  8 +++++---
>  4 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7772bb6..e38f8d3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -315,7 +315,8 @@ struct drm_i915_display_funcs {
>  	int (*get_fifo_size)(struct drm_device *dev, int plane);
>  	void (*update_wm)(struct drm_device *dev);
>  	void (*update_sprite_wm)(struct drm_device *dev, int pipe,
> -				 uint32_t sprite_width, int pixel_size);
> +				 uint32_t sprite_width, int pixel_size,
> +				 bool enable);
>  	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_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 75a7f22..21427aa 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -731,7 +731,7 @@ extern void intel_ddi_init(struct drm_device *dev, enum port port);
>  extern void intel_update_watermarks(struct drm_device *dev);
>  extern void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
>  					   uint32_t sprite_width,
> -					   int pixel_size);
> +					   int pixel_size, bool enable);
>  
>  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 e198f38..3ebb8e9 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2195,7 +2195,8 @@ sandybridge_compute_sprite_srwm(struct drm_device *dev, int plane,
>  }
>  
>  static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
> -					 uint32_t sprite_width, int pixel_size)
> +					 uint32_t sprite_width, int pixel_size,
> +					 bool enable)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	int latency = SNB_READ_WM0_LATENCY() * 100;	/* In unit 0.1us */
> @@ -2203,6 +2204,9 @@ static void sandybridge_update_sprite_wm(struct drm_device *dev, int pipe,
>  	int sprite_wm, reg;
>  	int ret;
>  
> +	if (!enable)
> +		return;
> +
>  	switch (pipe) {
>  	case 0:
>  		reg = WM0_PIPEA_ILK;
> @@ -2314,13 +2318,14 @@ void intel_update_watermarks(struct drm_device *dev)
>  }
>  
>  void intel_update_sprite_watermarks(struct drm_device *dev, int pipe,
> -				    uint32_t sprite_width, int pixel_size)
> +				    uint32_t sprite_width, int pixel_size,
> +				    bool enable)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
>  	if (dev_priv->display.update_sprite_wm)
>  		dev_priv->display.update_sprite_wm(dev, pipe, sprite_width,
> -						   pixel_size);
> +						   pixel_size, enable);
>  }
>  
>  static struct drm_i915_gem_object *
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 19b9cb9..04d38d4 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -114,7 +114,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_framebuffer *fb,
>  	crtc_w--;
>  	crtc_h--;
>  
> -	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size);
> +	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
>  
>  	I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
>  	I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
> @@ -268,7 +268,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  	crtc_w--;
>  	crtc_h--;
>  
> -	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size);
> +	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
>  
>  	/*
>  	 * IVB workaround: must disable low power watermarks for at least
> @@ -335,6 +335,8 @@ ivb_disable_plane(struct drm_plane *plane)
>  
>  	dev_priv->sprite_scaling_enabled &= ~(1 << pipe);
>  
> +	intel_update_sprite_watermarks(dev, pipe, 0, 0, false);
> +
>  	/* potentially re-enable LP watermarks */
>  	if (scaling_was_enabled && !dev_priv->sprite_scaling_enabled)
>  		intel_update_watermarks(dev);
> @@ -453,7 +455,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
>  	crtc_w--;
>  	crtc_h--;
>  
> -	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size);
> +	intel_update_sprite_watermarks(dev, pipe, crtc_w, pixel_size, true);
>  
>  	dvsscale = 0;
>  	if (IS_GEN5(dev) || crtc_w != src_w || crtc_h != src_h)
> -- 
> 1.8.1.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC