mbox series

[0/3] drm/vmwgfx: Add support for userspace managed surfaces.

Message ID 20240812191639.649841-1-maaz.mombasawala@broadcom.com (mailing list archive)
Headers show
Series drm/vmwgfx: Add support for userspace managed surfaces. | expand

Message

Maaz Mombasawala Aug. 12, 2024, 7:16 p.m. UTC
This series introduces basic support for userspace managed surfaces. The
lifetime and id's of these surfaces is managed by userspace submitted
commands instead of relying on the kernel to manage them.

Maaz Mombasawala (3):
  drm/vmwgfx: Introduce userspace managed surfaces
  drm/vmwgfx: Support hw_destroy for userspace managed surfaces
  drm/vmwgfx: Add support for older define commands for userspace
    surfaces

 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h     |  24 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 331 ++++++++++++++++++++++--
 drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 196 +++++++++++++-
 3 files changed, 518 insertions(+), 33 deletions(-)

Comments

Zack Rusin Aug. 14, 2024, 2:33 a.m. UTC | #1
On Mon, Aug 12, 2024 at 3:16 PM Maaz Mombasawala
<maaz.mombasawala@broadcom.com> wrote:
>
> This series introduces basic support for userspace managed surfaces. The
> lifetime and id's of these surfaces is managed by userspace submitted
> commands instead of relying on the kernel to manage them.
>
> Maaz Mombasawala (3):
>   drm/vmwgfx: Introduce userspace managed surfaces
>   drm/vmwgfx: Support hw_destroy for userspace managed surfaces
>   drm/vmwgfx: Add support for older define commands for userspace
>     surfaces
>
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h     |  24 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 331 ++++++++++++++++++++++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 196 +++++++++++++-
>  3 files changed, 518 insertions(+), 33 deletions(-)
>

In general that looks great. Do you happen to have the userspace patch
somewhere where we could see it? In particular there are three things
I'm wondering about:
1) In the first patch you mark the gb surface as may_evict = false;
correctly, because if user space is the thing that attaches mob's then
kernel can not switch them underneath but then I'd like to see how are
the memory pressure situations handled on the user-side,
2) Since now we allow surface destroy commands from userspace could
one trigger some kernel oops when running old surface defines with
mob_create flag set and issuing the gb surface destroy or will the
res->id be reset properly?
3) how is userspace able to select whether it should self-manage the
mob's or let the kernel do it? i.e. what flag signifies that the
userspace is running on a kernel that is capable of handling this?

z
Maaz Mombasawala Aug. 16, 2024, 12:30 a.m. UTC | #2
On 8/13/24 19:33, Zack Rusin wrote:
> On Mon, Aug 12, 2024 at 3:16 PM Maaz Mombasawala
> <maaz.mombasawala@broadcom.com> wrote:
>>
>> This series introduces basic support for userspace managed surfaces. The
>> lifetime and id's of these surfaces is managed by userspace submitted
>> commands instead of relying on the kernel to manage them.
>>
>> Maaz Mombasawala (3):
>>   drm/vmwgfx: Introduce userspace managed surfaces
>>   drm/vmwgfx: Support hw_destroy for userspace managed surfaces
>>   drm/vmwgfx: Add support for older define commands for userspace
>>     surfaces
>>
>>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h     |  24 +-
>>  drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 331 ++++++++++++++++++++++--
>>  drivers/gpu/drm/vmwgfx/vmwgfx_surface.c | 196 +++++++++++++-
>>  3 files changed, 518 insertions(+), 33 deletions(-)
>>
> 
> In general that looks great. Do you happen to have the userspace patch
> somewhere where we could see it? In particular there are three things
> I'm wondering about:

There is a patch for mesa, I'll work on getting that upstreamed too.

> 1) In the first patch you mark the gb surface as may_evict = false;
> correctly, because if user space is the thing that attaches mob's then
> kernel can not switch them underneath but then I'd like to see how are
> the memory pressure situations handled on the user-side,

I haven't tested this under memory pressure conditions. That should be a
userspace issue though.

> 2) Since now we allow surface destroy commands from userspace could
> one trigger some kernel oops when running old surface defines with
> mob_create flag set and issuing the gb surface destroy or will the
> res->id be reset properly?

For userspace managed surfaces, the driver only accepts ids in the range
1 to 32768 (VMWGFX_NUM_GB_SURFACE) and for an old kernel managed surfaces
it only returns ids in range starting at 53040 (VMWGFX_NUM_MOB).
So if userspace defines a surface with the old ioctls and then tries to
issue the gb_surface_destroy on that surface with that id then the command
buffer submission will fail because it won't pass the res checks.

> 3) how is userspace able to select whether it should self-manage the
> mob's or let the kernel do it? i.e. what flag signifies that the
> userspace is running on a kernel that is capable of handling this?

There is no such flag right now, I will add this.

> 
> z