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

drivers: bcm2835_isp: Allow multiple users for the ISP driver. #4709

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

naushir
Copy link
Contributor

@naushir naushir commented Nov 18, 2021

Add a second (identical) set of device nodes to allow concurrent use of the ISP
hardware by another user. This change effectively creates a second state
structure (struct bcm2835_isp_dev) to maintain independent state for the second
user. Node and media entity names are appened with the instance index
appropriately.

Further users can be added by changing the BCM2835_ISP_NUM_INSTANCES define.

Signed-off-by: Naushir Patuck naush@raspberrypi.com

@naushir
Copy link
Contributor Author

naushir commented Nov 18, 2021

@6by9 and @davidplowman
Here are the changes need to allow concurrent usage for the ISP.

Copy link
Contributor

@6by9 6by9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few minor comments mainly around error handling and cleanup.

@naushir naushir marked this pull request as draft November 23, 2021 09:19
@naushir naushir marked this pull request as ready for review November 23, 2021 09:27
@naushir
Copy link
Contributor Author

naushir commented Nov 23, 2021

Small update to the node/entity names - we do not need to index over multiple instances any more.

@naushir
Copy link
Contributor Author

naushir commented Nov 23, 2021

@6by9 and @pelwell this should be ready to review and merge now.

@pelwell
Copy link
Contributor

pelwell commented Nov 23, 2021

It feels wrong that it is necessary to create separate instances like this to support concurrency, but I don't know the intricacies of the V4L2 world and you wouldn't be proposing this if it wasn't needed.

This implementation is much cleaner than the first version, so LGTM.

@naushir
Copy link
Contributor Author

naushir commented Nov 23, 2021

It feels wrong that it is necessary to create separate instances like this to support concurrency, but I don't know the intricacies of the V4L2 world and you wouldn't be proposing this if it wasn't needed.

Unfortunately that's how V4L2 works with concurrency, we cannot avoid duplicating the device nodes.

@6by9
Copy link
Contributor

6by9 commented Nov 23, 2021

It feels wrong that it is necessary to create separate instances like this to support concurrency, but I don't know the intricacies of the V4L2 world and you wouldn't be proposing this if it wasn't needed.

Annoyingly a single V4L2 node can only support one queue of each type (OUTPUT, CAPTURE, etc).
A simple memory to memory device such as a codec (1 input, 1 output) can be exposed as a single node, and each time you open the device you get a new instance.
With the ISP where we have 1 input but 3 output (high res, low res, and stats) we can't use a single node, and if split over multiple nodes then we can't support concurrent usage as you can't tell which instance each request or buffer is associated with.

So yes, ugly as heck, but no way around it.

Add a second (identical) set of device nodes to allow concurrent use of the ISP
hardware by another user. This change effectively creates a second state
structure (struct bcm2835_isp_dev) to maintain independent state for the second
user. Node and media entity names are appened with the instance index
appropriately.

Further users can be added by changing the BCM2835_ISP_NUM_INSTANCES define.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
@6by9
Copy link
Contributor

6by9 commented Nov 23, 2021

Ta, LGTM now.

@pelwell pelwell merged commit c293474 into raspberrypi:rpi-5.10.y Nov 23, 2021
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Nov 26, 2021
See: raspberrypi/linux#4736

kernel: vc4-kms: Rework HPD
See: raspberrypi/linux#4716

kernel: drm/vc4: Add support for composite syncs to vc4_dpi
See: raspberrypi/linux#4733

kernel: Pass V4L2_CID_MPEG_VIDEO_H264_MIN_QP/MAX_QP to bcm2835-v4l2-codec
See: raspberrypi/linux#4705

kernel: media: i2c: ov5647: Support HFLIP and VFLIP
See: raspberrypi/linux#4731

kernel: drivers: bcm2835_isp: Fix div by 0 bug
See: raspberrypi/linux#4732

kernel: drivers: bcm2835_isp: Allow multiple users for the ISP driver.
See: raspberrypi/linux#4709

kernel: ARM: dts: Update rpi-400 and cm4 dts to match 4-b

kernel: ARM: dts: gpio-ranges property is now required
See: https://forums.raspberrypi.com/viewtopic.php?t=324585

kernel: ARM: dts: bcm2711: Fix PCIe interrupts
See: raspberrypi/linux#4666
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request Nov 26, 2021
See: raspberrypi/linux#4736

kernel: vc4-kms: Rework HPD
See: raspberrypi/linux#4716

kernel: drm/vc4: Add support for composite syncs to vc4_dpi
See: raspberrypi/linux#4733

kernel: Pass V4L2_CID_MPEG_VIDEO_H264_MIN_QP/MAX_QP to bcm2835-v4l2-codec
See: raspberrypi/linux#4705

kernel: media: i2c: ov5647: Support HFLIP and VFLIP
See: raspberrypi/linux#4731

kernel: drivers: bcm2835_isp: Fix div by 0 bug
See: raspberrypi/linux#4732

kernel: drivers: bcm2835_isp: Allow multiple users for the ISP driver.
See: raspberrypi/linux#4709

kernel: ARM: dts: Update rpi-400 and cm4 dts to match 4-b

kernel: ARM: dts: gpio-ranges property is now required
See: https://forums.raspberrypi.com/viewtopic.php?t=324585

kernel: ARM: dts: bcm2711: Fix PCIe interrupts
See: raspberrypi/linux#4666
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.

None yet

3 participants