Skip to content

Change vc4 DSI to being a bridge #4878

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Feb 16, 2022
Merged

Change vc4 DSI to being a bridge #4878

merged 10 commits into from
Feb 16, 2022

Conversation

6by9
Copy link
Contributor

@6by9 6by9 commented Feb 7, 2022

Bridges and the atomic API are the current approach.

Creating this PR as I need to send the framework patches upstream.

6by9 added 10 commits February 10, 2022 11:49
@6by9
drm: Introduce DRM_BRIDGE_OP_UPSTREAM_FIRST to alter bridge init order
2c55355 
DSI sink devices typically want the DSI host powered up and configured
before they are powered up. pre_enable is the place this would normally
happen, but they are called in reverse order from panel/connector towards
the encoder, which is the "wrong" order.

Add a new flag DRM_BRIDGE_OP_UPSTREAM_FIRST that any bridge can set
to swap the order of pre_enable (and post_disable) so that any upstream
bridges are called first to create the desired state.

eg:
- Panel
- Bridge 1
- Bridge 2 DRM_BRIDGE_OP_UPSTREAM_FIRST
- Bridge 3
- Encoder
Would result in pre_enable's being called as Panel, Bridge 1, Bridge 3,
Bridge 2.

Signed-off-by: Dave Stevenson <[email protected]>
@6by9
drm/dsi: Document the meaning and spec references for MIPI_DSI_MODE_*
6e27cd2 
The MIPI_DSI_MODE_* flags have fairly terse descriptions and no reference
to the DSI specification as to their exact meaning. Usage has therefore
been rather fluid.

Extend the descriptions and provide references to the part of the
MIPI DSI specification regarding what they mean.

Signed-off-by: Dave Stevenson <[email protected]>
@6by9
drm/bridge: tc358762: Ignore EPROBE_DEFER when logging errors
e90473a 
mipi_dsi_attach can fail due to resources not being available
yet, therefore do not log error messages should they occur.

Signed-off-by: Dave Stevenson <[email protected]>
@6by9
drm/tc358762: Set the DRM_BRIDGE_OP_UPSTREAM_FIRST flag to configure …
97cdbef 
…DSI host

TC358762 wants the DSI host to be prepared before it is powered up, so
set the flag to request that the upstream bridges have their
pre_enable called first.

Signed-off-by: Dave Stevenson <[email protected]>
@6by9
drm/vc4: Rename bridge to out_bridge
deb4859 
In preparation for converting the encoder to being a bridge,
rename the variable holding the next bridge in the chain to
out_bridge, so that our bridge can be called bridge.

Signed-off-by: Dave Stevenson <[email protected]>
@6by9
drm/vc4: Move DSI initialisation to encoder_mode_set.
911b4e8 
Breaking the bridge chain does not work for atomic bridges/panels
and generally causes issues.
We need to initialise the DSI host before the bridge pre_enables
are called, so move that to encoder_mode_set in the same way that
dw-mipi-dsi does.

Signed-off-by: Dave Stevenson <[email protected]>
@6by9
drm/vc4: Remove splitting the bridge chain from the driver.
4b8b607 
Splitting the bridge chain fails for atomic bridges as the
framework can't add the relevant state in
drm_atomic_add_encoder_bridges.
The chain was split because we needed to power up before
calling pre_enable, but that is now done in mode_set, and will
move into the framework.

Signed-off-by: Dave Stevenson <[email protected]>
@6by9
drm/vc4: Convert vc4_dsi to use atomic enable/disable/mode_set.
e9b590c 
The atomic calls are preferred as the non-atomic ones
are deprecated. In preparation for conversion to a bridge,
switch to the atomic calls.

Signed-off-by: Dave Stevenson <[email protected]>
@6by9
drm/vc4: Convert vc4_dsi to using a bridge instead of encoder.
5d74a18 
Remove the encoder functions, and create a bridge attached to
this dumb encoder which implements the same functionality.

As a bridge has state which an encoder doesn't, we need to
add the state management functions as well.

As there is no bridge atomic_mode_set, move the initialisation
code that was in mode_set into _pre_enable.
The code to actually enable and disable sending video are split
from the general control into _enable and _disable.

Signed-off-by: Dave Stevenson <[email protected]>
@6by9
drm/vc4: Remove entry to ULPS from vc4_dsi post_disable
ff223f3 
Post_disable was sending the D-PHY sequence to put any device
into ULPS suspend mode, and then cutting power to the DSI block.
The power-on reset state of the DSI block is for DSI to be in
an operational state, not ULPS, so it then never sent the sequence
for exiting ULPS. Any attached device that didn't have an external
reset therefore remained in ULPS / standby, and didn't function.

Use of ULPS isn't well specified in DRM, therefore remove entering
it to avoid the above situation.

