Commit 9e4dde28 authored by Rob Clark's avatar Rob Clark
Browse files

drm/msm: Avoid dirtyfb stalls on video mode displays (v2) 



Someone on IRC once asked an innocent enough sounding question:  Why
with xf86-video-modesetting is es2gears limited at 120fps.

So I broke out the perfetto tracing mesa MR and took a look.  It turns
out the problem was drm_atomic_helper_dirtyfb(), which would end up
waiting for vblank.. es2gears would rapidly push two frames to Xorg,
which would blit them to screen and in idle hook (I assume) call the
DIRTYFB ioctl.  Which in turn would do an atomic update to flush the
dirty rects, which would stall until the next vblank.  And then the
whole process would repeat.

But this is a bit silly, we only need dirtyfb for command mode DSI
panels.  So track in plane state whether dirtyfb is required, and
track in the fb how many attached planes require dirtyfb so that we
can skip it when not required.  (Note, mdp4 does not actually have
cmd mode support.)

Signed-off-by: default avatarRob Clark <robdclark@chromium.org>
Reviewed-by: default avatarDmitry Baryshkov <dmitry.baryshkov@linaro.org>
Link: https://lore.kernel.org/r/20220223191118.881321-1-robdclark@gmail.com


Signed-off-by: default avatarRob Clark <robdclark@chromium.org>
parent db22583d

drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c

+19 −1
Original line number Diff line number Diff line
@@ -1046,6 +1046,20 @@ struct plane_state { 
	u32 pipe_id;

};



static bool dpu_crtc_needs_dirtyfb(struct drm_crtc_state *cstate)

{

	struct drm_crtc *crtc = cstate->crtc;

	struct drm_encoder *encoder;



	drm_for_each_encoder_mask (encoder, crtc->dev, cstate->encoder_mask) {

		if (dpu_encoder_get_intf_mode(encoder) == INTF_MODE_CMD) {

			return true;

		}

	}



	return false;

}



static int dpu_crtc_atomic_check(struct drm_crtc *crtc,

		struct drm_atomic_state *state)

{
@@ -1066,6 +1080,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc, 
	const struct drm_plane_state *pipe_staged[SSPP_MAX];

	int left_zpos_cnt = 0, right_zpos_cnt = 0;

	struct drm_rect crtc_rect = { 0 };

	bool needs_dirtyfb = dpu_crtc_needs_dirtyfb(crtc_state);



	pstates = kzalloc(sizeof(*pstates) * DPU_STAGE_MAX * 4, GFP_KERNEL);
@@ -1097,6 +1112,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc, 


