Re: [PATCH v2] drm/i915/skl: Make sure to allocate mininum sizes in the DDB

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

On Mon, Feb 09, 2015 at 01:35:10PM +0000, Damien Lespiau wrote:
> I overlooked the fact that we need to allocate a minimum 8 blocks and
> that just allocating the planes depending on how much they need to fetch
> from the DDB in proportion of how much memory bw is necessary for the
> whole display can lead to cases where we don't respect those minima (and
> thus overrun).
> 
> So, instead, start by allocating 8 blocks to each active display plane
> and then allocate the remaining blocks like before.
> 
> v2: Rebase on top of -nightly
> 
> Cc: Mahesh Kumar <mahesh1.kumar@intel.com>
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index bebefe7..f6c7e53 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2502,6 +2502,7 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
>  	enum pipe pipe = intel_crtc->pipe;
>  	struct skl_ddb_entry *alloc = &ddb->pipe[pipe];
>  	uint16_t alloc_size, start, cursor_blocks;
> +	uint16_t minimum[I915_MAX_PLANES];
>  	unsigned int total_data_rate;
>  	int plane;
>  
> @@ -2520,9 +2521,21 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
>  	alloc_size -= cursor_blocks;
>  	alloc->end -= cursor_blocks;
>  
> +	/* 1. Allocate the mininum required blocks for each active plane */
> +	for_each_plane(pipe, plane) {
> +		const struct intel_plane_wm_parameters *p;
> +
> +		p = &params->plane[plane];
> +		if (!p->enabled)
> +			continue;
> +
> +		minimum[plane] = 8;
> +		alloc_size -= minimum[plane];
> +	}
> +
>  	/*
> -	 * Each active plane get a portion of the remaining space, in
> -	 * proportion to the amount of data they need to fetch from memory.
> +	 * 2. Distribute the remaining space in proportion to the amount of
> +	 * data each plane needs to fetch from memory.
>  	 *
>  	 * FIXME: we may not allocate every single block here.
>  	 */
> @@ -2544,8 +2557,9 @@ skl_allocate_pipe_ddb(struct drm_crtc *crtc,
>  		 * promote the expression to 64 bits to avoid overflowing, the
>  		 * result is < available as data_rate / total_data_rate < 1
>  		 */
> -		plane_blocks = div_u64((uint64_t)alloc_size * data_rate,
> -				       total_data_rate);
> +		plane_blocks = minimum[plane];
> +		plane_blocks += div_u64((uint64_t)alloc_size * data_rate,
> +					total_data_rate);

The minimum[] array seems pointless. The value here is always going to
be 8.

>  
>  		ddb->plane[pipe][plane].start = start;
>  		ddb->plane[pipe][plane].end = start + plane_blocks;
> -- 
> 1.8.3.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx