diff mbox series

[11/17] drm/msm/dpu: drop src_split and multirect check from dpu_crtc_atomic_check

Message ID 20210624145733.2561992-12-dmitry.baryshkov@linaro.org (mailing list archive)
State Superseded, archived
Headers show
Series [01/17] drm/msm/dpu: move LUT levels out of QOS config | expand

Commit Message

Dmitry Baryshkov June 24, 2021, 2:57 p.m. UTC 
Neither source split nor multirect are properly supported at this
moment. Both of these checks depend on zpos being equal for several
planes (which is a clear userspace bug). Drop these checks to simplify
dpu_crtc_atomic_check(). The actual support for either of these features
is not removed from the backend code (sspp, ctl, etc).

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 123 +----------------------
 1 file changed, 3 insertions(+), 120 deletions(-)

Comments

kernel test robot June 25, 2021, 3:41 a.m. UTC | #1 
Hi Dmitry,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20210624]
[also build test WARNING on v5.13-rc7]
[cannot apply to linus/master v5.13-rc7 v5.13-rc6 v5.13-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dmitry-Baryshkov/drm-msm-dpu-switch-dpu_plane-to-be-virtual/20210624-225947
base:    2a8927f0efb6fb34b9d11dab3bd3f018e866d36d
config: arm-defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/3842e184f54916b9d22989d840a70bfb0bfebf10
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dmitry-Baryshkov/drm-msm-dpu-switch-dpu_plane-to-be-virtual/20210624-225947
        git checkout 3842e184f54916b9d22989d840a70bfb0bfebf10
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c: In function 'dpu_crtc_atomic_check':
>> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c:898:23: warning: variable 'mixer_width' set but not used [-Wunused-but-set-variable]
     898 |  int cnt = 0, rc = 0, mixer_width = 0, i;
         |                       ^~~~~~~~~~~


vim +/mixer_width +898 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c

   884	
   885	static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
   886			struct drm_atomic_state *state)
   887	{
   888		struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state,
   889										  crtc);
   890		struct dpu_crtc *dpu_crtc = to_dpu_crtc(crtc);
   891		struct dpu_crtc_state *cstate = to_dpu_crtc_state(crtc_state);
   892		struct plane_state *pstates;
   893	
   894		const struct drm_plane_state *pstate;
   895		struct drm_plane *plane;
   896		struct drm_display_mode *mode;
   897	
 > 898		int cnt = 0, rc = 0, mixer_width = 0, i;
   899	
   900		struct drm_rect crtc_rect = { 0 };
   901	
   902		pstates = kzalloc(sizeof(*pstates) * DPU_STAGE_MAX * 4, GFP_KERNEL);
   903	
   904		if (!crtc_state->enable || !crtc_state->active) {
   905			DRM_DEBUG_ATOMIC("crtc%d -> enable %d, active %d, skip atomic_check\n",
   906					crtc->base.id, crtc_state->enable,
   907					crtc_state->active);
   908			memset(&cstate->new_perf, 0, sizeof(cstate->new_perf));
   909			goto end;
   910		}
   911	
   912		mode = &crtc_state->adjusted_mode;
   913		DRM_DEBUG_ATOMIC("%s: check\n", dpu_crtc->name);
   914	
   915		/* force a full mode set if active state changed */
   916		if (crtc_state->active_changed)
   917			crtc_state->mode_changed = true;
   918	
   919		if (cstate->num_mixers) {
   920			mixer_width = mode->hdisplay / cstate->num_mixers;
   921	
   922			_dpu_crtc_setup_lm_bounds(crtc, crtc_state);
   923		}
   924	
   925		crtc_rect.x2 = mode->hdisplay;
   926		crtc_rect.y2 = mode->vdisplay;
   927	
   928		 /* get plane state for all drm planes associated with crtc state */
   929		drm_atomic_crtc_state_for_each_plane_state(plane, pstate, crtc_state) {
   930			struct drm_rect dst, clip = crtc_rect;
   931	
   932			if (IS_ERR_OR_NULL(pstate)) {
   933				rc = PTR_ERR(pstate);
   934				DPU_ERROR("%s: failed to get plane%d state, %d\n",
   935						dpu_crtc->name, plane->base.id, rc);
   936				goto end;
   937			}
   938			if (cnt >= DPU_STAGE_MAX * 4)
   939				continue;
   940	
   941			pstates[cnt].dpu_pstate = to_dpu_plane_state(pstate);
   942			pstates[cnt].drm_pstate = pstate;
   943			pstates[cnt].stage = pstate->normalized_zpos;
   944	
   945			dpu_plane_clear_multirect(pstate);
   946	
   947			cnt++;
   948	
   949			dst = drm_plane_state_dest(pstate);
   950			if (!drm_rect_intersect(&clip, &dst)) {
   951				DPU_ERROR("invalid vertical/horizontal destination\n");
   952				DPU_ERROR("display: " DRM_RECT_FMT " plane: "
   953					  DRM_RECT_FMT "\n", DRM_RECT_ARG(&crtc_rect),
   954					  DRM_RECT_ARG(&dst));
   955				rc = -E2BIG;
   956				goto end;
   957			}
   958		}
   959	
   960		for (i = 0; i < cnt; i++) {
   961			int z_pos = pstates[i].stage;
   962	
   963			/* verify z_pos setting before using it */
   964			if (z_pos >= DPU_STAGE_MAX - DPU_STAGE_0) {
   965				DPU_ERROR("> %d plane stages assigned\n",
   966						DPU_STAGE_MAX - DPU_STAGE_0);
   967				rc = -EINVAL;
   968				goto end;
   969			}
   970	
   971			pstates[i].dpu_pstate->stage = z_pos + DPU_STAGE_0;
   972			DRM_DEBUG_ATOMIC("%s: zpos %d\n", dpu_crtc->name, z_pos);
   973		}
   974	
   975		atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
   976	
   977		rc = dpu_core_perf_crtc_check(crtc, crtc_state);
   978		if (rc) {
   979			DPU_ERROR("crtc%d failed performance check %d\n",
   980					crtc->base.id, rc);
   981			goto end;
   982		}
   983	
   984	end:
   985		kfree(pstates);
   986		return rc;
   987	}
   988	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 6fe0af9ffc23..b62e8e4e8d0a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -895,12 +895,8 @@  static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 	struct drm_plane *plane;
 	struct drm_display_mode *mode;
 
