On Tue, May 17, 2016 at 03:08:01PM +0200, Maarten Lankhorst wrote:
> All of intel_post_plane_update is handled there now, so move it over.
> This is run after the hw state checker because it can't handle checking
> crtc's separately yet.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankho...@linux.intel.com>
First this patch is massive, and imo too big and should have been split
up.
> ---
> drivers/gpu/drm/i915/intel_atomic.c | 11 ++
> drivers/gpu/drm/i915/intel_display.c | 344
> ++++++++++++++++++++++-------------
> drivers/gpu/drm/i915/intel_drv.h | 5 +-
> 3 files changed, 228 insertions(+), 132 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 50ff90aea721..b4927f6bbeac 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -311,6 +311,17 @@ intel_atomic_state_alloc(struct drm_device *dev)
> void intel_atomic_state_clear(struct drm_atomic_state *s)
> {
> struct intel_atomic_state *state = to_intel_atomic_state(s);
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(state->work); i++) {
> + struct intel_flip_work *work = state->work[i];
> +
> + if (work)
> + intel_free_flip_work(work);
> +
> + state->work[i] = NULL;
> + }
> +
> drm_atomic_state_default_clear(&state->base);
> state->dpll_set = state->modeset = false;
> }
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 69abc808a2c4..16d8e299994d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4537,39 +4537,6 @@ intel_pre_disable_primary_noatomic(struct drm_crtc
> *crtc)
> }
> }
>
> -static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
> -{
> - struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> - struct drm_atomic_state *old_state = old_crtc_state->base.state;
> - struct intel_crtc_state *pipe_config =
> - to_intel_crtc_state(crtc->base.state);
> - struct drm_device *dev = crtc->base.dev;
> - struct drm_plane *primary = crtc->base.primary;
> - struct drm_plane_state *old_pri_state =
> - drm_atomic_get_existing_plane_state(old_state, primary);
> -
> - intel_frontbuffer_flip(dev, pipe_config->fb_bits);
> -
> - crtc->wm.cxsr_allowed = true;
> -
> - if (pipe_config->update_wm_post && pipe_config->base.active)
> - intel_update_watermarks(&crtc->base);
> -
> - if (old_pri_state) {
> - struct intel_plane_state *primary_state =
> - to_intel_plane_state(primary->state);
> - struct intel_plane_state *old_primary_state =
> - to_intel_plane_state(old_pri_state);
> -
> - intel_fbc_post_update(crtc);
> -
> - if (primary_state->visible &&
> - (needs_modeset(&pipe_config->base) ||
> - !old_primary_state->visible))
> - intel_post_enable_primary(&crtc->base);
> - }
> -}
The above seems to have moved/disappeared entirely, and is definitely not
unpin related. Really, this must be split up. I guess another reason to
revert this for now.
Or at least the commit should have a more accurate summary. But since I
can't even find the new callsite of these functions in this diff here
something seems fishy, but I didn't look at the entire series.
Anyway, with that rant out of the way see below for what I really wanted
to comment on.
[snip]
> +static void intel_prepare_work(struct drm_crtc *crtc,
> + struct intel_flip_work *work,
> + struct drm_atomic_state *state,
> + struct drm_crtc_state *old_crtc_state)
> {
> - unsigned last_vblank_count[I915_MAX_PIPES];
> - enum pipe pipe;
> - int ret;
> + struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_plane_state *old_plane_state;
> + struct drm_plane *plane;
> + int i, j = 0;
>
> - if (!crtc_mask)
> - return;
> + INIT_WORK(&work->unpin_work, intel_unpin_work_fn);
> + INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
> + atomic_inc(&intel_crtc->unpin_work_count);
>
> - for_each_pipe(dev_priv, pipe) {
> - struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> + for_each_plane_in_state(state, plane, old_plane_state, i) {
> + struct intel_plane_state *old_state =
> to_intel_plane_state(old_plane_state);
> + struct intel_plane_state *new_state =
> to_intel_plane_state(plane->state);
>
> - if (!((1 << pipe) & crtc_mask))
> + if (old_state->base.crtc != crtc &&
> + new_state->base.crtc != crtc)
> continue;
>
> - ret = drm_crtc_vblank_get(crtc);
> - if (WARN_ON(ret != 0)) {
> - crtc_mask &= ~(1 << pipe);
> - continue;
> + if (plane->type == DRM_PLANE_TYPE_PRIMARY) {
> + plane->fb = new_state->base.fb;
> + crtc->x = new_state->base.src_x >> 16;
> + crtc->y = new_state->base.src_y >> 16;
> }
>
> - last_vblank_count[pipe] = drm_crtc_vblank_count(crtc);
> + old_state->wait_req = new_state->wait_req;
> + new_state->wait_req = NULL;
> +
> + old_state->base.fence = new_state->base.fence;
> + new_state->base.fence = NULL;
> +
> + /* remove plane state from the atomic state and move it to work
> */
> + old_plane_state->state = NULL;
> + state->planes[i] = NULL;
> + state->plane_states[i] = NULL;
> +
> + work->old_plane_state[j] = old_state;
> + work->new_plane_state[j++] = new_state;
> }
>
> - for_each_pipe(dev_priv, pipe) {
> - struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> - long lret;
> + old_crtc_state->state = NULL;
> + state->crtcs[drm_crtc_index(crtc)] = NULL;
> + state->crtc_states[drm_crtc_index(crtc)] = NULL;
>
> - if (!((1 << pipe) & crtc_mask))
> - continue;
> + work->old_crtc_state = to_intel_crtc_state(old_crtc_state);
> + work->new_crtc_state = to_intel_crtc_state(crtc->state);
> + work->num_planes = j;
>
> - lret = wait_event_timeout(dev->vblank[pipe].queue,
> - last_vblank_count[pipe] !=
> - drm_crtc_vblank_count(crtc),
> - msecs_to_jiffies(50));
> + work->event = crtc->state->event;
> + crtc->state->event = NULL;
>
> - WARN(!lret, "pipe %c vblank wait timed out\n", pipe_name(pipe));
> + if (needs_modeset(crtc->state) || work->new_crtc_state->update_pipe) {
> + struct drm_connector *conn;
> + struct drm_connector_state *old_conn_state;
> + int k = 0;
>
> - drm_crtc_vblank_put(crtc);
> - }
> + j = 0;
> +
> + /*
> + * intel_unpin_work_fn cannot depend on the connector list
> + * because it may be freed from underneath it, so add
> + * them all to the work struct while we're holding locks.
> + */
Thanks to connector refcounting no longer true, and as long as you hold
onto drm_atomic_state the connectors will no longer disappear. Please
revise/update.
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
