Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit

2018-03-19 Thread Jeykumar Sankaran 

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

2018-03-19 Thread Sean Paul 
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

2018-03-12 Thread Sean Paul 
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(>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(>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(>pending_crtcs_event.lock);
> > > >>> > - DBG("end: %08x", crtc_mask);
> > > >>> > - priv->pending_crtcs &= ~crtc_mask;
> > > >>> > - wake_up_all_locked(>pending_crtcs_event);
> > > >>> > - spin_unlock(>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

2018-03-08 Thread Jeykumar Sankaran 

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(>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(>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(>pending_crtcs_event.lock);
>>> > - DBG("end: %08x", crtc_mask);
>>> > - priv->pending_crtcs &= ~crtc_mask;
>>> > - wake_up_all_locked(>pending_crtcs_event);
>>> > - spin_unlock(>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),

Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit

2018-03-02 Thread Sean Paul 
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(>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(>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(>pending_crtcs_event.lock);
> >>> > - DBG("end: %08x", crtc_mask);
> >>> > - priv->pending_crtcs &= ~crtc_mask;
> >>> > - wake_up_all_locked(>pending_crtcs_event);
> >>> > - spin_unlock(>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);
> >>> > -
> >>> > -

Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit

2018-03-01 Thread Rob Clark 
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(>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(>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(>pending_crtcs_event.lock);
>>> > - DBG("end: %08x", crtc_mask);
>>> > - priv->pending_crtcs &= ~crtc_mask;
>>> > - wake_up_all_locked(>pending_crtcs_event);
>>> > - spin_unlock(>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

Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit

2018-03-01 Thread jsanka 

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(>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(>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(>pending_crtcs_event.lock);
> -  DBG("end: %08x", crtc_mask);
> -  priv->pending_crtcs &= ~crtc_mask;
> -  wake_up_all_locked(>pending_crtcs_event);
> -  spin_unlock(>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(>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

2018-03-01 Thread Sean Paul 
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(>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(>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(>pending_crtcs_event.lock);
> > -   DBG("end: %08x", crtc_mask);
> > -   priv->pending_crtcs &= ~crtc_mask;
> > -   wake_up_all_locked(>pending_crtcs_event);
> > -   spin_unlock(>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(>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)
> >  {

Re: [DPU PATCH 06/11] drm/msm: Remove msm_commit/kthread, use atomic helper commit

2018-02-28 Thread jsanka 

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(>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(>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(>pending_crtcs_event.lock);
-   DBG("end: %08x", crtc_mask);
-   priv->pending_crtcs &= ~crtc_mask;
-   wake_up_all_locked(>pending_crtcs_event);
-   spin_unlock(>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(>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 = 0; j <