On Wed, Nov 20, 2024 at 4:22 AM Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> wrote:
>
> Please avoid all struct edid based interfaces, in this case
> drm_connector_update_edid_property(). They will be removed in the
> future, and adding more is counter-productive. Everything should be
> struct drm_edid based going forward.
>
> Of course, actually grafting the EDID needs struct edid. And that's kind
> of annoying too. Do we really want to spread the EDID details all over
> the place? This one combines drm_edid.h structs and magic numbers in a
> jumble. I'm kind of hoping we'd get rid of driver usage of struct edid,
> though that's a long road. But we've made a lot of progress towards it,
> there aren't that many places left that directly look at the guts of
> EDID, and most of it is centralized in drm_edid.c.
>
drm_edid isn't exported so we can't use it. I know you've read the EDID spec so complaining about the magic numbers is silly.
I didn't choose to make the whole spec based on bizarrely packed 10 and 12 bit numbers, they are unavoidable.
>
> > Maybe it resolves problems with
> > compositors, but it is a step backwards for the overall ecosystem. If
> > the connector changes, your driver should increment the epoch counter.
> > [1] That will send a hotplug event to userspace. The EDID alone does not
> > say anything about connector status.
>
> Yeah, unplugging and replugging the same display with the same EDID
> isn't a problem for other drivers, and they don't have to do this kind
> of stuff to trick userspace. Maybe vmwgfx should handle (or simulate)
> hotplugs better?
>
> And preferrably let the EDID functions handle epoch counter updates
> instead of doing it yourself, if at all possible.
>
> BR,
> Jani.
>
You're both missing the fact that virtual devices never disconnect active displays. We don't have to do a disconnect/connect cycle like a physical monitor and we wouldn't want to because it would be poor user experience. The issue is not sending the hotplug event, it's that userspace will ignore hotplug events on connectors that were previously connected because they assume a disconnect/connect cycle must occur for changes to occur. The whole point of hotplug_mode_update was as a hint to userspace that the display can be "re-plugged" without a disconnect first and to always rescan the connector for changes on hotplug.
Currently compositors are taking an arbitrary set of connector properties that they've organically collected over the years and doing a diff to trigger a refresh in the modes/display layout. EDID is the only property that they universally agree should trigger a display layout change. The fact that Autofit works on any compsitors using vmwgfx, qxl, or virtio is completely by accident.
EDID is also a standardized connector property so it's not really fair to say that all devices should export this property and then when we fix our deficiency to block it. Now that it's standardized it is part of the uapi and if userspace does weird things with it it's not really our concern. The fact that it's standardized is also likely the reason it is being used in such a fashion.
Ian,
