Hi all,
Just a small fly-by idea.
On Thu, 19 Mar 2020 at 20:25, <rscheidegger.oss@xxxxxxxxx> wrote:
>
> From: Deepak Rawat <drawat.floss@xxxxxxxxx>
>
> Surface define v4 added new member buffer_byte_stride. With this patch
> add buffer_byte_stride in surface metadata and create surface using new
> command if support is available.
>
> Also with this patch replace device specific data types with kernel
> types.
>
> Signed-off-by: Deepak Rawat <drawat.floss@xxxxxxxxx>
> Reviewed-by: Thomas Hellström (VMware) <thomas_os@xxxxxxxxxxxx>
> Signed-off-by: Roland Scheidegger (VMware) <rscheidegger.oss@xxxxxxxxx>
> ---
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 2 ++
> drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 36 +++++++++++++++++++++++--
> include/uapi/drm/vmwgfx_drm.h | 12 +++++----
> 3 files changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 212b66da2d4c..aa4131f5f8fc 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -238,6 +238,7 @@ struct vmw_surface_offset;
> * @array_size: Number of array elements for a 1D/2D texture. For cubemap
> texture number of faces * array_size. This should be 0 for pre
> SM4 device.
> + * @buffer_byte_stride: Buffer byte stride.
> * @num_sizes: Size of @sizes. For GB surface this should always be 1.
> * @base_size: Surface dimension.
> * @sizes: Array representing mip sizes. Legacy only.
> @@ -255,6 +256,7 @@ struct vmw_surface_metadata {
> u32 autogen_filter;
> u32 array_size;
> u32 num_sizes;
> + u32 buffer_byte_stride;
> struct drm_vmw_size base_size;
> struct drm_vmw_size *sizes;
> bool scanout;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> index 0e71a3ea281b..dad49ab4a0bf 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_surface.c
> @@ -1082,6 +1082,10 @@ static int vmw_gb_surface_create(struct vmw_resource *res)
> SVGA3dCmdHeader header;
> SVGA3dCmdDefineGBSurface_v3 body;
> } *cmd3;
> + struct {
> + SVGA3dCmdHeader header;
> + SVGA3dCmdDefineGBSurface_v4 body;
> + } *cmd4;
>
> if (likely(res->id != -1))
> return 0;
> @@ -1098,7 +1102,11 @@ static int vmw_gb_surface_create(struct vmw_resource *res)
> goto out_no_fifo;
> }
>
> - if (has_sm4_1_context(dev_priv) && metadata->array_size > 0) {
> + if (has_sm5_context(dev_priv) && metadata->array_size > 0) {
> + cmd_id = SVGA_3D_CMD_DEFINE_GB_SURFACE_V4;
> + cmd_len = sizeof(cmd4->body);
> + submit_len = sizeof(*cmd4);
> + } else if (has_sm4_1_context(dev_priv) && metadata->array_size > 0) {
> cmd_id = SVGA_3D_CMD_DEFINE_GB_SURFACE_V3;
> cmd_len = sizeof(cmd3->body);
> submit_len = sizeof(*cmd3);
> @@ -1116,12 +1124,29 @@ static int vmw_gb_surface_create(struct vmw_resource *res)
> cmd = VMW_FIFO_RESERVE(dev_priv, submit_len);
> cmd2 = (typeof(cmd2))cmd;
> cmd3 = (typeof(cmd3))cmd;
> + cmd4 = (typeof(cmd4))cmd;
> if (unlikely(!cmd)) {
> ret = -ENOMEM;
> goto out_no_fifo;
> }
>
I'm not sure if SVGA3dCmdDefineGBSurface_** is ABI or not.
If not one could tweak and simplify things a fair bit:
- tweak the struct layout - always append newly introduced variables
- allocate the max size (in VMW_FIFO_RESERVE)
- cut down the massive duplication as seen below
cmd4->foo = foo... cmd3->foo = foo... cmd2->foo = foo... cmd->foo = foo
> - if (has_sm4_1_context(dev_priv) && metadata->array_size > 0) {
> + if (has_sm5_context(dev_priv) && metadata->array_size > 0) {
> + cmd4->header.id = cmd_id;
> + cmd4->header.size = cmd_len;
> + cmd4->body.sid = srf->res.id;
> + cmd4->body.surfaceFlags = metadata->flags;
> + cmd4->body.format = metadata->format;
> + cmd4->body.numMipLevels = metadata->mip_levels[0];
> + cmd4->body.multisampleCount = metadata->multisample_count;
> + cmd4->body.multisamplePattern = metadata->multisample_pattern;
> + cmd4->body.qualityLevel = metadata->quality_level;
> + cmd4->body.autogenFilter = metadata->autogen_filter;
> + cmd4->body.size.width = metadata->base_size.width;
> + cmd4->body.size.height = metadata->base_size.height;
> + cmd4->body.size.depth = metadata->base_size.depth;
> + cmd4->body.arraySize = metadata->array_size;
> + cmd4->body.bufferByteStride = metadata->buffer_byte_stride;
> + } else if (has_sm4_1_context(dev_priv) && metadata->array_size > 0) {
> cmd3->header.id = cmd_id;
> cmd3->header.size = cmd_len;
> cmd3->body.sid = srf->res.id;
HTH
Emil
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
