Re: [PATCH 1/3] drm/amd/display: Add functions for new DC commit sequence

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

On 2022-10-11 14:11, Rodrigo Siqueira wrote:
> Since DCN32, our DC commit become more complex and requires more
> adjustments to work well with some features. This is a preparation work
> for replacing dc_commit_state and dc_commit_updates_for_stream with
> dc_update_planes_and_stream and dc_update_planes_and_stream

I think you mean
"dc_commit_streams and dc_update_planes_and_stream"

Harry

> respectively.
> 
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> Co-developed-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
> Signed-off-by: Aurabindo Pillai <aurabindo.pillai@amd.com>
> ---
>  drivers/gpu/drm/amd/display/dc/core/dc.c      |  92 ++++++++
>  .../gpu/drm/amd/display/dc/core/dc_resource.c | 204 +++++++++++++++++-
>  drivers/gpu/drm/amd/display/dc/dc.h           |   6 +
>  drivers/gpu/drm/amd/display/dc/dc_stream.h    |  11 +-
>  4 files changed, 308 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index 660316a536f7..ad3aafb73cba 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -59,6 +59,7 @@
>  
>  #include "dc_link_dp.h"
>  #include "dc_dmub_srv.h"
> +#include "dc_stream.h"
>  
>  #include "dsc.h"
>  
> @@ -1505,6 +1506,25 @@ static bool context_changed(
>  	return false;
>  }
>  
> +static bool streams_changed(struct dc *dc,
> +			    struct dc_stream_state *streams[],
> +			    uint8_t stream_count)
> +{
> +	uint8_t i;
> +
> +	if (stream_count != dc->current_state->stream_count)
> +		return true;
> +
> +	for (i = 0; i < dc->current_state->stream_count; i++) {
> +		if (dc->current_state->streams[i] != streams[i])
> +			return true;
> +		if (!streams[i]->link->link_state_valid)
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>  bool dc_validate_boot_timing(const struct dc *dc,
>  				const struct dc_sink *sink,
>  				struct dc_crtc_timing *crtc_timing)
> @@ -1923,6 +1943,78 @@ bool dc_commit_state(struct dc *dc, struct dc_state *context)
>  	return (result == DC_OK);
>  }
>  
> +enum dc_status dc_commit_streams(struct dc *dc,
> +				 struct dc_stream_state *streams[],
> +				 uint8_t stream_count)
> +{
> +	int i, j;
> +	struct dc_state *context;
> +	enum dc_status res = DC_OK;
> +	struct dc_validation_set set[MAX_STREAMS] = {0};
> +
> +	if (dc->ctx->dce_environment == DCE_ENV_VIRTUAL_HW)
> +		return res;
> +
> +	if (!streams_changed(dc, streams, stream_count))
> +		return res;
> +
> +	DC_LOG_DC("%s: %d streams\n", __func__, stream_count);
> +
> +	for (i = 0; i < stream_count; i++) {
> +		struct dc_stream_state *stream = streams[i];
> +		struct dc_stream_status *status = dc_stream_get_status(stream);
> +
> +		dc_stream_log(dc, stream);
> +
> +		set[i].stream = stream;
> +
> +		if (status) {
> +			set[i].plane_count = status->plane_count;
> +			for (j = 0; j < status->plane_count; j++)
> +				set[i].plane_states[j] = status->plane_states[j];
> +		}
> +	}
> +
> +	context = dc_create_state(dc);
> +	if (!context)
> +		goto context_alloc_fail;
> +
> +	dc_resource_state_copy_construct_current(dc, context);
> +
> +	res = dc_validate_with_context(dc, set, stream_count, context, false);
> +	if (res != DC_OK) {
> +		BREAK_TO_DEBUGGER();
> +		goto fail;
> +	}
> +
> +	res = dc_commit_state_no_check(dc, context);
> +
> +	for (i = 0; i < stream_count; i++) {
> +		for (j = 0; j < context->stream_count; j++) {
> +			if (streams[i]->stream_id == context->streams[j]->stream_id)
> +				streams[i]->out.otg_offset = context->stream_status[j].primary_otg_inst;
> +
> +			if (dc_is_embedded_signal(streams[i]->signal)) {
> +				struct dc_stream_status *status = dc_stream_get_status_from_state(context, streams[i]);
> +
> +				if (dc->hwss.is_abm_supported)
> +					status->is_abm_supported = dc->hwss.is_abm_supported(dc, context, streams[i]);
> +				else
> +					status->is_abm_supported = true;
> +			}
> +		}
> +	}
> +
> +fail:
> +	dc_release_state(context);
> +
> +context_alloc_fail:
> +
> +	DC_LOG_DC("%s Finished.\n", __func__);
> +
> +	return res;
> +}
> +
>  bool dc_acquire_release_mpc_3dlut(
>  		struct dc *dc, bool acquire,
>  		struct dc_stream_state *stream,
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> index fd8db482e56f..0c58a19e9115 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_resource.c
> @@ -2593,6 +2593,208 @@ bool dc_resource_is_dsc_encoding_supported(const struct dc *dc)
>  	return dc->res_pool->res_cap->num_dsc > 0;
>  }
>  
> +static bool planes_changed_for_existing_stream(struct dc_state *context,
> +					       struct dc_stream_state *stream,
> +					       const struct dc_validation_set set[],
> +					       int set_count)
> +{
> +	int i, j;
> +	struct dc_stream_status *stream_status = NULL;
> +
> +	for (i = 0; i < context->stream_count; i++)
> +		if (context->streams[i] == stream) {
> +			stream_status = &context->stream_status[i];
> +			break;
> +		}
> +
> +	if (!stream_status)
> +		ASSERT(0);
> +
> +	for (i = 0; i < set_count; i++)
> +		if (set[i].stream == stream)
> +			break;
> +
> +	if (i == set_count)
> +		ASSERT(0);
> +
> +	if (set[i].plane_count != stream_status->plane_count)
> +		return true;
> +
> +	for (j = 0; j < set[i].plane_count; j++)
> +		if (set[i].plane_states[j] != stream_status->plane_states[j])
> +			return true;
> +
> +	return false;
> +}
> +
> +enum dc_status dc_validate_with_context(struct dc *dc,
> +					const struct dc_validation_set set[],
> +					int set_count,
> +					struct dc_state *context,
> +					bool fast_validate)
> +{
> +	struct dc_stream_state *unchanged_streams[MAX_PIPES] = { 0 };
> +	struct dc_stream_state *del_streams[MAX_PIPES] = { 0 };
> +	struct dc_stream_state *add_streams[MAX_PIPES] = { 0 };
> +	int old_stream_count = context->stream_count;
> +	enum dc_status res = DC_ERROR_UNEXPECTED;
> +	int unchanged_streams_count = 0;
> +	int del_streams_count = 0;
> +	int add_streams_count = 0;
> +	bool found = false;
> +	int i, j, k;
> +
> +	DC_LOGGER_INIT(dc->ctx->logger);
> +
> +	/* First remove from context all deleted streams */
> +	for (i = 0; i < old_stream_count; i++) {
> +		struct dc_stream_state *stream = context->streams[i];
> +
> +		for (j = 0; j < set_count; j++) {
> +			if (stream == set[j].stream) {
> +				found = true;
> +				break;
> +			}
> +		}
> +
> +		if (!found)
> +			del_streams[del_streams_count++] = stream;
> +
> +		found = false;
> +	}
> +
> +	/* Now add new streams */
> +	for (i = 0; i < set_count; i++) {
> +		struct dc_stream_state *stream = set[i].stream;
> +
> +		for (j = 0; j < old_stream_count; j++) {
> +			if (stream == context->streams[j]) {
> +				found = true;
> +				break;
> +			}
> +		}
> +
> +		if (!found)
> +			add_streams[add_streams_count++] = stream;
> +
> +		found = false;
> +	}
> +
> +	/*
> +	 * Build array of unchanged  streams, it is needed to handle
> +	 * planes only changes (added/removed/updated)
> +	 */
> +	for (i = 0; i < set_count; i++) {
> +		for (j = 0; j < del_streams_count; j++) {
> +			if (set[i].stream == del_streams[j]) {
> +				found = true;
> +				break;
> +			}
> +		}
> +
> +		if (!found) {
> +			for (j = 0; j < add_streams_count; j++) {
> +				if (set[i].stream == add_streams[j]) {
> +					found = true;
> +					break;
> +				}
> +			}
> +		}
> +
> +		if (!found)
> +			unchanged_streams[unchanged_streams_count++] = set[i].stream;
> +
> +		found = false;
> +	}
> +
> +	/* Remove all planes for unchanged streams if planes changed */
> +	for (i = 0; i < unchanged_streams_count; i++) {
> +		if (planes_changed_for_existing_stream(context,
> +						       unchanged_streams[i],
> +						       set,
> +						       set_count)) {
> +			if (!dc_rem_all_planes_for_stream(dc,
> +							  unchanged_streams[i],
> +							  context)) {
> +				res = DC_FAIL_DETACH_SURFACES;
> +				goto fail;
> +			}
> +		}
> +	}
> +
> +	/* Remove all planes for removed streams and then remove the streams */
> +	for (i = 0; i < del_streams_count; i++) {
> +		/* Need to cpy the dwb data from the old stream in order to efc to work */
> +		if (del_streams[i]->num_wb_info > 0) {
> +			for (j = 0; j < add_streams_count; j++) {
> +				if (del_streams[i]->sink == add_streams[j]->sink) {
> +					add_streams[j]->num_wb_info = del_streams[i]->num_wb_info;
> +					for (k = 0; k < del_streams[i]->num_wb_info; k++)
> +						add_streams[j]->writeback_info[k] = del_streams[i]->writeback_info[k];
> +				}
> +			}
> +		}
> +
> +		if (!dc_rem_all_planes_for_stream(dc, del_streams[i], context)) {
> +			res = DC_FAIL_DETACH_SURFACES;
> +			goto fail;
> +		}
> +
> +		res = dc_remove_stream_from_ctx(dc, context, del_streams[i]);
> +		if (res != DC_OK)
> +			goto fail;
> +	}
> +
> +	/* Swap seamless boot stream to pipe 0 (if needed) to ensure pipe_ctx matches.
> +	 * This may change in the future if seamless_boot_stream can be multiple.
> +	 */
> +	for (i = 0; i < add_streams_count; i++) {
> +		mark_seamless_boot_stream(dc, add_streams[i]);
> +		if (add_streams[i]->apply_seamless_boot_optimization && i != 0) {
> +			struct dc_stream_state *temp = add_streams[0];
> +
> +			add_streams[0] = add_streams[i];
> +			add_streams[i] = temp;
> +			break;
> +		}
> +	}
> +
> +	/* Add new streams and then add all planes for the new stream */
> +	for (i = 0; i < add_streams_count; i++) {
> +		calculate_phy_pix_clks(add_streams[i]);
> +		res = dc_add_stream_to_ctx(dc, context, add_streams[i]);
> +		if (res != DC_OK)
> +			goto fail;
> +
> +		if (!add_all_planes_for_stream(dc, add_streams[i], set, set_count, context)) {
> +			res = DC_FAIL_ATTACH_SURFACES;
> +			goto fail;
> +		}
> +	}
> +
> +	/* Add all planes for unchanged streams if planes changed */
> +	for (i = 0; i < unchanged_streams_count; i++) {
> +		if (planes_changed_for_existing_stream(context,
> +						       unchanged_streams[i],
> +						       set,
> +						       set_count)) {
> +			if (!add_all_planes_for_stream(dc, unchanged_streams[i], set, set_count, context)) {
> +				res = DC_FAIL_ATTACH_SURFACES;
> +				goto fail;
> +			}
> +		}
> +	}
> +
> +	res = dc_validate_global_state(dc, context, fast_validate);
> +
> +fail:
> +	if (res != DC_OK) {
> +		DC_LOG_WARNING("%s:resource validation failed, dc_status:%d\n",
> +			       __func__,
> +			       res);
> +	}
> +	return res;
> +}
>  
>  /**
>   * dc_validate_global_state() - Determine if HW can support a given state
> @@ -3734,4 +3936,4 @@ bool dc_resource_acquire_secondary_pipe_for_mpc_odm(
>  	}
>  
>  	return true;
> -}
> \ No newline at end of file
> +}
> diff --git a/drivers/gpu/drm/amd/display/dc/dc.h b/drivers/gpu/drm/amd/display/dc/dc.h
> index bfc5474c0f4c..2753d9eabd16 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc.h
> @@ -1286,6 +1286,12 @@ enum dc_status dc_validate_plane(struct dc *dc, const struct dc_plane_state *pla
>  
>  void get_clock_requirements_for_state(struct dc_state *state, struct AsicStateEx *info);
>  
> +enum dc_status dc_validate_with_context(struct dc *dc,
> +					const struct dc_validation_set set[],
> +					int set_count,
> +					struct dc_state *context,
> +					bool fast_validate);
> +
>  bool dc_set_generic_gpio_for_stereo(bool enable,
>  		struct gpio_service *gpio_service);
>  
> diff --git a/drivers/gpu/drm/amd/display/dc/dc_stream.h b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> index 9e6025c98db9..d5b3c80635ea 100644
> --- a/drivers/gpu/drm/amd/display/dc/dc_stream.h
> +++ b/drivers/gpu/drm/amd/display/dc/dc_stream.h
> @@ -378,10 +378,9 @@ bool dc_stream_get_scanoutpos(const struct dc_stream_state *stream,
>  				  uint32_t *h_position,
>  				  uint32_t *v_position);
>  
> -enum dc_status dc_add_stream_to_ctx(
> -			struct dc *dc,
> -		struct dc_state *new_ctx,
> -		struct dc_stream_state *stream);
> +enum dc_status dc_add_stream_to_ctx(struct dc *dc,
> +				    struct dc_state *new_ctx,
> +				    struct dc_stream_state *stream);
>  
>  enum dc_status dc_remove_stream_from_ctx(
>  		struct dc *dc,
> @@ -445,6 +444,10 @@ enum dc_status dc_validate_stream(struct dc *dc, struct dc_stream_state *stream)
>   *   Phy, Encoder, Timing Generator are programmed and enabled.
>   *   New streams are enabled with blank stream; no memory read.
>   */
> +enum dc_status dc_commit_streams(struct dc *dc,
> +				 struct dc_stream_state *streams[],
> +				 uint8_t stream_count);
> +
>  /*
>   * Enable stereo when commit_streams is not required,
>   * for example, frame alternate.