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

[Jeykumar Sankaran <jsanka-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On 2018-08-28 15:52, Sean Paul wrote:
> On Tue, Aug 28, 2018, 6:04 PM Jeykumar Sankaran
> <jsanka@codeaurora.org> wrote:
> 
>> On 2018-08-28 13:50, 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)
>>> 
>>> Cc: Sravanthi Kollukuduru <skolluku@codeaurora.org>
>>> Cc: Jeykumar Sankaran <jsanka@codeaurora.org>
>>> Signed-off-by: Sean Paul <seanpaul@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?

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

-- 
Jeykumar S
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno