* [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[parent not found: <20180912135459.216217-1-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>]
* [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
[parent not found: <20180912135459.216217-2-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>]
* 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
* [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
[parent not found: <20180912135459.216217-3-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>]
* 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
* [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
[parent not found: <20180912135459.216217-4-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>]
* 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
* [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
[parent not found: <20180912135459.216217-5-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>]
* 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
* [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
[parent not found: <20180912135459.216217-6-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>]
* 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
* [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
[parent not found: <20180912135459.216217-7-sean-p7yTbzM4H96eqtR555YLDQ@public.gmane.org>]
* 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] ` <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
[parent not found: <3782b720d2c4bee1bc20cacff8f6f7f5-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]
* 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).