-	int cnt = 0, rc = 0, mixer_width = 0, i, z_pos;
+	int cnt = 0, rc = 0, mixer_width = 0, i;
 
-	struct dpu_multirect_plane_states multirect_plane[DPU_STAGE_MAX * 2];
-	int multirect_count = 0;
-	const struct drm_plane_state *pipe_staged[SSPP_MAX];
-	int left_zpos_cnt = 0, right_zpos_cnt = 0;
 	struct drm_rect crtc_rect = { 0 };
 
 	pstates = kzalloc(sizeof(*pstates) * DPU_STAGE_MAX * 4, GFP_KERNEL);
@@ -920,8 +916,6 @@  static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 	if (crtc_state->active_changed)
 		crtc_state->mode_changed = true;
 
-	memset(pipe_staged, 0, sizeof(pipe_staged));
-
 	if (cstate->num_mixers) {
 		mixer_width = mode->hdisplay / cstate->num_mixers;
 
@@ -947,18 +941,8 @@  static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 		pstates[cnt].dpu_pstate = to_dpu_plane_state(pstate);
 		pstates[cnt].drm_pstate = pstate;
 		pstates[cnt].stage = pstate->normalized_zpos;
-		pstates[cnt].pipe_id = dpu_plane_pipe(plane);
-
-		if (pipe_staged[pstates[cnt].pipe_id]) {
-			multirect_plane[multirect_count].r0 =
-				pipe_staged[pstates[cnt].pipe_id];
-			multirect_plane[multirect_count].r1 = pstate;
-			multirect_count++;
 
-			pipe_staged[pstates[cnt].pipe_id] = NULL;
-		} else {
-			pipe_staged[pstates[cnt].pipe_id] = pstate;
-		}
+		dpu_plane_clear_multirect(pstate);
 
 		cnt++;
 
@@ -973,19 +957,8 @@  static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 		}
 	}
 