Signed-off-by: Dave Stevenson <[email protected]>
@6by9 6by9 changed the title WIP: Change DSI to being a bridge Change vc4 DSI to being a bridge Feb 11, 2022
@6by9
Copy link
Contributor Author

6by9 commented Feb 11, 2022

These patches seem pretty solid now. Tested on CM4, Pi4, Pi3 with 7" panel, and CM4 with SN65DSI83 DSI->LVDS bridge.
I'll be sending the main framework changes to dri-devel next week, but these should be at a place that is worth a review.

pelwell
pelwell reviewed Feb 14, 2022
View reviewed changes
Copy link
Contributor

@pelwell pelwell left a comment

Choose a reason for hiding this comment

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

This is definitely TL;DR for a topic outside my experience. A few strategic comments wouldn't hurt. And I suppose there is no elegant way to avoid the duplication between the atomic and non-atomic APIs without introducing more framework.

drivers/gpu/drm/drm_bridge.c
if (next->ops & DRM_BRIDGE_OP_UPSTREAM_FIRST) {
limit = next;

list_for_each_entry_from(next, &encoder->bridge_chain,
Copy link
Contributor

Choose a reason for hiding this comment

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

This loop would benefit from at least a single explanatory comment.

drivers/gpu/drm/drm_bridge.c
list_for_each_entry_from(bridge, &encoder->bridge_chain, chain_node) {
if (bridge->funcs->atomic_post_disable) {
struct drm_bridge_state *old_bridge_state;
limit = NULL;
Copy link
Contributor

Choose a reason for hiding this comment

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

This all looks very familiar - it's a shame the non-atomic case can't be a handled as a special case of the atomic API.

@pelwell pelwell merged commit c2ece12 into raspberrypi:rpi-5.15.y Feb 16, 2022
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Feb 16, 2022
@popcornmix
kernel: Bump to 5.15.23
630ca24 
kernel: overlays: rpi-poe(-plus): Fix parameters
See: raspberrypi/linux#4877

kernel: i2c: bcm2835: Make clock-stretch timeout configurable
See: raspberrypi/linux#4855

kernel: Add DPI mode 3 (rgb565-padhi) support to vc4-kms-dpi-generic
See: raspberrypi/linux#4882

kernel: media: i2c: imx219: Scale the pixel clock rate for the 640x480 mode
See: raspberrypi/linux#4880

kernel: vc4_dpi fixes
See: raspberrypi/linux#4889

kernel: Change vc4 DSI to being a bridge
See: raspberrypi/linux#4878

kernel: sc16is7xx: Fix for incorrect data being transmitted
See: raspberrypi/linux#4885
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request Feb 16, 2022
@popcornmix
kernel: Bump to 5.15.23
9e99d81 
kernel: overlays: rpi-poe(-plus): Fix parameters
See: raspberrypi/linux#4877

kernel: i2c: bcm2835: Make clock-stretch timeout configurable
See: raspberrypi/linux#4855

kernel: Add DPI mode 3 (rgb565-padhi) support to vc4-kms-dpi-generic
See: raspberrypi/linux#4882

kernel: media: i2c: imx219: Scale the pixel clock rate for the 640x480 mode
See: raspberrypi/linux#4880

kernel: vc4_dpi fixes
See: raspberrypi/linux#4889

kernel: Change vc4 DSI to being a bridge
See: raspberrypi/linux#4878

kernel: sc16is7xx: Fix for incorrect data being transmitted
See: raspberrypi/linux#4885
@pelwell
Copy link
Contributor

pelwell commented Feb 17, 2022

This does not apply cleanly to 5.16 - can you fix it up?

@6by9 6by9 mentioned this pull request Feb 17, 2022
Merged
popcornmix added a commit to raspberrypi/rpi-firmware that referenced this pull request Feb 18, 2022
@popcornmix
kernel: Bump to 5.15.24
77451e6 
kernel: usb: xhci: add a quirk for Superspeed bulk OUT transfers on VL805
See: raspberrypi/linux#4893

kernel: Change vc4 DSI to being a bridge
See: raspberrypi/linux#4878

kernel: Fix for the cursor corruption with KMS (again)
See: raspberrypi/linux#4895

kernel: OV7251 updates for libcamera compatibility
See: raspberrypi/linux#4897
popcornmix added a commit to raspberrypi/firmware that referenced this pull request Feb 18, 2022
@popcornmix
kernel: Bump to 5.15.24
a6496ae 
kernel: usb: xhci: add a quirk for Superspeed bulk OUT transfers on VL805
See: raspberrypi/linux#4893

kernel: Change vc4 DSI to being a bridge
See: raspberrypi/linux#4878

kernel: Fix for the cursor corruption with KMS (again)
See: raspberrypi/linux#4895

kernel: OV7251 updates for libcamera compatibility
See: raspberrypi/linux#4897
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pelwell pelwell pelwell left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@6by9 @pelwell