Op 01-04-16 om 03:46 schreef Matt Roper:
> We eventually want to calculate watermark values at atomic 'check' time
> instead of atomic 'commit' time so that any requested configurations
> that result in impossible watermark requirements are properly rejected.
> The first step along this path is to allocate the DDB at atomic 'check'
> time. As we perform this transition, allow the main allocation function
> to operate successfully on either an in-flight state or an
> already-commited state. Once we complete the transition in a future
> patch, we'll come back and remove the unnecessary logic for the
> already-committed case.
>
> Note one other minor change here...when working with the
> already-committed state, we pull the active CRTC's from
> hweight(dev_priv->active_crtcs) instead of
> dev_priv->wm.config.num_pipes_active. The values should be equivalent,
> but dev_priv->wm.config is pretty much unused at this point and it would
> be nice to eventually remove it entirely.
> ---
> drivers/gpu/drm/i915/i915_drv.h | 6 +++
> drivers/gpu/drm/i915/intel_pm.c | 99
> ++++++++++++++++++++++++++++++-----------
> 2 files changed, 78 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d3ebb2f..79f1974 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -318,6 +318,12 @@ struct i915_hotplug {
> &dev->mode_config.plane_list, \
> base.head)
>
> +#define for_each_intel_plane_mask(dev, intel_plane, plane_mask)
> \
> + list_for_each_entry(intel_plane, &dev->mode_config.plane_list, \
> + base.head) \
> + for_each_if ((plane_mask) & \
> + (1 << drm_plane_index(&intel_plane->base)))
> +
> #define for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) \
> list_for_each_entry(intel_plane, \
> &(dev)->mode_config.plane_list, \
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index e92513e..e0ca2b9 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -2849,15 +2849,15 @@ skl_wm_plane_id(const struct intel_plane *plane)
> static void
> skl_ddb_get_pipe_allocation_limits(struct drm_device *dev,
> const struct intel_crtc_state *cstate,
> - const struct intel_wm_config *config,
> + const unsigned active_crtcs,
> struct skl_ddb_entry *alloc /* out */)
> {
> - struct drm_crtc *for_crtc = cstate->base.crtc;
> - struct drm_crtc *crtc;
> + struct drm_crtc *crtc = cstate->base.crtc;
> unsigned int pipe_size, ddb_size;
> + unsigned int num_active = hweight32(active_crtcs);
> int nth_active_pipe;
>
> - if (!cstate->base.active) {
> + if (!cstate->base.active || WARN_ON(num_active == 0)) {
> alloc->start = 0;
> alloc->end = 0;
> return;
> @@ -2870,25 +2870,16 @@ skl_ddb_get_pipe_allocation_limits(struct drm_device
> *dev,
>
> ddb_size -= 4; /* 4 blocks for bypass path allocation */
>
> - nth_active_pipe = 0;
> - for_each_crtc(dev, crtc) {
> - if (!to_intel_crtc(crtc)->active)
> - continue;
> + nth_active_pipe = hweight32(active_crtcs & (drm_crtc_mask(crtc) - 1));
>
> - if (crtc == for_crtc)
> - break;
> -
> - nth_active_pipe++;
> - }
> -
> - pipe_size = ddb_size / config->num_pipes_active;
> - alloc->start = nth_active_pipe * ddb_size / config->num_pipes_active;
> + pipe_size = ddb_size / num_active;
> + alloc->start = nth_active_pipe * ddb_size / num_active;
> alloc->end = alloc->start + pipe_size;
> }
>
> -static unsigned int skl_cursor_allocation(const struct intel_wm_config
> *config)
> +static unsigned int skl_cursor_allocation(const unsigned active_crtcs)
> {
> - if (config->num_pipes_active == 1)
> + if (hweight32(active_crtcs) == 1)
> return 32;
>
> return 8;
> @@ -3018,14 +3009,13 @@ skl_get_total_relative_data_rate(const struct
> intel_crtc_state *cstate)
> return total_data_rate;
> }
>
> -static void
> +static int
> skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> struct skl_ddb_allocation *ddb /* out */)
> {
> struct drm_crtc *crtc = cstate->base.crtc;
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_wm_config *config = &dev_priv->wm.config;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> struct intel_plane *intel_plane;
> enum pipe pipe = intel_crtc->pipe;
> @@ -3034,17 +3024,43 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> uint16_t minimum[I915_MAX_PLANES];
> uint16_t y_minimum[I915_MAX_PLANES];
> unsigned int total_data_rate;
> + unsigned active_crtcs = 0;
>
> - skl_ddb_get_pipe_allocation_limits(dev, cstate, config, alloc);
> + if (!cstate->base.active) {
> + ddb->pipe[pipe].start = ddb->pipe[pipe].end = 0;
> + memset(ddb->plane[pipe], 0,
> + I915_MAX_PLANES * sizeof(struct skl_ddb_entry));
> + memset(ddb->y_plane[pipe], 0,
> + I915_MAX_PLANES * sizeof(struct skl_ddb_entry));
> + return 0;
> + }
> +
> + /*
> + * TODO: At the moment we might call this on either an in-flight CRTC
> + * state or an already-committed state, so look up the number of
> + * active CRTC's accordingly. Eventually this will only be called
> + * on in-flight states and we'll be able to drop some of this extra
> + * logic.
> + */
> + if (cstate->base.state) {
> + struct intel_atomic_state *intel_state =
> + to_intel_atomic_state(cstate->base.state);
> +
> + active_crtcs = intel_state->active_crtcs;
> + } else {
> + active_crtcs = dev_priv->active_crtcs;
> + }
> +
> + skl_ddb_get_pipe_allocation_limits(dev, cstate, active_crtcs, alloc);
> alloc_size = skl_ddb_entry_size(alloc);
> if (alloc_size == 0) {
> memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> memset(&ddb->plane[pipe][PLANE_CURSOR], 0,
> sizeof(ddb->plane[pipe][PLANE_CURSOR]));
> - return;
> + return 0;
> }
>
> - cursor_blocks = skl_cursor_allocation(config);
> + cursor_blocks = skl_cursor_allocation(active_crtcs);
> ddb->plane[pipe][PLANE_CURSOR].start = alloc->end - cursor_blocks;
> ddb->plane[pipe][PLANE_CURSOR].end = alloc->end;
>
> @@ -3054,15 +3070,30 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> /* 1. Allocate the mininum required blocks for each active plane */
> for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> struct drm_plane *plane = &intel_plane->base;
> + struct drm_plane_state *pstate;
> struct drm_framebuffer *fb = plane->state->fb;
> int id = skl_wm_plane_id(intel_plane);
>
> - if (!to_intel_plane_state(plane->state)->visible)
> + /*
> + * TODO: Remove support for already-committed state once we
> + * only allocate DDB on in-flight states.
> + */
> + if (cstate->base.state) {
> + pstate = drm_atomic_get_plane_state(cstate->base.state,
> + plane);
> + if (IS_ERR(pstate))
> + return PTR_ERR(pstate);
> + } else {
> + pstate = plane->state;
> + }
> +
> + if (!to_intel_plane_state(pstate)->visible)
> continue;
>
> if (plane->type == DRM_PLANE_TYPE_CURSOR)
> continue;
>
> + fb = pstate->fb;
> minimum[id] = 8;
> alloc_size -= minimum[id];
> y_minimum[id] = (fb->pixel_format == DRM_FORMAT_NV12) ? 8 : 0;
> @@ -3078,13 +3109,26 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> total_data_rate = skl_get_total_relative_data_rate(cstate);
>
> start = alloc->start;
> - for_each_intel_plane_on_crtc(dev, intel_crtc, intel_plane) {
> + for_each_intel_plane_mask(dev, intel_plane, cstate->base.plane_mask) {
> struct drm_plane *plane = &intel_plane->base;
> - struct drm_plane_state *pstate = intel_plane->base.state;
> + struct drm_plane_state *pstate;
> unsigned int data_rate, y_data_rate;
> uint16_t plane_blocks, y_plane_blocks = 0;
> int id = skl_wm_plane_id(intel_plane);
>
> + /*
> + * TODO: Remove support for already-committed state once we
> + * only allocate DDB on in-flight states.
> + */
> + if (cstate->base.state) {
> + pstate = drm_atomic_get_plane_state(cstate->base.state,
> + plane);
>
Yuck again? :( No way around this by storing data rates for example?
Oh well, at least set cstate->base.planes_changed please.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
