-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
VC4 DRM/KMS - use correct dma-ranges #3623
Conversation
vc4_drv isn't necessarily under the /soc node in DT, but it is the one that does the allocations. The DMA addresses are only ever consumed by the HVS, and that requires VideoCore cache alias address mapping, as set up by the dma-ranges under /soc. During probe find the HVS device node and adopt the DMA configuration of that node. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
Under FKMS the firmware also requires the VideoCore cache aliases as defined by the dma-ranges under /soc, so also look for a rpi-firmware-kms node if there is no enabled HVS node. Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
@@ -270,6 +270,9 @@ static int vc4_drm_bind(struct device *dev) | |||
of_node_put(node); | |||
|
|||
hvs_node = of_find_compatible_node(NULL, NULL, "brcm,bcm2835-hvs"); | |||
if (!hvs_node) | |||
hvs_node = of_find_compatible_node(NULL, NULL, | |||
"raspberrypi,rpi-firmware-kms"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You state that the firmware needs access to the cache aliases, so why use the rpi-firmware-kms node rather than using the firmware node? As written, aren't you also relying on the fact that rpi-firmware-kms node is also in /soc, which wasn't a stated assumption?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maxime's comment via email was
Since it's a virtual node, it shouldn't really be put under /soc, however, the
underlying issue is that the DRM device is also virtual, and therefore doesn't
really reflect the DMA capabilities of the devices that compose it.
The basic idea remains though, you have to "inherit" the DMA
configuration of the HVS on the main, virtual, DRM device that will perform all
the allocations, and you can do that using of_dma_configure with a different
device_node pointer. In your case, you'd use it on the drm_device's device, with
the HVS device_node.
I guess taking it further you could have a Pi3 with vc4_drv (brcm,bcm2835-vc4), vc4_v3d (brcm,bcm2835-v3d), and rpi-firmware enabled, but no composition pipeline. That would (correctly?) fail to find dma ranges in the current code, but not if looking for rpi-firmware which isn't a dependency.
Then again Pi0-3 V3D also needs the dma ranges, so perhaps we need to add a list of preferred devices for copying the DMA ranges from. Pass in a list to of_find_matching_node_and_match
and allow it to find our preferred node to copy from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My problem stems from the wording "the firmware also requires the VideoCore cache aliases as defined by the dma-ranges under /soc, so also look for a rpi-firmware-kms node". What you meant (I now realise) was "the firmware display support requires that addresses are passed with the correct VideoCore cache aliases, and the rpi-firmware-kms node is guaranteed to have the correct dma-ranges, so copy that as a fallback", whereas I read it as "the firmware node also has the correct dma-ranges to use VideoCore cache aliases, so copy that as a fallback". I think both approaches are valid, but the commit message could be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I had revised but not pushed based on your comments, but you've now merged.
I've pushed the alternate version (which has reworded commits) to https://github.com/6by9/linux/commits/rpi-5.4.y.
Revert and take the alternate, or keep the current?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rewound rpi-5.4.y, so we can use the updated version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Sorry for not having replied earlier.
See: raspberrypi/linux#3626 kernel: VC4 DRM/KMS - use correct dma-ranges See: raspberrypi/linux#3623 kernel: media: bcm2835-unicam: Retain packing information on G_FMT See: raspberrypi/linux#3622 kernel: Switch to snd_soc_dai_set_bclk_ratio See: raspberrypi/linux#3620 kernel: V4L2 H264 framing fixes See: raspberrypi/linux#3614 kernel: drm/vc4: Fix VIC usage with Broadcast RGB See: raspberrypi/linux#3611 kernel: media: bcm2835-unicam: Always service interrupts See: raspberrypi/linux#3608 kernel: overlays: Fix audio parameter of vc4-kms-v3 See: raspberrypi/linux#2489 kernel: configs: Restore missing NF_TABLES settings See: raspberrypi/linux#3615 kernel: sc16is7xx: Fix for hardware flow control See: raspberrypi/linux#2542 kernel: Use the upstream cpufreq driver on non-BCM2835 Pis See: raspberrypi/linux#3604 kernel: Backport of udmabuf and dma-heaps See: raspberrypi/linux#3571 kernel: imx477 v4l2 driver See: raspberrypi/linux#3605 firmware: isp: fix ISP component to return non-zero focus FoMs firmware: Fix for IMX477 focal length, f_number and aperture firmware: Update firmware for USB MSD boot firmware: platform: Fix overflow on high arm overclocks firmware: video_encode: Add option to include header bytes with frame firmware: DSI display: Close I2C handle if the display doesn't probe firmware: mmal/vc: Add mapping for OMX_IndexConfigBufferStall / MMAL_PARAMETER_VIDEO_STALL_THRESHOLD See: https://www.raspberrypi.org/forums/viewtopic.php?f=70&t=273123&p=1655481 firmware: hdmi: Request an I2C interrupt for EDID reading firmware: i2c: Move using_interrupt flag into periph_setup
See: raspberrypi/linux#3626 kernel: VC4 DRM/KMS - use correct dma-ranges See: raspberrypi/linux#3623 kernel: media: bcm2835-unicam: Retain packing information on G_FMT See: raspberrypi/linux#3622 kernel: Switch to snd_soc_dai_set_bclk_ratio See: raspberrypi/linux#3620 kernel: V4L2 H264 framing fixes See: raspberrypi/linux#3614 kernel: drm/vc4: Fix VIC usage with Broadcast RGB See: raspberrypi/linux#3611 kernel: media: bcm2835-unicam: Always service interrupts See: raspberrypi/linux#3608 kernel: overlays: Fix audio parameter of vc4-kms-v3 See: raspberrypi/linux#2489 kernel: configs: Restore missing NF_TABLES settings See: raspberrypi/linux#3615 kernel: sc16is7xx: Fix for hardware flow control See: raspberrypi/linux#2542 kernel: Use the upstream cpufreq driver on non-BCM2835 Pis See: raspberrypi/linux#3604 kernel: Backport of udmabuf and dma-heaps See: raspberrypi/linux#3571 kernel: imx477 v4l2 driver See: raspberrypi/linux#3605 firmware: isp: fix ISP component to return non-zero focus FoMs firmware: Fix for IMX477 focal length, f_number and aperture firmware: Update firmware for USB MSD boot firmware: platform: Fix overflow on high arm overclocks firmware: video_encode: Add option to include header bytes with frame firmware: DSI display: Close I2C handle if the display doesn't probe firmware: mmal/vc: Add mapping for OMX_IndexConfigBufferStall / MMAL_PARAMETER_VIDEO_STALL_THRESHOLD See: https://www.raspberrypi.org/forums/viewtopic.php?f=70&t=273123&p=1655481 firmware: hdmi: Request an I2C interrupt for EDID reading firmware: i2c: Move using_interrupt flag into periph_setup
This is the suggested approach by @mripard to use the dma-ranges from the HVS (or firmware-kms) driver for the base DRM device, and therefore allocations should be done with the correct cache alias information for when it is passed to the HVS later.