Hi Ander,
Thanks for review
On Wednesday 12 April 2017 02:47 PM, Ander Conselvan De Oliveira wrote:
On Tue, 2017-02-28 at 17:01 +0530, Mahesh Kumar wrote:
DDB minimum requirement may also exceed the allocated DDB for CRTC.
Instead of directly deducting from alloc_size, check against
total_min_ddb requirement. if exceeding fail the flip.
Instead of doing a low level description of the code change, including variable
names and whatnot, use this space to describe in a high level what the patch
does and why it is necessary. For instance, in this particular case the patch
changes the modeset to fail when there isn't enough ddb for the given
configuration. Previously it succeeded, but the plane ddb allocations would be
bogus because of the negative value of alloc_size, leading to screen corruption
and system hangs.
Ok, will make changes,
Not only modeset, any flip whose requirement is exceeding available ddb
allocation, we should fail that ioctl.
Do I understand correctly that this patch is independent from the rest of the
series? Might make sense to merge it earlier, since the bug is there with the
old ddb allocation algorithm too.
Yes, patch 3,4 as well are independent of the series & applicable for
old algorithm too.
Signed-off-by: Mahesh Kumar <mahesh1.kumar@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 76c986166664..24e9e5d69833 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3380,6 +3380,7 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
int num_active;
unsigned plane_data_rate[I915_MAX_PLANES] = {};
unsigned plane_y_data_rate[I915_MAX_PLANES] = {};
+ uint16_t total_min_blocks = 0;
/* Clear the partitioning for disabled planes. */
memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
@@ -3407,10 +3408,18 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
*/
for_each_plane_id_on_crtc(intel_crtc, plane_id) {
- alloc_size -= minimum[plane_id];
- alloc_size -= y_minimum[plane_id];
+ total_min_blocks += minimum[plane_id];
+ total_min_blocks += y_minimum[plane_id];
}
+ if ((total_min_blocks > alloc_size)) {
One '(' is enough.
will do :)
+ DRM_DEBUG_KMS("Requested display configuration exceeds system DDB limitations");
+ DRM_DEBUG_KMS("minimum required %d/%d\n", total_min_blocks,
+ alloc_size);
+ return-EINVAL;
Space between return and -EINVAL please.
will update.
-Mahesh
Ander
+ }
+
+ alloc_size -= total_min_blocks;
ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - minimum[PLANE_CURSOR];
ddb->plane[pipe][PLANE_CURSOR].end = alloc->end;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
