[PATCH 16/31] drm/amd/display: refactor function transmitter_to_phy_id
From: Nicholas Choi [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 Acked-by: Alan Liu Reviewed-by: Nicholas Kazlauskas Signed-off-by: Nathan Chancellor Signed-off-by: Alex Deucher --- 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 +,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
Re: [PATCH 16/31] drm/amd/display: refactor function transmitter_to_phy_id
Hi Rodrigo, On Fri, Jun 17, 2022 at 03:34:57PM -0400, Rodrigo Siqueira wrote: > From: Nicholas Choi > > [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 > Acked-by: Alan Liu > Reviewed-by: Nicholas Kazlauskas > Signed-off-by: Nathan Chancellor > Signed-off-by: Alex Deucher 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 +,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 > >
Re: [PATCH 16/31] drm/amd/display: refactor function transmitter_to_phy_id
On 2022-06-17 15:51, Nathan Chancellor wrote: Hi Rodrigo, On Fri, Jun 17, 2022 at 03:34:57PM -0400, Rodrigo Siqueira wrote: From: Nicholas Choi [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 Acked-by: Alan Liu Reviewed-by: Nicholas Kazlauskas Signed-off-by: Nathan Chancellor Signed-off-by: Alex Deucher 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. Hi Nathan, Thanks for reporting this error; it looks like our scripts have some issues. I'll take a look at that. About this patch, I'll drop it. Thanks Siqueira 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 +,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