Re: [Intel-gfx] [PATCH 1/3] drm/atomic-helpers: remove legacy_cursor_update hacks

2020-10-22 Thread Rob Clark
On Thu, Oct 22, 2020 at 10:02 AM Rob Clark wrote: > > On Wed, Oct 21, 2020 at 9:32 AM Daniel Vetter wrote: > > > > The stuff never really worked, and leads to lots of fun because it > > out-of-order frees atomic states. Which upsets KASAN, among other > > things. &g

Re: [Intel-gfx] [PATCH 1/3] drm/atomic-helpers: remove legacy_cursor_update hacks

2020-10-22 Thread Rob Clark
On Wed, Oct 21, 2020 at 9:32 AM Daniel Vetter wrote: > > The stuff never really worked, and leads to lots of fun because it > out-of-order frees atomic states. Which upsets KASAN, among other > things. > > For async updates we now have a more solid solution with the > ->atomic_async_check and

Re: [PATCH V2 3/8] drm/msm: Unconditionally call dev_pm_opp_of_remove_table()

2020-10-21 Thread Rob Clark
On Wed, Oct 21, 2020 at 12:24 AM Viresh Kumar wrote: > > On 05-10-20, 11:56, Viresh Kumar wrote: > > On 28-08-20, 11:37, Viresh Kumar wrote: > > > dev_pm_opp_of_remove_table() doesn't report any errors when it fails to > > > find the OPP table with error -ENODEV (i.e. OPP table not present for >

[PATCH] drm/msm/atomic: Drop per-CRTC locks in reverse order

2020-10-20 Thread Rob Clark
From: Rob Clark lockdep dislikes seeing locks unwound in a non-nested fashion. Fixes: 37c2016e3608 ("drm/msm: Fix race condition in msm driver with async layer updates") Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_atomic.c | 2 +- drivers/gpu/drm/msm/msm_kms.h| 4 +++

Re: [PATCH 0/3] drm/msm: kthread_worker conversion

2020-10-20 Thread Rob Clark
On Tue, Oct 20, 2020 at 11:14 AM Daniel Vetter wrote: > > On Tue, Oct 20, 2020 at 7:23 PM Rob Clark wrote: > > > > On Tue, Oct 20, 2020 at 10:02 AM Daniel Vetter wrote: > > > > > > On Tue, Oct 20, 2020 at 5:08 PM Rob Clark wrote: > > > > >

Re: [PATCH 0/3] drm/msm: kthread_worker conversion

2020-10-20 Thread Rob Clark
On Tue, Oct 20, 2020 at 10:02 AM Daniel Vetter wrote: > > On Tue, Oct 20, 2020 at 5:08 PM Rob Clark wrote: > > > > On Tue, Oct 20, 2020 at 7:29 AM Daniel Vetter wrote: > > > > > > On Tue, Oct 20, 2020 at 4:01 PM Rob Clark wrote: > > > > >

Re: [PATCH 0/3] drm/msm: kthread_worker conversion

2020-10-20 Thread Rob Clark
On Tue, Oct 20, 2020 at 7:29 AM Daniel Vetter wrote: > > On Tue, Oct 20, 2020 at 4:01 PM Rob Clark wrote: > > > > On Tue, Oct 20, 2020 at 1:24 AM Daniel Vetter wrote: > > > > > > On Mon, Oct 19, 2020 at 02:10:50PM -0700, Rob Clark wrote: > > > >

Re: [PATCH v2 07/22] drm/msm: Do rpm get sooner in the submit path

2020-10-20 Thread Rob Clark
On Tue, Oct 20, 2020 at 4:24 AM Viresh Kumar wrote: > > On 20-10-20, 12:56, Daniel Vetter wrote: > > Yeah that's bad practice. Generally you shouldn't need to hold locks > > in setup/teardown code, since there's no other thread which can > > possible hold a reference to anything your touching

Re: [PATCH 0/3] drm/msm: kthread_worker conversion

2020-10-20 Thread Rob Clark
On Tue, Oct 20, 2020 at 1:24 AM Daniel Vetter wrote: > > On Mon, Oct 19, 2020 at 02:10:50PM -0700, Rob Clark wrote: > > From: Rob Clark > > > > In particular, converting the async atomic commit (for cursor updates, > > etc) to SCHED_FIFO kthread_worker helps with so

[PATCH 3/3] drm/msm/atomic: Convert to per-CRTC kthread_work

2020-10-19 Thread Rob Clark
From: Rob Clark Use a SCHED_FIFO kthread_worker for async atomic commits. We have a hard deadline if we don't want to miss a frame. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_atomic.c | 25 - drivers/gpu/drm/msm/msm_drv.h| 3 ++- drivers/gpu/drm/msm

[PATCH 2/3] drm/msm/kms: Update msm_kms_init/destroy

2020-10-19 Thread Rob Clark
From: Rob Clark Add msm_kms_destroy() and add err return from msm_kms_init(). Prep work for next patch. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 8 +++- drivers/gpu/drm/msm/disp/mdp4/mdp4_kms.c | 8 +++- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 11

[PATCH 1/3] drm/msm/gpu: Convert retire/recover work to kthread_worker

2020-10-19 Thread Rob Clark
From: Rob Clark Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 3 +-- drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 6 ++--- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 4 +-- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 4 +-- drivers/gpu/drm/msm/msm_gpu.c

[PATCH 0/3] drm/msm: kthread_worker conversion

2020-10-19 Thread Rob Clark
From: Rob Clark In particular, converting the async atomic commit (for cursor updates, etc) to SCHED_FIFO kthread_worker helps with some cases where we wouldn't manage to flush the updates within the 1ms-before-vblank deadline resulting in fps drops when there is cursor movement. Rob Clark (3

[PATCH v3 17/23] drm/msm: Remove obj->gpu

2020-10-19 Thread Rob Clark
From: Rob Clark It cannot be atomically updated with obj->active_count, and the only purpose is a useless WARN_ON() (which becomes a buggy WARN_ON() once retire_submits() is not serialized with incoming submits via struct_mutex) Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c

[PATCH v3 22/23] drm/msm: Drop struct_mutex in shrinker path

2020-10-19 Thread Rob Clark
From: Rob Clark Now that the inactive_list is protected by mm_lock, and everything else on per-obj basis is protected by obj->lock, we no longer depend on struct_mutex. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 1 - drivers/gpu/drm/msm/msm_gem_shrinker.c |

[PATCH v3 23/23] drm/msm: Don't implicit-sync if only a single ring

2020-10-19 Thread Rob Clark
From: Rob Clark If there is only a single ring (no-preemption), everything is FIFO order and there is no need to implicit-sync. Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as behavior is undefined when fences are not used to synchronize buffer usage across contexts (which

[PATCH v3 19/23] drm/msm: Drop struct_mutex in free_object() path

2020-10-19 Thread Rob Clark
From: Rob Clark Now that active_list/inactive_list is protected by mm_lock, we no longer need dev->struct_mutex in the free_object() path. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem.

[PATCH v3 15/23] drm/msm: Protect ring->submits with it's own lock

2020-10-19 Thread Rob Clark
From: Rob Clark One less place to rely on dev->struct_mutex. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gem_submit.c | 2 ++ drivers/gpu/drm/msm/msm_gpu.c| 37 ++-- drivers/gpu/drm/msm/msm_ringbuffer.c | 1 + drivers/

[PATCH v3 14/23] drm/msm: Document and rename preempt_lock

2020-10-19 Thread Rob Clark
From: Rob Clark Before adding another lock, give ring->lock a more descriptive name. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++-- drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 12 ++-- drivers/gpu/drm/msm/adreno/a6xx_gp

[PATCH v3 21/23] drm/msm: Drop struct_mutex in madvise path

2020-10-19 Thread Rob Clark
From: Rob Clark The obj->lock is sufficient for what we need. This *does* have the implication that userspace can try to shoot themselves in the foot by racing madvise(DONTNEED) with submit. But the result will be about the same if they did madvise(DONTNEED) before the submit ioctl,

[PATCH v3 12/23] drm/msm: Move update_fences()

2020-10-19 Thread Rob Clark
From: Rob Clark Small cleanup, update_fences() is used in the hangcheck path, but also in the normal retire path. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gpu.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff

[PATCH v3 18/23] drm/msm: Drop struct_mutex from the retire path

2020-10-19 Thread Rob Clark
From: Rob Clark Now that we are not relying on dev->struct_mutex to protect the ring->submits lists, drop the struct_mutex lock. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gpu.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff

[PATCH v3 20/23] drm/msm: Remove msm_gem_free_work

2020-10-19 Thread Rob Clark
From: Rob Clark Now that we don't need struct_mutex in the free path, we can get rid of the asynchronous free altogether. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_drv.c | 3 --- drivers/gpu/drm/msm/msm_drv.h | 5 - drivers/gpu/drm/msm/msm_gem.c | 27

[PATCH v3 16/23] drm/msm: Refcount submits

2020-10-19 Thread Rob Clark
From: Rob Clark Before we remove dev->struct_mutex from the retire path, we have to deal with the situation of a submit retiring before the submit ioctl returns. To deal with this, ring->submits will hold a reference to the submit, which is dropped when the submit is retired. And the

[PATCH v3 13/23] drm/msm: Add priv->mm_lock to protect active/inactive lists

2020-10-19 Thread Rob Clark
From: Rob Clark Rather than relying on the big dev->struct_mutex hammer, introduce a more specific lock for protecting the bo lists. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_debugfs.c | 7 +++ drivers/gpu/drm/msm/msm_drv.c |

[PATCH v3 02/23] drm/msm/gem: Add obj->lock wrappers

2020-10-19 Thread Rob Clark
From: Rob Clark This will make it easier to transition over to obj->resv locking for everything that is per-bo locking. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 99 --- drivers/gpu/drm/msm/msm_gem.h | 28 ++ 2 files changed,

[PATCH v3 03/23] drm/msm/gem: Rename internal get_iova_locked helper

2020-10-19 Thread Rob Clark
From: Rob Clark We'll need to introduce a _locked() version of msm_gem_get_iova(), so we need to make that name available. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem.c b

[PATCH v3 11/23] drm/msm: Drop chatty trace

2020-10-19 Thread Rob Clark
From: Rob Clark It is somewhat redundant with the gpu tracepoints, and anyways not too useful to justify spamming the log when debug traces are enabled. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gpu.c | 1 - 1 file changed, 1 deletion(-) diff --git

[PATCH v3 00/23] drm/msm: de-struct_mutex-ification

2020-10-19 Thread Rob Clark
From: Rob Clark 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

[PATCH v3 07/23] drm/msm/submit: Move copy_from_user ahead of locking bos

2020-10-19 Thread Rob Clark
From: Rob Clark We cannot switch to using obj->resv for locking without first moving all the copy_from_user() ahead of submit_lock_objects(). Otherwise in the mm fault path we aquire mm->mmap_sem before obj lock, but in the submit path the order is reversed. Signed-off-by: Rob

[PATCH v3 10/23] drm/msm: Use correct drm_gem_object_put() in fail case

2020-10-19 Thread Rob Clark
From: Rob Clark We only want to use the _unlocked() variant in the unlocked case. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index

[PATCH v3 04/23] drm/msm/gem: Move prototypes to msm_gem.h

2020-10-19 Thread Rob Clark
From: Rob Clark Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 + drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c| 1 + drivers/gpu/drm/msm/msm_drv.h | 54 -- drivers/gpu/drm/msm

[PATCH v3 09/23] drm/msm/gem: Switch over to obj->resv for locking

2020-10-19 Thread Rob Clark
From: Rob Clark This also converts the special msm_gem_get_vaddr_active() to expect the lock to already be held. There are two call-sites for this, one already has the lock held, so it is more straightforward to just open-code the locking for the other caller. Signed-off-by: Rob Clark

[PATCH v3 08/23] drm/msm: Do rpm get sooner in the submit path

2020-10-19 Thread Rob Clark
From: Rob Clark Unfortunately, due to an dev_pm_opp locking interaction with mm->mmap_sem, we need to do pm get before aquiring obj locks, otherwise we can have anger lockdep with the chain: opp_table_lock --> >mmap_sem --> reservation_ww_class_mutex For an explicit fenci

[PATCH v3 05/23] drm/msm/gem: Add some _locked() helpers

2020-10-19 Thread Rob Clark
From: Rob Clark When we cut-over to using dma_resv_lock/etc instead of msm_obj->lock, we'll need these for the submit path (where resv->lock is already held). Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 89 +++ drivers/gpu/drm/msm/msm

[PATCH v3 06/23] drm/msm/gem: Move locking in shrinker path

2020-10-19 Thread Rob Clark
From: Rob Clark Move grabbing the bo lock into shrinker, with a msm_gem_trylock() to skip over bo's that are already locked. This gets rid of the nested lock classes. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 24 + drivers/gpu/drm/msm/msm_gem.h

[PATCH v3 01/23] drm/msm: Fix a couple incorrect usages of get_vaddr_active()

2020-10-19 Thread Rob Clark
From: Rob Clark The microcode bo's should never be madvise(WONTNEED), so these should not be using msm_gem_get_vaddr_active(). Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 2 +- drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions

Re: [PATCH v2 0/3] drm: commit_work scheduling

2020-10-16 Thread Rob Clark
On Thu, Oct 8, 2020 at 1:24 AM Ville Syrjälä wrote: > > On Wed, Oct 07, 2020 at 09:44:09AM -0700, Rob Clark wrote: > > On Mon, Oct 5, 2020 at 5:15 AM Ville Syrjälä > > wrote: > > > > > > On Fri, Oct 02, 2020 at 10:55:52AM -0700, Rob Clark wrote: > >

Re: [v3] drm/msm: Fix race condition in msm driver with async layer updates

2020-10-16 Thread Rob Clark
s such that second commit waits > until first commit flushes the composition. > > This change also introduces per crtc commit lock, such that > commits on different crtcs are not blocked by each other. > > Changes in v2: > - Use an array of mutexes in kms to handle commi

Re: [v2] drm/msm: Fix race condition in msm driver with async layer updates

2020-10-15 Thread Rob Clark
s such that second commit waits > until first commit flushes the composition. > > This change also introduces per crtc commit lock, such that > commits on different crtcs are not blocked by each other. > > Changes in v2: > - Use an array of mutexes in kms to handle commi

Re: [v1] drm/msm: Fix race condition in msm driver with async layer updates

2020-10-14 Thread Rob Clark
On Wed, Oct 14, 2020 at 5:58 AM Krishna Manikandan wrote: > > When there are back to back commits with async cursor update, > there is a case where second commit can program the DPU hw > blocks while first didn't complete flushing config to HW. > > Synchronize the compositions such that second

Re: [Freedreno] [PATCH v2 22/22] drm/msm: Don't implicit-sync if only a single ring

2020-10-13 Thread Rob Clark
On Tue, Oct 13, 2020 at 4:08 AM Daniel Vetter wrote: > > On Mon, Oct 12, 2020 at 08:07:38AM -0700, Rob Clark wrote: > > On Mon, Oct 12, 2020 at 7:40 AM Daniel Vetter wrote: > > > > > > On Sun, Oct 11, 2020 at 07:09:49PM -0700, Rob Clark wrote: > > > >

Re: [PATCH 2/3] drm/msm: add DRM_MSM_GEM_SYNC_CACHE for non-coherent cache maintenance

2020-10-13 Thread Rob Clark
On Tue, Oct 13, 2020 at 6:42 AM Robin Murphy wrote: > > On 2020-10-07 07:25, Christoph Hellwig wrote: > > On Tue, Oct 06, 2020 at 09:19:32AM -0400, Jonathan Marek wrote: > >> One example why drm/msm can't use DMA API is multiple page table support > >> (that is landing in 5.10), which is

Re: [PATCH v2 07/22] drm/msm: Do rpm get sooner in the submit path

2020-10-12 Thread Rob Clark
On Mon, Oct 12, 2020 at 7:35 AM Daniel Vetter wrote: > > On Sun, Oct 11, 2020 at 07:09:34PM -0700, Rob Clark wrote: > > From: Rob Clark > > > > Unfortunately, due to an dev_pm_opp locking interaction with > > mm->mmap_sem, we need to do pm get before aquiring obj

Re: [PATCH v2 22/22] drm/msm: Don't implicit-sync if only a single ring

2020-10-12 Thread Rob Clark
On Mon, Oct 12, 2020 at 7:40 AM Daniel Vetter wrote: > > On Sun, Oct 11, 2020 at 07:09:49PM -0700, Rob Clark wrote: > > From: Rob Clark > > > > Any cross-device sync use-cases *must* use explicit sync. And if there > > is only a single ring (no-preempt

[PATCH v2 17/22] drm/msm: Drop struct_mutex from the retire path

2020-10-11 Thread Rob Clark
From: Rob Clark Now that we are not relying on dev->struct_mutex to protect the ring->submits lists, drop the struct_mutex lock. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gpu.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff

[PATCH v2 20/22] drm/msm: drop struct_mutex in madvise path

2020-10-11 Thread Rob Clark
From: Rob Clark The obj->lock is sufficient for what we need. This *does* have the implication that userspace can try to shoot themselves in the foot by racing madvise(DONTNEED) with submit. But the result will be about the same if they did madvise(DONTNEED) before the submit ioctl,

[PATCH v2 08/22] drm/msm/gem: Switch over to obj->resv for locking

2020-10-11 Thread Rob Clark
From: Rob Clark Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c| 4 +--- drivers/gpu/drm/msm/msm_gem.h| 16 +--- drivers/gpu/drm/msm/msm_gem_submit.c | 4 ++-- drivers/gpu/drm/msm/msm_gpu.c| 2 +- 4 files changed, 9 insertions(+), 17 deletions

[PATCH v2 15/22] drm/msm: Refcount submits

2020-10-11 Thread Rob Clark
From: Rob Clark Before we remove dev->struct_mutex from the retire path, we have to deal with the situation of a submit retiring before the submit ioctl returns. To deal with this, ring->submits will hold a reference to the submit, which is dropped when the submit is retired. And the

[PATCH v2 14/22] drm/msm: Protect ring->submits with it's own lock

2020-10-11 Thread Rob Clark
From: Rob Clark One less place to rely on dev->struct_mutex. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gem_submit.c | 2 ++ drivers/gpu/drm/msm/msm_gpu.c| 37 ++-- drivers/gpu/drm/msm/msm_ringbuffer.c | 1 + drivers/

[PATCH v2 02/22] drm/msm/gem: Rename internal get_iova_locked helper

2020-10-11 Thread Rob Clark
From: Rob Clark We'll need to introduce a _locked() version of msm_gem_get_iova(), so we need to make that name available. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem.c b

[PATCH v2 18/22] drm/msm: Drop struct_mutex in free_object() path

2020-10-11 Thread Rob Clark
From: Rob Clark Now that active_list/inactive_list is protected by mm_lock, we no longer need dev->struct_mutex in the free_object() path. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem.

[PATCH v2 19/22] drm/msm: remove msm_gem_free_work

2020-10-11 Thread Rob Clark
From: Rob Clark Now that we don't need struct_mutex in the free path, we can get rid of the asynchronous free altogether. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_drv.c | 3 --- drivers/gpu/drm/msm/msm_drv.h | 5 - drivers/gpu/drm/msm/msm_gem.c | 27

[PATCH v2 13/22] drm/msm: Document and rename preempt_lock

2020-10-11 Thread Rob Clark
From: Rob Clark Before adding another lock, give ring->lock a more descriptive name. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++-- drivers/gpu/drm/msm/adreno/a5xx_preempt.c | 12 ++-- drivers/gpu/drm/msm/adreno/a6xx_gp

[PATCH v2 11/22] drm/msm: Move update_fences()

2020-10-11 Thread Rob Clark
From: Rob Clark Small cleanup, update_fences() is used in the hangcheck path, but also in the normal retire path. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gpu.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff

[PATCH v2 05/22] drm/msm/gem: Move locking in shrinker path

2020-10-11 Thread Rob Clark
From: Rob Clark Move grabbing the bo lock into shrinker, with a msm_gem_trylock() to skip over bo's that are already locked. This gets rid of the nested lock classes. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 24 + drivers/gpu/drm/msm/msm_gem.h

[PATCH v2 09/22] drm/msm: Use correct drm_gem_object_put() in fail case

2020-10-11 Thread Rob Clark
From: Rob Clark We only want to use the _unlocked() variant in the unlocked case. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index

[PATCH v2 03/22] drm/msm/gem: Move prototypes to msm_gem.h

2020-10-11 Thread Rob Clark
From: Rob Clark Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 + drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c| 1 + drivers/gpu/drm/msm/msm_drv.h | 54 -- drivers/gpu/drm/msm

[PATCH v2 16/22] drm/msm: Remove obj->gpu

2020-10-11 Thread Rob Clark
From: Rob Clark It cannot be atomically updated with obj->active_count, and the only purpose is a useless WARN_ON() (which becomes a buggy WARN_ON() once retire_submits() is not serialized with incoming submits via struct_mutex) Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c

[PATCH v2 07/22] drm/msm: Do rpm get sooner in the submit path

2020-10-11 Thread Rob Clark
From: Rob Clark Unfortunately, due to an dev_pm_opp locking interaction with mm->mmap_sem, we need to do pm get before aquiring obj locks, otherwise we can have anger lockdep with the chain: opp_table_lock --> >mmap_sem --> reservation_ww_class_mutex For an explicit fenci

[PATCH v2 12/22] drm/msm: Add priv->mm_lock to protect active/inactive lists

2020-10-11 Thread Rob Clark
From: Rob Clark Rather than relying on the big dev->struct_mutex hammer, introduce a more specific lock for protecting the bo lists. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_debugfs.c | 7 +++ drivers/gpu/drm/msm/msm_drv.c |

[PATCH v2 04/22] drm/msm/gem: Add some _locked() helpers

2020-10-11 Thread Rob Clark
From: Rob Clark When we cut-over to using dma_resv_lock/etc instead of msm_obj->lock, we'll need these for the submit path (where resv->lock is already held). Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 50 +++ drivers/gpu/drm/msm/msm

[PATCH v2 01/22] drm/msm/gem: Add obj->lock wrappers

2020-10-11 Thread Rob Clark
From: Rob Clark This will make it easier to transition over to obj->resv locking for everything that is per-bo locking. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 99 --- drivers/gpu/drm/msm/msm_gem.h | 28 ++ 2 files changed,

[PATCH v2 22/22] drm/msm: Don't implicit-sync if only a single ring

2020-10-11 Thread Rob Clark
From: Rob Clark Any cross-device sync use-cases *must* use explicit sync. And if there is only a single ring (no-preemption), everything is FIFO order and there is no need to implicit-sync. Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as behavior is undefined when fences

[PATCH v2 06/22] drm/msm/submit: Move copy_from_user ahead of locking bos

2020-10-11 Thread Rob Clark
From: Rob Clark We cannot switch to using obj->resv for locking without first moving all the copy_from_user() ahead of submit_lock_objects(). Otherwise in the mm fault path we aquire mm->mmap_sem before obj lock, but in the submit path the order is reversed. Signed-off-by: Rob

[PATCH v2 21/22] drm/msm: Drop struct_mutex in shrinker path

2020-10-11 Thread Rob Clark
From: Rob Clark Now that the inactive_list is protected by mm_lock, and everything else on per-obj basis is protected by obj->lock, we no longer depend on struct_mutex. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 1 - drivers/gpu/drm/msm/msm_gem_shrinker.c |

[PATCH 00/14] drm/msm: de-struct_mutex-ification

2020-10-11 Thread Rob Clark
From: Rob Clark 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

[PATCH v2 10/22] drm/msm: Drop chatty trace

2020-10-11 Thread Rob Clark
From: Rob Clark It is somewhat redundant with the gpu tracepoints, and anyways not too useful to justify spamming the log when debug traces are enabled. Signed-off-by: Rob Clark Reviewed-by: Jordan Crouse --- drivers/gpu/drm/msm/msm_gpu.c | 1 - 1 file changed, 1 deletion(-) diff --git

Re: [PATCH v2 0/3] drm: commit_work scheduling

2020-10-07 Thread Rob Clark
On Mon, Oct 5, 2020 at 5:15 AM Ville Syrjälä wrote: > > On Fri, Oct 02, 2020 at 10:55:52AM -0700, Rob Clark wrote: > > On Fri, Oct 2, 2020 at 4:05 AM Ville Syrjälä > > wrote: > > > > > > On Fri, Oct 02, 2020 at 01:52:56PM +0300, Ville Syrjälä wrote: > &

Re: [PATCH v2 0/3] drm: commit_work scheduling

2020-10-07 Thread Rob Clark
On Wed, Oct 7, 2020 at 3:36 AM Qais Yousef wrote: > > On 10/06/20 13:04, Rob Clark wrote: > > On Tue, Oct 6, 2020 at 3:59 AM Qais Yousef wrote: > > > > > > On 10/05/20 16:24, Rob Clark wrote: > > > > > > [...] > > > > > > >

Re: [PATCH v2 0/3] drm: commit_work scheduling

2020-10-06 Thread Rob Clark
On Tue, Oct 6, 2020 at 3:59 AM Qais Yousef wrote: > > On 10/05/20 16:24, Rob Clark wrote: > > [...] > > > > RT planning and partitioning is not easy task for sure. You might want to > > > consider using affinities too to get stronger guarantees for some tas

Re: [PATCH 13/14] drm/msm: Drop struct_mutex in shrinker path

2020-10-05 Thread Rob Clark
gt; > > > > > On Sun, 4 Oct 2020 12:21:45 > > > > > From: Rob Clark > > > > > > > > > > Now that the inactive_list is protected by mm_lock, and everything > > > > > else on per-obj basis is protected by obj->lock, we no longer

Re: [Freedreno] [PATCH 00/14] drm/msm: de-struct_mutex-ification

2020-10-05 Thread Rob Clark
On Mon, Oct 5, 2020 at 11:20 AM Daniel Vetter wrote: > > On Mon, Oct 5, 2020 at 6:24 PM Kristian Høgsberg wrote: > > > > On Sun, Oct 4, 2020 at 9:21 PM Rob Clark wrote: > > > > > > From: Rob Clark > > > > > > This doesn't remove *all* the

Re: [PATCH v2 0/3] drm: commit_work scheduling

2020-10-05 Thread Rob Clark
On Mon, Oct 5, 2020 at 8:00 AM Qais Yousef wrote: > > +CC Steve and Peter - they might be interested. > > On 10/02/20 11:07, Rob Clark wrote: > > On Fri, Oct 2, 2020 at 4:01 AM Qais Yousef wrote: > > > > > > On 09/30/20 14:17, Rob Clark wrote: > > > &

Re: [PATCH v2 0/3] drm: commit_work scheduling

2020-10-05 Thread Rob Clark
On Mon, Oct 5, 2020 at 7:15 AM Daniel Vetter wrote: > > On Mon, Oct 05, 2020 at 03:15:24PM +0300, Ville Syrjälä wrote: > > On Fri, Oct 02, 2020 at 10:55:52AM -0700, Rob Clark wrote: > > > On Fri, Oct 2, 2020 at 4:05 AM Ville Syrjälä > > > wrote: > > > >

Re: [PATCH 13/14] drm/msm: Drop struct_mutex in shrinker path

2020-10-05 Thread Rob Clark
On Mon, Oct 5, 2020 at 7:02 AM Daniel Vetter wrote: > > On Mon, Oct 05, 2020 at 05:24:19PM +0800, Hillf Danton wrote: > > > > On Sun, 4 Oct 2020 12:21:45 > > > From: Rob Clark > > > > > > Now that the inactive_list is protected by mm_lock, and everyt

Re: [PATCH 07/14] drm/msm: Refcount submits

2020-10-05 Thread Rob Clark
On Mon, Oct 5, 2020 at 6:56 AM Daniel Vetter wrote: > > On Sun, Oct 04, 2020 at 12:21:39PM -0700, Rob Clark wrote: > > From: Rob Clark > > > > Before we remove dev->struct_mutex from the retire path, we have to deal > > with the situation of a submit retirin

Re: [PATCH 04/14] drm/msm: Add priv->mm_lock to protect active/inactive lists

2020-10-04 Thread Rob Clark
On Sun, Oct 4, 2020 at 3:15 PM Daniel Vetter wrote: > > On Sun, Oct 4, 2020 at 9:21 PM Rob Clark wrote: > > > > From: Rob Clark > > > > Rather than relying on the big dev->struct_mutex hammer, introduce a > > more specific lock for protecting the bo

[PATCH 07/14] drm/msm: Refcount submits

2020-10-04 Thread Rob Clark
From: Rob Clark Before we remove dev->struct_mutex from the retire path, we have to deal with the situation of a submit retiring before the submit ioctl returns. To deal with this, ring->submits will hold a reference to the submit, which is dropped when the submit is retired. And the

[PATCH 10/14] drm/msm: Drop struct_mutex in free_object() path

2020-10-04 Thread Rob Clark
From: Rob Clark Now that active_list/inactive_list is protected by mm_lock, we no longer need dev->struct_mutex in the free_object() path. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 8 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem.

[PATCH 14/14] drm/msm: Don't implicit-sync if only a single ring

2020-10-04 Thread Rob Clark
From: Rob Clark Any cross-device sync use-cases *must* use explicit sync. And if there is only a single ring (no-preemption), everything is FIFO order and there is no need to implicit-sync. Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as behavior is undefined when fences

[PATCH 11/14] drm/msm: remove msm_gem_free_work

2020-10-04 Thread Rob Clark
From: Rob Clark Now that we don't need struct_mutex in the free path, we can get rid of the asynchronous free altogether. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_drv.c | 3 --- drivers/gpu/drm/msm/msm_drv.h | 5 - drivers/gpu/drm/msm/msm_gem.c | 27

[PATCH 09/14] drm/msm: Drop struct_mutex from the retire path

2020-10-04 Thread Rob Clark
From: Rob Clark Now that we are not relying on dev->struct_mutex to protect the ring->submits lists, drop the struct_mutex lock. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gpu.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/gpu/drm/msm/msm

[PATCH 06/14] drm/msm: Protect ring->submits with it's own lock

2020-10-04 Thread Rob Clark
From: Rob Clark One less place to rely on dev->struct_mutex. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem_submit.c | 2 ++ drivers/gpu/drm/msm/msm_gpu.c| 37 ++-- drivers/gpu/drm/msm/msm_ringbuffer.c | 1 + drivers/gpu/drm/msm/msm_ringbuffe

[PATCH 12/14] drm/msm: drop struct_mutex in madvise path

2020-10-04 Thread Rob Clark
From: Rob Clark The obj->lock is sufficient for what we need. This *does* have the implication that userspace can try to shoot themselves in the foot by racing madvise(DONTNEED) with submit. But the result will be about the same if they did madvise(DONTNEED) before the submit ioctl,

[PATCH 02/14] drm/msm: Drop chatty trace

2020-10-04 Thread Rob Clark
From: Rob Clark It is somewhat redundant with the gpu tracepoints, and anyways not too useful to justify spamming the log when debug traces are enabled. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gpu.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_gpu.c

[PATCH 08/14] drm/msm: Remove obj->gpu

2020-10-04 Thread Rob Clark
From: Rob Clark It cannot be atomically updated with obj->active_count, and the only purpose is a useless WARN_ON() (which becomes a buggy WARN_ON() once retire_submits() is not serialized with incoming submits via struct_mutex) Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c

[PATCH 01/14] drm/msm: Use correct drm_gem_object_put() in fail case

2020-10-04 Thread Rob Clark
From: Rob Clark We only want to use the _unlocked() variant in the unlocked case. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c index

[PATCH 13/14] drm/msm: Drop struct_mutex in shrinker path

2020-10-04 Thread Rob Clark
From: Rob Clark Now that the inactive_list is protected by mm_lock, and everything else on per-obj basis is protected by obj->lock, we no longer depend on struct_mutex. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gem.c | 1 - drivers/gpu/drm/msm/msm_gem_shrinker.c |

[PATCH 04/14] drm/msm: Add priv->mm_lock to protect active/inactive lists

2020-10-04 Thread Rob Clark
From: Rob Clark Rather than relying on the big dev->struct_mutex hammer, introduce a more specific lock for protecting the bo lists. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_debugfs.c | 7 +++ drivers/gpu/drm/msm/msm_drv.c | 1 + drivers/gpu/drm/msm/msm_dr

[PATCH 03/14] drm/msm: Move update_fences()

2020-10-04 Thread Rob Clark
From: Rob Clark Small cleanup, update_fences() is used in the hangcheck path, but also in the normal retire path. Signed-off-by: Rob Clark --- drivers/gpu/drm/msm/msm_gpu.c | 28 ++-- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/msm

[PATCH 05/14] drm/msm: Document and rename preempt_lock

2020-10-04 Thread Rob Clark
From: Rob Clark Before adding another lock, give ring->lock a more descriptive name. Signed-off-by: Rob Clark --- 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/

[PATCH 00/14] drm/msm: de-struct_mutex-ification

2020-10-04 Thread Rob Clark
From: Rob Clark 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

Re: [RFC PATCH] drm: add support for taking drm atomic state snapshot

2020-10-02 Thread Rob Clark
On Fri, Oct 2, 2020 at 11:54 AM Daniel Vetter wrote: > > On Fri, Oct 02, 2020 at 10:22:42AM -0700, Rob Clark wrote: > > On Fri, Oct 2, 2020 at 12:36 AM Daniel Vetter wrote: > > > > > > On Fri, Oct 2, 2020 at 3:47 AM Abhinav Kumar > > > wrote: > &g

Re: [PATCH v2 0/3] drm: commit_work scheduling

2020-10-02 Thread Rob Clark
On Fri, Oct 2, 2020 at 4:01 AM Qais Yousef wrote: > > On 09/30/20 14:17, Rob Clark wrote: > > From: Rob Clark > > > > The android userspace treats the display pipeline as a realtime problem. > > And arguably, if your goal is to not miss frame deadlines (ie. v

Re: [PATCH v2 0/3] drm: commit_work scheduling

2020-10-02 Thread Rob Clark
On Fri, Oct 2, 2020 at 4:05 AM Ville Syrjälä wrote: > > On Fri, Oct 02, 2020 at 01:52:56PM +0300, Ville Syrjälä wrote: > > On Thu, Oct 01, 2020 at 05:25:55PM +0200, Daniel Vetter wrote: > > > On Thu, Oct 1, 2020 at 5:15 PM Rob Clark wrote: > > > > > > >

Re: [RFC PATCH] drm: add support for taking drm atomic state snapshot

2020-10-02 Thread Rob Clark
On Fri, Oct 2, 2020 at 12:36 AM Daniel Vetter wrote: > > On Fri, Oct 2, 2020 at 3:47 AM Abhinav Kumar wrote: > > > > Add support to capture the drm atomic state snapshot which > > can then be wired up with the devcoredump of the relevant display > > errors to give useful information to debug the

Re: [PATCH v2 0/3] drm: commit_work scheduling

2020-10-01 Thread Rob Clark
On Thu, Oct 1, 2020 at 12:25 AM Daniel Vetter wrote: > > On Wed, Sep 30, 2020 at 11:16 PM Rob Clark wrote: > > > > From: Rob Clark > > > > The android userspace treats the display pipeline as a realtime problem. > > And arguably, if your goal is to

[PATCH v2 3/3] drm: Expose CRTC's kworker task id

2020-09-30 Thread Rob Clark
From: Rob Clark This will allow userspace to control the scheduling policy and priority. In particular if the userspace half of the display pipeline is SCHED_FIFO then it will want to use the same scheduling policy and an appropriate priority to ensure that it is not preempting commit_work

[PATCH v2 0/3] drm: commit_work scheduling

2020-09-30 Thread Rob Clark
From: Rob Clark The android userspace treats the display pipeline as a realtime problem. And arguably, if your goal is to not miss frame deadlines (ie. vblank), it is. (See https://lwn.net/Articles/809545/ for the best explaination that I found.) But this presents a problem with using

  1   2   3   4   5   6   7   8   9   10   >