Hi Jerry,

Thanks for catching that, you're right.

I went back through the sequence and confirmed that my new
dp_set_dsc_on_rx() call runs in the pre-enable phase, before
enable_link() performs DP link training. I had mixed up the requirement
to configure source-side DSC before link setup with the separate
requirement to enable DSC on the sink only after training.

It happened to work with the X13 and monitor I tested, including the
hotplug cases, but that clearly doesn't make the ordering safe for other
sinks. I won't keep patch 2 in its current form.

Would the right place for dp_set_dsc_on_rx() be at the start of the
post-enable phase, after link training but before
hwss->enable_stream()? Or should it stay in its current location after
hwss->enable_stream()?

I'll wait for your guidance on the intended placement before sending a
revision. Patch 1 should be independent of this sink DSC ordering change.

Thanks,
Mauricio


Em ter., 8 de set. de 2026 às 15:14, Zuo, Jerry <[email protected]> escreveu:
>
> AMD General
>
> Hi Mauricio:
>
>      This change breaks dsc, causing black screen.
>
>      Sink RX dsc enable should land after link training. Writing dsc enable 
> to the RX on an untrained link is too early, that should be done after 
> training. That is likely to be the main sequence regression.
>
> Regards,
> Jerry
>
> > -----Original Message-----
> > From: amd-gfx <[email protected]> On Behalf Of
> > Mauricio Bellon
> > Sent: Tuesday, August 25, 2026 17:18
> > To: [email protected]
> > Cc: Lin, Wayne <[email protected]>; Limonciello, Mario
> > <[email protected]>; Mauricio Bellon <[email protected]>;
> > Wentland, Harry <[email protected]>; Li, Sun peng (Leo)
> > <[email protected]>; Rodrigo Siqueira <[email protected]>; Deucher,
> > Alexander <[email protected]>; Koenig, Christian
> > <[email protected]>; [email protected]
> > Subject: [PATCH 2/2] drm/amd/display: enable sink DSC before source DSC
> >
> > [Some people who received this message don't often get email from
> > [email protected]. Learn why this is important at
> > https://aka.ms/LearnAboutSenderIdentification ]
> >
> > From: Mauricio Bellon <[email protected]>
> >
> > Direct DisplayPort high-refresh modes using DSC and two-way ODM can
> > commit without visible output during a 4K120-to-4K144 transition. A working
> > 4K144 stream can also return black after long-pulse hotplug reconstruction.
> >
> > The DPMS-on path currently enables source DSC before link enablement but
> > does not enable sink DSC until after the source stream is enabled. This 
> > leaves
> > the sink and source compression state ordered differently while the stream 
> > is
> > brought up.
> >
> > Enable sink DSC immediately before source DSC in the pre-enable phase, and
> > remove the later redundant sink-enable operation near PPS packet
> > programming. This establishes the order as sink DSC, source DSC, stream
> > enablement, and PPS before the compressed stream becomes visible.
> >
> > Cc: [email protected]
> > Signed-off-by: Mauricio Bellon <[email protected]>
> > ---
> >  drivers/gpu/drm/amd/display/dc/link/link_dpms.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > 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 48b086d15..412ce52b3 100644
> > --- a/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
> > +++ b/drivers/gpu/drm/amd/display/dc/link/link_dpms.c
> > @@ -2635,8 +2635,10 @@ static enum dc_status
> > link_set_dpms_on_pre_enable_link(
> >          */
> >         if (stream->timing.flags.DSC) {
> >                 if (dc_is_dp_signal(stream->signal) ||
> > -                       dc_is_virtual_signal(stream->signal))
> > +                       dc_is_virtual_signal(stream->signal)) {
> > +                       dp_set_dsc_on_rx(pipe_ctx, true);
> >                         link_set_dsc_enable(pipe_ctx, true);
> > +               }
> >         }
> >
> >         if (link->replay_settings.config.replay_supported
> > && !dc_is_embedded_signal(link->connector_signal))
> > @@ -2729,7 +2731,6 @@ static enum dc_status
> > link_set_dpms_on_post_enable_link(
> >         if (stream->timing.flags.DSC) {
> >                 if (dc_is_dp_signal(stream->signal) ||
> >                                 dc_is_virtual_signal(stream->signal)) {
> > -                       dp_set_dsc_on_rx(pipe_ctx, true);
> >                         link_set_dsc_pps_packet(pipe_ctx, true, true);
> >                 }
> >         }
> > --
> > 2.53.0
>

Reply via email to