On Fri, Aug 1, 2025 at 6:02 PM Timur Kristóf <timur.kris...@gmail.com> wrote:
> > > 2025. augusztus 1., péntek dátummal Alexandre Demers < > alexandre.f.dem...@gmail.com> ezt írta: > >> We support two kinds of analog connections: > >> > >> 1. VGA, which only supports analog signals: > >> For VGA, we need to create a link encoder that only works with the > >> DAC without perturbing any digital transmitter functionality. > >> This is achieved by the new dce110_analog_link_encoder_construct. > >> > >> 2. DVI-I, which allows both digital and analog signals: > >> The DC code base only allows 1 encoder per connector, and the > >> preferred engine type is still going to be digital. So, for DVI-I > >> to work, we need to make sure the pre-existing link encoder can > >> also work with analog signals. > >> > >> Signed-off-by: Timur Kristóf <timur.kristof at gmail.com> > >> --- > >> .../drm/amd/display/dc/dce/dce_link_encoder.c | 100 ++++++++++++++++++ > >> .../drm/amd/display/dc/dce/dce_link_encoder.h | 21 ++-- > > > > I have the same comment about the use of "dce110_" prefix under > general/global DCE code that I left on the previous patch. > > > > For consistency with the current code, I understand why this prefix is > used, but I'd gladly clean this up once the patches have landed in if there > is a common agreement. > > > > Alexandre > > Hi Alexandre, > > With regards to the coding style. I already replied to your other thread > about it, let's have that conversation there. > > With regards to the link encoders specifically. Due to DVI-I, we need > dce110_link_encoder to handle analog signals in addition to digital, so the > question about this part is, is there any need to have a separate > dce110_analog_link_encoder? When I wrote the patch I felt yes, but now I > feel maybe we should just let dce110_link_encoder handle VGA as well. > > What do you think about that? > > Thanks, > Timur > > > Since a distinction is already made in the code between digital and analog encoders, I would be tempted to go with the dce110_analog_link_encoder so it may be just easier/quicker to distinguish what this part of code does. Alexandre