* [PATCH v2 0/2] R-Car DU: Clip planes to screen boundaries @ 2017-11-13 8:46 Laurent Pinchart 2017-11-13 8:46 ` [PATCH v2 1/2] drm: rcar-du: Share plane atomic check code between Gen2 and Gen3 Laurent Pinchart 2017-11-13 8:47 ` Laurent Pinchart 0 siblings, 2 replies; 7+ messages in thread From: Laurent Pinchart @ 2017-11-13 8:46 UTC (permalink / raw) To: dri-devel; +Cc: linux-renesas-soc, Kieran Bingham Hello, This patch series fixes support for planes that cross the screen boundaries. The KMS API supports such a configuration, but the DU and VSP hardware doesn't. This leads to different kind of dispay artifacts or hangs. The series starts with a bit of refactoring to share existing code and make it easier to share new code. The second patch then clips planes that are either partially or fully off-screen, disabling planes that end up being completely invisible. Compared to v1, fully off-screen planes are now accepted (but obviously not shown). The patches are based on top of Dave's latest master branch. Kieran, I have tested this on your Salvator-X ES1.1 board using the DU test suite. All tests pass (the suspend/resume test requires additional patches, but that's an unrelated issue), but I can't check the display output remotely. Would you be able to run the plane boundaries test locally and check that everything is normal ? Please note that I've pushed a new version of that test to the test suite repository. Laurent Pinchart (2): drm: rcar-du: Share plane atomic check code between Gen2 and Gen3 drm: rcar-du: Clip planes to screen boundaries drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 3 +- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 64 ++++++++++++++++++++++++--------- drivers/gpu/drm/rcar-du/rcar_du_plane.h | 4 +++ drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 32 +++++------------ 4 files changed, 61 insertions(+), 42 deletions(-) -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] drm: rcar-du: Share plane atomic check code between Gen2 and Gen3 2017-11-13 8:46 [PATCH v2 0/2] R-Car DU: Clip planes to screen boundaries Laurent Pinchart @ 2017-11-13 8:46 ` Laurent Pinchart 2017-11-13 8:47 ` Laurent Pinchart 1 sibling, 0 replies; 7+ messages in thread From: Laurent Pinchart @ 2017-11-13 8:46 UTC (permalink / raw) To: dri-devel; +Cc: linux-renesas-soc, Kieran Bingham The plane atomic check implementation is identical on Gen2 (DU planes) and Gen3 (VSP planes), but two separate functions exist as they operate on different data structures. Refactor the code to share the implementation. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> Tested-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com> --- drivers/gpu/drm/rcar-du/rcar_du_plane.c | 27 +++++++++++++++++---------- drivers/gpu/drm/rcar-du/rcar_du_plane.h | 4 ++++ drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 22 +--------------------- 3 files changed, 22 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.c b/drivers/gpu/drm/rcar-du/rcar_du_plane.c index 61833cc1c699..4f076c364f25 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.c @@ -565,27 +565,26 @@ void __rcar_du_plane_setup(struct rcar_du_group *rgrp, } } -static int rcar_du_plane_atomic_check(struct drm_plane *plane, - struct drm_plane_state *state) +int __rcar_du_plane_atomic_check(struct drm_plane *plane, + struct drm_plane_state *state, + const struct rcar_du_format_info **format) { - struct rcar_du_plane_state *rstate = to_rcar_plane_state(state); - struct rcar_du_plane *rplane = to_rcar_plane(plane); - struct rcar_du_device *rcdu = rplane->group->dev; + struct drm_device *dev = plane->dev; if (!state->fb || !state->crtc) { - rstate->format = NULL; + *format = NULL; return 0; } if (state->src_w >> 16 != state->crtc_w || state->src_h >> 16 != state->crtc_h) { - dev_dbg(rcdu->dev, "%s: scaling not supported\n", __func__); + dev_dbg(dev->dev, "%s: scaling not supported\n", __func__); return -EINVAL; } - rstate->format = rcar_du_format_info(state->fb->format->format); - if (rstate->format == NULL) { - dev_dbg(rcdu->dev, "%s: unsupported format %08x\n", __func__, + *format = rcar_du_format_info(state->fb->format->format); + if (*format == NULL) { + dev_dbg(dev->dev, "%s: unsupported format %08x\n", __func__, state->fb->format->format); return -EINVAL; } @@ -593,6 +592,14 @@ static int rcar_du_plane_atomic_check(struct drm_plane *plane, return 0; } +static int rcar_du_plane_atomic_check(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct rcar_du_plane_state *rstate = to_rcar_plane_state(state); + + return __rcar_du_plane_atomic_check(plane, state, &rstate->format); +} + static void rcar_du_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_state) { diff --git a/drivers/gpu/drm/rcar-du/rcar_du_plane.h b/drivers/gpu/drm/rcar-du/rcar_du_plane.h index f62e09f195de..890321b4665d 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_plane.h +++ b/drivers/gpu/drm/rcar-du/rcar_du_plane.h @@ -73,6 +73,10 @@ to_rcar_plane_state(struct drm_plane_state *state) int rcar_du_atomic_check_planes(struct drm_device *dev, struct drm_atomic_state *state); +int __rcar_du_plane_atomic_check(struct drm_plane *plane, + struct drm_plane_state *state, + const struct rcar_du_format_info **format); + int rcar_du_planes_init(struct rcar_du_group *rgrp); void __rcar_du_plane_setup(struct rcar_du_group *rgrp, diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index 2c96147bc444..dd66dcb8da23 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -268,28 +268,8 @@ static int rcar_du_vsp_plane_atomic_check(struct drm_plane *plane, struct drm_plane_state *state) { struct rcar_du_vsp_plane_state *rstate = to_rcar_vsp_plane_state(state); - struct rcar_du_vsp_plane *rplane = to_rcar_vsp_plane(plane); - struct rcar_du_device *rcdu = rplane->vsp->dev; - - if (!state->fb || !state->crtc) { - rstate->format = NULL; - return 0; - } - if (state->src_w >> 16 != state->crtc_w || - state->src_h >> 16 != state->crtc_h) { - dev_dbg(rcdu->dev, "%s: scaling not supported\n", __func__); - return -EINVAL; - } - - rstate->format = rcar_du_format_info(state->fb->format->format); - if (rstate->format == NULL) { - dev_dbg(rcdu->dev, "%s: unsupported format %08x\n", __func__, - state->fb->format->format); - return -EINVAL; - } - - return 0; + return __rcar_du_plane_atomic_check(plane, state, &rstate->format); } static void rcar_du_vsp_plane_atomic_update(struct drm_plane *plane, -- Regards, Laurent Pinchart ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] drm: rcar-du: Clip planes to screen boundaries 2017-11-13 8:46 [PATCH v2 0/2] R-Car DU: Clip planes to screen boundaries Laurent Pinchart @ 2017-11-13 8:47 ` Laurent Pinchart 2017-11-13 8:47 ` Laurent Pinchart 1 sibling, 0 replies; 7+ messages in thread From: Laurent Pinchart @ 2017-11-13 8:47 UTC (permalink / raw) To: dri-devel; +Cc: linux-renesas-soc, Kieran Bingham 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 <laurent.pinchart+renesas@ideasonboard.com> --- 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; - } + }; - 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; + + 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); @@ -607,7 +630,7 @@ static void rcar_du_plane_atomic_update(struct drm_plane *plane, struct rcar_du_plane_state *old_rstate; struct rcar_du_plane_state *new_rstate; - if (!plane->state->crtc) + if (!plane->state->visible) return; rcar_du_plane_setup(rplane); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index dd66dcb8da23..6d1a82ee50ed 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -212,7 +212,11 @@ static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane, unsigned int i; int ret; - if (!state->fb) + /* + * There's no need to prepare (and unprepare) the framebuffer when the + * plane is not visible, as it will not be displayed. + */ + if (!state->visible) return 0; for (i = 0; i < rstate->format->planes; ++i) { @@ -253,7 +257,7 @@ static void rcar_du_vsp_plane_cleanup_fb(struct drm_plane *plane, struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp; unsigned int i; - if (!state->fb) + if (!state->visible) return; for (i = 0; i < rstate->format->planes; ++i) { @@ -278,7 +282,7 @@ static void rcar_du_vsp_plane_atomic_update(struct drm_plane *plane, struct rcar_du_vsp_plane *rplane = to_rcar_vsp_plane(plane); struct rcar_du_crtc *crtc = to_rcar_crtc(old_state->crtc); - if (plane->state->crtc) + if (plane->state->visible) rcar_du_vsp_plane_setup(rplane); else vsp1_du_atomic_update(rplane->vsp->vsp, crtc->vsp_pipe, -- Regards, Laurent Pinchart ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] drm: rcar-du: Clip planes to screen boundaries @ 2017-11-13 8:47 ` Laurent Pinchart 0 siblings, 0 replies; 7+ messages in thread From: Laurent Pinchart @ 2017-11-13 8:47 UTC (permalink / raw) To: dri-devel; +Cc: linux-renesas-soc, Kieran Bingham 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 <laurent.pinchart+renesas@ideasonboard.com> --- 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; - } + }; - 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; + + 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); @@ -607,7 +630,7 @@ static void rcar_du_plane_atomic_update(struct drm_plane *plane, struct rcar_du_plane_state *old_rstate; struct rcar_du_plane_state *new_rstate; - if (!plane->state->crtc) + if (!plane->state->visible) return; rcar_du_plane_setup(rplane); diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index dd66dcb8da23..6d1a82ee50ed 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c @@ -212,7 +212,11 @@ static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane, unsigned int i; int ret; - if (!state->fb) + /* + * There's no need to prepare (and unprepare) the framebuffer when the + * plane is not visible, as it will not be displayed. + */ + if (!state->visible) return 0; for (i = 0; i < rstate->format->planes; ++i) { @@ -253,7 +257,7 @@ static void rcar_du_vsp_plane_cleanup_fb(struct drm_plane *plane, struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp; unsigned int i; - if (!state->fb) + if (!state->visible) return; for (i = 0; i < rstate->format->planes; ++i) { @@ -278,7 +282,7 @@ static void rcar_du_vsp_plane_atomic_update(struct drm_plane *plane, struct rcar_du_vsp_plane *rplane = to_rcar_vsp_plane(plane); struct rcar_du_crtc *crtc = to_rcar_crtc(old_state->crtc); - if (plane->state->crtc) + if (plane->state->visible) rcar_du_vsp_plane_setup(rplane); else vsp1_du_atomic_update(rplane->vsp->vsp, crtc->vsp_pipe, -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] drm: rcar-du: Clip planes to screen boundaries 2017-11-13 8:47 ` Laurent Pinchart @ 2017-11-16 17:04 ` Ville Syrjälä -1 siblings, 0 replies; 7+ messages in thread From: Ville Syrjälä @ 2017-11-16 17:04 UTC (permalink / raw) To: Laurent Pinchart; +Cc: dri-devel, linux-renesas-soc, Kieran Bingham 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 <laurent.pinchart+renesas@ideasonboard.com> > --- > 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 ; > > - 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. 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. 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. [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); > @@ -607,7 +630,7 @@ static void rcar_du_plane_atomic_update(struct drm_plane *plane, > struct rcar_du_plane_state *old_rstate; > struct rcar_du_plane_state *new_rstate; > > - if (!plane->state->crtc) > + if (!plane->state->visible) > return; > > rcar_du_plane_setup(rplane); > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > index dd66dcb8da23..6d1a82ee50ed 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > @@ -212,7 +212,11 @@ static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane, > unsigned int i; > int ret; > > - if (!state->fb) > + /* > + * There's no need to prepare (and unprepare) the framebuffer when the > + * plane is not visible, as it will not be displayed. > + */ > + if (!state->visible) > return 0; > > for (i = 0; i < rstate->format->planes; ++i) { > @@ -253,7 +257,7 @@ static void rcar_du_vsp_plane_cleanup_fb(struct drm_plane *plane, > struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp; > unsigned int i; > > - if (!state->fb) > + if (!state->visible) > return; > > for (i = 0; i < rstate->format->planes; ++i) { > @@ -278,7 +282,7 @@ static void rcar_du_vsp_plane_atomic_update(struct drm_plane *plane, > struct rcar_du_vsp_plane *rplane = to_rcar_vsp_plane(plane); > struct rcar_du_crtc *crtc = to_rcar_crtc(old_state->crtc); > > - if (plane->state->crtc) > + if (plane->state->visible) > rcar_du_vsp_plane_setup(rplane); > else > vsp1_du_atomic_update(rplane->vsp->vsp, crtc->vsp_pipe, > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrj�l� Intel OTC ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] drm: rcar-du: Clip planes to screen boundaries @ 2017-11-16 17:04 ` Ville Syrjälä 0 siblings, 0 replies; 7+ messages in thread From: Ville Syrjälä @ 2017-11-16 17:04 UTC (permalink / raw) To: Laurent Pinchart; +Cc: linux-renesas-soc, Kieran Bingham, dri-devel 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 <laurent.pinchart+renesas@ideasonboard.com> > --- > 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 ; > > - 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. 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. 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. [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); > @@ -607,7 +630,7 @@ static void rcar_du_plane_atomic_update(struct drm_plane *plane, > struct rcar_du_plane_state *old_rstate; > struct rcar_du_plane_state *new_rstate; > > - if (!plane->state->crtc) > + if (!plane->state->visible) > return; > > rcar_du_plane_setup(rplane); > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > index dd66dcb8da23..6d1a82ee50ed 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c > @@ -212,7 +212,11 @@ static int rcar_du_vsp_plane_prepare_fb(struct drm_plane *plane, > unsigned int i; > int ret; > > - if (!state->fb) > + /* > + * There's no need to prepare (and unprepare) the framebuffer when the > + * plane is not visible, as it will not be displayed. > + */ > + if (!state->visible) > return 0; > > for (i = 0; i < rstate->format->planes; ++i) { > @@ -253,7 +257,7 @@ static void rcar_du_vsp_plane_cleanup_fb(struct drm_plane *plane, > struct rcar_du_vsp *vsp = to_rcar_vsp_plane(plane)->vsp; > unsigned int i; > > - if (!state->fb) > + if (!state->visible) > return; > > for (i = 0; i < rstate->format->planes; ++i) { > @@ -278,7 +282,7 @@ static void rcar_du_vsp_plane_atomic_update(struct drm_plane *plane, > struct rcar_du_vsp_plane *rplane = to_rcar_vsp_plane(plane); > struct rcar_du_crtc *crtc = to_rcar_crtc(old_state->crtc); > > - if (plane->state->crtc) > + if (plane->state->visible) > rcar_du_vsp_plane_setup(rplane); > else > vsp1_du_atomic_update(rplane->vsp->vsp, crtc->vsp_pipe, > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] drm: rcar-du: Clip planes to screen boundaries 2017-11-16 17:04 ` Ville Syrjälä (?) @ 2017-11-26 11:18 ` Laurent Pinchart -1 siblings, 0 replies; 7+ messages in thread From: Laurent Pinchart @ 2017-11-26 11:18 UTC (permalink / raw) To: Ville Syrjälä Cc: Laurent Pinchart, dri-devel, linux-renesas-soc, Kieran Bingham 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 > > <laurent.pinchart+renesas@ideasonboard.com> > > --- > > > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-11-26 11:18 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-11-13 8:46 [PATCH v2 0/2] R-Car DU: Clip planes to screen boundaries Laurent Pinchart 2017-11-13 8:46 ` [PATCH v2 1/2] drm: rcar-du: Share plane atomic check code between Gen2 and Gen3 Laurent Pinchart 2017-11-13 8:47 ` [PATCH v2 2/2] drm: rcar-du: Clip planes to screen boundaries Laurent Pinchart 2017-11-13 8:47 ` Laurent Pinchart 2017-11-16 17:04 ` Ville Syrjälä 2017-11-16 17:04 ` Ville Syrjälä 2017-11-26 11:18 ` Laurent Pinchart
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.