[PATCH v1 2/2] drm/virtio: Add the hotplug_mode_update property for rescanning of modes

[Daniel Vetter]

On Tue, Jan 10, 2023 at 01:58:52AM +0000, Kasireddy, Vivek wrote:
> Hi Daniel,
> 
> > 
> > On Fri, Jan 06, 2023 at 09:56:40AM +0100, Gerd Hoffmann wrote:
> > > On Thu, Nov 17, 2022 at 05:30:54PM -0800, Vivek Kasireddy wrote:
> > > > Setting this property will allow the userspace to look for new modes or
> > > > position info when a hotplug event occurs.
> > >
> > > This works just fine for modes today.
> > >
> > > I assume this is this need to have userspace also check for position
> > > info updates added by patch #1)?
> > 
> > What does this thing even do? Quick grep says qxl and vmwgfx also use
> > this, but it's not documented anywhere, and it's also not done with any
> > piece of common code. Which all looks really fishy.
> [Kasireddy, Vivek] AFAIU, this property appears to be useful only for virtual
> GPU drivers to share the Host output(s) layout with the Guest compositor. The
> suggested_x/y properties are specifically used for this purpose but it looks like
> the hotplug_mode_update property also needs to be set in order to have Guest
> compositors (Mutter cares but Weston does not) look at suggested_x/y.
> 
> > 
> > I think we need to do a bit of refactoring/documenting here first.
> [Kasireddy, Vivek] Just for reference, here is Dave's commit that added this
> property for qxl:
> commit 4695b03970df378dcb93fe3e7158381f1e980fa2
> Author: Dave Airlie <airlied at redhat.com>
> Date:   Fri Oct 11 11:05:00 2013 +1000
> 
>     qxl: add a connector property to denote hotplug should rescan modes.
> 
>     So GNOME userspace has an issue with when it rescans for modes on hotplug
>     events, if the monitor has no EDID it assumes that nothing has changed on
>     EDID as with real hw we'd never have new modes without a new EDID, and they
>     kind off rely on the behaviour now, however with virtual GPUs we would
>     like to rescan the modes and get a new preferred mode on hotplug events
>     to handle dynamic guest resizing (where you resize the host window and the
>     guest resizes with it).

Ok this is just terrible. Because _anything_ without an EDID is impacted,
and we're certainly not going to sprinkle this property all over gpu
drivers just so Gnome takes the right path.

Can we fix gnome instead to be slightly less dense about this stuff?

>     This is a simple property we can make userspace watch for to trigger new
>     behaviour based on it, and can be used to replaced EDID hacks in virtual
>     drivers.
> 
> Are you suggesting that this property needs to be part of drm_mode_config
> just like suggested_x/y properties?

I think this thing shouldn't exist :-)

> > Also in principle, userspace needs to look at everything in the connector
> > again when it gets a hotplug event. We do have hotplug events for specific
> > properties nowadays, but those are fairly new.
> [Kasireddy, Vivek] From what I understand, Mutter does probe all the
> connector properties during hotplug but it still needs this property to be set in
> order to consider suggested_x/y values. And, it appears, some customers and
> users have relied on this behavior from when these properties were first
> introduced for virtual GPU drivers.

Be more like Weston. Whatever it is mutter does, it doesn't make much
sense to me. We can't remove this from existing gpu drivers because no
regression guarantee, but we definitely shouldn't add it anywhere.

Also can you please document that this is a "do not ever use this in any
new driver" kind of property when you go about docementing these?

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