| Message ID | 1476278901-15750-5-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Oct 12, 2016 at 03:28:17PM +0200, Maarten Lankhorst wrote: > Move calculating minimum allocations to a helper, which cleans up the > code some more. The cursor is still allocated in advance because it > doesn't count towards data rate and should always be reserved. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 66 ++++++++++++++++++++++++----------------- > 1 file changed, 39 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 83c1b0acef38..45fb8275abea 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3342,6 +3342,32 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate, > return DIV_ROUND_UP((4 * src_w * plane_bpp), 512) * min_scanlines/4 + 3; > } > > +static void > +skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active, > + uint16_t *minimum, uint16_t *y_minimum) > +{ > + const struct drm_plane_state *pstate; > + struct drm_plane *plane; > + enum pipe pipe = to_intel_crtc(cstate->base.crtc)->pipe; > + > + drm_atomic_crtc_state_for_each_plane_state(plane, pstate, &cstate->base) { > + struct intel_plane *intel_plane = to_intel_plane(plane); > + int id = skl_wm_plane_id(intel_plane); > + > + if (intel_plane->pipe != pipe || > + id == PLANE_CURSOR) > + continue; > + > + if (!pstate->visible) > + continue; > + > + minimum[id] = skl_ddb_min_alloc(pstate, 0); > + y_minimum[id] = skl_ddb_min_alloc(pstate, 1); > + } > + > + minimum[PLANE_CURSOR] = skl_cursor_allocation(num_active); > +} > + > static int > skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, > struct skl_ddb_allocation *ddb /* out */) > @@ -3350,12 +3376,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, > struct drm_crtc *crtc = cstate->base.crtc; > struct drm_device *dev = crtc->dev; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - struct intel_plane *intel_plane; > - struct drm_plane *plane; > - const struct drm_plane_state *pstate; > enum pipe pipe = intel_crtc->pipe; > struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb; > - uint16_t alloc_size, start, cursor_blocks; > + uint16_t alloc_size, start; > uint16_t minimum[I915_MAX_PLANES] = {}; > uint16_t y_minimum[I915_MAX_PLANES] = {}; > unsigned int total_data_rate; > @@ -3384,35 +3407,21 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, > return 0; > } > > - cursor_blocks = skl_cursor_allocation(num_active); > - ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - cursor_blocks; > - ddb->plane[pipe][PLANE_CURSOR].end = alloc->end; > - > - alloc_size -= cursor_blocks; > - > - /* 1. Allocate the mininum required blocks for each active plane */ > - drm_atomic_crtc_state_for_each_plane_state(plane, pstate, &cstate->base) { > - intel_plane = to_intel_plane(plane); > - id = skl_wm_plane_id(intel_plane); > - > - if (intel_plane->pipe != pipe) > - continue; > - > - if (!pstate->visible) > - continue; > + skl_ddb_calc_min(cstate, num_active, minimum, y_minimum); > > - if (plane->type == DRM_PLANE_TYPE_CURSOR) > - continue; > - > - minimum[id] = skl_ddb_min_alloc(pstate, 0); > - y_minimum[id] = skl_ddb_min_alloc(pstate, 1); > - } > + /* 1. Allocate the mininum required blocks for each active plane Minor style nitpick; different multi-line comment format than we typically use (and that we use for #2 below). Otherwise, Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > + * and allocate the cursor, it doesn't require extra allocation > + * proportional to the data rate. > + */ > > - for (i = 0; i < PLANE_CURSOR; i++) { > + for (i = 0; i < I915_MAX_PLANES; i++) { > alloc_size -= minimum[i]; > alloc_size -= y_minimum[i]; > } > > + ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - minimum[PLANE_CURSOR]; > + ddb->plane[pipe][PLANE_CURSOR].end = alloc->end; > + > /* > * 2. Distribute the remaining space in proportion to the amount of > * data each plane needs to fetch from memory. > @@ -3428,6 +3437,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, > unsigned rate; > uint16_t plane_blocks, y_plane_blocks = 0; > > + if (id == PLANE_CURSOR) > + continue; > + > rate = data_rate[id]; > > /* > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Em Qua, 2016-10-12 às 15:28 +0200, Maarten Lankhorst escreveu: > Move calculating minimum allocations to a helper, which cleans up the > code some more. The cursor is still allocated in advance because it > doesn't count towards data rate and should always be reserved. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_pm.c | 66 ++++++++++++++++++++++++------- > ---------- > 1 file changed, 39 insertions(+), 27 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_pm.c > b/drivers/gpu/drm/i915/intel_pm.c > index 83c1b0acef38..45fb8275abea 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3342,6 +3342,32 @@ skl_ddb_min_alloc(const struct drm_plane_state > *pstate, > return DIV_ROUND_UP((4 * src_w * plane_bpp), 512) * > min_scanlines/4 + 3; > } > > +static void > +skl_ddb_calc_min(const struct intel_crtc_state *cstate, int > num_active, > + uint16_t *minimum, uint16_t *y_minimum) > +{ > + const struct drm_plane_state *pstate; > + struct drm_plane *plane; > + enum pipe pipe = to_intel_crtc(cstate->base.crtc)->pipe; > + > + drm_atomic_crtc_state_for_each_plane_state(plane, pstate, > &cstate->base) { > + struct intel_plane *intel_plane = > to_intel_plane(plane); > + int id = skl_wm_plane_id(intel_plane); > + > + if (intel_plane->pipe != pipe || > + id == PLANE_CURSOR) You can also remove the check for pipe here. > + continue; > + > + if (!pstate->visible) > + continue; > + > + minimum[id] = skl_ddb_min_alloc(pstate, 0); > + y_minimum[id] = skl_ddb_min_alloc(pstate, 1); > + } > + > + minimum[PLANE_CURSOR] = skl_cursor_allocation(num_active); > +} > + > static int > skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, > struct skl_ddb_allocation *ddb /* out */) > @@ -3350,12 +3376,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state > *cstate, > struct drm_crtc *crtc = cstate->base.crtc; > struct drm_device *dev = crtc->dev; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - struct intel_plane *intel_plane; > - struct drm_plane *plane; > - const struct drm_plane_state *pstate; > enum pipe pipe = intel_crtc->pipe; > struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb; > - uint16_t alloc_size, start, cursor_blocks; > + uint16_t alloc_size, start; > uint16_t minimum[I915_MAX_PLANES] = {}; > uint16_t y_minimum[I915_MAX_PLANES] = {}; > unsigned int total_data_rate; > @@ -3384,35 +3407,21 @@ skl_allocate_pipe_ddb(struct intel_crtc_state > *cstate, > return 0; > } > > - cursor_blocks = skl_cursor_allocation(num_active); > - ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - > cursor_blocks; > - ddb->plane[pipe][PLANE_CURSOR].end = alloc->end; > - > - alloc_size -= cursor_blocks; > - > - /* 1. Allocate the mininum required blocks for each active > plane */ > - drm_atomic_crtc_state_for_each_plane_state(plane, pstate, > &cstate->base) { > - intel_plane = to_intel_plane(plane); > - id = skl_wm_plane_id(intel_plane); > - > - if (intel_plane->pipe != pipe) > - continue; > - > - if (!pstate->visible) > - continue; > + skl_ddb_calc_min(cstate, num_active, minimum, y_minimum); > > - if (plane->type == DRM_PLANE_TYPE_CURSOR) > - continue; > - > - minimum[id] = skl_ddb_min_alloc(pstate, 0); > - y_minimum[id] = skl_ddb_min_alloc(pstate, 1); > - } > + /* 1. Allocate the mininum required blocks for each active > plane > + * and allocate the cursor, it doesn't require extra > allocation > + * proportional to the data rate. > + */ > > - for (i = 0; i < PLANE_CURSOR; i++) { > + for (i = 0; i < I915_MAX_PLANES; i++) { As I mentioned earlier, this is also an unsafe loop. I know you didn't introduce it, so we can fix this in a next patch. With the pipe check removed (and Matt's requests addressed): Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > alloc_size -= minimum[i]; > alloc_size -= y_minimum[i]; > } > > + ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - > minimum[PLANE_CURSOR]; > + ddb->plane[pipe][PLANE_CURSOR].end = alloc->end; > + > /* > * 2. Distribute the remaining space in proportion to the > amount of > * data each plane needs to fetch from memory. > @@ -3428,6 +3437,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state > *cstate, > unsigned rate; > uint16_t plane_blocks, y_plane_blocks = 0; > > + if (id == PLANE_CURSOR) > + continue; > + > rate = data_rate[id]; > > /*
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 83c1b0acef38..45fb8275abea 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3342,6 +3342,32 @@ skl_ddb_min_alloc(const struct drm_plane_state *pstate, return DIV_ROUND_UP((4 * src_w * plane_bpp), 512) * min_scanlines/4 + 3; } +static void +skl_ddb_calc_min(const struct intel_crtc_state *cstate, int num_active, + uint16_t *minimum, uint16_t *y_minimum) +{ + const struct drm_plane_state *pstate; + struct drm_plane *plane; + enum pipe pipe = to_intel_crtc(cstate->base.crtc)->pipe; + + drm_atomic_crtc_state_for_each_plane_state(plane, pstate, &cstate->base) { + struct intel_plane *intel_plane = to_intel_plane(plane); + int id = skl_wm_plane_id(intel_plane); + + if (intel_plane->pipe != pipe || + id == PLANE_CURSOR) + continue; + + if (!pstate->visible) + continue; + + minimum[id] = skl_ddb_min_alloc(pstate, 0); + y_minimum[id] = skl_ddb_min_alloc(pstate, 1); + } + + minimum[PLANE_CURSOR] = skl_cursor_allocation(num_active); +} + static int skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, struct skl_ddb_allocation *ddb /* out */) @@ -3350,12 +3376,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, struct drm_crtc *crtc = cstate->base.crtc; struct drm_device *dev = crtc->dev; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct intel_plane *intel_plane; - struct drm_plane *plane; - const struct drm_plane_state *pstate; enum pipe pipe = intel_crtc->pipe; struct skl_ddb_entry *alloc = &cstate->wm.skl.ddb; - uint16_t alloc_size, start, cursor_blocks; + uint16_t alloc_size, start; uint16_t minimum[I915_MAX_PLANES] = {}; uint16_t y_minimum[I915_MAX_PLANES] = {}; unsigned int total_data_rate; @@ -3384,35 +3407,21 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, return 0; } - cursor_blocks = skl_cursor_allocation(num_active); - ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - cursor_blocks; - ddb->plane[pipe][PLANE_CURSOR].end = alloc->end; - - alloc_size -= cursor_blocks; - - /* 1. Allocate the mininum required blocks for each active plane */ - drm_atomic_crtc_state_for_each_plane_state(plane, pstate, &cstate->base) { - intel_plane = to_intel_plane(plane); - id = skl_wm_plane_id(intel_plane); - - if (intel_plane->pipe != pipe) - continue; - - if (!pstate->visible) - continue; + skl_ddb_calc_min(cstate, num_active, minimum, y_minimum); - if (plane->type == DRM_PLANE_TYPE_CURSOR) - continue; - - minimum[id] = skl_ddb_min_alloc(pstate, 0); - y_minimum[id] = skl_ddb_min_alloc(pstate, 1); - } + /* 1. Allocate the mininum required blocks for each active plane + * and allocate the cursor, it doesn't require extra allocation + * proportional to the data rate. + */ - for (i = 0; i < PLANE_CURSOR; i++) { + for (i = 0; i < I915_MAX_PLANES; i++) { alloc_size -= minimum[i]; alloc_size -= y_minimum[i]; } + ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - minimum[PLANE_CURSOR]; + ddb->plane[pipe][PLANE_CURSOR].end = alloc->end; + /* * 2. Distribute the remaining space in proportion to the amount of * data each plane needs to fetch from memory. @@ -3428,6 +3437,9 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate, unsigned rate; uint16_t plane_blocks, y_plane_blocks = 0; + if (id == PLANE_CURSOR) + continue; + rate = data_rate[id]; /*
Move calculating minimum allocations to a helper, which cleans up the code some more. The cursor is still allocated in advance because it doesn't count towards data rate and should always be reserved. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/intel_pm.c | 66 ++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 27 deletions(-)