Re: [PATCH 1/3] drm/amd/display: support HDMI 1.4 3D modes on HDMI connectors

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] CRTC timings for 3D Frame Packing modes are silently overwritten with native 2D timings when hardware scaling is enabled, breaking 3D output.
- [High] FreeSync Video Mode optimization overwrites 3D mode flags, silently downgrading 3D streams to 2D.
--

commit 70b8f690db70299cb56618e8af16a07b44f95bd2
Author: Adrian Betschart <adrian.betschart@cinemaone.ch>
Subject: drm/amd/display: support HDMI 1.4 3D modes on HDMI connectors

This commit enables stereo modes (such as 3D Frame Packing, Top-and-Bottom,
and Side-by-Side) derived from a sink's HDMI VSDB on HDMI connectors. It
treats them as 2D streams by packing the views in the frame itself and relies
on the HDMI vendor infoframe to inform the sink.

> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> index 0e71ba498e8aa..7a2beb1756249 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_connector.c
[ ... ]
> @@ -887,7 +906,15 @@ STATIC_IFN_KUNIT void fill_stream_properties_from_drm_display_mode(
>  	 */
>  	timing_out->pixel_encoding = requested_encoding;
>  
> +	/*
> +	 * The source packs both views into the frame itself (side-by-side,
> +	 * top-and-bottom, or the doubled frame-packing timing), so the display
> +	 * core scans it out as a plain 2D stream and only the HDMI vendor
> +	 * infoframe tells the sink how the frame is laid out. Any DC stereo
> +	 * timing format would make the hardware treat the surface as two views.
> +	 */
>  	timing_out->timing_3d_format = TIMING_3D_FORMAT_NONE;
> +	stream->vsif_3d_format = amdgpu_dm_vsif_3d_format(mode_in->flags);

[Severity: High]
If amdgpu_dm_is_freesync_video_mode() matched earlier in
create_stream_for_sink(), will this silently downgrade the 3D stream to 2D?

In create_stream_for_sink(), if a 3D mode (like Side-by-Side or Top-and-Bottom)
matches the base timings of a FreeSync mode, the entire mode struct is
overwritten:

create_stream_for_sink() {
	...
	if (recalculate_timing) {
		freesync_mode = amdgpu_dm_get_highest_refresh_rate_mode(aconnector, false);
		drm_mode_copy(&saved_mode, &mode);
		...
		drm_mode_copy(&mode, freesync_mode);
		...
	}
}

This wipes the DRM_MODE_FLAG_3D_* flags from mode_in. When
amdgpu_dm_vsif_3d_format(mode_in->flags) is called here, it will evaluate to 0
and fall back to TIMING_3D_FORMAT_NONE, causing the HDMI vendor infoframe to
never be sent and leaving the sink in 2D mode.

>  	timing_out->display_color_depth = amdgpu_dm_convert_color_depth_from_display_info(
>  		connector,
>  		(timing_out->pixel_encoding == PIXEL_ENCODING_YCBCR420),
[ ... ]
> @@ -1451,6 +1478,9 @@ create_stream_for_sink(struct drm_connector *connector,
>  	struct dc_sink *sink = NULL;
>  
>  	drm_mode_init(&mode, drm_mode);
> +	/* frame packing scans out both views plus the active space in one frame */
> +	if (mode.flags & DRM_MODE_FLAG_3D_FRAME_PACKING)
> +		drm_mode_set_crtcinfo(&mode, CRTC_STEREO_DOUBLE);

[Severity: High]
Are these CRTC_STEREO_DOUBLE timings at risk of being silently overwritten
if hardware scaling is enabled?

Later in create_stream_for_sink(), the mode is passed for timing adjustments:

create_stream_for_sink() {
	...
	} else {
		decide_crtc_timing_for_drm_display_mode(
				&mode, preferred_mode, scale);
	...
}

If scale is true, decide_crtc_timing_for_drm_display_mode() unconditionally
copies the standard 2D timings from the preferred mode:

decide_crtc_timing_for_drm_display_mode() {
	if (scale_enabled || ...) {
		if (native_mode->crtc_clock)
			copy_crtc_timing_for_drm_display_mode(native_mode, drm_mode);
	}
}

Will this wipe out the stereo-doubled timings just set here, misconfiguring
the hardware scanout with standard 2D timings instead and leading to
truncated/corrupted 3D output?

>  	memset(&saved_mode, 0, sizeof(saved_mode));
>  
>  	if (connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907100626.54465-1-adrian.betschart@cinemaone.ch?part=1