[PATCH 16/31] drm/amd/display: refactor function transmitter_to_phy_id

Nathan Chancellor nathan at kernel.org
Fri Jun 17 19:51:51 UTC 2022


Hi Rodrigo,

On Fri, Jun 17, 2022 at 03:34:57PM -0400, Rodrigo Siqueira wrote:
> From: Nicholas Choi <Nicholas.Choi at 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.Wang at amd.com>
> Acked-by: Alan Liu <HaoPing.Liu at amd.com>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas at amd.com>
> Signed-off-by: Nathan Chancellor <natechancellor at gmail.com>
> Signed-off-by: Alex Deucher <alexander.deucher at 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
> 
> 


More information about the amd-gfx mailing list