Hi Rodrigo,

On Fri, Jun 17, 2022 at 03:34:57PM -0400, Rodrigo Siqueira wrote:
> From: Nicholas Choi <nicholas.c...@amd.com>
> 
> [Why & How]
> Since we only need transmitter value in function transmitter_to_phy_id().
> Replace argument struct dc_link with enum transmitter.
> 
> Reviewed-by: Chao-kai Wang <stylon.w...@amd.com>
> Acked-by: Alan Liu <haoping....@amd.com>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlaus...@amd.com>
> Signed-off-by: Nathan Chancellor <natechancel...@gmail.com>
> Signed-off-by: Alex Deucher <alexander.deuc...@amd.com>

How did I end up in the signoff chain for a patch I have never seen up
until this point? That should definitely be cleaned up.

Additionally, this commit message doesn't really seem to line up with
the change. It says that "struct dc_link" is being replaced with "enum
transmitter", when it is really the reverse, and that only the
transmitter value is needed, which is already the case, right? I guess
this is so that you can use DC_ERROR(), which requires a dc_ctx
variable? It is not immediately obvious from the commit message so that
should be clarified as well.

Cheers,
Nathan

> ---
>  drivers/gpu/drm/amd/display/dc/core/dc_link.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 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 43b55bc6e2db..58882d42eff5 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c
> @@ -3185,8 +3185,11 @@ bool dc_link_get_psr_state(const struct dc_link *link, 
> enum dc_psr_state *state)
>  }
>  
>  static inline enum physical_phy_id
> -transmitter_to_phy_id(enum transmitter transmitter_value)
> +transmitter_to_phy_id(struct dc_link *link)
>  {
> +     struct dc_context *dc_ctx = link->ctx;
> +     enum transmitter transmitter_value = link->link_enc->transmitter;
> +
>       switch (transmitter_value) {
>       case TRANSMITTER_UNIPHY_A:
>               return PHYLD_0;
> @@ -3213,8 +3216,7 @@ transmitter_to_phy_id(enum transmitter 
> transmitter_value)
>       case TRANSMITTER_UNKNOWN:
>               return PHYLD_UNKNOWN;
>       default:
> -             WARN_ONCE(1, "Unknown transmitter value %d\n",
> -                       transmitter_value);
> +             DC_ERROR("Unknown transmitter value %d\n", transmitter_value);
>               return PHYLD_UNKNOWN;
>       }
>  }
> @@ -3331,7 +3333,7 @@ bool dc_link_setup_psr(struct dc_link *link,
>       psr_context->phyType = PHY_TYPE_UNIPHY;
>       /*PhyId is associated with the transmitter id*/
>       psr_context->smuPhyId =
> -             transmitter_to_phy_id(link->link_enc->transmitter);
> +             transmitter_to_phy_id(link);
>  
>       psr_context->crtcTimingVerticalTotal = stream->timing.v_total;
>       psr_context->vsync_rate_hz = div64_u64(div64_u64((stream->
> -- 
> 2.25.1
> 
> 

Reply via email to