[Freedreno] [PATCH v2] drm/msm: dpu: Allow planes to extend past active display

[Sean Paul]

On Tue, Aug 28, 2018 at 05:01:13PM -0700, Jeykumar Sankaran wrote:
> On 2018-08-28 15:52, Sean Paul wrote:
> > On Tue, Aug 28, 2018, 6:04 PM Jeykumar Sankaran
> > <jsanka at codeaurora.org> wrote:
> > 
> > > On 2018-08-28 13:50, Sean Paul wrote:
> > > > From: Sean Paul <seanpaul at 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)
> > > > 
> > > > Cc: Sravanthi Kollukuduru <skolluku at codeaurora.org>
> > > > Cc: Jeykumar Sankaran <jsanka at codeaurora.org>
> > > > Signed-off-by: Sean Paul <seanpaul at chromium.org>
> > > > ---
> > > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c  |  3 +--
> > > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 14 +++++++++++++-
> > > > 2 files changed, 14 insertions(+), 3 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..adfd16625188 100644
> > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > > @@ -1254,7 +1254,8 @@ 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;
> > > > +     struct drm_rect clip = { 0 }, src, dst;
> > > > +     int hscale, vscale;
> > > > 
> > > > if (!plane) {
> > > > DPU_ERROR("invalid plane\n");
> > > > @@ -1300,6 +1301,17 @@ static int
> > > dpu_plane_sspp_atomic_update(struct
> > > > drm_plane *plane,
> > > > 
> > > > dst = drm_plane_state_dest(state);
> > > > 
> > > > +     hscale = drm_rect_calc_hscale(&src, &dst,
> > > > +                                   pdpu->pipe_sblk->maxupscale,
> > > > +                                   pdpu->pipe_sblk->maxdwnscale);
> > > > +     vscale = drm_rect_calc_vscale(&src, &dst,
> > > > +                                   pdpu->pipe_sblk->maxupscale,
> > > > +                                   pdpu->pipe_sblk->maxdwnscale);
> > > > +
> > > > +     clip.x2 = crtc->state->adjusted_mode.hdisplay;
> > > > +     clip.y2 = crtc->state->adjusted_mode.vdisplay;
> > > > +     drm_rect_clip_scaled(&src, &dst, &clip, hscale, vscale);
> > > > +
> > > > DPU_DEBUG_PLANE(pdpu, "FB[%u] " DRM_RECT_FMT "->crtc%u "
> > > > DRM_RECT_FMT
> > > > ", %4.4s ubwc %d\n", fb->base.id [1],
> > > > DRM_RECT_ARG(&src),
> > > > crtc->base.id [1], DRM_RECT_ARG(&dst),
> > > 
> > > Don't you have to update pdpu->pipe_cfg.src_rect and
> > > pdpu->pipe_cfg.dst_rect with clip?
> > 
> > No, clip is the active area, so it'll always be equal to the adjusted
> > mode.
> > 
> > Sean
> > 
> 
> Got it! In that case, can drm_atomic_helper_check_plane_state helper be
> useful here?

Yeah, good call, I'll switch over to that.

Thanks,

Sean

> 
> > > --
> > > Jeykumar S
> > 
> > 
> > Links:
> > ------
> > [1] http://base.id
> 
> -- 
> Jeykumar S

-- 
Sean Paul, Software Engineer, Google / Chromium OS