[PATCH] drm/amd/display: Old sequence for HUBP blank

Rodrigo Siqueira Rodrigo.Siqueira at amd.com
Wed Feb 17 13:40:14 UTC 2021


Hi,

Is this commit a direct revert from
be7af780ef3cbb8fe1004db48dc66caf2da595cd ?

If so, I recommend you to use the standard way to identify "revert"
commits by using 'Revert "Commit header"' and the message "This reverts
commit HASH" followed by the original commit description.

Thanks
Siqueira

On 02/16, Aurabindo Pillai wrote:
> This reverts commit be7af780ef3cbb8fe1004db48dc66caf2da595cd because the
> new proposed sequence for HUBP blanking causes regressions. This change
> brings back the old sequence.
> 
> Signed-off-by: Aurabindo Pillai <aurabindo.pillai at amd.com>
> Signed-off-by: Bhawanpreet Lakha <bhawanpreet.lakha at amd.com>
> ---
>  .../amd/display/dc/dcn10/dcn10_hw_sequencer.c | 36 +++--------
>  .../amd/display/dc/dcn10/dcn10_hw_sequencer.h |  3 -
>  .../gpu/drm/amd/display/dc/dcn10/dcn10_init.c |  1 -
>  .../drm/amd/display/dc/dcn20/dcn20_hwseq.c    | 13 +++-
>  .../gpu/drm/amd/display/dc/dcn20/dcn20_init.c |  1 -
>  .../gpu/drm/amd/display/dc/dcn21/dcn21_init.c |  1 -
>  .../drm/amd/display/dc/dcn30/dcn30_hwseq.c    | 62 ++++---------------
>  .../drm/amd/display/dc/dcn30/dcn30_hwseq.h    |  4 --
>  .../gpu/drm/amd/display/dc/dcn30/dcn30_init.c |  1 -
>  .../drm/amd/display/dc/dcn301/dcn301_init.c   |  1 -
>  .../gpu/drm/amd/display/dc/inc/hw_sequencer.h |  4 --
>  11 files changed, 30 insertions(+), 97 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> index 89912bb5014f..361a97edc8ee 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.c
> @@ -2635,7 +2635,7 @@ static void dcn10_update_dchubp_dpp(
>  	hws->funcs.update_plane_addr(dc, pipe_ctx);
>  
>  	if (is_pipe_tree_visible(pipe_ctx))
> -		dc->hwss.set_hubp_blank(dc, pipe_ctx, false);
> +		hubp->funcs->set_blank(hubp, false);
>  }
>  
>  void dcn10_blank_pixel_data(
> @@ -3146,16 +3146,13 @@ void dcn10_setup_stereo(struct pipe_ctx *pipe_ctx, struct dc *dc)
>  	return;
>  }
>  
> -static struct pipe_ctx *get_pipe_ctx_by_hubp_inst(struct dc_state *context, int mpcc_inst)
> +static struct hubp *get_hubp_by_inst(struct resource_pool *res_pool, int mpcc_inst)
>  {
>  	int i;
>  
> -	for (i = 0; i < MAX_PIPES; i++) {
> -		if (context->res_ctx.pipe_ctx[i].plane_res.hubp
> -				&& context->res_ctx.pipe_ctx[i].plane_res.hubp->inst == mpcc_inst) {
> -			return &context->res_ctx.pipe_ctx[i];
> -		}
> -
> +	for (i = 0; i < res_pool->pipe_count; i++) {
> +		if (res_pool->hubps[i]->inst == mpcc_inst)
> +			return res_pool->hubps[i];
>  	}
>  	ASSERT(false);
>  	return NULL;
> @@ -3178,23 +3175,11 @@ void dcn10_wait_for_mpcc_disconnect(
>  
>  	for (mpcc_inst = 0; mpcc_inst < MAX_PIPES; mpcc_inst++) {
>  		if (pipe_ctx->stream_res.opp->mpcc_disconnect_pending[mpcc_inst]) {
> -			struct pipe_ctx *restore_bottom_pipe;
> -			struct pipe_ctx *restore_top_pipe;
> -			struct pipe_ctx *inst_pipe_ctx = get_pipe_ctx_by_hubp_inst(dc->current_state, mpcc_inst);
> +			struct hubp *hubp = get_hubp_by_inst(res_pool, mpcc_inst);
>  
> -			ASSERT(inst_pipe_ctx);
>  			res_pool->mpc->funcs->wait_for_idle(res_pool->mpc, mpcc_inst);
>  			pipe_ctx->stream_res.opp->mpcc_disconnect_pending[mpcc_inst] = false;
> -			/*
> -			 * Set top and bottom pipes NULL, as we don't want
> -			 * to blank those pipes when disconnecting from MPCC
> -			 */
> -			restore_bottom_pipe = inst_pipe_ctx->bottom_pipe;
> -			restore_top_pipe = inst_pipe_ctx->top_pipe;
> -			inst_pipe_ctx->top_pipe = inst_pipe_ctx->bottom_pipe = NULL;
> -			dc->hwss.set_hubp_blank(dc, inst_pipe_ctx, true);
> -			inst_pipe_ctx->top_pipe = restore_top_pipe;
> -			inst_pipe_ctx->bottom_pipe = restore_bottom_pipe;
> +			hubp->funcs->set_blank(hubp, true);
>  		}
>  	}
>  
> @@ -3747,10 +3732,3 @@ void dcn10_get_clock(struct dc *dc,
>  				dc->clk_mgr->funcs->get_clock(dc->clk_mgr, context, clock_type, clock_cfg);
>  
>  }
> -
> -void dcn10_set_hubp_blank(const struct dc *dc,
> -				struct pipe_ctx *pipe_ctx,
> -				bool blank_enable)
> -{
> -	pipe_ctx->plane_res.hubp->funcs->set_blank(pipe_ctx->plane_res.hubp, blank_enable);
> -}
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h
> index 89e6dfb63da0..dee8ad1ebaa4 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hw_sequencer.h
> @@ -204,8 +204,5 @@ void dcn10_wait_for_pending_cleared(struct dc *dc,
>  		struct dc_state *context);
>  void dcn10_set_hdr_multiplier(struct pipe_ctx *pipe_ctx);
>  void dcn10_verify_allow_pstate_change_high(struct dc *dc);
> -void dcn10_set_hubp_blank(const struct dc *dc,
> -				struct pipe_ctx *pipe_ctx,
> -				bool blank_enable);
>  
>  #endif /* __DC_HWSS_DCN10_H__ */
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_init.c b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_init.c
> index 2f1b802e66a1..254300b06b43 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_init.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_init.c
> @@ -79,7 +79,6 @@ static const struct hw_sequencer_funcs dcn10_funcs = {
>  	.set_backlight_level = dce110_set_backlight_level,
>  	.set_abm_immediate_disable = dce110_set_abm_immediate_disable,
>  	.set_pipe = dce110_set_pipe,
> -	.set_hubp_blank = dcn10_set_hubp_blank,
>  };
>  
>  static const struct hwseq_private_funcs dcn10_private_funcs = {
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> index b79a17f6a9cc..48d1e0e2cf75 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_hwseq.c
> @@ -1576,7 +1576,7 @@ static void dcn20_update_dchubp_dpp(
>  
>  
>  	if (pipe_ctx->update_flags.bits.enable)
> -		dc->hwss.set_hubp_blank(dc, pipe_ctx, false);
> +		hubp->funcs->set_blank(hubp, false);
>  }
>  
>  
> @@ -1772,10 +1772,19 @@ void dcn20_post_unlock_program_front_end(
>  
>  	for (i = 0; i < dc->res_pool->pipe_count; i++) {
>  		struct pipe_ctx *pipe = &context->res_ctx.pipe_ctx[i];
> +		struct pipe_ctx *mpcc_pipe;
>  
>  		if (pipe->vtp_locked) {
> -			dc->hwss.set_hubp_blank(dc, pipe, true);
> +			dc->hwseq->funcs.wait_for_blank_complete(pipe->stream_res.opp);
> +			pipe->plane_res.hubp->funcs->set_blank(pipe->plane_res.hubp, true);
>  			pipe->vtp_locked = false;
> +
> +			for (mpcc_pipe = pipe->bottom_pipe; mpcc_pipe; mpcc_pipe = mpcc_pipe->bottom_pipe)
> +				mpcc_pipe->plane_res.hubp->funcs->set_blank(mpcc_pipe->plane_res.hubp, true);
> +
> +			for (i = 0; i < dc->res_pool->pipe_count; i++)
> +				if (context->res_ctx.pipe_ctx[i].update_flags.bits.disable)
> +					dc->hwss.disable_plane(dc, &dc->current_state->res_ctx.pipe_ctx[i]);
>  		}
>  	}
>  	/* WA to apply WM setting*/
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_init.c b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_init.c
> index 51a4166e9750..de9dcbeea150 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_init.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_init.c
> @@ -94,7 +94,6 @@ static const struct hw_sequencer_funcs dcn20_funcs = {
>  	.optimize_timing_for_fsft = dcn20_optimize_timing_for_fsft,
>  #endif
>  	.set_disp_pattern_generator = dcn20_set_disp_pattern_generator,
> -	.set_hubp_blank = dcn10_set_hubp_blank,
>  };
>  
>  static const struct hwseq_private_funcs dcn20_private_funcs = {
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_init.c b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_init.c
> index 0597391b2171..074e2713257f 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_init.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn21/dcn21_init.c
> @@ -99,7 +99,6 @@ static const struct hw_sequencer_funcs dcn21_funcs = {
>  #endif
>  	.is_abm_supported = dcn21_is_abm_supported,
>  	.set_disp_pattern_generator = dcn20_set_disp_pattern_generator,
> -	.set_hubp_blank = dcn10_set_hubp_blank,
>  };
>  
>  static const struct hwseq_private_funcs dcn21_private_funcs = {
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> index ab93da667d51..d84164f0000c 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.c
> @@ -938,53 +938,6 @@ void dcn30_hardware_release(struct dc *dc)
>  				dc->res_pool->hubbub, true, true);
>  }
>  
> -void dcn30_set_hubp_blank(const struct dc *dc,
> -		struct pipe_ctx *pipe_ctx,
> -		bool blank_enable)
> -{
> -	struct pipe_ctx *mpcc_pipe;
> -	struct pipe_ctx *odm_pipe;
> -
> -	if (blank_enable) {
> -		struct plane_resource *plane_res = &pipe_ctx->plane_res;
> -		struct stream_resource *stream_res = &pipe_ctx->stream_res;
> -
> -		/* Wait for enter vblank */
> -		stream_res->tg->funcs->wait_for_state(stream_res->tg, CRTC_STATE_VBLANK);
> -
> -		/* Blank HUBP to allow p-state during blank on all timings */
> -		pipe_ctx->plane_res.hubp->funcs->set_blank(pipe_ctx->plane_res.hubp, true);
> -		/* Confirm hubp in blank */
> -		ASSERT(plane_res->hubp->funcs->hubp_in_blank(plane_res->hubp));
> -		/* Toggle HUBP_DISABLE */
> -		plane_res->hubp->funcs->hubp_soft_reset(plane_res->hubp, true);
> -		plane_res->hubp->funcs->hubp_soft_reset(plane_res->hubp, false);
> -		for (mpcc_pipe = pipe_ctx->bottom_pipe; mpcc_pipe; mpcc_pipe = mpcc_pipe->bottom_pipe) {
> -			mpcc_pipe->plane_res.hubp->funcs->set_blank(mpcc_pipe->plane_res.hubp, true);
> -			/* Confirm hubp in blank */
> -			ASSERT(mpcc_pipe->plane_res.hubp->funcs->hubp_in_blank(mpcc_pipe->plane_res.hubp));
> -			/* Toggle HUBP_DISABLE */
> -			mpcc_pipe->plane_res.hubp->funcs->hubp_soft_reset(mpcc_pipe->plane_res.hubp, true);
> -			mpcc_pipe->plane_res.hubp->funcs->hubp_soft_reset(mpcc_pipe->plane_res.hubp, false);
> -
> -		}
> -		for (odm_pipe = pipe_ctx->next_odm_pipe; odm_pipe; odm_pipe = odm_pipe->next_odm_pipe) {
> -			odm_pipe->plane_res.hubp->funcs->set_blank(odm_pipe->plane_res.hubp, true);
> -			/* Confirm hubp in blank */
> -			ASSERT(odm_pipe->plane_res.hubp->funcs->hubp_in_blank(odm_pipe->plane_res.hubp));
> -			/* Toggle HUBP_DISABLE */
> -			odm_pipe->plane_res.hubp->funcs->hubp_soft_reset(odm_pipe->plane_res.hubp, true);
> -			odm_pipe->plane_res.hubp->funcs->hubp_soft_reset(odm_pipe->plane_res.hubp, false);
> -		}
> -	} else {
> -		pipe_ctx->plane_res.hubp->funcs->set_blank(pipe_ctx->plane_res.hubp, false);
> -		for (mpcc_pipe = pipe_ctx->bottom_pipe; mpcc_pipe; mpcc_pipe = mpcc_pipe->bottom_pipe)
> -			mpcc_pipe->plane_res.hubp->funcs->set_blank(mpcc_pipe->plane_res.hubp, false);
> -		for (odm_pipe = pipe_ctx->next_odm_pipe; odm_pipe; odm_pipe = odm_pipe->next_odm_pipe)
> -			odm_pipe->plane_res.hubp->funcs->set_blank(odm_pipe->plane_res.hubp, false);
> -	}
> -}
> -
>  void dcn30_set_disp_pattern_generator(const struct dc *dc,
>  		struct pipe_ctx *pipe_ctx,
>  		enum controller_dp_test_pattern test_pattern,
> @@ -994,6 +947,7 @@ void dcn30_set_disp_pattern_generator(const struct dc *dc,
>  		int width, int height, int offset)
>  {
>  	struct stream_resource *stream_res = &pipe_ctx->stream_res;
> +	struct pipe_ctx *mpcc_pipe;
>  
>  	if (test_pattern != CONTROLLER_DP_TEST_PATTERN_VIDEOMODE) {
>  		pipe_ctx->vtp_locked = false;
> @@ -1005,12 +959,20 @@ void dcn30_set_disp_pattern_generator(const struct dc *dc,
>  		if (stream_res->tg->funcs->is_tg_enabled(stream_res->tg)) {
>  			if (stream_res->tg->funcs->is_locked(stream_res->tg))
>  				pipe_ctx->vtp_locked = true;
> -			else
> -				dc->hwss.set_hubp_blank(dc, pipe_ctx, true);
> +			else {
> +				/* Blank HUBP to allow p-state during blank on all timings */
> +				pipe_ctx->plane_res.hubp->funcs->set_blank(pipe_ctx->plane_res.hubp, true);
> +
> +				for (mpcc_pipe = pipe_ctx->bottom_pipe; mpcc_pipe; mpcc_pipe = mpcc_pipe->bottom_pipe)
> +					mpcc_pipe->plane_res.hubp->funcs->set_blank(mpcc_pipe->plane_res.hubp, true);
> +			}
>  		}
>  	} else {
> -		dc->hwss.set_hubp_blank(dc, pipe_ctx, false);
>  		/* turning off DPG */
> +		pipe_ctx->plane_res.hubp->funcs->set_blank(pipe_ctx->plane_res.hubp, false);
> +		for (mpcc_pipe = pipe_ctx->bottom_pipe; mpcc_pipe; mpcc_pipe = mpcc_pipe->bottom_pipe)
> +			mpcc_pipe->plane_res.hubp->funcs->set_blank(mpcc_pipe->plane_res.hubp, false);
> +
>  		stream_res->opp->funcs->opp_set_disp_pattern_generator(stream_res->opp, test_pattern, color_space,
>  				color_depth, solid_color, width, height, offset);
>  	}
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> index 3b7d4812e311..e9a0005288d3 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_hwseq.h
> @@ -80,8 +80,4 @@ void dcn30_set_disp_pattern_generator(const struct dc *dc,
>  		const struct tg_color *solid_color,
>  		int width, int height, int offset);
>  
> -void dcn30_set_hubp_blank(const struct dc *dc,
> -		struct pipe_ctx *pipe_ctx,
> -		bool blank_enable);
> -
>  #endif /* __DC_HWSS_DCN30_H__ */
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> index 204444fead97..c4c14e9c1309 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_init.c
> @@ -98,7 +98,6 @@ static const struct hw_sequencer_funcs dcn30_funcs = {
>  	.hardware_release = dcn30_hardware_release,
>  	.set_pipe = dcn21_set_pipe,
>  	.set_disp_pattern_generator = dcn30_set_disp_pattern_generator,
> -	.set_hubp_blank = dcn30_set_hubp_blank,
>  };
>  
>  static const struct hwseq_private_funcs dcn30_private_funcs = {
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> index b8bf6d61005b..bdad72140cbc 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn301/dcn301_init.c
> @@ -98,7 +98,6 @@ static const struct hw_sequencer_funcs dcn301_funcs = {
>  	.set_abm_immediate_disable = dcn21_set_abm_immediate_disable,
>  	.set_pipe = dcn21_set_pipe,
>  	.set_disp_pattern_generator = dcn30_set_disp_pattern_generator,
> -	.set_hubp_blank = dcn30_set_hubp_blank,
>  };
>  
>  static const struct hwseq_private_funcs dcn301_private_funcs = {
> diff --git a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> index 0586ab2ffd6a..613b3e3cb46a 100644
> --- a/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> +++ b/drivers/gpu/drm/amd/display/dc/inc/hw_sequencer.h
> @@ -231,10 +231,6 @@ struct hw_sequencer_funcs {
>  			enum dc_color_depth color_depth,
>  			const struct tg_color *solid_color,
>  			int width, int height, int offset);
> -
> -	void (*set_hubp_blank)(const struct dc *dc,
> -			struct pipe_ctx *pipe_ctx,
> -			bool blank_enable);
>  };
>  
>  void color_space_to_black_color(
> -- 
> 2.30.1
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7CRodrigo.Siqueira%40amd.com%7C7700f89c242d4d5b837308d8d2d6cec7%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637491167330054893%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=uxidmH%2FvP7CIAaeYbJHtqQ1GM%2BZt5zfbNyfCWnSkhR4%3D&reserved=0

-- 
Rodrigo Siqueira
https://siqueira.tech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20210217/b662ddce/attachment.sig>


More information about the amd-gfx mailing list