amdv/drm: Dolby Vision output-path fixes#29
Conversation
| strstr(common->fmt_attr, "422") == NULL && | ||
| strstr(common->fmt_attr, "444") == NULL) { | ||
| if (is_hdmi4k_support_420(vic & 0xff)) { | ||
| if (is_hdmi4k_support_420(vic & 0xff) && !is_amdv_enable()) { |
There was a problem hiding this comment.
This is not need as it is already be checked by hdmi_current_eotf_type.
It get set/reset by vsif package from amdv. So it never reach this place when in DV mode.
There was a problem hiding this comment.
Agreed, on its own in this driver PR is a no-op. A Kodi side fix is needed.
When a Dolby Vision stream starts at the same HDMI timing as the current (SDR) desktop mode, no resolution change occurs, so Kodi never triggers a modeset and the driver keeps the existing SDR color attribute instead of re-deciding it for DV. The DV tunnel is then stuffed into whatever base was already on the wire: at 4K50/60 that's the kernel's forced 10-bit YUV420 base, which is an invalid DV container, so the sink drops the link (audio passes, TV shows no picture). Even when it does lock, the leftover color depth is nondeterministic so the output ends up player-led or TV-led "at random", not matching the user's selection. Both stem from the same gap: DV entry needs to force an HDMI color-attr re-decide to the correct DV base ({444,8}) even when the resolution itself doesn't change.
I can send a Kodi PR with these two changes to make the issue clearer. I'm also happy to work on a better fix if this one doesn't work.
There was a problem hiding this comment.
Not completely true, the case of same resolution but DV/non DV is already covered:
https://github.com/CoreELEC/xbmc/blob/f256069b8a6e43fd2596946dab611fe06271e3aa/xbmc/windowing/amlogic/WinSystemAmlogicGLESContext.cpp#L117
There was a problem hiding this comment.
@Portisch, you're right again. The DV on/off case is covered by your force-switch rework, so that's not it. My patch was trying to fix two remaining problems by forcing a colour re-decide, and I think that approach is in the wrong place. Two issues:
-
Display at 4K50/60, auto-refresh-rate disabled, play a 24p DV file. There's no resolution change on DV entry, so decide runs and lands on an invalid DV base (YUV420) that the sink can't tunnel → no signal on my TV. With the CoreElec TV-Led setting, it should output RGB 444 to start the tunnel.
-
Display at any resolution, auto-refresh-rate enabled, TV-led selected in settings. Play a 24.000Hz movie → correct TV-led 8-bit RGB. Then, while it's playing, start another DV title at 23.976Hz → it comes out player-led 12-bit RGB (and the reverse, 23.976→24.000, breaks the same way). So it isn't resolution changes as you suggested, it's the fractional-rate crossing (23.976 ↔ 24.000). Same-rate switches stay correct; both directions across the boundary break. My "always re-evaluate on DV transition" suggested patch catches it as it triggers all the time, but the real trigger is the frac-rate mode switch.
Happy to share logs / a repro for either.
There was a problem hiding this comment.
-
auto-refresh-rate disabled
This interpretation is not technically correct. When users disable the automatic refresh‑rate switching, Kodi will adhere to that setting and intentionally avoid any mode change. In other words, the absence of a mode switch is expected behavior and originates from the user’s configuration, not from a functional limitation.A forced Dolby Vision mode switch is already on my roadmap. Implementing it, however, is non‑trivial: Kodi contains several internal safeguards that explicitly suppress mode changes when the refresh‑rate option is disabled. Achieving a DV‑specific override requires modifying multiple points in the video and windowing pipeline to bypass these checks without destabilizing normal mode‑switch logic.
-
comes out player-led 12-bit RGB
Works here:
Playback of first sample: 4K24 RGB BT2020 8b DV 297MHz
Then switch directly without stop to second sample: 4K23.976 RGB BT2020 8b DV 297MHz
And back to first sample: 4K24 RGB BT2020 8b DV 297MHz
There was a problem hiding this comment.
For #2, I was wrong to say "any resolution." The real trigger is that the second file's color depth changes on the switch except if your GUI depth is 8-bit, in which case it stays 8-bit the whole time and you never see the toggle.
Could you try again with a switch to player-led mode and run the same test.
My tests:
-
4K @ 59.94Hz, default GUI color depth = 10-bit (automatically selected), TV-led, switch resolution on start/stop, no delay after refresh rate change. Playing the two DV files then toggles the second one to 10-bit RGB, which the display reads as player-led.
-
1080p @ 59.94Hz, GUI color depth = 8-bit (forced), player-led, switch resolution on start/stop, no delay after refresh rate change. Playing the two DV files then toggles the second one to YUV422 16-bit (invalid mode).
There was a problem hiding this comment.
@Portisch, I pulled this drm/hdmi colour-base commit out of this PR. It's a topic of its own.
I've "got a fix for it" that's probably on the wrong track compared to what you laid out above, but I'll send it as 2 PRs in case any of it is useful. Two parts:
- a Kodi AML change that triggers a mode switch when it's needed
- a driver change to correct the colour depth
| current_mode = dolby_vision_mode; | ||
| if (amdv_policy_process | ||
| (vf, ¤t_mode, check_format)) { | ||
| if (amdv_honor_output_change( |
There was a problem hiding this comment.
It's caused maybe by the amdv_wait_delay where it wait for default 2 frames to decide if DV or not.
So double checked?
There was a problem hiding this comment.
Good call, I checked it directly. With the debounce compiled out, I swept amdv_wait_delay = 0 / 1 / 2 / 6 and replayed a DV title from the start each time. The BYPASS→DV→SDR8→DV oscillation is identical at every value so it doesn't look that the wait delay produces it:
# amdv_wait_delay=0 (debounce off)
output change from 5 to 1 # BYPASS -> DV (IPT_TUNNEL)
decide_color_attr [eotf:1] => attr[YUV444,8bit]
output change from 1 to 4 # DV -> SDR8 <-- the away excursion
VOUT_EVENT_MODE_CHANGE
output change from 4 to 1 # SDR8 -> DV
output change from 1 to 4 # ...and again
output change from 4 to 1
# amdv_wait_delay = 1, 2, 6: same sequence, same timing
Happy to move to a better layer if you see one, but lowering amdv_wait_delay doesn't remove the excursion.
When Dolby Vision detection fails and the driver falls back to SDR, it re-committed with the stale enhancement-layer flag and metadata still attached, and skipped the core reset that every other format change performs. That could leave a core pointer stranded and smash the kernel stack canary. Build a clean SDR input (dropping the leftover EL fields) and reset the core before re-committing. Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
At the start of a Dolby Vision title the output-mode policy briefly oscillates BYPASS -> DV -> SDR8 -> DV. The transient DV->SDR8 step, taken while the source is still Dolby Vision, forces an unnecessary core reset with no config commit following it; this leaves an internal core pointer stranded and may be implicated in a rare stack-corruption. Hold the DV output for a few frames when the requested mode drops to SDR while the source is still Dolby Vision, so the spurious away-switch never commits. Genuine changes (the source leaving Dolby Vision, or DV->HDR10/BYPASS) are honored immediately. The hold length is tunable via the amdv_away_from_dv_debounce module parameter (frames to hold; 1 disables it). Signed-off-by: Olivier Allauzen <olivier.allauzen@gmail.com>
2b7036a to
18440d4
Compare
|
The commit about the core reset look valid to me. About the mode toggle please check and debug hdmitx_common_output_disable. It does clear out the current set info packet by Maybe a fix like this will do already the job and the TV stay in the mode set before by the vsif package. |
|
After merge of 6e10fef I maybe see what you mean. It's from like 1080p50 to 1080p50 and then to 2160p24 DV. This is a new behaviour because of the last commit. Kodi only switch to 2160p24, nothing else. Edit: |
2 participants
Three DV output-path fixes: