On Wed, May 17, 2017 at 05:28:30PM +0530, Mahesh Kumar wrote:
> From: "Kumar, Mahesh" <mahesh1.kumar@xxxxxxxxx>
>
> This patch implements new DDB allocation algorithm as per HW team
> recommendation. This algo takecare of scenario where we allocate less DDB
> for the planes with lower relative pixel rate, but they require more DDB
> to work.
> It also takes care of enabling same watermark level for each
> plane in crtc, for efficient power saving.
>
> Changes since v1:
> - Rebase on top of Paulo's patch series
>
> Changes since v2:
> - Fix the for loop condition to enable WM
>
> Changes since v3:
> - Fix crash in cursor i-g-t reported by Maarten
> - Rebase after addressing Paulo's comments
> - Few other ULT fixes
> Changes since v4:
> - Rebase on drm-tip
> - Added separate function to enable WM levels
> Changes since v5:
> - Fix a crash identified in skl-6770HQ system
> Changes since v6:
> - Address review comments from Matt
>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@xxxxxxxxx>
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> ---
...
> @@ -4096,10 +4126,48 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
...
> +
> + /*
> + * If This this level can successfully be enabled with the
Typo; repeated word ("This this").
...
> @@ -4381,64 +4452,41 @@ static int skl_compute_plane_wm(const struct drm_i915_private *dev_priv,
> }
> }
>
> - if (res_blocks >= ddb_allocation || res_lines > 31) {
> - *enabled = false;
> + if (res_lines >= 31 && level == 0) {
> + struct drm_plane *plane = pstate->plane;
>
> - /*
> - * If there are no valid level 0 watermarks, then we can't
> - * support this display configuration.
> - */
> - if (level) {
> - return 0;
> - } else {
> - struct drm_plane *plane = pstate->plane;
> -
> - DRM_DEBUG_KMS("Requested display configuration exceeds system watermark limitations\n");
> - DRM_DEBUG_KMS("[PLANE:%d:%s] blocks required = %u/%u, lines required = %u/31\n",
> - plane->base.id, plane->name,
> - res_blocks, ddb_allocation, res_lines);
> - return -EINVAL;
> - }
> + DRM_DEBUG_KMS("Requested display configuration exceeds system watermark limitations\n");
> + DRM_DEBUG_KMS("[PLANE:%d:%s] lines required = %u/31\n",
> + plane->base.id, plane->name, res_lines);
> }
I'm still confused why we don't need to return an error in this case?
We print the debug message here, but the function still returns 0
(success). When we run skl_enable_plane_wm_levels(), it will disable
level 0 for that plane, which means we should be rejecting the whole
atomic flip (since this plane fails at all levels), but I don't see
anywhere that we actually check that again and trigger the failure?
Matt
>
> *out_blocks = res_blocks;
> *out_lines = res_lines;
> - *enabled = true;
>
> return 0;
> }
>
...
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
