diff mbox series

[v3,2/2] drm/i915/skl: distribute DDB based on panel resolution

Message ID 20180801151113.5337-1-mahesh1.kumar@intel.com (mailing list archive)
State New, archived
Headers show
Series None | expand

Commit Message

Kumar, Mahesh Aug. 1, 2018, 3:11 p.m. UTC
We distribute DDB equally among all pipes irrespective of display
buffer requirement of each pipe. This leads to a situation where high
resolution y-tiled display can not be enabled with 2 low resolution
displays.

Main contributing factor for DDB requirement is width of the display.
This patch make changes to distribute ddb based on display width.
So display with higher width will get bigger chunk of DDB.

Changes Since V1:
 - pipe_size/ddb_size will not overflow u16 so use appropriate
   data-types during computation (Chris)
Changes Since V2:
 - avoid redundancy and possible truncation errors (Chris)

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107113
Cc: raviraj.p.sitaram@intel.com
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 55 ++++++++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 15 deletions(-)

Comments

Chris Wilson Aug. 1, 2018, 3:11 p.m. UTC | #1
Quoting Mahesh Kumar (2018-08-01 16:11:13)
> We distribute DDB equally among all pipes irrespective of display
> buffer requirement of each pipe. This leads to a situation where high
> resolution y-tiled display can not be enabled with 2 low resolution
> displays.
> 
> Main contributing factor for DDB requirement is width of the display.
> This patch make changes to distribute ddb based on display width.
> So display with higher width will get bigger chunk of DDB.
> 
> Changes Since V1:
>  - pipe_size/ddb_size will not overflow u16 so use appropriate
>    data-types during computation (Chris)
> Changes Since V2:
>  - avoid redundancy and possible truncation errors (Chris)
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107113
> Cc: raviraj.p.sitaram@intel.com
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
Maarten Lankhorst Aug. 7, 2018, 8:35 a.m. UTC | #2
Op 01-08-18 om 17:11 schreef Chris Wilson:
> Quoting Mahesh Kumar (2018-08-01 16:11:13)
>> We distribute DDB equally among all pipes irrespective of display
>> buffer requirement of each pipe. This leads to a situation where high
>> resolution y-tiled display can not be enabled with 2 low resolution
>> displays.
>>
>> Main contributing factor for DDB requirement is width of the display.
>> This patch make changes to distribute ddb based on display width.
>> So display with higher width will get bigger chunk of DDB.
>>
>> Changes Since V1:
>>  - pipe_size/ddb_size will not overflow u16 so use appropriate
>>    data-types during computation (Chris)
>> Changes Since V2:
>>  - avoid redundancy and possible truncation errors (Chris)
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107113
>> Cc: raviraj.p.sitaram@intel.com
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Thanks, reviewed and pushed. :)
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 91120560a61b..382efc048fd8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3814,8 +3814,12 @@  skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct drm_crtc *for_crtc = cstate->base.crtc;
-	u16 pipe_size, ddb_size;
-	int nth_active_pipe;
+	const struct drm_crtc_state *crtc_state;
+	const struct drm_crtc *crtc;
+	u32 pipe_width = 0, total_width = 0, width_before_pipe = 0;
+	enum pipe for_pipe = to_intel_crtc(for_crtc)->pipe;
+	u16 ddb_size;
+	u32 i;
 
 	if (WARN_ON(!state) || !cstate->base.active) {
 		alloc->start = 0;
@@ -3833,14 +3837,14 @@  skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
 				      *num_active, ddb);
 
 	/*
-	 * If the state doesn't change the active CRTC's, then there's
-	 * no need to recalculate; the existing pipe allocation limits
-	 * should remain unchanged.  Note that we're safe from racing
-	 * commits since any racing commit that changes the active CRTC
-	 * list would need to grab _all_ crtc locks, including the one
-	 * we currently hold.
+	 * If the state doesn't change the active CRTC's or there is no
+	 * modeset request, then there's no need to recalculate;
+	 * the existing pipe allocation limits should remain unchanged.
+	 * Note that we're safe from racing commits since any racing commit
+	 * that changes the active CRTC list or do modeset would need to
+	 * grab _all_ crtc locks, including the one we currently hold.
 	 */
-	if (!intel_state->active_pipe_changes) {
+	if (!intel_state->active_pipe_changes && !intel_state->modeset) {
 		/*
 		 * alloc may be cleared by clear_intel_crtc_state,
 		 * copy from old state to be sure
@@ -3849,11 +3853,32 @@  skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
 		return;
 	}
 
-	nth_active_pipe = hweight32(intel_state->active_crtcs &
-				    (drm_crtc_mask(for_crtc) - 1));
-	pipe_size = ddb_size / hweight32(intel_state->active_crtcs);
-	alloc->start = nth_active_pipe * ddb_size / *num_active;
-	alloc->end = alloc->start + pipe_size;
+	/*
+	 * Watermark/ddb requirement highly depends upon width of the
+	 * framebuffer, So instead of allocating DDB equally among pipes
+	 * distribute DDB based on resolution/width of the display.
+	 */
+	for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+		const struct drm_display_mode *adjusted_mode;
+		int hdisplay, vdisplay;
+		enum pipe pipe;
+
+		if (!crtc_state->enable)
+			continue;
+
+		pipe = to_intel_crtc(crtc)->pipe;
+		adjusted_mode = &crtc_state->adjusted_mode;
+		drm_mode_get_hv_timing(adjusted_mode, &hdisplay, &vdisplay);
+		total_width += hdisplay;
+
+		if (pipe < for_pipe)
+			width_before_pipe += hdisplay;
+		else if (pipe == for_pipe)
+			pipe_width = hdisplay;
+	}
+
+	alloc->start = ddb_size * width_before_pipe / total_width;
+	alloc->end = ddb_size * (width_before_pipe + pipe_width) / total_width;
 }
 
 static unsigned int skl_cursor_allocation(int num_active)
@@ -5259,7 +5284,7 @@  skl_ddb_add_affected_pipes(struct drm_atomic_state *state, bool *changed)
 	 * any other display updates race with this transaction, so we need
 	 * to grab the lock on *all* CRTC's.
 	 */
-	if (intel_state->active_pipe_changes) {
+	if (intel_state->active_pipe_changes || intel_state->modeset) {
 		realloc_pipes = ~0;
 		intel_state->wm_results.dirty_pipes = ~0;
 	}