drm/vc4: Support frame packed stereo modes - #7524
Merged
Merged
Conversation
When parsing the per-VIC 3D_Structure_X entries of the HDMI VSDB, a side-by-side (half) entry is only accepted if the accompanying 3D_Detail_X field is 0001, "horizontal sub-sampling". HDMI 1.4b Table H-8 also defines 0000, "all of the horizontal sub-sampling and Quincunx matrix methods", which is a superset of 0001. The remaining defined values, 0110 to 1001, each select one specific Quincunx matrix variant, and everything else is reserved. DRM never populates hdmi_vendor_infoframe.s3d_ext_data, so the 3D_Ext_Data it transmits for a side-by-side (half) mode is 0000, "horizontal sub-sampling, odd left and odd right picture". A sink advertising 3D_Detail_X of 0000 supports that, so the mode should be offered. Instead the entry is silently discarded and such a sink ends up with no side-by-side modes at all. Seen on a Panasonic TX-P50VT30B, which reports 3D_Detail_X = 0000 for all of its side-by-side entries: 3D VIC indices with specific capabilities: VIC 16: 1920x1080 60.000000 Hz 16:9 (side-by-side, any subsampling) ... Fixes: 0e5083a ("drm/edid: parse the list of additional 3D modes") Signed-off-by: Dom Cobley <popcornmix@gmail.com>
An HDMI frame packing 3D mode transmits the left and right eye views, separated by an active space gap, within a single frame period. Per HDMI 1.4b section 8.2.3.2 the resulting raster has twice the vertical total of the base 2D mode, so the pixel clock - and with it the TMDS character rate - is doubled. drm_hdmi_compute_mode_clock() derives the rate from mode->clock, which for a stereo mode still describes the base 2D timing; the doubling only appears in the crtc_* timings, and only once a driver asks for it with CRTC_STEREO_DOUBLE. The computed rate was therefore half the real one, so the sink's max_tmds_clock was checked against the wrong value and drivers programmed half the pixel clock the mode needs. Only a driver that both advertises stereo modes and uses the HDMI connector helpers can see a change here, and vc4 is currently the only one. i915 and nouveau set connector->stereo_allowed but compute their TMDS rates themselves. The remaining callers of drm_hdmi_compute_mode_clock() - ite-it6263, dw_hdmi_qp-rockchip, inno_hdmi and sun4i_hdmi - leave stereo_allowed clear, so drm_mode_validate_flag() prunes 3D modes from their connectors and none ever reaches this function. Should userspace hand such a driver a 3D-flagged mode directly, it is now rejected as exceeding the sink's TMDS limit rather than being driven at half rate. Non-stereo modes, and the 3D layouts that fit within the 2D timing (side-by-side, top-and-bottom, and friends), are unaffected. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
The vertical timings written to the pixelvalve already come from the
crtc_* fields of the adjusted mode, but three places that describe the
same raster still read the mode->* fields directly:
- the HVS bandwidth estimate in vc4_crtc_atomic_check()
- the audio CTS divisor in vc4_hdmi_set_n_cts()
- the HVS display FIFO width and height in {vc4,vc6}_hvs_init_channel()
The two sets of fields differ only for stereo modes that pack more than
one eye into a frame, which vc4 does not yet handle, so switch them over
to mode->crtc_clock and drm_mode_get_hv_timing() ahead of adding that
support. drm_mode_get_hv_timing() is used rather than crtc_vdisplay
because the HVS is fed a whole frame even for interlaced modes, whose
crtc_vdisplay describes a single field.
No functional change.
Signed-off-by: Dom Cobley <popcornmix@gmail.com>
vc4 has always set connector->stereo_allowed, so a client with DRM_CLIENT_CAP_STEREO_3D is offered the frame packing modes advertised by the sink, but nothing ever expanded the timings for them. The pixelvalve was programmed with the base 2D raster while the sink expected two eyes and an active space gap, so the picture was wrong. Ask for CRTC_STEREO_DOUBLE when recomputing the adjusted mode's crtc_* fields. drm_mode_set_crtcinfo() then doubles crtc_vdisplay, crtc_vsync_start, crtc_vsync_end, crtc_vtotal and crtc_clock, which is everything the pixelvalve, the HVS and the audio CTS calculation need. The matching TMDS character rate comes from the HDMI connector helpers and the 3D_Structure vendor infoframe from drm_hdmi_vendor_infoframe_from_display_mode(), so nothing else has to change. CRTC_INTERLACE_HALVE_V has to be passed as well: drm_mode_set_crtcinfo() rebuilds every crtc_* field from scratch, and without it the halved field timings the core derived earlier for interlaced modes would be lost. Non-stereo modes are unaffected, as the CRTC_STEREO_DOUBLE adjustment applies only to modes carrying DRM_MODE_FLAG_3D_FRAME_PACKING. The WiFi interference workaround just above deliberately keeps keying off mode->clock, i.e. the 2D pixel clock. Frame packing modes landing in the affected range are not known to exist, and reworking that test would change behaviour for existing 2D modes. Tested on a Raspberry Pi 5. Signed-off-by: Dom Cobley <popcornmix@gmail.com>
popcornmix
marked this pull request as ready for review
July 29, 2026 14:40
Collaborator
Author
|
This is working fine on a Pi5 with a supporting kodi build. @6by9 any comments? |
Contributor
|
Does the Kodi build also work against any other DRM driver that supports 3D modes? That would appear to be i915 or nouveau. |
Collaborator
Author
|
No, I had to replicate 3d support for DRM renderer in kodi. This is an example output from kodi driving FP: You can see the 1125 (1080+45) as the offset for the left eye. And you can see GUI is a single 1920x2205 framebuffer (which includes the 45 line blanking). |
Contributor
|
I keep on forgetting that i915 (and other GPUs) tend not to support multiple planes at the DRM level. If you've compared the resulting HDMI output to the GL framebuffer that is generated via the other renderer, then that's probably good enough. The choice of plane numbers may cause you grief with zpos. Default assignment is always lowest plane number is the furthest back, so your second eye plane (plane 7) is likely in front of the GUI plane (plane 6), rather than behind it. You may have dealt with that by setting zpos, but I can't tell from the kmsprint output. This looks reasonable to merge, so go for it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Report support for frame packed 3d stereo hdmi mode (
CRTC_STEREO_DOUBLE).This also fixes a bug in upstream
drm_hdmi_compute_mode_clockwhere the clock should be doubled for frame packed, and a bug where 3D_Detail_X is incorrectly parsed from edid, meaning my SBS modes were not identified.