在2024年7月21日七月 下午6:08，Simon Glass写道：
[...]
>> >> > +
>> >> > + set_scanout_in = malloc(sizeof(struct virtio_gpu_set_scanout));
>> >
>> > Could these structs be inside priv instead of allocating each one?
>>
>> Those structs are all used only once at initialization.
>>
>> We can save some runtime memory by freeing them here :-)
>
> Then you can just use a local var.
That will yield a huge stack frame that breaks booting on RISC-V virt board :-(
Thanks
- Jiaxun
>
>>
>> >
>> >> > + while (scanout_mask) {
>> >> > + u32 active_scanout = ffs(scanout_mask) - 1;
>> >> > +
>> >> > + set_scanout_in->r.x = 0;
>> >> > + set_scanout_in->r.y = 0;
>> >> > + set_scanout_in->r.width = cpu_to_virtio32(dev,
>> >> > uc_priv->xsize);
>> >> > + set_scanout_in->r.height = cpu_to_virtio32(dev,
>> >> > uc_priv->ysize);
>> >> > + set_scanout_in->scanout_id = cpu_to_virtio32(dev,
>> >> > active_scanout);
>> >> > + set_scanout_in->resource_id = cpu_to_virtio32(dev,
>> >> > priv->scanout_res_id);
>> >> > +
>
> [..]
>
>> >> > +/* data passed in the cursor vq */
>> >> > +
>> >> > +struct virtio_gpu_cursor_pos {
>> >> > + __le32 scanout_id;
>> >> > + __le32 x;
>> >> > + __le32 y;
>> >> > + __le32 padding;
>> >> > +};
>> >> > +
>> >> > +/* VIRTIO_GPU_CMD_UPDATE_CURSOR, VIRTIO_GPU_CMD_MOVE_CURSOR */
>> >> > +struct virtio_gpu_update_cursor {
>> >> > + struct virtio_gpu_ctrl_hdr hdr;
>> >> > + struct virtio_gpu_cursor_pos pos; /* update & move */
>> >> > + __le32 resource_id; /* update only */
>> >> > + __le32 hot_x; /* update only */
>> >> > + __le32 hot_y; /* update only */
>> >> > + __le32 padding;
>> >> > +};
>> >> > +
>> >> > +/* data passed in the control vq, 2d related */
>> >
>> > But there are no comments so we don't know what these structs are for
>> > or what the fields do. Can you add documentation, or a pointer to
>> > somewhere where it exists?
>>
>> The whole header is copied from Linux kernel so I’m not really sure if we
>> want to edit it.
>>
>> Maybe I can mention about the specification at start of the driver code?
>
> Yes that would help.
>
> I thought Linux's aversion to comments had softened a little?
>
> Regards,
> Simon
--
- Jiaxun
