Re: [PATCH v2 01/23] drm/amd/display: Update link encoder object creation.

[Harry Wentland <harry.wentland@amd.com>]

On 2021-10-05 03:51, Wayne Lin wrote:
> From: Jimmy Kizito <Jimmy.Kizito@amd.com>
> 
> [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.

Harry

> Reviewed-by: Jun Lei <Jun.Lei@amd.com>
> Acked-by: Wayne Lin <Wayne.Lin@amd.com>
> Acked-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Signed-off-by: Jimmy Kizito <Jimmy.Kizito@amd.com>
> ---
>  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;
>