On Wed, Apr 08, 2015 at 06:56:53PM -0700, Matt Roper wrote:
> Switch from our plane update/disable entrypoints to use the full atomic
> helpers (which generate a top-level atomic transaction) rather than the
> transitional helpers (which only create/manipulate orphaned plane states
> independent of a top-level transaction). Various upcoming work (SKL
> scalers, atomic watermarks, etc.) requires a full atomic transaction to
> behave properly/cleanly.
>
> Last time we tried this, we had to back out the change because we still
> call the drm_plane vfuncs directly from within our legacy modesetting
> code. This potentially results in nested atomic transactions, locking
> collisions, and other failures. To avoid that problem again, we
> sidestep the issue by calling the transitional helpers directly (rather
> than through a vfunc) when we're nested inside of other legacy
> modesetting code. However this does allow legacy SetPlane() ioctl's to
> process an entire drm_atomic_state transaction, which is important for
> upcoming patches.
>
> Cc: Chandra Konduru <chandra.konduru@xxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
Maarten is working to fix this properly (by wiring up plane states to the
transitional drm_atomic_state scaffolding from Ander), but seems like a
good interim idea to get back some solid testing for our atomic code.
Can I apply this without patches 1&2 right away or is there a tricky
depency?
-Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++------------
> drivers/gpu/drm/i915/intel_sprite.c | 10 +++++-----
> 2 files changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1cf91ad..3a74923 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5576,7 +5576,7 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
> dev_priv->display.crtc_disable(crtc);
> dev_priv->display.off(crtc);
>
> - crtc->primary->funcs->disable_plane(crtc->primary);
> + drm_plane_helper_disable(crtc->primary);
>
> /* Update computed state. */
> list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> @@ -11731,11 +11731,11 @@ static int __intel_set_mode(struct drm_crtc *crtc,
> int vdisplay, hdisplay;
>
> drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
> - ret = primary->funcs->update_plane(primary, &intel_crtc->base,
> - fb, 0, 0,
> - hdisplay, vdisplay,
> - x << 16, y << 16,
> - hdisplay << 16, vdisplay << 16);
> + ret = drm_plane_helper_update(primary, &intel_crtc->base,
> + fb, 0, 0,
> + hdisplay, vdisplay,
> + x << 16, y << 16,
> + hdisplay << 16, vdisplay << 16);
> }
>
> /* Now enable the clocks, plane, pipe, and connectors that we set up. */
> @@ -12286,10 +12286,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
> int vdisplay, hdisplay;
>
> drm_crtc_get_hv_timing(set->mode, &hdisplay, &vdisplay);
> - ret = primary->funcs->update_plane(primary, set->crtc, set->fb,
> - 0, 0, hdisplay, vdisplay,
> - set->x << 16, set->y << 16,
> - hdisplay << 16, vdisplay << 16);
> + ret = drm_plane_helper_update(primary, set->crtc, set->fb,
> + 0, 0, hdisplay, vdisplay,
> + set->x << 16, set->y << 16,
> + hdisplay << 16, vdisplay << 16);
>
> /*
> * We need to make sure the primary plane is re-enabled if it
> @@ -12830,8 +12830,8 @@ void intel_plane_destroy(struct drm_plane *plane)
> }
>
> const struct drm_plane_funcs intel_plane_funcs = {
> - .update_plane = drm_plane_helper_update,
> - .disable_plane = drm_plane_helper_disable,
> + .update_plane = drm_atomic_helper_update_plane,
> + .disable_plane = drm_atomic_helper_disable_plane,
> .destroy = intel_plane_destroy,
> .set_property = drm_atomic_helper_plane_set_property,
> .atomic_get_property = intel_plane_atomic_get_property,
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 492abcd..f4094d2 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -1149,11 +1149,11 @@ int intel_plane_restore(struct drm_plane *plane)
> if (!plane->crtc || !plane->state->fb)
> return 0;
>
> - return plane->funcs->update_plane(plane, plane->crtc, plane->state->fb,
> - plane->state->crtc_x, plane->state->crtc_y,
> - plane->state->crtc_w, plane->state->crtc_h,
> - plane->state->src_x, plane->state->src_y,
> - plane->state->src_w, plane->state->src_h);
> + return drm_plane_helper_update(plane, plane->crtc, plane->state->fb,
> + plane->state->crtc_x, plane->state->crtc_y,
> + plane->state->crtc_w, plane->state->crtc_h,
> + plane->state->src_x, plane->state->src_y,
> + plane->state->src_w, plane->state->src_h);
> }
>
> static uint32_t ilk_plane_formats[] = {
> --
> 1.8.5.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
