On Tue, Jan 28, 2014 at 4:52 PM, Sean Paul seanpaul@chromium.org wrote:
On Mon, Nov 25, 2013 at 9:47 AM, Rob Clark robdclark@gmail.com wrote:
Break the mutable state of a plane out into a separate structure and use atomic properties mechanism to set plane attributes. This makes it easier to have some helpers for plane->set_property() and for checking for invalid params. The idea is that individual drivers can wrap the state struct in their own struct which adds driver specific parameters, for easy build-up of state across multiple set_property() calls and for easy atomic commit or roll- back.
The same should be done for CRTC, encoder, and connector, but this patch only includes the first part (plane).
drivers/gpu/drm/drm_atomic_helper.c | 156 ++++++++++- drivers/gpu/drm/drm_crtc.c | 397 +++++++++++++++++++--------- drivers/gpu/drm/drm_fb_helper.c | 17 +- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 4 +- drivers/gpu/drm/exynos/exynos_drm_encoder.c | 6 +- drivers/gpu/drm/exynos/exynos_drm_plane.c | 15 +- drivers/gpu/drm/i915/intel_sprite.c | 21 +- drivers/gpu/drm/msm/mdp4/mdp4_crtc.c | 2 +- drivers/gpu/drm/msm/mdp4/mdp4_plane.c | 19 +- drivers/gpu/drm/nouveau/dispnv04/overlay.c | 8 +- drivers/gpu/drm/omapdrm/omap_crtc.c | 4 +- drivers/gpu/drm/omapdrm/omap_drv.c | 2 +- drivers/gpu/drm/omapdrm/omap_plane.c | 33 ++- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 8 +- drivers/gpu/drm/shmobile/shmob_drm_crtc.c | 2 +- drivers/gpu/drm/shmobile/shmob_drm_plane.c | 6 +- drivers/gpu/drm/tegra/dc.c | 16 +- include/drm/drm_atomic_helper.h | 39 ++- include/drm/drm_crtc.h | 88 +++++- 19 files changed, 654 insertions(+), 189 deletions(-)
<snip>
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c index 2e31fb8..d585a4c 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c @@ -10,6 +10,7 @@ */
#include <drm/drmP.h> +#include <drm/drm_atomic_helper.h>
#include <drm/exynos_drm.h> #include "exynos_drm_drv.h" @@ -149,7 +150,7 @@ void exynos_plane_commit(struct drm_plane *plane) struct exynos_plane *exynos_plane = to_exynos_plane(plane); struct exynos_drm_overlay *overlay = &exynos_plane->overlay;
exynos_drm_fn_encoder(plane->crtc, &overlay->zpos,
exynos_drm_fn_encoder(plane->state->crtc, &overlay->zpos, exynos_drm_encoder_plane_commit);}
@@ -162,7 +163,7 @@ void exynos_plane_dpms(struct drm_plane *plane, int mode) if (exynos_plane->enabled) return;
exynos_drm_fn_encoder(plane->crtc, &overlay->zpos,
exynos_drm_fn_encoder(plane->state->crtc, &overlay->zpos, exynos_drm_encoder_plane_enable); exynos_plane->enabled = true;@@ -170,7 +171,7 @@ void exynos_plane_dpms(struct drm_plane *plane, int mode) if (!exynos_plane->enabled) return;
exynos_drm_fn_encoder(plane->crtc, &overlay->zpos,
exynos_drm_fn_encoder(plane->state->crtc, &overlay->zpos, exynos_drm_encoder_plane_disable); exynos_plane->enabled = false;@@ -192,7 +193,7 @@ exynos_update_plane(struct drm_plane *plane, struct drm_crtc *crtc, if (ret < 0) return ret;
plane->crtc = crtc;
plane->state->crtc = crtc; exynos_plane_commit(plane); exynos_plane_dpms(plane, DRM_MODE_DPMS_ON);@@ -225,13 +226,17 @@ static int exynos_plane_set_property(struct drm_plane *plane, struct drm_device *dev = plane->dev; struct exynos_plane *exynos_plane = to_exynos_plane(plane); struct exynos_drm_private *dev_priv = dev->dev_private;
struct drm_plane_state *pstate = drm_atomic_get_plane_state(plane, state);Hi Rob, This seems to cause a deadlock. drm_mode_obj_set_property_ioctl performs a drm_modeset_lock_all first, then drm_atomic_helper_get_plane_state tries to lock the crtc[s] again.
oh, whoops, I guess neither modetest or glplane test does a set-property ioctl. I think we simply need to drop the locking at the top level ioctl fxn, and let the atomic locking take care of things.
BR, -R
Trace: (__schedule+0x4d8/0x71c) from [<c0512590>] (schedule+0x90/0x94) (schedule+0x90/0x94) from [<c0512b1c>] (schedule_preempt_disabled+0x18/0x1c) (schedule_preempt_disabled+0x18/0x1c) from [<c0510c78>] (__ww_mutex_lock_slowpath+0x208/0x2cc) (__ww_mutex_lock_slowpath+0x208/0x2cc) from [<c0510d8c>] (__ww_mutex_lock+0x50/0xc4) (__ww_mutex_lock+0x50/0xc4) from [<c028dfc0>] (modeset_lock_state+0x58/0x174) (modeset_lock_state+0x58/0x174) from [<c028e0fc>] (drm_modeset_lock+0x20/0x30) (drm_modeset_lock+0x20/0x30) from [<c028e148>] (drm_modeset_lock_all_crtcs+0x3c/0x54) (drm_modeset_lock_all_crtcs+0x3c/0x54) from [<c029c234>] (drm_atomic_helper_get_plane_state+0x40/0xc4) (drm_atomic_helper_get_plane_state+0x40/0xc4) from [<c02a2d58>] (exynos_plane_set_property+0x3c/0xbc) (exynos_plane_set_property+0x3c/0xbc) from [<c028dab4>] (drm_mode_plane_set_obj_prop+0x3c/0x4c) (drm_mode_plane_set_obj_prop+0x3c/0x4c) from [<c0290734>] (drm_mode_set_obj_prop+0xac/0x128) (drm_mode_set_obj_prop+0xac/0x128) from [<c0294814>] (drm_mode_obj_set_property_ioctl+0xe4/0x15c) (drm_mode_obj_set_property_ioctl+0xe4/0x15c) from [<c028468c>] (drm_ioctl+0x2e0/0x40c) (drm_ioctl+0x2e0/0x40c) from [<c0110c48>] (do_vfs_ioctl+0x508/0x5dc)
Sean
if (IS_ERR(pstate))return PTR_ERR(pstate); if (property == dev_priv->plane_zpos_property) { exynos_plane->overlay.zpos = val; return 0; }
return -EINVAL;
return drm_plane_set_property(plane, pstate, property, val, blob_data);}
static struct drm_plane_funcs exynos_plane_funcs = {
<snip>
-- 1.8.4.2
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel