Re: [PATCH] drm/amdgpu/display: create fake mst encoders ahead of time (v2)

["Kazlauskas, Nicholas" <nicholas.kazlauskas@xxxxxxx>]

On 2020-07-10 9:58 a.m., Alex Deucher wrote:
> Prevents a warning in the MST create connector case.
>
> v2: create global fake encoders rather per connector fake encoders
> to avoid running out of encoder indices.
>
> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1108
> Fixes: c6385e503aeaf9 ("drm/amdgpu: drop legacy drm load and unload callbacks")
> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>

I thought it was rather odd that the last patch was creating 6 per 
connector even though we were only using one.

Makes a lot more sense to be on the adev instead.

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>

Regards,
Nicholas Kazlauskas

> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  9 ++++
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 11 +++-
>   .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 53 +++++++++----------
>   .../display/amdgpu_dm/amdgpu_dm_mst_types.h   |  3 ++
>   4 files changed, 48 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 0e5d99a85307..74d8e61f30e4 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -978,6 +978,9 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>   	/* Update the actual used number of crtc */
>   	adev->mode_info.num_crtc = adev->dm.display_indexes_num;
>   
> +	/* create fake encoders for MST */
> +	dm_dp_create_fake_mst_encoders(adev);
> +
>   	/* TODO: Add_display_info? */
>   
>   	/* TODO use dynamic cursor width */
> @@ -1001,6 +1004,12 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
>   
>   static void amdgpu_dm_fini(struct amdgpu_device *adev)
>   {
> +	int i;
> +
> +	for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
> +		drm_encoder_cleanup(&adev->dm.mst_encoders[i].base);
> +	}
> +
>   	amdgpu_dm_audio_fini(adev);
>   
>   	amdgpu_dm_destroy_drm_device(&adev->dm);
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> index 86c132ddc452..3f50328fe537 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -43,6 +43,9 @@
>    */
>   
>   #define AMDGPU_DM_MAX_DISPLAY_INDEX 31
> +
> +#define AMDGPU_DM_MAX_CRTC 6
> +
>   /*
>   #include "include/amdgpu_dal_power_if.h"
>   #include "amdgpu_dm_irq.h"
> @@ -330,6 +333,13 @@ struct amdgpu_display_manager {
>   	 * available in FW
>   	 */
>   	const struct gpu_info_soc_bounding_box_v1_0 *soc_bounding_box;
> +
> +	/**
> +	 * @mst_encoders:
> +	 *
> +	 * fake encoders used for DP MST.
> +	 */
> +	struct amdgpu_encoder mst_encoders[AMDGPU_DM_MAX_CRTC];
>   };
>   
>   struct amdgpu_dm_connector {
> @@ -358,7 +368,6 @@ struct amdgpu_dm_connector {
>   	struct amdgpu_dm_dp_aux dm_dp_aux;
>   	struct drm_dp_mst_port *port;
>   	struct amdgpu_dm_connector *mst_port;
> -	struct amdgpu_encoder *mst_encoder;
>   	struct drm_dp_aux *dsc_aux;
>   
>   	/* TODO see if we can merge with ddc_bus or make a dm_connector */
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> index cf15248739f7..176973da18ef 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> @@ -95,7 +95,6 @@ dm_dp_mst_connector_destroy(struct drm_connector *connector)
>   {
>   	struct amdgpu_dm_connector *aconnector =
>   		to_amdgpu_dm_connector(connector);
> -	struct amdgpu_encoder *amdgpu_encoder = aconnector->mst_encoder;
>   
>   	if (aconnector->dc_sink) {
>   		dc_link_remove_remote_sink(aconnector->dc_link,
> @@ -105,8 +104,6 @@ dm_dp_mst_connector_destroy(struct drm_connector *connector)
>   
>   	kfree(aconnector->edid);
>   
> -	drm_encoder_cleanup(&amdgpu_encoder->base);
> -	kfree(amdgpu_encoder);
>   	drm_connector_cleanup(connector);
>   	drm_dp_mst_put_port_malloc(aconnector->port);
>   	kfree(aconnector);
> @@ -243,7 +240,11 @@ static struct drm_encoder *
>   dm_mst_atomic_best_encoder(struct drm_connector *connector,
>   			   struct drm_connector_state *connector_state)
>   {
> -	return &to_amdgpu_dm_connector(connector)->mst_encoder->base;
> +	struct drm_device *dev = connector->dev;
> +	struct amdgpu_device *adev = dev->dev_private;
> +	struct amdgpu_crtc *acrtc = to_amdgpu_crtc(connector_state->crtc);
> +
> +	return &adev->dm.mst_encoders[acrtc->crtc_id].base;
>   }
>   
>   static int
> @@ -306,31 +307,27 @@ static const struct drm_encoder_funcs amdgpu_dm_encoder_funcs = {
>   	.destroy = amdgpu_dm_encoder_destroy,
>   };
>   
> -static struct amdgpu_encoder *
> -dm_dp_create_fake_mst_encoder(struct amdgpu_dm_connector *connector)
> +void
> +dm_dp_create_fake_mst_encoders(struct amdgpu_device *adev)
>   {
> -	struct drm_device *dev = connector->base.dev;
> -	struct amdgpu_device *adev = dev->dev_private;
> -	struct amdgpu_encoder *amdgpu_encoder;
> -	struct drm_encoder *encoder;
> -
> -	amdgpu_encoder = kzalloc(sizeof(*amdgpu_encoder), GFP_KERNEL);
> -	if (!amdgpu_encoder)
> -		return NULL;
> +	struct drm_device *dev = adev->ddev;
> +	int i;
>   
> -	encoder = &amdgpu_encoder->base;
> -	encoder->possible_crtcs = amdgpu_dm_get_encoder_crtc_mask(adev);
> +	for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
> +		struct amdgpu_encoder *amdgpu_encoder = &adev->dm.mst_encoders[i];
> +		struct drm_encoder *encoder = &amdgpu_encoder->base;
>   
> -	drm_encoder_init(
> -		dev,
> -		&amdgpu_encoder->base,
> -		&amdgpu_dm_encoder_funcs,
> -		DRM_MODE_ENCODER_DPMST,
> -		NULL);
> +		encoder->possible_crtcs = amdgpu_dm_get_encoder_crtc_mask(adev);
>   
> -	drm_encoder_helper_add(encoder, &amdgpu_dm_encoder_helper_funcs);
> +		drm_encoder_init(
> +			dev,
> +			&amdgpu_encoder->base,
> +			&amdgpu_dm_encoder_funcs,
> +			DRM_MODE_ENCODER_DPMST,
> +			NULL);
>   
> -	return amdgpu_encoder;
> +		drm_encoder_helper_add(encoder, &amdgpu_dm_encoder_helper_funcs);
> +	}
>   }
>   
>   static struct drm_connector *
> @@ -343,6 +340,7 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
>   	struct amdgpu_device *adev = dev->dev_private;
>   	struct amdgpu_dm_connector *aconnector;
>   	struct drm_connector *connector;
> +	int i;
>   
>   	aconnector = kzalloc(sizeof(*aconnector), GFP_KERNEL);
>   	if (!aconnector)
> @@ -369,9 +367,10 @@ dm_dp_add_mst_connector(struct drm_dp_mst_topology_mgr *mgr,
>   		master->dc_link,
>   		master->connector_id);
>   
> -	aconnector->mst_encoder = dm_dp_create_fake_mst_encoder(master);
> -	drm_connector_attach_encoder(&aconnector->base,
> -				     &aconnector->mst_encoder->base);
> +	for (i = 0; i < AMDGPU_DM_MAX_CRTC; i++) {
> +		drm_connector_attach_encoder(&aconnector->base,
> +					     &adev->dm.mst_encoders[i].base);
> +	}
>   
>   	connector->max_bpc_property = master->base.max_bpc_property;
>   	if (connector->max_bpc_property)
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h
> index d2c56579a2cc..b38bd68121ce 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h
> @@ -35,6 +35,9 @@ void amdgpu_dm_initialize_dp_connector(struct amdgpu_display_manager *dm,
>   				       struct amdgpu_dm_connector *aconnector,
>   				       int link_index);
>   
> +void
> +dm_dp_create_fake_mst_encoders(struct amdgpu_device *adev);
> +
>   #if defined(CONFIG_DRM_AMD_DC_DCN)
>   bool compute_mst_dsc_configs_for_state(struct drm_atomic_state *state,
>   				       struct dc_state *dc_state);

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx