[PATCH 1/4] drm/vmwgfx: Assign eviction priorities to resources

[Daniel Vetter]

On Tue, Jun 18, 2019 at 03:08:01PM +0200, Thomas Hellstrom wrote:
> On 6/18/19 2:19 PM, Daniel Vetter wrote:
> > On Tue, Jun 18, 2019 at 11:54:08AM +0100, Emil Velikov wrote:
> > > Hi Thomas,
> > > 
> > > On 2019/06/18, Thomas Hellström (VMware) wrote:
> > > > From: Thomas Hellstrom <thellstrom at vmware.com>
> > > > 
> > > > TTM provides a means to assign eviction priorities to buffer object. This
> > > > means that all buffer objects with a lower priority will be evicted first
> > > > on memory pressure.
> > > > Use this to make sure surfaces and in particular non-dirty surfaces are
> > > > evicted first. Evicting in particular shaders, cotables and contexts imply
> > > > a significant performance hit on vmwgfx, so make sure these resources are
> > > > evicted last.
> > > > Some buffer objects are sub-allocated in user-space which means we can have
> > > > many resources attached to a single buffer object or resource. In that case
> > > > the buffer object is given the highest priority of the attached resources.
> > > > 
> > > > Signed-off-by: Thomas Hellstrom <thellstrom at vmware.com>
> > > > Reviewed-by: Deepak Rawat <drawat at vmware.com>
> > > Fwiw patches 1-3 are:
> > > Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
> > > 
> > > Patch 4 is:
> > > Acked-by: Emil Velikov <emil.velikov at collabora.com>
> > > 
> > > Huge thanks for sorting this out.
> > Oh, does this mean we can remove the varios master* callbacks from
> > drm_driver now? Iirc vmwgfx was the only user, and those callbacks seem
> > very tempting to various folks for implementing questionable driver hacks
> > ... Happy to type the patches, but maybe simpler if you do that since all
> > this gets merged through the vmwgfx tree.
> > 
> > Cheers, Daniel
> 
> In case someone follow this, I'll paste in the commit message of 4/4 which
> is the relevant one here..
> 
> 8<--------------------------------------------
> 
> At one point, the GPU command verifier and user-space handle manager
> couldn't properly protect GPU clients from accessing each other's data.
> Instead there was an elaborate mechanism to make sure only the active
> master's primary clients could render. The other clients were either
> put to sleep or even killed (if the master had exited). VRAM was
> evicted on master switch. With the advent of render-node functionality,
> we relaxed the VRAM eviction, but the other mechanisms stayed in place.
> 
> Now that the GPU  command verifier and ttm object manager properly
> isolates primary clients from different master realms we can remove the
> master switch related code and drop those legacy features.
> 
> 8<-------------------------------------------
> 
> I think we can at least take a look. I'm out on a fairly long vacation soon
> so in any case it won't be before August or so.

Ah don't worry, if this all lands in the 5.3 merge window I can take a
look in a few weeks.

> One use we still have for master_set() is that if a master is switched away,
> and then the mode list changes, and then the master is switched back, it
> will typically not remember to act on the sysfs event received while
> switched out, and come back in an incorrect mode. Since mode-list changes
> happen quite frequently with virtual display adapters that's bad.
> 
> But perhaps we can consider moving that to core, if that's what needed to
> get rid of the master switch callbacks.

Hm, this sounds a bit like papering over userspace bugs, at least if
you're referring to drm_sysfs_hotplug_event(). Userspace is supposed to
either keep listening or to re-acquire all the kms output state and do the
hotplugg processing in one go when becoming active again.

Ofc it exists, so we can't just remove it. I wouldn't want to make this
part of the uapi though, feels like duct-taping around sloppy userspace.
Maybe we could work on a gradual plan to deprecate this, with limiting it
only to older vmwgfx versions as a start?

These kind of tiny but important differences in how drivers implement kms
is why I'd much, much prefer it's not even possible to do stuff like this.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch