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.
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?
Thanks,
Timur
>
> > Also add a dc_is_rgb_signal similar to other dc_is_*_signal.
> >
> > Signed-off-by: Timur Kristóf <timur.kris...@gmail.com>
> > ---
> > drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c | 3 ++-
> > drivers/gpu/drm/amd/display/dc/link/link_dpms.c | 6 ++++-
> > -
> > drivers/gpu/drm/amd/display/include/signal_types.h | 5 +++++
> > 3 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git
> > a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > index b68bcc9fca0a..f3470716734d 100644
> > --- a/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > +++ b/drivers/gpu/drm/amd/display/dc/link/hwss/link_hwss_dio.c
> > @@ -115,7 +115,8 @@ void setup_dio_stream_attribute(struct pipe_ctx
> > *pipe_ctx)
> > struct dc_stream_state *stream = pipe_ctx->stream;
> > struct dc_link *link = stream->link;
> >
> > - if (!dc_is_virtual_signal(stream->signal))
> > + if (!dc_is_virtual_signal(stream->signal) &&
> > + !dc_is_rgb_signal(stream->signal))
> > stream_encoder->funcs->setup_stereo_sync(
> > stream_encoder,
> > pipe_ctx->stream_res.tg->inst,
> > diff --git a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
> > b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
> > index 8c8682f743d6..d6b7347c6c11 100644
> > --- a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
> > +++ b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
> > @@ -2369,7 +2369,8 @@ void link_set_dpms_off(struct pipe_ctx
> > *pipe_ctx)
> > set_avmute(pipe_ctx, true);
> > }
> >
> > - dc->hwss.disable_audio_stream(pipe_ctx);
> > + if (!dc_is_rgb_signal(pipe_ctx->stream->signal))
> > + dc->hwss.disable_audio_stream(pipe_ctx);
> >
> > update_psp_stream_config(pipe_ctx, true);
> > dc->hwss.blank_stream(pipe_ctx);
> > @@ -2656,7 +2657,8 @@ void link_set_dpms_on(
> > enable_stream_features(pipe_ctx);
> > update_psp_stream_config(pipe_ctx, false);
> >
> > - dc->hwss.enable_audio_stream(pipe_ctx);
> > + if (!dc_is_rgb_signal(pipe_ctx->stream->signal))
> > + dc->hwss.enable_audio_stream(pipe_ctx);
> >
> > if (dc_is_hdmi_signal(pipe_ctx->stream->signal)) {
> > set_avmute(pipe_ctx, false);
> > 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)
> > +{
> > + return (signal == SIGNAL_TYPE_RGB);
> > +}
> > +
> > static inline bool dc_is_tmds_signal(enum signal_type signal)
> > {
> > switch (signal) {
