From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Paul Subject: Re: [PATCH v2] drm/msm: dpu: Allow planes to extend past active display Date: Tue, 28 Aug 2018 18:52:31 -0400 Message-ID: References: <20180828205106.15168-1-sean@poorly.run> <8e057e8759588455e8a1816dd8336c33@codeaurora.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1490578237==" Return-path: In-Reply-To: <8e057e8759588455e8a1816dd8336c33-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: freedreno-bounces-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org Sender: "Freedreno" To: Jeykumar Sankaran Cc: Rajesh Yadav , linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jordan Crouse , dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, Rob Clark , Sean Paul , freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, Sravanthi Kollukuduru List-Id: linux-arm-msm@vger.kernel.org --===============1490578237== Content-Type: multipart/alternative; boundary="0000000000002dfe5e057486b30e" --0000000000002dfe5e057486b30e Content-Type: text/plain; charset="UTF-8" On Tue, Aug 28, 2018, 6:04 PM Jeykumar Sankaran wrote: > On 2018-08-28 13:50, Sean Paul wrote: > > From: Sean Paul > > > > 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 > > Cc: Jeykumar Sankaran > > Signed-off-by: Sean Paul > > --- > > 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, > > DRM_RECT_ARG(&src), > > crtc->base.id, 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 > -- > Jeykumar S > --0000000000002dfe5e057486b30e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


= On Tue, Aug 28, 2018, 6:04 PM Jeykumar Sankaran <jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
On 2018-08-28 13:50, Sean Paul wrote:
> From: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>
> The atomic_check is a bit too aggressive with respect to planes which<= br> > 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 clip= s
> 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-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Cc: Jeykumar Sankaran <jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Signed-off-by: Sean Paul <seanpaul-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>=C2=A0 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c=C2=A0 |=C2=A0 3 +--
>=C2=A0 drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 14 +++++++++++++- >=C2=A0 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,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0cnt++;
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dst =3D drm_plan= e_state_dest(pstate);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!drm_rect_interse= ct(&clip, &dst) ||
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0!drm_re= ct_equals(&clip, &dst)) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!drm_rect_interse= ct(&clip, &dst)) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0DPU_ERROR("invalid vertical/horizontal
> destination\n");
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0DPU_ERROR("display: " DRM_RECT_FMT " plane: &qu= ot;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DRM_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,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0const struct dpu_format *fmt;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct drm_crtc *crtc;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct drm_framebuffer *fb;
> -=C2=A0 =C2=A0 =C2=A0struct drm_rect src, dst;
> +=C2=A0 =C2=A0 =C2=A0struct drm_rect clip =3D { 0 }, src, dst;
> +=C2=A0 =C2=A0 =C2=A0int hscale, vscale;
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!plane) {
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DPU_ERROR("= invalid plane\n");
> @@ -1300,6 +1301,17 @@ static int dpu_plane_sspp_atomic_update(struct<= br> > drm_plane *plane,
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0dst =3D drm_plane_state_dest(state);
>
> +=C2=A0 =C2=A0 =C2=A0hscale =3D drm_rect_calc_hscale(&src, &ds= t,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pdpu->pipe_sblk-= >maxupscale,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pdpu->pipe_sblk-= >maxdwnscale);
> +=C2=A0 =C2=A0 =C2=A0vscale =3D drm_rect_calc_vscale(&src, &ds= t,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pdpu->pipe_sblk-= >maxupscale,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pdpu->pipe_sblk-= >maxdwnscale);
> +
> +=C2=A0 =C2=A0 =C2=A0clip.x2 =3D crtc->state->adjusted_mode.hdis= play;
> +=C2=A0 =C2=A0 =C2=A0clip.y2 =3D crtc->state->adjusted_mode.vdis= play;
> +=C2=A0 =C2=A0 =C2=A0drm_rect_clip_scaled(&src, &dst, &cli= p, hscale, vscale);
> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0DPU_DEBUG_PLANE(pdpu, "FB[%u] " DR= M_RECT_FMT "->crtc%u "
> DRM_RECT_FMT
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0", %4.4s ubwc %d\n", fb->base.id,
> DRM_RECT_ARG(&src),
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0crtc->base.id, 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= 9;ll always be equal to the adjusted mode.=C2=A0
Sean


--
Jeykumar S
--0000000000002dfe5e057486b30e-- --===============1490578237== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KRnJlZWRyZW5v IG1haWxpbmcgbGlzdApGcmVlZHJlbm9AbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZnJlZWRyZW5vCg== --===============1490578237==--