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?
>
> > > > 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?
Thanks,
Timur
>
> > > > +{
> > > > + return (signal == SIGNAL_TYPE_RGB);
> > > > +}
> > > > +
> > > > static inline bool dc_is_tmds_signal(enum signal_type signal)
> > > > {
> > > > switch (signal) {
