Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201004192152.3298573-1-robdclark@gmail.com> <CAOeoa-d4-KyuVjwPCBcau6yp6sxeeHN4K0cQzZ=H+OQVgHsLFw@mail.gmail.com>
In-Reply-To: <CAOeoa-d4-KyuVjwPCBcau6yp6sxeeHN4K0cQzZ=H+OQVgHsLFw@mail.gmail.com>
From:   Daniel Vetter <daniel@ffwll.ch>
Date:   Mon, 5 Oct 2020 20:20:22 +0200
Message-ID: <CAKMK7uHeZptOKWKTanBB_c2ek2emNjs9fFCbTwgafS19rL1sww@mail.gmail.com>
Subject: Re: [Freedreno] [PATCH 00/14] drm/msm: de-struct_mutex-ification
To:     =?UTF-8?Q?Kristian_H=C3=B8gsberg?= <hoegsberg@gmail.com>
Cc:     Rob Clark <robdclark@gmail.com>,
        Rob Clark <robdclark@chromium.org>,
        "open list:DRM DRIVER FOR MSM ADRENO GPU" 
        <freedreno@lists.freedesktop.org>,
        Emil Velikov <emil.velikov@collabora.com>,
        Jonathan Marek <jonathan@marek.ca>,
        "open list:DRM DRIVER FOR MSM ADRENO GPU" 
        <linux-arm-msm@vger.kernel.org>,
        Sharat Masetty <smasetty@codeaurora.org>,
        open list <linux-kernel@vger.kernel.org>,
        dri-devel <dri-devel@lists.freedesktop.org>,
        "moderated list:DMA BUFFER SHARING FRAMEWORK" 
        <linaro-mm-sig@lists.linaro.org>,
        Ben Dooks <ben.dooks@codethink.co.uk>,
        AngeloGioacchino Del Regno <kholk11@gmail.com>,
        Sam Ravnborg <sam@ravnborg.org>,
        "open list:DMA BUFFER SHARING FRAMEWORK" 
        <linux-media@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Mon, Oct 5, 2020 at 6:24 PM Kristian H=C3=B8gsberg <hoegsberg@gmail.com>=
 wrote:
>
> On Sun, Oct 4, 2020 at 9:21 PM Rob Clark <robdclark@gmail.com> wrote:
> >
> > From: Rob Clark <robdclark@chromium.org>
> >
> > This doesn't remove *all* the struct_mutex, but it covers the worst
> > of it, ie. shrinker/madvise/free/retire.  The submit path still uses
> > struct_mutex, but it still needs *something* serialize a portion of
> > the submit path, and lock_stat mostly just shows the lock contention
> > there being with other submits.  And there are a few other bits of
> > struct_mutex usage in less critical paths (debugfs, etc).  But this
> > seems like a reasonable step in the right direction.
>
> What a great patch set. Daniel has some good points and nothing that
> requires big changes, but on the other hand, I'm not sure it's
> something that needs to block this set either.

Personally I'd throw the lockdep priming on top to make sure this
stays correct (it's 3 lines), but yes imo this is all good to go. Just
figured I'll sprinkle the latest in terms of gem locking over the
series while it's here :-)
-Daniel

> Either way, for the series
>
> Reviewed-by: Kristian H. Kristensen <hoegsberg@google.com>
>
> > Rob Clark (14):
> >   drm/msm: Use correct drm_gem_object_put() in fail case
> >   drm/msm: Drop chatty trace
> >   drm/msm: Move update_fences()
> >   drm/msm: Add priv->mm_lock to protect active/inactive lists
> >   drm/msm: Document and rename preempt_lock
> >   drm/msm: Protect ring->submits with it's own lock
> >   drm/msm: Refcount submits
> >   drm/msm: Remove obj->gpu
> >   drm/msm: Drop struct_mutex from the retire path
> >   drm/msm: Drop struct_mutex in free_object() path
> >   drm/msm: remove msm_gem_free_work
> >   drm/msm: drop struct_mutex in madvise path
> >   drm/msm: Drop struct_mutex in shrinker path
> >   drm/msm: Don't implicit-sync if only a single ring
> >
> >  drivers/gpu/drm/msm/adreno/a5xx_gpu.c     |  4 +-
> >  drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 12 +--
> >  drivers/gpu/drm/msm/adreno/a6xx_gpu.c     |  4 +-
> >  drivers/gpu/drm/msm/msm_debugfs.c         |  7 ++
> >  drivers/gpu/drm/msm/msm_drv.c             | 15 +---
> >  drivers/gpu/drm/msm/msm_drv.h             | 19 +++--
> >  drivers/gpu/drm/msm/msm_gem.c             | 76 ++++++------------
> >  drivers/gpu/drm/msm/msm_gem.h             | 53 +++++++++----
> >  drivers/gpu/drm/msm/msm_gem_shrinker.c    | 58 ++------------
> >  drivers/gpu/drm/msm/msm_gem_submit.c      | 17 ++--
> >  drivers/gpu/drm/msm/msm_gpu.c             | 96 ++++++++++++++---------
> >  drivers/gpu/drm/msm/msm_gpu.h             |  5 +-
> >  drivers/gpu/drm/msm/msm_ringbuffer.c      |  3 +-
> >  drivers/gpu/drm/msm/msm_ringbuffer.h      | 13 ++-
> >  14 files changed, 188 insertions(+), 194 deletions(-)
> >
> > --
> > 2.26.2
> >
> > _______________________________________________
> > Freedreno mailing list
> > Freedreno@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/freedreno
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


--=20
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch