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

[Pekka Paalanen]

On Thu, 12 Apr 2018 15:37:43 +0300
Pekka Paalanen <ppaalanen at gmail.com> wrote:

> 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.

Oops, I suppose this happens at a later step in the series. I think my
explanation should still apply even at this step though, for the missed
unplug event part.


Thanks,
pq

> 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/e7763ca7/attachment-0001.sig>