...
> +static struct
> +drm_plane_state *virtio_gpu_plane_duplicate_state(struct drm_plane *plane)
> +{
> +     struct virtio_gpu_plane_state *new;
> +
> +     if (WARN_ON(!plane->state))
> +             return NULL;

When plane->state can be NULL?

> +     new = kzalloc(sizeof(*new), GFP_KERNEL);
> +     if (!new)
> +             return NULL;
> +
> +     __drm_atomic_helper_plane_duplicate_state(plane, &new->base);
> +
> +     return &new->base;
> +}
> +
> +static void virtio_gpu_plane_destroy_state(struct drm_plane *plane,
> +                                        struct drm_plane_state *state)
> +{
> +     __drm_atomic_helper_plane_destroy_state(state);
> +     kfree(to_virtio_gpu_plane_state(state));
> +}
> +
>  static const struct drm_plane_funcs virtio_gpu_plane_funcs = {
>       .update_plane           = drm_atomic_helper_update_plane,
>       .disable_plane          = drm_atomic_helper_disable_plane,
>       .reset                  = drm_atomic_helper_plane_reset,
> -     .atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
> -     .atomic_destroy_state   = drm_atomic_helper_plane_destroy_state,
> +     .atomic_duplicate_state = virtio_gpu_plane_duplicate_state,
> +     .atomic_destroy_state   = virtio_gpu_plane_destroy_state,

Similar to the other email, please see how container_of() works. There
is no need change .atomic_destroy_state

...
> @@ -237,41 +262,29 @@ static int virtio_gpu_plane_prepare_fb(struct drm_plane 
> *plane,
>       struct drm_device *dev = plane->dev;
>       struct virtio_gpu_device *vgdev = dev->dev_private;
>       struct virtio_gpu_framebuffer *vgfb;
> +     struct virtio_gpu_plane_state *vgplane_st;
>       struct virtio_gpu_object *bo;
>  
>       if (!new_state->fb)
>               return 0;
>  
>       vgfb = to_virtio_gpu_framebuffer(new_state->fb);
> +     vgplane_st = to_virtio_gpu_plane_state(new_state);
>       bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]);
>       if (!bo || (plane->type == DRM_PLANE_TYPE_PRIMARY && !bo->guest_blob))
>               return 0;
>  
> -     if (bo->dumb && (plane->state->fb != new_state->fb)) {
> -             vgfb->fence = virtio_gpu_fence_alloc(vgdev, 
> vgdev->fence_drv.context,
> +     if (bo->dumb) {

Why "&& (plane->state->fb != new_state->fb)" disappeared?

> +             vgplane_st->fence = virtio_gpu_fence_alloc(vgdev,
> +                                                  vgdev->fence_drv.context,
>                                                    0);
> -             if (!vgfb->fence)
> +             if (!vgplane_st->fence)
>                       return -ENOMEM;
>       }
>  
>       return 0;
>  }
>  
> -static void virtio_gpu_plane_cleanup_fb(struct drm_plane *plane,
> -                                     struct drm_plane_state *state)
> -{
> -     struct virtio_gpu_framebuffer *vgfb;
> -
> -     if (!state->fb)
> -             return;
> -
> -     vgfb = to_virtio_gpu_framebuffer(state->fb);
> -     if (vgfb->fence) {
> -             dma_fence_put(&vgfb->fence->f);
> -             vgfb->fence = NULL;
> -     }
> -}
How come that virtio_gpu_plane_cleanup_fb() isn't needed anymore? You
created fence in prepare_fb(), you must release it in cleanup_fb() if
fence still presents.

-- 
Best regards,
Dmitry

Reply via email to