[RFCv4 09/14] drm: convert plane to properties/state (original) (raw)

Sean Paul seanpaul at chromium.org
Mon Feb 10 09:45:42 PST 2014

On Fri, Feb 7, 2014 at 7:28 AM, Rob Clark <robdclark at gmail.com> wrote:

On Thu, Feb 6, 2014 at 9:53 PM, Sean Paul <seanpaul at chromium.org> wrote:

On Wed, Jan 29, 2014 at 8:44 AM, Rob Clark <robdclark at gmail.com> wrote:

On Tue, Jan 28, 2014 at 4:52 PM, Sean Paul <seanpaul at chromium.org> wrote:

On Mon, Nov 25, 2013 at 9:47 AM, Rob Clark <robdclark at 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->setproperty() 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 setproperty() 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/drmatomichelper.c | 156 ++++++++++- drivers/gpu/drm/drmcrtc.c | 397 +++++++++++++++++++--------- drivers/gpu/drm/drmfbhelper.c | 17 +- drivers/gpu/drm/exynos/exynosdrmcrtc.c | 4 +- drivers/gpu/drm/exynos/exynosdrmencoder.c | 6 +- drivers/gpu/drm/exynos/exynosdrmplane.c | 15 +- drivers/gpu/drm/i915/intelsprite.c | 21 +- drivers/gpu/drm/msm/mdp4/mdp4crtc.c | 2 +- drivers/gpu/drm/msm/mdp4/mdp4plane.c | 19 +- drivers/gpu/drm/nouveau/dispnv04/overlay.c | 8 +- drivers/gpu/drm/omapdrm/omapcrtc.c | 4 +- drivers/gpu/drm/omapdrm/omapdrv.c | 2 +- drivers/gpu/drm/omapdrm/omapplane.c | 33 ++- drivers/gpu/drm/rcar-du/rcarduplane.c | 8 +- drivers/gpu/drm/shmobile/shmobdrmcrtc.c | 2 +- drivers/gpu/drm/shmobile/shmobdrmplane.c | 6 +- drivers/gpu/drm/tegra/dc.c | 16 +- include/drm/drmatomichelper.h | 39 ++- include/drm/drmcrtc.h | 88 +++++- 19 files changed, 654 insertions(+), 189 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynosdrmplane.c b/drivers/gpu/drm/exynos/exynosdrmplane.c index 2e31fb8..d585a4c 100644 --- a/drivers/gpu/drm/exynos/exynosdrmplane.c +++ b/drivers/gpu/drm/exynos/exynosdrmplane.c @@ -10,6 +10,7 @@ */ #include <drm/drmP.h> +#include <drm/drmatomichelper.h> #include <drm/exynosdrm.h> #include "exynosdrmdrv.h" @@ -149,7 +150,7 @@ void exynosplanecommit(struct drmplane *plane) struct exynosplane *exynosplane = toexynosplane(plane); struct exynosdrmoverlay *overlay = &exynosplane->overlay; - exynosdrmfnencoder(plane->crtc, &overlay->zpos, + exynosdrmfnencoder(plane->state->crtc, &overlay->zpos, exynosdrmencoderplanecommit); } @@ -162,7 +163,7 @@ void exynosplanedpms(struct drmplane *plane, int mode) if (exynosplane->enabled) return; - exynosdrmfnencoder(plane->crtc, &overlay->zpos, + exynosdrmfnencoder(plane->state->crtc, &overlay->zpos, exynosdrmencoderplaneenable); exynosplane->enabled = true; @@ -170,7 +171,7 @@ void exynosplanedpms(struct drmplane *plane, int mode) if (!exynosplane->enabled) return; - exynosdrmfnencoder(plane->crtc, &overlay->zpos, + exynosdrmfnencoder(plane->state->crtc, &overlay->zpos, exynosdrmencoderplanedisable); exynosplane->enabled = false; @@ -192,7 +193,7 @@ exynosupdateplane(struct drmplane *plane, struct drmcrtc *crtc, if (ret < 0)_ _return ret;_ _- plane->crtc = crtc; + plane->state->crtc = crtc; exynosplanecommit(plane); exynosplanedpms(plane, DRMMODEDPMSON); @@ -225,13 +226,17 @@ static int exynosplanesetproperty(struct drmplane *plane, struct drmdevice *dev = plane->dev; struct exynosplane *exynosplane = toexynosplane(plane); struct exynosdrmprivate *devpriv = dev->devprivate; + struct drmplanestate *pstate = drmatomicgetplanestate(plane, state); Hi Rob, This seems to cause a deadlock. drmmodeobjsetpropertyioctl performs a drmmodesetlockall first, then drmatomichelpergetplanestate 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 drmfbhelperrestorefbdevmode path. (modesetlockstate+0x58/0x174) from [] (drmmodesetlock+0x20/0x30) (drmmodesetlock+0x20/0x30) from [] (drmmodesetlockallcrtcs+0x3c/0x54) (drmmodesetlockallcrtcs+0x3c/0x54) from [] (drmatomichelpergetplanestate+0x40/0xc4) (drmatomichelpergetplanestate+0x40/0xc4) from [] (exynosplanesetproperty+0x3c/0xbc) (exynosplanesetproperty+0x3c/0xbc) from [] (drmmodeplanesetobjprop+0x3c/0x4c) (drmmodeplanesetobjprop+0x3c/0x4c) from [] (drmplaneforcedisable+0x40/0x60) (drmfbhelperrestorefbdevmode+0x80/0x158) from [] (exynosdrmfbdevrestoremode+0x54/0x6c) (exynosdrmfbdevrestoremode+0x54/0x6c) from [] (exynosdrmlastclose+0x34/0x44) (exynosdrmlastclose+0x34/0x44) from [] (drmlastclose+0x44/0x158) (drmlastclose+0x44/0x158) from [] (drmrelease+0x4c4/0x51c) _(drmrelease+0x4c4/0x51c) from [] (fput+0x108/0x208) In theory the drmmodeset{lock,unlock}all() stuff should be removed from exynosdrmfbdevrestoremode().. perhaps I missed updating exynos?

