Re: [Patch v2 2/3] drm/mst: Refactor the flow for payload allocation/removement

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

 



Two small comments:

On Mon, 2023-08-07 at 10:56 +0800, Wayne Lin wrote:
> [Why]
> Today, the allocation/deallocation steps and status is a bit unclear.
> 
> For instance, payload->vc_start_slot = -1 stands for "the failure of
> updating DPCD payload ID table" and can also represent as "payload is not
> allocated yet". These two cases should be handled differently and hence
> better to distinguish them for better understanding.
> 
> [How]
> Define enumeration - ALLOCATION_LOCAL, ALLOCATION_DFP and ALLOCATION_REMOTE
> to distinguish different allocation status. Adjust the code to handle
> different status accordingly for better understanding the sequence of
> payload allocation and payload removement.
> 
> For payload creation, the procedure should look like this:
> DRM part 1:
> * step 1 - update sw mst mgr variables to add a new payload
> * step 2 - add payload at immediate DFP DPCD payload table
> 
> Driver:
> * Add new payload in HW and sync up with DFP by sending ACT
> 
> DRM Part 2:
> * Send ALLOCATE_PAYLOAD sideband message to allocate bandwidth along the
>   virtual channel.
> 
> And as for payload removement, the procedure should look like this:
> DRM part 1:
> * step 1 - Send ALLOCATE_PAYLOAD sideband message to release bandwidth
>            along the virtual channel
> * step 2 - Clear payload allocation at immediate DFP DPCD payload table
> 
> Driver:
> * Remove the payload in HW and sync up with DFP by sending ACT
> 
> DRM part 2:
> * update sw mst mgr variables to remove the payload
> 
> Note that it's fine to fail when communicate with the branch device
> connected at immediate downstrean-facing port, but updating variables of
> SW mst mgr and HW configuration should be conducted anyway. That's because
> it's under commit_tail and we need to complete the HW programming.
> 
> Changes since v1:
> * Remove the set but not use variable 'old_payload' in function
>   'nv50_msto_prepare'. Catched by kernel test robot <lkp@xxxxxxxxx>
> 
> Signed-off-by: Wayne Lin <Wayne.Lin@xxxxxxx>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c |  20 ++-
>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 159 +++++++++++-------
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |  18 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c       |  21 +--
>  include/drm/display/drm_dp_mst_helper.h       |  23 ++-
>  5 files changed, 153 insertions(+), 88 deletions(-)
> 
> 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 d9a482908380..9ad509279b0a 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
> @@ -219,7 +219,7 @@ static void dm_helpers_construct_old_payload(
>  	/* Set correct time_slots/PBN of old payload.
>  	 * other fields (delete & dsc_enabled) in
>  	 * struct drm_dp_mst_atomic_payload are don't care fields
> -	 * while calling drm_dp_remove_payload()
> +	 * while calling drm_dp_remove_payload_part2()
>  	 */
>  	for (i = 0; i < current_link_table.stream_count; i++) {
>  		dc_alloc =
> @@ -262,13 +262,12 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
>  
>  	mst_mgr = &aconnector->mst_root->mst_mgr;
>  	mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
> -
> -	/* It's OK for this to fail */
>  	new_payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port);
>  
>  	if (enable) {
>  		target_payload = new_payload;
>  
> +		/* It's OK for this to fail */
>  		drm_dp_add_payload_part1(mst_mgr, mst_state, new_payload);
>  	} else {
>  		/* construct old payload by VCPI*/
> @@ -276,7 +275,7 @@ bool dm_helpers_dp_mst_write_payload_allocation_table(
>  						new_payload, &old_payload);
>  		target_payload = &old_payload;
>  
> -		drm_dp_remove_payload(mst_mgr, mst_state, &old_payload, new_payload);
> +		drm_dp_remove_payload_part1(mst_mgr, mst_state, new_payload);
>  	}
>  
>  	/* mst_mgr->->payloads are VC payload notify MST branch using DPCD or
> @@ -342,7 +341,7 @@ bool dm_helpers_dp_mst_send_payload_allocation(
>  	struct amdgpu_dm_connector *aconnector;
>  	struct drm_dp_mst_topology_state *mst_state;
>  	struct drm_dp_mst_topology_mgr *mst_mgr;
> -	struct drm_dp_mst_atomic_payload *payload;
> +	struct drm_dp_mst_atomic_payload *new_payload, *old_payload;
>  	enum mst_progress_status set_flag = MST_ALLOCATE_NEW_PAYLOAD;
>  	enum mst_progress_status clr_flag = MST_CLEAR_ALLOCATED_PAYLOAD;
>  	int ret = 0;
> @@ -355,15 +354,20 @@ bool dm_helpers_dp_mst_send_payload_allocation(
>  	mst_mgr = &aconnector->mst_root->mst_mgr;
>  	mst_state = to_drm_dp_mst_topology_state(mst_mgr->base.state);
>  
> -	payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port);
> +	new_payload = drm_atomic_get_mst_payload_state(mst_state, aconnector->mst_output_port);
>  
>  	if (!enable) {
>  		set_flag = MST_CLEAR_ALLOCATED_PAYLOAD;
>  		clr_flag = MST_ALLOCATE_NEW_PAYLOAD;
>  	}
>  
> -	if (enable)
> -		ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, payload);
> +	if (enable) {
> +		ret = drm_dp_add_payload_part2(mst_mgr, mst_state->base.state, new_payload);
> +	} else {
> +		dm_helpers_construct_old_payload(stream->link, mst_state->pbn_div,
> +						 new_payload, old_payload);
> +		drm_dp_remove_payload_part2(mst_mgr, mst_state, old_payload, new_payload);
> +	}
>  
>  	if (ret) {
>  		amdgpu_dm_set_mst_status(&aconnector->mst_status,
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index 4d80426757ab..e04f87ff755a 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -3255,15 +3255,15 @@ int drm_dp_send_query_stream_enc_status(struct drm_dp_mst_topology_mgr *mgr,
>  }
>  EXPORT_SYMBOL(drm_dp_send_query_stream_enc_status);
>  
> -static int drm_dp_create_payload_step1(struct drm_dp_mst_topology_mgr *mgr,
> -				       struct drm_dp_mst_atomic_payload *payload)
> +static int drm_dp_create_payload_at_dfp(struct drm_dp_mst_topology_mgr *mgr,
> +					struct drm_dp_mst_atomic_payload *payload)
>  {
>  	return drm_dp_dpcd_write_payload(mgr, payload->vcpi, payload->vc_start_slot,
>  					 payload->time_slots);
>  }
>  
> -static int drm_dp_create_payload_step2(struct drm_dp_mst_topology_mgr *mgr,
> -				       struct drm_dp_mst_atomic_payload *payload)
> +static int drm_dp_create_payload_to_remote(struct drm_dp_mst_topology_mgr *mgr,
> +					   struct drm_dp_mst_atomic_payload *payload)
>  {
>  	int ret;
>  	struct drm_dp_mst_port *port = drm_dp_mst_topology_get_port_validated(mgr, payload->port);
> @@ -3276,17 +3276,20 @@ static int drm_dp_create_payload_step2(struct drm_dp_mst_topology_mgr *mgr,
>  	return ret;
>  }
>  
> -static int drm_dp_destroy_payload_step1(struct drm_dp_mst_topology_mgr *mgr,
> -					struct drm_dp_mst_topology_state *mst_state,
> -					struct drm_dp_mst_atomic_payload *payload)
> +static void drm_dp_destroy_payload_at_remote_and_dfp(struct drm_dp_mst_topology_mgr *mgr,
> +						     struct drm_dp_mst_topology_state *mst_state,
> +						     struct drm_dp_mst_atomic_payload *payload)
>  {
>  	drm_dbg_kms(mgr->dev, "\n");
>  
>  	/* it's okay for these to fail */
> -	drm_dp_payload_send_msg(mgr, payload->port, payload->vcpi, 0);
> -	drm_dp_dpcd_write_payload(mgr, payload->vcpi, payload->vc_start_slot, 0);
> +	if (payload->payload_allocation_status == DRM_DP_MST_PAYLOAD_ALLOCATION_REMOTE) {
> +		drm_dp_payload_send_msg(mgr, payload->port, payload->vcpi, 0);
> +		payload->payload_allocation_status = DRM_DP_MST_PAYLOAD_ALLOCATION_DFP;
> +	}
>  
> -	return 0;
> +	if (payload->payload_allocation_status == DRM_DP_MST_PAYLOAD_ALLOCATION_DFP)
> +		drm_dp_dpcd_write_payload(mgr, payload->vcpi, payload->vc_start_slot, 0);
>  }
>  
>  /**
> @@ -3296,81 +3299,105 @@ static int drm_dp_destroy_payload_step1(struct drm_dp_mst_topology_mgr *mgr,
>   * @payload: The payload to write
>   *
>   * Determines the starting time slot for the given payload, and programs the VCPI for this payload
> - * into hardware. After calling this, the driver should generate ACT and payload packets.
> + * into the DPCD of DPRX. After calling this, the driver should generate ACT and payload packets.
>   *
> - * Returns: 0 on success, error code on failure. In the event that this fails,
> - * @payload.vc_start_slot will also be set to -1.
> + * Returns: 0 on success, error code on failure.
>   */
>  int drm_dp_add_payload_part1(struct drm_dp_mst_topology_mgr *mgr,
>  			     struct drm_dp_mst_topology_state *mst_state,
>  			     struct drm_dp_mst_atomic_payload *payload)
>  {
>  	struct drm_dp_mst_port *port;
> -	int ret;
> +	int ret = 0;
> +	bool allocate = true;
> +
> +	/* Update mst mgr info */
> +	if (mgr->payload_count == 0)
> +		mgr->next_start_slot = mst_state->start_slot;
> +
> +	payload->vc_start_slot = mgr->next_start_slot;
> +
> +	mgr->payload_count++;
> +	mgr->next_start_slot += payload->time_slots;
>  
> +	/* Allocate payload to immediate downstream facing port */
>  	port = drm_dp_mst_topology_get_port_validated(mgr, payload->port);
>  	if (!port) {
>  		drm_dbg_kms(mgr->dev,
> -			    "VCPI %d for port %p not in topology, not creating a payload\n",
> +			    "VCPI %d for port %p not in topology, not creating a payload to remote\n",
>  			    payload->vcpi, payload->port);
> -		payload->vc_start_slot = -1;
> -		return 0;
> +		allocate = false;
>  	}
>  
> -	if (mgr->payload_count == 0)
> -		mgr->next_start_slot = mst_state->start_slot;
> -
> -	payload->vc_start_slot = mgr->next_start_slot;
> +	if (allocate) {
> +		ret = drm_dp_create_payload_at_dfp(mgr, payload);
> +		if (ret < 0)
> +			drm_warn(mgr->dev, "Failed to create MST payload for port %p: %d\n",
> +				 payload->port, ret);
>  
> -	ret = drm_dp_create_payload_step1(mgr, payload);
> -	drm_dp_mst_topology_put_port(port);
> -	if (ret < 0) {
> -		drm_warn(mgr->dev, "Failed to create MST payload for port %p: %d\n",
> -			 payload->port, ret);
> -		payload->vc_start_slot = -1;
> -		return ret;
>  	}
>  
> -	mgr->payload_count++;
> -	mgr->next_start_slot += payload->time_slots;
> +	payload->payload_allocation_status =
> +		(!allocate || ret < 0) ? DRM_DP_MST_PAYLOAD_ALLOCATION_LOCAL :
> +								DRM_DP_MST_PAYLOAD_ALLOCATION_DFP;

Just need to fix indenting here

>  
> -	return 0;
> +	drm_dp_mst_topology_put_port(port);
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(drm_dp_add_payload_part1);
>  
>  /**
> - * drm_dp_remove_payload() - Remove an MST payload
> + * drm_dp_remove_payload_part1() - Remove an MST payload along the virtual channel
>   * @mgr: Manager to use.
>   * @mst_state: The MST atomic state
> - * @old_payload: The payload with its old state
> - * @new_payload: The payload to write
> + * @payload: The payload to remove
>   *
> - * Removes a payload from an MST topology if it was successfully assigned a start slot. Also updates
> - * the starting time slots of all other payloads which would have been shifted towards the start of
> - * the VC table as a result. After calling this, the driver should generate ACT and payload packets.
> + * Removes a payload along the virtual channel if it was successfully allocated.
> + * After calling this, the driver should set HW to generate ACT and then switch to new
> + * payload allocation state.
>   */
> -void drm_dp_remove_payload(struct drm_dp_mst_topology_mgr *mgr,
> -			   struct drm_dp_mst_topology_state *mst_state,
> -			   const struct drm_dp_mst_atomic_payload *old_payload,
> -			   struct drm_dp_mst_atomic_payload *new_payload)
> +void drm_dp_remove_payload_part1(struct drm_dp_mst_topology_mgr *mgr,
> +				 struct drm_dp_mst_topology_state *mst_state,
> +				 struct drm_dp_mst_atomic_payload *payload)
>  {
> -	struct drm_dp_mst_atomic_payload *pos;
> +	/* Remove remote payload allocation */
>  	bool send_remove = false;
>  
> -	/* We failed to make the payload, so nothing to do */
> -	if (new_payload->vc_start_slot == -1)
> -		return;
> -
>  	mutex_lock(&mgr->lock);
> -	send_remove = drm_dp_mst_port_downstream_of_branch(new_payload->port, mgr->mst_primary);
> +	send_remove = drm_dp_mst_port_downstream_of_branch(payload->port, mgr->mst_primary);
>  	mutex_unlock(&mgr->lock);
>  
>  	if (send_remove)
> -		drm_dp_destroy_payload_step1(mgr, mst_state, new_payload);
> +		drm_dp_destroy_payload_at_remote_and_dfp(mgr, mst_state, payload);
>  	else
>  		drm_dbg_kms(mgr->dev, "Payload for VCPI %d not in topology, not sending remove\n",
> -			    new_payload->vcpi);
> +			    payload->vcpi);
> +
> +	payload->payload_allocation_status = DRM_DP_MST_PAYLOAD_ALLOCATION_LOCAL;
> +}
> +EXPORT_SYMBOL(drm_dp_remove_payload_part1);
>  
> +/**
> + * drm_dp_remove_payload_part2() - Remove an MST payload locally
> + * @mgr: Manager to use.
> + * @mst_state: The MST atomic state
> + * @old_payload: The payload with its old state
> + * @new_payload: The payload with its latest state
> + *
> + * Updates the starting time slots of all other payloads which would have been shifted towards
> + * the start of the payload ID table as a result of removing a payload. Driver should call this
> + * function whenever it removes a payload in its HW. It's independent to the result of payload
> + * allocation/deallocation at branch devices along the virtual channel.
> + */
> +void drm_dp_remove_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
> +				 struct drm_dp_mst_topology_state *mst_state,
> +				 const struct drm_dp_mst_atomic_payload *old_payload,
> +				 struct drm_dp_mst_atomic_payload *new_payload)
> +{
> +	struct drm_dp_mst_atomic_payload *pos;
> +
> +	/* Remove local payload allocation */
>  	list_for_each_entry(pos, &mst_state->payloads, next) {
>  		if (pos != new_payload && pos->vc_start_slot > new_payload->vc_start_slot)
>  			pos->vc_start_slot -= old_payload->time_slots;
> @@ -3382,9 +3409,10 @@ void drm_dp_remove_payload(struct drm_dp_mst_topology_mgr *mgr,
>  
>  	if (new_payload->delete)
>  		drm_dp_mst_put_port_malloc(new_payload->port);
> -}
> -EXPORT_SYMBOL(drm_dp_remove_payload);
>  
> +	new_payload->payload_allocation_status = DRM_DP_MST_PAYLOAD_ALLOCATION_NONE;
> +}
> +EXPORT_SYMBOL(drm_dp_remove_payload_part2);
>  /**
>   * drm_dp_add_payload_part2() - Execute payload update part 2
>   * @mgr: Manager to use.
> @@ -3403,17 +3431,19 @@ int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
>  	int ret = 0;
>  
>  	/* Skip failed payloads */
> -	if (payload->vc_start_slot == -1) {
> -		drm_dbg_kms(mgr->dev, "Part 1 of payload creation for %s failed, skipping part 2\n",
> +	if (payload->payload_allocation_status != DRM_DP_MST_PAYLOAD_ALLOCATION_DFP) {
> +		drm_dbg_kms(state->dev, "Part 1 of payload creation for %s failed, skipping part 2\n",
>  			    payload->port->connector->name);
>  		return -EIO;
>  	}
>  
> -	ret = drm_dp_create_payload_step2(mgr, payload);
> -	if (ret < 0) {
> +	/* Allocate payload to remote end */
> +	ret = drm_dp_create_payload_to_remote(mgr, payload);
> +	if (ret < 0)
>  		drm_err(mgr->dev, "Step 2 of creating MST payload for %p failed: %d\n",
>  			payload->port, ret);
> -	}
> +	else
> +		payload->payload_allocation_status = DRM_DP_MST_PAYLOAD_ALLOCATION_REMOTE;
>  
>  	return ret;
>  }
> @@ -4324,6 +4354,7 @@ int drm_dp_atomic_find_time_slots(struct drm_atomic_state *state,
>  		drm_dp_mst_get_port_malloc(port);
>  		payload->port = port;
>  		payload->vc_start_slot = -1;
> +		payload->payload_allocation_status = DRM_DP_MST_PAYLOAD_ALLOCATION_NONE;
>  		list_add(&payload->next, &topology_state->payloads);
>  	}
>  	payload->time_slots = req_slots;
> @@ -4493,7 +4524,7 @@ void drm_dp_mst_atomic_wait_for_dependencies(struct drm_atomic_state *state)
>  		}
>  
>  		/* Now that previous state is committed, it's safe to copy over the start slot
> -		 * assignments
> +		 * and allocation status assignments
>  		 */
>  		list_for_each_entry(old_payload, &old_mst_state->payloads, next) {
>  			if (old_payload->delete)
> @@ -4502,6 +4533,8 @@ void drm_dp_mst_atomic_wait_for_dependencies(struct drm_atomic_state *state)
>  			new_payload = drm_atomic_get_mst_payload_state(new_mst_state,
>  								       old_payload->port);
>  			new_payload->vc_start_slot = old_payload->vc_start_slot;
> +			new_payload->payload_allocation_status =
> +							old_payload->payload_allocation_status;

And here. Besides that, I think patches 1/3 and 2/3 look great! Just finished
testing them on nouveau:

Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx>

Patch 3/3 I gotta think about a bit more (and am curious about Imre's current
take on it) I think

>  		}
>  	}
>  }
> @@ -4818,6 +4851,13 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>  	struct drm_dp_mst_atomic_payload *payload;
>  	int i, ret;
>  
> +	static const char *const status[] = {
> +		"None",
> +		"Local",
> +		"DFP",
> +		"Remote",
> +	};
> +
>  	mutex_lock(&mgr->lock);
>  	if (mgr->mst_primary)
>  		drm_dp_mst_dump_mstb(m, mgr->mst_primary);
> @@ -4834,7 +4874,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>  	seq_printf(m, "payload_mask: %x, max_payloads: %d, start_slot: %u, pbn_div: %d\n",
>  		   state->payload_mask, mgr->max_payloads, state->start_slot, state->pbn_div);
>  
> -	seq_printf(m, "\n| idx | port | vcpi | slots | pbn | dsc |     sink name     |\n");
> +	seq_printf(m, "\n| idx | port | vcpi | slots | pbn | dsc | status |     sink name     |\n");
>  	for (i = 0; i < mgr->max_payloads; i++) {
>  		list_for_each_entry(payload, &state->payloads, next) {
>  			char name[14];
> @@ -4843,7 +4883,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>  				continue;
>  
>  			fetch_monitor_name(mgr, payload->port, name, sizeof(name));
> -			seq_printf(m, " %5d %6d %6d %02d - %02d %5d %5s %19s\n",
> +			seq_printf(m, " %5d %6d %6d %02d - %02d %5d %5s %8s %19s\n",
>  				   i,
>  				   payload->port->port_num,
>  				   payload->vcpi,
> @@ -4851,6 +4891,7 @@ void drm_dp_mst_dump_topology(struct seq_file *m,
>  				   payload->vc_start_slot + payload->time_slots - 1,
>  				   payload->pbn,
>  				   payload->dsc_enabled ? "Y" : "N",
> +				   status[payload->payload_allocation_status],
>  				   (*name != 0) ? name : "Unknown");
>  		}
>  	}
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index e3f176a093d2..5f73cdabe7a1 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -557,12 +557,8 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state,
>  	struct intel_dp *intel_dp = &dig_port->dp;
>  	struct intel_connector *connector =
>  		to_intel_connector(old_conn_state->connector);
> -	struct drm_dp_mst_topology_state *old_mst_state =
> -		drm_atomic_get_old_mst_topology_state(&state->base, &intel_dp->mst_mgr);
>  	struct drm_dp_mst_topology_state *new_mst_state =
>  		drm_atomic_get_new_mst_topology_state(&state->base, &intel_dp->mst_mgr);
> -	const struct drm_dp_mst_atomic_payload *old_payload =
> -		drm_atomic_get_mst_payload_state(old_mst_state, connector->port);
>  	struct drm_dp_mst_atomic_payload *new_payload =
>  		drm_atomic_get_mst_payload_state(new_mst_state, connector->port);
>  	struct drm_i915_private *i915 = to_i915(connector->base.dev);
> @@ -572,8 +568,7 @@ static void intel_mst_disable_dp(struct intel_atomic_state *state,
>  
>  	intel_hdcp_disable(intel_mst->connector);
>  
> -	drm_dp_remove_payload(&intel_dp->mst_mgr, new_mst_state,
> -			      old_payload, new_payload);
> +	drm_dp_remove_payload_part1(&intel_dp->mst_mgr, new_mst_state, new_payload);
>  
>  	intel_audio_codec_disable(encoder, old_crtc_state, old_conn_state);
>  }
> @@ -588,6 +583,14 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  	struct intel_dp *intel_dp = &dig_port->dp;
>  	struct intel_connector *connector =
>  		to_intel_connector(old_conn_state->connector);
> +	struct drm_dp_mst_topology_state *old_mst_state =
> +		drm_atomic_get_old_mst_topology_state(&state->base, &intel_dp->mst_mgr);
> +	struct drm_dp_mst_topology_state *new_mst_state =
> +		drm_atomic_get_new_mst_topology_state(&state->base, &intel_dp->mst_mgr);
> +	const struct drm_dp_mst_atomic_payload *old_payload =
> +		drm_atomic_get_mst_payload_state(old_mst_state, connector->port);
> +	struct drm_dp_mst_atomic_payload *new_payload =
> +		drm_atomic_get_mst_payload_state(new_mst_state, connector->port);
>  	struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
>  	bool last_mst_stream;
>  
> @@ -608,6 +611,9 @@ static void intel_mst_post_disable_dp(struct intel_atomic_state *state,
>  
>  	wait_for_act_sent(encoder, old_crtc_state);
>  
> +	drm_dp_remove_payload_part2(&intel_dp->mst_mgr, new_mst_state,
> +				    old_payload, new_payload);
> +
>  	intel_ddi_disable_transcoder_func(old_crtc_state);
>  
>  	if (DISPLAY_VER(dev_priv) >= 9)
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 4e7c9c353c51..bba01fa0780c 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -882,21 +882,26 @@ struct nouveau_encoder *nv50_real_outp(struct drm_encoder *encoder)
>  
>  static void
>  nv50_msto_cleanup(struct drm_atomic_state *state,
> -		  struct drm_dp_mst_topology_state *mst_state,
> +		  struct drm_dp_mst_topology_state *new_mst_state,
>  		  struct drm_dp_mst_topology_mgr *mgr,
>  		  struct nv50_msto *msto)
>  {
>  	struct nouveau_drm *drm = nouveau_drm(msto->encoder.dev);
> -	struct drm_dp_mst_atomic_payload *payload =
> -		drm_atomic_get_mst_payload_state(mst_state, msto->mstc->port);
> +	struct drm_dp_mst_atomic_payload *new_payload =
> +		drm_atomic_get_mst_payload_state(new_mst_state, msto->mstc->port);
> +	struct drm_dp_mst_topology_state *old_mst_state =
> +		drm_atomic_get_old_mst_topology_state(state, mgr);
> +	const struct drm_dp_mst_atomic_payload *old_payload =
> +		drm_atomic_get_mst_payload_state(old_mst_state, msto->mstc->port);
>  
>  	NV_ATOMIC(drm, "%s: msto cleanup\n", msto->encoder.name);
>  
>  	if (msto->disabled) {
>  		msto->mstc = NULL;
>  		msto->disabled = false;
> +		drm_dp_remove_payload_part2(mgr, new_mst_state, old_payload, new_payload);
>  	} else if (msto->enabled) {
> -		drm_dp_add_payload_part2(mgr, state, payload);
> +		drm_dp_add_payload_part2(mgr, state, new_payload);
>  		msto->enabled = false;
>  	}
>  }
> @@ -910,19 +915,15 @@ nv50_msto_prepare(struct drm_atomic_state *state,
>  	struct nouveau_drm *drm = nouveau_drm(msto->encoder.dev);
>  	struct nv50_mstc *mstc = msto->mstc;
>  	struct nv50_mstm *mstm = mstc->mstm;
> -	struct drm_dp_mst_topology_state *old_mst_state;
> -	struct drm_dp_mst_atomic_payload *payload, *old_payload;
> +	struct drm_dp_mst_atomic_payload *payload;
>  
>  	NV_ATOMIC(drm, "%s: msto prepare\n", msto->encoder.name);
>  
> -	old_mst_state = drm_atomic_get_old_mst_topology_state(state, mgr);
> -
>  	payload = drm_atomic_get_mst_payload_state(mst_state, mstc->port);
> -	old_payload = drm_atomic_get_mst_payload_state(old_mst_state, mstc->port);
>  
>  	// TODO: Figure out if we want to do a better job of handling VCPI allocation failures here?
>  	if (msto->disabled) {
> -		drm_dp_remove_payload(mgr, mst_state, old_payload, payload);
> +		drm_dp_remove_payload_part1(mgr, mst_state, payload);
>  
>  		nvif_outp_dp_mst_vcpi(&mstm->outp->outp, msto->head->base.index, 0, 0, 0, 0);
>  	} else {
> diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
> index ed5c9660563c..4429d3b1745b 100644
> --- a/include/drm/display/drm_dp_mst_helper.h
> +++ b/include/drm/display/drm_dp_mst_helper.h
> @@ -46,6 +46,13 @@ struct drm_dp_mst_topology_ref_history {
>  };
>  #endif /* IS_ENABLED(CONFIG_DRM_DEBUG_DP_MST_TOPOLOGY_REFS) */
>  
> +enum drm_dp_mst_payload_allocation {
> +	DRM_DP_MST_PAYLOAD_ALLOCATION_NONE,
> +	DRM_DP_MST_PAYLOAD_ALLOCATION_LOCAL,
> +	DRM_DP_MST_PAYLOAD_ALLOCATION_DFP,
> +	DRM_DP_MST_PAYLOAD_ALLOCATION_REMOTE,
> +};
> +
>  struct drm_dp_mst_branch;
>  
>  /**
> @@ -537,7 +544,7 @@ struct drm_dp_mst_atomic_payload {
>  	 *   drm_dp_mst_atomic_wait_for_dependencies() has been called, which will ensure the
>  	 *   previous MST states payload start slots have been copied over to the new state. Note
>  	 *   that a new start slot won't be assigned/removed from this payload until
> -	 *   drm_dp_add_payload_part1()/drm_dp_remove_payload() have been called.
> +	 *   drm_dp_add_payload_part1()/drm_dp_remove_payload_part2() have been called.
>  	 * * Acquire the MST modesetting lock, and then wait for any pending MST-related commits to
>  	 *   get committed to hardware by calling drm_crtc_commit_wait() on each of the
>  	 *   &drm_crtc_commit structs in &drm_dp_mst_topology_state.commit_deps.
> @@ -564,6 +571,9 @@ struct drm_dp_mst_atomic_payload {
>  	/** @dsc_enabled: Whether or not this payload has DSC enabled */
>  	bool dsc_enabled : 1;
>  
> +	/** @payload_allocation_status: The allocation status of this payload */
> +	enum drm_dp_mst_payload_allocation payload_allocation_status;
> +
>  	/** @next: The list node for this payload */
>  	struct list_head next;
>  };
> @@ -842,10 +852,13 @@ int drm_dp_add_payload_part1(struct drm_dp_mst_topology_mgr *mgr,
>  int drm_dp_add_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
>  			     struct drm_atomic_state *state,
>  			     struct drm_dp_mst_atomic_payload *payload);
> -void drm_dp_remove_payload(struct drm_dp_mst_topology_mgr *mgr,
> -			   struct drm_dp_mst_topology_state *mst_state,
> -			   const struct drm_dp_mst_atomic_payload *old_payload,
> -			   struct drm_dp_mst_atomic_payload *new_payload);
> +void drm_dp_remove_payload_part1(struct drm_dp_mst_topology_mgr *mgr,
> +				 struct drm_dp_mst_topology_state *mst_state,
> +				 struct drm_dp_mst_atomic_payload *payload);
> +void drm_dp_remove_payload_part2(struct drm_dp_mst_topology_mgr *mgr,
> +				 struct drm_dp_mst_topology_state *mst_state,
> +				 const struct drm_dp_mst_atomic_payload *old_payload,
> +				 struct drm_dp_mst_atomic_payload *new_payload);
>  
>  int drm_dp_check_act_status(struct drm_dp_mst_topology_mgr *mgr);
>  

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux