Re: [PATCH v4] drm/amd/display: Add MST atomic routines

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2019-10-17 at 12:52 -0400, mikita.lipski@xxxxxxx wrote:
> From: Mikita Lipski <mikita.lipski@xxxxxxx>
> 
> - Adding encoder atomic check to find vcpi slots for a connector
> - Using DRM helper functions to calculate PBN
> - Adding connector atomic check to release vcpi slots if connector
> loses CRTC
> - Calculate  PBN and VCPI slots only once during atomic
> check and store them on crtc_state to eliminate
> redundant calculation
> - Call drm_dp_mst_atomic_check to verify validity of MST topology
> during state atomic check
> 
> v2:
> - squashed previous 3 separate patches
> - removed DSC PBN calculation,
> - added PBN and VCPI slots properties to amdgpu connector
> 
> v3:
> - moved vcpi_slots and pbn properties to dm_crtc_state and dc_stream_state
> - updates stream's vcpi_slots and pbn on commit
> - separated patch from the DSC MST series
> 
> v4:
> - set vcpi_slots and pbn properties to dm_connector_state
> - copy porperties from connector state on to crtc state
> 
> Cc: Jerry Zuo <Jerry.Zuo@xxxxxxx>
> Cc: Harry Wentland <harry.wentland@xxxxxxx>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>
> Cc: Lyude Paul <lyude@xxxxxxxxxx>
> Signed-off-by: Mikita Lipski <mikita.lipski@xxxxxxx>
> ---
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 72 +++++++++++++++++--
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h |  6 ++
>  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 42 +----------
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   | 32 +++++++++
>  drivers/gpu/drm/amd/display/dc/dc_stream.h    |  3 +
>  5 files changed, 112 insertions(+), 43 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 10cce584719f..1f1146a4e85e 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -3811,6 +3811,11 @@ create_stream_for_sink(struct amdgpu_dm_connector
> *aconnector,
>  
>  	update_stream_signal(stream, sink);
>  
> +	if(dm_state){

nit: if (dm_state) {

But that's it! The rest of this looks good to me. With that nitpick addressed:

Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx>
> +		stream->vcpi_slots = dm_state->vcpi_slots;
> +		stream->pbn = dm_state->pbn;
> +	}
> +
>  	if (stream->signal == SIGNAL_TYPE_HDMI_TYPE_A)
>  		mod_build_hf_vsif_infopacket(stream, &stream->vsp_infopacket,
> false, false);
>  
> @@ -3889,6 +3894,8 @@ dm_crtc_duplicate_state(struct drm_crtc *crtc)
>  	state->crc_src = cur->crc_src;
>  	state->cm_has_degamma = cur->cm_has_degamma;
>  	state->cm_is_degamma_srgb = cur->cm_is_degamma_srgb;
> +	state->vcpi_slots = cur->vcpi_slots;
> +	state->pbn = cur->pbn;
>  
>  	/* TODO Duplicate dc_stream after objects are stream object is
> flattened */
>  
> @@ -4157,7 +4164,8 @@ void amdgpu_dm_connector_funcs_reset(struct
> drm_connector *connector)
>  		state->underscan_hborder = 0;
>  		state->underscan_vborder = 0;
>  		state->base.max_requested_bpc = 8;
> -
> +		state->vcpi_slots = 0;
> +		state->pbn = 0;
>  		if (connector->connector_type == DRM_MODE_CONNECTOR_eDP)
>  			state->abm_level = amdgpu_dm_abm_level;
>  
> @@ -4186,7 +4194,8 @@ amdgpu_dm_connector_atomic_duplicate_state(struct
> drm_connector *connector)
>  	new_state->underscan_enable = state->underscan_enable;
>  	new_state->underscan_hborder = state->underscan_hborder;
>  	new_state->underscan_vborder = state->underscan_vborder;
> -
> +	new_state->vcpi_slots = state->vcpi_slots;
> +	new_state->pbn = state->pbn;
>  	return &new_state->base;
>  }
>  
> @@ -4587,6 +4596,37 @@ static int dm_encoder_helper_atomic_check(struct
> drm_encoder *encoder,
>  					  struct drm_crtc_state *crtc_state,
>  					  struct drm_connector_state
> *conn_state)
>  {
> +	struct drm_atomic_state *state = crtc_state->state;
> +	struct drm_connector *connector = conn_state->connector;
> +	struct amdgpu_dm_connector *aconnector =
> to_amdgpu_dm_connector(connector);
> +	struct dm_connector_state *dm_new_connector_state =
> to_dm_connector_state(conn_state);
> +	const struct drm_display_mode *adjusted_mode = &crtc_state-
> >adjusted_mode;
> +	struct drm_dp_mst_topology_mgr *mst_mgr;
> +	struct drm_dp_mst_port *mst_port;
> +	int clock, bpp = 0;
> +
> +	if (!aconnector->port || !aconnector->dc_sink)
> +		return 0;
> +
> +	mst_port = aconnector->port;
> +	mst_mgr = &aconnector->mst_port->mst_mgr;
> +
> +	if (!crtc_state->connectors_changed && !crtc_state->mode_changed)
> +		return 0;
> +
> +	if(!state->duplicated) {
> +		bpp = (uint8_t)connector->display_info.bpc * 3;
> +		clock = adjusted_mode->clock;
> +		dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock,
> bpp);
> +	}
> +	dm_new_connector_state->vcpi_slots =
> drm_dp_atomic_find_vcpi_slots(state,
> +							       mst_mgr,
> +							       mst_port,
> +							       dm_new_connecto
> r_state->pbn);
> +	if (dm_new_connector_state->vcpi_slots < 0) {
> +		DRM_DEBUG_ATOMIC("failed finding vcpi slots: %d\n",
> dm_new_connector_state->vcpi_slots);
> +		return dm_new_connector_state->vcpi_slots;
> +	}
>  	return 0;
>  }
>  
> @@ -6127,6 +6167,9 @@ static void amdgpu_dm_commit_planes(struct
> drm_atomic_state *state,
>  				acrtc_state->stream->out_transfer_func;
>  		}
>  
> +		acrtc_state->stream->vcpi_slots = acrtc_state->vcpi_slots;
> +		acrtc_state->stream->pbn = acrtc_state->pbn;
> +
>  		acrtc_state->stream->abm_level = acrtc_state->abm_level;
>  		if (acrtc_state->abm_level != dm_old_crtc_state->abm_level)
>  			bundle->stream_update.abm_level = &acrtc_state-
> >abm_level;
> @@ -6527,7 +6570,7 @@ static void amdgpu_dm_atomic_commit_tail(struct
> drm_atomic_state *state)
>  		struct dc_stream_update stream_update;
>  		struct dc_info_packet hdr_packet;
>  		struct dc_stream_status *status = NULL;
> -		bool abm_changed, hdr_changed, scaling_changed;
> +		bool abm_changed, hdr_changed, scaling_changed,
> mst_vcpi_changed;
>  
>  		memset(&dummy_updates, 0, sizeof(dummy_updates));
>  		memset(&stream_update, 0, sizeof(stream_update));
> @@ -6553,7 +6596,12 @@ static void amdgpu_dm_atomic_commit_tail(struct
> drm_atomic_state *state)
>  		hdr_changed =
>  			is_hdr_metadata_different(old_con_state,
> new_con_state);
>  
> -		if (!scaling_changed && !abm_changed && !hdr_changed)
> +		mst_vcpi_changed = (dm_new_crtc_state->vcpi_slots !=
> +				   dm_old_crtc_state->vcpi_slots) ||
> +				   (dm_new_crtc_state->pbn !=
> +				   dm_old_crtc_state->pbn);
> +
> +		if (!scaling_changed && !abm_changed && !hdr_changed &&
> !mst_vcpi_changed)
>  			continue;
>  
>  		stream_update.stream = dm_new_crtc_state->stream;
> @@ -6576,6 +6624,11 @@ static void amdgpu_dm_atomic_commit_tail(struct
> drm_atomic_state *state)
>  			stream_update.hdr_static_metadata = &hdr_packet;
>  		}
>  
> +		if (mst_vcpi_changed) {
> +			dm_new_crtc_state->stream->vcpi_slots =
> dm_new_crtc_state->vcpi_slots;
> +			dm_new_crtc_state->stream->pbn = dm_new_crtc_state-
> >pbn;
> +		}
> +
>  		status = dc_stream_get_status(dm_new_crtc_state->stream);
>  		WARN_ON(!status);
>  		WARN_ON(!status->plane_count);
> @@ -6924,6 +6977,9 @@ static int dm_update_crtc_state(struct
> amdgpu_display_manager *dm,
>  
>  		dm_new_crtc_state->abm_level = dm_new_conn_state->abm_level;
>  
> +		dm_new_crtc_state->vcpi_slots = dm_new_conn_state->vcpi_slots;
> +		dm_new_crtc_state->pbn = dm_new_conn_state->pbn;
> +
>  		ret = fill_hdr_info_packet(drm_new_conn_state,
>  					   &new_stream->hdr_static_metadata);
>  		if (ret)
> @@ -7062,6 +7118,9 @@ static int dm_update_crtc_state(struct
> amdgpu_display_manager *dm,
>  	/* ABM settings */
>  	dm_new_crtc_state->abm_level = dm_new_conn_state->abm_level;
>  
> +	dm_new_crtc_state->vcpi_slots = dm_new_conn_state->vcpi_slots;
> +	dm_new_crtc_state->pbn = dm_new_conn_state->pbn;
> +
>  	/*
>  	 * Color management settings. We also update color properties
>  	 * when a modeset is needed, to ensure it gets reprogrammed.
> @@ -7606,6 +7665,11 @@ static int amdgpu_dm_atomic_check(struct drm_device
> *dev,
>  	if (ret)
>  		goto fail;
>  
> +	/* Perform validation of MST topology in the state*/
> +	ret = drm_dp_mst_atomic_check(state);
> +	if (ret)
> +		goto fail;
> +
>  	if (state->legacy_cursor_update) {
>  		/*
>  		 * This is a fast cursor update coming from the plane update
> 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 c6fdebee7189..6e85c50820f8 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h
> @@ -338,6 +338,10 @@ struct dm_crtc_state {
>  	struct dc_info_packet vrr_infopacket;
>  
>  	int abm_level;
> +
> +	/* MST specific */
> +	int vcpi_slots;
> +	int pbn;
>  };
>  
>  #define to_dm_crtc_state(x) container_of(x, struct dm_crtc_state, base)
> @@ -360,6 +364,8 @@ struct dm_connector_state {
>  	bool freesync_enable;
>  	bool freesync_capable;
>  	uint8_t abm_level;
> +	uint64_t vcpi_slots;
> +	uint64_t pbn;
>  };
>  
>  #define to_dm_connector_state(x)\
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> index 11e5784aa62a..21e364c92df5 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c
> @@ -184,11 +184,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
>  	struct amdgpu_dm_connector *aconnector;
>  	struct drm_dp_mst_topology_mgr *mst_mgr;
>  	struct drm_dp_mst_port *mst_port;
> -	int slots = 0;
>  	bool ret;
> -	int clock;
> -	int bpp = 0;
> -	int pbn = 0;
>  
>  	aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context;
>  
> @@ -203,42 +199,10 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
>  	mst_port = aconnector->port;
>  
>  	if (enable) {
> -		clock = stream->timing.pix_clk_100hz / 10;
> -
> -		switch (stream->timing.display_color_depth) {
> -
> -		case COLOR_DEPTH_666:
> -			bpp = 6;
> -			break;
> -		case COLOR_DEPTH_888:
> -			bpp = 8;
> -			break;
> -		case COLOR_DEPTH_101010:
> -			bpp = 10;
> -			break;
> -		case COLOR_DEPTH_121212:
> -			bpp = 12;
> -			break;
> -		case COLOR_DEPTH_141414:
> -			bpp = 14;
> -			break;
> -		case COLOR_DEPTH_161616:
> -			bpp = 16;
> -			break;
> -		default:
> -			ASSERT(bpp != 0);
> -			break;
> -		}
> -
> -		bpp = bpp * 3;
> -
> -		/* TODO need to know link rate */
> -
> -		pbn = drm_dp_calc_pbn_mode(clock, bpp);
> -
> -		slots = drm_dp_find_vcpi_slots(mst_mgr, pbn);
> -		ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, pbn, slots);
>  
> +		ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port,
> +					       stream->pbn,
> +					       stream->vcpi_slots);
>  		if (!ret)
>  			return false;
>  
> 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 3af2b429ff1b..7f3ce29bd14c 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
> @@ -250,10 +250,42 @@ dm_mst_atomic_best_encoder(struct drm_connector
> *connector,
>  	return &to_amdgpu_dm_connector(connector)->mst_encoder->base;
>  }
>  
> +static int dm_dp_mst_atomic_check(struct drm_connector *connector,
> +				struct drm_atomic_state *state)
> +{
> +	struct drm_connector_state *new_conn_state =
> +			drm_atomic_get_new_connector_state(state, connector);
> +	struct drm_connector_state *old_conn_state =
> +			drm_atomic_get_old_connector_state(state, connector);
> +	struct amdgpu_dm_connector *aconnector =
> to_amdgpu_dm_connector(connector);
> +	struct drm_crtc_state *new_crtc_state;
> +	struct drm_dp_mst_topology_mgr *mst_mgr;
> +	struct drm_dp_mst_port *mst_port;
> +
> +	mst_port = aconnector->port;
> +	mst_mgr = &aconnector->mst_port->mst_mgr;
> +
> +	if (!old_conn_state->crtc)
> +		return 0;
> +
> +	if (new_conn_state->crtc) {
> +		new_crtc_state = drm_atomic_get_old_crtc_state(state,
> new_conn_state->crtc);
> +		if (!new_crtc_state ||
> +		    !drm_atomic_crtc_needs_modeset(new_crtc_state) ||
> +		    new_crtc_state->enable)
> +			return 0;
> +	}
> +
> +	return drm_dp_atomic_release_vcpi_slots(state,
> +						mst_mgr,
> +						mst_port);
> +}
> +
>  static const struct drm_connector_helper_funcs
> dm_dp_mst_connector_helper_funcs = {
>  	.get_modes = dm_dp_mst_get_modes,
>  	.mode_valid = amdgpu_dm_connector_mode_valid,
>  	.atomic_best_encoder = dm_mst_atomic_best_encoder,
> +	.atomic_check = dm_dp_mst_atomic_check,
>  };
>  
>  static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder)
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> index fdb6adc37857..010041096505 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> @@ -208,6 +208,9 @@ struct dc_stream_state {
>  	unsigned int num_wb_info;
>  	struct dc_writeback_info writeback_info[MAX_DWB_PIPES];
>  #endif
> +
> +	unsigned int vcpi_slots;
> +	unsigned int pbn;
>  	/* Computed state bits */
>  	bool mode_changed : 1;
>  
-- 
Cheers,
	Lyude Paul

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux