[PATCH v2 1/7] drm/msm: dpu: Remove impossible checks

[PATCH v2 1/7] drm/msm: dpu: Remove impossible checks

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

This patch removes some checks which are impossible to hit. As a result,
we can move some of the local var assignments into the declarations.

Changes in v2:
- None

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 46de0de466ff..0f428f66b951 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1080,30 +1080,13 @@ static int dpu_plane_sspp_atomic_check(struct drm_plane *plane,
 		struct drm_plane_state *state)
 {
 	int ret = 0;
-	struct dpu_plane *pdpu;
-	struct dpu_plane_state *pstate;
+	struct dpu_plane *pdpu = to_dpu_plane(plane);
 	const struct dpu_format *fmt;
 	struct drm_rect src, dst, fb_rect = { 0 };
 	uint32_t max_upscale = 1, max_downscale = 1;
 	uint32_t min_src_size, max_linewidth;
 	int hscale = 1, vscale = 1;
 
-	if (!plane || !state) {
-		DPU_ERROR("invalid arg(s), plane %d state %d\n",
-				plane != 0, state != 0);
-		ret = -EINVAL;
-		goto exit;
-	}
-
-	pdpu = to_dpu_plane(plane);
-	pstate = to_dpu_plane_state(state);
-
-	if (!pdpu->pipe_sblk) {
-		DPU_ERROR_PLANE(pdpu, "invalid catalog\n");
-		ret = -EINVAL;
-		goto exit;
-	}
-
 	src.x1 = state->src_x >> 16;
 	src.y1 = state->src_y >> 16;
 	src.x2 = src.x1 + (state->src_w >> 16);
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 2/7] drm/msm: dpu: Move atomic_check_plane_state() call to atomic_check

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

src/dst rects are checked in both atomic_check and atomic_update, with
the more comprehensive check occurring in atomic_update, which is
backwards. So consolodate the checks in atomic_check.

Changes in v2:
- Use the correct crtc state (Jeykumar)

Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 57 +++++++----------------
 1 file changed, 17 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 0f428f66b951..96e042b66cd7 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1079,13 +1079,27 @@ static bool dpu_plane_validate_src(struct drm_rect *src,
 static int dpu_plane_sspp_atomic_check(struct drm_plane *plane,
 		struct drm_plane_state *state)
 {
-	int ret = 0;
+	int ret = 0, min_scale;
 	struct dpu_plane *pdpu = to_dpu_plane(plane);
+	const struct drm_crtc_state *crtc_state = NULL;
 	const struct dpu_format *fmt;
 	struct drm_rect src, dst, fb_rect = { 0 };
-	uint32_t max_upscale = 1, max_downscale = 1;
 	uint32_t min_src_size, max_linewidth;
-	int hscale = 1, vscale = 1;
+
+	if (state->crtc)
+		crtc_state = drm_atomic_get_new_crtc_state(state->state,
+							   state->crtc);
+
+	min_scale = FRAC_16_16(1, pdpu->pipe_sblk->maxdwnscale);
+	ret = drm_atomic_helper_check_plane_state(state, crtc_state, min_scale,
+					  pdpu->pipe_sblk->maxupscale << 16,
+					  true, true);
+	if (ret) {
+		DPU_ERROR_PLANE(pdpu, "Check plane state failed (%d)\n", ret);
+		return ret;
+	}
+	if (!state->visible)
+		return 0;
 
 	src.x1 = state->src_x >> 16;
 	src.y1 = state->src_y >> 16;
@@ -1099,25 +1113,6 @@ static int dpu_plane_sspp_atomic_check(struct drm_plane *plane,
 
 	max_linewidth = pdpu->pipe_sblk->common->maxlinewidth;
 
-	if (pdpu->features & DPU_SSPP_SCALER) {
-		max_downscale = pdpu->pipe_sblk->maxdwnscale;
-		max_upscale = pdpu->pipe_sblk->maxupscale;
-	}
-	if (drm_rect_width(&src) < drm_rect_width(&dst))
-		hscale = drm_rect_calc_hscale(&src, &dst, 1, max_upscale);
-	else
-		hscale = drm_rect_calc_hscale(&dst, &src, 1, max_downscale);
-	if (drm_rect_height(&src) < drm_rect_height(&dst))
-		vscale = drm_rect_calc_vscale(&src, &dst, 1, max_upscale);
-	else
-		vscale = drm_rect_calc_vscale(&dst, &src, 1, max_downscale);
-
-	DPU_DEBUG_PLANE(pdpu, "check %d -> %d\n",
-		dpu_plane_enabled(plane->state), dpu_plane_enabled(state));
-
-	if (!dpu_plane_enabled(state))
-		goto exit;
-
 	fmt = to_dpu_format(msm_framebuffer_format(state->fb));
 
 	min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
@@ -1156,16 +1151,8 @@ static int dpu_plane_sspp_atomic_check(struct drm_plane *plane,
 		DPU_ERROR_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
 				DRM_RECT_ARG(&src), max_linewidth);
 		ret = -E2BIG;
-
-	/* check scaler capability */
-	} else if (hscale < 0 || vscale < 0) {
-		DPU_ERROR_PLANE(pdpu, "invalid scaling requested src="
-				DRM_RECT_FMT " dst=" DRM_RECT_FMT "\n",
-				DRM_RECT_ARG(&src), DRM_RECT_ARG(&dst));
-		ret = -E2BIG;
 	}
 
-exit:
 	return ret;
 }
 
@@ -1237,7 +1224,6 @@ static int dpu_plane_sspp_atomic_update(struct drm_plane *plane,
 	const struct dpu_format *fmt;
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *fb;
-	int ret, min_scale;
 
 	if (!plane) {
 		DPU_ERROR("invalid plane\n");
@@ -1276,15 +1262,6 @@ static int dpu_plane_sspp_atomic_update(struct drm_plane *plane,
 	pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
 	_dpu_plane_set_qos_ctrl(plane, false, DPU_PLANE_QOS_PANIC_CTRL);
 
-	min_scale = FRAC_16_16(1, pdpu->pipe_sblk->maxdwnscale);
-	ret = drm_atomic_helper_check_plane_state(state, crtc->state, min_scale,
-					  pdpu->pipe_sblk->maxupscale << 16,
-					  true, false);
-	if (ret) {
-		DPU_ERROR_PLANE(pdpu, "Check plane state failed (%d)\n", ret);
-		return ret;
-	}
-
 	DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u " DRM_RECT_FMT
 			", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_FP_ARG(&state->src),
 			crtc->base.id, DRM_RECT_ARG(&state->dst),
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 3/7] drm/msm: dpu: Consolidate atomic_check functions()

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

dpu_plane_atomic_check() is a very thin wrapper around
dpu_plane_sspp_atomic_check(). All it does is a NULL-check of state->fb,
which is already done by drm_atomic_helper_check_plane_state(). Further,
the helper sets state->visible = false when this is true. So remove
dpu_plane_atomic_check() and just use dpu_plane_sspp_atomic_check()
directly.

Changes in v2:
- Fix spelling mistake in Subject (Jeykumar)

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>
Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 96e042b66cd7..2b1f61114e1c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1076,8 +1076,8 @@ static bool dpu_plane_validate_src(struct drm_rect *src,
 		drm_rect_equals(fb_rect, src);
 }
 
-static int dpu_plane_sspp_atomic_check(struct drm_plane *plane,
-		struct drm_plane_state *state)
+static int dpu_plane_atomic_check(struct drm_plane *plane,
+				  struct drm_plane_state *state)
 {
 	int ret = 0, min_scale;
 	struct dpu_plane *pdpu = to_dpu_plane(plane);
@@ -1156,17 +1156,6 @@ static int dpu_plane_sspp_atomic_check(struct drm_plane *plane,
 	return ret;
 }
 
-static int dpu_plane_atomic_check(struct drm_plane *plane,
-		struct drm_plane_state *state)
-{
-	if (!state->fb)
-		return 0;
-
-	DPU_DEBUG_PLANE(to_dpu_plane(plane), "\n");
-
-	return dpu_plane_sspp_atomic_check(plane, state);
-}
-
 void dpu_plane_flush(struct drm_plane *plane)
 {
 	struct dpu_plane *pdpu;
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 4/7] drm/msm: dpu: Remove dpu_plane_sspp_enabled()

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

It's doing the same thing dpu_plane_enabled() is.

Changes in v2:
- None

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 2b1f61114e1c..0c6798df1e6b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -138,11 +138,6 @@ static bool dpu_plane_enabled(struct drm_plane_state *state)
 	return state && state->fb && state->crtc;
 }
 
-static bool dpu_plane_sspp_enabled(struct drm_plane_state *state)
-{
-	return state && state->crtc;
-}
-
 /**
  * _dpu_plane_calc_fill_level - calculate fill level of the given source format
  * @plane:		Pointer to drm plane
@@ -1398,7 +1393,7 @@ static void dpu_plane_atomic_update(struct drm_plane *plane,
 
 	DPU_DEBUG_PLANE(pdpu, "\n");
 
-	if (!dpu_plane_sspp_enabled(state)) {
+	if (!dpu_plane_enabled(state)) {
 		_dpu_plane_atomic_disable(plane, old_state);
 	} else {
 		int ret;
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 5/7] drm/msm: dpu: Remove dpu_plane_enabled()

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

plane->state->visible encompasses all of these checks and more, so we
can just check visible.

Changes in v2:
- None

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 0c6798df1e6b..ce80c51b10aa 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -133,11 +133,6 @@ static struct dpu_kms *_dpu_plane_get_kms(struct drm_plane *plane)
 	return to_dpu_kms(priv->kms);
 }
 
-static bool dpu_plane_enabled(struct drm_plane_state *state)
-{
-	return state && state->fb && state->crtc;
-}
-
 /**
  * _dpu_plane_calc_fill_level - calculate fill level of the given source format
  * @plane:		Pointer to drm plane
@@ -163,7 +158,7 @@ static inline int _dpu_plane_calc_fill_level(struct drm_plane *plane,
 	fixed_buff_size = pdpu->pipe_sblk->common->pixel_ram_size;
 
 	list_for_each_entry(tmp, &pdpu->mplane_list, mplane_list) {
-		if (!dpu_plane_enabled(tmp->base.state))
+		if (!tmp->base.state->visible)
 			continue;
 		DPU_DEBUG("plane%d/%d src_width:%d/%d\n",
 				pdpu->base.base.id, tmp->base.base.id,
@@ -1393,7 +1388,7 @@ static void dpu_plane_atomic_update(struct drm_plane *plane,
 
 	DPU_DEBUG_PLANE(pdpu, "\n");
 
-	if (!dpu_plane_enabled(state)) {
+	if (!state->visible) {
 		_dpu_plane_atomic_disable(plane, old_state);
 	} else {
 		int ret;
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 6/7] drm/msm: dpu: Make dpu_plane_sspp_atomic_update() void

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

All of the checks in dpu_plane_sspp_atomic_update() are impossible, so
remove them and make the function void. This removes the need to error
check in dpu_plane_atomic_update(). Additionally, remove impossible checks
in dpu_plane_atomic_update().

Changes in v2:
- None

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 71 +++++------------------
 1 file changed, 14 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index ce80c51b10aa..2cfa5583463e 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1192,45 +1192,17 @@ void dpu_plane_set_error(struct drm_plane *plane, bool error)
 	pdpu->is_error = error;
 }
 
-static int dpu_plane_sspp_atomic_update(struct drm_plane *plane,
-				struct drm_plane_state *old_state)
+static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
+					 struct drm_plane_state *old_state)
 {
-	uint32_t nplanes, src_flags;
-	struct dpu_plane *pdpu;
-	struct drm_plane_state *state;
-	struct dpu_plane_state *pstate;
-	struct dpu_plane_state *old_pstate;
-	const struct dpu_format *fmt;
-	struct drm_crtc *crtc;
-	struct drm_framebuffer *fb;
-
-	if (!plane) {
-		DPU_ERROR("invalid plane\n");
-		return -EINVAL;
-	} else if (!plane->state) {
-		DPU_ERROR("invalid plane state\n");
-		return -EINVAL;
-	} else if (!old_state) {
-		DPU_ERROR("invalid old state\n");
-		return -EINVAL;
-	}
-
-	pdpu = to_dpu_plane(plane);
-	state = plane->state;
-
-	pstate = to_dpu_plane_state(state);
-
-	old_pstate = to_dpu_plane_state(old_state);
-
-	crtc = state->crtc;
-	fb = state->fb;
-	if (!crtc || !fb) {
-		DPU_ERROR_PLANE(pdpu, "invalid crtc %d or fb %d\n",
-				crtc != 0, fb != 0);
-		return -EINVAL;
-	}
-	fmt = to_dpu_format(msm_framebuffer_format(fb));
-	nplanes = fmt->num_planes;
+	uint32_t src_flags;
+	struct dpu_plane *pdpu = to_dpu_plane(plane);
+	struct drm_plane_state *state = plane->state;
+	struct dpu_plane_state *pstate = to_dpu_plane_state(state);
+	struct drm_crtc *crtc = state->crtc;
+	struct drm_framebuffer *fb = state->fb;
+	const struct dpu_format *fmt =
+		to_dpu_format(msm_framebuffer_format(fb));
 
 	memset(&(pdpu->pipe_cfg), 0, sizeof(struct dpu_hw_pipe_cfg));
 
@@ -1261,7 +1233,7 @@ static int dpu_plane_sspp_atomic_update(struct drm_plane *plane,
 	/* override for color fill */
 	if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) {
 		/* skip remaining processing on color fill */
-		return 0;
+		return;
 	}
 
 	if (pdpu->pipe_hw->ops.setup_rects) {
@@ -1332,7 +1304,6 @@ static int dpu_plane_sspp_atomic_update(struct drm_plane *plane,
 	}
 
 	_dpu_plane_set_qos_remap(plane);
-	return 0;
 }
 
 static void _dpu_plane_atomic_disable(struct drm_plane *plane,
@@ -1371,31 +1342,17 @@ static void _dpu_plane_atomic_disable(struct drm_plane *plane,
 static void dpu_plane_atomic_update(struct drm_plane *plane,
 				struct drm_plane_state *old_state)
 {
-	struct dpu_plane *pdpu;
-	struct drm_plane_state *state;
-
-	if (!plane) {
-		DPU_ERROR("invalid plane\n");
-		return;
-	} else if (!plane->state) {
-		DPU_ERROR("invalid plane state\n");
-		return;
-	}
+	struct dpu_plane *pdpu = to_dpu_plane(plane);
+	struct drm_plane_state *state = plane->state;
 
-	pdpu = to_dpu_plane(plane);
 	pdpu->is_error = false;
-	state = plane->state;
 
 	DPU_DEBUG_PLANE(pdpu, "\n");
 
 	if (!state->visible) {
 		_dpu_plane_atomic_disable(plane, old_state);
 	} else {
-		int ret;
-
-		ret = dpu_plane_sspp_atomic_update(plane, old_state);
-		/* atomic_check should have ensured that this doesn't fail */
-		WARN_ON(ret < 0);
+		dpu_plane_sspp_atomic_update(plane, old_state);
 	}
 }
 
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[PATCH v2 7/7] drm/msm: dpu: Don't continue after error in atomic_check

[Sean Paul]

From: Sean Paul <seanpaul@chromium.org>

There's no benefit in falling out of the if, just return directly.

Changes in v2:
- None

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
index 2cfa5583463e..1ce76460d710 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
@@ -1113,13 +1113,13 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 		 | BIT(DPU_SSPP_CSC_10BIT))))) {
 		DPU_ERROR_PLANE(pdpu,
 				"plane doesn't have scaler/csc for yuv\n");
-		ret = -EINVAL;
+		return -EINVAL;
 
 	/* check src bounds */
 	} else if (!dpu_plane_validate_src(&src, &fb_rect, min_src_size)) {
 		DPU_ERROR_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
 				DRM_RECT_ARG(&src));
-		ret = -E2BIG;
+		return -E2BIG;
 
 	/* valid yuv image */
 	} else if (DPU_FORMAT_IS_YUV(fmt) &&
@@ -1128,22 +1128,22 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
 		    drm_rect_height(&src) & 0x1)) {
 		DPU_ERROR_PLANE(pdpu, "invalid yuv source " DRM_RECT_FMT "\n",
 				DRM_RECT_ARG(&src));
-		ret = -EINVAL;
+		return -EINVAL;
 
 	/* min dst support */
 	} else if (drm_rect_width(&dst) < 0x1 || drm_rect_height(&dst) < 0x1) {
 		DPU_ERROR_PLANE(pdpu, "invalid dest rect " DRM_RECT_FMT "\n",
 				DRM_RECT_ARG(&dst));
-		ret = -EINVAL;
+		return -EINVAL;
 
 	/* check decimated source width */
 	} else if (drm_rect_width(&src) > max_linewidth) {
 		DPU_ERROR_PLANE(pdpu, "invalid src " DRM_RECT_FMT " line:%u\n",
 				DRM_RECT_ARG(&src), max_linewidth);
-		ret = -E2BIG;
+		return -E2BIG;
 	}
 
-	return ret;
+	return 0;
 }
 
 void dpu_plane_flush(struct drm_plane *plane)
