* [PATCH v2 0/6] drm/msm: Switch to atomic helpers
@ 2018-03-28 19:06 Sean Paul
2018-03-28 19:06 ` Sean Paul
` (2 more replies)
0 siblings, 3 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: architt-sgV2jX0FEOL9JmXXK+q4OQ, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
robdclark-Re5JQEeQqe8AvxtiuMwx3w, Sean Paul,
jsanka-sgV2jX0FEOL9JmXXK+q4OQ, hoegsberg-F7+t8E8rja9g9hUCZPvPmw
I noticed a couple of rough edges yesterday as I was running some tests,
so here's v2 which fixes them. Most notably, I was experiencing issues when
the crtc was being disabled with abandoned events and vblank timeouts.
Note that I've bundled the private_obj change into this set since it
shares the same theme. It's technically v5, but I'll label it v2 with
the rest of the set.
PTAL,
Sean
Sean Paul (6):
drm/msm: Use drm_private_obj/state instead of subclassing
drm/msm: Refactor complete_commit() to look more the helpers
drm/msm: Mark the crtc->state->event consumed
drm/msm: Issue queued events when disabling crtc
drm/msm: Remove msm_commit/worker, use atomic helper commit
drm/msm: Switch to atomic_helper_commit()
drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 10 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 77 +++----
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 11 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c | 12 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c | 28 ++-
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 19 +-
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h | 4 +-
drivers/gpu/drm/msm/msm_atomic.c | 223 +--------------------
drivers/gpu/drm/msm/msm_drv.c | 95 ++++++++-
drivers/gpu/drm/msm/msm_drv.h | 10 +-
drivers/gpu/drm/msm/msm_kms.h | 21 +-
12 files changed, 204 insertions(+), 307 deletions(-)
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v2 1/6] drm/msm: Use drm_private_obj/state instead of subclassing
2018-03-28 19:06 [PATCH v2 0/6] drm/msm: Switch to atomic helpers Sean Paul
@ 2018-03-28 19:06 ` Sean Paul
[not found] ` <20180328190657.218661-1-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-03-28 19:06 ` Sean Paul
2 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno, linux-arm-msm, dri-devel; +Cc: linux-kernel, abhinavk, hoegsberg
Now that we have private state handled by the core, we can use those
instead of rolling our own swap_state for private data.
Originally posted here: https://patchwork.freedesktop.org/patch/211361/
Changes in v2:
- Use state->state in disp duplicate_state callback (Jeykumar)
Changes in v3:
- Update comment describing msm_kms_state (Jeykumar)
Changes in v4:
- Rebased on msm-next
- Don't always use private state from atomic state (Archit)
- Renamed some of the state accessors
- Tested on mdp5 db410c
Changes in v5:
- None
Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
Cc: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 77 ++++++++++---------
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 11 +--
drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c | 12 ++-
drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c | 28 ++++---
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 19 +++--
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h | 4 +-
drivers/gpu/drm/msm/msm_atomic.c | 37 ---------
drivers/gpu/drm/msm/msm_drv.c | 87 +++++++++++++++++++++-
drivers/gpu/drm/msm/msm_drv.h | 3 -
drivers/gpu/drm/msm/msm_kms.h | 21 ++++--
10 files changed, 183 insertions(+), 116 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 6d8e3a9a6fc0..366670043190 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -70,60 +70,62 @@ static int mdp5_hw_init(struct msm_kms *kms)
return 0;
}
-struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
+struct mdp5_state *mdp5_state_from_atomic(struct drm_atomic_state *state)
{
- struct msm_drm_private *priv = s->dev->dev_private;
- struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct msm_kms_state *state = to_kms_state(s);
- struct mdp5_state *new_state;
- int ret;
+ struct msm_kms_state *kms_state = msm_kms_state_from_atomic(state);
- if (state->state)
- return state->state;
+ if (IS_ERR_OR_NULL(kms_state))
+ return (struct mdp5_state *)kms_state; /* casting ERR_PTR */
- ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx);
- if (ret)
- return ERR_PTR(ret);
+ return kms_state->state;
+}
- new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
- if (!new_state)
- return ERR_PTR(-ENOMEM);
+struct mdp5_state *mdp5_state_from_dev(struct drm_device *dev)
+{
+ struct msm_kms_state *kms_state = msm_kms_state_from_dev(dev);
- /* Copy state: */
- new_state->hwpipe = mdp5_kms->state->hwpipe;
- new_state->hwmixer = mdp5_kms->state->hwmixer;
- if (mdp5_kms->smp)
- new_state->smp = mdp5_kms->state->smp;
+ if (IS_ERR_OR_NULL(kms_state))
+ return (struct mdp5_state *)kms_state; /* casting ERR_PTR */
- state->state = new_state;
+ return kms_state->state;
+}
+
+static void *mdp5_duplicate_state(void *state)
+{
+ if (!state)
+ return kzalloc(sizeof(struct mdp5_state), GFP_KERNEL);
- return new_state;
+ return kmemdup(state, sizeof(struct mdp5_state), GFP_KERNEL);
}
-static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state *state)
+static void mdp5_destroy_state(void *state)
{
- struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
- swap(to_kms_state(state)->state, mdp5_kms->state);
+ struct mdp5_state *mdp_state = (struct mdp5_state *)state;
+ kfree(mdp_state);
}
-static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *state)
+static void mdp5_prepare_commit(struct msm_kms *kms,
+ struct drm_atomic_state *old_state)
{
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
+ struct mdp5_state *mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
struct device *dev = &mdp5_kms->pdev->dev;
pm_runtime_get_sync(dev);
if (mdp5_kms->smp)
- mdp5_smp_prepare_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
+ mdp5_smp_prepare_commit(mdp5_kms->smp, &mdp5_state->smp);
}
-static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state *state)
+static void mdp5_complete_commit(struct msm_kms *kms,
+ struct drm_atomic_state *old_state)
{
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
+ struct mdp5_state *mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
struct device *dev = &mdp5_kms->pdev->dev;
if (mdp5_kms->smp)
- mdp5_smp_complete_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
+ mdp5_smp_complete_commit(mdp5_kms->smp, &mdp5_state->smp);
pm_runtime_put_sync(dev);
}
@@ -229,7 +231,8 @@ static const struct mdp_kms_funcs kms_funcs = {
.irq = mdp5_irq,
.enable_vblank = mdp5_enable_vblank,
.disable_vblank = mdp5_disable_vblank,
- .swap_state = mdp5_swap_state,
+ .duplicate_state = mdp5_duplicate_state,
+ .destroy_state = mdp5_destroy_state,
.prepare_commit = mdp5_prepare_commit,
.complete_commit = mdp5_complete_commit,
.wait_for_crtc_commit_done = mdp5_wait_for_crtc_commit_done,
@@ -726,8 +729,6 @@ static void mdp5_destroy(struct platform_device *pdev)
if (mdp5_kms->rpm_enabled)
pm_runtime_disable(&pdev->dev);
-
- kfree(mdp5_kms->state);
}
static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt,
@@ -859,7 +860,8 @@ static int interface_init(struct mdp5_kms *mdp5_kms)
return 0;
}
-static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
+static int mdp5_init(struct platform_device *pdev, struct drm_device *dev,
+ struct msm_kms_state *initial_state)
{
struct msm_drm_private *priv = dev->dev_private;
struct mdp5_kms *mdp5_kms;
@@ -880,9 +882,8 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
mdp5_kms->dev = dev;
mdp5_kms->pdev = pdev;
- drm_modeset_lock_init(&mdp5_kms->state_lock);
- mdp5_kms->state = kzalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
- if (!mdp5_kms->state) {
+ initial_state->state = mdp5_duplicate_state(NULL);
+ if (!initial_state->state) {
ret = -ENOMEM;
goto fail;
}
@@ -940,7 +941,8 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
* this section initializes the SMP:
*/
if (mdp5_kms->caps & MDP_CAP_SMP) {
- mdp5_kms->smp = mdp5_smp_init(mdp5_kms, &config->hw->smp);
+ mdp5_kms->smp = mdp5_smp_init(mdp5_kms, &config->hw->smp,
+ initial_state->state);
if (IS_ERR(mdp5_kms->smp)) {
ret = PTR_ERR(mdp5_kms->smp);
mdp5_kms->smp = NULL;
@@ -980,10 +982,11 @@ static int mdp5_bind(struct device *dev, struct device *master, void *data)
{
struct drm_device *ddev = dev_get_drvdata(master);
struct platform_device *pdev = to_platform_device(dev);
+ struct msm_kms_state *initial_state = (struct msm_kms_state *)data;
DBG("");
- return mdp5_init(pdev, ddev);
+ return mdp5_init(pdev, ddev, initial_state);
}
static void mdp5_unbind(struct device *dev, struct device *master,
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
index 425a03d213e5..e23117b82aad 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
@@ -28,8 +28,6 @@
#include "mdp5_ctl.h"
#include "mdp5_smp.h"
-struct mdp5_state;
-
struct mdp5_kms {
struct mdp_kms base;
@@ -49,12 +47,6 @@ struct mdp5_kms {
struct mdp5_cfg_handler *cfg;
uint32_t caps; /* MDP capabilities (MDP_CAP_XXX bits) */
- /**
- * Global atomic state. Do not access directly, use mdp5_get_state()
- */
- struct mdp5_state *state;
- struct drm_modeset_lock state_lock;
-
struct mdp5_smp *smp;
struct mdp5_ctl_manager *ctlm;
@@ -93,7 +85,8 @@ struct mdp5_state {
};
struct mdp5_state *__must_check
-mdp5_get_state(struct drm_atomic_state *s);
+mdp5_state_from_atomic(struct drm_atomic_state *s);
+struct mdp5_state *__must_check mdp5_state_from_dev(struct drm_device *dev);
/* Atomic plane state. Subclasses the base drm_plane_state in order to
* track assigned hwpipe and hw specific state.
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
index 8a00991f03c7..fdd5e12a9d56 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
@@ -52,10 +52,11 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc,
{
struct msm_drm_private *priv = s->dev->dev_private;
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct mdp5_state *state = mdp5_get_state(s);
+ struct mdp5_state *state;
struct mdp5_hw_mixer_state *new_state;
int i;
+ state = mdp5_state_from_atomic(s);
if (IS_ERR(state))
return PTR_ERR(state);
@@ -129,12 +130,17 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc,
void mdp5_mixer_release(struct drm_atomic_state *s, struct mdp5_hw_mixer *mixer)
{
- struct mdp5_state *state = mdp5_get_state(s);
- struct mdp5_hw_mixer_state *new_state = &state->hwmixer;
+ struct mdp5_state *state;
+ struct mdp5_hw_mixer_state *new_state;
if (!mixer)
return;
+ state = mdp5_state_from_atomic(s);
+ if (IS_ERR(state))
+ return;
+
+ new_state = &state->hwmixer;
if (WARN_ON(!new_state->hwmixer_to_crtc[mixer->idx]))
return;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
index ff52c49095f9..dc66ab9fdd50 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
@@ -24,17 +24,20 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
{
struct msm_drm_private *priv = s->dev->dev_private;
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct mdp5_state *state;
+ struct mdp5_state *old_mdp5_state, *new_mdp5_state;
struct mdp5_hw_pipe_state *old_state, *new_state;
int i, j;
- state = mdp5_get_state(s);
- if (IS_ERR(state))
- return PTR_ERR(state);
+ new_mdp5_state = mdp5_state_from_atomic(s);
+ if (IS_ERR(new_mdp5_state))
+ return PTR_ERR(new_mdp5_state);
- /* grab old_state after mdp5_get_state(), since now we hold lock: */
- old_state = &mdp5_kms->state->hwpipe;
- new_state = &state->hwpipe;
+ old_mdp5_state = mdp5_state_from_dev(s->dev);
+ if (IS_ERR(old_mdp5_state))
+ return PTR_ERR(old_mdp5_state);
+
+ old_state = &old_mdp5_state->hwpipe;
+ new_state = &new_mdp5_state->hwpipe;
for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
struct mdp5_hw_pipe *cur = mdp5_kms->hwpipes[i];
@@ -107,7 +110,7 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
WARN_ON(r_hwpipe);
DBG("%s: alloc SMP blocks", (*hwpipe)->name);
- ret = mdp5_smp_assign(mdp5_kms->smp, &state->smp,
+ ret = mdp5_smp_assign(mdp5_kms->smp, &new_mdp5_state->smp,
(*hwpipe)->pipe, blkcfg);
if (ret)
return -ENOMEM;
@@ -132,12 +135,17 @@ void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe)
{
struct msm_drm_private *priv = s->dev->dev_private;
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct mdp5_state *state = mdp5_get_state(s);
- struct mdp5_hw_pipe_state *new_state = &state->hwpipe;
+ struct mdp5_state *state;
+ struct mdp5_hw_pipe_state *new_state;
if (!hwpipe)
return;
+ state = mdp5_state_from_atomic(s);
+ if (IS_ERR(state))
+ return;
+
+ new_state = &state->hwpipe;
if (WARN_ON(!new_state->hwpipe_to_plane[hwpipe->idx]))
return;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
index ae4983d9d0a5..918e4123e781 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
@@ -338,6 +338,7 @@ void mdp5_smp_complete_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state
void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
{
struct mdp5_kms *mdp5_kms = get_kms(smp);
+ struct mdp5_state *mdp5_state;
struct mdp5_hw_pipe_state *hwpstate;
struct mdp5_smp_state *state;
int total = 0, i, j;
@@ -346,11 +347,12 @@ void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
drm_printf(p, "----\t-----\t-----\n");
if (drm_can_sleep())
- drm_modeset_lock(&mdp5_kms->state_lock, NULL);
+ drm_modeset_lock_all(mdp5_kms->dev);
- /* grab these *after* we hold the state_lock */
- hwpstate = &mdp5_kms->state->hwpipe;
- state = &mdp5_kms->state->smp;
+ /* grab these *after* we hold the modeset_lock */
+ mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
+ hwpstate = &mdp5_state->hwpipe;
+ state = &mdp5_state->smp;
for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
struct mdp5_hw_pipe *hwpipe = mdp5_kms->hwpipes[i];
@@ -374,7 +376,7 @@ void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
bitmap_weight(state->state, smp->blk_cnt));
if (drm_can_sleep())
- drm_modeset_unlock(&mdp5_kms->state_lock);
+ drm_modeset_unlock_all(mdp5_kms->dev);
}
void mdp5_smp_destroy(struct mdp5_smp *smp)
@@ -382,12 +384,15 @@ void mdp5_smp_destroy(struct mdp5_smp *smp)
kfree(smp);
}
-struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms, const struct mdp5_smp_block *cfg)
+struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms,
+ const struct mdp5_smp_block *cfg,
+ struct mdp5_state *mdp5_state)
{
- struct mdp5_smp_state *state = &mdp5_kms->state->smp;
+ struct mdp5_smp_state *state;
struct mdp5_smp *smp = NULL;
int ret;
+ state = &mdp5_state->smp;
smp = kzalloc(sizeof(*smp), GFP_KERNEL);
if (unlikely(!smp)) {
ret = -ENOMEM;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
index b41d0448fbe8..1bfa22b63a2d 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
@@ -69,6 +69,7 @@ struct mdp5_smp_state {
};
struct mdp5_kms;
+struct mdp5_state;
struct mdp5_smp;
/*
@@ -78,7 +79,8 @@ struct mdp5_smp;
*/
struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms,
- const struct mdp5_smp_block *cfg);
+ const struct mdp5_smp_block *cfg,
+ struct mdp5_state *mdp5_state);
void mdp5_smp_destroy(struct mdp5_smp *smp);
void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p);
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index bf5f8c39f34d..e792158676aa 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -220,16 +220,6 @@ int msm_atomic_commit(struct drm_device *dev,
BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
- /*
- * This is the point of no return - everything below never fails except
- * when the hw goes bonghits. Which means we can commit the new state on
- * the software side now.
- *
- * swap driver private state while still holding state_lock
- */
- if (to_kms_state(state)->state)
- priv->kms->funcs->swap_state(priv->kms, state);
-
/*
* Everything below can be run asynchronously without the need to grab
* any modeset locks at all under one conditions: It must be guaranteed
@@ -262,30 +252,3 @@ int msm_atomic_commit(struct drm_device *dev,
drm_atomic_helper_cleanup_planes(dev, state);
return ret;
}
-
-struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev)
-{
- struct msm_kms_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
-
- if (!state || drm_atomic_state_init(dev, &state->base) < 0) {
- kfree(state);
- return NULL;
- }
-
- return &state->base;
-}
-
-void msm_atomic_state_clear(struct drm_atomic_state *s)
-{
- struct msm_kms_state *state = to_kms_state(s);
- drm_atomic_state_default_clear(&state->base);
- kfree(state->state);
- state->state = NULL;
-}
-
-void msm_atomic_state_free(struct drm_atomic_state *state)
-{
- kfree(to_kms_state(state)->state);
- drm_atomic_state_default_release(state);
- kfree(state);
-}
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 30cd514d8f7c..7e8f640062ef 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -42,11 +42,79 @@ static const struct drm_mode_config_funcs mode_config_funcs = {
.output_poll_changed = drm_fb_helper_output_poll_changed,
.atomic_check = drm_atomic_helper_check,
.atomic_commit = msm_atomic_commit,
- .atomic_state_alloc = msm_atomic_state_alloc,
- .atomic_state_clear = msm_atomic_state_clear,
- .atomic_state_free = msm_atomic_state_free,
};
+static inline
+struct msm_kms *msm_kms_from_obj(struct drm_private_obj *obj)
+{
+ return container_of(obj, struct msm_kms, base);
+}
+
+static inline
+struct msm_kms_state *msm_kms_state_from_priv(struct drm_private_state *priv)
+{
+ return container_of(priv, struct msm_kms_state, base);
+}
+
+
+struct msm_kms_state *msm_kms_state_from_atomic(struct drm_atomic_state *state)
+{
+ struct msm_drm_private *priv = state->dev->dev_private;
+ struct drm_private_obj *obj = &priv->kms->base;
+ struct drm_private_state *priv_state;
+
+ priv_state = drm_atomic_get_private_obj_state(state, obj);
+ if (IS_ERR_OR_NULL(priv_state))
+ return (struct msm_kms_state *)state; /* casting ERR_PTR */
+
+ return msm_kms_state_from_priv(priv_state);
+}
+
+struct msm_kms_state *msm_kms_state_from_dev(struct drm_device *dev)
+{
+ struct msm_drm_private *priv = dev->dev_private;
+ struct drm_private_obj *obj = &priv->kms->base;
+ struct drm_private_state *priv_state = obj->state;
+
+ return msm_kms_state_from_priv(priv_state);
+}
+
+static struct drm_private_state *
+msm_kms_duplicate_state(struct drm_private_obj *obj)
+{
+ struct msm_kms *kms = msm_kms_from_obj(obj);
+ struct msm_kms_state *state = msm_kms_state_from_priv(obj->state);
+ struct msm_kms_state *new_state;
+
+ new_state = kzalloc(sizeof(*new_state), GFP_KERNEL);
+ if (!new_state)
+ return NULL;
+
+ if (kms->funcs->duplicate_state)
+ new_state->state = kms->funcs->duplicate_state(state->state);
+
+ __drm_atomic_helper_private_obj_duplicate_state(obj, &new_state->base);
+
+ return &new_state->base;
+}
+
+static void msm_kms_destroy_state(struct drm_private_obj *obj,
+ struct drm_private_state *priv_state)
+{
+ struct msm_kms *kms = msm_kms_from_obj(obj);
+ struct msm_kms_state *state = msm_kms_state_from_priv(priv_state);
+
+ if (kms->funcs->destroy_state)
+ kms->funcs->destroy_state(state->state);
+ kfree(state);
+}
+
+static const struct drm_private_state_funcs kms_state_funcs = {
+ .atomic_duplicate_state = msm_kms_duplicate_state,
+ .atomic_destroy_state = msm_kms_destroy_state,
+};
+
+
#ifdef CONFIG_DRM_MSM_REGISTER_LOGGING
static bool reglog = false;
MODULE_PARM_DESC(reglog, "Enable register read/write logging");
@@ -245,6 +313,8 @@ static int msm_drm_uninit(struct device *dev)
flush_workqueue(priv->atomic_wq);
destroy_workqueue(priv->atomic_wq);
+ drm_atomic_private_obj_fini(&kms->base);
+
if (kms && kms->funcs)
kms->funcs->destroy(kms);
@@ -356,6 +426,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
struct drm_device *ddev;
struct msm_drm_private *priv;
struct msm_kms *kms;
+ struct msm_kms_state *initial_state;
int ret;
ddev = drm_dev_alloc(drv, dev);
@@ -364,6 +435,10 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
return PTR_ERR(ddev);
}
+ initial_state = kzalloc(sizeof(*initial_state), GFP_KERNEL);
+ if (!initial_state)
+ return -ENOMEM;
+
platform_set_drvdata(pdev, ddev);
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
@@ -394,7 +469,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
drm_mode_config_init(ddev);
/* Bind all our sub-components: */
- ret = component_bind_all(dev, ddev);
+ ret = component_bind_all(dev, initial_state);
if (ret) {
msm_mdss_destroy(ddev);
kfree(priv);
@@ -433,6 +508,10 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
goto fail;
}
+ drm_atomic_private_obj_init(&kms->base,
+ &initial_state->base,
+ &kms_state_funcs);
+
if (kms) {
ret = kms->funcs->hw_init(kms);
if (ret) {
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 48ed5b9a8580..c5b8c989b859 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -162,9 +162,6 @@ struct msm_format {
int msm_atomic_commit(struct drm_device *dev,
struct drm_atomic_state *state, bool nonblock);
-struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev);
-void msm_atomic_state_clear(struct drm_atomic_state *state);
-void msm_atomic_state_free(struct drm_atomic_state *state);
void msm_gem_unmap_vma(struct msm_gem_address_space *aspace,
struct msm_gem_vma *vma, struct sg_table *sgt);
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 17d5824417ad..24d09fcebf16 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -42,6 +42,8 @@ struct msm_kms_funcs {
void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc);
/* swap global atomic state: */
void (*swap_state)(struct msm_kms *kms, struct drm_atomic_state *state);
+ void *(*duplicate_state)(void *state);
+ void (*destroy_state)(void *state);
/* modeset, bracketing atomic_commit(): */
void (*prepare_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
void (*complete_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
@@ -68,6 +70,8 @@ struct msm_kms_funcs {
};
struct msm_kms {
+ struct drm_private_obj base;
+
const struct msm_kms_funcs *funcs;
/* irq number to be passed on to drm_irq_install */
@@ -78,16 +82,23 @@ struct msm_kms {
};
/**
- * Subclass of drm_atomic_state, to allow kms backend to have driver
+ * Subclass of drm_private_state, to allow kms backend to have driver
* private global state. The kms backend can do whatever it wants
- * with the ->state ptr. On ->atomic_state_clear() the ->state ptr
- * is kfree'd and set back to NULL.
+ * with the ->state ptr.
*/
struct msm_kms_state {
- struct drm_atomic_state base;
+ struct drm_private_state base;
void *state;
};
-#define to_kms_state(x) container_of(x, struct msm_kms_state, base)
+
+/**
+ * Extracts the msm_kms_state from either an atomic state, or the current
+ * device. One or the other might be desirable depending on whether you want the
+ * currently configured msm_kms_state (from_dev), or you would like the state
+ * which is about to be applied (from_atomic).
+ */
+struct msm_kms_state *msm_kms_state_from_dev(struct drm_device *dev);
+struct msm_kms_state *msm_kms_state_from_atomic(struct drm_atomic_state *state);
static inline void msm_kms_init(struct msm_kms *kms,
const struct msm_kms_funcs *funcs)
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 1/6] drm/msm: Use drm_private_obj/state instead of subclassing
@ 2018-03-28 19:06 ` Sean Paul
0 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno, linux-arm-msm, dri-devel
Cc: robdclark, hoegsberg, jsanka, abhinavk, architt, Sean Paul, linux-kernel
Now that we have private state handled by the core, we can use those
instead of rolling our own swap_state for private data.
Originally posted here: https://patchwork.freedesktop.org/patch/211361/
Changes in v2:
- Use state->state in disp duplicate_state callback (Jeykumar)
Changes in v3:
- Update comment describing msm_kms_state (Jeykumar)
Changes in v4:
- Rebased on msm-next
- Don't always use private state from atomic state (Archit)
- Renamed some of the state accessors
- Tested on mdp5 db410c
Changes in v5:
- None
Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
Cc: Archit Taneja <architt@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 77 ++++++++++---------
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 11 +--
drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c | 12 ++-
drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c | 28 ++++---
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 19 +++--
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h | 4 +-
drivers/gpu/drm/msm/msm_atomic.c | 37 ---------
drivers/gpu/drm/msm/msm_drv.c | 87 +++++++++++++++++++++-
drivers/gpu/drm/msm/msm_drv.h | 3 -
drivers/gpu/drm/msm/msm_kms.h | 21 ++++--
10 files changed, 183 insertions(+), 116 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 6d8e3a9a6fc0..366670043190 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -70,60 +70,62 @@ static int mdp5_hw_init(struct msm_kms *kms)
return 0;
}
-struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
+struct mdp5_state *mdp5_state_from_atomic(struct drm_atomic_state *state)
{
- struct msm_drm_private *priv = s->dev->dev_private;
- struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct msm_kms_state *state = to_kms_state(s);
- struct mdp5_state *new_state;
- int ret;
+ struct msm_kms_state *kms_state = msm_kms_state_from_atomic(state);
- if (state->state)
- return state->state;
+ if (IS_ERR_OR_NULL(kms_state))
+ return (struct mdp5_state *)kms_state; /* casting ERR_PTR */
- ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx);
- if (ret)
- return ERR_PTR(ret);
+ return kms_state->state;
+}
- new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
- if (!new_state)
- return ERR_PTR(-ENOMEM);
+struct mdp5_state *mdp5_state_from_dev(struct drm_device *dev)
+{
+ struct msm_kms_state *kms_state = msm_kms_state_from_dev(dev);
- /* Copy state: */
- new_state->hwpipe = mdp5_kms->state->hwpipe;
- new_state->hwmixer = mdp5_kms->state->hwmixer;
- if (mdp5_kms->smp)
- new_state->smp = mdp5_kms->state->smp;
+ if (IS_ERR_OR_NULL(kms_state))
+ return (struct mdp5_state *)kms_state; /* casting ERR_PTR */
- state->state = new_state;
+ return kms_state->state;
+}
+
+static void *mdp5_duplicate_state(void *state)
+{
+ if (!state)
+ return kzalloc(sizeof(struct mdp5_state), GFP_KERNEL);
- return new_state;
+ return kmemdup(state, sizeof(struct mdp5_state), GFP_KERNEL);
}
-static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state *state)
+static void mdp5_destroy_state(void *state)
{
- struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
- swap(to_kms_state(state)->state, mdp5_kms->state);
+ struct mdp5_state *mdp_state = (struct mdp5_state *)state;
+ kfree(mdp_state);
}
-static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *state)
+static void mdp5_prepare_commit(struct msm_kms *kms,
+ struct drm_atomic_state *old_state)
{
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
+ struct mdp5_state *mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
struct device *dev = &mdp5_kms->pdev->dev;
pm_runtime_get_sync(dev);
if (mdp5_kms->smp)
- mdp5_smp_prepare_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
+ mdp5_smp_prepare_commit(mdp5_kms->smp, &mdp5_state->smp);
}
-static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state *state)
+static void mdp5_complete_commit(struct msm_kms *kms,
+ struct drm_atomic_state *old_state)
{
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
+ struct mdp5_state *mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
struct device *dev = &mdp5_kms->pdev->dev;
if (mdp5_kms->smp)
- mdp5_smp_complete_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
+ mdp5_smp_complete_commit(mdp5_kms->smp, &mdp5_state->smp);
pm_runtime_put_sync(dev);
}
@@ -229,7 +231,8 @@ static const struct mdp_kms_funcs kms_funcs = {
.irq = mdp5_irq,
.enable_vblank = mdp5_enable_vblank,
.disable_vblank = mdp5_disable_vblank,
- .swap_state = mdp5_swap_state,
+ .duplicate_state = mdp5_duplicate_state,
+ .destroy_state = mdp5_destroy_state,
.prepare_commit = mdp5_prepare_commit,
.complete_commit = mdp5_complete_commit,
.wait_for_crtc_commit_done = mdp5_wait_for_crtc_commit_done,
@@ -726,8 +729,6 @@ static void mdp5_destroy(struct platform_device *pdev)
if (mdp5_kms->rpm_enabled)
pm_runtime_disable(&pdev->dev);
-
- kfree(mdp5_kms->state);
}
static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt,
@@ -859,7 +860,8 @@ static int interface_init(struct mdp5_kms *mdp5_kms)
return 0;
}
-static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
+static int mdp5_init(struct platform_device *pdev, struct drm_device *dev,
+ struct msm_kms_state *initial_state)
{
struct msm_drm_private *priv = dev->dev_private;
struct mdp5_kms *mdp5_kms;
@@ -880,9 +882,8 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
mdp5_kms->dev = dev;
mdp5_kms->pdev = pdev;
- drm_modeset_lock_init(&mdp5_kms->state_lock);
- mdp5_kms->state = kzalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
- if (!mdp5_kms->state) {
+ initial_state->state = mdp5_duplicate_state(NULL);
+ if (!initial_state->state) {
ret = -ENOMEM;
goto fail;
}
@@ -940,7 +941,8 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
* this section initializes the SMP:
*/
if (mdp5_kms->caps & MDP_CAP_SMP) {
- mdp5_kms->smp = mdp5_smp_init(mdp5_kms, &config->hw->smp);
+ mdp5_kms->smp = mdp5_smp_init(mdp5_kms, &config->hw->smp,
+ initial_state->state);
if (IS_ERR(mdp5_kms->smp)) {
ret = PTR_ERR(mdp5_kms->smp);
mdp5_kms->smp = NULL;
@@ -980,10 +982,11 @@ static int mdp5_bind(struct device *dev, struct device *master, void *data)
{
struct drm_device *ddev = dev_get_drvdata(master);
struct platform_device *pdev = to_platform_device(dev);
+ struct msm_kms_state *initial_state = (struct msm_kms_state *)data;
DBG("");
- return mdp5_init(pdev, ddev);
+ return mdp5_init(pdev, ddev, initial_state);
}
static void mdp5_unbind(struct device *dev, struct device *master,
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
index 425a03d213e5..e23117b82aad 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
@@ -28,8 +28,6 @@
#include "mdp5_ctl.h"
#include "mdp5_smp.h"
-struct mdp5_state;
-
struct mdp5_kms {
struct mdp_kms base;
@@ -49,12 +47,6 @@ struct mdp5_kms {
struct mdp5_cfg_handler *cfg;
uint32_t caps; /* MDP capabilities (MDP_CAP_XXX bits) */
- /**
- * Global atomic state. Do not access directly, use mdp5_get_state()
- */
- struct mdp5_state *state;
- struct drm_modeset_lock state_lock;
-
struct mdp5_smp *smp;
struct mdp5_ctl_manager *ctlm;
@@ -93,7 +85,8 @@ struct mdp5_state {
};
struct mdp5_state *__must_check
-mdp5_get_state(struct drm_atomic_state *s);
+mdp5_state_from_atomic(struct drm_atomic_state *s);
+struct mdp5_state *__must_check mdp5_state_from_dev(struct drm_device *dev);
/* Atomic plane state. Subclasses the base drm_plane_state in order to
* track assigned hwpipe and hw specific state.
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
index 8a00991f03c7..fdd5e12a9d56 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
@@ -52,10 +52,11 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc,
{
struct msm_drm_private *priv = s->dev->dev_private;
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct mdp5_state *state = mdp5_get_state(s);
+ struct mdp5_state *state;
struct mdp5_hw_mixer_state *new_state;
int i;
+ state = mdp5_state_from_atomic(s);
if (IS_ERR(state))
return PTR_ERR(state);
@@ -129,12 +130,17 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc,
void mdp5_mixer_release(struct drm_atomic_state *s, struct mdp5_hw_mixer *mixer)
{
- struct mdp5_state *state = mdp5_get_state(s);
- struct mdp5_hw_mixer_state *new_state = &state->hwmixer;
+ struct mdp5_state *state;
+ struct mdp5_hw_mixer_state *new_state;
if (!mixer)
return;
+ state = mdp5_state_from_atomic(s);
+ if (IS_ERR(state))
+ return;
+
+ new_state = &state->hwmixer;
if (WARN_ON(!new_state->hwmixer_to_crtc[mixer->idx]))
return;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
index ff52c49095f9..dc66ab9fdd50 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
@@ -24,17 +24,20 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
{
struct msm_drm_private *priv = s->dev->dev_private;
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct mdp5_state *state;
+ struct mdp5_state *old_mdp5_state, *new_mdp5_state;
struct mdp5_hw_pipe_state *old_state, *new_state;
int i, j;
- state = mdp5_get_state(s);
- if (IS_ERR(state))
- return PTR_ERR(state);
+ new_mdp5_state = mdp5_state_from_atomic(s);
+ if (IS_ERR(new_mdp5_state))
+ return PTR_ERR(new_mdp5_state);
- /* grab old_state after mdp5_get_state(), since now we hold lock: */
- old_state = &mdp5_kms->state->hwpipe;
- new_state = &state->hwpipe;
+ old_mdp5_state = mdp5_state_from_dev(s->dev);
+ if (IS_ERR(old_mdp5_state))
+ return PTR_ERR(old_mdp5_state);
+
+ old_state = &old_mdp5_state->hwpipe;
+ new_state = &new_mdp5_state->hwpipe;
for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
struct mdp5_hw_pipe *cur = mdp5_kms->hwpipes[i];
@@ -107,7 +110,7 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
WARN_ON(r_hwpipe);
DBG("%s: alloc SMP blocks", (*hwpipe)->name);
- ret = mdp5_smp_assign(mdp5_kms->smp, &state->smp,
+ ret = mdp5_smp_assign(mdp5_kms->smp, &new_mdp5_state->smp,
(*hwpipe)->pipe, blkcfg);
if (ret)
return -ENOMEM;
@@ -132,12 +135,17 @@ void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe)
{
struct msm_drm_private *priv = s->dev->dev_private;
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct mdp5_state *state = mdp5_get_state(s);
- struct mdp5_hw_pipe_state *new_state = &state->hwpipe;
+ struct mdp5_state *state;
+ struct mdp5_hw_pipe_state *new_state;
if (!hwpipe)
return;
+ state = mdp5_state_from_atomic(s);
+ if (IS_ERR(state))
+ return;
+
+ new_state = &state->hwpipe;
if (WARN_ON(!new_state->hwpipe_to_plane[hwpipe->idx]))
return;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
index ae4983d9d0a5..918e4123e781 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
@@ -338,6 +338,7 @@ void mdp5_smp_complete_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state
void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
{
struct mdp5_kms *mdp5_kms = get_kms(smp);
+ struct mdp5_state *mdp5_state;
struct mdp5_hw_pipe_state *hwpstate;
struct mdp5_smp_state *state;
int total = 0, i, j;
@@ -346,11 +347,12 @@ void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
drm_printf(p, "----\t-----\t-----\n");
if (drm_can_sleep())
- drm_modeset_lock(&mdp5_kms->state_lock, NULL);
+ drm_modeset_lock_all(mdp5_kms->dev);
- /* grab these *after* we hold the state_lock */
- hwpstate = &mdp5_kms->state->hwpipe;
- state = &mdp5_kms->state->smp;
+ /* grab these *after* we hold the modeset_lock */
+ mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
+ hwpstate = &mdp5_state->hwpipe;
+ state = &mdp5_state->smp;
for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
struct mdp5_hw_pipe *hwpipe = mdp5_kms->hwpipes[i];
@@ -374,7 +376,7 @@ void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
bitmap_weight(state->state, smp->blk_cnt));
if (drm_can_sleep())
- drm_modeset_unlock(&mdp5_kms->state_lock);
+ drm_modeset_unlock_all(mdp5_kms->dev);
}
void mdp5_smp_destroy(struct mdp5_smp *smp)
@@ -382,12 +384,15 @@ void mdp5_smp_destroy(struct mdp5_smp *smp)
kfree(smp);
}
-struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms, const struct mdp5_smp_block *cfg)
+struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms,
+ const struct mdp5_smp_block *cfg,
+ struct mdp5_state *mdp5_state)
{
- struct mdp5_smp_state *state = &mdp5_kms->state->smp;
+ struct mdp5_smp_state *state;
struct mdp5_smp *smp = NULL;
int ret;
+ state = &mdp5_state->smp;
smp = kzalloc(sizeof(*smp), GFP_KERNEL);
if (unlikely(!smp)) {
ret = -ENOMEM;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
index b41d0448fbe8..1bfa22b63a2d 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
@@ -69,6 +69,7 @@ struct mdp5_smp_state {
};
struct mdp5_kms;
+struct mdp5_state;
struct mdp5_smp;
/*
@@ -78,7 +79,8 @@ struct mdp5_smp;
*/
struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms,
- const struct mdp5_smp_block *cfg);
+ const struct mdp5_smp_block *cfg,
+ struct mdp5_state *mdp5_state);
void mdp5_smp_destroy(struct mdp5_smp *smp);
void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p);
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index bf5f8c39f34d..e792158676aa 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -220,16 +220,6 @@ int msm_atomic_commit(struct drm_device *dev,
BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
- /*
- * This is the point of no return - everything below never fails except
- * when the hw goes bonghits. Which means we can commit the new state on
- * the software side now.
- *
- * swap driver private state while still holding state_lock
- */
- if (to_kms_state(state)->state)
- priv->kms->funcs->swap_state(priv->kms, state);
-
/*
* Everything below can be run asynchronously without the need to grab
* any modeset locks at all under one conditions: It must be guaranteed
@@ -262,30 +252,3 @@ int msm_atomic_commit(struct drm_device *dev,
drm_atomic_helper_cleanup_planes(dev, state);
return ret;
}
-
-struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev)
-{
- struct msm_kms_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
-
- if (!state || drm_atomic_state_init(dev, &state->base) < 0) {
- kfree(state);
- return NULL;
- }
-
- return &state->base;
-}
-
-void msm_atomic_state_clear(struct drm_atomic_state *s)
-{
- struct msm_kms_state *state = to_kms_state(s);
- drm_atomic_state_default_clear(&state->base);
- kfree(state->state);
- state->state = NULL;
-}
-
-void msm_atomic_state_free(struct drm_atomic_state *state)
-{
- kfree(to_kms_state(state)->state);
- drm_atomic_state_default_release(state);
- kfree(state);
-}
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 30cd514d8f7c..7e8f640062ef 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -42,11 +42,79 @@ static const struct drm_mode_config_funcs mode_config_funcs = {
.output_poll_changed = drm_fb_helper_output_poll_changed,
.atomic_check = drm_atomic_helper_check,
.atomic_commit = msm_atomic_commit,
- .atomic_state_alloc = msm_atomic_state_alloc,
- .atomic_state_clear = msm_atomic_state_clear,
- .atomic_state_free = msm_atomic_state_free,
};
+static inline
+struct msm_kms *msm_kms_from_obj(struct drm_private_obj *obj)
+{
+ return container_of(obj, struct msm_kms, base);
+}
+
+static inline
+struct msm_kms_state *msm_kms_state_from_priv(struct drm_private_state *priv)
+{
+ return container_of(priv, struct msm_kms_state, base);
+}
+
+
+struct msm_kms_state *msm_kms_state_from_atomic(struct drm_atomic_state *state)
+{
+ struct msm_drm_private *priv = state->dev->dev_private;
+ struct drm_private_obj *obj = &priv->kms->base;
+ struct drm_private_state *priv_state;
+
+ priv_state = drm_atomic_get_private_obj_state(state, obj);
+ if (IS_ERR_OR_NULL(priv_state))
+ return (struct msm_kms_state *)state; /* casting ERR_PTR */
+
+ return msm_kms_state_from_priv(priv_state);
+}
+
+struct msm_kms_state *msm_kms_state_from_dev(struct drm_device *dev)
+{
+ struct msm_drm_private *priv = dev->dev_private;
+ struct drm_private_obj *obj = &priv->kms->base;
+ struct drm_private_state *priv_state = obj->state;
+
+ return msm_kms_state_from_priv(priv_state);
+}
+
+static struct drm_private_state *
+msm_kms_duplicate_state(struct drm_private_obj *obj)
+{
+ struct msm_kms *kms = msm_kms_from_obj(obj);
+ struct msm_kms_state *state = msm_kms_state_from_priv(obj->state);
+ struct msm_kms_state *new_state;
+
+ new_state = kzalloc(sizeof(*new_state), GFP_KERNEL);
+ if (!new_state)
+ return NULL;
+
+ if (kms->funcs->duplicate_state)
+ new_state->state = kms->funcs->duplicate_state(state->state);
+
+ __drm_atomic_helper_private_obj_duplicate_state(obj, &new_state->base);
+
+ return &new_state->base;
+}
+
+static void msm_kms_destroy_state(struct drm_private_obj *obj,
+ struct drm_private_state *priv_state)
+{
+ struct msm_kms *kms = msm_kms_from_obj(obj);
+ struct msm_kms_state *state = msm_kms_state_from_priv(priv_state);
+
+ if (kms->funcs->destroy_state)
+ kms->funcs->destroy_state(state->state);
+ kfree(state);
+}
+
+static const struct drm_private_state_funcs kms_state_funcs = {
+ .atomic_duplicate_state = msm_kms_duplicate_state,
+ .atomic_destroy_state = msm_kms_destroy_state,
+};
+
+
#ifdef CONFIG_DRM_MSM_REGISTER_LOGGING
static bool reglog = false;
MODULE_PARM_DESC(reglog, "Enable register read/write logging");
@@ -245,6 +313,8 @@ static int msm_drm_uninit(struct device *dev)
flush_workqueue(priv->atomic_wq);
destroy_workqueue(priv->atomic_wq);
+ drm_atomic_private_obj_fini(&kms->base);
+
if (kms && kms->funcs)
kms->funcs->destroy(kms);
@@ -356,6 +426,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
struct drm_device *ddev;
struct msm_drm_private *priv;
struct msm_kms *kms;
+ struct msm_kms_state *initial_state;
int ret;
ddev = drm_dev_alloc(drv, dev);
@@ -364,6 +435,10 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
return PTR_ERR(ddev);
}
+ initial_state = kzalloc(sizeof(*initial_state), GFP_KERNEL);
+ if (!initial_state)
+ return -ENOMEM;
+
platform_set_drvdata(pdev, ddev);
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
@@ -394,7 +469,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
drm_mode_config_init(ddev);
/* Bind all our sub-components: */
- ret = component_bind_all(dev, ddev);
+ ret = component_bind_all(dev, initial_state);
if (ret) {
msm_mdss_destroy(ddev);
kfree(priv);
@@ -433,6 +508,10 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
goto fail;
}
+ drm_atomic_private_obj_init(&kms->base,
+ &initial_state->base,
+ &kms_state_funcs);
+
if (kms) {
ret = kms->funcs->hw_init(kms);
if (ret) {
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 48ed5b9a8580..c5b8c989b859 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -162,9 +162,6 @@ struct msm_format {
int msm_atomic_commit(struct drm_device *dev,
struct drm_atomic_state *state, bool nonblock);
-struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev);
-void msm_atomic_state_clear(struct drm_atomic_state *state);
-void msm_atomic_state_free(struct drm_atomic_state *state);
void msm_gem_unmap_vma(struct msm_gem_address_space *aspace,
struct msm_gem_vma *vma, struct sg_table *sgt);
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 17d5824417ad..24d09fcebf16 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -42,6 +42,8 @@ struct msm_kms_funcs {
void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc);
/* swap global atomic state: */
void (*swap_state)(struct msm_kms *kms, struct drm_atomic_state *state);
+ void *(*duplicate_state)(void *state);
+ void (*destroy_state)(void *state);
/* modeset, bracketing atomic_commit(): */
void (*prepare_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
void (*complete_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
@@ -68,6 +70,8 @@ struct msm_kms_funcs {
};
struct msm_kms {
+ struct drm_private_obj base;
+
const struct msm_kms_funcs *funcs;
/* irq number to be passed on to drm_irq_install */
@@ -78,16 +82,23 @@ struct msm_kms {
};
/**
- * Subclass of drm_atomic_state, to allow kms backend to have driver
+ * Subclass of drm_private_state, to allow kms backend to have driver
* private global state. The kms backend can do whatever it wants
- * with the ->state ptr. On ->atomic_state_clear() the ->state ptr
- * is kfree'd and set back to NULL.
+ * with the ->state ptr.
*/
struct msm_kms_state {
- struct drm_atomic_state base;
+ struct drm_private_state base;
void *state;
};
-#define to_kms_state(x) container_of(x, struct msm_kms_state, base)
+
+/**
+ * Extracts the msm_kms_state from either an atomic state, or the current
+ * device. One or the other might be desirable depending on whether you want the
+ * currently configured msm_kms_state (from_dev), or you would like the state
+ * which is about to be applied (from_atomic).
+ */
+struct msm_kms_state *msm_kms_state_from_dev(struct drm_device *dev);
+struct msm_kms_state *msm_kms_state_from_atomic(struct drm_atomic_state *state);
static inline void msm_kms_init(struct msm_kms *kms,
const struct msm_kms_funcs *funcs)
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 2/6] drm/msm: Refactor complete_commit() to look more the helpers
2018-03-28 19:06 [PATCH v2 0/6] drm/msm: Switch to atomic helpers Sean Paul
@ 2018-03-28 19:06 ` Sean Paul
[not found] ` <20180328190657.218661-1-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-03-28 19:06 ` Sean Paul
2 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: architt-sgV2jX0FEOL9JmXXK+q4OQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
robdclark-Re5JQEeQqe8AvxtiuMwx3w, Sean Paul,
jsanka-sgV2jX0FEOL9JmXXK+q4OQ, hoegsberg-F7+t8E8rja9g9hUCZPvPmw
Factor out the commit_tail() portions of complete_commit() into a
separate function to facilitate moving to the atomic helpers in future
patches.
Changes in v2:
- None
Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/msm_atomic.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index e792158676aa..671a18ee977d 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -97,18 +97,12 @@ static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
}
}
-/* The (potentially) asynchronous part of the commit. At this point
- * nothing can fail short of armageddon.
- */
-static void complete_commit(struct msm_commit *c, bool async)
+static void msm_atomic_commit_tail(struct drm_atomic_state *state)
{
- struct drm_atomic_state *state = c->state;
struct drm_device *dev = state->dev;
struct msm_drm_private *priv = dev->dev_private;
struct msm_kms *kms = priv->kms;
- drm_atomic_helper_wait_for_fences(dev, state, false);
-
kms->funcs->prepare_commit(kms, state);
drm_atomic_helper_commit_modeset_disables(dev, state);
@@ -135,6 +129,19 @@ static void complete_commit(struct msm_commit *c, bool async)
drm_atomic_helper_cleanup_planes(dev, state);
kms->funcs->complete_commit(kms, state);
+}
+
+/* The (potentially) asynchronous part of the commit. At this point
+ * nothing can fail short of armageddon.
+ */
+static void complete_commit(struct msm_commit *c)
+{
+ struct drm_atomic_state *state = c->state;
+ struct drm_device *dev = state->dev;
+
+ drm_atomic_helper_wait_for_fences(dev, state, false);
+
+ msm_atomic_commit_tail(state);
drm_atomic_state_put(state);
@@ -143,7 +150,7 @@ static void complete_commit(struct msm_commit *c, bool async)
static void commit_worker(struct work_struct *work)
{
- complete_commit(container_of(work, struct msm_commit, work), true);
+ complete_commit(container_of(work, struct msm_commit, work));
}
/**
@@ -242,7 +249,7 @@ int msm_atomic_commit(struct drm_device *dev,
return 0;
}
- complete_commit(c, false);
+ complete_commit(c);
return 0;
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 2/6] drm/msm: Refactor complete_commit() to look more the helpers
@ 2018-03-28 19:06 ` Sean Paul
0 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno, linux-arm-msm, dri-devel
Cc: robdclark, hoegsberg, jsanka, abhinavk, architt, Sean Paul, linux-kernel
Factor out the commit_tail() portions of complete_commit() into a
separate function to facilitate moving to the atomic helpers in future
patches.
Changes in v2:
- None
Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/msm_atomic.c | 25 ++++++++++++++++---------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index e792158676aa..671a18ee977d 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -97,18 +97,12 @@ static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
}
}
-/* The (potentially) asynchronous part of the commit. At this point
- * nothing can fail short of armageddon.
- */
-static void complete_commit(struct msm_commit *c, bool async)
+static void msm_atomic_commit_tail(struct drm_atomic_state *state)
{
- struct drm_atomic_state *state = c->state;
struct drm_device *dev = state->dev;
struct msm_drm_private *priv = dev->dev_private;
struct msm_kms *kms = priv->kms;
- drm_atomic_helper_wait_for_fences(dev, state, false);
-
kms->funcs->prepare_commit(kms, state);
drm_atomic_helper_commit_modeset_disables(dev, state);
@@ -135,6 +129,19 @@ static void complete_commit(struct msm_commit *c, bool async)
drm_atomic_helper_cleanup_planes(dev, state);
kms->funcs->complete_commit(kms, state);
+}
+
+/* The (potentially) asynchronous part of the commit. At this point
+ * nothing can fail short of armageddon.
+ */
+static void complete_commit(struct msm_commit *c)
+{
+ struct drm_atomic_state *state = c->state;
+ struct drm_device *dev = state->dev;
+
+ drm_atomic_helper_wait_for_fences(dev, state, false);
+
+ msm_atomic_commit_tail(state);
drm_atomic_state_put(state);
@@ -143,7 +150,7 @@ static void complete_commit(struct msm_commit *c, bool async)
static void commit_worker(struct work_struct *work)
{
- complete_commit(container_of(work, struct msm_commit, work), true);
+ complete_commit(container_of(work, struct msm_commit, work));
}
/**
@@ -242,7 +249,7 @@ int msm_atomic_commit(struct drm_device *dev,
return 0;
}
- complete_commit(c, false);
+ complete_commit(c);
return 0;
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 3/6] drm/msm: Mark the crtc->state->event consumed
2018-03-28 19:06 [PATCH v2 0/6] drm/msm: Switch to atomic helpers Sean Paul
@ 2018-03-28 19:06 ` Sean Paul
[not found] ` <20180328190657.218661-1-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-03-28 19:06 ` Sean Paul
2 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: architt-sgV2jX0FEOL9JmXXK+q4OQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
robdclark-Re5JQEeQqe8AvxtiuMwx3w, Sean Paul,
jsanka-sgV2jX0FEOL9JmXXK+q4OQ, hoegsberg-F7+t8E8rja9g9hUCZPvPmw
Don't leave the event != NULL once it's consumed, this is used a signal
to the atomic helpers that the event will be handled by the driver.
Changes in v2:
- None
Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index 6e5e1aa54ce1..b001699297c4 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -351,6 +351,7 @@ static void mdp4_crtc_atomic_flush(struct drm_crtc *crtc,
spin_lock_irqsave(&dev->event_lock, flags);
mdp4_crtc->event = crtc->state->event;
+ crtc->state->event = NULL;
spin_unlock_irqrestore(&dev->event_lock, flags);
blend_setup(crtc);
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index 9893e43ba6c5..76b96081916f 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -708,6 +708,7 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc,
spin_lock_irqsave(&dev->event_lock, flags);
mdp5_crtc->event = crtc->state->event;
+ crtc->state->event = NULL;
spin_unlock_irqrestore(&dev->event_lock, flags);
/*
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 3/6] drm/msm: Mark the crtc->state->event consumed
@ 2018-03-28 19:06 ` Sean Paul
0 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno, linux-arm-msm, dri-devel
Cc: robdclark, hoegsberg, jsanka, abhinavk, architt, Sean Paul, linux-kernel
Don't leave the event != NULL once it's consumed, this is used a signal
to the atomic helpers that the event will be handled by the driver.
Changes in v2:
- None
Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
index 6e5e1aa54ce1..b001699297c4 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
@@ -351,6 +351,7 @@ static void mdp4_crtc_atomic_flush(struct drm_crtc *crtc,
spin_lock_irqsave(&dev->event_lock, flags);
mdp4_crtc->event = crtc->state->event;
+ crtc->state->event = NULL;
spin_unlock_irqrestore(&dev->event_lock, flags);
blend_setup(crtc);
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index 9893e43ba6c5..76b96081916f 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -708,6 +708,7 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc,
spin_lock_irqsave(&dev->event_lock, flags);
mdp5_crtc->event = crtc->state->event;
+ crtc->state->event = NULL;
spin_unlock_irqrestore(&dev->event_lock, flags);
/*
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 4/6] drm/msm: Issue queued events when disabling crtc
2018-03-28 19:06 [PATCH v2 0/6] drm/msm: Switch to atomic helpers Sean Paul
@ 2018-03-28 19:06 ` Sean Paul
[not found] ` <20180328190657.218661-1-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-03-28 19:06 ` Sean Paul
2 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: architt-sgV2jX0FEOL9JmXXK+q4OQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
robdclark-Re5JQEeQqe8AvxtiuMwx3w, Sean Paul,
jsanka-sgV2jX0FEOL9JmXXK+q4OQ, hoegsberg-F7+t8E8rja9g9hUCZPvPmw
Ensure that any queued events are issued when disabling the crtc. This
avoids timeouts when we come back and wait for dependencies (like the
previous frame's flip_done).
Changes in v2:
- None
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index 76b96081916f..10271359789e 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -430,6 +430,7 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc,
struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
struct mdp5_kms *mdp5_kms = get_kms(crtc);
struct device *dev = &mdp5_kms->pdev->dev;
+ unsigned long flags;
DBG("%s", crtc->name);
@@ -445,6 +446,14 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc,
mdp_irq_unregister(&mdp5_kms->base, &mdp5_crtc->err);
pm_runtime_put_sync(dev);
+ if (crtc->state->event && !crtc->state->active) {
+ WARN_ON(mdp5_crtc->event);
+ spin_lock_irqsave(&mdp5_kms->dev->event_lock, flags);
+ drm_crtc_send_vblank_event(crtc, crtc->state->event);
+ crtc->state->event = NULL;
+ spin_unlock_irqrestore(&mdp5_kms->dev->event_lock, flags);
+ }
+
mdp5_crtc->enabled = false;
}
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 4/6] drm/msm: Issue queued events when disabling crtc
@ 2018-03-28 19:06 ` Sean Paul
0 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno, linux-arm-msm, dri-devel
Cc: robdclark, hoegsberg, jsanka, abhinavk, architt, Sean Paul, linux-kernel
Ensure that any queued events are issued when disabling the crtc. This
avoids timeouts when we come back and wait for dependencies (like the
previous frame's flip_done).
Changes in v2:
- None
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index 76b96081916f..10271359789e 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -430,6 +430,7 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc,
struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
struct mdp5_kms *mdp5_kms = get_kms(crtc);
struct device *dev = &mdp5_kms->pdev->dev;
+ unsigned long flags;
DBG("%s", crtc->name);
@@ -445,6 +446,14 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc,
mdp_irq_unregister(&mdp5_kms->base, &mdp5_crtc->err);
pm_runtime_put_sync(dev);
+ if (crtc->state->event && !crtc->state->active) {
+ WARN_ON(mdp5_crtc->event);
+ spin_lock_irqsave(&mdp5_kms->dev->event_lock, flags);
+ drm_crtc_send_vblank_event(crtc, crtc->state->event);
+ crtc->state->event = NULL;
+ spin_unlock_irqrestore(&mdp5_kms->dev->event_lock, flags);
+ }
+
mdp5_crtc->enabled = false;
}
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 5/6] drm/msm: Remove msm_commit/worker, use atomic helper commit
2018-03-28 19:06 [PATCH v2 0/6] drm/msm: Switch to atomic helpers Sean Paul
@ 2018-03-28 19:06 ` Sean Paul
[not found] ` <20180328190657.218661-1-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-03-28 19:06 ` Sean Paul
2 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: architt-sgV2jX0FEOL9JmXXK+q4OQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
robdclark-Re5JQEeQqe8AvxtiuMwx3w, Sean Paul,
jsanka-sgV2jX0FEOL9JmXXK+q4OQ, hoegsberg-F7+t8E8rja9g9hUCZPvPmw
Moving further towards switching fully to the the atomic helpers, this
patch removes the hand-rolled worker nonblock commit code and uses the
atomic helpers commit_work model.
Changes in v2:
- Remove commit_destroy()
- Shuffle order of commit_tail calls to further serialize commits
- Use stall in swap_state to avoid abandoned events on disable
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/msm_atomic.c | 153 +++++++++----------------------
drivers/gpu/drm/msm/msm_drv.c | 1 -
drivers/gpu/drm/msm/msm_drv.h | 4 -
3 files changed, 42 insertions(+), 116 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 671a18ee977d..52249565cc03 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -20,66 +20,6 @@
#include "msm_gem.h"
#include "msm_fence.h"
-struct msm_commit {
- struct drm_device *dev;
- struct drm_atomic_state *state;
- struct work_struct work;
- uint32_t crtc_mask;
-};
-
-static void commit_worker(struct work_struct *work);
-
-/* block until specified crtcs are no longer pending update, and
- * atomically mark them as pending update
- */
-static int start_atomic(struct msm_drm_private *priv, uint32_t crtc_mask)
-{
- int ret;
-
- spin_lock(&priv->pending_crtcs_event.lock);
- ret = wait_event_interruptible_locked(priv->pending_crtcs_event,
- !(priv->pending_crtcs & crtc_mask));
- if (ret == 0) {
- DBG("start: %08x", crtc_mask);
- priv->pending_crtcs |= crtc_mask;
- }
- spin_unlock(&priv->pending_crtcs_event.lock);
-
- return ret;
-}
-
-/* clear specified crtcs (no longer pending update)
- */
-static void end_atomic(struct msm_drm_private *priv, uint32_t crtc_mask)
-{
- spin_lock(&priv->pending_crtcs_event.lock);
- DBG("end: %08x", crtc_mask);
- priv->pending_crtcs &= ~crtc_mask;
- wake_up_all_locked(&priv->pending_crtcs_event);
- spin_unlock(&priv->pending_crtcs_event.lock);
-}
-
-static struct msm_commit *commit_init(struct drm_atomic_state *state)
-{
- struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL);
-
- if (!c)
- return NULL;
-
- c->dev = state->dev;
- c->state = state;
-
- INIT_WORK(&c->work, commit_worker);
-
- return c;
-}
-
-static void commit_destroy(struct msm_commit *c)
-{
- end_atomic(c->dev->dev_private, c->crtc_mask);
- kfree(c);
-}
-
static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
struct drm_atomic_state *old_state)
{
@@ -126,31 +66,37 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *state)
msm_atomic_wait_for_commit_done(dev, state);
- drm_atomic_helper_cleanup_planes(dev, state);
-
kms->funcs->complete_commit(kms, state);
+
+ drm_atomic_helper_wait_for_vblanks(dev, state);
+
+ drm_atomic_helper_commit_hw_done(state);
+
+ drm_atomic_helper_cleanup_planes(dev, state);
}
/* The (potentially) asynchronous part of the commit. At this point
* nothing can fail short of armageddon.
*/
-static void complete_commit(struct msm_commit *c)
+static void commit_tail(struct drm_atomic_state *state)
{
- struct drm_atomic_state *state = c->state;
- struct drm_device *dev = state->dev;
+ drm_atomic_helper_wait_for_fences(state->dev, state, false);
- drm_atomic_helper_wait_for_fences(dev, state, false);
+ drm_atomic_helper_wait_for_dependencies(state);
msm_atomic_commit_tail(state);
- drm_atomic_state_put(state);
+ drm_atomic_helper_commit_cleanup_done(state);
- commit_destroy(c);
+ drm_atomic_state_put(state);
}
-static void commit_worker(struct work_struct *work)
+static void commit_work(struct work_struct *work)
{
- complete_commit(container_of(work, struct msm_commit, work));
+ struct drm_atomic_state *state = container_of(work,
+ struct drm_atomic_state,
+ commit_work);
+ commit_tail(state);
}
/**
@@ -169,17 +115,12 @@ int msm_atomic_commit(struct drm_device *dev,
struct drm_atomic_state *state, bool nonblock)
{
struct msm_drm_private *priv = dev->dev_private;
- struct msm_commit *c;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
struct drm_plane *plane;
struct drm_plane_state *old_plane_state, *new_plane_state;
int i, ret;
- ret = drm_atomic_helper_prepare_planes(dev, state);
- if (ret)
- return ret;
-
/*
* Note that plane->atomic_async_check() should fail if we need
* to re-assign hwpipe or anything that touches global atomic
@@ -187,45 +128,39 @@ int msm_atomic_commit(struct drm_device *dev,
* cases.
*/
if (state->async_update) {
+ ret = drm_atomic_helper_prepare_planes(dev, state);
+ if (ret)
+ return ret;
+
drm_atomic_helper_async_commit(dev, state);
drm_atomic_helper_cleanup_planes(dev, state);
return 0;
}
- c = commit_init(state);
- if (!c) {
- ret = -ENOMEM;
- goto error;
- }
+ ret = drm_atomic_helper_setup_commit(state, nonblock);
+ if (ret)
+ return ret;
- /*
- * Figure out what crtcs we have:
- */
- for_each_new_crtc_in_state(state, crtc, crtc_state, i)
- c->crtc_mask |= drm_crtc_mask(crtc);
+ INIT_WORK(&state->commit_work, commit_work);
- /*
- * Figure out what fence to wait for:
- */
- for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
- if ((new_plane_state->fb != old_plane_state->fb) && new_plane_state->fb) {
- struct drm_gem_object *obj = msm_framebuffer_bo(new_plane_state->fb, 0);
- struct msm_gem_object *msm_obj = to_msm_bo(obj);
- struct dma_fence *fence = reservation_object_get_excl_rcu(msm_obj->resv);
-
- drm_atomic_set_fence_for_plane(new_plane_state, fence);
- }
+ ret = drm_atomic_helper_prepare_planes(dev, state);
+ if (ret)
+ return ret;
+
+ if (!nonblock) {
+ ret = drm_atomic_helper_wait_for_fences(dev, state, true);
+ if (ret)
+ goto error;
}
/*
- * Wait for pending updates on any of the same crtc's and then
- * mark our set of crtc's as busy:
+ * This is the point of no return - everything below never fails except
+ * when the hw goes bonghits. Which means we can commit the new state on
+ * the software side now.
+ *
+ * swap driver private state while still holding state_lock
*/
- ret = start_atomic(dev->dev_private, c->crtc_mask);
- if (ret)
- goto err_free;
-
- BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
+ BUG_ON(drm_atomic_helper_swap_state(state, true) < 0);
/*
* Everything below can be run asynchronously without the need to grab
@@ -244,17 +179,13 @@ int msm_atomic_commit(struct drm_device *dev,
*/
drm_atomic_state_get(state);
- if (nonblock) {
- queue_work(priv->atomic_wq, &c->work);
- return 0;
- }
-
- complete_commit(c);
+ if (nonblock)
+ queue_work(system_unbound_wq, &state->commit_work);
+ else
+ commit_tail(state);
return 0;
-err_free:
- kfree(c);
error:
drm_atomic_helper_cleanup_planes(dev, state);
return ret;
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 7e8f640062ef..d1a45a68c287 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -459,7 +459,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
priv->wq = alloc_ordered_workqueue("msm", 0);
priv->atomic_wq = alloc_ordered_workqueue("msm:atomic", 0);
- init_waitqueue_head(&priv->pending_crtcs_event);
INIT_LIST_HEAD(&priv->inactive_list);
INIT_LIST_HEAD(&priv->vblank_ctrl.event_list);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index c5b8c989b859..b89fa5186eb8 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -117,10 +117,6 @@ struct msm_drm_private {
struct workqueue_struct *wq;
struct workqueue_struct *atomic_wq;
- /* crtcs pending async atomic updates: */
- uint32_t pending_crtcs;
- wait_queue_head_t pending_crtcs_event;
-
unsigned int num_planes;
struct drm_plane *planes[16];
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 5/6] drm/msm: Remove msm_commit/worker, use atomic helper commit
@ 2018-03-28 19:06 ` Sean Paul
0 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno, linux-arm-msm, dri-devel
Cc: robdclark, hoegsberg, jsanka, abhinavk, architt, Sean Paul, linux-kernel
Moving further towards switching fully to the the atomic helpers, this
patch removes the hand-rolled worker nonblock commit code and uses the
atomic helpers commit_work model.
Changes in v2:
- Remove commit_destroy()
- Shuffle order of commit_tail calls to further serialize commits
- Use stall in swap_state to avoid abandoned events on disable
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/msm_atomic.c | 153 +++++++++----------------------
drivers/gpu/drm/msm/msm_drv.c | 1 -
drivers/gpu/drm/msm/msm_drv.h | 4 -
3 files changed, 42 insertions(+), 116 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 671a18ee977d..52249565cc03 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -20,66 +20,6 @@
#include "msm_gem.h"
#include "msm_fence.h"
-struct msm_commit {
- struct drm_device *dev;
- struct drm_atomic_state *state;
- struct work_struct work;
- uint32_t crtc_mask;
-};
-
-static void commit_worker(struct work_struct *work);
-
-/* block until specified crtcs are no longer pending update, and
- * atomically mark them as pending update
- */
-static int start_atomic(struct msm_drm_private *priv, uint32_t crtc_mask)
-{
- int ret;
-
- spin_lock(&priv->pending_crtcs_event.lock);
- ret = wait_event_interruptible_locked(priv->pending_crtcs_event,
- !(priv->pending_crtcs & crtc_mask));
- if (ret == 0) {
- DBG("start: %08x", crtc_mask);
- priv->pending_crtcs |= crtc_mask;
- }
- spin_unlock(&priv->pending_crtcs_event.lock);
-
- return ret;
-}
-
-/* clear specified crtcs (no longer pending update)
- */
-static void end_atomic(struct msm_drm_private *priv, uint32_t crtc_mask)
-{
- spin_lock(&priv->pending_crtcs_event.lock);
- DBG("end: %08x", crtc_mask);
- priv->pending_crtcs &= ~crtc_mask;
- wake_up_all_locked(&priv->pending_crtcs_event);
- spin_unlock(&priv->pending_crtcs_event.lock);
-}
-
-static struct msm_commit *commit_init(struct drm_atomic_state *state)
-{
- struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL);
-
- if (!c)
- return NULL;
-
- c->dev = state->dev;
- c->state = state;
-
- INIT_WORK(&c->work, commit_worker);
-
- return c;
-}
-
-static void commit_destroy(struct msm_commit *c)
-{
- end_atomic(c->dev->dev_private, c->crtc_mask);
- kfree(c);
-}
-
static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
struct drm_atomic_state *old_state)
{
@@ -126,31 +66,37 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *state)
msm_atomic_wait_for_commit_done(dev, state);
- drm_atomic_helper_cleanup_planes(dev, state);
-
kms->funcs->complete_commit(kms, state);
+
+ drm_atomic_helper_wait_for_vblanks(dev, state);
+
+ drm_atomic_helper_commit_hw_done(state);
+
+ drm_atomic_helper_cleanup_planes(dev, state);
}
/* The (potentially) asynchronous part of the commit. At this point
* nothing can fail short of armageddon.
*/
-static void complete_commit(struct msm_commit *c)
+static void commit_tail(struct drm_atomic_state *state)
{
- struct drm_atomic_state *state = c->state;
- struct drm_device *dev = state->dev;
+ drm_atomic_helper_wait_for_fences(state->dev, state, false);
- drm_atomic_helper_wait_for_fences(dev, state, false);
+ drm_atomic_helper_wait_for_dependencies(state);
msm_atomic_commit_tail(state);
- drm_atomic_state_put(state);
+ drm_atomic_helper_commit_cleanup_done(state);
- commit_destroy(c);
+ drm_atomic_state_put(state);
}
-static void commit_worker(struct work_struct *work)
+static void commit_work(struct work_struct *work)
{
- complete_commit(container_of(work, struct msm_commit, work));
+ struct drm_atomic_state *state = container_of(work,
+ struct drm_atomic_state,
+ commit_work);
+ commit_tail(state);
}
/**
@@ -169,17 +115,12 @@ int msm_atomic_commit(struct drm_device *dev,
struct drm_atomic_state *state, bool nonblock)
{
struct msm_drm_private *priv = dev->dev_private;
- struct msm_commit *c;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
struct drm_plane *plane;
struct drm_plane_state *old_plane_state, *new_plane_state;
int i, ret;
- ret = drm_atomic_helper_prepare_planes(dev, state);
- if (ret)
- return ret;
-
/*
* Note that plane->atomic_async_check() should fail if we need
* to re-assign hwpipe or anything that touches global atomic
@@ -187,45 +128,39 @@ int msm_atomic_commit(struct drm_device *dev,
* cases.
*/
if (state->async_update) {
+ ret = drm_atomic_helper_prepare_planes(dev, state);
+ if (ret)
+ return ret;
+
drm_atomic_helper_async_commit(dev, state);
drm_atomic_helper_cleanup_planes(dev, state);
return 0;
}
- c = commit_init(state);
- if (!c) {
- ret = -ENOMEM;
- goto error;
- }
+ ret = drm_atomic_helper_setup_commit(state, nonblock);
+ if (ret)
+ return ret;
- /*
- * Figure out what crtcs we have:
- */
- for_each_new_crtc_in_state(state, crtc, crtc_state, i)
- c->crtc_mask |= drm_crtc_mask(crtc);
+ INIT_WORK(&state->commit_work, commit_work);
- /*
- * Figure out what fence to wait for:
- */
- for_each_oldnew_plane_in_state(state, plane, old_plane_state, new_plane_state, i) {
- if ((new_plane_state->fb != old_plane_state->fb) && new_plane_state->fb) {
- struct drm_gem_object *obj = msm_framebuffer_bo(new_plane_state->fb, 0);
- struct msm_gem_object *msm_obj = to_msm_bo(obj);
- struct dma_fence *fence = reservation_object_get_excl_rcu(msm_obj->resv);
-
- drm_atomic_set_fence_for_plane(new_plane_state, fence);
- }
+ ret = drm_atomic_helper_prepare_planes(dev, state);
+ if (ret)
+ return ret;
+
+ if (!nonblock) {
+ ret = drm_atomic_helper_wait_for_fences(dev, state, true);
+ if (ret)
+ goto error;
}
/*
- * Wait for pending updates on any of the same crtc's and then
- * mark our set of crtc's as busy:
+ * This is the point of no return - everything below never fails except
+ * when the hw goes bonghits. Which means we can commit the new state on
+ * the software side now.
+ *
+ * swap driver private state while still holding state_lock
*/
- ret = start_atomic(dev->dev_private, c->crtc_mask);
- if (ret)
- goto err_free;
-
- BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
+ BUG_ON(drm_atomic_helper_swap_state(state, true) < 0);
/*
* Everything below can be run asynchronously without the need to grab
@@ -244,17 +179,13 @@ int msm_atomic_commit(struct drm_device *dev,
*/
drm_atomic_state_get(state);
- if (nonblock) {
- queue_work(priv->atomic_wq, &c->work);
- return 0;
- }
-
- complete_commit(c);
+ if (nonblock)
+ queue_work(system_unbound_wq, &state->commit_work);
+ else
+ commit_tail(state);
return 0;
-err_free:
- kfree(c);
error:
drm_atomic_helper_cleanup_planes(dev, state);
return ret;
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 7e8f640062ef..d1a45a68c287 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -459,7 +459,6 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
priv->wq = alloc_ordered_workqueue("msm", 0);
priv->atomic_wq = alloc_ordered_workqueue("msm:atomic", 0);
- init_waitqueue_head(&priv->pending_crtcs_event);
INIT_LIST_HEAD(&priv->inactive_list);
INIT_LIST_HEAD(&priv->vblank_ctrl.event_list);
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index c5b8c989b859..b89fa5186eb8 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -117,10 +117,6 @@ struct msm_drm_private {
struct workqueue_struct *wq;
struct workqueue_struct *atomic_wq;
- /* crtcs pending async atomic updates: */
- uint32_t pending_crtcs;
- wait_queue_head_t pending_crtcs_event;
-
unsigned int num_planes;
struct drm_plane *planes[16];
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 6/6] drm/msm: Switch to atomic_helper_commit()
2018-03-28 19:06 [PATCH v2 0/6] drm/msm: Switch to atomic helpers Sean Paul
@ 2018-03-28 19:06 ` Sean Paul
[not found] ` <20180328190657.218661-1-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-03-28 19:06 ` Sean Paul
2 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno, linux-arm-msm, dri-devel; +Cc: linux-kernel, abhinavk, hoegsberg
Now that all of the msm-specific goo is tucked safely away we can switch
over to using the atomic helper commit directly. \o/
Changes in v2:
- None
Cc: Abhinav Kumar <abhinavk@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/msm_atomic.c | 120 +------------------------------
drivers/gpu/drm/msm/msm_drv.c | 7 +-
drivers/gpu/drm/msm/msm_drv.h | 3 +-
3 files changed, 8 insertions(+), 122 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 52249565cc03..d593ac3de1df 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -17,8 +17,6 @@
#include "msm_drv.h"
#include "msm_kms.h"
-#include "msm_gem.h"
-#include "msm_fence.h"
static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
struct drm_atomic_state *old_state)
@@ -37,7 +35,7 @@ static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
}
}
-static void msm_atomic_commit_tail(struct drm_atomic_state *state)
+void msm_atomic_commit_tail(struct drm_atomic_state *state)
{
struct drm_device *dev = state->dev;
struct msm_drm_private *priv = dev->dev_private;
@@ -74,119 +72,3 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *state)
drm_atomic_helper_cleanup_planes(dev, state);
}
-
-/* The (potentially) asynchronous part of the commit. At this point
- * nothing can fail short of armageddon.
- */
-static void commit_tail(struct drm_atomic_state *state)
-{
- drm_atomic_helper_wait_for_fences(state->dev, state, false);
-
- drm_atomic_helper_wait_for_dependencies(state);
-
- msm_atomic_commit_tail(state);
-
- drm_atomic_helper_commit_cleanup_done(state);
-
- drm_atomic_state_put(state);
-}
-
-static void commit_work(struct work_struct *work)
-{
- struct drm_atomic_state *state = container_of(work,
- struct drm_atomic_state,
- commit_work);
- commit_tail(state);
-}
-
-/**
- * drm_atomic_helper_commit - commit validated state object
- * @dev: DRM device
- * @state: the driver state object
- * @nonblock: nonblocking commit
- *
- * This function commits a with drm_atomic_helper_check() pre-validated state
- * object. This can still fail when e.g. the framebuffer reservation fails.
- *
- * RETURNS
- * Zero for success or -errno.
- */
-int msm_atomic_commit(struct drm_device *dev,
- struct drm_atomic_state *state, bool nonblock)
-{
- struct msm_drm_private *priv = dev->dev_private;
- struct drm_crtc *crtc;
- struct drm_crtc_state *crtc_state;
- struct drm_plane *plane;
- struct drm_plane_state *old_plane_state, *new_plane_state;
- int i, ret;
-
- /*
- * Note that plane->atomic_async_check() should fail if we need
- * to re-assign hwpipe or anything that touches global atomic
- * state, so we'll never go down the async update path in those
- * cases.
- */
- if (state->async_update) {
- ret = drm_atomic_helper_prepare_planes(dev, state);
- if (ret)
- return ret;
-
- drm_atomic_helper_async_commit(dev, state);
- drm_atomic_helper_cleanup_planes(dev, state);
- return 0;
- }
-
- ret = drm_atomic_helper_setup_commit(state, nonblock);
- if (ret)
- return ret;
-
- INIT_WORK(&state->commit_work, commit_work);
-
- ret = drm_atomic_helper_prepare_planes(dev, state);
- if (ret)
- return ret;
-
- if (!nonblock) {
- ret = drm_atomic_helper_wait_for_fences(dev, state, true);
- if (ret)
- goto error;
- }
-
- /*
- * This is the point of no return - everything below never fails except
- * when the hw goes bonghits. Which means we can commit the new state on
- * the software side now.
- *
- * swap driver private state while still holding state_lock
- */
- BUG_ON(drm_atomic_helper_swap_state(state, true) < 0);
-
- /*
- * Everything below can be run asynchronously without the need to grab
- * any modeset locks at all under one conditions: It must be guaranteed
- * that the asynchronous work has either been cancelled (if the driver
- * supports it, which at least requires that the framebuffers get
- * cleaned up with drm_atomic_helper_cleanup_planes()) or completed
- * before the new state gets committed on the software side with
- * drm_atomic_helper_swap_state().
- *
- * This scheme allows new atomic state updates to be prepared and
- * checked in parallel to the asynchronous completion of the previous
- * update. Which is important since compositors need to figure out the
- * composition of the next frame right after having submitted the
- * current layout.
- */
-
- drm_atomic_state_get(state);
- if (nonblock)
- queue_work(system_unbound_wq, &state->commit_work);
- else
- commit_tail(state);
-
- return 0;
-
-error:
- drm_atomic_helper_cleanup_planes(dev, state);
- return ret;
-}
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index d1a45a68c287..eb5569d4dd57 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -41,7 +41,11 @@ static const struct drm_mode_config_funcs mode_config_funcs = {
.fb_create = msm_framebuffer_create,
.output_poll_changed = drm_fb_helper_output_poll_changed,
.atomic_check = drm_atomic_helper_check,
- .atomic_commit = msm_atomic_commit,
+ .atomic_commit = drm_atomic_helper_commit,
+};
+
+static const struct drm_mode_config_helper_funcs mode_config_helper_funcs = {
+ .atomic_commit_tail = msm_atomic_commit_tail,
};
static inline
@@ -520,6 +524,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
}
ddev->mode_config.funcs = &mode_config_funcs;
+ ddev->mode_config.helper_private = &mode_config_helper_funcs;
ret = drm_vblank_init(ddev, priv->num_crtcs);
if (ret < 0) {
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index b89fa5186eb8..3e89d7eac02f 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -156,8 +156,7 @@ struct msm_format {
uint32_t pixel_format;
};
-int msm_atomic_commit(struct drm_device *dev,
- struct drm_atomic_state *state, bool nonblock);
+void msm_atomic_commit_tail(struct drm_atomic_state *state);
void msm_gem_unmap_vma(struct msm_gem_address_space *aspace,
struct msm_gem_vma *vma, struct sg_table *sgt);
--
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v2 6/6] drm/msm: Switch to atomic_helper_commit()
@ 2018-03-28 19:06 ` Sean Paul
0 siblings, 0 replies; 21+ messages in thread
From: Sean Paul @ 2018-03-28 19:06 UTC (permalink / raw)
To: freedreno, linux-arm-msm, dri-devel
Cc: robdclark, hoegsberg, jsanka, abhinavk, architt, Sean Paul, linux-kernel
Now that all of the msm-specific goo is tucked safely away we can switch
over to using the atomic helper commit directly. \o/
Changes in v2:
- None
Cc: Abhinav Kumar <abhinavk@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
drivers/gpu/drm/msm/msm_atomic.c | 120 +------------------------------
drivers/gpu/drm/msm/msm_drv.c | 7 +-
drivers/gpu/drm/msm/msm_drv.h | 3 +-
3 files changed, 8 insertions(+), 122 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 52249565cc03..d593ac3de1df 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -17,8 +17,6 @@
#include "msm_drv.h"
#include "msm_kms.h"
-#include "msm_gem.h"
-#include "msm_fence.h"
static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
struct drm_atomic_state *old_state)
@@ -37,7 +35,7 @@ static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
}
}
-static void msm_atomic_commit_tail(struct drm_atomic_state *state)
+void msm_atomic_commit_tail(struct drm_atomic_state *state)
{
struct drm_device *dev = state->dev;
struct msm_drm_private *priv = dev->dev_private;
@@ -74,119 +72,3 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *state)
drm_atomic_helper_cleanup_planes(dev, state);
}
-
-/* The (potentially) asynchronous part of the commit. At this point
- * nothing can fail short of armageddon.
- */
-static void commit_tail(struct drm_atomic_state *state)
-{
- drm_atomic_helper_wait_for_fences(state->dev, state, false);
-
- drm_atomic_helper_wait_for_dependencies(state);
-
- msm_atomic_commit_tail(state);
-
- drm_atomic_helper_commit_cleanup_done(state);
-
- drm_atomic_state_put(state);
-}
-
-static void commit_work(struct work_struct *work)
-{
- struct drm_atomic_state *state = container_of(work,
- struct drm_atomic_state,
- commit_work);
- commit_tail(state);
-}
-
-/**
- * drm_atomic_helper_commit - commit validated state object
- * @dev: DRM device
- * @state: the driver state object
- * @nonblock: nonblocking commit
- *
- * This function commits a with drm_atomic_helper_check() pre-validated state
- * object. This can still fail when e.g. the framebuffer reservation fails.
- *
- * RETURNS
- * Zero for success or -errno.
- */
-int msm_atomic_commit(struct drm_device *dev,
- struct drm_atomic_state *state, bool nonblock)
-{
- struct msm_drm_private *priv = dev->dev_private;
- struct drm_crtc *crtc;
- struct drm_crtc_state *crtc_state;
- struct drm_plane *plane;
- struct drm_plane_state *old_plane_state, *new_plane_state;
- int i, ret;
-
- /*
- * Note that plane->atomic_async_check() should fail if we need
- * to re-assign hwpipe or anything that touches global atomic
- * state, so we'll never go down the async update path in those
- * cases.
- */
- if (state->async_update) {
- ret = drm_atomic_helper_prepare_planes(dev, state);
- if (ret)
- return ret;
-
- drm_atomic_helper_async_commit(dev, state);
- drm_atomic_helper_cleanup_planes(dev, state);
- return 0;
- }
-
- ret = drm_atomic_helper_setup_commit(state, nonblock);
- if (ret)
- return ret;
-
- INIT_WORK(&state->commit_work, commit_work);
-
- ret = drm_atomic_helper_prepare_planes(dev, state);
- if (ret)
- return ret;
-
- if (!nonblock) {
- ret = drm_atomic_helper_wait_for_fences(dev, state, true);
- if (ret)
- goto error;
- }
-
- /*
- * This is the point of no return - everything below never fails except
- * when the hw goes bonghits. Which means we can commit the new state on
- * the software side now.
- *
- * swap driver private state while still holding state_lock
- */
- BUG_ON(drm_atomic_helper_swap_state(state, true) < 0);
-
- /*
- * Everything below can be run asynchronously without the need to grab
- * any modeset locks at all under one conditions: It must be guaranteed
- * that the asynchronous work has either been cancelled (if the driver
- * supports it, which at least requires that the framebuffers get
- * cleaned up with drm_atomic_helper_cleanup_planes()) or completed
- * before the new state gets committed on the software side with
- * drm_atomic_helper_swap_state().
- *
- * This scheme allows new atomic state updates to be prepared and
- * checked in parallel to the asynchronous completion of the previous
- * update. Which is important since compositors need to figure out the
- * composition of the next frame right after having submitted the
- * current layout.
- */
-
- drm_atomic_state_get(state);
- if (nonblock)
- queue_work(system_unbound_wq, &state->commit_work);
- else
- commit_tail(state);
-
- return 0;
-
-error:
- drm_atomic_helper_cleanup_planes(dev, state);
- return ret;
-}
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index d1a45a68c287..eb5569d4dd57 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -41,7 +41,11 @@ static const struct drm_mode_config_funcs mode_config_funcs = {
.fb_create = msm_framebuffer_create,
.output_poll_changed = drm_fb_helper_output_poll_changed,
.atomic_check = drm_atomic_helper_check,
- .atomic_commit = msm_atomic_commit,
+ .atomic_commit = drm_atomic_helper_commit,
+};
+
+static const struct drm_mode_config_helper_funcs mode_config_helper_funcs = {
+ .atomic_commit_tail = msm_atomic_commit_tail,
};
static inline
@@ -520,6 +524,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
}
ddev->mode_config.funcs = &mode_config_funcs;
+ ddev->mode_config.helper_private = &mode_config_helper_funcs;
ret = drm_vblank_init(ddev, priv->num_crtcs);
if (ret < 0) {
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index b89fa5186eb8..3e89d7eac02f 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -156,8 +156,7 @@ struct msm_format {
uint32_t pixel_format;
};
-int msm_atomic_commit(struct drm_device *dev,
- struct drm_atomic_state *state, bool nonblock);
+void msm_atomic_commit_tail(struct drm_atomic_state *state);
void msm_gem_unmap_vma(struct msm_gem_address_space *aspace,
struct msm_gem_vma *vma, struct sg_table *sgt);
--
Sean Paul, Software Engineer, Google / Chromium OS
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/6] drm/msm: Use drm_private_obj/state instead of subclassing
2018-03-28 19:06 ` Sean Paul
@ 2018-04-02 5:01 ` Archit Taneja
-1 siblings, 0 replies; 21+ messages in thread
From: Archit Taneja @ 2018-04-02 5:01 UTC (permalink / raw)
To: Sean Paul, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: jsanka-sgV2jX0FEOL9JmXXK+q4OQ, robdclark-Re5JQEeQqe8AvxtiuMwx3w,
hoegsberg-F7+t8E8rja9g9hUCZPvPmw,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Thursday 29 March 2018 12:36 AM, Sean Paul wrote:
> Now that we have private state handled by the core, we can use those
> instead of rolling our own swap_state for private data.
>
> Originally posted here: https://patchwork.freedesktop.org/patch/211361/
>
> Changes in v2:
> - Use state->state in disp duplicate_state callback (Jeykumar)
> Changes in v3:
> - Update comment describing msm_kms_state (Jeykumar)
> Changes in v4:
> - Rebased on msm-next
> - Don't always use private state from atomic state (Archit)
> - Renamed some of the state accessors
> - Tested on mdp5 db410c
> Changes in v5:
> - None
>
> Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
> Cc: Archit Taneja <architt@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 77 ++++++++++---------
> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 11 +--
> drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c | 12 ++-
> drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c | 28 ++++---
> drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 19 +++--
> drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h | 4 +-
> drivers/gpu/drm/msm/msm_atomic.c | 37 ---------
> drivers/gpu/drm/msm/msm_drv.c | 87 +++++++++++++++++++++-
> drivers/gpu/drm/msm/msm_drv.h | 3 -
> drivers/gpu/drm/msm/msm_kms.h | 21 ++++--
> 10 files changed, 183 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 6d8e3a9a6fc0..366670043190 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -70,60 +70,62 @@ static int mdp5_hw_init(struct msm_kms *kms)
> return 0;
> }
>
> -struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
> +struct mdp5_state *mdp5_state_from_atomic(struct drm_atomic_state *state)
> {
> - struct msm_drm_private *priv = s->dev->dev_private;
> - struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> - struct msm_kms_state *state = to_kms_state(s);
> - struct mdp5_state *new_state;
> - int ret;
> + struct msm_kms_state *kms_state = msm_kms_state_from_atomic(state);
>
> - if (state->state)
> - return state->state;
> + if (IS_ERR_OR_NULL(kms_state))
> + return (struct mdp5_state *)kms_state; /* casting ERR_PTR */
>
> - ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx);
> - if (ret)
> - return ERR_PTR(ret);
> + return kms_state->state;
> +}
>
> - new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
> - if (!new_state)
> - return ERR_PTR(-ENOMEM);
> +struct mdp5_state *mdp5_state_from_dev(struct drm_device *dev)
> +{
> + struct msm_kms_state *kms_state = msm_kms_state_from_dev(dev);
>
> - /* Copy state: */
> - new_state->hwpipe = mdp5_kms->state->hwpipe;
> - new_state->hwmixer = mdp5_kms->state->hwmixer;
> - if (mdp5_kms->smp)
> - new_state->smp = mdp5_kms->state->smp;
> + if (IS_ERR_OR_NULL(kms_state))
> + return (struct mdp5_state *)kms_state; /* casting ERR_PTR */
>
> - state->state = new_state;
> + return kms_state->state;
> +}
> +
> +static void *mdp5_duplicate_state(void *state)
> +{
> + if (!state)
> + return kzalloc(sizeof(struct mdp5_state), GFP_KERNEL);
>
> - return new_state;
> + return kmemdup(state, sizeof(struct mdp5_state), GFP_KERNEL);
> }
>
> -static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state *state)
> +static void mdp5_destroy_state(void *state)
> {
> - struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
> - swap(to_kms_state(state)->state, mdp5_kms->state);
> + struct mdp5_state *mdp_state = (struct mdp5_state *)state;
> + kfree(mdp_state);
> }
>
> -static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *state)
> +static void mdp5_prepare_commit(struct msm_kms *kms,
> + struct drm_atomic_state *old_state)
> {
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
> + struct mdp5_state *mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
> struct device *dev = &mdp5_kms->pdev->dev;
>
> pm_runtime_get_sync(dev);
>
> if (mdp5_kms->smp)
> - mdp5_smp_prepare_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
> + mdp5_smp_prepare_commit(mdp5_kms->smp, &mdp5_state->smp);
> }
>
> -static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state *state)
> +static void mdp5_complete_commit(struct msm_kms *kms,
> + struct drm_atomic_state *old_state)
> {
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
> + struct mdp5_state *mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
> struct device *dev = &mdp5_kms->pdev->dev;
>
> if (mdp5_kms->smp)
> - mdp5_smp_complete_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
> + mdp5_smp_complete_commit(mdp5_kms->smp, &mdp5_state->smp);
>
> pm_runtime_put_sync(dev);
> }
> @@ -229,7 +231,8 @@ static const struct mdp_kms_funcs kms_funcs = {
> .irq = mdp5_irq,
> .enable_vblank = mdp5_enable_vblank,
> .disable_vblank = mdp5_disable_vblank,
> - .swap_state = mdp5_swap_state,
> + .duplicate_state = mdp5_duplicate_state,
> + .destroy_state = mdp5_destroy_state,
> .prepare_commit = mdp5_prepare_commit,
> .complete_commit = mdp5_complete_commit,
> .wait_for_crtc_commit_done = mdp5_wait_for_crtc_commit_done,
> @@ -726,8 +729,6 @@ static void mdp5_destroy(struct platform_device *pdev)
>
> if (mdp5_kms->rpm_enabled)
> pm_runtime_disable(&pdev->dev);
> -
> - kfree(mdp5_kms->state);
> }
>
> static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt,
> @@ -859,7 +860,8 @@ static int interface_init(struct mdp5_kms *mdp5_kms)
> return 0;
> }
>
> -static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
> +static int mdp5_init(struct platform_device *pdev, struct drm_device *dev,
> + struct msm_kms_state *initial_state)
> {
> struct msm_drm_private *priv = dev->dev_private;
> struct mdp5_kms *mdp5_kms;
> @@ -880,9 +882,8 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
> mdp5_kms->dev = dev;
> mdp5_kms->pdev = pdev;
>
> - drm_modeset_lock_init(&mdp5_kms->state_lock);
> - mdp5_kms->state = kzalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
> - if (!mdp5_kms->state) {
> + initial_state->state = mdp5_duplicate_state(NULL);
> + if (!initial_state->state) {
> ret = -ENOMEM;
> goto fail;
> }
> @@ -940,7 +941,8 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
> * this section initializes the SMP:
> */
> if (mdp5_kms->caps & MDP_CAP_SMP) {
> - mdp5_kms->smp = mdp5_smp_init(mdp5_kms, &config->hw->smp);
> + mdp5_kms->smp = mdp5_smp_init(mdp5_kms, &config->hw->smp,
> + initial_state->state);
> if (IS_ERR(mdp5_kms->smp)) {
> ret = PTR_ERR(mdp5_kms->smp);
> mdp5_kms->smp = NULL;
> @@ -980,10 +982,11 @@ static int mdp5_bind(struct device *dev, struct device *master, void *data)
> {
> struct drm_device *ddev = dev_get_drvdata(master);
> struct platform_device *pdev = to_platform_device(dev);
> + struct msm_kms_state *initial_state = (struct msm_kms_state *)data;
>
> DBG("");
>
> - return mdp5_init(pdev, ddev);
> + return mdp5_init(pdev, ddev, initial_state);
> }
>
> static void mdp5_unbind(struct device *dev, struct device *master,
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
> index 425a03d213e5..e23117b82aad 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
> @@ -28,8 +28,6 @@
> #include "mdp5_ctl.h"
> #include "mdp5_smp.h"
>
> -struct mdp5_state;
> -
> struct mdp5_kms {
> struct mdp_kms base;
>
> @@ -49,12 +47,6 @@ struct mdp5_kms {
> struct mdp5_cfg_handler *cfg;
> uint32_t caps; /* MDP capabilities (MDP_CAP_XXX bits) */
>
> - /**
> - * Global atomic state. Do not access directly, use mdp5_get_state()
> - */
> - struct mdp5_state *state;
> - struct drm_modeset_lock state_lock;
> -
> struct mdp5_smp *smp;
> struct mdp5_ctl_manager *ctlm;
>
> @@ -93,7 +85,8 @@ struct mdp5_state {
> };
>
> struct mdp5_state *__must_check
> -mdp5_get_state(struct drm_atomic_state *s);
> +mdp5_state_from_atomic(struct drm_atomic_state *s);
> +struct mdp5_state *__must_check mdp5_state_from_dev(struct drm_device *dev);
>
> /* Atomic plane state. Subclasses the base drm_plane_state in order to
> * track assigned hwpipe and hw specific state.
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
> index 8a00991f03c7..fdd5e12a9d56 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
> @@ -52,10 +52,11 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc,
> {
> struct msm_drm_private *priv = s->dev->dev_private;
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> - struct mdp5_state *state = mdp5_get_state(s);
> + struct mdp5_state *state;
> struct mdp5_hw_mixer_state *new_state;
> int i;
>
> + state = mdp5_state_from_atomic(s);
> if (IS_ERR(state))
> return PTR_ERR(state);
>
> @@ -129,12 +130,17 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc,
>
> void mdp5_mixer_release(struct drm_atomic_state *s, struct mdp5_hw_mixer *mixer)
> {
> - struct mdp5_state *state = mdp5_get_state(s);
> - struct mdp5_hw_mixer_state *new_state = &state->hwmixer;
> + struct mdp5_state *state;
> + struct mdp5_hw_mixer_state *new_state;
>
> if (!mixer)
> return;
>
> + state = mdp5_state_from_atomic(s);
> + if (IS_ERR(state))
> + return;
> +
> + new_state = &state->hwmixer;
> if (WARN_ON(!new_state->hwmixer_to_crtc[mixer->idx]))
> return;
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
> index ff52c49095f9..dc66ab9fdd50 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
> @@ -24,17 +24,20 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
> {
> struct msm_drm_private *priv = s->dev->dev_private;
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> - struct mdp5_state *state;
> + struct mdp5_state *old_mdp5_state, *new_mdp5_state;
> struct mdp5_hw_pipe_state *old_state, *new_state;
> int i, j;
>
> - state = mdp5_get_state(s);
> - if (IS_ERR(state))
> - return PTR_ERR(state);
> + new_mdp5_state = mdp5_state_from_atomic(s);
> + if (IS_ERR(new_mdp5_state))
> + return PTR_ERR(new_mdp5_state);
>
> - /* grab old_state after mdp5_get_state(), since now we hold lock: */
> - old_state = &mdp5_kms->state->hwpipe;
> - new_state = &state->hwpipe;
> + old_mdp5_state = mdp5_state_from_dev(s->dev);
> + if (IS_ERR(old_mdp5_state))
> + return PTR_ERR(old_mdp5_state);
> +
> + old_state = &old_mdp5_state->hwpipe;
> + new_state = &new_mdp5_state->hwpipe;
>
> for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
> struct mdp5_hw_pipe *cur = mdp5_kms->hwpipes[i];
> @@ -107,7 +110,7 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
> WARN_ON(r_hwpipe);
>
> DBG("%s: alloc SMP blocks", (*hwpipe)->name);
> - ret = mdp5_smp_assign(mdp5_kms->smp, &state->smp,
> + ret = mdp5_smp_assign(mdp5_kms->smp, &new_mdp5_state->smp,
> (*hwpipe)->pipe, blkcfg);
> if (ret)
> return -ENOMEM;
> @@ -132,12 +135,17 @@ void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe)
> {
> struct msm_drm_private *priv = s->dev->dev_private;
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> - struct mdp5_state *state = mdp5_get_state(s);
> - struct mdp5_hw_pipe_state *new_state = &state->hwpipe;
> + struct mdp5_state *state;
> + struct mdp5_hw_pipe_state *new_state;
>
> if (!hwpipe)
> return;
>
> + state = mdp5_state_from_atomic(s);
> + if (IS_ERR(state))
> + return;
> +
> + new_state = &state->hwpipe;
> if (WARN_ON(!new_state->hwpipe_to_plane[hwpipe->idx]))
> return;
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
> index ae4983d9d0a5..918e4123e781 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
> @@ -338,6 +338,7 @@ void mdp5_smp_complete_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state
> void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
> {
> struct mdp5_kms *mdp5_kms = get_kms(smp);
> + struct mdp5_state *mdp5_state;
> struct mdp5_hw_pipe_state *hwpstate;
> struct mdp5_smp_state *state;
> int total = 0, i, j;
> @@ -346,11 +347,12 @@ void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
> drm_printf(p, "----\t-----\t-----\n");
>
> if (drm_can_sleep())
> - drm_modeset_lock(&mdp5_kms->state_lock, NULL);
> + drm_modeset_lock_all(mdp5_kms->dev);
>
> - /* grab these *after* we hold the state_lock */
> - hwpstate = &mdp5_kms->state->hwpipe;
> - state = &mdp5_kms->state->smp;
> + /* grab these *after* we hold the modeset_lock */
> + mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
> + hwpstate = &mdp5_state->hwpipe;
> + state = &mdp5_state->smp;
>
> for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
> struct mdp5_hw_pipe *hwpipe = mdp5_kms->hwpipes[i];
> @@ -374,7 +376,7 @@ void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
> bitmap_weight(state->state, smp->blk_cnt));
>
> if (drm_can_sleep())
> - drm_modeset_unlock(&mdp5_kms->state_lock);
> + drm_modeset_unlock_all(mdp5_kms->dev);
> }
>
> void mdp5_smp_destroy(struct mdp5_smp *smp)
> @@ -382,12 +384,15 @@ void mdp5_smp_destroy(struct mdp5_smp *smp)
> kfree(smp);
> }
>
> -struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms, const struct mdp5_smp_block *cfg)
> +struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms,
> + const struct mdp5_smp_block *cfg,
> + struct mdp5_state *mdp5_state)
> {
> - struct mdp5_smp_state *state = &mdp5_kms->state->smp;
> + struct mdp5_smp_state *state;
> struct mdp5_smp *smp = NULL;
> int ret;
>
> + state = &mdp5_state->smp;
> smp = kzalloc(sizeof(*smp), GFP_KERNEL);
> if (unlikely(!smp)) {
> ret = -ENOMEM;
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
> index b41d0448fbe8..1bfa22b63a2d 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
> @@ -69,6 +69,7 @@ struct mdp5_smp_state {
> };
>
> struct mdp5_kms;
> +struct mdp5_state;
> struct mdp5_smp;
>
> /*
> @@ -78,7 +79,8 @@ struct mdp5_smp;
> */
>
> struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms,
> - const struct mdp5_smp_block *cfg);
> + const struct mdp5_smp_block *cfg,
> + struct mdp5_state *mdp5_state);
> void mdp5_smp_destroy(struct mdp5_smp *smp);
>
> void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p);
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
> index bf5f8c39f34d..e792158676aa 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -220,16 +220,6 @@ int msm_atomic_commit(struct drm_device *dev,
>
> BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
>
> - /*
> - * This is the point of no return - everything below never fails except
> - * when the hw goes bonghits. Which means we can commit the new state on
> - * the software side now.
> - *
> - * swap driver private state while still holding state_lock
> - */
> - if (to_kms_state(state)->state)
> - priv->kms->funcs->swap_state(priv->kms, state);
> -
> /*
> * Everything below can be run asynchronously without the need to grab
> * any modeset locks at all under one conditions: It must be guaranteed
> @@ -262,30 +252,3 @@ int msm_atomic_commit(struct drm_device *dev,
> drm_atomic_helper_cleanup_planes(dev, state);
> return ret;
> }
> -
> -struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev)
> -{
> - struct msm_kms_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
> -
> - if (!state || drm_atomic_state_init(dev, &state->base) < 0) {
> - kfree(state);
> - return NULL;
> - }
> -
> - return &state->base;
> -}
> -
> -void msm_atomic_state_clear(struct drm_atomic_state *s)
> -{
> - struct msm_kms_state *state = to_kms_state(s);
> - drm_atomic_state_default_clear(&state->base);
> - kfree(state->state);
> - state->state = NULL;
> -}
> -
> -void msm_atomic_state_free(struct drm_atomic_state *state)
> -{
> - kfree(to_kms_state(state)->state);
> - drm_atomic_state_default_release(state);
> - kfree(state);
> -}
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 30cd514d8f7c..7e8f640062ef 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -42,11 +42,79 @@ static const struct drm_mode_config_funcs mode_config_funcs = {
> .output_poll_changed = drm_fb_helper_output_poll_changed,
> .atomic_check = drm_atomic_helper_check,
> .atomic_commit = msm_atomic_commit,
> - .atomic_state_alloc = msm_atomic_state_alloc,
> - .atomic_state_clear = msm_atomic_state_clear,
> - .atomic_state_free = msm_atomic_state_free,
> };
>
> +static inline
> +struct msm_kms *msm_kms_from_obj(struct drm_private_obj *obj)
> +{
> + return container_of(obj, struct msm_kms, base);
> +}
> +
> +static inline
> +struct msm_kms_state *msm_kms_state_from_priv(struct drm_private_state *priv)
> +{
> + return container_of(priv, struct msm_kms_state, base);
> +}
> +
> +
> +struct msm_kms_state *msm_kms_state_from_atomic(struct drm_atomic_state *state)
> +{
> + struct msm_drm_private *priv = state->dev->dev_private;
> + struct drm_private_obj *obj = &priv->kms->base;
> + struct drm_private_state *priv_state;
> +
> + priv_state = drm_atomic_get_private_obj_state(state, obj);
> + if (IS_ERR_OR_NULL(priv_state))
> + return (struct msm_kms_state *)state; /* casting ERR_PTR */
> +
> + return msm_kms_state_from_priv(priv_state);
> +}
> +
> +struct msm_kms_state *msm_kms_state_from_dev(struct drm_device *dev)
> +{
> + struct msm_drm_private *priv = dev->dev_private;
> + struct drm_private_obj *obj = &priv->kms->base;
> + struct drm_private_state *priv_state = obj->state;
> +
> + return msm_kms_state_from_priv(priv_state);
> +}
> +
> +static struct drm_private_state *
> +msm_kms_duplicate_state(struct drm_private_obj *obj)
> +{
> + struct msm_kms *kms = msm_kms_from_obj(obj);
> + struct msm_kms_state *state = msm_kms_state_from_priv(obj->state);
> + struct msm_kms_state *new_state;
> +
> + new_state = kzalloc(sizeof(*new_state), GFP_KERNEL);
> + if (!new_state)
> + return NULL;
> +
> + if (kms->funcs->duplicate_state)
> + new_state->state = kms->funcs->duplicate_state(state->state);
> +
> + __drm_atomic_helper_private_obj_duplicate_state(obj, &new_state->base);
> +
> + return &new_state->base;
> +}
> +
> +static void msm_kms_destroy_state(struct drm_private_obj *obj,
> + struct drm_private_state *priv_state)
> +{
> + struct msm_kms *kms = msm_kms_from_obj(obj);
> + struct msm_kms_state *state = msm_kms_state_from_priv(priv_state);
> +
> + if (kms->funcs->destroy_state)
> + kms->funcs->destroy_state(state->state);
> + kfree(state);
> +}
> +
> +static const struct drm_private_state_funcs kms_state_funcs = {
> + .atomic_duplicate_state = msm_kms_duplicate_state,
> + .atomic_destroy_state = msm_kms_destroy_state,
> +};
> +
> +
Minor nit: Two blank lines here^
> #ifdef CONFIG_DRM_MSM_REGISTER_LOGGING
> static bool reglog = false;
> MODULE_PARM_DESC(reglog, "Enable register read/write logging");
> @@ -245,6 +313,8 @@ static int msm_drm_uninit(struct device *dev)
> flush_workqueue(priv->atomic_wq);
> destroy_workqueue(priv->atomic_wq);
>
> + drm_atomic_private_obj_fini(&kms->base);
> +
> if (kms && kms->funcs)
> kms->funcs->destroy(kms);
>
> @@ -356,6 +426,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
> struct drm_device *ddev;
> struct msm_drm_private *priv;
> struct msm_kms *kms;
> + struct msm_kms_state *initial_state;
> int ret;
>
> ddev = drm_dev_alloc(drv, dev);
> @@ -364,6 +435,10 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
> return PTR_ERR(ddev);
> }
>
> + initial_state = kzalloc(sizeof(*initial_state), GFP_KERNEL);
> + if (!initial_state)
> + return -ENOMEM;
> +
> platform_set_drvdata(pdev, ddev);
>
> priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> @@ -394,7 +469,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
> drm_mode_config_init(ddev);
>
> /* Bind all our sub-components: */
> - ret = component_bind_all(dev, ddev);
> + ret = component_bind_all(dev, initial_state);
Only the core kms component (mdp4/mdp5/dpu) will use initial_state,
and the encoders drivers (DSI/HDMI) would discard it, right? I guess
it's still better than passing ddev, which can be derived from the
component master.
Reviewed-by: Archit Taneja <architt@codeaurora.org>
> if (ret) {
> msm_mdss_destroy(ddev);
> kfree(priv);
> @@ -433,6 +508,10 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
> goto fail;
> }
>
> + drm_atomic_private_obj_init(&kms->base,
> + &initial_state->base,
> + &kms_state_funcs);
> +
> if (kms) {
> ret = kms->funcs->hw_init(kms);
> if (ret) {
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 48ed5b9a8580..c5b8c989b859 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -162,9 +162,6 @@ struct msm_format {
>
> int msm_atomic_commit(struct drm_device *dev,
> struct drm_atomic_state *state, bool nonblock);
> -struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev);
> -void msm_atomic_state_clear(struct drm_atomic_state *state);
> -void msm_atomic_state_free(struct drm_atomic_state *state);
>
> void msm_gem_unmap_vma(struct msm_gem_address_space *aspace,
> struct msm_gem_vma *vma, struct sg_table *sgt);
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index 17d5824417ad..24d09fcebf16 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -42,6 +42,8 @@ struct msm_kms_funcs {
> void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc);
> /* swap global atomic state: */
> void (*swap_state)(struct msm_kms *kms, struct drm_atomic_state *state);
> + void *(*duplicate_state)(void *state);
> + void (*destroy_state)(void *state);
> /* modeset, bracketing atomic_commit(): */
> void (*prepare_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
> void (*complete_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
> @@ -68,6 +70,8 @@ struct msm_kms_funcs {
> };
>
> struct msm_kms {
> + struct drm_private_obj base;
> +
> const struct msm_kms_funcs *funcs;
>
> /* irq number to be passed on to drm_irq_install */
> @@ -78,16 +82,23 @@ struct msm_kms {
> };
>
> /**
> - * Subclass of drm_atomic_state, to allow kms backend to have driver
> + * Subclass of drm_private_state, to allow kms backend to have driver
> * private global state. The kms backend can do whatever it wants
> - * with the ->state ptr. On ->atomic_state_clear() the ->state ptr
> - * is kfree'd and set back to NULL.
> + * with the ->state ptr.
> */
> struct msm_kms_state {
> - struct drm_atomic_state base;
> + struct drm_private_state base;
> void *state;
> };
> -#define to_kms_state(x) container_of(x, struct msm_kms_state, base)
> +
> +/**
> + * Extracts the msm_kms_state from either an atomic state, or the current
> + * device. One or the other might be desirable depending on whether you want the
> + * currently configured msm_kms_state (from_dev), or you would like the state
> + * which is about to be applied (from_atomic).
> + */
> +struct msm_kms_state *msm_kms_state_from_dev(struct drm_device *dev);
> +struct msm_kms_state *msm_kms_state_from_atomic(struct drm_atomic_state *state);
>
> static inline void msm_kms_init(struct msm_kms *kms,
> const struct msm_kms_funcs *funcs)
>
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 1/6] drm/msm: Use drm_private_obj/state instead of subclassing
@ 2018-04-02 5:01 ` Archit Taneja
0 siblings, 0 replies; 21+ messages in thread
From: Archit Taneja @ 2018-04-02 5:01 UTC (permalink / raw)
To: Sean Paul, freedreno, linux-arm-msm, dri-devel
Cc: robdclark, hoegsberg, jsanka, abhinavk, linux-kernel
On Thursday 29 March 2018 12:36 AM, Sean Paul wrote:
> Now that we have private state handled by the core, we can use those
> instead of rolling our own swap_state for private data.
>
> Originally posted here: https://patchwork.freedesktop.org/patch/211361/
>
> Changes in v2:
> - Use state->state in disp duplicate_state callback (Jeykumar)
> Changes in v3:
> - Update comment describing msm_kms_state (Jeykumar)
> Changes in v4:
> - Rebased on msm-next
> - Don't always use private state from atomic state (Archit)
> - Renamed some of the state accessors
> - Tested on mdp5 db410c
> Changes in v5:
> - None
>
> Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
> Cc: Archit Taneja <architt@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 77 ++++++++++---------
> drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 11 +--
> drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c | 12 ++-
> drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c | 28 ++++---
> drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 19 +++--
> drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h | 4 +-
> drivers/gpu/drm/msm/msm_atomic.c | 37 ---------
> drivers/gpu/drm/msm/msm_drv.c | 87 +++++++++++++++++++++-
> drivers/gpu/drm/msm/msm_drv.h | 3 -
> drivers/gpu/drm/msm/msm_kms.h | 21 ++++--
> 10 files changed, 183 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> index 6d8e3a9a6fc0..366670043190 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
> @@ -70,60 +70,62 @@ static int mdp5_hw_init(struct msm_kms *kms)
> return 0;
> }
>
> -struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
> +struct mdp5_state *mdp5_state_from_atomic(struct drm_atomic_state *state)
> {
> - struct msm_drm_private *priv = s->dev->dev_private;
> - struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> - struct msm_kms_state *state = to_kms_state(s);
> - struct mdp5_state *new_state;
> - int ret;
> + struct msm_kms_state *kms_state = msm_kms_state_from_atomic(state);
>
> - if (state->state)
> - return state->state;
> + if (IS_ERR_OR_NULL(kms_state))
> + return (struct mdp5_state *)kms_state; /* casting ERR_PTR */
>
> - ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx);
> - if (ret)
> - return ERR_PTR(ret);
> + return kms_state->state;
> +}
>
> - new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
> - if (!new_state)
> - return ERR_PTR(-ENOMEM);
> +struct mdp5_state *mdp5_state_from_dev(struct drm_device *dev)
> +{
> + struct msm_kms_state *kms_state = msm_kms_state_from_dev(dev);
>
> - /* Copy state: */
> - new_state->hwpipe = mdp5_kms->state->hwpipe;
> - new_state->hwmixer = mdp5_kms->state->hwmixer;
> - if (mdp5_kms->smp)
> - new_state->smp = mdp5_kms->state->smp;
> + if (IS_ERR_OR_NULL(kms_state))
> + return (struct mdp5_state *)kms_state; /* casting ERR_PTR */
>
> - state->state = new_state;
> + return kms_state->state;
> +}
> +
> +static void *mdp5_duplicate_state(void *state)
> +{
> + if (!state)
> + return kzalloc(sizeof(struct mdp5_state), GFP_KERNEL);
>
> - return new_state;
> + return kmemdup(state, sizeof(struct mdp5_state), GFP_KERNEL);
> }
>
> -static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state *state)
> +static void mdp5_destroy_state(void *state)
> {
> - struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
> - swap(to_kms_state(state)->state, mdp5_kms->state);
> + struct mdp5_state *mdp_state = (struct mdp5_state *)state;
> + kfree(mdp_state);
> }
>
> -static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *state)
> +static void mdp5_prepare_commit(struct msm_kms *kms,
> + struct drm_atomic_state *old_state)
> {
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
> + struct mdp5_state *mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
> struct device *dev = &mdp5_kms->pdev->dev;
>
> pm_runtime_get_sync(dev);
>
> if (mdp5_kms->smp)
> - mdp5_smp_prepare_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
> + mdp5_smp_prepare_commit(mdp5_kms->smp, &mdp5_state->smp);
> }
>
> -static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state *state)
> +static void mdp5_complete_commit(struct msm_kms *kms,
> + struct drm_atomic_state *old_state)
> {
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
> + struct mdp5_state *mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
> struct device *dev = &mdp5_kms->pdev->dev;
>
> if (mdp5_kms->smp)
> - mdp5_smp_complete_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
> + mdp5_smp_complete_commit(mdp5_kms->smp, &mdp5_state->smp);
>
> pm_runtime_put_sync(dev);
> }
> @@ -229,7 +231,8 @@ static const struct mdp_kms_funcs kms_funcs = {
> .irq = mdp5_irq,
> .enable_vblank = mdp5_enable_vblank,
> .disable_vblank = mdp5_disable_vblank,
> - .swap_state = mdp5_swap_state,
> + .duplicate_state = mdp5_duplicate_state,
> + .destroy_state = mdp5_destroy_state,
> .prepare_commit = mdp5_prepare_commit,
> .complete_commit = mdp5_complete_commit,
> .wait_for_crtc_commit_done = mdp5_wait_for_crtc_commit_done,
> @@ -726,8 +729,6 @@ static void mdp5_destroy(struct platform_device *pdev)
>
> if (mdp5_kms->rpm_enabled)
> pm_runtime_disable(&pdev->dev);
> -
> - kfree(mdp5_kms->state);
> }
>
> static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt,
> @@ -859,7 +860,8 @@ static int interface_init(struct mdp5_kms *mdp5_kms)
> return 0;
> }
>
> -static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
> +static int mdp5_init(struct platform_device *pdev, struct drm_device *dev,
> + struct msm_kms_state *initial_state)
> {
> struct msm_drm_private *priv = dev->dev_private;
> struct mdp5_kms *mdp5_kms;
> @@ -880,9 +882,8 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
> mdp5_kms->dev = dev;
> mdp5_kms->pdev = pdev;
>
> - drm_modeset_lock_init(&mdp5_kms->state_lock);
> - mdp5_kms->state = kzalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
> - if (!mdp5_kms->state) {
> + initial_state->state = mdp5_duplicate_state(NULL);
> + if (!initial_state->state) {
> ret = -ENOMEM;
> goto fail;
> }
> @@ -940,7 +941,8 @@ static int mdp5_init(struct platform_device *pdev, struct drm_device *dev)
> * this section initializes the SMP:
> */
> if (mdp5_kms->caps & MDP_CAP_SMP) {
> - mdp5_kms->smp = mdp5_smp_init(mdp5_kms, &config->hw->smp);
> + mdp5_kms->smp = mdp5_smp_init(mdp5_kms, &config->hw->smp,
> + initial_state->state);
> if (IS_ERR(mdp5_kms->smp)) {
> ret = PTR_ERR(mdp5_kms->smp);
> mdp5_kms->smp = NULL;
> @@ -980,10 +982,11 @@ static int mdp5_bind(struct device *dev, struct device *master, void *data)
> {
> struct drm_device *ddev = dev_get_drvdata(master);
> struct platform_device *pdev = to_platform_device(dev);
> + struct msm_kms_state *initial_state = (struct msm_kms_state *)data;
>
> DBG("");
>
> - return mdp5_init(pdev, ddev);
> + return mdp5_init(pdev, ddev, initial_state);
> }
>
> static void mdp5_unbind(struct device *dev, struct device *master,
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
> index 425a03d213e5..e23117b82aad 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
> @@ -28,8 +28,6 @@
> #include "mdp5_ctl.h"
> #include "mdp5_smp.h"
>
> -struct mdp5_state;
> -
> struct mdp5_kms {
> struct mdp_kms base;
>
> @@ -49,12 +47,6 @@ struct mdp5_kms {
> struct mdp5_cfg_handler *cfg;
> uint32_t caps; /* MDP capabilities (MDP_CAP_XXX bits) */
>
> - /**
> - * Global atomic state. Do not access directly, use mdp5_get_state()
> - */
> - struct mdp5_state *state;
> - struct drm_modeset_lock state_lock;
> -
> struct mdp5_smp *smp;
> struct mdp5_ctl_manager *ctlm;
>
> @@ -93,7 +85,8 @@ struct mdp5_state {
> };
>
> struct mdp5_state *__must_check
> -mdp5_get_state(struct drm_atomic_state *s);
> +mdp5_state_from_atomic(struct drm_atomic_state *s);
> +struct mdp5_state *__must_check mdp5_state_from_dev(struct drm_device *dev);
>
> /* Atomic plane state. Subclasses the base drm_plane_state in order to
> * track assigned hwpipe and hw specific state.
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
> index 8a00991f03c7..fdd5e12a9d56 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
> @@ -52,10 +52,11 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc,
> {
> struct msm_drm_private *priv = s->dev->dev_private;
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> - struct mdp5_state *state = mdp5_get_state(s);
> + struct mdp5_state *state;
> struct mdp5_hw_mixer_state *new_state;
> int i;
>
> + state = mdp5_state_from_atomic(s);
> if (IS_ERR(state))
> return PTR_ERR(state);
>
> @@ -129,12 +130,17 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct drm_crtc *crtc,
>
> void mdp5_mixer_release(struct drm_atomic_state *s, struct mdp5_hw_mixer *mixer)
> {
> - struct mdp5_state *state = mdp5_get_state(s);
> - struct mdp5_hw_mixer_state *new_state = &state->hwmixer;
> + struct mdp5_state *state;
> + struct mdp5_hw_mixer_state *new_state;
>
> if (!mixer)
> return;
>
> + state = mdp5_state_from_atomic(s);
> + if (IS_ERR(state))
> + return;
> +
> + new_state = &state->hwmixer;
> if (WARN_ON(!new_state->hwmixer_to_crtc[mixer->idx]))
> return;
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
> index ff52c49095f9..dc66ab9fdd50 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
> @@ -24,17 +24,20 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
> {
> struct msm_drm_private *priv = s->dev->dev_private;
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> - struct mdp5_state *state;
> + struct mdp5_state *old_mdp5_state, *new_mdp5_state;
> struct mdp5_hw_pipe_state *old_state, *new_state;
> int i, j;
>
> - state = mdp5_get_state(s);
> - if (IS_ERR(state))
> - return PTR_ERR(state);
> + new_mdp5_state = mdp5_state_from_atomic(s);
> + if (IS_ERR(new_mdp5_state))
> + return PTR_ERR(new_mdp5_state);
>
> - /* grab old_state after mdp5_get_state(), since now we hold lock: */
> - old_state = &mdp5_kms->state->hwpipe;
> - new_state = &state->hwpipe;
> + old_mdp5_state = mdp5_state_from_dev(s->dev);
> + if (IS_ERR(old_mdp5_state))
> + return PTR_ERR(old_mdp5_state);
> +
> + old_state = &old_mdp5_state->hwpipe;
> + new_state = &new_mdp5_state->hwpipe;
>
> for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
> struct mdp5_hw_pipe *cur = mdp5_kms->hwpipes[i];
> @@ -107,7 +110,7 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct drm_plane *plane,
> WARN_ON(r_hwpipe);
>
> DBG("%s: alloc SMP blocks", (*hwpipe)->name);
> - ret = mdp5_smp_assign(mdp5_kms->smp, &state->smp,
> + ret = mdp5_smp_assign(mdp5_kms->smp, &new_mdp5_state->smp,
> (*hwpipe)->pipe, blkcfg);
> if (ret)
> return -ENOMEM;
> @@ -132,12 +135,17 @@ void mdp5_pipe_release(struct drm_atomic_state *s, struct mdp5_hw_pipe *hwpipe)
> {
> struct msm_drm_private *priv = s->dev->dev_private;
> struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
> - struct mdp5_state *state = mdp5_get_state(s);
> - struct mdp5_hw_pipe_state *new_state = &state->hwpipe;
> + struct mdp5_state *state;
> + struct mdp5_hw_pipe_state *new_state;
>
> if (!hwpipe)
> return;
>
> + state = mdp5_state_from_atomic(s);
> + if (IS_ERR(state))
> + return;
> +
> + new_state = &state->hwpipe;
> if (WARN_ON(!new_state->hwpipe_to_plane[hwpipe->idx]))
> return;
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
> index ae4983d9d0a5..918e4123e781 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c
> @@ -338,6 +338,7 @@ void mdp5_smp_complete_commit(struct mdp5_smp *smp, struct mdp5_smp_state *state
> void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
> {
> struct mdp5_kms *mdp5_kms = get_kms(smp);
> + struct mdp5_state *mdp5_state;
> struct mdp5_hw_pipe_state *hwpstate;
> struct mdp5_smp_state *state;
> int total = 0, i, j;
> @@ -346,11 +347,12 @@ void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
> drm_printf(p, "----\t-----\t-----\n");
>
> if (drm_can_sleep())
> - drm_modeset_lock(&mdp5_kms->state_lock, NULL);
> + drm_modeset_lock_all(mdp5_kms->dev);
>
> - /* grab these *after* we hold the state_lock */
> - hwpstate = &mdp5_kms->state->hwpipe;
> - state = &mdp5_kms->state->smp;
> + /* grab these *after* we hold the modeset_lock */
> + mdp5_state = mdp5_state_from_dev(mdp5_kms->dev);
> + hwpstate = &mdp5_state->hwpipe;
> + state = &mdp5_state->smp;
>
> for (i = 0; i < mdp5_kms->num_hwpipes; i++) {
> struct mdp5_hw_pipe *hwpipe = mdp5_kms->hwpipes[i];
> @@ -374,7 +376,7 @@ void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p)
> bitmap_weight(state->state, smp->blk_cnt));
>
> if (drm_can_sleep())
> - drm_modeset_unlock(&mdp5_kms->state_lock);
> + drm_modeset_unlock_all(mdp5_kms->dev);
> }
>
> void mdp5_smp_destroy(struct mdp5_smp *smp)
> @@ -382,12 +384,15 @@ void mdp5_smp_destroy(struct mdp5_smp *smp)
> kfree(smp);
> }
>
> -struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms, const struct mdp5_smp_block *cfg)
> +struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms,
> + const struct mdp5_smp_block *cfg,
> + struct mdp5_state *mdp5_state)
> {
> - struct mdp5_smp_state *state = &mdp5_kms->state->smp;
> + struct mdp5_smp_state *state;
> struct mdp5_smp *smp = NULL;
> int ret;
>
> + state = &mdp5_state->smp;
> smp = kzalloc(sizeof(*smp), GFP_KERNEL);
> if (unlikely(!smp)) {
> ret = -ENOMEM;
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
> index b41d0448fbe8..1bfa22b63a2d 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.h
> @@ -69,6 +69,7 @@ struct mdp5_smp_state {
> };
>
> struct mdp5_kms;
> +struct mdp5_state;
> struct mdp5_smp;
>
> /*
> @@ -78,7 +79,8 @@ struct mdp5_smp;
> */
>
> struct mdp5_smp *mdp5_smp_init(struct mdp5_kms *mdp5_kms,
> - const struct mdp5_smp_block *cfg);
> + const struct mdp5_smp_block *cfg,
> + struct mdp5_state *mdp5_state);
> void mdp5_smp_destroy(struct mdp5_smp *smp);
>
> void mdp5_smp_dump(struct mdp5_smp *smp, struct drm_printer *p);
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
> index bf5f8c39f34d..e792158676aa 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -220,16 +220,6 @@ int msm_atomic_commit(struct drm_device *dev,
>
> BUG_ON(drm_atomic_helper_swap_state(state, false) < 0);
>
> - /*
> - * This is the point of no return - everything below never fails except
> - * when the hw goes bonghits. Which means we can commit the new state on
> - * the software side now.
> - *
> - * swap driver private state while still holding state_lock
> - */
> - if (to_kms_state(state)->state)
> - priv->kms->funcs->swap_state(priv->kms, state);
> -
> /*
> * Everything below can be run asynchronously without the need to grab
> * any modeset locks at all under one conditions: It must be guaranteed
> @@ -262,30 +252,3 @@ int msm_atomic_commit(struct drm_device *dev,
> drm_atomic_helper_cleanup_planes(dev, state);
> return ret;
> }
> -
> -struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev)
> -{
> - struct msm_kms_state *state = kzalloc(sizeof(*state), GFP_KERNEL);
> -
> - if (!state || drm_atomic_state_init(dev, &state->base) < 0) {
> - kfree(state);
> - return NULL;
> - }
> -
> - return &state->base;
> -}
> -
> -void msm_atomic_state_clear(struct drm_atomic_state *s)
> -{
> - struct msm_kms_state *state = to_kms_state(s);
> - drm_atomic_state_default_clear(&state->base);
> - kfree(state->state);
> - state->state = NULL;
> -}
> -
> -void msm_atomic_state_free(struct drm_atomic_state *state)
> -{
> - kfree(to_kms_state(state)->state);
> - drm_atomic_state_default_release(state);
> - kfree(state);
> -}
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index 30cd514d8f7c..7e8f640062ef 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -42,11 +42,79 @@ static const struct drm_mode_config_funcs mode_config_funcs = {
> .output_poll_changed = drm_fb_helper_output_poll_changed,
> .atomic_check = drm_atomic_helper_check,
> .atomic_commit = msm_atomic_commit,
> - .atomic_state_alloc = msm_atomic_state_alloc,
> - .atomic_state_clear = msm_atomic_state_clear,
> - .atomic_state_free = msm_atomic_state_free,
> };
>
> +static inline
> +struct msm_kms *msm_kms_from_obj(struct drm_private_obj *obj)
> +{
> + return container_of(obj, struct msm_kms, base);
> +}
> +
> +static inline
> +struct msm_kms_state *msm_kms_state_from_priv(struct drm_private_state *priv)
> +{
> + return container_of(priv, struct msm_kms_state, base);
> +}
> +
> +
> +struct msm_kms_state *msm_kms_state_from_atomic(struct drm_atomic_state *state)
> +{
> + struct msm_drm_private *priv = state->dev->dev_private;
> + struct drm_private_obj *obj = &priv->kms->base;
> + struct drm_private_state *priv_state;
> +
> + priv_state = drm_atomic_get_private_obj_state(state, obj);
> + if (IS_ERR_OR_NULL(priv_state))
> + return (struct msm_kms_state *)state; /* casting ERR_PTR */
> +
> + return msm_kms_state_from_priv(priv_state);
> +}
> +
> +struct msm_kms_state *msm_kms_state_from_dev(struct drm_device *dev)
> +{
> + struct msm_drm_private *priv = dev->dev_private;
> + struct drm_private_obj *obj = &priv->kms->base;
> + struct drm_private_state *priv_state = obj->state;
> +
> + return msm_kms_state_from_priv(priv_state);
> +}
> +
> +static struct drm_private_state *
> +msm_kms_duplicate_state(struct drm_private_obj *obj)
> +{
> + struct msm_kms *kms = msm_kms_from_obj(obj);
> + struct msm_kms_state *state = msm_kms_state_from_priv(obj->state);
> + struct msm_kms_state *new_state;
> +
> + new_state = kzalloc(sizeof(*new_state), GFP_KERNEL);
> + if (!new_state)
> + return NULL;
> +
> + if (kms->funcs->duplicate_state)
> + new_state->state = kms->funcs->duplicate_state(state->state);
> +
> + __drm_atomic_helper_private_obj_duplicate_state(obj, &new_state->base);
> +
> + return &new_state->base;
> +}
> +
> +static void msm_kms_destroy_state(struct drm_private_obj *obj,
> + struct drm_private_state *priv_state)
> +{
> + struct msm_kms *kms = msm_kms_from_obj(obj);
> + struct msm_kms_state *state = msm_kms_state_from_priv(priv_state);
> +
> + if (kms->funcs->destroy_state)
> + kms->funcs->destroy_state(state->state);
> + kfree(state);
> +}
> +
> +static const struct drm_private_state_funcs kms_state_funcs = {
> + .atomic_duplicate_state = msm_kms_duplicate_state,
> + .atomic_destroy_state = msm_kms_destroy_state,
> +};
> +
> +
Minor nit: Two blank lines here^
> #ifdef CONFIG_DRM_MSM_REGISTER_LOGGING
> static bool reglog = false;
> MODULE_PARM_DESC(reglog, "Enable register read/write logging");
> @@ -245,6 +313,8 @@ static int msm_drm_uninit(struct device *dev)
> flush_workqueue(priv->atomic_wq);
> destroy_workqueue(priv->atomic_wq);
>
> + drm_atomic_private_obj_fini(&kms->base);
> +
> if (kms && kms->funcs)
> kms->funcs->destroy(kms);
>
> @@ -356,6 +426,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
> struct drm_device *ddev;
> struct msm_drm_private *priv;
> struct msm_kms *kms;
> + struct msm_kms_state *initial_state;
> int ret;
>
> ddev = drm_dev_alloc(drv, dev);
> @@ -364,6 +435,10 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
> return PTR_ERR(ddev);
> }
>
> + initial_state = kzalloc(sizeof(*initial_state), GFP_KERNEL);
> + if (!initial_state)
> + return -ENOMEM;
> +
> platform_set_drvdata(pdev, ddev);
>
> priv = kzalloc(sizeof(*priv), GFP_KERNEL);
> @@ -394,7 +469,7 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
> drm_mode_config_init(ddev);
>
> /* Bind all our sub-components: */
> - ret = component_bind_all(dev, ddev);
> + ret = component_bind_all(dev, initial_state);
Only the core kms component (mdp4/mdp5/dpu) will use initial_state,
and the encoders drivers (DSI/HDMI) would discard it, right? I guess
it's still better than passing ddev, which can be derived from the
component master.
Reviewed-by: Archit Taneja <architt@codeaurora.org>
> if (ret) {
> msm_mdss_destroy(ddev);
> kfree(priv);
> @@ -433,6 +508,10 @@ static int msm_drm_init(struct device *dev, struct drm_driver *drv)
> goto fail;
> }
>
> + drm_atomic_private_obj_init(&kms->base,
> + &initial_state->base,
> + &kms_state_funcs);
> +
> if (kms) {
> ret = kms->funcs->hw_init(kms);
> if (ret) {
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 48ed5b9a8580..c5b8c989b859 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -162,9 +162,6 @@ struct msm_format {
>
> int msm_atomic_commit(struct drm_device *dev,
> struct drm_atomic_state *state, bool nonblock);
> -struct drm_atomic_state *msm_atomic_state_alloc(struct drm_device *dev);
> -void msm_atomic_state_clear(struct drm_atomic_state *state);
> -void msm_atomic_state_free(struct drm_atomic_state *state);
>
> void msm_gem_unmap_vma(struct msm_gem_address_space *aspace,
> struct msm_gem_vma *vma, struct sg_table *sgt);
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index 17d5824417ad..24d09fcebf16 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -42,6 +42,8 @@ struct msm_kms_funcs {
> void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc);
> /* swap global atomic state: */
> void (*swap_state)(struct msm_kms *kms, struct drm_atomic_state *state);
> + void *(*duplicate_state)(void *state);
> + void (*destroy_state)(void *state);
> /* modeset, bracketing atomic_commit(): */
> void (*prepare_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
> void (*complete_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
> @@ -68,6 +70,8 @@ struct msm_kms_funcs {
> };
>
> struct msm_kms {
> + struct drm_private_obj base;
> +
> const struct msm_kms_funcs *funcs;
>
> /* irq number to be passed on to drm_irq_install */
> @@ -78,16 +82,23 @@ struct msm_kms {
> };
>
> /**
> - * Subclass of drm_atomic_state, to allow kms backend to have driver
> + * Subclass of drm_private_state, to allow kms backend to have driver
> * private global state. The kms backend can do whatever it wants
> - * with the ->state ptr. On ->atomic_state_clear() the ->state ptr
> - * is kfree'd and set back to NULL.
> + * with the ->state ptr.
> */
> struct msm_kms_state {
> - struct drm_atomic_state base;
> + struct drm_private_state base;
> void *state;
> };
> -#define to_kms_state(x) container_of(x, struct msm_kms_state, base)
> +
> +/**
> + * Extracts the msm_kms_state from either an atomic state, or the current
> + * device. One or the other might be desirable depending on whether you want the
> + * currently configured msm_kms_state (from_dev), or you would like the state
> + * which is about to be applied (from_atomic).
> + */
> +struct msm_kms_state *msm_kms_state_from_dev(struct drm_device *dev);
> +struct msm_kms_state *msm_kms_state_from_atomic(struct drm_atomic_state *state);
>
> static inline void msm_kms_init(struct msm_kms *kms,
> const struct msm_kms_funcs *funcs)
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/6] drm/msm: Refactor complete_commit() to look more the helpers
2018-03-28 19:06 ` Sean Paul
@ 2018-04-02 5:22 ` Archit Taneja
-1 siblings, 0 replies; 21+ messages in thread
From: Archit Taneja @ 2018-04-02 5:22 UTC (permalink / raw)
To: Sean Paul, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: jsanka-sgV2jX0FEOL9JmXXK+q4OQ, robdclark-Re5JQEeQqe8AvxtiuMwx3w,
hoegsberg-F7+t8E8rja9g9hUCZPvPmw,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Thursday 29 March 2018 12:36 AM, Sean Paul wrote:
> Factor out the commit_tail() portions of complete_commit() into a
> separate function to facilitate moving to the atomic helpers in future
> patches.
>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
> Changes in v2:
> - None
>
> Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/gpu/drm/msm/msm_atomic.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
> index e792158676aa..671a18ee977d 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -97,18 +97,12 @@ static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
> }
> }
>
> -/* The (potentially) asynchronous part of the commit. At this point
> - * nothing can fail short of armageddon.
> - */
> -static void complete_commit(struct msm_commit *c, bool async)
> +static void msm_atomic_commit_tail(struct drm_atomic_state *state)
> {
> - struct drm_atomic_state *state = c->state;
> struct drm_device *dev = state->dev;
> struct msm_drm_private *priv = dev->dev_private;
> struct msm_kms *kms = priv->kms;
>
> - drm_atomic_helper_wait_for_fences(dev, state, false);
> -
> kms->funcs->prepare_commit(kms, state);
>
> drm_atomic_helper_commit_modeset_disables(dev, state);
> @@ -135,6 +129,19 @@ static void complete_commit(struct msm_commit *c, bool async)
> drm_atomic_helper_cleanup_planes(dev, state);
>
> kms->funcs->complete_commit(kms, state);
> +}
> +
> +/* The (potentially) asynchronous part of the commit. At this point
> + * nothing can fail short of armageddon.
> + */
> +static void complete_commit(struct msm_commit *c)
> +{
> + struct drm_atomic_state *state = c->state;
> + struct drm_device *dev = state->dev;
> +
> + drm_atomic_helper_wait_for_fences(dev, state, false);
> +
> + msm_atomic_commit_tail(state);
>
> drm_atomic_state_put(state);
>
> @@ -143,7 +150,7 @@ static void complete_commit(struct msm_commit *c, bool async)
>
> static void commit_worker(struct work_struct *work)
> {
> - complete_commit(container_of(work, struct msm_commit, work), true);
> + complete_commit(container_of(work, struct msm_commit, work));
> }
>
> /**
> @@ -242,7 +249,7 @@ int msm_atomic_commit(struct drm_device *dev,
> return 0;
> }
>
> - complete_commit(c, false);
> + complete_commit(c);
>
> return 0;
>
>
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 2/6] drm/msm: Refactor complete_commit() to look more the helpers
@ 2018-04-02 5:22 ` Archit Taneja
0 siblings, 0 replies; 21+ messages in thread
From: Archit Taneja @ 2018-04-02 5:22 UTC (permalink / raw)
To: Sean Paul, freedreno, linux-arm-msm, dri-devel
Cc: robdclark, hoegsberg, jsanka, abhinavk, linux-kernel
On Thursday 29 March 2018 12:36 AM, Sean Paul wrote:
> Factor out the commit_tail() portions of complete_commit() into a
> separate function to facilitate moving to the atomic helpers in future
> patches.
>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
> Changes in v2:
> - None
>
> Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/gpu/drm/msm/msm_atomic.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
> index e792158676aa..671a18ee977d 100644
> --- a/drivers/gpu/drm/msm/msm_atomic.c
> +++ b/drivers/gpu/drm/msm/msm_atomic.c
> @@ -97,18 +97,12 @@ static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
> }
> }
>
> -/* The (potentially) asynchronous part of the commit. At this point
> - * nothing can fail short of armageddon.
> - */
> -static void complete_commit(struct msm_commit *c, bool async)
> +static void msm_atomic_commit_tail(struct drm_atomic_state *state)
> {
> - struct drm_atomic_state *state = c->state;
> struct drm_device *dev = state->dev;
> struct msm_drm_private *priv = dev->dev_private;
> struct msm_kms *kms = priv->kms;
>
> - drm_atomic_helper_wait_for_fences(dev, state, false);
> -
> kms->funcs->prepare_commit(kms, state);
>
> drm_atomic_helper_commit_modeset_disables(dev, state);
> @@ -135,6 +129,19 @@ static void complete_commit(struct msm_commit *c, bool async)
> drm_atomic_helper_cleanup_planes(dev, state);
>
> kms->funcs->complete_commit(kms, state);
> +}
> +
> +/* The (potentially) asynchronous part of the commit. At this point
> + * nothing can fail short of armageddon.
> + */
> +static void complete_commit(struct msm_commit *c)
> +{
> + struct drm_atomic_state *state = c->state;
> + struct drm_device *dev = state->dev;
> +
> + drm_atomic_helper_wait_for_fences(dev, state, false);
> +
> + msm_atomic_commit_tail(state);
>
> drm_atomic_state_put(state);
>
> @@ -143,7 +150,7 @@ static void complete_commit(struct msm_commit *c, bool async)
>
> static void commit_worker(struct work_struct *work)
> {
> - complete_commit(container_of(work, struct msm_commit, work), true);
> + complete_commit(container_of(work, struct msm_commit, work));
> }
>
> /**
> @@ -242,7 +249,7 @@ int msm_atomic_commit(struct drm_device *dev,
> return 0;
> }
>
> - complete_commit(c, false);
> + complete_commit(c);
>
> return 0;
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/6] drm/msm: Mark the crtc->state->event consumed
2018-03-28 19:06 ` Sean Paul
@ 2018-04-02 5:22 ` Archit Taneja
-1 siblings, 0 replies; 21+ messages in thread
From: Archit Taneja @ 2018-04-02 5:22 UTC (permalink / raw)
To: Sean Paul, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: jsanka-sgV2jX0FEOL9JmXXK+q4OQ, robdclark-Re5JQEeQqe8AvxtiuMwx3w,
hoegsberg-F7+t8E8rja9g9hUCZPvPmw,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Thursday 29 March 2018 12:36 AM, Sean Paul wrote:
> Don't leave the event != NULL once it's consumed, this is used a signal
s/used a/used as a ?
> to the atomic helpers that the event will be handled by the driver.
>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
> Changes in v2:
> - None
>
> Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 +
> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> index 6e5e1aa54ce1..b001699297c4 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> @@ -351,6 +351,7 @@ static void mdp4_crtc_atomic_flush(struct drm_crtc *crtc,
>
> spin_lock_irqsave(&dev->event_lock, flags);
> mdp4_crtc->event = crtc->state->event;
> + crtc->state->event = NULL;
> spin_unlock_irqrestore(&dev->event_lock, flags);
>
> blend_setup(crtc);
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index 9893e43ba6c5..76b96081916f 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -708,6 +708,7 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc,
>
> spin_lock_irqsave(&dev->event_lock, flags);
> mdp5_crtc->event = crtc->state->event;
> + crtc->state->event = NULL;
> spin_unlock_irqrestore(&dev->event_lock, flags);
>
> /*
>
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 3/6] drm/msm: Mark the crtc->state->event consumed
@ 2018-04-02 5:22 ` Archit Taneja
0 siblings, 0 replies; 21+ messages in thread
From: Archit Taneja @ 2018-04-02 5:22 UTC (permalink / raw)
To: Sean Paul, freedreno, linux-arm-msm, dri-devel
Cc: robdclark, hoegsberg, jsanka, abhinavk, linux-kernel
On Thursday 29 March 2018 12:36 AM, Sean Paul wrote:
> Don't leave the event != NULL once it's consumed, this is used a signal
s/used a/used as a ?
> to the atomic helpers that the event will be handled by the driver.
>
Reviewed-by: Archit Taneja <architt@codeaurora.org>
> Changes in v2:
> - None
>
> Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 +
> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> index 6e5e1aa54ce1..b001699297c4 100644
> --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c
> @@ -351,6 +351,7 @@ static void mdp4_crtc_atomic_flush(struct drm_crtc *crtc,
>
> spin_lock_irqsave(&dev->event_lock, flags);
> mdp4_crtc->event = crtc->state->event;
> + crtc->state->event = NULL;
> spin_unlock_irqrestore(&dev->event_lock, flags);
>
> blend_setup(crtc);
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index 9893e43ba6c5..76b96081916f 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -708,6 +708,7 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc,
>
> spin_lock_irqsave(&dev->event_lock, flags);
> mdp5_crtc->event = crtc->state->event;
> + crtc->state->event = NULL;
> spin_unlock_irqrestore(&dev->event_lock, flags);
>
> /*
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/6] drm/msm: Issue queued events when disabling crtc
2018-03-28 19:06 ` Sean Paul
@ 2018-04-02 5:23 ` Archit Taneja
-1 siblings, 0 replies; 21+ messages in thread
From: Archit Taneja @ 2018-04-02 5:23 UTC (permalink / raw)
To: Sean Paul, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: jsanka-sgV2jX0FEOL9JmXXK+q4OQ, robdclark-Re5JQEeQqe8AvxtiuMwx3w,
hoegsberg-F7+t8E8rja9g9hUCZPvPmw,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Thursday 29 March 2018 12:36 AM, Sean Paul wrote:
> Ensure that any queued events are issued when disabling the crtc. This
> avoids timeouts when we come back and wait for dependencies (like the
> previous frame's flip_done).
Reviewed-by: Archit Taneja <architt@codeaurora.org>
>
> Changes in v2:
> - None
>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index 76b96081916f..10271359789e 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -430,6 +430,7 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc,
> struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
> struct mdp5_kms *mdp5_kms = get_kms(crtc);
> struct device *dev = &mdp5_kms->pdev->dev;
> + unsigned long flags;
>
> DBG("%s", crtc->name);
>
> @@ -445,6 +446,14 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc,
> mdp_irq_unregister(&mdp5_kms->base, &mdp5_crtc->err);
> pm_runtime_put_sync(dev);
>
> + if (crtc->state->event && !crtc->state->active) {
> + WARN_ON(mdp5_crtc->event);
> + spin_lock_irqsave(&mdp5_kms->dev->event_lock, flags);
> + drm_crtc_send_vblank_event(crtc, crtc->state->event);
> + crtc->state->event = NULL;
> + spin_unlock_irqrestore(&mdp5_kms->dev->event_lock, flags);
> + }
> +
> mdp5_crtc->enabled = false;
> }
>
>
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v2 4/6] drm/msm: Issue queued events when disabling crtc
@ 2018-04-02 5:23 ` Archit Taneja
0 siblings, 0 replies; 21+ messages in thread
From: Archit Taneja @ 2018-04-02 5:23 UTC (permalink / raw)
To: Sean Paul, freedreno, linux-arm-msm, dri-devel
Cc: robdclark, hoegsberg, jsanka, abhinavk, linux-kernel
On Thursday 29 March 2018 12:36 AM, Sean Paul wrote:
> Ensure that any queued events are issued when disabling the crtc. This
> avoids timeouts when we come back and wait for dependencies (like the
> previous frame's flip_done).
Reviewed-by: Archit Taneja <architt@codeaurora.org>
>
> Changes in v2:
> - None
>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> index 76b96081916f..10271359789e 100644
> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
> @@ -430,6 +430,7 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc,
> struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
> struct mdp5_kms *mdp5_kms = get_kms(crtc);
> struct device *dev = &mdp5_kms->pdev->dev;
> + unsigned long flags;
>
> DBG("%s", crtc->name);
>
> @@ -445,6 +446,14 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc,
> mdp_irq_unregister(&mdp5_kms->base, &mdp5_crtc->err);
> pm_runtime_put_sync(dev);
>
> + if (crtc->state->event && !crtc->state->active) {
> + WARN_ON(mdp5_crtc->event);
> + spin_lock_irqsave(&mdp5_kms->dev->event_lock, flags);
> + drm_crtc_send_vblank_event(crtc, crtc->state->event);
> + crtc->state->event = NULL;
> + spin_unlock_irqrestore(&mdp5_kms->dev->event_lock, flags);
> + }
> +
> mdp5_crtc->enabled = false;
> }
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2018-04-02 5:23 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-28 19:06 [PATCH v2 0/6] drm/msm: Switch to atomic helpers Sean Paul
2018-03-28 19:06 ` [PATCH v2 1/6] drm/msm: Use drm_private_obj/state instead of subclassing Sean Paul
2018-03-28 19:06 ` Sean Paul
[not found] ` <20180328190657.218661-2-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-04-02 5:01 ` Archit Taneja
2018-04-02 5:01 ` Archit Taneja
[not found] ` <20180328190657.218661-1-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-03-28 19:06 ` [PATCH v2 2/6] drm/msm: Refactor complete_commit() to look more the helpers Sean Paul
2018-03-28 19:06 ` Sean Paul
[not found] ` <20180328190657.218661-3-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-04-02 5:22 ` Archit Taneja
2018-04-02 5:22 ` Archit Taneja
2018-03-28 19:06 ` [PATCH v2 3/6] drm/msm: Mark the crtc->state->event consumed Sean Paul
2018-03-28 19:06 ` Sean Paul
[not found] ` <20180328190657.218661-4-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-04-02 5:22 ` Archit Taneja
2018-04-02 5:22 ` Archit Taneja
2018-03-28 19:06 ` [PATCH v2 4/6] drm/msm: Issue queued events when disabling crtc Sean Paul
2018-03-28 19:06 ` Sean Paul
[not found] ` <20180328190657.218661-5-seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
2018-04-02 5:23 ` Archit Taneja
2018-04-02 5:23 ` Archit Taneja
2018-03-28 19:06 ` [PATCH v2 5/6] drm/msm: Remove msm_commit/worker, use atomic helper commit Sean Paul
2018-03-28 19:06 ` Sean Paul
2018-03-28 19:06 ` [PATCH v2 6/6] drm/msm: Switch to atomic_helper_commit() Sean Paul
2018-03-28 19:06 ` Sean Paul
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.