* [PATCH v2 1/7] drm/msm: dpu: Remove impossible checks
@ 2018-09-12 13:54 Sean Paul
[not found] ` <20180912135459.216217-1-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Sean Paul @ 2018-09-12 13:54 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
Sean Paul, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/7] drm/msm: dpu: Move atomic_check_plane_state() call to atomic_check
[not found] ` <20180912135459.216217-1-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
@ 2018-09-12 13:54 ` Sean Paul
[not found] ` <20180912135459.216217-2-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-12 13:54 ` [PATCH v2 3/7] drm/msm: dpu: Consolidate atomic_check functions() Sean Paul
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Sean Paul @ 2018-09-12 13:54 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
Sean Paul, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/7] drm/msm: dpu: Consolidate atomic_check functions()
[not found] ` <20180912135459.216217-1-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-12 13:54 ` [PATCH v2 2/7] drm/msm: dpu: Move atomic_check_plane_state() call to atomic_check Sean Paul
@ 2018-09-12 13:54 ` Sean Paul
[not found] ` <20180912135459.216217-3-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-12 13:54 ` [PATCH v2 4/7] drm/msm: dpu: Remove dpu_plane_sspp_enabled() Sean Paul
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Sean Paul @ 2018-09-12 13:54 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
Sean Paul, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 4/7] drm/msm: dpu: Remove dpu_plane_sspp_enabled()
[not found] ` <20180912135459.216217-1-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-12 13:54 ` [PATCH v2 2/7] drm/msm: dpu: Move atomic_check_plane_state() call to atomic_check Sean Paul
2018-09-12 13:54 ` [PATCH v2 3/7] drm/msm: dpu: Consolidate atomic_check functions() Sean Paul
@ 2018-09-12 13:54 ` Sean Paul
[not found] ` <20180912135459.216217-4-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-12 13:54 ` [PATCH v2 5/7] drm/msm: dpu: Remove dpu_plane_enabled() Sean Paul
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Sean Paul @ 2018-09-12 13:54 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
Sean Paul, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 5/7] drm/msm: dpu: Remove dpu_plane_enabled()
[not found] ` <20180912135459.216217-1-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
` (2 preceding siblings ...)
2018-09-12 13:54 ` [PATCH v2 4/7] drm/msm: dpu: Remove dpu_plane_sspp_enabled() Sean Paul
@ 2018-09-12 13:54 ` Sean Paul
[not found] ` <20180912135459.216217-5-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-12 13:54 ` [PATCH v2 6/7] drm/msm: dpu: Make dpu_plane_sspp_atomic_update() void Sean Paul
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Sean Paul @ 2018-09-12 13:54 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
Sean Paul, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 6/7] drm/msm: dpu: Make dpu_plane_sspp_atomic_update() void
[not found] ` <20180912135459.216217-1-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
` (3 preceding siblings ...)
2018-09-12 13:54 ` [PATCH v2 5/7] drm/msm: dpu: Remove dpu_plane_enabled() Sean Paul
@ 2018-09-12 13:54 ` Sean Paul
[not found] ` <20180912135459.216217-6-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-12 13:54 ` [PATCH v2 7/7] drm/msm: dpu: Don't continue after error in atomic_check Sean Paul
2018-09-13 22:23 ` [PATCH v2 1/7] drm/msm: dpu: Remove impossible checks Jeykumar Sankaran
6 siblings, 1 reply; 15+ messages in thread
From: Sean Paul @ 2018-09-12 13:54 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
Sean Paul, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 7/7] drm/msm: dpu: Don't continue after error in atomic_check
[not found] ` <20180912135459.216217-1-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
` (4 preceding siblings ...)
2018-09-12 13:54 ` [PATCH v2 6/7] drm/msm: dpu: Make dpu_plane_sspp_atomic_update() void Sean Paul
@ 2018-09-12 13:54 ` Sean Paul
[not found] ` <20180912135459.216217-7-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-13 22:23 ` [PATCH v2 1/7] drm/msm: dpu: Remove impossible checks Jeykumar Sankaran
6 siblings, 1 reply; 15+ messages in thread
From: Sean Paul @ 2018-09-12 13:54 UTC (permalink / raw)
To: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw, jsanka-sgV2jX0FEOL9JmXXK+q4OQ,
Sean Paul, abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/7] drm/msm: dpu: Remove impossible checks
[not found] ` <20180912135459.216217-1-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
` (5 preceding siblings ...)
2018-09-12 13:54 ` [PATCH v2 7/7] drm/msm: dpu: Don't continue after error in atomic_check Sean Paul
@ 2018-09-13 22:23 ` Jeykumar Sankaran
[not found] ` <3782b720d2c4bee1bc20cacff8f6f7f5-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
6 siblings, 1 reply; 15+ messages in thread
From: Jeykumar Sankaran @ 2018-09-13 22:23 UTC (permalink / raw)
To: Sean Paul
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Sean Paul,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/7] drm/msm: dpu: Move atomic_check_plane_state() call to atomic_check
[not found] ` <20180912135459.216217-2-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
@ 2018-09-13 22:24 ` Jeykumar Sankaran
0 siblings, 0 replies; 15+ messages in thread
From: Jeykumar Sankaran @ 2018-09-13 22:24 UTC (permalink / raw)
To: Sean Paul
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Sean Paul,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 3/7] drm/msm: dpu: Consolidate atomic_check functions()
[not found] ` <20180912135459.216217-3-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
@ 2018-09-13 22:24 ` Jeykumar Sankaran
0 siblings, 0 replies; 15+ messages in thread
From: Jeykumar Sankaran @ 2018-09-13 22:24 UTC (permalink / raw)
To: Sean Paul
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Sean Paul,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 4/7] drm/msm: dpu: Remove dpu_plane_sspp_enabled()
[not found] ` <20180912135459.216217-4-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
@ 2018-09-13 22:25 ` Jeykumar Sankaran
0 siblings, 0 replies; 15+ messages in thread
From: Jeykumar Sankaran @ 2018-09-13 22:25 UTC (permalink / raw)
To: Sean Paul
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Sean Paul,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 5/7] drm/msm: dpu: Remove dpu_plane_enabled()
[not found] ` <20180912135459.216217-5-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
@ 2018-09-13 22:30 ` Jeykumar Sankaran
0 siblings, 0 replies; 15+ messages in thread
From: Jeykumar Sankaran @ 2018-09-13 22:30 UTC (permalink / raw)
To: Sean Paul
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Sean Paul,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 6/7] drm/msm: dpu: Make dpu_plane_sspp_atomic_update() void
[not found] ` <20180912135459.216217-6-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
@ 2018-09-13 22:33 ` Jeykumar Sankaran
0 siblings, 0 replies; 15+ messages in thread
From: Jeykumar Sankaran @ 2018-09-13 22:33 UTC (permalink / raw)
To: Sean Paul
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Sean Paul,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 7/7] drm/msm: dpu: Don't continue after error in atomic_check
[not found] ` <20180912135459.216217-7-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
@ 2018-09-13 22:34 ` Jeykumar Sankaran
0 siblings, 0 replies; 15+ messages in thread
From: Jeykumar Sankaran @ 2018-09-13 22:34 UTC (permalink / raw)
To: Sean Paul
Cc: bzwang-F7+t8E8rja9g9hUCZPvPmw,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Sean Paul,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/7] drm/msm: dpu: Remove impossible checks
[not found] ` <3782b720d2c4bee1bc20cacff8f6f7f5-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
@ 2018-09-17 15:44 ` Sean Paul
0 siblings, 0 replies; 15+ messages in thread
From: Sean Paul @ 2018-09-17 15:44 UTC (permalink / raw)
To: Jeykumar Sankaran
Cc: Sean Paul, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
abhinavk-sgV2jX0FEOL9JmXXK+q4OQ, bzwang-F7+t8E8rja9g9hUCZPvPmw,
Sean Paul, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-09-17 15:44 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-12 13:54 [PATCH v2 1/7] drm/msm: dpu: Remove impossible checks Sean Paul
[not found] ` <20180912135459.216217-1-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-12 13:54 ` [PATCH v2 2/7] drm/msm: dpu: Move atomic_check_plane_state() call to atomic_check Sean Paul
[not found] ` <20180912135459.216217-2-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-13 22:24 ` Jeykumar Sankaran
2018-09-12 13:54 ` [PATCH v2 3/7] drm/msm: dpu: Consolidate atomic_check functions() Sean Paul
[not found] ` <20180912135459.216217-3-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-13 22:24 ` Jeykumar Sankaran
2018-09-12 13:54 ` [PATCH v2 4/7] drm/msm: dpu: Remove dpu_plane_sspp_enabled() Sean Paul
[not found] ` <20180912135459.216217-4-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-13 22:25 ` Jeykumar Sankaran
2018-09-12 13:54 ` [PATCH v2 5/7] drm/msm: dpu: Remove dpu_plane_enabled() Sean Paul
[not found] ` <20180912135459.216217-5-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-13 22:30 ` Jeykumar Sankaran
2018-09-12 13:54 ` [PATCH v2 6/7] drm/msm: dpu: Make dpu_plane_sspp_atomic_update() void Sean Paul
[not found] ` <20180912135459.216217-6-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-13 22:33 ` Jeykumar Sankaran
2018-09-12 13:54 ` [PATCH v2 7/7] drm/msm: dpu: Don't continue after error in atomic_check Sean Paul
[not found] ` <20180912135459.216217-7-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>
2018-09-13 22:34 ` Jeykumar Sankaran
2018-09-13 22:23 ` [PATCH v2 1/7] drm/msm: dpu: Remove impossible checks Jeykumar Sankaran
[not found] ` <3782b720d2c4bee1bc20cacff8f6f7f5-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-09-17 15:44 ` Sean Paul
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.