Re: [PATCH 7/8] drm/i915/skl: Update watermarks for Y tiling

[Damien Lespiau <damien.lespiau@intel.com>]

On Wed, Feb 25, 2015 at 04:47:23PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> Display watermarks need different programming for different tiling
> modes.
> 
> Set the relevant flag so this happens during the plane commit and
> add relevant data into a structure made available to the watermark
> computation code.
> 
> v2: Pass in tiling info to sprite plane updates as well.
> v3: Rebased for plane handling changes.
> v4: Handle fb == NULL when plane is disabled.
> v5: Refactored for addfb2 interface.
> v6: Refactored for fb modifier changes.
> v7: Updated for atomic commit by only updating watermarks when tiling changes.
> v8: BSpec watermark calculation updates.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Acked-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Acked-by: Matt Roper <matthew.d.roper@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c |  6 ++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_pm.c      | 56 ++++++++++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_sprite.c  |  6 ++++
>  4 files changed, 60 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 626e6538..1d50934 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11994,6 +11994,12 @@ intel_check_primary_plane(struct drm_plane *plane,
>  			INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
>  
>  		intel_crtc->atomic.update_fbc = true;
> +
> +		/* Update watermarks on tiling changes. */
> +		if (!plane->state->fb || !state->base.fb ||
> +		    plane->state->fb->modifier[0] !=
> +		    state->base.fb->modifier[0])
> +			intel_crtc->atomic.update_wm = true;
>  	}
>  
>  	return 0;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 399d2b2..b124548 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -501,6 +501,7 @@ struct intel_plane_wm_parameters {
>  	uint8_t bytes_per_pixel;
>  	bool enabled;
>  	bool scaled;
> +	u64 tiling;
>  };
>  
>  struct intel_plane {
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 626c3c2..e0d6ebc 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2603,7 +2603,7 @@ static uint32_t skl_wm_method1(uint32_t pixel_rate, uint8_t bytes_per_pixel,
>  
>  static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
>  			       uint32_t horiz_pixels, uint8_t bytes_per_pixel,
> -			       uint32_t latency)
> +			       uint64_t tiling, uint32_t latency)
>  {

Hum, does this compile? I'm seeing an extra argument to skl_wm_method2()
but no update at the calling site?

>  	uint32_t ret;
>  	uint32_t plane_bytes_per_line, plane_blocks_per_line;
> @@ -2613,7 +2613,16 @@ static uint32_t skl_wm_method2(uint32_t pixel_rate, uint32_t pipe_htotal,
>  		return UINT_MAX;
>  
>  	plane_bytes_per_line = horiz_pixels * bytes_per_pixel;
> -	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
> +
> +	if (tiling == I915_FORMAT_MOD_Y_TILED ||
> +	    tiling == I915_FORMAT_MOD_Yf_TILED) {
> +		plane_bytes_per_line *= 4;
> +		plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
> +		plane_blocks_per_line /= 4;
> +	} else {
> +		plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
> +	}
> +
>  	wm_intermediate_val = latency * pixel_rate;
>  	ret = DIV_ROUND_UP(wm_intermediate_val, pipe_htotal * 1000) *
>  				plane_blocks_per_line;
> @@ -2665,6 +2674,7 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum pipe pipe = intel_crtc->pipe;
>  	struct drm_plane *plane;
> +	struct drm_framebuffer *fb;
>  	int i = 1; /* Index for sprite planes start */
>  
>  	p->active = intel_crtc_active(crtc);
> @@ -2680,6 +2690,14 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc,
>  			crtc->primary->fb->bits_per_pixel / 8;
>  		p->plane[0].horiz_pixels = intel_crtc->config->pipe_src_w;
>  		p->plane[0].vert_pixels = intel_crtc->config->pipe_src_h;
> +		p->plane[0].tiling = DRM_FORMAT_MOD_NONE;
> +		fb = crtc->primary->fb;
> +		/*
> +		 * Framebuffer can be NULL on plane disable, but it does not
> +		 * matter for watermarks if we assume no tiling in that case.
> +		 */
> +		if (fb)
> +			p->plane[0].tiling = fb->modifier[0];
>  
>  		p->cursor.enabled = true;
>  		p->cursor.bytes_per_pixel = 4;
> @@ -2709,6 +2727,7 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  	uint32_t plane_bytes_per_line, plane_blocks_per_line;
>  	uint32_t res_blocks, res_lines;
>  	uint32_t result_blocks;
> +	uint32_t y_tile_minimum;

The scope of the y_tile_minimum variable could be restricted further,
but well...

>  	if (latency == 0 || !p->active || !p_params->enabled)
>  		return false;
> @@ -2720,23 +2739,34 @@ static bool skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
>  				 p->pipe_htotal,
>  				 p_params->horiz_pixels,
>  				 p_params->bytes_per_pixel,
> +				 p_params->tiling,
>  				 latency);
>  
>  	plane_bytes_per_line = p_params->horiz_pixels *
>  					p_params->bytes_per_pixel;
>  	plane_blocks_per_line = DIV_ROUND_UP(plane_bytes_per_line, 512);
>  
> -	/* For now xtile and linear */
> -	if ((ddb_allocation / plane_blocks_per_line) >= 1)
> -		result_blocks = min(method1, method2);
> -	else
> -		result_blocks = method1;
> +	if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
> +	    p_params->tiling == I915_FORMAT_MOD_Yf_TILED) {
> +		y_tile_minimum = plane_blocks_per_line * 4;
> +		result_blocks = max(method2, y_tile_minimum);
> +	} else {
> +		if ((ddb_allocation / plane_blocks_per_line) >= 1)
> +			result_blocks = min(method1, method2);
> +		else
> +			result_blocks = method1;
> +	}
>  
>  	res_blocks = result_blocks + 1;
>  	res_lines = DIV_ROUND_UP(result_blocks, plane_blocks_per_line);
>  
> -	if (level >=1 && level <= 7)
> -		res_blocks++;
> +	if (level >=1 && level <= 7) {
> +		if (p_params->tiling == I915_FORMAT_MOD_Y_TILED ||
> +		    p_params->tiling == I915_FORMAT_MOD_Yf_TILED)
> +			res_lines += 4;
> +		else
> +			res_blocks++;
> +	}
>  
>  	if (res_blocks > ddb_allocation || res_lines > 31)
>  		return false;
> @@ -3165,12 +3195,20 @@ skl_update_sprite_wm(struct drm_plane *plane, struct drm_crtc *crtc,
>  		     int pixel_size, bool enabled, bool scaled)
>  {
>  	struct intel_plane *intel_plane = to_intel_plane(plane);
> +	struct drm_framebuffer *fb = plane->fb;
>  
>  	intel_plane->wm.enabled = enabled;
>  	intel_plane->wm.scaled = scaled;
>  	intel_plane->wm.horiz_pixels = sprite_width;
>  	intel_plane->wm.vert_pixels = sprite_height;
>  	intel_plane->wm.bytes_per_pixel = pixel_size;
> +	intel_plane->wm.tiling = DRM_FORMAT_MOD_NONE;
> +	/*
> +	 * Framebuffer can be NULL on plane disable, but it does not
> +	 * matter for watermarks if we assume no tiling in that case.
> +	 */
> +	if (fb)
> +		intel_plane->wm.tiling = fb->modifier[0];
>  
>  	skl_update_wm(crtc);
>  }
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index b7103bd..29ec206 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1256,6 +1256,12 @@ finish:
>  
>  		if (!intel_crtc->primary_enabled && !state->hides_primary)
>  			intel_crtc->atomic.post_enable_primary = true;
> +
> +		/* Update watermarks on tiling changes. */
> +		if (!plane->state->fb || !state->base.fb ||
> +		    plane->state->fb->modifier[0] !=
> +		    state->base.fb->modifier[0])
> +			intel_crtc->atomic.update_wm = true;
>  	}
>  
>  	return 0;
> -- 
> 2.3.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx