On Mon, Dec 2, 2024 at 11:30 PM Dmitry Osipenko
<dmitry.osipenko@xxxxxxxxxxxxx> wrote:
>
> On 11/29/24 15:24, Ryosuke Yasuoka wrote:
> ....
> > + } else {
> > + iosys_map_set_vaddr(&sb->map[0], bo->base.vaddr);
> > + }
> > +
> > + sb->format = plane->state->fb->format;
> > + sb->height = plane->state->fb->height;
> > + sb->width = plane->state->fb->width;
> > + sb->pitch[0] = plane->state->fb->pitches[0];
> > + return 0;
> > +}
> ...
> > +static void virtio_panic_flush(struct drm_plane *plane)
> > +{
> > + struct virtio_gpu_object *bo;
> > + struct drm_device *dev = plane->dev;
> > + struct virtio_gpu_device *vgdev = dev->dev_private;
> > + struct drm_rect rect;
> > + void *p_vbuf = vgdev->panic_vbuf;
> > + struct virtio_gpu_vbuffer *vbuf_dumb_bo = p_vbuf;
> > + struct virtio_gpu_vbuffer *vbuf_resource_flush = p_vbuf + VBUFFER_SIZE;
>
> This p_vbuf + VBUFFER_SIZE looks suspicious. The VBUFFER_SIZE macro
> isn't guarded with parentheses (), hence this vbuf_resource_flush is
> pointing at the CMD part of the vbuf_dumb_bo?
No. I intend vbuf_resource_flush to point to a different vbuf than
vbuf_dumb_bo. I allocate 2 vbufs in virtio_gpu_plane_init() so that
vbuf_dumb_bo points to the first vbuf and vbuf_resource_flush points
to the second one.
> Won't be using kmem_cache_zalloc(vgdev->vbufs, GFP_ATOMIC) to
> dynamically allocate both buffers make everything cleaner?
>
> ...
> > -#define MAX_INLINE_CMD_SIZE 96
> > -#define MAX_INLINE_RESP_SIZE 24
> > -#define VBUFFER_SIZE (sizeof(struct virtio_gpu_vbuffer) \
> > - + MAX_INLINE_CMD_SIZE \
> > - + MAX_INLINE_RESP_SIZE)...
>
Yes, it is feasible.
Thank you for your review.
Ryosuke
> Best regards,
> Dmitry
>