-- 
Sean Paul, Software Engineer, Google / Chromium OS

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 1/7] drm/msm: dpu: Remove impossible checks

[Jeykumar Sankaran]

On 2018-09-12 06:54, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> This patch removes some checks which are impossible to hit. As a 
> result,
> we can move some of the local var assignments into the declarations.
> 
> Changes in v2:
> - None
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 19 +------------------
>  1 file changed, 1 insertion(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 46de0de466ff..0f428f66b951 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1080,30 +1080,13 @@ static int dpu_plane_sspp_atomic_check(struct
> drm_plane *plane,
>  		struct drm_plane_state *state)
>  {
>  	int ret = 0;
> -	struct dpu_plane *pdpu;
> -	struct dpu_plane_state *pstate;
> +	struct dpu_plane *pdpu = to_dpu_plane(plane);
>  	const struct dpu_format *fmt;
>  	struct drm_rect src, dst, fb_rect = { 0 };
>  	uint32_t max_upscale = 1, max_downscale = 1;
>  	uint32_t min_src_size, max_linewidth;
>  	int hscale = 1, vscale = 1;
> 
> -	if (!plane || !state) {
> -		DPU_ERROR("invalid arg(s), plane %d state %d\n",
> -				plane != 0, state != 0);
> -		ret = -EINVAL;
> -		goto exit;
> -	}
> -
> -	pdpu = to_dpu_plane(plane);
> -	pstate = to_dpu_plane_state(state);
> -
> -	if (!pdpu->pipe_sblk) {
> -		DPU_ERROR_PLANE(pdpu, "invalid catalog\n");
> -		ret = -EINVAL;
> -		goto exit;
> -	}
> -
>  	src.x1 = state->src_x >> 16;
>  	src.y1 = state->src_y >> 16;
>  	src.x2 = src.x1 + (state->src_w >> 16);

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 2/7] drm/msm: dpu: Move atomic_check_plane_state() call to atomic_check

[Jeykumar Sankaran]

On 2018-09-12 06:54, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> src/dst rects are checked in both atomic_check and atomic_update, with
> the more comprehensive check occurring in atomic_update, which is
> backwards. So consolodate the checks in atomic_check.
> 
> Changes in v2:
> - Use the correct crtc state (Jeykumar)
> 
> Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 57 +++++++----------------
>  1 file changed, 17 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 0f428f66b951..96e042b66cd7 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1079,13 +1079,27 @@ static bool dpu_plane_validate_src(struct 
> drm_rect
> *src,
>  static int dpu_plane_sspp_atomic_check(struct drm_plane *plane,
>  		struct drm_plane_state *state)
>  {
> -	int ret = 0;
> +	int ret = 0, min_scale;
>  	struct dpu_plane *pdpu = to_dpu_plane(plane);
> +	const struct drm_crtc_state *crtc_state = NULL;
>  	const struct dpu_format *fmt;
>  	struct drm_rect src, dst, fb_rect = { 0 };
> -	uint32_t max_upscale = 1, max_downscale = 1;
>  	uint32_t min_src_size, max_linewidth;
> -	int hscale = 1, vscale = 1;
> +
> +	if (state->crtc)
> +		crtc_state = drm_atomic_get_new_crtc_state(state->state,
> +							   state->crtc);
> +
> +	min_scale = FRAC_16_16(1, pdpu->pipe_sblk->maxdwnscale);
> +	ret = drm_atomic_helper_check_plane_state(state, crtc_state,
> min_scale,
> +					  pdpu->pipe_sblk->maxupscale <<
> 16,
> +					  true, true);
> +	if (ret) {
> +		DPU_ERROR_PLANE(pdpu, "Check plane state failed (%d)\n",
> ret);
> +		return ret;
> +	}
> +	if (!state->visible)
> +		return 0;
> 
>  	src.x1 = state->src_x >> 16;
>  	src.y1 = state->src_y >> 16;
> @@ -1099,25 +1113,6 @@ static int dpu_plane_sspp_atomic_check(struct
> drm_plane *plane,
> 
>  	max_linewidth = pdpu->pipe_sblk->common->maxlinewidth;
> 
> -	if (pdpu->features & DPU_SSPP_SCALER) {
> -		max_downscale = pdpu->pipe_sblk->maxdwnscale;
> -		max_upscale = pdpu->pipe_sblk->maxupscale;
> -	}
> -	if (drm_rect_width(&src) < drm_rect_width(&dst))
> -		hscale = drm_rect_calc_hscale(&src, &dst, 1, max_upscale);
> -	else
> -		hscale = drm_rect_calc_hscale(&dst, &src, 1,
> max_downscale);
> -	if (drm_rect_height(&src) < drm_rect_height(&dst))
> -		vscale = drm_rect_calc_vscale(&src, &dst, 1, max_upscale);
> -	else
> -		vscale = drm_rect_calc_vscale(&dst, &src, 1,
> max_downscale);
> -
> -	DPU_DEBUG_PLANE(pdpu, "check %d -> %d\n",
> -		dpu_plane_enabled(plane->state),
> dpu_plane_enabled(state));
> -
> -	if (!dpu_plane_enabled(state))
> -		goto exit;
> -
>  	fmt = to_dpu_format(msm_framebuffer_format(state->fb));
> 
>  	min_src_size = DPU_FORMAT_IS_YUV(fmt) ? 2 : 1;
> @@ -1156,16 +1151,8 @@ static int dpu_plane_sspp_atomic_check(struct
> drm_plane *plane,
>  		DPU_ERROR_PLANE(pdpu, "invalid src " DRM_RECT_FMT "
> line:%u\n",
>  				DRM_RECT_ARG(&src), max_linewidth);
>  		ret = -E2BIG;
> -
> -	/* check scaler capability */
> -	} else if (hscale < 0 || vscale < 0) {
> -		DPU_ERROR_PLANE(pdpu, "invalid scaling requested src="
> -				DRM_RECT_FMT " dst=" DRM_RECT_FMT "\n",
> -				DRM_RECT_ARG(&src), DRM_RECT_ARG(&dst));
> -		ret = -E2BIG;
>  	}
> 
> -exit:
>  	return ret;
>  }
> 
> @@ -1237,7 +1224,6 @@ static int dpu_plane_sspp_atomic_update(struct
> drm_plane *plane,
>  	const struct dpu_format *fmt;
>  	struct drm_crtc *crtc;
>  	struct drm_framebuffer *fb;
> -	int ret, min_scale;
> 
>  	if (!plane) {
>  		DPU_ERROR("invalid plane\n");
> @@ -1276,15 +1262,6 @@ static int dpu_plane_sspp_atomic_update(struct
> drm_plane *plane,
>  	pdpu->is_rt_pipe = (dpu_crtc_get_client_type(crtc) != NRT_CLIENT);
>  	_dpu_plane_set_qos_ctrl(plane, false, DPU_PLANE_QOS_PANIC_CTRL);
> 
> -	min_scale = FRAC_16_16(1, pdpu->pipe_sblk->maxdwnscale);
> -	ret = drm_atomic_helper_check_plane_state(state, crtc->state,
> min_scale,
> -					  pdpu->pipe_sblk->maxupscale <<
> 16,
> -					  true, false);
> -	if (ret) {
> -		DPU_ERROR_PLANE(pdpu, "Check plane state failed (%d)\n",
> ret);
> -		return ret;
> -	}
> -
>  	DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FP_FMT "->crtc%u "
> DRM_RECT_FMT
>  			", %4.4s ubwc %d\n", fb->base.id,
> DRM_RECT_FP_ARG(&state->src),
>  			crtc->base.id, DRM_RECT_ARG(&state->dst),

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 3/7] drm/msm: dpu: Consolidate atomic_check functions()

[Jeykumar Sankaran]

On 2018-09-12 06:54, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> dpu_plane_atomic_check() is a very thin wrapper around
> dpu_plane_sspp_atomic_check(). All it does is a NULL-check of 
> state->fb,
> which is already done by drm_atomic_helper_check_plane_state(). 
> Further,
> the helper sets state->visible = false when this is true. So remove
> dpu_plane_atomic_check() and just use dpu_plane_sspp_atomic_check()
> directly.
> 
> Changes in v2:
> - Fix spelling mistake in Subject (Jeykumar)
> 
> Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 96e042b66cd7..2b1f61114e1c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1076,8 +1076,8 @@ static bool dpu_plane_validate_src(struct 
> drm_rect
> *src,
>  		drm_rect_equals(fb_rect, src);
>  }
> 
> -static int dpu_plane_sspp_atomic_check(struct drm_plane *plane,
> -		struct drm_plane_state *state)
> +static int dpu_plane_atomic_check(struct drm_plane *plane,
> +				  struct drm_plane_state *state)
>  {
>  	int ret = 0, min_scale;
>  	struct dpu_plane *pdpu = to_dpu_plane(plane);
> @@ -1156,17 +1156,6 @@ static int dpu_plane_sspp_atomic_check(struct
> drm_plane *plane,
>  	return ret;
>  }
> 
> -static int dpu_plane_atomic_check(struct drm_plane *plane,
> -		struct drm_plane_state *state)
> -{
> -	if (!state->fb)
> -		return 0;
> -
> -	DPU_DEBUG_PLANE(to_dpu_plane(plane), "\n");
> -
> -	return dpu_plane_sspp_atomic_check(plane, state);
> -}
> -
>  void dpu_plane_flush(struct drm_plane *plane)
>  {
>  	struct dpu_plane *pdpu;

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 4/7] drm/msm: dpu: Remove dpu_plane_sspp_enabled()

[Jeykumar Sankaran]

On 2018-09-12 06:54, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> It's doing the same thing dpu_plane_enabled() is.
> 
> Changes in v2:
> - None
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 2b1f61114e1c..0c6798df1e6b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -138,11 +138,6 @@ static bool dpu_plane_enabled(struct 
> drm_plane_state
> *state)
>  	return state && state->fb && state->crtc;
>  }
> 
> -static bool dpu_plane_sspp_enabled(struct drm_plane_state *state)
> -{
> -	return state && state->crtc;
> -}
> -
>  /**
>   * _dpu_plane_calc_fill_level - calculate fill level of the given 
> source
> format
>   * @plane:		Pointer to drm plane
> @@ -1398,7 +1393,7 @@ static void dpu_plane_atomic_update(struct 
> drm_plane
> *plane,
> 
>  	DPU_DEBUG_PLANE(pdpu, "\n");
> 
> -	if (!dpu_plane_sspp_enabled(state)) {
> +	if (!dpu_plane_enabled(state)) {
>  		_dpu_plane_atomic_disable(plane, old_state);
>  	} else {
>  		int ret;

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 5/7] drm/msm: dpu: Remove dpu_plane_enabled()

[Jeykumar Sankaran]

On 2018-09-12 06:54, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> plane->state->visible encompasses all of these checks and more, so we
> can just check visible.
> 
> Changes in v2:
> - None
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 0c6798df1e6b..ce80c51b10aa 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -133,11 +133,6 @@ static struct dpu_kms *_dpu_plane_get_kms(struct
> drm_plane *plane)
>  	return to_dpu_kms(priv->kms);
>  }
> 
> -static bool dpu_plane_enabled(struct drm_plane_state *state)
> -{
> -	return state && state->fb && state->crtc;
> -}
> -
>  /**
>   * _dpu_plane_calc_fill_level - calculate fill level of the given 
> source
> format
>   * @plane:		Pointer to drm plane
> @@ -163,7 +158,7 @@ static inline int _dpu_plane_calc_fill_level(struct
> drm_plane *plane,
>  	fixed_buff_size = pdpu->pipe_sblk->common->pixel_ram_size;
> 
>  	list_for_each_entry(tmp, &pdpu->mplane_list, mplane_list) {
> -		if (!dpu_plane_enabled(tmp->base.state))
> +		if (!tmp->base.state->visible)
>  			continue;
>  		DPU_DEBUG("plane%d/%d src_width:%d/%d\n",
>  				pdpu->base.base.id, tmp->base.base.id,
> @@ -1393,7 +1388,7 @@ static void dpu_plane_atomic_update(struct 
> drm_plane
> *plane,
> 
>  	DPU_DEBUG_PLANE(pdpu, "\n");
> 
> -	if (!dpu_plane_enabled(state)) {
> +	if (!state->visible) {
>  		_dpu_plane_atomic_disable(plane, old_state);
>  	} else {
>  		int ret;

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 6/7] drm/msm: dpu: Make dpu_plane_sspp_atomic_update() void

[Jeykumar Sankaran]

On 2018-09-12 06:54, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> All of the checks in dpu_plane_sspp_atomic_update() are impossible, so
> remove them and make the function void. This removes the need to error
> check in dpu_plane_atomic_update(). Additionally, remove impossible 
> checks
> in dpu_plane_atomic_update().
> 
> Changes in v2:
> - None
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 71 +++++------------------
>  1 file changed, 14 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index ce80c51b10aa..2cfa5583463e 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1192,45 +1192,17 @@ void dpu_plane_set_error(struct drm_plane 
> *plane,
> bool error)
>  	pdpu->is_error = error;
>  }
> 
> -static int dpu_plane_sspp_atomic_update(struct drm_plane *plane,
> -				struct drm_plane_state *old_state)
> +static void dpu_plane_sspp_atomic_update(struct drm_plane *plane,
> +					 struct drm_plane_state
> *old_state)
>  {
> -	uint32_t nplanes, src_flags;
> -	struct dpu_plane *pdpu;
> -	struct drm_plane_state *state;
> -	struct dpu_plane_state *pstate;
> -	struct dpu_plane_state *old_pstate;
> -	const struct dpu_format *fmt;
> -	struct drm_crtc *crtc;
> -	struct drm_framebuffer *fb;
> -
> -	if (!plane) {
> -		DPU_ERROR("invalid plane\n");
> -		return -EINVAL;
> -	} else if (!plane->state) {
> -		DPU_ERROR("invalid plane state\n");
> -		return -EINVAL;
> -	} else if (!old_state) {
> -		DPU_ERROR("invalid old state\n");
> -		return -EINVAL;
> -	}
> -
> -	pdpu = to_dpu_plane(plane);
> -	state = plane->state;
> -
> -	pstate = to_dpu_plane_state(state);
> -
> -	old_pstate = to_dpu_plane_state(old_state);
> -
> -	crtc = state->crtc;
> -	fb = state->fb;
> -	if (!crtc || !fb) {
> -		DPU_ERROR_PLANE(pdpu, "invalid crtc %d or fb %d\n",
> -				crtc != 0, fb != 0);
> -		return -EINVAL;
> -	}
> -	fmt = to_dpu_format(msm_framebuffer_format(fb));
> -	nplanes = fmt->num_planes;
> +	uint32_t src_flags;
> +	struct dpu_plane *pdpu = to_dpu_plane(plane);
> +	struct drm_plane_state *state = plane->state;
> +	struct dpu_plane_state *pstate = to_dpu_plane_state(state);
> +	struct drm_crtc *crtc = state->crtc;
> +	struct drm_framebuffer *fb = state->fb;
> +	const struct dpu_format *fmt =
> +		to_dpu_format(msm_framebuffer_format(fb));
> 
>  	memset(&(pdpu->pipe_cfg), 0, sizeof(struct dpu_hw_pipe_cfg));
> 
> @@ -1261,7 +1233,7 @@ static int dpu_plane_sspp_atomic_update(struct
> drm_plane *plane,
>  	/* override for color fill */
>  	if (pdpu->color_fill & DPU_PLANE_COLOR_FILL_FLAG) {
>  		/* skip remaining processing on color fill */
> -		return 0;
> +		return;
>  	}
> 
>  	if (pdpu->pipe_hw->ops.setup_rects) {
> @@ -1332,7 +1304,6 @@ static int dpu_plane_sspp_atomic_update(struct
> drm_plane *plane,
>  	}
> 
>  	_dpu_plane_set_qos_remap(plane);
> -	return 0;
>  }
> 
>  static void _dpu_plane_atomic_disable(struct drm_plane *plane,
> @@ -1371,31 +1342,17 @@ static void _dpu_plane_atomic_disable(struct
> drm_plane *plane,
>  static void dpu_plane_atomic_update(struct drm_plane *plane,
>  				struct drm_plane_state *old_state)
>  {
> -	struct dpu_plane *pdpu;
> -	struct drm_plane_state *state;
> -
> -	if (!plane) {
> -		DPU_ERROR("invalid plane\n");
> -		return;
> -	} else if (!plane->state) {
> -		DPU_ERROR("invalid plane state\n");
> -		return;
> -	}
> +	struct dpu_plane *pdpu = to_dpu_plane(plane);
> +	struct drm_plane_state *state = plane->state;
> 
> -	pdpu = to_dpu_plane(plane);
>  	pdpu->is_error = false;
> -	state = plane->state;
> 
>  	DPU_DEBUG_PLANE(pdpu, "\n");
> 
>  	if (!state->visible) {
>  		_dpu_plane_atomic_disable(plane, old_state);
>  	} else {
> -		int ret;
> -
> -		ret = dpu_plane_sspp_atomic_update(plane, old_state);
> -		/* atomic_check should have ensured that this doesn't fail
> */
> -		WARN_ON(ret < 0);
> +		dpu_plane_sspp_atomic_update(plane, old_state);
>  	}
>  }

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 7/7] drm/msm: dpu: Don't continue after error in atomic_check

[Jeykumar Sankaran]

On 2018-09-12 06:54, Sean Paul wrote:
> From: Sean Paul <seanpaul@chromium.org>
> 
> There's no benefit in falling out of the if, just return directly.
> 
> Changes in v2:
> - None
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---

Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

>  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index 2cfa5583463e..1ce76460d710 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -1113,13 +1113,13 @@ static int dpu_plane_atomic_check(struct 
> drm_plane
> *plane,
>  		 | BIT(DPU_SSPP_CSC_10BIT))))) {
>  		DPU_ERROR_PLANE(pdpu,
>  				"plane doesn't have scaler/csc for
> yuv\n");
> -		ret = -EINVAL;
> +		return -EINVAL;
> 
>  	/* check src bounds */
>  	} else if (!dpu_plane_validate_src(&src, &fb_rect, min_src_size))
> {
>  		DPU_ERROR_PLANE(pdpu, "invalid source " DRM_RECT_FMT "\n",
>  				DRM_RECT_ARG(&src));
> -		ret = -E2BIG;
> +		return -E2BIG;
> 
>  	/* valid yuv image */
>  	} else if (DPU_FORMAT_IS_YUV(fmt) &&
> @@ -1128,22 +1128,22 @@ static int dpu_plane_atomic_check(struct 
> drm_plane
> *plane,
>  		    drm_rect_height(&src) & 0x1)) {
>  		DPU_ERROR_PLANE(pdpu, "invalid yuv source " DRM_RECT_FMT
> "\n",
>  				DRM_RECT_ARG(&src));
> -		ret = -EINVAL;
> +		return -EINVAL;
> 
>  	/* min dst support */
>  	} else if (drm_rect_width(&dst) < 0x1 || drm_rect_height(&dst) <
> 0x1) {
>  		DPU_ERROR_PLANE(pdpu, "invalid dest rect " DRM_RECT_FMT
> "\n",
>  				DRM_RECT_ARG(&dst));
> -		ret = -EINVAL;
> +		return -EINVAL;
> 
>  	/* check decimated source width */
>  	} else if (drm_rect_width(&src) > max_linewidth) {
>  		DPU_ERROR_PLANE(pdpu, "invalid src " DRM_RECT_FMT "
> line:%u\n",
>  				DRM_RECT_ARG(&src), max_linewidth);
> -		ret = -E2BIG;
> +		return -E2BIG;
>  	}
> 
> -	return ret;
> +	return 0;
>  }
> 
>  void dpu_plane_flush(struct drm_plane *plane)

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH v2 1/7] drm/msm: dpu: Remove impossible checks

[Sean Paul]

On Thu, Sep 13, 2018 at 03:23:47PM -0700, Jeykumar Sankaran wrote:
> On 2018-09-12 06:54, Sean Paul wrote:
> > From: Sean Paul <seanpaul@chromium.org>
> > 
> > This patch removes some checks which are impossible to hit. As a result,
> > we can move some of the local var assignments into the declarations.
> > 
> > Changes in v2:
> > - None
> > 
> > Signed-off-by: Sean Paul <seanpaul@chromium.org>
> > ---
> 
> Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org>

Thanks for the reviews! I've pushed the whole stack to dpu-staging

Sean

> 
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 19 +------------------
> >  1 file changed, 1 insertion(+), 18 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > index 46de0de466ff..0f428f66b951 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -1080,30 +1080,13 @@ static int dpu_plane_sspp_atomic_check(struct
> > drm_plane *plane,
> >  		struct drm_plane_state *state)
> >  {
> >  	int ret = 0;
> > -	struct dpu_plane *pdpu;
> > -	struct dpu_plane_state *pstate;
> > +	struct dpu_plane *pdpu = to_dpu_plane(plane);
> >  	const struct dpu_format *fmt;
> >  	struct drm_rect src, dst, fb_rect = { 0 };
> >  	uint32_t max_upscale = 1, max_downscale = 1;
> >  	uint32_t min_src_size, max_linewidth;
> >  	int hscale = 1, vscale = 1;
> > 
> > -	if (!plane || !state) {
> > -		DPU_ERROR("invalid arg(s), plane %d state %d\n",
> > -				plane != 0, state != 0);
> > -		ret = -EINVAL;
> > -		goto exit;
> > -	}
> > -
> > -	pdpu = to_dpu_plane(plane);
> > -	pstate = to_dpu_plane_state(state);
> > -
> > -	if (!pdpu->pipe_sblk) {
> > -		DPU_ERROR_PLANE(pdpu, "invalid catalog\n");
> > -		ret = -EINVAL;
> > -		goto exit;
> > -	}
> > -
> >  	src.x1 = state->src_x >> 16;
> >  	src.y1 = state->src_y >> 16;
> >  	src.x2 = src.x1 + (state->src_w >> 16);
> 
> -- 
> Jeykumar S
> _______________________________________________
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno