[RESEND PATCH v4] drm: Don't free jobs in wait_event_interruptible()

2019-10-24 Thread Steven Price
() and simply return a job for processing if there is one. The caller can then call the free_job() callback outside the wait_event_interruptible() where sleeping is possible before re-checking and returning to sleep if necessary. Signed-off-by: Steven Price --- Previous posting: https

Re: [PATCH v2] panfrost: Properly undo pm_runtime_enable when deferring a probe

2019-10-23 Thread Steven Price
Reported-by: Chen-Yu Tsai > Cc: Robin Murphy > Fixes: f4a3c6a44b35 ("drm/panfrost: Disable PM on probe failure") As Robin pointed out this should be: Fixes: 635430797d3f ("drm/panfrost: Rework runtime PM initialization") But other than that, Reviewed-by: S

Re: [PATCH v2] drm/panfrost: fix -Wmissing-prototypes warnings

2019-10-23 Thread Steven Price
his. > For file panfrost_perfcnt.c, include header file can fix this. > > Signed-off-by: Yi Wang > Reviewed-by: Steven Price > --- > > v2: align parameter line, modify comment and sort header > includes alphabetically. Thanks to Steve. > --- > drivers/gpu/drm/panfrost/pan

Re: [PATCH] drm/panfrost: fix -Wmissing-prototypes warnings

2019-10-21 Thread Steven Price
e panfrost_perfcnt.c, include head file can fix this. Nit: s/head/header/ > > Signed-off-by: Yi Wang Looks good, just a few minor style issues (below), with those fixed: Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_mmu.c | 5 +++-- > drivers

Re: [PATCH] drm/panfrost: DMA map all pages shared with the GPU

2019-10-14 Thread Steven Price
On 14/10/2019 16:55, Steven Price wrote: > On 14/10/2019 16:46, Robin Murphy wrote: >> On 14/10/2019 16:16, Steven Price wrote: >>> Pages shared with the GPU are (often) not cache coherent with the CPU so >>> cache maintenance is required to flush the CPU's caches. Thi

Re: [PATCH] drm/panfrost: DMA map all pages shared with the GPU

2019-10-14 Thread Steven Price
On 14/10/2019 16:46, Robin Murphy wrote: > On 14/10/2019 16:16, Steven Price wrote: >> Pages shared with the GPU are (often) not cache coherent with the CPU so >> cache maintenance is required to flush the CPU's caches. This was >> already done when mapping pages on fault,

[PATCH] drm/panfrost: DMA map all pages shared with the GPU

2019-10-14 Thread Steven Price
). In theory it also allows the GPU (and by extension user space) to observe the memory contents prior to sanitization. Fixes: f3ba91228e8e ("drm/panfrost: Add initial panfrost driver") Signed-off-by: Steven Price --- drivers/gpu/drm/panfrost/panfrost_mmu.c | 20 1 fi

[PATCH] drm/panfrost: Add missing GPU feature registers

2019-10-14 Thread Steven Price
Three feature registers were declared but never actually read from the GPU. Add THREAD_MAX_THREADS, THREAD_MAX_WORKGROUP_SIZE and THREAD_MAX_BARRIER_SIZE so that the complete set are available. Fixes: 4bced8bea094 ("drm/panfrost: Export all GPU feature registers") Signed-off-by: St

[PATCH v2 1/2] drm/panfrost: Handle resetting on timeout better

2019-10-09 Thread Steven Price
Signed-off-by: Steven Price --- v2: * Added fixes and tested-by tags * Moved cleanup of panfrost_core_dump() comment to separate patch drivers/gpu/drm/panfrost/panfrost_job.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c

[PATCH v2 2/2] drm/panfrost: Remove commented out call to panfrost_core_dump

2019-10-09 Thread Steven Price
panfrost_core_dump() has never existed in mainline, so remove it and add a TODO entry that core dump support is currently lacking. Signed-off-by: Steven Price --- v2: * New patch drivers/gpu/drm/panfrost/TODO | 2 ++ drivers/gpu/drm/panfrost/panfrost_job.c | 2 -- 2 files changed, 2

Re: [PATCH] drm/panfrost: Handle resetting on timeout better

2019-10-09 Thread Steven Price
On 07/10/2019 17:14, Tomeu Vizoso wrote: > On 10/7/19 6:09 AM, Neil Armstrong wrote: >> Hi Steven, >> >> On 07/10/2019 14:50, Steven Price wrote: >>> Panfrost uses multiple schedulers (one for each slot, so 2 in reality), >>> and on a timeout has to st

[PATCH] drm/panfrost: Handle resetting on timeout better

2019-10-07 Thread Steven Price
-by: Steven Price --- This is a tidied up version of the patch orginally posted here: http://lkml.kernel.org/r/26ae2a4d-8df1-e8db-3060-41638ed63e2a%40arm.com drivers/gpu/drm/panfrost/panfrost_job.c | 17 +++-- 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm

Re: drm_sched with panfrost crash on T820

2019-10-07 Thread Steven Price
On 04/10/2019 17:33, Koenig, Christian wrote: > > > Am 04.10.2019 18:02 schrieb Steven Price : > On 04/10/2019 16:34, Koenig, Christian wrote: >> Am 04.10.19 um 17:27 schrieb Steven Price: >>> On 04/10/2019 16:03, Neil Armstrong wrote: >>>> On 04/

