Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync Panthor with drm-tip #322

Closed
wants to merge 13 commits into from
Closed

Conversation

ginkage
Copy link
Collaborator

@ginkage ginkage commented Jan 28, 2025

Yet another round, this time bumping uAPI to 1.3.

bbrezillon and others added 13 commits January 28, 2025 13:40
WARN() will return true if the condition is true, false otherwise.
If we store the return of drm_WARN_ON() in ret, we lose the actual
error code.

v3:
- Add R-b
v2:
- Add R-b

Fixes: 5fe909c ("drm/panthor: Add the device logical block")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Reviewed-by: Adrian Larumbe <adrian.larumbe@collabora.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241211075419.2333731-2-boris.brezillon@collabora.com
…end path

The runtime PM resume operation is not guaranteed to succeed, but if it
fails, the device should be in a suspended state. Make sure we're robust
to resume failures in the unplug path.

v3:
- Fix typo
- Add R-bs

v2:
- Move the bit that belonged in the next commit
- Drop the panthor_device_unplug() changes

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Adrian Larumbe <adrian.larumbe@collabora.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241211075419.2333731-3-boris.brezillon@collabora.com
devfreq_{resume,suspend}_device() don't bother undoing the suspend_count
modifications if something fails, so either it assumes failures are
harmless, or it's super fragile/buggy. In either case it's not something
we can address at the driver level, so let's just assume failures are
harmless for now, like is done in panfrost.

v3:
- Add R-b

v2:
- Add R-b

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Reviewed-by: Adrian Larumbe <adrian.larumbe@collabora.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241211075419.2333731-4-boris.brezillon@collabora.com
When the runtime PM resume callback returns an error, it puts the device
in a state where it can't be resumed anymore. Make sure we can recover
from such transient failures by calling pm_runtime_set_suspended()
explicitly after a pm_runtime_resume_and_get() failure.

v3:
- Add R-b/A-b

v2:
- Add a comment explaining potential races in
  panthor_device_resume_and_get()

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Adrian Larumbe <adrian.larumbe@collabora.com>
Acked-by: Steven Price <steven.price@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241211075419.2333731-5-boris.brezillon@collabora.com
If we do a GPU soft-reset, that's no longer fast reset. This also means
the slow reset fallback doesn't work because the MCU state is only reset
after a GPU soft-reset.

Let's move the retry logic to panthor_device_resume() to issue a
soft-reset between the fast and slow attempts, and patch
panthor_gpu_suspend() to only power-off the L2 when a fast reset is
requested.

v3:
- No changes

v2:
- Add R-b

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241211075419.2333731-6-boris.brezillon@collabora.com
Groups can be killed during a reset even though they did nothing wrong.
That usually happens when the FW is put in a bad state by other groups,
resulting in group suspension failures when the reset happens.

If we end up in that situation, flag the group innocent and report
innocence through a new DRM_PANTHOR_GROUP_STATE flag.

Bump the minor driver version to reflect the uAPI change.

Changes in v4:
- Add an entry to the driver version changelog
- Add R-bs

Changes in v3:
- Actually report innocence to userspace

Changes in v2:
- New patch

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241211080500.2349505-1-boris.brezillon@collabora.com
Remove unused function declaration in panthor_gem.h:
- `panthor_gem_prime_import_sg_table()`

Remove duplicate macro definitions:
- `MAX_CSG_PRIO`
- `MIN_CS_PER_CSG`
- `MIN_CSGS`

Signed-off-by: Florent Tomasin <florent.tomasin@arm.com>
Reviewed-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Signed-off-by: Steven Price <steven.price@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250107173310.88329-1-florent.tomasin@arm.com
Use the correct format for all kernel-doc comments.
Use structname.membername for named structs.
Don't precede function names in kernel-doc with '@' sign.
Use the correct function parameter names in kernel-doc comments.

This fixes around 80 kernel-doc warnings.

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Cc: Boris Brezillon <boris.brezillon@collabora.com>
Cc: Steven Price <steven.price@arm.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Reviewed-by: Steven Price <steven.price@arm.com>
Signed-off-by: Steven Price <steven.price@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20250111062832.910495-1-rdunlap@infradead.org
If a reset is scheduled when the suspend happens, we drop the
reset-pending info on the floor assuming the resume will fix things,
but the resume logic might try a fast reset. If we're lucky, the
fast reset fails and we fallback to a slow reset, but if the FW was
corrupted in a way that makes it partially functional (it boots but
doesn't quite do what it's expected to do), we won't notice immediately
that things are not working correctly, leading to a new reset further
down the road.

Fixes: 5fe909c ("drm/panthor: Add the device logical block")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Liviu Dudau <liviu.dudau@arm.com>
Reviewed-by: Steven Price <steven.price@arm.com>
Signed-off-by: Steven Price <steven.price@arm.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20241217092457.1582053-1-boris.brezillon@collabora.com
Commit baf4afc5831438 ("drm/sched: Improve teardown documentation")
documents problems of drm_sched_fini() in form of a list. The checklist
triggers htmldocs warning (but renders correctly in htmldocs output):

Documentation/gpu/drm-mm:571: ./drivers/gpu/drm/scheduler/sched_main.c:1359: ERROR: Unexpected indentation.

Separate the list from the preceding paragraph by a blank line to fix
the warning. While at it, also end the aforementioned paragraph by a
colon.

Fixes: baf4afc58314 ("drm/sched: Improve teardown documentation")
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Closes: https://lore.kernel.org/r/20241108175655.6d3fcfb7@canb.auug.org.au/
Signed-off-by: Bagas Sanjaya <bagasdotme@gmail.com>
[phasta: Adjust commit message]
Signed-off-by: Philipp Stanner <phasta@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20241217034915.62594-1-bagasdotme@gmail.com
No driver is using the update_job_credits() schduler vfunc
so lets remove it.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Danilo Krummrich <dakr@redhat.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Philipp Stanner <pstanner@redhat.com>
Acked-by: Danilo Krummrich <dakr@kernel.org>
Acked-by: Boris Brezillon <boris.brezillon@collabora.com>
Acked-by: Matt Coster <matt.coster@imgtec.com>
Signed-off-by: Philipp Stanner <phasta@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20250110111301.76909-1-tvrtko.ursulin@igalia.com
There is no need to check the boolean in the work item's prologues since
the boolean can be set at any later time anyway.

The helper which pauses submission sets it and synchronously cancels the
work and helpers which queue the work check for the flag so all should be
good.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Danilo Krummrich <dakr@redhat.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Philipp Stanner <pstanner@redhat.com>
Signed-off-by: Philipp Stanner <phasta@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20250114105942.64832-1-tvrtko.ursulin@igalia.com
Lets isolate scheduler internals from drivers such as pvr which currently
walks the dependency array to look for fences.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: Danilo Krummrich <dakr@redhat.com>
Cc: Matthew Brost <matthew.brost@intel.com>
Cc: Philipp Stanner <pstanner@redhat.com>
Reviewed-by: Matt Coster <matt.coster@imgtec.com>
Acked-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Philipp Stanner <phasta@kernel.org>
Link: https://patchwork.freedesktop.org/patch/msgid/20250113103341.43914-1-tvrtko.ursulin@igalia.com
@HeyMeco
Copy link
Collaborator

HeyMeco commented Jan 28, 2025

Just applied this on top of rkr5
Edit: I didn't realize you closed the other PR and opened a new one on top rkr5 so I had just manually applied them via git am

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants