On Fri, Nov 15, 2019 at 09:42:00PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Yank out the code for the plane->fb/old_fb/crtc handling from
> the page flip path into page_flip_internal(), and provide a
> simpler variant for atomic drivers.
>
> We'll also move the fb vs. src viewport checks into the new
> functions as they are slightly different between the two paths.
> If the atomic .page_flip() implementations are guaranteed
> to call drm_atomic_plane_check() we could even drop the check
> entirely from page_flip_atomic(). For now toss in a FIXME.
>
> v2: Bit more polish in page_flip_internal()
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_plane.c | 159 +++++++++++++++++++++++-------------
> 1 file changed, 102 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 38878da5b704..6052475a20a5 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -1031,13 +1031,109 @@ int drm_mode_cursor2_ioctl(struct drm_device *dev,
> return drm_mode_cursor_common(dev, req, file_priv);
> }
>
> +static int page_flip_check_fbs(const struct drm_framebuffer *fb,
> + const struct drm_framebuffer *old_fb)
> +{
> + /* The framebuffer is currently unbound, presumably
> + * due to a hotplug event, that userspace has not
> + * yet discovered.
> + */
> + if (!old_fb)
> + return -EBUSY;
> +
> + if (old_fb->format != fb->format) {
> + DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer format.\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int page_flip_internal(struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + struct drm_pending_vblank_event *e,
> + u32 flags,
> + u32 target_vblank,
> + struct drm_modeset_acquire_ctx *ctx)
> +{
> + struct drm_plane *plane = crtc->primary;
> + int ret;
> +
> + WARN_ON(drm_drv_uses_atomic_modeset(crtc->dev));
> +
> + ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y,
> + &crtc->mode, fb);
> + if (ret)
> + return ret;
> +
> + ret = page_flip_check_fbs(fb, plane->fb);
> + if (ret)
> + return ret;
> +
> + plane->old_fb = plane->fb;
> + if (crtc->funcs->page_flip_target)
> + ret = crtc->funcs->page_flip_target(crtc, fb, e, flags,
> + target_vblank, ctx);
> + else
> + ret = crtc->funcs->page_flip(crtc, fb, e, flags, ctx);
> + if (ret) {
> + plane->old_fb = NULL;
> + return ret;
> + }
> +
> + plane->fb = fb;
> + drm_framebuffer_get(plane->fb);
> +
> + drm_framebuffer_put(plane->old_fb);
> + plane->old_fb = NULL;
> +
> + return 0;
> +}
> +
> +static int page_flip_atomic(struct drm_crtc *crtc,
> + struct drm_framebuffer *fb,
> + struct drm_pending_vblank_event *e,
> + u32 flags,
> + u32 target_vblank,
> + struct drm_modeset_acquire_ctx *ctx)
> +{
> + struct drm_plane *plane = crtc->primary;
> + struct drm_plane_state *plane_state = plane->state;
> + int ret;
> +
> + WARN_ON(!drm_drv_uses_atomic_modeset(crtc->dev));
> +
> + /*
> + * FIXME: Can we assume all drivers end up calling
> + * drm_atomic_plane_check() in their page flip paths?
> + * If so we could remove this.
> + */
This is definitely wrong, core has to check these. Currently no driver
checks this at all. I think you're thinking of
drm_atomic_helper_check_plane_state().
That aside I'm kinda not getting the point of the shuffling/cleanup in the
first 4 patches here, so will skip them.
-Daniel
> + ret = drm_framebuffer_check_src_coords(plane_state->src_x,
> + plane_state->src_y,
> + plane_state->src_w,
> + plane_state->src_h,
> + fb);
> + if (ret)
> + return ret;
> +
> + ret = page_flip_check_fbs(fb, plane_state->fb);
> + if (ret)
> + return ret;
> +
> + if (crtc->funcs->page_flip_target)
> + return crtc->funcs->page_flip_target(crtc, fb, e, flags,
> + target_vblank, ctx);
> + else
> + return crtc->funcs->page_flip(crtc, fb, e, flags, ctx);
> +}
> +
> int drm_mode_page_flip_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv)
> {
> struct drm_mode_crtc_page_flip_target *page_flip = data;
> struct drm_crtc *crtc;
> struct drm_plane *plane;
> - struct drm_framebuffer *fb, *old_fb;
> + struct drm_framebuffer *fb;
> struct drm_pending_vblank_event *e = NULL;
> u32 target_vblank = page_flip->sequence;
> struct drm_modeset_acquire_ctx ctx;
> @@ -1145,65 +1241,14 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
> if (ret)
> goto out;
>
> - if (plane->state)
> - old_fb = plane->state->fb;
> + if (drm_drv_uses_atomic_modeset(dev))
> + ret = page_flip_atomic(crtc, fb, e, page_flip->flags,
> + target_vblank, &ctx);
> else
> - old_fb = plane->fb;
> -
> - if (old_fb == NULL) {
> - /* The framebuffer is currently unbound, presumably
> - * due to a hotplug event, that userspace has not
> - * yet discovered.
> - */
> - ret = -EBUSY;
> - goto out;
> - }
> -
> - if (plane->state) {
> - const struct drm_plane_state *state = plane->state;
> -
> - ret = drm_framebuffer_check_src_coords(state->src_x,
> - state->src_y,
> - state->src_w,
> - state->src_h,
> - fb);
> - } else {
> - ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y,
> - &crtc->mode, fb);
> - }
> - if (ret)
> - goto out;
> -
> - if (old_fb->format != fb->format) {
> - DRM_DEBUG_KMS("Page flip is not allowed to change frame buffer format.\n");
> - ret = -EINVAL;
> - goto out;
> - }
> -
> - plane->old_fb = plane->fb;
> - if (crtc->funcs->page_flip_target)
> - ret = crtc->funcs->page_flip_target(crtc, fb, e,
> - page_flip->flags,
> - target_vblank,
> - &ctx);
> - else
> - ret = crtc->funcs->page_flip(crtc, fb, e, page_flip->flags,
> - &ctx);
> - if (ret) {
> - /* Keep the old fb, don't unref it. */
> - plane->old_fb = NULL;
> - } else {
> - if (!plane->state) {
> - plane->fb = fb;
> - drm_framebuffer_get(fb);
> - }
> - }
> + ret = page_flip_internal(crtc, fb, e, page_flip->flags,
> + target_vblank, &ctx);
>
> out:
> - if (plane->old_fb)
> - drm_framebuffer_put(plane->old_fb);
> - plane->old_fb = NULL;
> -
> if (ret == -EDEADLK) {
> ret = drm_modeset_backoff(&ctx);
> if (!ret)
> --
> 2.23.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