Re: drm_sched with panfrost crash on T820

2019-10-04 Thread Steven Price
On 04/10/2019 16:34, Koenig, Christian wrote: > Am 04.10.19 um 17:27 schrieb Steven Price: >> On 04/10/2019 16:03, Neil Armstrong wrote: >>> On 04/10/2019 16:53, Grodzovsky, Andrey wrote: >>>> On 10/3/19 4:34 AM, Neil Armstrong wrote: >>>>> Hi Andrey,

Re: drm_sched with panfrost crash on T820

2019-10-04 Thread Steven Price
On 04/10/2019 16:03, Neil Armstrong wrote: > On 04/10/2019 16:53, Grodzovsky, Andrey wrote: >> >> On 10/3/19 4:34 AM, Neil Armstrong wrote: >>> Hi Andrey, >>> >>> Le 02/10/2019 à 16:40, Grodzovsky, Andrey a écrit : On 9/30/19 10:52 AM, Hillf Danton wrote: > On Mon, 30 Sep 2019 11:17:45

[PATCH] drm/panfrost: Remove NULL check for regulator

2019-10-04 Thread Steven Price
devm_regulator_get() is used to populate pfdev->regulator which ensures that this cannot be NULL (a dummy regulator will be returned if necessary). So remove the check in panfrost_devfreq_target(). Signed-off-by: Steven Price --- This looks like it was accidentally reintroduced by the merge f

Re: [PATCH 2/2] drm/panfrost: Use coherent pagetable walk on Juno

2019-09-30 Thread Steven Price
On 30/09/2019 16:24, Robin Murphy wrote: > Although going full "dma-coherent" ends badly due to GEM objects still > being forcibly mapped non-cacheable, we can at least take advantage of > Juno's ACE-lite integration to skip cache maintenance for pagetables. > > CC: Rob Herring > CC: Tomeu

Re: drm_sched with panfrost crash on T820

2019-09-27 Thread Steven Price
On 27/09/2019 12:48, Neil Armstrong wrote: > Hi, > > On 27/09/2019 13:27, Neil Armstrong wrote: >> Hi Steven, >> >> Thanks for your prompt reaction ! >> >> On 27/09/2019 12:48, Steven Price wrote: >>> On 27/09/2019 10:55, Steven Price wrote: >>

Re: [PATCH v2 2/6] drm/v3d: use drm_gem_objects_lookup_user

2019-09-27 Thread Steven Price
On 27/09/2019 14:46, Qiang Yu wrote: > v2: > use drm_gem_objects_lookup_user instead of > drm_gem_objects_lookup. > > Cc: Eric Anholt > Signed-off-by: Qiang Yu Looks familiar :) Nit: please write a commit message. But otherwise: Reviewed-by: Steven Price > --- &

Re: [PATCH v2 1/6] drm/gem: refine drm_gem_objects_lookup

2019-09-27 Thread Steven Price
user space handles. > > v2: > 1. add drm_gem_objects_lookup_user > 2. remove none-zero check as all caller will check before >calling this function > > Cc: Rob Herring > Cc: Tomeu Vizoso > Cc: Steven Price > Cc: Alyssa Rosenzweig > Suggested-by: Rob Herri

Re: drm_sched with panfrost crash on T820

2019-09-27 Thread Steven Price
On 27/09/2019 10:55, Steven Price wrote: [...] > One obvious issue with the DRM scheduler is that there is a call to > cancel_delayed_work() in drm_sched_stop() which to me looks like it > should be cancel_delayed_work_sync() to ensure that the timeout handling > has completed.

Re: drm_sched with panfrost crash on T820