Yeah, seems like it. IIRC, this was added during Daniel's locking changes, so maybe it was too new.

Sean

BR, -R

Sean

BR, -R Trace: _(schedule+0x4d8/0x71c) from [] (schedule+0x90/0x94) (schedule+0x90/0x94) from [] (schedulepreemptdisabled+0x18/0x1c) (schedulepreemptdisabled+0x18/0x1c) from [] _(wwmutexlockslowpath+0x208/0x2cc) _(wwmutexlockslowpath+0x208/0x2cc) from [] _(wwmutexlock+0x50/0xc4) _(wwmutexlock+0x50/0xc4) from [] (modesetlockstate+0x58/0x174) (modesetlockstate+0x58/0x174) from [] (drmmodesetlock+0x20/0x30) (drmmodesetlock+0x20/0x30) from [] (drmmodesetlockallcrtcs+0x3c/0x54) (drmmodesetlockallcrtcs+0x3c/0x54) from [] (drmatomichelpergetplanestate+0x40/0xc4) (drmatomichelpergetplanestate+0x40/0xc4) from [] (exynosplanesetproperty+0x3c/0xbc) (exynosplanesetproperty+0x3c/0xbc) from [] (drmmodeplanesetobjprop+0x3c/0x4c) (drmmodeplanesetobjprop+0x3c/0x4c) from [] (drmmodesetobjprop+0xac/0x128) (drmmodesetobjprop+0xac/0x128) from [] (drmmodeobjsetpropertyioctl+0xe4/0x15c) (drmmodeobjsetpropertyioctl+0xe4/0x15c) from [] (drmioctl+0x2e0/0x40c) (drmioctl+0x2e0/0x40c) from [] (dovfsioctl+0x508/0x5dc)

Sean

+ + if (ISERR(pstate)) + return PTRERR(pstate); if (property == devpriv->planezposproperty) { exynosplane->overlay.zpos = val; return 0; } - return -EINVAL; + return drmplanesetproperty(plane, pstate, property, val, blobdata); } static struct drmplanefuncs exynosplanefuncs = { -- 1.8.4.2

dri-devel mailing list dri-devel at lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel

More information about the dri-devel mailing list