Skip to content

drm/vc4: Support frame packed stereo modes - #7524

Draft
popcornmix wants to merge 4 commits into
raspberrypi:rpi-6.18.yfrom
popcornmix:3dfp
Draft

drm/vc4: Support frame packed stereo modes#7524
popcornmix wants to merge 4 commits into
raspberrypi:rpi-6.18.yfrom
popcornmix:3dfp

Conversation

@popcornmix

Copy link
Copy Markdown
Collaborator

Report support for frame packed 3d stereo hdmi mode (CRTC_STEREO_DOUBLE).

This also fixes a bug in upstream drm_hdmi_compute_mode_clock where 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.

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>
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.

1 participant