Hi,
On Thursday 18 May 2017 02:44 AM, Matt Roper wrote:
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?
I really missed the point you were trying to make previously, Indeed
this should return -EINVAL,
I missed that while recreating the patch. thanks alot for pointing that out.
submitting new version of patch.
-Mahesh
Matt
*out_blocks = res_blocks;
*out_lines = res_lines;
- *enabled = true;
return 0;
}
...
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
