[PATCH 07/20] drm/amd/display: Don't use stereo sync and audio on RGB signals

[Harry Wentland]

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