Pispbe/rpi 6.6.y/split jobs handling v8 #6905
Conversation
|
@njhollinghurst FYI |
|
Should this be targeting 6.12 instead? |
|
Yes, it should be 6.12 not 6.6. I've tagged the patches on linux-media, so if Nick is happy with these changes, we can merge this series. |
|
I think it's OK but I'll need to take a little time getting back up to speed on this.
(Edit) Ah, but now there is a |
|
LGTM |
Commit jmondi@1f2322f shouldn't introduce any functional difference. It simply make use of the runtime_pm helpers to properly balance its usage count, but in the end the resume/suspend function that gets called are the same ones that were called explicitly by the driver ?
Yes, we copy in @naushir should I push another branch for v6.12 ? |
Yes please, or simply update this PR to point to 6.12. We don't want to update 6.6 any more. |
|
Actually I seem to have the magic powers to update the base, so let me do that and see if there are any conflicts. |
|
@jmondi annoyingly I think you are going to have to do this manually with a new PR. |
A comment in the pisp_be driver references the pispbe_schedule_internal() function which doesn't exist. Drop it. Reviewed-by: Laurent Pinchart <[email protected]> Reviewed-by: Naushir Patuck <[email protected]> Signed-off-by: Jacopo Mondi <[email protected]>
The config parameters buffer is already validated in pisp_be_validate_config() at .buf_prepare() time. However some of the same validations are also performed at pispbe_schedule() time. In particular the function checks that: 1) config.num_tiles is valid 2) At least one of the BAYER or RGB input is enabled The input config validation is already performed in pisp_be_validate_config() and while job.hw_enables is modified by pispbe_xlate_addrs(), the function only resets the input masks if - there is no input buffer available, but pispbe_prepare_job() fails before calling pispbe_xlate_addrs() in this case - bayer_enable is 0, but in this case rgb_enable is valid as guaranteed by pisp_be_validate_config() - only outputs are reset in rgb_enable For this reasons there is no need to repeat the check at pispbe_schedule() time. The num_tiles validation can be moved to pisp_be_validate_config() as well. As num_tiles is a u32 it can'be be < 0, so change the sanity check accordingly. Reviewed-by: Laurent Pinchart <[email protected]> Reviewed-by: Naushir Patuck <[email protected]> Signed-off-by: Jacopo Mondi <[email protected]>
Currently the 'pispbe_schedule()' function does two things: 1) Tries to assemble a job by inspecting all the video node queues to make sure all the required buffers are available 2) Submit the job to the hardware The pispbe_schedule() function is called at: - video device start_streaming() time - video device qbuf() time - irq handler As assembling a job requires inspecting all queues, it is a rather time consuming operation which is better not run in IRQ context. To avoid executing the time consuming job creation in interrupt context split the job creation and job scheduling in two distinct operations. When a well-formed job is created, append it to the newly introduced 'pispbe->job_queue' where it will be dequeued from by the scheduling routine. As the per-node 'ready_queue' buffer list is only accessed in vb2 ops callbacks, protected by the node->queue_lock mutex, it is not necessary to guard it with a dedicated spinlock so drop it. Also use the spin_lock_irq() variant in all functions not called from an IRQ context where the spin_lock_irqsave() version was used. Reviewed-by: Laurent Pinchart <[email protected]> Signed-off-by: Jacopo Mondi <[email protected]>
During the probe() routine, the PiSP BE driver needs to power up the interface in order to identify and initialize the hardware. The driver resumes the interface by calling the pispbe_runtime_resume() function directly, without going through the pm_runtime helpers, but later suspends it by calling pm_runtime_put_autosuspend(). This causes a PM usage count imbalance at probe time, notified by the runtime_pm framework with the below message in the system log: pispbe 1000880000.pisp_be: Runtime PM usage count underflow! Fix this by resuming the interface using the pm runtime helpers instead of calling the resume function directly and use the pm_runtime framework in the probe() error path. While at it, remove manual suspend of the interface in the remove() function. The driver cannot be unloaded if in use, so simply disable runtime pm. To simplify the implementation, make the driver depend on PM as the RPI5 platform where the ISP is integrated in uses the PM framework by default. Fixes: 12187bd ("media: raspberrypi: Add support for PiSP BE") Cc: [email protected] Reviewed-by: Laurent Pinchart <[email protected]> Signed-off-by: Jacopo Mondi <[email protected]>
pelwell
force-pushed
the
pispbe/rpi-6.6.y/split-jobs-handling-v8
branch
from
1f2322f to
d4c4662
Compare
|
I've tried to patch it up (updating @jmondi's 6.6 branch to point to 6.12 in the process). Take a look to check I've not broken anything - there was a minor merge conflict with the Kconfig change. |
I've locally rebased on v6.12 and the only conflict I had was in Kconfig indeed. |
And I've now compared my local changes with the updated PR content and there's nothing there, so it's all good here |
Version of https://gitlab.freedesktop.org/linux-media/users/jmondi/-/commits/b4/pispbe-mainline-split-jobs-handling-v6 rebased on rpi-6.6.y
This series includes a few cleanups and splits jobs creation and scheduling to reduce the time spent in irq context.
This set of changes will be required for proper supporting multi-context operations without registering multiple media graphs to userspace
UPDATE: Now changed to target rpi-6.12.y (@pelwell)