Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit
On 2018-03-19 08:01, Sean Paul wrote: On Mon, Mar 12, 2018 at 04:23:10PM -0400, Sean Paul wrote: On Thu, Mar 08, 2018 at 05:08:03PM -0800, Jeykumar Sankaran wrote: > On 2018-03-02 06:56, Sean Paul wrote: > > On Thu, Mar 01, 2018 at 07:37:10PM -0500, Rob Clark wrote: > > > On Thu, Mar 1, 2018 at 3:37 PM, wrote: > > > > On 2018-03-01 07:27, Sean Paul wrote: > > > >> > > > >> On Wed, Feb 28, 2018 at 08:07:00PM -0800, jsa...@codeaurora.org > > wrote: > > > >>> > > > >>> On 2018-02-28 11:19, Sean Paul wrote: > > > >>> > Moving further towards switching fully to the the atomic helpers, > > this > > > >>> > patch removes the hand-rolled kthread nonblock commit code and > > uses > > > >> > > > >> the > > > >>> > > > >>> > atomic helpers commit_work model. > > > >>> > > > > >>> > There's still a lot of copypasta here, but it's still needed to > > > >>> > facilitate the swap_state and prepare_fence private functions. > > These > > > >>> > will be sorted out in a follow-on patch. > > > >>> > > > > >>> > Change-Id: I9fcba27824ba63d3fab96cb2bc194bfa6f3475b7 > > > >>> > Signed-off-by: Sean Paul > > > >>> > --- > > > >> > > > >>> > - /* only return zero if work > > is > > > >>> > - * queued successfully. > > > >>> > - */ > > > >>> > - ret = 0; > > > >>> > - } else { > > > >>> > - DRM_ERROR(" Error for > > crtc_id: > > > >>> > %d\n", > > > >>> > - > > > >>> > priv->disp_thread[j].crtc_id); > > > >>> > - } > > > >>> > - break; > > > >>> > - } > > > >>> > - } > > Care to remove priv->disp_thread and all its references as a part of this > change? Definitely! Will revise. Now that I look at it, disp_thread doesn't seem relevant to this change. It seems like it's used for deferred cleanup. So perhaps we could get rid of it, but IMO that would be a different patch. Sean hmm.. disp_threads are created per CRTC (per display) to allow concurrency of hardware programming. So its not entirely irrelevant to this chnage. But since it involves more than just priv->disp_thread cleanup, I am fine with cleaning that in a separate patch. Reviewed-by: Jeykumar Sankaran > > - Jeykumar S -- Jeykumar S ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit
On Mon, Mar 12, 2018 at 04:23:10PM -0400, Sean Paul wrote: > On Thu, Mar 08, 2018 at 05:08:03PM -0800, Jeykumar Sankaran wrote: > > On 2018-03-02 06:56, Sean Paul wrote: > > > On Thu, Mar 01, 2018 at 07:37:10PM -0500, Rob Clark wrote: > > > > On Thu, Mar 1, 2018 at 3:37 PM, wrote: > > > > > On 2018-03-01 07:27, Sean Paul wrote: > > > > >> > > > > >> On Wed, Feb 28, 2018 at 08:07:00PM -0800, jsa...@codeaurora.org > > > wrote: > > > > >>> > > > > >>> On 2018-02-28 11:19, Sean Paul wrote: > > > > >>> > Moving further towards switching fully to the the atomic helpers, > > > this > > > > >>> > patch removes the hand-rolled kthread nonblock commit code and > > > uses > > > > >> > > > > >> the > > > > >>> > > > > >>> > atomic helpers commit_work model. > > > > >>> > > > > > >>> > There's still a lot of copypasta here, but it's still needed to > > > > >>> > facilitate the swap_state and prepare_fence private functions. > > > These > > > > >>> > will be sorted out in a follow-on patch. > > > > >>> > > > > > >>> > Change-Id: I9fcba27824ba63d3fab96cb2bc194bfa6f3475b7 > > > > >>> > Signed-off-by: Sean Paul > > > > >>> > --- > > > > >> > > > > >>> > - /* only return zero if work > > > is > > > > >>> > - * queued successfully. > > > > >>> > - */ > > > > >>> > - ret = 0; > > > > >>> > - } else { > > > > >>> > - DRM_ERROR(" Error for > > > crtc_id: > > > > >>> > %d\n", > > > > >>> > - > > > > >>> > priv->disp_thread[j].crtc_id); > > > > >>> > - } > > > > >>> > - break; > > > > >>> > - } > > > > >>> > - } > > > > Care to remove priv->disp_thread and all its references as a part of this > > change? > > Definitely! Will revise. > Now that I look at it, disp_thread doesn't seem relevant to this change. It seems like it's used for deferred cleanup. So perhaps we could get rid of it, but IMO that would be a different patch. > Sean > > > > > - Jeykumar S -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit
On Thu, Mar 08, 2018 at 05:08:03PM -0800, Jeykumar Sankaran wrote: > On 2018-03-02 06:56, Sean Paul wrote: > > On Thu, Mar 01, 2018 at 07:37:10PM -0500, Rob Clark wrote: > > > On Thu, Mar 1, 2018 at 3:37 PM, wrote: > > > > On 2018-03-01 07:27, Sean Paul wrote: > > > >> > > > >> On Wed, Feb 28, 2018 at 08:07:00PM -0800, jsa...@codeaurora.org > > wrote: > > > >>> > > > >>> On 2018-02-28 11:19, Sean Paul wrote: > > > >>> > Moving further towards switching fully to the the atomic helpers, > > this > > > >>> > patch removes the hand-rolled kthread nonblock commit code and > > uses > > > >> > > > >> the > > > >>> > > > >>> > atomic helpers commit_work model. > > > >>> > > > > >>> > There's still a lot of copypasta here, but it's still needed to > > > >>> > facilitate the swap_state and prepare_fence private functions. > > These > > > >>> > will be sorted out in a follow-on patch. > > > >>> > > > > >>> > Change-Id: I9fcba27824ba63d3fab96cb2bc194bfa6f3475b7 > > > >>> > Signed-off-by: Sean Paul > > > >>> > --- > > > >>> > drivers/gpu/drm/msm/msm_atomic.c | 199 > > > >> > > > >> ++- > > > >>> > > > >>> > drivers/gpu/drm/msm/msm_drv.c| 1 - > > > >>> > drivers/gpu/drm/msm/msm_drv.h| 4 - > > > >>> > 3 files changed, 35 insertions(+), 169 deletions(-) > > > >>> > > > > >>> > diff --git a/drivers/gpu/drm/msm/msm_atomic.c > > > >>> > b/drivers/gpu/drm/msm/msm_atomic.c > > > >>> > index 3a18bd3dc215..7e54eb65d096 100644 > > > >>> > --- a/drivers/gpu/drm/msm/msm_atomic.c > > > >>> > +++ b/drivers/gpu/drm/msm/msm_atomic.c > > > >>> > @@ -21,51 +21,6 @@ > > > >>> > #include "msm_gem.h" > > > >>> > #include "msm_fence.h" > > > >>> > > > > >>> > -struct msm_commit { > > > >>> > - struct drm_device *dev; > > > >>> > - struct drm_atomic_state *state; > > > >>> > - uint32_t crtc_mask; > > > >>> > - bool nonblock; > > > >>> > - struct kthread_work commit_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 void commit_destroy(struct msm_commit *c) > > > >>> > -{ > > > >>> > - end_atomic(c->dev->dev_private, c->crtc_mask); > > > >>> > - if (c->nonblock) > > > >>> > - kfree(c); > > > >>> > -} > > > >>> > - > > > >>> > static void msm_atomic_wait_for_commit_done( > > > >>> > struct drm_device *dev, > > > >>> > struct drm_atomic_state *old_state) > > > >>> > @@ -118,6 +73,10 @@ static void msm_atomic_commit_tail(struct > > > >>> > drm_atomic_state *state) > > > >>> > > > > >>> > msm_atomic_wait_for_commit_done(dev, state); > > > >>> > > > > >>> > + drm_atomic_helper_commit_hw_done(state); > > > >>> > + > > > >>> > + drm_atomic_helper_wait_for_vblanks(dev, state); > > > >>> > + > > > >>> > drm_atomic_helper_cleanup_planes(dev, state); > > > >>> > > > > >>> > kms->funcs->complete_commit(kms, state); > > > >>> > @@ -126,109 +85,25 @@ static void msm_atomic_commit_tail(struct > > > >>> > drm_atomic_state *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); > > > >>> > > > > >>> >
Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit
On 2018-03-02 06:56, Sean Paul wrote: On Thu, Mar 01, 2018 at 07:37:10PM -0500, Rob Clark wrote: On Thu, Mar 1, 2018 at 3:37 PM, wrote: > On 2018-03-01 07:27, Sean Paul wrote: >> >> On Wed, Feb 28, 2018 at 08:07:00PM -0800, jsa...@codeaurora.org wrote: >>> >>> On 2018-02-28 11:19, Sean Paul wrote: >>> > Moving further towards switching fully to the the atomic helpers, this >>> > patch removes the hand-rolled kthread nonblock commit code and uses >> >> the >>> >>> > atomic helpers commit_work model. >>> > >>> > There's still a lot of copypasta here, but it's still needed to >>> > facilitate the swap_state and prepare_fence private functions. These >>> > will be sorted out in a follow-on patch. >>> > >>> > Change-Id: I9fcba27824ba63d3fab96cb2bc194bfa6f3475b7 >>> > Signed-off-by: Sean Paul >>> > --- >>> > drivers/gpu/drm/msm/msm_atomic.c | 199 >> >> ++- >>> >>> > drivers/gpu/drm/msm/msm_drv.c| 1 - >>> > drivers/gpu/drm/msm/msm_drv.h| 4 - >>> > 3 files changed, 35 insertions(+), 169 deletions(-) >>> > >>> > diff --git a/drivers/gpu/drm/msm/msm_atomic.c >>> > b/drivers/gpu/drm/msm/msm_atomic.c >>> > index 3a18bd3dc215..7e54eb65d096 100644 >>> > --- a/drivers/gpu/drm/msm/msm_atomic.c >>> > +++ b/drivers/gpu/drm/msm/msm_atomic.c >>> > @@ -21,51 +21,6 @@ >>> > #include "msm_gem.h" >>> > #include "msm_fence.h" >>> > >>> > -struct msm_commit { >>> > - struct drm_device *dev; >>> > - struct drm_atomic_state *state; >>> > - uint32_t crtc_mask; >>> > - bool nonblock; >>> > - struct kthread_work commit_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 void commit_destroy(struct msm_commit *c) >>> > -{ >>> > - end_atomic(c->dev->dev_private, c->crtc_mask); >>> > - if (c->nonblock) >>> > - kfree(c); >>> > -} >>> > - >>> > static void msm_atomic_wait_for_commit_done( >>> > struct drm_device *dev, >>> > struct drm_atomic_state *old_state) >>> > @@ -118,6 +73,10 @@ static void msm_atomic_commit_tail(struct >>> > drm_atomic_state *state) >>> > >>> > msm_atomic_wait_for_commit_done(dev, state); >>> > >>> > + drm_atomic_helper_commit_hw_done(state); >>> > + >>> > + drm_atomic_helper_wait_for_vblanks(dev, state); >>> > + >>> > drm_atomic_helper_cleanup_planes(dev, state); >>> > >>> > kms->funcs->complete_commit(kms, state); >>> > @@ -126,109 +85,25 @@ static void msm_atomic_commit_tail(struct >>> > drm_atomic_state *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); >>> > -} >>> > - >>> > -static void _msm_drm_commit_work_cb(struct kthread_work *work) >>> > -{ >>> > - struct msm_commit *commit = NULL; >>> > - >>> > - if (!work) { >>> > - DRM_ERROR("%s: Invalid commit work data!\n", __func__); >>> > - return; >>> > - } >>> > - >>> > - commit = container_of(work, struct msm_commit, commit_work); >>> > - >>> > - complete_commit(commit); >>> > - >>> > - commit_destroy(commit); >>> > -} >>> > - >>> > -static struct msm_commit *commit_init(struct drm_atomic_state *state, >>> > - bool nonblock) >>> > -{ >>> > - struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL); >>> > + dr
Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit
On Thu, Mar 01, 2018 at 07:37:10PM -0500, Rob Clark wrote: > On Thu, Mar 1, 2018 at 3:37 PM, wrote: > > On 2018-03-01 07:27, Sean Paul wrote: > >> > >> On Wed, Feb 28, 2018 at 08:07:00PM -0800, jsa...@codeaurora.org wrote: > >>> > >>> On 2018-02-28 11:19, Sean Paul wrote: > >>> > Moving further towards switching fully to the the atomic helpers, this > >>> > patch removes the hand-rolled kthread nonblock commit code and uses > >> > >> the > >>> > >>> > atomic helpers commit_work model. > >>> > > >>> > There's still a lot of copypasta here, but it's still needed to > >>> > facilitate the swap_state and prepare_fence private functions. These > >>> > will be sorted out in a follow-on patch. > >>> > > >>> > Change-Id: I9fcba27824ba63d3fab96cb2bc194bfa6f3475b7 > >>> > Signed-off-by: Sean Paul > >>> > --- > >>> > drivers/gpu/drm/msm/msm_atomic.c | 199 > >> > >> ++- > >>> > >>> > drivers/gpu/drm/msm/msm_drv.c| 1 - > >>> > drivers/gpu/drm/msm/msm_drv.h| 4 - > >>> > 3 files changed, 35 insertions(+), 169 deletions(-) > >>> > > >>> > diff --git a/drivers/gpu/drm/msm/msm_atomic.c > >>> > b/drivers/gpu/drm/msm/msm_atomic.c > >>> > index 3a18bd3dc215..7e54eb65d096 100644 > >>> > --- a/drivers/gpu/drm/msm/msm_atomic.c > >>> > +++ b/drivers/gpu/drm/msm/msm_atomic.c > >>> > @@ -21,51 +21,6 @@ > >>> > #include "msm_gem.h" > >>> > #include "msm_fence.h" > >>> > > >>> > -struct msm_commit { > >>> > - struct drm_device *dev; > >>> > - struct drm_atomic_state *state; > >>> > - uint32_t crtc_mask; > >>> > - bool nonblock; > >>> > - struct kthread_work commit_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 void commit_destroy(struct msm_commit *c) > >>> > -{ > >>> > - end_atomic(c->dev->dev_private, c->crtc_mask); > >>> > - if (c->nonblock) > >>> > - kfree(c); > >>> > -} > >>> > - > >>> > static void msm_atomic_wait_for_commit_done( > >>> > struct drm_device *dev, > >>> > struct drm_atomic_state *old_state) > >>> > @@ -118,6 +73,10 @@ static void msm_atomic_commit_tail(struct > >>> > drm_atomic_state *state) > >>> > > >>> > msm_atomic_wait_for_commit_done(dev, state); > >>> > > >>> > + drm_atomic_helper_commit_hw_done(state); > >>> > + > >>> > + drm_atomic_helper_wait_for_vblanks(dev, state); > >>> > + > >>> > drm_atomic_helper_cleanup_planes(dev, state); > >>> > > >>> > kms->funcs->complete_commit(kms, state); > >>> > @@ -126,109 +85,25 @@ static void msm_atomic_commit_tail(struct > >>> > drm_atomic_state *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); > >>> > -} > >>> > - > >>> > -static void _msm_drm_commit_work_cb(struct kthread_work *work) > >>> > -{ > >>> > - struct msm_commit *commit = NULL; > >>> > - > >>> > - if (!work) { > >>> > - DRM_ERROR("%s: Invalid commit work data!\n", __func__); > >>> > - return; > >>> > - } > >>> > - > >>> > - commit = container_of(work, struct msm_commit, commit_work); > >>> > - > >>> > - complete_commit(commit); > >>> > - > >>> > - commit_destroy(commit); > >>> > -}
Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit
On Thu, Mar 1, 2018 at 3:37 PM, wrote: > On 2018-03-01 07:27, Sean Paul wrote: >> >> On Wed, Feb 28, 2018 at 08:07:00PM -0800, jsa...@codeaurora.org wrote: >>> >>> On 2018-02-28 11:19, Sean Paul wrote: >>> > Moving further towards switching fully to the the atomic helpers, this >>> > patch removes the hand-rolled kthread nonblock commit code and uses >> >> the >>> >>> > atomic helpers commit_work model. >>> > >>> > There's still a lot of copypasta here, but it's still needed to >>> > facilitate the swap_state and prepare_fence private functions. These >>> > will be sorted out in a follow-on patch. >>> > >>> > Change-Id: I9fcba27824ba63d3fab96cb2bc194bfa6f3475b7 >>> > Signed-off-by: Sean Paul >>> > --- >>> > drivers/gpu/drm/msm/msm_atomic.c | 199 >> >> ++- >>> >>> > drivers/gpu/drm/msm/msm_drv.c| 1 - >>> > drivers/gpu/drm/msm/msm_drv.h| 4 - >>> > 3 files changed, 35 insertions(+), 169 deletions(-) >>> > >>> > diff --git a/drivers/gpu/drm/msm/msm_atomic.c >>> > b/drivers/gpu/drm/msm/msm_atomic.c >>> > index 3a18bd3dc215..7e54eb65d096 100644 >>> > --- a/drivers/gpu/drm/msm/msm_atomic.c >>> > +++ b/drivers/gpu/drm/msm/msm_atomic.c >>> > @@ -21,51 +21,6 @@ >>> > #include "msm_gem.h" >>> > #include "msm_fence.h" >>> > >>> > -struct msm_commit { >>> > - struct drm_device *dev; >>> > - struct drm_atomic_state *state; >>> > - uint32_t crtc_mask; >>> > - bool nonblock; >>> > - struct kthread_work commit_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 void commit_destroy(struct msm_commit *c) >>> > -{ >>> > - end_atomic(c->dev->dev_private, c->crtc_mask); >>> > - if (c->nonblock) >>> > - kfree(c); >>> > -} >>> > - >>> > static void msm_atomic_wait_for_commit_done( >>> > struct drm_device *dev, >>> > struct drm_atomic_state *old_state) >>> > @@ -118,6 +73,10 @@ static void msm_atomic_commit_tail(struct >>> > drm_atomic_state *state) >>> > >>> > msm_atomic_wait_for_commit_done(dev, state); >>> > >>> > + drm_atomic_helper_commit_hw_done(state); >>> > + >>> > + drm_atomic_helper_wait_for_vblanks(dev, state); >>> > + >>> > drm_atomic_helper_cleanup_planes(dev, state); >>> > >>> > kms->funcs->complete_commit(kms, state); >>> > @@ -126,109 +85,25 @@ static void msm_atomic_commit_tail(struct >>> > drm_atomic_state *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); >>> > -} >>> > - >>> > -static void _msm_drm_commit_work_cb(struct kthread_work *work) >>> > -{ >>> > - struct msm_commit *commit = NULL; >>> > - >>> > - if (!work) { >>> > - DRM_ERROR("%s: Invalid commit work data!\n", __func__); >>> > - return; >>> > - } >>> > - >>> > - commit = container_of(work, struct msm_commit, commit_work); >>> > - >>> > - complete_commit(commit); >>> > - >>> > - commit_destroy(commit); >>> > -} >>> > - >>> > -static struct msm_commit *commit_init(struct drm_atomic_state *state, >>> > - bool nonblock) >>> > -{ >>> > - struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL); >>> > + drm_atomic_helper_commit_cleanup_done(state); >>> > >>> > - if (!c) >>> > - return NULL; >>> > - >>> >
Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit
On 2018-03-01 07:27, Sean Paul wrote: On Wed, Feb 28, 2018 at 08:07:00PM -0800, jsa...@codeaurora.org wrote: On 2018-02-28 11:19, Sean Paul wrote: > Moving further towards switching fully to the the atomic helpers, this > patch removes the hand-rolled kthread nonblock commit code and uses the > atomic helpers commit_work model. > > There's still a lot of copypasta here, but it's still needed to > facilitate the swap_state and prepare_fence private functions. These > will be sorted out in a follow-on patch. > > Change-Id: I9fcba27824ba63d3fab96cb2bc194bfa6f3475b7 > Signed-off-by: Sean Paul > --- > drivers/gpu/drm/msm/msm_atomic.c | 199 ++- > drivers/gpu/drm/msm/msm_drv.c| 1 - > drivers/gpu/drm/msm/msm_drv.h| 4 - > 3 files changed, 35 insertions(+), 169 deletions(-) > > diff --git a/drivers/gpu/drm/msm/msm_atomic.c > b/drivers/gpu/drm/msm/msm_atomic.c > index 3a18bd3dc215..7e54eb65d096 100644 > --- a/drivers/gpu/drm/msm/msm_atomic.c > +++ b/drivers/gpu/drm/msm/msm_atomic.c > @@ -21,51 +21,6 @@ > #include "msm_gem.h" > #include "msm_fence.h" > > -struct msm_commit { > - struct drm_device *dev; > - struct drm_atomic_state *state; > - uint32_t crtc_mask; > - bool nonblock; > - struct kthread_work commit_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 void commit_destroy(struct msm_commit *c) > -{ > - end_atomic(c->dev->dev_private, c->crtc_mask); > - if (c->nonblock) > - kfree(c); > -} > - > static void msm_atomic_wait_for_commit_done( >struct drm_device *dev, >struct drm_atomic_state *old_state) > @@ -118,6 +73,10 @@ static void msm_atomic_commit_tail(struct > drm_atomic_state *state) > >msm_atomic_wait_for_commit_done(dev, state); > > + drm_atomic_helper_commit_hw_done(state); > + > + drm_atomic_helper_wait_for_vblanks(dev, state); > + >drm_atomic_helper_cleanup_planes(dev, state); > >kms->funcs->complete_commit(kms, state); > @@ -126,109 +85,25 @@ static void msm_atomic_commit_tail(struct > drm_atomic_state *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); > -} > - > -static void _msm_drm_commit_work_cb(struct kthread_work *work) > -{ > - struct msm_commit *commit = NULL; > - > - if (!work) { > - DRM_ERROR("%s: Invalid commit work data!\n", __func__); > - return; > - } > - > - commit = container_of(work, struct msm_commit, commit_work); > - > - complete_commit(commit); > - > - commit_destroy(commit); > -} > - > -static struct msm_commit *commit_init(struct drm_atomic_state *state, > - bool nonblock) > -{ > - struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL); > + drm_atomic_helper_commit_cleanup_done(state); > > - if (!c) > - return NULL; > - > - c->dev = state->dev; > - c->state = state; > - c->nonblock = nonblock; > - > - kthread_init_work(&c->commit_work, _msm_drm_commit_work_cb); > - > - return c; > + drm_atomic_state_put(state); > } > > -/* Start display thread function */ > -static void msm_atomic_commit_dispatch(struct drm_device *dev, > - struct drm_atomic_state *state, struct msm_commit *commit) > +static void commit_work(struct work_struct *work) > { > - struct msm_drm_private *priv = dev->dev_private; > - struct drm_crtc *crtc = NULL; > - struct drm_crtc_state *new_crtc_state = NULL; > - int ret = -EINVAL, i = 0, j = 0; > - bool nonblock; > - > - /* cache since work will kfree commit in non-blocking case */ > - nonblock = commit->nonblock; > - > -
Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit
On Wed, Feb 28, 2018 at 08:07:00PM -0800, jsa...@codeaurora.org wrote: > On 2018-02-28 11:19, Sean Paul wrote: > > Moving further towards switching fully to the the atomic helpers, this > > patch removes the hand-rolled kthread nonblock commit code and uses the > > atomic helpers commit_work model. > > > > There's still a lot of copypasta here, but it's still needed to > > facilitate the swap_state and prepare_fence private functions. These > > will be sorted out in a follow-on patch. > > > > Change-Id: I9fcba27824ba63d3fab96cb2bc194bfa6f3475b7 > > Signed-off-by: Sean Paul > > --- > > drivers/gpu/drm/msm/msm_atomic.c | 199 ++- > > drivers/gpu/drm/msm/msm_drv.c| 1 - > > drivers/gpu/drm/msm/msm_drv.h| 4 - > > 3 files changed, 35 insertions(+), 169 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/msm_atomic.c > > b/drivers/gpu/drm/msm/msm_atomic.c > > index 3a18bd3dc215..7e54eb65d096 100644 > > --- a/drivers/gpu/drm/msm/msm_atomic.c > > +++ b/drivers/gpu/drm/msm/msm_atomic.c > > @@ -21,51 +21,6 @@ > > #include "msm_gem.h" > > #include "msm_fence.h" > > > > -struct msm_commit { > > - struct drm_device *dev; > > - struct drm_atomic_state *state; > > - uint32_t crtc_mask; > > - bool nonblock; > > - struct kthread_work commit_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 void commit_destroy(struct msm_commit *c) > > -{ > > - end_atomic(c->dev->dev_private, c->crtc_mask); > > - if (c->nonblock) > > - kfree(c); > > -} > > - > > static void msm_atomic_wait_for_commit_done( > > struct drm_device *dev, > > struct drm_atomic_state *old_state) > > @@ -118,6 +73,10 @@ static void msm_atomic_commit_tail(struct > > drm_atomic_state *state) > > > > msm_atomic_wait_for_commit_done(dev, state); > > > > + drm_atomic_helper_commit_hw_done(state); > > + > > + drm_atomic_helper_wait_for_vblanks(dev, state); > > + > > drm_atomic_helper_cleanup_planes(dev, state); > > > > kms->funcs->complete_commit(kms, state); > > @@ -126,109 +85,25 @@ static void msm_atomic_commit_tail(struct > > drm_atomic_state *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); > > -} > > - > > -static void _msm_drm_commit_work_cb(struct kthread_work *work) > > -{ > > - struct msm_commit *commit = NULL; > > - > > - if (!work) { > > - DRM_ERROR("%s: Invalid commit work data!\n", __func__); > > - return; > > - } > > - > > - commit = container_of(work, struct msm_commit, commit_work); > > - > > - complete_commit(commit); > > - > > - commit_destroy(commit); > > -} > > - > > -static struct msm_commit *commit_init(struct drm_atomic_state *state, > > - bool nonblock) > > -{ > > - struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL); > > + drm_atomic_helper_commit_cleanup_done(state); > > > > - if (!c) > > - return NULL; > > - > > - c->dev = state->dev; > > - c->state = state; > > - c->nonblock = nonblock; > > - > > - kthread_init_work(&c->commit_work, _msm_drm_commit_work_cb); > > - > > - return c; > > + drm_atomic_state_put(state); > > } > > > > -/* Start display thread function */ > > -static void msm_atomic_commit_dispatch(struct drm_device *dev, > > - struct drm_atomic_state *state, struct msm_commit *commit) > > +static void commit_work(struct work_struct *wor
Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit
On 2018-02-28 11:19, Sean Paul wrote: Moving further towards switching fully to the the atomic helpers, this patch removes the hand-rolled kthread nonblock commit code and uses the atomic helpers commit_work model. There's still a lot of copypasta here, but it's still needed to facilitate the swap_state and prepare_fence private functions. These will be sorted out in a follow-on patch. Change-Id: I9fcba27824ba63d3fab96cb2bc194bfa6f3475b7 Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/msm_atomic.c | 199 ++- drivers/gpu/drm/msm/msm_drv.c| 1 - drivers/gpu/drm/msm/msm_drv.h| 4 - 3 files changed, 35 insertions(+), 169 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c index 3a18bd3dc215..7e54eb65d096 100644 --- a/drivers/gpu/drm/msm/msm_atomic.c +++ b/drivers/gpu/drm/msm/msm_atomic.c @@ -21,51 +21,6 @@ #include "msm_gem.h" #include "msm_fence.h" -struct msm_commit { - struct drm_device *dev; - struct drm_atomic_state *state; - uint32_t crtc_mask; - bool nonblock; - struct kthread_work commit_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 void commit_destroy(struct msm_commit *c) -{ - end_atomic(c->dev->dev_private, c->crtc_mask); - if (c->nonblock) - kfree(c); -} - static void msm_atomic_wait_for_commit_done( struct drm_device *dev, struct drm_atomic_state *old_state) @@ -118,6 +73,10 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *state) msm_atomic_wait_for_commit_done(dev, state); + drm_atomic_helper_commit_hw_done(state); + + drm_atomic_helper_wait_for_vblanks(dev, state); + drm_atomic_helper_cleanup_planes(dev, state); kms->funcs->complete_commit(kms, state); @@ -126,109 +85,25 @@ static void msm_atomic_commit_tail(struct drm_atomic_state *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); -} - -static void _msm_drm_commit_work_cb(struct kthread_work *work) -{ - struct msm_commit *commit = NULL; - - if (!work) { - DRM_ERROR("%s: Invalid commit work data!\n", __func__); - return; - } - - commit = container_of(work, struct msm_commit, commit_work); - - complete_commit(commit); - - commit_destroy(commit); -} - -static struct msm_commit *commit_init(struct drm_atomic_state *state, - bool nonblock) -{ - struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL); + drm_atomic_helper_commit_cleanup_done(state); - if (!c) - return NULL; - - c->dev = state->dev; - c->state = state; - c->nonblock = nonblock; - - kthread_init_work(&c->commit_work, _msm_drm_commit_work_cb); - - return c; + drm_atomic_state_put(state); } -/* Start display thread function */ -static void msm_atomic_commit_dispatch(struct drm_device *dev, - struct drm_atomic_state *state, struct msm_commit *commit) +static void commit_work(struct work_struct *work) { - struct msm_drm_private *priv = dev->dev_private; - struct drm_crtc *crtc = NULL; - struct drm_crtc_state *new_crtc_state = NULL; - int ret = -EINVAL, i = 0, j = 0; - bool nonblock; - - /* cache since work will kfree commit in non-blocking case */ - nonblock = commit->nonblock; - - for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) { - for (j =