Hi Ville,
On Thursday, 16 November 2017 19:04:26 EET Ville Syrjälä wrote:
> On Mon, Nov 13, 2017 at 10:47:00AM +0200, Laurent Pinchart wrote:
> > Unlike the KMS API, the hardware doesn't support planes exceeding the
> > screen boundaries or planes being located fully off-screen. We need to
> > clip plane coordinates to support the use case.
> >
> > Fortunately the DRM core offers the drm_plane_helper_check_state()
> > helper that valides the scaling factor and clips the plane coordinates.
> > Use it to implement the plane atomic check and use the clipped source
> > and destination rectangles from the plane state instead of the unclipped
> > source and CRTC coordinates to configure the device.
> >
> > Signed-off-by: Laurent Pinchart
> > <[email protected]>
> > ---
> >
> > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 3 ++-
> > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 37 +++++++++++++++++++++-------
> > drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 10 ++++++---
> > 3 files changed, 39 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index b492063a6e1f..5685d5af6998
> > 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> > @@ -319,7 +319,8 @@ static void rcar_du_crtc_update_planes(struct
> > rcar_du_crtc *rcrtc)>
> > struct rcar_du_plane *plane = &rcrtc->group->planes[i];
> > unsigned int j;
> >
> > - if (plane->plane.state->crtc != &rcrtc->crtc)
> > + if (plane->plane.state->crtc != &rcrtc->crtc ||
> > + !plane->plane.state->visible)
> >
> > continue;
> >
> > /* Insert the plane in the sorted planes array. */
> >
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index
> > 4f076c364f25..9cf02b44902d 100644
> > --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c
> > @@ -570,16 +570,39 @@ int __rcar_du_plane_atomic_check(struct drm_plane
> > *plane,
> > const struct rcar_du_format_info **format)
> > {
> > struct drm_device *dev = plane->dev;
> > + struct drm_crtc_state *crtc_state;
> > + struct drm_rect clip;
> > + int ret;
> >
> > - if (!state->fb || !state->crtc) {
> > + if (!state->crtc) {
> > + /*
> > + * The visible field is not reset by the DRM core but only
> > + * updated by drm_plane_helper_check_state(), set it manually.
> > + */
> > + state->visible = false;
> > *format = NULL;
> > return 0;
> > - }
> > + };
>
> spurious ;
Oops, my bad, I'll fix that.
> > - if (state->src_w >> 16 != state->crtc_w ||
> > - state->src_h >> 16 != state->crtc_h) {
> > - dev_dbg(dev->dev, "%s: scaling not supported\n", __func__);
> > - return -EINVAL;
> > + crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
> > + if (IS_ERR(crtc_state))
> > + return PTR_ERR(crtc_state);
> > +
> > + clip.x1 = 0;
> > + clip.y1 = 0;
> > + clip.x2 = crtc_state->adjusted_mode.hdisplay;
> > + clip.y2 = crtc_state->adjusted_mode.vdisplay;
>
> crtc_state->mode would be more correct. I messed that up too in my
> recent vmwgfx fix [1]. But this should probably work just as well
> if you don't have a crtc scaler in your pipeline.
Indeed, my CRTC can't scale, so this works, but I'll fix it nonetheless.
> Also you may want to leave the clip empty when !crtc_state->enable.
> That way you'll be guaranteed to get visible==false. The helper is
> currently a bit broken wrt. the crtc->enable vs. crtc_state->enable.
> I've fixed that in [1] as well but those patches haven't been pushed
> yet.
[1] has landed in drm-misc, I'll rebase this series on top of that, and will
send a pull request when drm-misc gets merged in Dave's tree.
> After getting that stuff in, I'm going to attempt moving this
> clipping stuff entirely into the helper to avoid these kinds of
> mistakes in the future.
Good idea, thanks.
> [1] https://patchwork.freedesktop.org/series/33001/
>
> > +
> > + ret = drm_plane_helper_check_state(state, &clip,
> > + DRM_PLANE_HELPER_NO_SCALING,
> > + DRM_PLANE_HELPER_NO_SCALING,
> > + true, true);
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (!state->visible) {
> > + *format = NULL;
> > + return 0;
> > }
> >
> > *format = rcar_du_format_info(state->fb->format->format);
--
Regards,
Laurent Pinchart
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
