[PATCH weston v6 49/73] compositor-drm: move connector fields into drm_head

[Pekka Paalanen]

On Thu, 12 Apr 2018 14:03:32 +0200
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi Pekka,
> I've reviewed up to this point (the first half as discussed before,
> second half from this submission), and it all looks good so far. One
> question:
> 
> On 16 February 2018 at 15:57, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > +/** Replace connector data and monitor information
> > + *
> > + * @param head The head to update.
> > + * @param connector The connector data to be owned by the head, must match
> > + * the head's connector ID.
> > + * @return 0 on success, -1 on failure.
> > + *
> > + * Takes ownership of @c connector on success, not on failure.
> > + *
> > + * May schedule a heads changed call.
> > + */
> > +static int
> > +drm_head_assign_connector_info(struct drm_head *head,
> > +                              drmModeConnector *connector)
> > +{
> > +       drmModeObjectProperties *props;
> > +       const char *make = "unknown";
> > +       const char *model = "unknown";
> > +       const char *serial_number = "unknown";
> > +
> > +       assert(connector);
> > +       assert(head->connector_id == connector->connector_id);
> > +
> > +       props = drmModeObjectGetProperties(head->backend->drm.fd,
> > +                                          head->connector_id,
> > +                                          DRM_MODE_OBJECT_CONNECTOR);
> > +       if (!props) {
> > +               weston_log("Error: failed to get connector '%s' properties\n",
> > +                          head->base.name);
> > +               return -1;
> > +       }
> > +
> > +       if (head->connector)
> > +               drmModeFreeConnector(head->connector);
> > +       head->connector = connector;  
> 
> One thing I'm missing (possibly post-lunch tiredness, but): when
> exactly would we replace a connector? Is it only if we have a
> hot-unplugged connector which later appears as a connector of the same
> name? e.g. DP-4 as MST disappears, and then reappears with the same
> name but a different connector ID.

Every time there is any hotplug event, all connectors' information is
reloaded. The information could have changed without us seeing the
connector disconnected in between. This runs by connector IDs, not
names.

drm_head is always created for all connectors we find, regardless of
connected or not.

drm_head_assign_connector_info() deals with disconnected connectors as
well, so it handles also the connected/disconnected changes. After all,
drmModeGetConnector() must be called anyway to see if the connector got
dis/connected.

To avoid triggering a heads changed when nothing actually changed, the
weston_head_set_*() functions check if the data really changed.

> This bit is slightly confusing to me, but I'm pretty sure I've
> followed the rest and it's looking good.


Cool, thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180412/2aa48baa/attachment.sig>