[PATCH 10/20] drm/amd/display: Implement DCE analog stream encoders

[Alexandre Demers]

Hi Timur,

I agree with you about the coding style (the prefix), it was more of a
general comment since the style is inconsistent with some other parts in
the DC code.

If the code applies to any DCE version (even though it is unclear if any >
DCE 10 is in the field with a DAC), then it's good for me. My only
remaining concern would be about encountering one of those special GPU
cards hardly testable. But this seems unlikely.

Alexandre

On Fri, Aug 1, 2025 at 5:29 PM Timur Kristóf <timur.kristof at gmail.com>
wrote:

>
> 2025. augusztus 1., péntek dátummal Alexandre Demers <
> alexandre.f.demers at gmail.com> ezt írta:
> >> Add stream encoders for DCE6-10 only, because there are definitely
> >> graphics cards with analog connectors out there with these DCE
> >> versions. I am not aware of newer ones.
> >
> >> Considering that all stream encoder functions currently have to do
> >> with digital streams, there is nothing for an analog stream
> >> encoder to do, making them basically a no-op.
> >> That being said, we still need some kind of stream encoder to
> >> represent an analog stream, and it is beneficial to split them from
> >> digital stream encoders in the code to make sure they don't
> >> accidentally write any DIG* registers.
> >>
> >> Signed-off-by: Timur Kristóf <timur.kristof at gmail.com>
> >> ---
> >> .../drm/amd/display/dc/dce/dce_stream_encoder.c | 14 ++++++++++++++
> >> .../drm/amd/display/dc/dce/dce_stream_encoder.h | 5 +++++
> >> .../display/dc/resource/dce100/dce100_resource.c | 6 ++++++
> >> .../amd/display/dc/resource/dce60/dce60_resource.c | 8 ++++++++
> >> .../amd/display/dc/resource/dce80/dce80_resource.c | 8 ++++++++
> >> 5 files changed, 41 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> >> index 1130d7619b26..f8996ee2856b 100644
> >> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> >> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_stream_encoder.c
> >> @@ -1567,3 +1567,17 @@ void dce110_stream_encoder_construct(
> >> enc110->se_shift = se_shift;
> >> enc110->se_mask = se_mask;
> >> }
> >> +
> >> +static const struct stream_encoder_funcs dce110_an_str_enc_funcs = {0};
> >> +
> >> +void dce110_analog_stream_encoder_construct(
> >> + struct dce110_stream_encoder *enc110,
> >> + struct dc_context *ctx,
> >> + struct dc_bios *bp,
> >> + enum engine_id eng_id)
> >> +{
> >> + enc110->base.funcs = &dce110_an_str_enc_funcs;
> >> + enc110->base.ctx = ctx;
> >> + enc110->base.id = eng_id;
> >> + enc110->base.bp = bp;
> >> +}
> >
> > Since we are adding analog stream encoder support only up to DCE10,
> wouldn't it be better if the prefix "dce100_" was used instead? I know
> there are a few functions in there that use "dce110_" as prefix and are
> replaced by functions specific to the DCE versions that behave differently
> (we even have dce60_ and dce80_ in the current patch), but this seems off
> otherwise.
> >
> > IMO, if thie DCE code should be revisited, "dce_" should be the general
> prefix instead of "dce110_", with "dceXY_" being specific (as it is right
> now).
> > Alexandre
>
> Hi Alexandre,
>
> Two reasons:
>
> 1. As best as I can tell, all DCE versions use dce110_stream_encoder
> regardless of its name. I agree that this is somewhat counter-intuitive but
> I wanted to follow the conventions in the rest of this file.
>
> 2. It is unclear which DCE versions actually have a DAC. On one hand there
> is some conversation on this ML that suggests that Hawaii and newer don't
> have a DAC, which is apparently wrong. Looking at the register headers, all
> DCE including the latest one have the DAC registers. Theoretically, if
> there were newer GPUs with DAC, this code would work fine on them too.
>
> If you are not happy with the code style, I respect that but I would like
> if we tackled that separarely.
>
> With these series, I want to just focus on fixing things and getting DC to
> work on SI and CIK with the least possible risk of breaking things.
>
> Best regards,
> Timur
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20250803/a6865ba2/attachment.htm>