Inline.

> -----Original Message-----
> From: Kazlauskas, Nicholas <nicholas.kazlaus...@amd.com>
> Sent: 2019/October/17, Thursday 9:37 AM
> To: Liu, Zhan <zhan....@amd.com>; amd-gfx@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....@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....@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> >

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Reply via email to