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.ku...@intel.com>
> Signed-off-by: Damien Lespiau <damien.lesp...@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

Reply via email to