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

[Jeykumar Sankaran <jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

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,  <jsanka@codeaurora.org> wrote:
>> > > > > On 2018-03-01 07:27, Sean Paul wrote:
>> > > > >>
>> > > > >> On Wed, Feb 28, 2018 at 08:07:00PM -0800, jsanka@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 <seanpaul@chromium.org>
>> > > > >>> > ---
> 
> <snip />
> 
>> > > > >>
>> > > > >>> > -                                     /* 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 <jsanka@codeaurora.org>

>> >
>> > - Jeykumar S
> 
> <snip />

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno