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@xxxxxxxxxxxx> > > > > 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@xxxxxxxxxxxxxx> > > Cc: Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx> > > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Signed-off-by: Sean Paul <seanpaul@xxxxxxxxxxxx> > > --- > > Reviewed-by: Jeykumar Sankaran <jsanka@xxxxxxxxxxxxxx> 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 -- Sean Paul, Software Engineer, Google / Chromium OS