| Message ID | 20180829175019.45396-1-sean@poorly.run (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] drm/msm: dpu: Allow planes to extend past active display | expand |
On 2018-08-29 10:49, Sean Paul wrote: > From: Sean Paul <seanpaul@chromium.org> > > The atomic_check is a bit too aggressive with respect to planes which > leave the active area. This caused a bunch of log spew when the cursor > got to the edge of the screen and stopped it from going all the way. > > This patch removes the conservative bounds checks from atomic and clips > the dst rect such that we properly display planes which go off the > screen. > > Changes in v2: > - Apply the clip to src as well (taking into account scaling) > Changes in v3: > - Use drm_atomic_helper_check_plane_state() to clip src/dst > > Cc: Sravanthi Kollukuduru <skolluku@codeaurora.org> > Cc: Jeykumar Sankaran <jsanka@codeaurora.org> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Sean Paul <seanpaul@chromium.org> > --- Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 3 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 34 +++++++++++++--------- > drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 1 - > drivers/gpu/drm/msm/msm_drv.h | 2 ++ > 4 files changed, 24 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > index 07c2d15b45f2..f0a5e776ba32 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > @@ -1551,8 +1551,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc > *crtc, > cnt++; > > dst = drm_plane_state_dest(pstate); > - if (!drm_rect_intersect(&clip, &dst) || > - !drm_rect_equals(&clip, &dst)) { > + if (!drm_rect_intersect(&clip, &dst)) { > DPU_ERROR("invalid vertical/horizontal destination\n"); > DPU_ERROR("display: " DRM_RECT_FMT " plane: " > DRM_RECT_FMT "\n", DRM_RECT_ARG(&crtc_rect), > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > index efdf9b200dd9..46de0de466ff 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > @@ -1254,7 +1254,7 @@ static int dpu_plane_sspp_atomic_update(struct > drm_plane *plane, > const struct dpu_format *fmt; > struct drm_crtc *crtc; > struct drm_framebuffer *fb; > - struct drm_rect src, dst; > + int ret, min_scale; > > if (!plane) { > DPU_ERROR("invalid plane\n"); > @@ -1293,21 +1293,29 @@ 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); > > - src.x1 = state->src_x >> 16; > - src.y1 = state->src_y >> 16; > - src.x2 = src.x1 + (state->src_w >> 16); > - src.y2 = src.y1 + (state->src_h >> 16); > + 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; > + } > > - dst = drm_plane_state_dest(state); > + 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), > + (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt)); > > - DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FMT "->crtc%u " DRM_RECT_FMT > - ", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_ARG(&src), > - crtc->base.id, DRM_RECT_ARG(&dst), > - (char *)&fmt->base.pixel_format, > - DPU_FORMAT_IS_UBWC(fmt)); > + pdpu->pipe_cfg.src_rect = state->src; > + > + /* state->src is 16.16, src_rect is not */ > + pdpu->pipe_cfg.src_rect.x1 >>= 16; > + pdpu->pipe_cfg.src_rect.x2 >>= 16; > + pdpu->pipe_cfg.src_rect.y1 >>= 16; > + pdpu->pipe_cfg.src_rect.y2 >>= 16; > > - pdpu->pipe_cfg.src_rect = src; > - pdpu->pipe_cfg.dst_rect = dst; > + pdpu->pipe_cfg.dst_rect = state->dst; > > _dpu_plane_setup_scaler(pdpu, pstate, fmt, false); > > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > index 7d306c5acd09..273cbbe27c2e 100644 > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > @@ -259,7 +259,6 @@ static void mdp5_plane_cleanup_fb(struct drm_plane > *plane, > msm_framebuffer_cleanup(fb, kms->aspace); > } > > -#define FRAC_16_16(mult, div) (((mult) << 16) / (div)) > static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state > *crtc_state, > struct drm_plane_state *state) > { > diff --git a/drivers/gpu/drm/msm/msm_drv.h > b/drivers/gpu/drm/msm/msm_drv.h > index 8e510d5c758a..9d11f321f5a9 100644 > --- a/drivers/gpu/drm/msm/msm_drv.h > +++ b/drivers/gpu/drm/msm/msm_drv.h > @@ -62,6 +62,8 @@ struct msm_gem_vma; > #define MAX_BRIDGES 8 > #define MAX_CONNECTORS 8 > > +#define FRAC_16_16(mult, div) (((mult) << 16) / (div)) > + > struct msm_file_private { > rwlock_t queuelock; > struct list_head submitqueues;
On Tue, Sep 04, 2018 at 05:14:01PM -0700, Jeykumar Sankaran wrote: > On 2018-08-29 10:49, Sean Paul wrote: > > From: Sean Paul <seanpaul@chromium.org> > > > > The atomic_check is a bit too aggressive with respect to planes which > > leave the active area. This caused a bunch of log spew when the cursor > > got to the edge of the screen and stopped it from going all the way. > > > > This patch removes the conservative bounds checks from atomic and clips > > the dst rect such that we properly display planes which go off the > > screen. > > > > Changes in v2: > > - Apply the clip to src as well (taking into account scaling) > > Changes in v3: > > - Use drm_atomic_helper_check_plane_state() to clip src/dst > > > > Cc: Sravanthi Kollukuduru <skolluku@codeaurora.org> > > Cc: Jeykumar Sankaran <jsanka@codeaurora.org> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Sean Paul <seanpaul@chromium.org> > > --- > > Reviewed-by: Jeykumar Sankaran <jsanka@codeaurora.org> Thanks for your review, I've pushed to dpu-staging/for-next. Sean > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 3 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 34 +++++++++++++--------- > > drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 1 - > > drivers/gpu/drm/msm/msm_drv.h | 2 ++ > > 4 files changed, 24 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > index 07c2d15b45f2..f0a5e776ba32 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c > > @@ -1551,8 +1551,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc > > *crtc, > > cnt++; > > > > dst = drm_plane_state_dest(pstate); > > - if (!drm_rect_intersect(&clip, &dst) || > > - !drm_rect_equals(&clip, &dst)) { > > + if (!drm_rect_intersect(&clip, &dst)) { > > DPU_ERROR("invalid vertical/horizontal destination\n"); > > DPU_ERROR("display: " DRM_RECT_FMT " plane: " > > DRM_RECT_FMT "\n", DRM_RECT_ARG(&crtc_rect), > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > index efdf9b200dd9..46de0de466ff 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c > > @@ -1254,7 +1254,7 @@ static int dpu_plane_sspp_atomic_update(struct > > drm_plane *plane, > > const struct dpu_format *fmt; > > struct drm_crtc *crtc; > > struct drm_framebuffer *fb; > > - struct drm_rect src, dst; > > + int ret, min_scale; > > > > if (!plane) { > > DPU_ERROR("invalid plane\n"); > > @@ -1293,21 +1293,29 @@ 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); > > > > - src.x1 = state->src_x >> 16; > > - src.y1 = state->src_y >> 16; > > - src.x2 = src.x1 + (state->src_w >> 16); > > - src.y2 = src.y1 + (state->src_h >> 16); > > + 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; > > + } > > > > - dst = drm_plane_state_dest(state); > > + 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), > > + (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt)); > > > > - DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FMT "->crtc%u " DRM_RECT_FMT > > - ", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_ARG(&src), > > - crtc->base.id, DRM_RECT_ARG(&dst), > > - (char *)&fmt->base.pixel_format, > > - DPU_FORMAT_IS_UBWC(fmt)); > > + pdpu->pipe_cfg.src_rect = state->src; > > + > > + /* state->src is 16.16, src_rect is not */ > > + pdpu->pipe_cfg.src_rect.x1 >>= 16; > > + pdpu->pipe_cfg.src_rect.x2 >>= 16; > > + pdpu->pipe_cfg.src_rect.y1 >>= 16; > > + pdpu->pipe_cfg.src_rect.y2 >>= 16; > > > > - pdpu->pipe_cfg.src_rect = src; > > - pdpu->pipe_cfg.dst_rect = dst; > > + pdpu->pipe_cfg.dst_rect = state->dst; > > > > _dpu_plane_setup_scaler(pdpu, pstate, fmt, false); > > > > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > > b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > > index 7d306c5acd09..273cbbe27c2e 100644 > > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > > @@ -259,7 +259,6 @@ static void mdp5_plane_cleanup_fb(struct drm_plane > > *plane, > > msm_framebuffer_cleanup(fb, kms->aspace); > > } > > > > -#define FRAC_16_16(mult, div) (((mult) << 16) / (div)) > > static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state > > *crtc_state, > > struct drm_plane_state *state) > > { > > diff --git a/drivers/gpu/drm/msm/msm_drv.h > > b/drivers/gpu/drm/msm/msm_drv.h > > index 8e510d5c758a..9d11f321f5a9 100644 > > --- a/drivers/gpu/drm/msm/msm_drv.h > > +++ b/drivers/gpu/drm/msm/msm_drv.h > > @@ -62,6 +62,8 @@ struct msm_gem_vma; > > #define MAX_BRIDGES 8 > > #define MAX_CONNECTORS 8 > > > > +#define FRAC_16_16(mult, div) (((mult) << 16) / (div)) > > + > > struct msm_file_private { > > rwlock_t queuelock; > > struct list_head submitqueues; > > -- > Jeykumar S
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c index 07c2d15b45f2..f0a5e776ba32 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c @@ -1551,8 +1551,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc, cnt++; dst = drm_plane_state_dest(pstate); - if (!drm_rect_intersect(&clip, &dst) || - !drm_rect_equals(&clip, &dst)) { + if (!drm_rect_intersect(&clip, &dst)) { DPU_ERROR("invalid vertical/horizontal destination\n"); DPU_ERROR("display: " DRM_RECT_FMT " plane: " DRM_RECT_FMT "\n", DRM_RECT_ARG(&crtc_rect), diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index efdf9b200dd9..46de0de466ff 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -1254,7 +1254,7 @@ static int dpu_plane_sspp_atomic_update(struct drm_plane *plane, const struct dpu_format *fmt; struct drm_crtc *crtc; struct drm_framebuffer *fb; - struct drm_rect src, dst; + int ret, min_scale; if (!plane) { DPU_ERROR("invalid plane\n"); @@ -1293,21 +1293,29 @@ 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); - src.x1 = state->src_x >> 16; - src.y1 = state->src_y >> 16; - src.x2 = src.x1 + (state->src_w >> 16); - src.y2 = src.y1 + (state->src_h >> 16); + 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; + } - dst = drm_plane_state_dest(state); + 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), + (char *)&fmt->base.pixel_format, DPU_FORMAT_IS_UBWC(fmt)); - DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FMT "->crtc%u " DRM_RECT_FMT - ", %4.4s ubwc %d\n", fb->base.id, DRM_RECT_ARG(&src), - crtc->base.id, DRM_RECT_ARG(&dst), - (char *)&fmt->base.pixel_format, - DPU_FORMAT_IS_UBWC(fmt)); + pdpu->pipe_cfg.src_rect = state->src; + + /* state->src is 16.16, src_rect is not */ + pdpu->pipe_cfg.src_rect.x1 >>= 16; + pdpu->pipe_cfg.src_rect.x2 >>= 16; + pdpu->pipe_cfg.src_rect.y1 >>= 16; + pdpu->pipe_cfg.src_rect.y2 >>= 16; - pdpu->pipe_cfg.src_rect = src; - pdpu->pipe_cfg.dst_rect = dst; + pdpu->pipe_cfg.dst_rect = state->dst; _dpu_plane_setup_scaler(pdpu, pstate, fmt, false); diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c index 7d306c5acd09..273cbbe27c2e 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c @@ -259,7 +259,6 @@ static void mdp5_plane_cleanup_fb(struct drm_plane *plane, msm_framebuffer_cleanup(fb, kms->aspace); } -#define FRAC_16_16(mult, div) (((mult) << 16) / (div)) static int mdp5_plane_atomic_check_with_state(struct drm_crtc_state *crtc_state, struct drm_plane_state *state) { diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h index 8e510d5c758a..9d11f321f5a9 100644 --- a/drivers/gpu/drm/msm/msm_drv.h +++ b/drivers/gpu/drm/msm/msm_drv.h @@ -62,6 +62,8 @@ struct msm_gem_vma; #define MAX_BRIDGES 8 #define MAX_CONNECTORS 8 +#define FRAC_16_16(mult, div) (((mult) << 16) / (div)) + struct msm_file_private { rwlock_t queuelock; struct list_head submitqueues;