| Message ID | 20170815230338.14913-3-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Laurent, Thankyou for the patch, This looks good, and passes the tests. On 16/08/17 00:03, Laurent Pinchart wrote: > There is no point in accepting fully off-screen planes as they won't be > displayed. Reject them in the atomic check. > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > --- > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c > index 4f076c364f25..714e02960635 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c > @@ -569,6 +569,8 @@ int __rcar_du_plane_atomic_check(struct drm_plane *plane, > struct drm_plane_state *state, > const struct rcar_du_format_info **format) > { > + const struct drm_display_mode *mode; > + struct drm_crtc_state *crtc_state; > struct drm_device *dev = plane->dev; > > if (!state->fb || !state->crtc) { > @@ -582,6 +584,20 @@ int __rcar_du_plane_atomic_check(struct drm_plane *plane, > return -EINVAL; > } > > + crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc); > + if (IS_ERR(crtc_state)) > + return PTR_ERR(crtc_state); > + > + mode = &crtc_state->adjusted_mode; > + if (state->crtc_x >= mode->hdisplay || > + state->crtc_y >= mode->vdisplay || > + state->crtc_x + (int)state->crtc_w <= 0 || > + state->crtc_y + (int)state->crtc_h <= 0) { > + dev_dbg(dev->dev, "%s: plane can't be fully off-screen\n", > + __func__); > + return -EINVAL; > + } > + > *format = rcar_du_format_info(state->fb->format->format); > if (*format == NULL) { > dev_dbg(dev->dev, "%s: unsupported format %08x\n", __func__, >
On Wed, Sep 27, 2017 at 07:58:42PM +0100, Kieran Bingham wrote: > Hi Laurent, > > Thankyou for the patch, > > This looks good, and passes the tests. > > On 16/08/17 00:03, Laurent Pinchart wrote: > > There is no point in accepting fully off-screen planes as they won't be > > displayed. Reject them in the atomic check. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Just a note, this is different from what i915 does. Not sure you want to intentionally be incompatible with i915 and hence defacto make this part of atomic undefined (or break some apps, depending upon how this pans out). I think X loves to put the cursor offscreen at least. I think we even have igts to test this properly. -Daniel > > > --- > > drivers/gpu/drm/rcar-du/rcar_du_plane.c | 16 ++++++++++++++++ > > 1 file changed, 16 insertions(+) > > > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c > > index 4f076c364f25..714e02960635 100644 > > --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c > > +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c > > @@ -569,6 +569,8 @@ int __rcar_du_plane_atomic_check(struct drm_plane *plane, > > struct drm_plane_state *state, > > const struct rcar_du_format_info **format) > > { > > + const struct drm_display_mode *mode; > > + struct drm_crtc_state *crtc_state; > > struct drm_device *dev = plane->dev; > > > > if (!state->fb || !state->crtc) { > > @@ -582,6 +584,20 @@ int __rcar_du_plane_atomic_check(struct drm_plane *plane, > > return -EINVAL; > > } > > > > + crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc); > > + if (IS_ERR(crtc_state)) > > + return PTR_ERR(crtc_state); > > + > > + mode = &crtc_state->adjusted_mode; > > + if (state->crtc_x >= mode->hdisplay || > > + state->crtc_y >= mode->vdisplay || > > + state->crtc_x + (int)state->crtc_w <= 0 || > > + state->crtc_y + (int)state->crtc_h <= 0) { > > + dev_dbg(dev->dev, "%s: plane can't be fully off-screen\n", > > + __func__); > > + return -EINVAL; > > + } > > + > > *format = rcar_du_format_info(state->fb->format->format); > > if (*format == NULL) { > > dev_dbg(dev->dev, "%s: unsupported format %08x\n", __func__, > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Daniel, On Thursday, 28 September 2017 10:10:34 EEST Daniel Vetter wrote: > On Wed, Sep 27, 2017 at 07:58:42PM +0100, Kieran Bingham wrote: > > Hi Laurent, > > > > Thankyou for the patch, > > > > This looks good, and passes the tests. > > > > On 16/08/17 00:03, Laurent Pinchart wrote: > > > There is no point in accepting fully off-screen planes as they won't be > > > displayed. Reject them in the atomic check. > > > > > > Signed-off-by: Laurent Pinchart > > > <laurent.pinchart+renesas@ideasonboard.com> > > > > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> > > Just a note, this is different from what i915 does. Not sure you want to > intentionally be incompatible with i915 and hence defacto make this part > of atomic undefined (or break some apps, depending upon how this pans > out). I think X loves to put the cursor offscreen at least. > > I think we even have igts to test this properly. If you update the documentation I'll submit a v2 that allows fully off-screen planes :-)
On Thu, Sep 28, 2017 at 11:16 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Daniel, > > On Thursday, 28 September 2017 10:10:34 EEST Daniel Vetter wrote: >> On Wed, Sep 27, 2017 at 07:58:42PM +0100, Kieran Bingham wrote: >> > Hi Laurent, >> > >> > Thankyou for the patch, >> > >> > This looks good, and passes the tests. >> > >> > On 16/08/17 00:03, Laurent Pinchart wrote: >> > > There is no point in accepting fully off-screen planes as they won't be >> > > displayed. Reject them in the atomic check. >> > > >> > > Signed-off-by: Laurent Pinchart >> > > <laurent.pinchart+renesas@ideasonboard.com> >> > >> > Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> >> >> Just a note, this is different from what i915 does. Not sure you want to >> intentionally be incompatible with i915 and hence defacto make this part >> of atomic undefined (or break some apps, depending upon how this pans >> out). I think X loves to put the cursor offscreen at least. >> >> I think we even have igts to test this properly. > > If you update the documentation I'll submit a v2 that allows fully off-screen > planes :-) Quoting them: "The destination rectangle can lie outside of the visible area of the current mode of the CRTC." This shit is better than I thought! https://dri.freedesktop.org/docs/drm/gpu/drm-kms.html#plane-composition-properties Now can I raise you one and challenge you to run the kms tests in igt and see how non-compliant your driver is :-) Cheers, Daniel
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 4f076c364f25..714e02960635 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -569,6 +569,8 @@ int __rcar_du_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state, const struct rcar_du_format_info **format) { + const struct drm_display_mode *mode; + struct drm_crtc_state *crtc_state; struct drm_device *dev = plane->dev; if (!state->fb || !state->crtc) { @@ -582,6 +584,20 @@ int __rcar_du_plane_atomic_check(struct drm_plane *plane, return -EINVAL; } + crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc); + if (IS_ERR(crtc_state)) + return PTR_ERR(crtc_state); + + mode = &crtc_state->adjusted_mode; + if (state->crtc_x >= mode->hdisplay || + state->crtc_y >= mode->vdisplay || + state->crtc_x + (int)state->crtc_w <= 0 || + state->crtc_y + (int)state->crtc_h <= 0) { + dev_dbg(dev->dev, "%s: plane can't be fully off-screen\n", + __func__); + return -EINVAL; + } + *format = rcar_du_format_info(state->fb->format->format); if (*format == NULL) { dev_dbg(dev->dev, "%s: unsupported format %08x\n", __func__,
There is no point in accepting fully off-screen planes as they won't be displayed. Reject them in the atomic check. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)