	 /* get plane state for all drm planes associated with crtc state */

	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {

		struct dpu_plane_state *dpu_pstate = to_dpu_plane_state(pstate);

		struct drm_rect dst, clip = crtc_rect;



		if (IS_ERR_OR_NULL(pstate)) {
@@ -1108,11 +1124,13 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc, 
		if (cnt >= DPU_STAGE_MAX * 4)

			continue;



		pstates[cnt].dpu_pstate = to_dpu_plane_state(pstate);

		pstates[cnt].dpu_pstate = dpu_pstate;

		pstates[cnt].drm_pstate = pstate;

		pstates[cnt].stage = pstate->normalized_zpos;

		pstates[cnt].pipe_id = dpu_plane_pipe(plane);



		dpu_pstate->needs_dirtyfb = needs_dirtyfb;



		if (pipe_staged[pstates[cnt].pipe_id]) {

			multirect_plane[multirect_count].r0 =

				pipe_staged[pstates[cnt].pipe_id];

drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c

+3 −2
Original line number Diff line number Diff line
@@ -902,7 +902,7 @@ static int dpu_plane_prepare_fb(struct drm_plane *plane, 


	if (pstate->aspace) {

		ret = msm_framebuffer_prepare(new_state->fb,

				pstate->aspace);

				pstate->aspace, pstate->needs_dirtyfb);

		if (ret) {

			DPU_ERROR("failed to prepare framebuffer\n");

			return ret;
@@ -933,7 +933,8 @@ static void dpu_plane_cleanup_fb(struct drm_plane *plane, 


	DPU_DEBUG_PLANE(pdpu, "FB[%u]\n", old_state->fb->base.id);



	msm_framebuffer_cleanup(old_state->fb, old_pstate->aspace);

	msm_framebuffer_cleanup(old_state->fb, old_pstate->aspace,

				old_pstate->needs_dirtyfb);

}



static bool dpu_plane_validate_src(struct drm_rect *src,

drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h

+3 −0
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ 
 * @pending:	whether the current update is still pending

 * @plane_fetch_bw: calculated BW per plane

 * @plane_clk: calculated clk per plane

 * @needs_dirtyfb: whether attached CRTC needs pixel data explicitly flushed

 */

struct dpu_plane_state {

	struct drm_plane_state base;
@@ -37,6 +38,8 @@ struct dpu_plane_state { 


	u64 plane_fetch_bw;

	u64 plane_clk;



	bool needs_dirtyfb;

};



/**

drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c

+17 −2
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ 
#include <drm/drm_atomic.h>

#include <drm/drm_damage_helper.h>

#include <drm/drm_fourcc.h>

#include <drm/drm_gem_atomic_helper.h>



#include "mdp4_kms.h"
@@ -90,6 +91,20 @@ static const struct drm_plane_funcs mdp4_plane_funcs = { 
		.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,

};



static int mdp4_plane_prepare_fb(struct drm_plane *plane,

				 struct drm_plane_state *new_state)

{

	struct msm_drm_private *priv = plane->dev->dev_private;

	struct msm_kms *kms = priv->kms;



	if (!new_state->fb)

		return 0;



	drm_gem_plane_helper_prepare_fb(plane, new_state);



	return msm_framebuffer_prepare(new_state->fb, kms->aspace, false);

}



static void mdp4_plane_cleanup_fb(struct drm_plane *plane,

				  struct drm_plane_state *old_state)

{
@@ -102,7 +117,7 @@ static void mdp4_plane_cleanup_fb(struct drm_plane *plane, 
		return;



	DBG("%s: cleanup: FB[%u]", mdp4_plane->name, fb->base.id);

	msm_framebuffer_cleanup(fb, kms->aspace);

	msm_framebuffer_cleanup(fb, kms->aspace, false);

}
@@ -130,7 +145,7 @@ static void mdp4_plane_atomic_update(struct drm_plane *plane, 
}



static const struct drm_plane_helper_funcs mdp4_plane_helper_funcs = {

		.prepare_fb = msm_atomic_prepare_fb,

		.prepare_fb = mdp4_plane_prepare_fb,

		.cleanup_fb = mdp4_plane_cleanup_fb,

		.atomic_check = mdp4_plane_atomic_check,

		.atomic_update = mdp4_plane_atomic_update,

drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c

+8 −0
Original line number Diff line number Diff line
@@ -690,6 +690,8 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc, 
{

	struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,

									  crtc);

	struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc_state);

	struct mdp5_interface *intf = mdp5_cstate->pipeline.intf;

	struct mdp5_kms *mdp5_kms = get_kms(crtc);

	struct drm_plane *plane;

	struct drm_device *dev = crtc->dev;
@@ -706,12 +708,18 @@ static int mdp5_crtc_atomic_check(struct drm_crtc *crtc, 
	DBG("%s: check", crtc->name);



	drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {

		struct mdp5_plane_state *mdp5_pstate =

				to_mdp5_plane_state(pstate);



		if (!pstate->visible)

			continue;



		pstates[cnt].plane = plane;

		pstates[cnt].state = to_mdp5_plane_state(pstate);



		mdp5_pstate->needs_dirtyfb =

			intf->mode == MDP5_INTF_DSI_MODE_COMMAND;



		/*

		 * if any plane on this crtc uses 2 hwpipes, then we need

		 * the crtc to have a right hwmixer.