[PATCH] drm/amd/display: Modify display link stream setup sequence.

[Liu, Zhan]

Inline.

> -----Original Message-----
> From: Kazlauskas, Nicholas <Nicholas.Kazlauskas at amd.com>
> Sent: 2019/October/17, Thursday 9:37 AM
> To: Liu, Zhan <Zhan.Liu at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH] drm/amd/display: Modify display link stream setup
> sequence.
> 
> On 2019-10-17 12:28 a.m., Liu, Zhan wrote:
> > From: Zhan Liu <zhan.liu at amd.com>
> >
> > [Why]
> > When a specific kind of connector is detected,
> > DC needs to set the attribute of the stream.
> > This step needs to be done before enabling link,
> > or some bugs (e.g. display won't light up)
> > will be observed.
> >
> > [How]
> > Setting the attribute of the stream first, then
> > enabling stream.
> >
> > Signed-off-by: Zhan Liu <zhan.liu at amd.com>
> 
> NAK:
> 
> 1. It's difficult to understand what issue this change is attempting to
> solve and why it actually does it. Specifics would help here.

Some of the details are IP-sensitive, that's why I choose not to include details.

> 
> 2. It affects a common code path for all ASICs which has been tested and
> known to be working correctly for those test cases.

As we discussed before, considering Navi10 and Navi14 are using the same DC code, and the issue is only happening on Navi14, its more likely the issue is a BIOS issue. However, if we want to fix it on display side, we can only do some kinds of workaround to fix it. Another alternative is to do stream setup twice, but there is no point to repeat the setup two times. 

If we really worry about all AISCs will be influenced, we can guard the section as a Navi14 specific code, and treat this patch as a "hack".

> 
> 3. The description is incorrect - the link enable/stream enable were
> both previously happening after the stream setup. What's changed in the
> patch is the link enable now happens before the link setup.
> 
> Both of these calls internally go through the command table to VBIOS so
> what behavior differences you're seeing may be caused by the input
> parameters to the ATOM_ENCODER_CMD_STREAM_SETUP or
> TRANSMITTER_CONTROL_ENABLE commands or the actual execution of
> those
> commands.
> 
> Nicholas Kazlauskas
> 
> > ---
> >   drivers/gpu/drm/amd/display/dc/core/dc_link.c | 20 +++++++++----------
> >   1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > index fb18681b502b..713caab82837 100644
> > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> > @@ -2745,16 +2745,6 @@ void core_link_enable_stream(
> >                          dc_is_virtual_signal(pipe_ctx->stream->signal))
> >                  return;
> >
> > -       if (!dc_is_virtual_signal(pipe_ctx->stream->signal)) {
> > -               stream->link->link_enc->funcs->setup(
> > -                       stream->link->link_enc,
> > -                       pipe_ctx->stream->signal);
> > -               pipe_ctx->stream_res.stream_enc->funcs->setup_stereo_sync(
> > -                       pipe_ctx->stream_res.stream_enc,
> > -                       pipe_ctx->stream_res.tg->inst,
> > -                       stream->timing.timing_3d_format !=
> TIMING_3D_FORMAT_NONE);
> > -       }
> > -
> >          if (dc_is_dp_signal(pipe_ctx->stream->signal))
> >                  pipe_ctx->stream_res.stream_enc->funcs-
> >dp_set_stream_attribute(
> >                          pipe_ctx->stream_res.stream_enc,
> > @@ -2841,6 +2831,16 @@ void core_link_enable_stream(
> >                                          CONTROLLER_DP_TEST_PATTERN_VIDEOMODE,
> >                                          COLOR_DEPTH_UNDEFINED);
> >
> > +               if (!dc_is_virtual_signal(pipe_ctx->stream->signal)) {
> > +                       stream->link->link_enc->funcs->setup(
> > +                               stream->link->link_enc,
> > +                               pipe_ctx->stream->signal);
> > +                       pipe_ctx->stream_res.stream_enc->funcs-
> >setup_stereo_sync(
> > +                               pipe_ctx->stream_res.stream_enc,
> > +                               pipe_ctx->stream_res.tg->inst,
> > +                               stream->timing.timing_3d_format !=
> TIMING_3D_FORMAT_NONE);
> > +               }
> > +
> >   #ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT
> >                  if (pipe_ctx->stream->timing.flags.DSC) {
> >                          if (dc_is_dp_signal(pipe_ctx->stream->signal) ||
> > --
> > 2.17.1
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >