Re: [PATCH 2/3] drm/amd/display: Enable new commit sequence only for DCN32x

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

On 2022-10-11 14:11, Rodrigo Siqueira wrote:
> Change commit sequence might impact multiple ASICs; for this reason, it
> is better to run it only on DCN32x for the first set of validation.
> After we get more confident about this change, we should expand it to
> all ASICs.
> 
> 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 | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c b/drivers/gpu/drm/amd/display/dc/core/dc.c
> index ad3aafb73cba..21d20900cfe3 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
> @@ -1913,6 +1913,13 @@ bool dc_commit_state(struct dc *dc, struct dc_state *context)
>  	enum dc_status result = DC_ERROR_UNEXPECTED;
>  	int i;
>  
> +	/* TODO: Enable the new commit sequence for all ASICs */
> +	if (dc->ctx->asic_id.chip_family == AMDGPU_FAMILY_GC_11_0_0 ||
> +	    dc->ctx->asic_id.chip_family == AMDGPU_FAMILY_GC_11_0_1) {

Why not check that dc_ctx->dce_version == DCN_VERSION_3_2?

I don't like using the family ID in DC unless we have a good
reason to.

> +		result = dc_commit_streams(dc, context->streams, context->stream_count);

Wasn't this function initially intended as a crutch for other OSes
that couldn't use dc_commit_state directly for some reasons?

I'm not sure this change is moving into the right direction, at least
not without a much more detailed explanation of why dc_commit_streams
and dc_update_planes_and_streams are the better solution.

Harry

> +		return result == DC_OK;
> +	}
> +
>  	if (!context_changed(dc, context))
>  		return DC_OK;
>  
> @@ -3898,6 +3905,15 @@ void dc_commit_updates_for_stream(struct dc *dc,
>  	struct dc_context *dc_ctx = dc->ctx;
>  	int i, j;
>  
> +	/* TODO: Enable the new commit sequence for all ASICs */
> +	if (dc->ctx->asic_id.chip_family == AMDGPU_FAMILY_GC_11_0_0 ||
> +	    dc->ctx->asic_id.chip_family == AMDGPU_FAMILY_GC_11_0_1) {
> +		dc_update_planes_and_stream(dc, srf_updates,
> +					    surface_count, stream,
> +					    stream_update);
> +		return;
> +	}
> +
>  	stream_status = dc_stream_get_status(stream);
>  	context = dc->current_state;
>