On 2021-10-05 10:15, Harry Wentland wrote: > > > On 2021-10-05 03:51, Wayne Lin wrote: >> From: Jimmy Kizito <Jimmy.Kizito@xxxxxxx> >> >> [Why & How] >> Add codes for commit "e1f4328f22c0 drm/amd/display: Update link >> encoder object creation" to support USB4 DP tunneling feature >> > > It looks like all of Jimmy's patches refer to porting code from some other > patches. Do those patches have a patch description? > > Basically, above "Why & How" description doesn't describe why this > change is made and how it is done. Please provide that description. > > Same with other patches in this set that talk about "Add codes..." > without providing an actual description. > A decent description for this would be something like this (note that I'm not a USB4 expert, so keeping this purposely high-level): "USB4 endpoints are dynamically mapped. We create additional link encoders for USB4 use when DC is created and destroy them when DC is destructed." Harry > Harry > >> Reviewed-by: Jun Lei <Jun.Lei@xxxxxxx> >> Acked-by: Wayne Lin <Wayne.Lin@xxxxxxx> >> Acked-by: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx> >> Signed-off-by: Jimmy Kizito <Jimmy.Kizito@xxxxxxx> >> --- >> drivers/gpu/drm/amd/display/dc/core/dc.c | 77 +++++++++++++++++++ >> .../gpu/drm/amd/display/dc/inc/core_types.h | 2 + >> drivers/gpu/drm/amd/display/dc/inc/resource.h | 1 + >> 3 files changed, 80 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c >> index 8e0bcd4fd000..673fb0ef7a89 100644 >> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c >> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c >> @@ -296,6 +296,75 @@ static bool create_links( >> return false; >> } >> >> +/* Create additional DIG link encoder objects if fewer than the platform >> + * supports were created during link construction. This can happen if the >> + * number of physical connectors is less than the number of DIGs. >> + */ >> +static bool create_link_encoders(struct dc *dc) >> +{ >> + bool res = true; >> + unsigned int num_usb4_dpia = dc->res_pool->res_cap->num_usb4_dpia; >> + unsigned int num_dig_link_enc = dc->res_pool->res_cap->num_dig_link_enc; >> + int i; >> + >> + /* A platform without USB4 DPIA endpoints has a fixed mapping between DIG >> + * link encoders and physical display endpoints and does not require >> + * additional link encoder objects. >> + */ >> + if (num_usb4_dpia == 0) >> + return res; >> + >> + /* Create as many link encoder objects as the platform supports. DPIA >> + * endpoints can be programmably mapped to any DIG. >> + */ >> + if (num_dig_link_enc > dc->res_pool->dig_link_enc_count) { >> + for (i = 0; i < num_dig_link_enc; i++) { >> + struct link_encoder *link_enc = dc->res_pool->link_encoders[i]; >> + >> + if (!link_enc && dc->res_pool->funcs->link_enc_create_minimal) { >> + link_enc = dc->res_pool->funcs->link_enc_create_minimal(dc->ctx, >> + (enum engine_id)(ENGINE_ID_DIGA + i)); >> + if (link_enc) { >> + dc->res_pool->link_encoders[i] = link_enc; >> + dc->res_pool->dig_link_enc_count++; >> + } else { >> + res = false; >> + } >> + } >> + } >> + } >> + >> + return res; >> +} >> + >> +/* Destroy any additional DIG link encoder objects created by >> + * create_link_encoders(). >> + * NB: Must only be called after destroy_links(). >> + */ >> +static void destroy_link_encoders(struct dc *dc) >> +{ >> + unsigned int num_usb4_dpia = dc->res_pool->res_cap->num_usb4_dpia; >> + unsigned int num_dig_link_enc = dc->res_pool->res_cap->num_dig_link_enc; >> + int i; >> + >> + /* A platform without USB4 DPIA endpoints has a fixed mapping between DIG >> + * link encoders and physical display endpoints and does not require >> + * additional link encoder objects. >> + */ >> + if (num_usb4_dpia == 0) >> + return; >> + >> + for (i = 0; i < num_dig_link_enc; i++) { >> + struct link_encoder *link_enc = dc->res_pool->link_encoders[i]; >> + >> + if (link_enc) { >> + link_enc->funcs->destroy(&link_enc); >> + dc->res_pool->link_encoders[i] = NULL; >> + dc->res_pool->dig_link_enc_count--; >> + } >> + } >> +} >> + >> static struct dc_perf_trace *dc_perf_trace_create(void) >> { >> return kzalloc(sizeof(struct dc_perf_trace), GFP_KERNEL); >> @@ -729,6 +798,8 @@ static void dc_destruct(struct dc *dc) >> >> destroy_links(dc); >> >> + destroy_link_encoders(dc); >> + >> if (dc->clk_mgr) { >> dc_destroy_clk_mgr(dc->clk_mgr); >> dc->clk_mgr = NULL; >> @@ -933,6 +1004,12 @@ static bool dc_construct(struct dc *dc, >> if (!create_links(dc, init_params->num_virtual_links)) >> goto fail; >> >> + /* Create additional DIG link encoder objects if fewer than the platform >> + * supports were created during link construction. >> + */ >> + if (!create_link_encoders(dc)) >> + goto fail; >> + >> /* Initialise DIG link encoder resource tracking variables. */ >> link_enc_cfg_init(dc, dc->current_state); >> >> diff --git a/drivers/gpu/drm/amd/display/dc/inc/core_types.h b/drivers/gpu/drm/amd/display/dc/inc/core_types.h >> index 0fea258c6db3..ed09af238911 100644 >> --- a/drivers/gpu/drm/amd/display/dc/inc/core_types.h >> +++ b/drivers/gpu/drm/amd/display/dc/inc/core_types.h >> @@ -245,6 +245,8 @@ struct resource_pool { >> * entries in link_encoders array. >> */ >> unsigned int dig_link_enc_count; >> + /* Number of USB4 DPIA (DisplayPort Input Adapter) link objects created.*/ >> + unsigned int usb4_dpia_count; >> >> #if defined(CONFIG_DRM_AMD_DC_DCN) >> unsigned int hpo_dp_stream_enc_count; >> diff --git a/drivers/gpu/drm/amd/display/dc/inc/resource.h b/drivers/gpu/drm/amd/display/dc/inc/resource.h >> index 3fbda9d7e257..372c0898facd 100644 >> --- a/drivers/gpu/drm/amd/display/dc/inc/resource.h >> +++ b/drivers/gpu/drm/amd/display/dc/inc/resource.h >> @@ -49,6 +49,7 @@ struct resource_caps { >> int num_vmid; >> int num_dsc; >> unsigned int num_dig_link_enc; // Total number of DIGs (digital encoders) in DIO (Display Input/Output). >> + unsigned int num_usb4_dpia; // Total number of USB4 DPIA (DisplayPort Input Adapters). >> #if defined(CONFIG_DRM_AMD_DC_DCN) >> int num_hpo_dp_stream_encoder; >> int num_hpo_dp_link_encoder; >> >