On Fri, Jun 10, 2016 at 12:07:53AM +0200, Daniel Vetter wrote:
> Now that the core helpers support nonblocking atomic commits there's
> no need to invent that wheel separately (instead of fixing the bug in
> the atomic implementation of virtio, as it should have been done!).
>
> v2: Rebased on top of
>
> commit e7cf0963f816fa44190caaf51aeffaa614c340c6
> Author: Gerd Hoffmann <kraxel at redhat.com>
> Date: Tue May 31 08:50:47 2016 +0200
>
> virtio-gpu: add atomic_commit function
>
> Cc: Gerd Hoffmann <kraxel at redhat.com>
> Tested-by: Gerd Hoffmann <kraxel at redhat.com> (v1)
> Reviewed-by: Gerd Hoffmann <kraxel at redhat.com> (v1)
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
Gerd, can you pls retest? I think due to your change in the above
referenced commit to switch to active_only=true in commit_planes() this is
now broken.
-Daniel
> ---
> drivers/gpu/drm/virtio/virtgpu_display.c | 71
> ++++++--------------------------
> 1 file changed, 13 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
> b/drivers/gpu/drm/virtio/virtgpu_display.c
> index 3d0fa049b34c..a09dc57fea9c 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_display.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_display.c
> @@ -38,56 +38,11 @@
> #define XRES_MAX 8192
> #define YRES_MAX 8192
>
> -static int virtio_gpu_page_flip(struct drm_crtc *crtc,
> - struct drm_framebuffer *fb,
> - struct drm_pending_vblank_event *event,
> - uint32_t flags)
> -{
> - struct virtio_gpu_device *vgdev = crtc->dev->dev_private;
> - struct virtio_gpu_output *output =
> - container_of(crtc, struct virtio_gpu_output, crtc);
> - struct drm_plane *plane = crtc->primary;
> - struct virtio_gpu_framebuffer *vgfb;
> - struct virtio_gpu_object *bo;
> - unsigned long irqflags;
> - uint32_t handle;
> -
> - plane->fb = fb;
> - vgfb = to_virtio_gpu_framebuffer(plane->fb);
> - bo = gem_to_virtio_gpu_obj(vgfb->obj);
> - handle = bo->hw_res_handle;
> -
> - DRM_DEBUG("handle 0x%x%s, crtc %dx%d\n", handle,
> - bo->dumb ? ", dumb" : "",
> - crtc->mode.hdisplay, crtc->mode.vdisplay);
> - if (bo->dumb) {
> - virtio_gpu_cmd_transfer_to_host_2d
> - (vgdev, handle, 0,
> - cpu_to_le32(crtc->mode.hdisplay),
> - cpu_to_le32(crtc->mode.vdisplay),
> - 0, 0, NULL);
> - }
> - virtio_gpu_cmd_set_scanout(vgdev, output->index, handle,
> - crtc->mode.hdisplay,
> - crtc->mode.vdisplay, 0, 0);
> - virtio_gpu_cmd_resource_flush(vgdev, handle, 0, 0,
> - crtc->mode.hdisplay,
> - crtc->mode.vdisplay);
> -
> - if (event) {
> - spin_lock_irqsave(&crtc->dev->event_lock, irqflags);
> - drm_crtc_send_vblank_event(crtc, event);
> - spin_unlock_irqrestore(&crtc->dev->event_lock, irqflags);
> - }
> -
> - return 0;
> -}
> -
> static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = {
> .set_config = drm_atomic_helper_set_config,
> .destroy = drm_crtc_cleanup,
>
> - .page_flip = virtio_gpu_page_flip,
> + .page_flip = drm_atomic_helper_page_flip,
> .reset = drm_atomic_helper_crtc_reset,
> .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> @@ -185,6 +140,7 @@ static void virtio_gpu_crtc_atomic_flush(struct drm_crtc
> *crtc,
> spin_lock_irqsave(&crtc->dev->event_lock, flags);
> if (crtc->state->event)
> drm_crtc_send_vblank_event(crtc, crtc->state->event);
> + crtc->state->event = NULL;
> spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> }
>
> @@ -388,30 +344,28 @@ virtio_gpu_user_framebuffer_create(struct drm_device
> *dev,
> return &virtio_gpu_fb->base;
> }
>
> -static int vgdev_atomic_commit(struct drm_device *dev,
> - struct drm_atomic_state *state,
> - bool nonblock)
> +static void vgdev_atomic_commit_tail(struct drm_atomic_state *state)
> {
> - if (nonblock)
> - return -EBUSY;
> -
> - drm_atomic_helper_swap_state(state, true);
> - drm_atomic_helper_wait_for_fences(dev, state);
> + struct drm_device *dev = state->dev;
>
> drm_atomic_helper_commit_modeset_disables(dev, state);
> drm_atomic_helper_commit_modeset_enables(dev, state);
> drm_atomic_helper_commit_planes(dev, state, true);
>
> + drm_atomic_helper_commit_hw_done(state);
> +
> drm_atomic_helper_wait_for_vblanks(dev, state);
> drm_atomic_helper_cleanup_planes(dev, state);
> - drm_atomic_state_free(state);
> - return 0;
> }
>
> +struct drm_mode_config_helper_funcs virtio_mode_config_helpers = {
> + .atomic_commit_tail = vgdev_atomic_commit_tail,
> +};
> +
> static const struct drm_mode_config_funcs virtio_gpu_mode_funcs = {
> .fb_create = virtio_gpu_user_framebuffer_create,
> .atomic_check = drm_atomic_helper_check,
> - .atomic_commit = vgdev_atomic_commit,
> + .atomic_commit = drm_atomic_helper_commit,
> };
>
> int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev)
> @@ -419,7 +373,8 @@ int virtio_gpu_modeset_init(struct virtio_gpu_device
> *vgdev)
> int i;
>
> drm_mode_config_init(vgdev->ddev);
> - vgdev->ddev->mode_config.funcs = (void *)&virtio_gpu_mode_funcs;
> + vgdev->ddev->mode_config.funcs = &virtio_gpu_mode_funcs;
> + vgdev->ddev->mode_config.helper_private = &virtio_mode_config_helpers;
>
> /* modes will be validated against the framebuffer size */
> vgdev->ddev->mode_config.min_width = XRES_MIN;
> --
> 2.8.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
