| Message ID | 20190905103746.32326-3-stanislav.lisovskiy@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Send a hotplug when edid changes | expand |
Op 05-09-2019 om 12:37 schreef Stanislav Lisovskiy: > This counter will be used by drm_helper_probe_detect caller to determine > if something else had changed except connection status, > like for example edid. Hardware specific drivers are responsible > for updating this counter when some change is detected to notify > the drm part, which can trigger for example hotplug event. > > Currently there is no way to propagate that to a calling layer, > as we send only connection_status update, however as we see with > edid the changes can be broader. > > v2: Added documentation for the new counter. Rename change_counter to > epoch_counter. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540 > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > --- > drivers/gpu/drm/drm_connector.c | 1 + > drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++++++++++++++-- > include/drm/drm_connector.h | 3 +++ > 3 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c > index 4c766624b20d..18b1ad2a4eee 100644 > --- a/drivers/gpu/drm/drm_connector.c > +++ b/drivers/gpu/drm/drm_connector.c > @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev, > INIT_LIST_HEAD(&connector->modes); > mutex_init(&connector->mutex); > connector->edid_blob_ptr = NULL; > + connector->epoch_counter = 0; > connector->tile_blob_ptr = NULL; > connector->status = connector_status_unknown; > connector->display_info.panel_orientation = > diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c > index 351cbc40f0f8..5131ae56e676 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -777,6 +777,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) > struct drm_connector_list_iter conn_iter; > enum drm_connector_status old_status; > bool changed = false; > + uint64_t old_epoch_counter; > > if (!dev->mode_config.poll_enabled) > return false; > @@ -790,20 +791,44 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) > > old_status = connector->status; > > + old_epoch_counter = connector->epoch_counter; > + > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old change counter %llu\n", connector->base.id, > + connector->name, > + old_epoch_counter); > + > connector->status = drm_helper_probe_detect(connector, NULL, false); Pass a u32 *changed as argument to drm_helper_probe_detect? would require signature change, > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", > connector->base.id, > connector->name, > drm_get_connector_status_name(old_status), > drm_get_connector_status_name(connector->status)); > - if (old_status != connector->status) > + > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] New change counter %llu\n", > + connector->base.id, > + connector->name, > + connector->epoch_counter); > + > + if (old_status != connector->status) { > changed = true; > + } > + > + /* Check changing of edid when a connector status still remains > + * as "connector_status_connected". > + */ > + if (connector->status == connector_status_connected && > + old_status == connector_status_connected && > + old_epoch_counter != connector->epoch_counter) { > + changed = true; > + } Could we bump the epoch counter for any event being sent out? Would make more sense. > } > drm_connector_list_iter_end(&conn_iter); > mutex_unlock(&dev->mode_config.mutex); > > - if (changed) > + if (changed) { > drm_kms_helper_hotplug_event(dev); > + DRM_DEBUG_KMS("Sent hotplug event\n"); > + } > > return changed; > } > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > index 681cb590f952..a3cc7d0927d6 100644 > --- a/include/drm/drm_connector.h > +++ b/include/drm/drm_connector.h > @@ -1288,6 +1288,9 @@ struct drm_connector { > /** @override_edid: has the EDID been overwritten through debugfs for testing? */ > bool override_edid; > > + /** @epoch_counter: used to detect any other changes in connector, besides status */ > + uint64_t epoch_counter; > + > #define DRM_CONNECTOR_MAX_ENCODER 3 > /** > * @encoder_ids: Valid encoders for this connector. Please only use
On Thu, 2019-09-05 at 13:01 +0200, Maarten Lankhorst wrote: > Op 05-09-2019 om 12:37 schreef Stanislav Lisovskiy: > > This counter will be used by drm_helper_probe_detect caller to > > determine > > if something else had changed except connection status, > > like for example edid. Hardware specific drivers are responsible > > for updating this counter when some change is detected to notify > > the drm part, which can trigger for example hotplug event. > > > > Currently there is no way to propagate that to a calling layer, > > as we send only connection_status update, however as we see with > > edid the changes can be broader. > > > > v2: Added documentation for the new counter. Rename change_counter > > to > > epoch_counter. > > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540 > > Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> > > --- > > drivers/gpu/drm/drm_connector.c | 1 + > > drivers/gpu/drm/drm_probe_helper.c | 29 > > +++++++++++++++++++++++++++-- > > include/drm/drm_connector.h | 3 +++ > > 3 files changed, 31 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_connector.c > > b/drivers/gpu/drm/drm_connector.c > > index 4c766624b20d..18b1ad2a4eee 100644 > > --- a/drivers/gpu/drm/drm_connector.c > > +++ b/drivers/gpu/drm/drm_connector.c > > @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev, > > INIT_LIST_HEAD(&connector->modes); > > mutex_init(&connector->mutex); > > connector->edid_blob_ptr = NULL; > > + connector->epoch_counter = 0; > > connector->tile_blob_ptr = NULL; > > connector->status = connector_status_unknown; > > connector->display_info.panel_orientation = > > diff --git a/drivers/gpu/drm/drm_probe_helper.c > > b/drivers/gpu/drm/drm_probe_helper.c > > index 351cbc40f0f8..5131ae56e676 100644 > > --- a/drivers/gpu/drm/drm_probe_helper.c > > +++ b/drivers/gpu/drm/drm_probe_helper.c > > @@ -777,6 +777,7 @@ bool drm_helper_hpd_irq_event(struct drm_device > > *dev) > > struct drm_connector_list_iter conn_iter; > > enum drm_connector_status old_status; > > bool changed = false; > > + uint64_t old_epoch_counter; > > > > if (!dev->mode_config.poll_enabled) > > return false; > > @@ -790,20 +791,44 @@ bool drm_helper_hpd_irq_event(struct > > drm_device *dev) > > > > old_status = connector->status; > > > > + old_epoch_counter = connector->epoch_counter; > > + > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old change counter > > %llu\n", connector->base.id, > > + connector->name, > > + old_epoch_counter); > > + > > connector->status = drm_helper_probe_detect(connector, > > NULL, false); > > Pass a u32 *changed as argument to drm_helper_probe_detect? would > require signature change, > > DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s > > to %s\n", > > connector->base.id, > > connector->name, > > drm_get_connector_status_name(old_status) > > , > > drm_get_connector_status_name(connector- > > >status)); > > - if (old_status != connector->status) > > + > > + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] New change counter > > %llu\n", > > + connector->base.id, > > + connector->name, > > + connector->epoch_counter); > > + > > + if (old_status != connector->status) { > > changed = true; > > + } > > + > > + /* Check changing of edid when a connector status still > > remains > > + * as "connector_status_connected". > > + */ > > + if (connector->status == connector_status_connected && > > + old_status == connector_status_connected && > > + old_epoch_counter != connector->epoch_counter) { > > + changed = true; > > + } > > Could we bump the epoch counter for any event being sent out? Would > make more sense. I was thinking about this. That would be quite sane approach I think. But then we might have to change also many other places where it checks connection status only. I will take a look as it anyways looks better than checking both epoch and connection status at the same time. > > } > > drm_connector_list_iter_end(&conn_iter); > > mutex_unlock(&dev->mode_config.mutex); > > > > - if (changed) > > + if (changed) { > > drm_kms_helper_hotplug_event(dev); > > + DRM_DEBUG_KMS("Sent hotplug event\n"); > > + } > > > > return changed; > > } > > diff --git a/include/drm/drm_connector.h > > b/include/drm/drm_connector.h > > index 681cb590f952..a3cc7d0927d6 100644 > > --- a/include/drm/drm_connector.h > > +++ b/include/drm/drm_connector.h > > @@ -1288,6 +1288,9 @@ struct drm_connector { > > /** @override_edid: has the EDID been overwritten through > > debugfs for testing? */ > > bool override_edid; > > > > + /** @epoch_counter: used to detect any other changes in > > connector, besides status */ > > + uint64_t epoch_counter; > > + > > #define DRM_CONNECTOR_MAX_ENCODER 3 > > /** > > * @encoder_ids: Valid encoders for this connector. Please only > > use > >
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 4c766624b20d..18b1ad2a4eee 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -246,6 +246,7 @@ int drm_connector_init(struct drm_device *dev, INIT_LIST_HEAD(&connector->modes); mutex_init(&connector->mutex); connector->edid_blob_ptr = NULL; + connector->epoch_counter = 0; connector->tile_blob_ptr = NULL; connector->status = connector_status_unknown; connector->display_info.panel_orientation = diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index 351cbc40f0f8..5131ae56e676 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -777,6 +777,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) struct drm_connector_list_iter conn_iter; enum drm_connector_status old_status; bool changed = false; + uint64_t old_epoch_counter; if (!dev->mode_config.poll_enabled) return false; @@ -790,20 +791,44 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev) old_status = connector->status; + old_epoch_counter = connector->epoch_counter; + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Old change counter %llu\n", connector->base.id, + connector->name, + old_epoch_counter); + connector->status = drm_helper_probe_detect(connector, NULL, false); DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", connector->base.id, connector->name, drm_get_connector_status_name(old_status), drm_get_connector_status_name(connector->status)); - if (old_status != connector->status) + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] New change counter %llu\n", + connector->base.id, + connector->name, + connector->epoch_counter); + + if (old_status != connector->status) { changed = true; + } + + /* Check changing of edid when a connector status still remains + * as "connector_status_connected". + */ + if (connector->status == connector_status_connected && + old_status == connector_status_connected && + old_epoch_counter != connector->epoch_counter) { + changed = true; + } } drm_connector_list_iter_end(&conn_iter); mutex_unlock(&dev->mode_config.mutex); - if (changed) + if (changed) { drm_kms_helper_hotplug_event(dev); + DRM_DEBUG_KMS("Sent hotplug event\n"); + } return changed; } diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 681cb590f952..a3cc7d0927d6 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1288,6 +1288,9 @@ struct drm_connector { /** @override_edid: has the EDID been overwritten through debugfs for testing? */ bool override_edid; + /** @epoch_counter: used to detect any other changes in connector, besides status */ + uint64_t epoch_counter; + #define DRM_CONNECTOR_MAX_ENCODER 3 /** * @encoder_ids: Valid encoders for this connector. Please only use
This counter will be used by drm_helper_probe_detect caller to determine if something else had changed except connection status, like for example edid. Hardware specific drivers are responsible for updating this counter when some change is detected to notify the drm part, which can trigger for example hotplug event. Currently there is no way to propagate that to a calling layer, as we send only connection_status update, however as we see with edid the changes can be broader. v2: Added documentation for the new counter. Rename change_counter to epoch_counter. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540 Signed-off-by: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> --- drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++++++++++++++-- include/drm/drm_connector.h | 3 +++ 3 files changed, 31 insertions(+), 2 deletions(-)