On 2025-07-30 13:08, Timur Kristóf wrote:
On Wed, 2025-07-30 at 10:34 -0400, Harry Wentland wrote:
On 2025-07-30 04:19, Timur Kristóf wrote:
On Tue, 2025-07-29 at 14:21 -0400, Harry Wentland wrote:
On 2025-07-23 11:58, Timur Kristóf wrote:
Features like stereo sync and audio are not supported by RGB
signals, so don't try to use them.
Where does it say that?
Harry
1. Audio
VGA ports (and the analog part of DVI-I ports) simply cannot carry
audio. So there is no hardware to control any audio, therefore
there is
nothing for this code to enable, which is why I added those ifs to
not
even try to enable audio on analog video signals.
My bad, I was thinking RGB as opposed to YCbCr. Forgot that we use
RGB signal to refer to VGA.
Sorry for the confusion.
As a side note, DVI-D ports (and the digital part of DVI-I ports)
may
have a non-standard extension to carry digital audio signals, but
that
is not revelant to supporting analog displays.
2. Stereo sync
With regards to stereo sync, I didn't find any reference to this in
the
legacy display code, so I assumed either it is unsupported or the
VBIOS
already sets it up correctly. At least, considering that the legacy
code didn't bother setting it up, we don't lose any functionality
if we
leave it out of DC as well.
That being said, upon some further digging in the DCE register
files, I
found a register called DAC_STEREOSYNC_SELECT so maybe I could
investigate using that. Maybe it would be better to work with the
registers directly instead of the VBIOS? Would it be okay to
investigate that further in a future patch series once this one is
merged?
I don't think DC supports stereo sync currently. I'm not sure there
is
much value in pursuing that.
If stereo sync is not supported, what does setup_stereo_sync() do?
My bad. Not sure then. But no objection if you want to explore it.
Harry
>>>
diff --git a/drivers/gpu/drm/amd/display/include/signal_types.h
b/drivers/gpu/drm/amd/display/include/signal_types.h
index a10d6b988aab..825a08fcb125 100644
--- a/drivers/gpu/drm/amd/display/include/signal_types.h
+++ b/drivers/gpu/drm/amd/display/include/signal_types.h
@@ -118,6 +118,11 @@ static inline bool dc_is_dvi_signal(enum
signal_type signal)
}
}
+static inline bool dc_is_rgb_signal(enum signal_type signal)
To avoid confusion with people that haven't worked on analog
signals in years (or ever) it might be better to name this
dc_is_analog_signal.
Harry
Sounds good, I'll rename it in the next version of the series.
To further ease the confusion, what do you think about renaming
SIGNAL_TYPE_RGB to SIGNAL_TYPE_ANALOG?
I think Harry hasn't answered your proposition. I must say that the
first time I looked for VGA in the legacy code, I stumbled upon the RGB
usage. But then, it began to make sense (I'm not completely sure if
signals and connector types are used properly everywhere), because we
are mostly translating DRM signal types to supported connector
types.That being said, while both dc_is_rgb_signal() and
dc_is_analog_signal() could be used here, we are specifically querying
the signal type and this signal type is RGB. Because of this, I would be
in favor of keeping dc_is_rgb_signal() unless there is another analog
type that could be queried and not rename SIGNAL_TYPE_RGB to
SIGNAL_TYPE_ANALOG in any case. Cheers, Alexandre >>
Thanks,
Timur
+{
+ return (signal == SIGNAL_TYPE_RGB);
+}
+
static inline bool dc_is_tmds_signal(enum signal_type signal)
{
switch (signal) {
