On Mon, Oct 5, 2020 at 11:20 AM Daniel Vetter <daniel@xxxxxxxx> wrote: > > On Mon, Oct 5, 2020 at 6:24 PM Kristian Høgsberg <hoegsberg@xxxxxxxxx> wrote: > > > > On Sun, Oct 4, 2020 at 9:21 PM Rob Clark <robdclark@xxxxxxxxx> wrote: > > > > > > From: Rob Clark <robdclark@xxxxxxxxxxxx> > > > > > > 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 :-) Yeah, I'll defn throw the lockdep priming into v2.. and I've got using obj->resv for locking on the todo list but looks like enough churn that it will probably be it's own series (but seems like there is room to introduce some lock/unlock helpers that don't really change anything but make an obj->lock transition easier) BR, -R > -Daniel > > > Either way, for the series > > > > Reviewed-by: Kristian H. Kristensen <hoegsberg@xxxxxxxxxx> > > > > > 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@xxxxxxxxxxxxxxxxxxxxx > > > https://lists.freedesktop.org/mailman/listinfo/freedreno > > _______________________________________________ > > dri-devel mailing list > > dri-devel@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch
