Hi,

First of all, thank you for your patch.

I just have one tiny comment inline.

On 02/26, Melissa Wen wrote:

Coding style clean up on enable_link_dp function as suggested by checkpatch.pl:

CHECK: Lines should not end with a '(' WARNING: line over 80 characters WARNING: suspect code indent for conditional statements (8, 24) CHECK: braces {} should be used on all arms of this statement ERROR: else should follow close brace '}' CHECK: Comparison to NULL could be written "link->preferred_training_settings.fec_enable"

Signed-off-by: Melissa Wen melissa.srw@gmail.com

drivers/gpu/drm/amd/display/dc/core/dc_link.c | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 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 a09119c10d7c..0f28b5694144 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c @@ -1480,9 +1480,8 @@ static void enable_stream_features(struct pipe_ctx *pipe_ctx) } }

-static enum dc_status enable_link_dp(

• struct dc_state *state,
  

• struct pipe_ctx *pipe_ctx)
  

+static enum dc_status enable_link_dp(struct dc_state *state,

• 		     struct pipe_ctx *pipe_ctx)
  

{ struct dc_stream_state *stream = pipe_ctx->stream; enum dc_status status; @@ -1512,27 +1511,28 @@ static enum dc_status enable_link_dp(

pipe_ctx->stream_res.pix_clk_params.requested_sym_clk = link_settings.link_rate * LINK_RATE_REF_FREQ_IN_KHZ;

• if (state->clk_mgr && !apply_seamless_boot_optimization)

• state->clk_mgr->funcs->update_clocks(state->clk_mgr, state, false);
  

• if (state->clk_mgr && !apply_seamless_boot_optimization) {

• state->clk_mgr->funcs->update_clocks(state->clk_mgr,
  

• 				     state, false);
  

• }

This `if` condition only has one action, which means that you don't need to add `{}` in the above statement. See:

https://www.kernel.org/doc/html/v4.10/process/coding-style.html#placing-brac...

Thanks

skip_video_pattern = true;

if (link_settings.link_rate == LINK_RATE_LOW)

• 	skip_video_pattern = false;
  

• if (perform_link_training_with_retries(

• 	&link_settings,
  

• 	skip_video_pattern,
  

• 	LINK_TRAINING_ATTEMPTS,
  

• 	pipe_ctx,
  

• 	pipe_ctx->stream->signal)) {
  

• skip_video_pattern = false;
  

• if (perform_link_training_with_retries(&link_settings,

• 			       skip_video_pattern,
  

• 			       LINK_TRAINING_ATTEMPTS,
  

• 			       pipe_ctx,
  

• 			       pipe_ctx->stream->signal)) {
  

  link->cur_link_settings = link_settings; status = DC_OK;

• }
• else

• } else { status = DC_FAIL_DP_LINK_TRAINING;
• }

• if (link->preferred_training_settings.fec_enable != NULL)

• if (link->preferred_training_settings.fec_enable) fec_enable = *link->preferred_training_settings.fec_enable; else fec_enable = true;

-- 2.25.0

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.free...

-----Original Message----- From: amd-gfx amd-gfx-bounces@lists.freedesktop.org On Behalf Of Melissa Wen Sent: 2020/February/26, Wednesday 5:08 PM To: Wentland, Harry Harry.Wentland@amd.com; Li, Sun peng (Leo) Sunpeng.Li@amd.com; Deucher, Alexander Alexander.Deucher@amd.com; Koenig, Christian Christian.Koenig@amd.com; Zhou, David(ChunMing) David1.Zhou@amd.com; David Airlie airlied@linux.ie; Daniel Vetter daniel@ffwll.ch; Rodrigo Siqueira rodrigosiqueiramelo@gmail.com Cc: dri-devel@lists.freedesktop.org; amd-gfx@lists.freedesktop.org; linux- kernel@vger.kernel.org Subject: [PATCH 2/2] drm/amd/display: dc_link: code clean up on detect_dp function

Removes codestyle issues on detect_dp function as suggested by checkpatch.pl.

CHECK: Lines should not end with a '(' WARNING: Missing a blank line after declarations WARNING: line over 80 characters CHECK: Alignment should match open parenthesis

Signed-off-by: Melissa Wen melissa.srw@gmail.com

Thank you Melissa for your contribution! Will apply it.

This patch is: Reviewed-by: Zhan Liu zhan.liu@amd.com

---

drivers/gpu/drm/amd/display/dc/core/dc_link.c | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 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 0f28b5694144..adb717f02c9c 100644 --- a/drivers/gpu/drm/amd/display/dc/core/dc_link.c +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link.c @@ -585,14 +585,14 @@ static void read_current_link_settings_on_detect(struct dc_link *link) LINK_SPREAD_05_DOWNSPREAD_30KHZ : LINK_SPREAD_DISABLED; }

-static bool detect_dp(

• struct dc_link *link,
• struct display_sink_capability *sink_caps,
• bool *converter_disable_audio,
• struct audio_support *audio_support,
• enum dc_detect_reason reason)

+static bool detect_dp(struct dc_link *link,

•       struct display_sink_capability *sink_caps,
  

•       bool *converter_disable_audio,
  

•       struct audio_support *audio_support,
  

•       enum dc_detect_reason reason)
  

{ bool boot = false;

• sink_caps->signal = link_detect_sink(link, reason); sink_caps->transaction_type = get_ddc_transaction_type(sink_caps->signal);

@@ -606,9 +606,8 @@ static bool detect_dp( sink_caps->signal = SIGNAL_TYPE_DISPLAY_PORT_MST; link->type = dc_connection_mst_branch;

• 	dal_ddc_service_set_transaction_type(
  

• 					link->ddc,
  

• 					sink_caps-
  

...

transaction_type);

• 	dal_ddc_service_set_transaction_type(link->ddc,
  

• 					     sink_caps-
  

...

transaction_type);

	/*
	 * This call will initiate MST topology discovery.

Which @@ -637,13 +636,10 @@ static bool detect_dp( if (reason == DETECT_REASON_BOOT) boot = true;

• 	dm_helpers_dp_update_branch_info(
  

• 		link->ctx,
  

• 		link);
  

• 	dm_helpers_dp_update_branch_info(link->ctx, link);
  
  

• 	if (!dm_helpers_dp_mst_start_top_mgr(
  

• 		link->ctx,
  

• 		link, boot)) {
  

• 	if (!dm_helpers_dp_mst_start_top_mgr(link->ctx,
  

• 					     link, boot)) {
  	/* MST not supported */
  	link->type = dc_connection_single;
  	sink_caps->signal =
  

SIGNAL_TYPE_DISPLAY_PORT; @@ -651,7 +647,7 @@ static bool detect_dp( }

if (link->type != dc_connection_mst_branch &&

• 	is_dp_active_dongle(link)) {
  

•     is_dp_active_dongle(link)) {
  /* DP active dongles */
  link->type = dc_connection_active_dongle;
  if (!link->dpcd_caps.sink_count.bits.SINK_COUNT)
  

{ @@ -662,14 +658,15 @@ static bool detect_dp( return true; }

• 	if (link->dpcd_caps.dongle_type !=
  

DISPLAY_DONGLE_DP_HDMI_CONVERTER)

• 	if (link->dpcd_caps.dongle_type !=
  

• 	    DISPLAY_DONGLE_DP_HDMI_CONVERTER)
  	*converter_disable_audio = true;
  

  } } else { /* DP passive dongles */ sink_caps->signal = dp_passive_dongle_detection(link->ddc,

• 		sink_caps,
  

• 		audio_support);
  

• 						sink_caps,
  

• audio_support); }

  return true;

-- 2.25.0

---

amd-gfx mailing list amd-gfx@lists.freedesktop.org