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@xxxxxxxxx>
> Signed-off-by: Damien Lespiau <damien.lespiau@xxxxxxxxx>
> ---
> 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 = ¶ms->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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