2019-09-27 Thread Steven Price
On 27/09/2019 09:12, Neil Armstrong wrote: > Hi Christian, > > In v5.3, running dEQP triggers the following kernel crash : > > [ 20.224982] Unable to handle kernel NULL pointer dereference at virtual > address 0038 > [...] > [ 20.291064] Hardware name: Khadas VIM2 (DT) > [

Re: [PATCH v4] drm: Don't free jobs in wait_event_interruptible()

2019-09-26 Thread Steven Price
On 26/09/2019 16:48, Grodzovsky, Andrey wrote: > > On 9/26/19 11:23 AM, Steven Price wrote: >> On 26/09/2019 16:14, Grodzovsky, Andrey wrote: >>> On 9/26/19 10:16 AM, Steven Price wrote: >>>> drm_sched_cleanup_jobs() attempts to free finished jobs,

Re: [PATCH v4] drm: Don't free jobs in wait_event_interruptible()

2019-09-26 Thread Steven Price
On 26/09/2019 16:14, Grodzovsky, Andrey wrote: > > On 9/26/19 10:16 AM, Steven Price wrote: >> drm_sched_cleanup_jobs() attempts to free finished jobs, however because >> it is called as the condition of wait_event_interruptible() it must not >> sleep. Unfortuantly some

Re: [PATCH 0/6] drm/lima: simplify driver by using more drm helpers

2019-09-26 Thread Steven Price
On 26/09/2019 15:10, Qiang Yu wrote: > By using shared drm helpers: > 1. drm_gem_objects_lookup > 2. drm_gem_(un)lock_reservations > 3. drm_gem_shmem_helpers > we can simplify lima driver a lot and benifit from updates to > these functions. > > drm_gem_objects_lookup need a refine in order to be

[PATCH v4] drm: Don't free jobs in wait_event_interruptible()

2019-09-26 Thread Steven Price
() and simply return a job for processing if there is one. The caller can then call the free_job() callback outside the wait_event_interruptible() where sleeping is possible before re-checking and returning to sleep if necessary. Signed-off-by: Steven Price --- Changes from v3: * drm_sched_main() re

Re: [PATCH v3] drm: Don't free jobs in wait_event_interruptible()

2019-09-26 Thread Steven Price
On 26/09/2019 14:37, Koenig, Christian wrote: > Am 26.09.19 um 14:31 schrieb Steven Price: >> drm_sched_cleanup_jobs() attempts to free finished jobs, however because >> it is called as the condition of wait_event_interruptible() it must not >> sleep. Unfortuantly some fr

[PATCH v3] drm: Don't free jobs in wait_event_interruptible()

2019-09-26 Thread Steven Price
() and simply return a job for processing if there is one. The caller can then call the free_job() callback outside the wait_event_interruptible() where sleeping is possible before re-checking and returning to sleep if necessary. Signed-off-by: Steven Price --- Changes from v2: * Actually move

Re: [PATCH v2] drm: Don't free jobs in wait_event_interruptible()

2019-09-26 Thread Steven Price
On 26/09/2019 10:54, Steven Price wrote: > drm_sched_cleanup_jobs() attempts to free finished jobs, however because > it is called as the condition of wait_event_interruptible() it must not > sleep. Unfortuantly some free callbacks (notibly for Panfrost) do sleep. > > Inste

[PATCH v2] drm: Don't free jobs in wait_event_interruptible()

2019-09-26 Thread Steven Price
() and simply return a job for processing if there is one. The caller can then call the free_job() callback outside the wait_event_interruptible() where sleeping is possible before re-checking and returning to sleep if necessary. Signed-off-by: Steven Price --- Changes from v1: * Move

Re: [PATCH] drm: Don't free jobs in wait_event_interruptible()

2019-09-26 Thread Steven Price
On 26/09/2019 08:07, Koenig, Christian wrote: > Am 25.09.19 um 17:14 schrieb Steven Price: >> drm_sched_cleanup_jobs() attempts to free finished jobs, however because >> it is called as the condition of wait_event_interruptible() it must not >> sleep. Unfortunately some fr

Re: [PATCH] drm: Don't free jobs in wait_event_interruptible()

2019-09-26 Thread Steven Price
On 25/09/2019 21:09, Grodzovsky, Andrey wrote: > > On 9/25/19 12:07 PM, Andrey Grodzovsky wrote: >> On 9/25/19 12:00 PM, Steven Price wrote: >> >>> On 25/09/2019 16:56, Grodzovsky, Andrey wrote: >>>> On 9/25/19 11:14 AM, Steven Price wrote: >>>&

Re: [PATCH] drm: Don't free jobs in wait_event_interruptible()

2019-09-25 Thread Steven Price
On 25/09/2019 16:56, Grodzovsky, Andrey wrote: > On 9/25/19 11:14 AM, Steven Price wrote: > >> drm_sched_cleanup_jobs() attempts to free finished jobs, however because >> it is called as the condition of wait_event_interruptible() it must not >> sleep. Unfortunately some

[PATCH] drm: Don't free jobs in wait_event_interruptible()

2019-09-25 Thread Steven Price
() and simply return a job for processing if there is one. The caller can then call the free_job() callback outside the wait_event_interruptible() where sleeping is possible before re-checking and returning to sleep if necessary. Signed-off-by: Steven Price --- drivers/gpu/drm/scheduler/sched_main.c

Re: blocking ops in drm_sched_cleanup_jobs()

2019-09-25 Thread Steven Price
On 25/09/2019 15:12, Koenig, Christian wrote: > Am 25.09.19 um 16:06 schrieb Steven Price: >> On 24/09/2019 10:55, Koenig, Christian wrote: >>> Sorry for the delayed response, have been busy on other stuff last week. >>> >>> Am 17.09.19 um 14:46 schrieb St

Re: blocking ops in drm_sched_cleanup_jobs()

2019-09-25 Thread Steven Price
On 24/09/2019 10:55, Koenig, Christian wrote: > Sorry for the delayed response, have been busy on other stuff last week. > > Am 17.09.19 um 14:46 schrieb Steven Price: >> On 17/09/2019 09:42, Koenig, Christian wrote: >>> Hi Steven, >>> >>> thought about t

Re: [PATCH v2] drm/panfrost: Reduce the amount of logs on deferred probe

2019-09-25 Thread Steven Price
ned-off-by: Krzysztof Kozlowski Reviewed-by: Steven Price > > --- > > Changes since v1: > 1. Remove second error message from calling panfrost_regulator_init(). > --- > drivers/gpu/drm/panfrost/panfrost_device.c | 8 > 1 file changed, 4 insertions(+), 4 d

Re: [PATCH 1/2] drm/panfrost: Allow passing extra information about BOs used by a job

2019-09-23 Thread Steven Price
On 17/09/2019 08:15, Boris Brezillon wrote: > On Mon, 16 Sep 2019 17:20:28 -0500 > Rob Herring wrote: > >> On Fri, Sep 13, 2019 at 6:17 AM Boris Brezillon >> wrote: >>> >>> The READ/WRITE flags are particularly useful if we want to avoid >>> serialization of jobs that read from the same BO but

Re: [PATCH v2 04/11] drm/shmem: drop VM_IO

2019-09-23 Thread Steven Price
On 17/09/2019 10:23, Gerd Hoffmann wrote: > VM_IO is wrong here, shmem uses normal ram not io memory. > > Signed-off-by: Gerd Hoffmann Reviewed-by: Steven Price > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > &

Re: [PATCH v2 03/11] drm/shmem: drop VM_DONTDUMP

2019-09-23 Thread Steven Price
. shmem gem objects surely don't need that ... > > Signed-off-by: Gerd Hoffmann Reviewed-by: Steven Price > --- > drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_gem_sh

Re: [PATCH v2 02/11] drm/shmem: switch shmem helper to _gem_object_funcs.mmap

2019-09-23 Thread Steven Price
On 17/09/2019 10:23, Gerd Hoffmann wrote: > Switch gem shmem helper to the new mmap() workflow, > from _driver.fops.mmap to _gem_object_funcs.mmap. > > v2: Fix vm_flags and vm_page_prot handling. > > Signed-off-by: Gerd Hoffmann Reviewed-by: Steven Price &g

Re: blocking ops in drm_sched_cleanup_jobs()

2019-09-17 Thread Steven Price
Regards, Christian. Am 13.09.19 um 16:50 schrieb Steven Price: Hi, I hit the below splat randomly with panfrost. From what I can tell this is a more general issue which would affect other drivers. 8<- [58604.913130] [ cut here ] [58604.918590] WARNING: CPU: 1

Re: [PATCH] drm/panfrost: Prevent race when handling page fault

2019-09-17 Thread Steven Price
On Thu, Sep 05, 2019 at 01:11:41PM +0100, Steven Price wrote: When handling a GPU page fault addr_to_drm_mm_node() is used to translate the GPU address to a buffer object. However it is possible for the buffer object to be freed after the function has returned resulting in a use-after-free

[PATCH v2] drm/panfrost: Prevent race when handling page fault

2019-09-13 Thread Steven Price
the panfrost_gem_object with an extra reference on it, preventing the BO from being freed until after the page fault has been handled. Signed-off-by: Steven Price --- Changes since v1: * Hold the mm_lock around drm_mm_for_each_node() I've also posted a new IGT test for this: https://patchwork.freedesktop.org/patch

blocking ops in drm_sched_cleanup_jobs()

2019-09-13 Thread Steven Price
Hi, I hit the below splat randomly with panfrost. From what I can tell this is a more general issue which would affect other drivers. 8<- [58604.913130] [ cut here ] [58604.918590] WARNING: CPU: 1 PID: 1758 at kernel/sched/core.c:6556 __might_sleep+0x74/0x98

Re: [PATCH 2/8] drm/shmem: switch shmem helper to _gem_object_funcs.mmap

2019-09-13 Thread Steven Price
On 13/09/2019 13:29, Gerd Hoffmann wrote: > Switch gem shmem helper to the new mmap() workflow, > from _driver.fops.mmap to _gem_object_funcs.mmap. > > Signed-off-by: Gerd Hoffmann > --- > include/drm/drm_gem_shmem_helper.h | 6 ++ > drivers/gpu/drm/drm_gem_shmem_helper.c | 26

Re: [PATCH 2/2] drm/panfrost: Extend the bo_wait() ioctl

2019-09-13 Thread Steven Price
On 13/09/2019 12:17, Boris Brezillon wrote: > So we can choose to wait for all BO users, or just for writers. > > Signed-off-by: Boris Brezillon Looks good to me: Reviewed-by: Steven Price Although I don't know if the term "writers" should be in the API or if "exc

Re: [PATCH 1/2] drm/panfrost: Allow passing extra information about BOs used by a job

2019-09-13 Thread Steven Price
On 13/09/2019 12:17, Boris Brezillon wrote: > The READ/WRITE flags are particularly useful if we want to avoid > serialization of jobs that read from the same BO but never write to it. > The NO_IMPLICIT_FENCE might be useful when the user knows the BO is > shared but jobs are using different

Re: [PATCH] drm/panfrost: Prevent race when handling page fault

2019-09-13 Thread Steven Price
On 07/09/2019 20:36, Daniel Vetter wrote: > On Fri, Sep 6, 2019 at 2:42 PM Steven Price wrote: >> >> On 06/09/2019 12:10, Rob Herring wrote: >>> On Thu, Sep 5, 2019 at 1:11 PM Steven Price wrote: >>>> >>>> When handling a GPU page fault addr_to

[PATCH 2/2] drm/panfrost: Simplify devfreq utilisation tracking

2019-09-12 Thread Steven Price
for being able to submit multiple jobs per slot which requires more values than the original boolean per slot. Signed-off-by: Steven Price --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 64 - drivers/gpu/drm/panfrost/panfrost_devfreq.h | 3 +- drivers/gpu/drm/panfrost

[PATCH 0/2] drm/panfrost: Tidy up the devfreq implementation

2019-09-12 Thread Steven Price
52282163dfa6 and e21dd290881b which would otherwise need reverting, see the previous discussion[2]. [1] https://lore.kernel.org/lkml/20190904123032.23263-1-broo...@kernel.org/ [2] https://lore.kernel.org/lkml/ccd81530-2dbd-3c02-ca0a-1085b0066...@arm.com/ Steven Price (2): drm/panfrost: Use generic code

[PATCH 1/2] drm/panfrost: Use generic code for devfreq

2019-09-12 Thread Steven Price
Use dev_pm_opp_set_rate() instead of open coding the devfreq integration, simplifying the code. Signed-off-by: Steven Price --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 62 - drivers/gpu/drm/panfrost/panfrost_device.h | 2 - 2 files changed, 10 insertions(+), 54

Re: [RESEND PATCH] drm/panfrost: Reduce the amount of logs on deferred probe

2019-09-12 Thread Steven Price
On 09/09/2019 16:51, Krzysztof Kozlowski wrote: > There is no point to print deferred probe (and its failures to get > resources) as an error. > > In case of multiple probe tries this would pollute the dmesg. > > Signed-off-by: Krzysztof Kozlowski Looks like a good idea, however from what I

Re: [PATCH] drm/panfrost: Fix regulator_get_optional() misuse

2019-09-09 Thread Steven Price
On 09/09/2019 16:41, Rob Herring wrote: > On Fri, Sep 6, 2019 at 4:23 PM Steven Price wrote: >> >> On 04/09/2019 13:30, Mark Brown wrote: >>> The panfrost driver requests a supply using regulator_get_optional() >>> but both the name of the supply and the usage pat

Re: [PATCH] drm/panfrost: Fix regulator_get_optional() misuse

2019-09-06 Thread Steven Price
for function, there is no meaningful handling for absent > supplies. Such regulators should use the vanilla regulator_get() > interface, it will ensure that even if a supply is not described in the > system integration one will be provided in software. > > Signed-off-by: Mark Brown Test

Re: [PATCH] drm/panfrost: Fix regulator_get_optional() misuse

2019-09-06 Thread Steven Price
On 06/09/2019 11:55, Mark Brown wrote: [...] >>> However you're probably better off hiding all this stuff with the >>> generic OPP code rather than open coding it - this already has much >>> better handling for this, it supports voltage ranges rather than single >>> voltages and optional

Re: [PATCH] drm/panfrost: Prevent race when handling page fault

2019-09-06 Thread Steven Price
On 06/09/2019 12:10, Rob Herring wrote: > On Thu, Sep 5, 2019 at 1:11 PM Steven Price wrote: >> >> When handling a GPU page fault addr_to_drm_mm_node() is used to >> translate the GPU address to a buffer object. However it is possible for >> the buffer object to be f

Re: [PATCH] drm/panfrost: Fix regulator_get_optional() misuse

2019-09-06 Thread Steven Price
(+CC Rob - I'm not sure why he was dropped) On 05/09/2019 17:34, Mark Brown wrote: > On Thu, Sep 05, 2019 at 02:02:38PM +0100, Steven Price wrote: >> On 05/09/2019 13:40, Mark Brown wrote: > >>> Is that safe? You can't rely on being able to change voltages even if &

Re: [PATCH] drm/panfrost: Fix regulator_get_optional() misuse

2019-09-05 Thread Steven Price
On 05/09/2019 13:40, Mark Brown wrote: > On Thu, Sep 05, 2019 at 10:37:53AM +0100, Steven Price wrote: > >> Ah, I didn't realise that regulator_get() will return a dummy regulator >> if none is provided in the DT. In theory that seems like a nicer >> solution to my two

[PATCH] drm/panfrost: Prevent race when handling page fault

2019-09-05 Thread Steven Price
the panfrost_gem_object with an extra reference on it, preventing the BO from being freed until after the page fault has been handled. Signed-off-by: Steven Price --- I've managed to trigger this, generating the following stack trace. Unable to handle kernel NULL pointer dereference at virtual address 0090 pgd

Re: [PATCH] drm/panfrost: Fix regulator_get_optional() misuse

2019-09-05 Thread Steven Price
On 05/09/2019 09:21, Rob Herring wrote: > +Steven > > On Wed, Sep 4, 2019 at 1:30 PM Mark Brown wrote: >> >> The panfrost driver requests a supply using regulator_get_optional() >> but both the name of the supply and the usage pattern suggest that it is >> being used for the main power for the

Re: [PATCH v3 6/8] drm/panfrost: Add cache/TLB flush before switching address space

2019-08-28 Thread Steven Price
On 26/08/2019 23:33, Rob Herring wrote: > It's not entirely clear if this is required, but add a flush of GPU caches > and TLBs before we change an address space to new page tables. > > Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces") > Cc: Tomeu V

Re: [PATCH v3 7/8] drm/panfrost: Flush and disable address space when freeing page tables

2019-08-28 Thread Steven Price
On 28/08/2019 13:35, Rob Herring wrote: > On Wed, Aug 28, 2019 at 5:55 AM Steven Price wrote: >> >> On 26/08/2019 23:33, Rob Herring wrote: >>> Currently, page tables are freed without disabling the address space first. >>> This probably is fine as we'll switch to

Re: [PATCH v3 8/8] drm/panfrost: Remove unnecessary hwaccess_lock spin_lock

2019-08-28 Thread Steven Price
y serialized by drm_sched. > > Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces") > Cc: Tomeu Vizoso > Cc: Steven Price > Cc: Alyssa Rosenzweig > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Rob Herring Reviewed-by: St

Re: [PATCH v3 7/8] drm/panfrost: Flush and disable address space when freeing page tables

2019-08-28 Thread Steven Price
les if we are not suspended. As > the tlb_inv_context() hook is only called when freeing the page tables and > we do a flush before disabling the AS, lets remove the flush from > tlb_inv_context and avoid any runtime PM issues. > > Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD ad

Re: [PATCH v3 5/8] drm/panfrost: Split mmu_hw_do_operation into locked and unlocked version

2019-08-28 Thread Steven Price
On 26/08/2019 23:33, Rob Herring wrote: > In preparation to call mmu_hw_do_operation with the as_lock already held, > Add a mmu_hw_do_operation_locked function. > > Fixes: 7282f7645d06 ("drm/panfrost: Implement per FD address spaces") > Cc: Tomeu Vizoso > Cc

Re: [PATCH v3 4/8] drm/panfrost: Rework page table flushing and runtime PM interaction

2019-08-28 Thread Steven Price
ng any locks associated with resuming which trigger > lockdep warnings. > > Fixes: 013b65101315 ("drm/panfrost: Add madvise and shrinker support") > Cc: Tomeu Vizoso > Cc: Steven Price > Cc: Alyssa Rosenzweig > Cc: David Airlie > Cc: Daniel Vetter > Signe

Re: [PATCH v3 3/8] drm/panfrost: Remove unnecessary mmu->lock mutex

2019-08-28 Thread Steven Price
; Suggested-by: Robin Murphy > Cc: Tomeu Vizoso > Cc: Steven Price > Cc: Alyssa Rosenzweig > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Rob Herring Reviewed-by: Steven Price Steve > --- > v3: > - new patch > > drivers/gpu/drm/panfrost/panfr

Re: [PATCH v3 2/8] drm/panfrost: Hold runtime PM reference until jobs complete

2019-08-28 Thread Steven Price
ot;drm/panfrost: Add initial panfrost driver") > Cc: Tomeu Vizoso > Cc: Steven Price > Cc: Alyssa Rosenzweig > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Rob Herring Reviewed-by: Steven Price Steve > --- > v3: > - Fix race between clearing pfdev-

Re: [PATCH v3 1/8] drm/panfrost: Rework runtime PM initialization

2019-08-28 Thread Steven Price
untime PM calls into the probe() function so they are > all in one place and are done after all initialization. > > Cc: Tomeu Vizoso > Cc: Steven Price > Cc: David Airlie > Cc: Daniel Vetter > Acked-by: Alyssa Rosenzweig > Signed-off-by: Rob Herring Reviewed-by: Steven

Re: [PATCH v2 8/8] drm/panfrost: Remove unnecessary flushing from tlb_inv_context

2019-08-23 Thread Steven Price
Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring Reviewed-by: Steven Price Although it might be worth trying to capture the justification about this in a comment somewhere - there's been a fair bit of discussion about this... Steve --- v2: new

Re: [PATCH v2 7/8] drm/panfrost: Rework page table flushing and runtime PM interaction

2019-08-23 Thread Steven Price
associated with resuming. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring Reviewed-by: Steven Price But one comment below... --- v2: new patch drivers/gpu/drm/panfrost/panfrost_mmu.c | 41 - 1 file

Re: [PATCH v2 7/8] drm/panfrost: Rework page table flushing and runtime PM interaction

2019-08-23 Thread Steven Price
awake. This avoids taking any locks associated with resuming. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring --- v2: new patch   drivers/gpu/drm/panfrost/panfrost_mmu.c | 41 -   1 file changed, 20

Re: [PATCH v2 6/8] drm/panfrost: Use mutex_trylock in panfrost_gem_purge

2019-08-23 Thread Steven Price
() instead and bail if a BO is locked already. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring Reviewed-by: Steven Price --- v2: new patch drivers/gpu/drm/panfrost/panfrost_gem_shrinker.c | 11 +++ 1 file changed

Re: [PATCH v2 5/8] drm/shmem: Use mutex_trylock in drm_gem_shmem_purge

2019-08-23 Thread Steven Price
ready given my R-b for this one, but just in case: Reviewed-by: Steven Price Steve --- drivers/gpu/drm/drm_gem_shmem_helper.c | 7 +-- include/drm/drm_gem_shmem_helper.h | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers

Re: [PATCH v2 3/8] drm/panfrost: Hold runtime PM reference until jobs complete

2019-08-23 Thread Steven Price
On 23/08/2019 03:12, Rob Herring wrote: Doing a pm_runtime_put as soon as a job is submitted is wrong as it should not happen until the job completes. It works because we are relying on the autosuspend timeout to keep the h/w enabled. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig

Re: [PATCH v2 1/8] drm/panfrost: Fix possible suspend in panfrost_remove

2019-08-23 Thread Steven Price
the pm_runtime_put_sync_suspend() after the panfrost_device_fini() call. Cc: Tomeu Vizoso Cc: Steven Price Cc: Alyssa Rosenzweig Cc: David Airlie Cc: Daniel Vetter Signed-off-by: Rob Herring Reviewed-by: Steven Price --- v2: new patch drivers/gpu/drm/panfrost/panfrost_drv.c | 6 -- 1

Re: [PATCH] MAINTAINERS: Add Steven and Alyssa as panfrost reviewers

2019-08-23 Thread Steven Price
On 23/08/2019 02:33, Rob Herring wrote: > Add Steven Price and Alyssa Rosenzweig as reviewers as they have been the > primary reviewers already. > > Cc: Steven Price > Cc: Alyssa Rosenzweig > Cc: Tomeu Vizoso > Signed-off-by: Rob Herring Acked-by: Steven Price Steve

Re: [PATCH] drm/panfrost: Add missing check for pfdev->regulator

2019-08-23 Thread Steven Price
On 23/08/2019 02:52, Rob Herring wrote: > On Thu, Aug 22, 2019 at 4:32 AM Steven Price wrote: >> >> When modifying panfrost_devfreq_target() to support a device without a >> regulator defined I missed the check on the error path. Let's add it. >> >> Rep

Re: [PATCH 2/4] drm/shmem: Use mutex_trylock in drm_gem_shmem_purge

2019-08-23 Thread Steven Price
f31efa81 (>shrinker_lock){+.+.}, at: > panfrost_gem_shrinker_scan+0x34/0x180 [panfrost] > > Fixes: 17acb9f35ed7 ("drm/shmem: Add madvise state and purge helpers") > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie >

Re: [PATCH 4/4] drm/panfrost: Fix sleeping while atomic in panfrost_gem_open

2019-08-23 Thread Steven Price
arm64_sys_ioctl+0x1c/0x28 > el0_svc_common.constprop.0+0x90/0x168 > el0_svc_handler+0x28/0x78 > el0_svc+0x8/0xc > > Fixes: 68337d0b8644 ("drm/panfrost: Restructure the GEM object creation") > Cc: Tomeu Vizoso > Cc: David Airlie > Cc: Daniel Vetter > Sign

Re: [PATCH 3/4] drm/panfrost: Fix shrinker lockdep issues using drm_gem_shmem_purge()

2019-08-23 Thread Steven Price
On 19/08/2019 17:12, Rob Herring wrote: > This fixes 2 issues found by lockdep. First, drm_gem_shmem_purge() > now uses mutex_trylock for the pages_lock to avoid a circular > dependency. NIT: This is in the previous patch. > Second, it drops the call to panfrost_mmu_unmap() which takes several >

Re: [PATCH 1/4] drm/shmem: Do dma_unmap_sg before purging pages

2019-08-23 Thread Steven Price
/shmem: Add madvise state and purge helpers") > Cc: Maarten Lankhorst > Cc: Maxime Ripard > Cc: Sean Paul > Cc: David Airlie > Cc: Daniel Vetter > Signed-off-by: Rob Herring Looks good to me: Reviewed-by: Steven Price > --- > drivers/gpu/drm/drm_gem_s

[PATCH] drm/panfrost: Add missing check for pfdev->regulator

2019-08-22 Thread Steven Price
When modifying panfrost_devfreq_target() to support a device without a regulator defined I missed the check on the error path. Let's add it. Reported-by: Dan Carpenter Fixes: e21dd290881b ("drm/panfrost: Enable devfreq to work without regulator") Signed-off-by: Steven Price --- drive

Re: [PATCH] drm/panfrost: Queue jobs on the hardware

2019-08-21 Thread Steven Price
On 20/08/2019 06:23, Tomeu Vizoso wrote: > On 8/16/19 11:31 AM, Steven Price wrote: >> The hardware has a set of '_NEXT' registers that can hold a second job >> while the first is executing. Make use of these registers to enqueue a >> second job per slot. > > I like

Re: [PATCH] drm/panfrost: Queue jobs on the hardware

2019-08-21 Thread Steven Price
On 19/08/2019 18:02, Rob Herring wrote: > On Mon, Aug 19, 2019 at 11:58 AM Rob Herring wrote: >> >> On Fri, Aug 16, 2019 at 4:31 AM Steven Price wrote: >>> >>> The hardware has a set of '_NEXT' registers that can hold a second job >>> while the first

[PATCH] drm/panfrost: Remove opp table when unloading

2019-08-16 Thread Steven Price
The devfreq opp table needs to be removed when unloading the driver to free the memory associated with it. Signed-off-by: Steven Price --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 6 ++ drivers/gpu/drm/panfrost/panfrost_devfreq.h | 1 + drivers/gpu/drm/panfrost/panfrost_drv.c | 5

[PATCH] drm/panfrost: Queue jobs on the hardware

2019-08-16 Thread Steven Price
The hardware has a set of '_NEXT' registers that can hold a second job while the first is executing. Make use of these registers to enqueue a second job per slot. Signed-off-by: Steven Price --- Note that this is based on top of Rob Herring's "per FD address space" patch[1].

[PATCH] drm/panfrost: Enable devfreq to work without regulator

2019-08-16 Thread Steven Price
the voltage. This patch allows frequency control of the GPU on this system. Signed-off-by: Steven Price --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost

Re: [PATCH -next] drm/panfrost: Fix missing unlock on error in panfrost_mmu_map_fault_addr()

2019-08-16 Thread Steven Price
un Well spotted. Reviewed-by: Steven Price Steve > --- > drivers/gpu/drm/panfrost/panfrost_mmu.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_mmu.c > b/drivers/gpu/drm/panfrost/panfrost_mmu.c > index 2ed411f0

Re: [PATCH v2] drm/panfrost: Implement per FD address spaces

2019-08-15 Thread Steven Price
ist and switch the AS > from the old FD to the new one. > > Cc: Tomeu Vizoso > Cc: David Airlie > Cc: Daniel Vetter > Cc: Robin Murphy > Cc: Steven Price > Cc: Alyssa Rosenzweig > Signed-off-by: Rob Herring Reviewed-by: Steven Price Steve > --- > v2: &g

Re: [PATCH] drm/panfrost: Add errata descriptions from kbase

2019-08-15 Thread Steven Price
On 09/08/2019 22:09, Alyssa Rosenzweig wrote: > While newer kbase include only the numbers of errata, older kbase > releases included one-line descriptions for each errata, which is useful > for those working on the driver. Import these descriptions. Most are > from kbase verbatim; a few I edited

Re: [PATCH] drm/panfrost: Implement per FD address spaces

2019-08-10 Thread Steven Price
On 09/08/2019 04:01, Rob Herring wrote: [...] > I was worried too. It seems to be working pretty well though, but more > testing would be good. I don't think there are a lot of usecases that > use more AS than the h/w has (8 on T860), but I'm not sure. Yeah, 8 is overkill. Some GPUs only have 4

Re: [PATCH v4 8/9] drm/panfrost: Add support for GPU heap allocations

2019-08-10 Thread Steven Price
me in order to utilize huge pages (if > enabled). Currently, once we've mapped pages in, they are only unmapped > if the BO is freed. Once we add shrinker support, we can unmap pages > with the shrinker. > > Cc: Tomeu Vizoso > Cc: Boris Brezillon > Cc: Robin Murphy > Cc:

Re: [PATCH] drm/panfrost: Implement per FD address spaces

2019-08-10 Thread Steven Price
> > Cc: Tomeu Vizoso > Cc: David Airlie > Cc: Daniel Vetter > Cc: Robin Murphy > Cc: Steven Price > Cc: Alyssa Rosenzweig > Signed-off-by: Rob Herring > --- > This depends on madvise support (now in drm-misc) and the heap/no-exec > series (just the rework).

Re: [PATCH v4 6/9] drm/panfrost: Consolidate reset handling

2019-08-10 Thread Steven Price
ork its call. In the process, we > hide the address space details within the MMU code in preparation to > support multiple address spaces. > > Cc: Tomeu Vizoso > Cc: David Airlie > Cc: Daniel Vetter > Cc: Robin Murphy > Cc: Steven Price > Cc: Alyssa Rosenzweig > Signed

Re: [PATCH v4 3/9] drm/panfrost: Restructure the GEM object creation

2019-08-10 Thread Steven Price
Boris Brezillon > Cc: Robin Murphy > Reviewed-by: Steven Price > Acked-by: Alyssa Rosenzweig > Signed-off-by: Rob Herring > --- > Steven, Alyssa, I kept your tags, but please take another look as things > moved around a bit here. Sadly this doesn't compile (bisection is broken

Re: [PATCH v3 4/8] drm/panfrost: Split panfrost_mmu_map SG list mapping to its own function

2019-08-09 Thread Steven Price
On 02/08/2019 20:51, Rob Herring wrote: > In preparation to create partial GPU mappings of BOs on page faults, > split out the SG list handling of panfrost_mmu_map(). > > Cc: Tomeu Vizoso > Cc: Boris Brezillon > Cc: Robin Murphy > Cc: Steven Price > Acked-by: Alyssa

Re: [PATCH v3 7/8] drm/panfrost: Add support for GPU heap allocations

2019-08-09 Thread Steven Price
me in order to utilize huge pages (if > enabled). Currently, once we've mapped pages in, they are only unmapped > if the BO is freed. Once we add shrinker support, we can unmap pages > with the shrinker. > > Cc: Tomeu Vizoso > Cc: Boris Brezillon > Cc: Robin Murphy > Cc:

Re: [PATCH v3 6/8] drm/panfrost: Convert MMU IRQ handler to threaded handler

2019-08-09 Thread Steven Price
c: Tomeu Vizoso > Cc: Boris Brezillon > Cc: Robin Murphy > Cc: Steven Price > Cc: Alyssa Rosenzweig > Signed-off-by: Rob Herring Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_mmu.c | 20 +++- > 1 file changed, 15 insertions(+),

<    2   3   4   5   6   7   8   >