-	for (i = 1; i < SSPP_MAX; i++) {
-		if (pipe_staged[i])
-			dpu_plane_clear_multirect(pipe_staged[i]);
-	}
-
-	z_pos = -1;
 	for (i = 0; i < cnt; i++) {
-		/* reset counts at every new blend stage */
-		if (pstates[i].stage != z_pos) {
-			left_zpos_cnt = 0;
-			right_zpos_cnt = 0;
-			z_pos = pstates[i].stage;
-		}
+		int z_pos = pstates[i].stage;
 
 		/* verify z_pos setting before using it */
 		if (z_pos >= DPU_STAGE_MAX - DPU_STAGE_0) {
@@ -993,40 +966,12 @@  static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 					DPU_STAGE_MAX - DPU_STAGE_0);
 			rc = -EINVAL;
 			goto end;
-		} else if (pstates[i].drm_pstate->crtc_x < mixer_width) {
-			if (left_zpos_cnt == 2) {
-				DPU_ERROR("> 2 planes @ stage %d on left\n",
-					z_pos);
-				rc = -EINVAL;
-				goto end;
-			}
-			left_zpos_cnt++;
-
-		} else {
-			if (right_zpos_cnt == 2) {
-				DPU_ERROR("> 2 planes @ stage %d on right\n",
-					z_pos);
-				rc = -EINVAL;
-				goto end;
-			}
-			right_zpos_cnt++;
 		}
 
 		pstates[i].dpu_pstate->stage = z_pos + DPU_STAGE_0;
 		DRM_DEBUG_ATOMIC("%s: zpos %d\n", dpu_crtc->name, z_pos);
 	}
 
-	for (i = 0; i < multirect_count; i++) {
-		if (dpu_plane_validate_multirect_v2(&multirect_plane[i])) {
-			DPU_ERROR(
-			"multirect validation failed for planes (%d - %d)\n",
-					multirect_plane[i].r0->plane->base.id,
-					multirect_plane[i].r1->plane->base.id);
-			rc = -EINVAL;
-			goto end;
-		}
-	}
-
 	atomic_inc(&_dpu_crtc_get_kms(crtc)->bandwidth_ref);
 
 	rc = dpu_core_perf_crtc_check(crtc, crtc_state);
@@ -1036,68 +981,6 @@  static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
 		goto end;
 	}
 
-	/* validate source split:
-	 * use pstates sorted by stage to check planes on same stage
-	 * we assume that all pipes are in source split so its valid to compare
-	 * without taking into account left/right mixer placement
-	 */
-	for (i = 1; i < cnt; i++) {
-		struct plane_state *prv_pstate, *cur_pstate;
-		struct drm_rect left_rect, right_rect;
-		int32_t left_pid, right_pid;
-		int32_t stage;
-
-		prv_pstate = &pstates[i - 1];
-		cur_pstate = &pstates[i];
-		if (prv_pstate->stage != cur_pstate->stage)
-			continue;
-
-		stage = cur_pstate->stage;
-
-		left_pid = prv_pstate->dpu_pstate->base.plane->base.id;
-		left_rect = drm_plane_state_dest(prv_pstate->drm_pstate);
-
-		right_pid = cur_pstate->dpu_pstate->base.plane->base.id;
-		right_rect = drm_plane_state_dest(cur_pstate->drm_pstate);
-
-		if (right_rect.x1 < left_rect.x1) {
-			swap(left_pid, right_pid);
-			swap(left_rect, right_rect);
-		}
-
-		/**
-		 * - planes are enumerated in pipe-priority order such that
-		 *   planes with lower drm_id must be left-most in a shared
-		 *   blend-stage when using source split.
-		 * - planes in source split must be contiguous in width
-		 * - planes in source split must have same dest yoff and height
-		 */
-		if (right_pid < left_pid) {
-			DPU_ERROR(
-				"invalid src split cfg. priority mismatch. stage: %d left: %d right: %d\n",
-				stage, left_pid, right_pid);
-			rc = -EINVAL;
-			goto end;
-		} else if (right_rect.x1 != drm_rect_width(&left_rect)) {
-			DPU_ERROR("non-contiguous coordinates for src split. "
-				  "stage: %d left: " DRM_RECT_FMT " right: "
-				  DRM_RECT_FMT "\n", stage,
-				  DRM_RECT_ARG(&left_rect),
-				  DRM_RECT_ARG(&right_rect));
-			rc = -EINVAL;
-			goto end;
-		} else if (left_rect.y1 != right_rect.y1 ||
-			   drm_rect_height(&left_rect) != drm_rect_height(&right_rect)) {
-			DPU_ERROR("source split at stage: %d. invalid "
-				  "yoff/height: left: " DRM_RECT_FMT " right: "
-				  DRM_RECT_FMT "\n", stage,
-				  DRM_RECT_ARG(&left_rect),
-				  DRM_RECT_ARG(&right_rect));
-			rc = -EINVAL;
-			goto end;
-		}
-	}
-
 end:
 	kfree(pstates);
 	return rc;