On Wed, Jan 29, 2014 at 8:44 AM, Rob Clark robdclark@gmail.com wrote:
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.
Seems like there's another one in the drm_fb_helper_restore_fbdev_mode path.
(modeset_lock_state+0x58/0x174) from [<c028e300>] (drm_modeset_lock+0x20/0x30) (drm_modeset_lock+0x20/0x30) from [<c028e34c>] (drm_modeset_lock_all_crtcs+0x3c/0x54) (drm_modeset_lock_all_crtcs+0x3c/0x54) from [<c029b54c>] (drm_atomic_helper_get_plane_state+0x40/0xc4) (drm_atomic_helper_get_plane_state+0x40/0xc4) from [<c02a2008>] (exynos_plane_set_property+0x3c/0xbc) (exynos_plane_set_property+0x3c/0xbc) from [<c028dcb8>] (drm_mode_plane_set_obj_prop+0x3c/0x4c) (drm_mode_plane_set_obj_prop+0x3c/0x4c) from [<c028dd08>] (drm_plane_force_disable+0x40/0x60) (drm_fb_helper_restore_fbdev_mode+0x80/0x158) from [<c029ee78>] (exynos_drm_fbdev_restore_mode+0x54/0x6c) (exynos_drm_fbdev_restore_mode+0x54/0x6c) from [<c029bc28>] (exynos_drm_lastclose+0x34/0x44) (exynos_drm_lastclose+0x34/0x44) from [<c0284bb0>] (drm_lastclose+0x44/0x158) (drm_lastclose+0x44/0x158) from [<c02857c0>] (drm_release+0x4c4/0x51c) (drm_release+0x4c4/0x51c) from [<c0101410>] (__fput+0x108/0x208)
Sean
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