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 <[email protected]> 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/[email protected]?part=1
