This series introduce to drm a way to determine if something else except connection_status had changed during probing, which can be used by other drivers as well. Another i915 specific part uses this approach to determine if edid had changed without changing the connection status and send a hotplug event.
Stanislav Lisovskiy (3): drm: Add helper to compare edids. drm: Introduce change counter to drm_connector drm/i915: Send hotplug event if edid had changed.
drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/drm_edid.c | 33 ++++++++++++++++++++ drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++-- drivers/gpu/drm/i915/display/intel_dp.c | 16 +++++++++- drivers/gpu/drm/i915/display/intel_hdmi.c | 16 ++++++++-- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 ++++++++++--- include/drm/drm_connector.h | 3 ++ include/drm/drm_edid.h | 9 ++++++ 8 files changed, 117 insertions(+), 11 deletions(-)
Many drivers would benefit from using drm helper to compare edid, rather than bothering with own implementation.
v2: Added documentation for this function.
Signed-off-by: Stanislav Lisovskiy stanislav.lisovskiy@intel.com --- drivers/gpu/drm/drm_edid.c | 33 +++++++++++++++++++++++++++++++++ include/drm/drm_edid.h | 9 +++++++++ 2 files changed, 42 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 82a4ceed3fcf..6cd086ea6253 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1362,6 +1362,39 @@ static bool drm_edid_is_zero(const u8 *in_edid, int length) return true; }
+/** + * drm_edid_are_equal - compare two edid blobs. + * @edid1: pointer to first blob + * @edid2: pointer to second blob + * This helper can be used during probing to determine if + * edid had changed. + */ +bool drm_edid_are_equal(struct edid *edid1, struct edid *edid2) +{ + int edid1_len, edid2_len; + bool edid1_present = edid1 != NULL; + bool edid2_present = edid2 != NULL; + + if (edid1_present != edid2_present) + return false; + + if (edid1) { + + edid1_len = EDID_LENGTH * (1 + edid1->extensions); + edid2_len = EDID_LENGTH * (1 + edid2->extensions); + + if (edid1_len != edid2_len) + return false; + + if (memcmp(edid1, edid2, edid1_len)) + return false; + } + + return true; +} +EXPORT_SYMBOL(drm_edid_are_equal); + + /** * drm_edid_block_valid - Sanity check the EDID block (base or extension) * @raw_edid: pointer to raw EDID block diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index b9719418c3d2..716964f63312 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -354,6 +354,15 @@ drm_load_edid_firmware(struct drm_connector *connector) } #endif
+/** + * drm_edid_are_equal - compare two edid blobs. + * @edid1: pointer to first blob + * @edid2: pointer to second blob + * This helper can be used during probing to determine if + * edid had changed. + */ +bool drm_edid_are_equal(struct edid *edid1, struct edid *edid2); + int drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, struct drm_connector *connector,
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 d49e19f3de3a..9c5e63b31527 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 ef2c468205a2..5857053174da 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -776,6 +776,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; @@ -789,20 +790,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 fc5d08438333..58b24d44de9c 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1282,6 +1282,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
Added edid checking to dp and hdmi edid setting functions, which are called from detect hooks. The result currently is propagated to calling layer using drm_connector->change_counter(proposed by Daniel Vetter). drm_helper_hpd_irq_event and intel_encoder_hotplug are currently both responsible for checking if this counter or connection status is changed.
There are conflicting parts in drm and i915 which attempt to do the same job - drm_helper_hpd_irq_event attempts to check connector status changes and then call hotplug, just as i915_hotplug_work_func, which calls encoder->hotplug hook which in turn calls generic intel_encoder_hotplug function which attempts to probe if output has changed. Looks like both needs to be changed, so added edid checking also to intel_encoder_hotplug function which is called both for hdmi and dp.
v2: Renamed change counter to epoch counter. Fixed type name.
v3: Fixed rebase conflict
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105540 Signed-off-by: Stanislav Lisovskiy stanislav.lisovskiy@intel.com --- drivers/gpu/drm/i915/display/intel_dp.c | 16 ++++++++++++++- drivers/gpu/drm/i915/display/intel_hdmi.c | 16 ++++++++++++--- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 +++++++++++++++----- 3 files changed, 44 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 0eb5d66f87a7..e4eb7e97a556 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -5316,10 +5316,24 @@ static void intel_dp_set_edid(struct intel_dp *intel_dp) { struct intel_connector *intel_connector = intel_dp->attached_connector; + struct drm_connector *connector = &intel_connector->base; struct edid *edid; + struct edid *old_edid;
- intel_dp_unset_edid(intel_dp); edid = intel_dp_get_edid(intel_dp); + old_edid = intel_connector->detect_edid; + + if (!drm_edid_are_equal(edid, old_edid)) { + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed! Updating blob property.\n", + connector->base.id, connector->name); + + connector->epoch_counter += 1; + DRM_DEBUG_KMS("Updating change counter to %llu\n", connector->epoch_counter); + + intel_connector_update_modes(&intel_connector->base, edid); + } + + intel_dp_unset_edid(intel_dp); intel_connector->detect_edid = edid;
intel_dp->has_audio = drm_detect_monitor_audio(edid); diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 9bf28de10401..72b3f2135228 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2503,7 +2503,7 @@ intel_hdmi_set_edid(struct drm_connector *connector) struct drm_i915_private *dev_priv = to_i915(connector->dev); struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector); intel_wakeref_t wakeref; - struct edid *edid; + struct edid *edid, *old_edid; bool connected = false; struct i2c_adapter *i2c;
@@ -2524,11 +2524,22 @@ intel_hdmi_set_edid(struct drm_connector *connector)
intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS, wakeref);
+ old_edid = to_intel_connector(connector)->detect_edid; + + if (!drm_edid_are_equal(edid, old_edid)) { + intel_connector_update_modes(connector, edid); + DRM_DEBUG_KMS("Updating change counter to %llu\n", connector->epoch_counter); + connector->epoch_counter += 1; + + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] Edid was changed! Updating blob property.\n", + connector->base.id, connector->name); + } + intel_hdmi_unset_edid(connector); to_intel_connector(connector)->detect_edid = edid; + if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) { intel_hdmi->has_audio = drm_detect_monitor_audio(edid); intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid); - connected = true; }
@@ -2555,7 +2566,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force) !intel_digital_port_connected(encoder)) goto out;
- intel_hdmi_unset_edid(connector);
if (intel_hdmi_set_edid(connector)) status = connector_status_connected; diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c b/drivers/gpu/drm/i915/display/intel_hotplug.c index c844ae4480af..dda07c9ab2cb 100644 --- a/drivers/gpu/drm/i915/display/intel_hotplug.c +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c @@ -280,23 +280,34 @@ intel_encoder_hotplug(struct intel_encoder *encoder, { struct drm_device *dev = connector->base.dev; enum drm_connector_status old_status; + u64 old_epoch_counter; + bool ret = false;
WARN_ON(!mutex_is_locked(&dev->mode_config.mutex)); old_status = connector->base.status;
+ old_epoch_counter = connector->base.epoch_counter; + connector->base.status = drm_helper_probe_detect(&connector->base, NULL, false);
- if (old_status == connector->base.status) - return INTEL_HOTPLUG_UNCHANGED; + if (old_status != connector->base.status) + ret = true; + + if (old_epoch_counter != connector->base.epoch_counter) + ret = true;
- DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n", + if (ret) { + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s(change counter %llu)\n", connector->base.base.id, connector->base.name, drm_get_connector_status_name(old_status), - drm_get_connector_status_name(connector->base.status)); + drm_get_connector_status_name(connector->base.status), + connector->base.epoch_counter); + return INTEL_HOTPLUG_CHANGED; + }
- return INTEL_HOTPLUG_CHANGED; + return INTEL_HOTPLUG_UNCHANGED; }
static bool intel_encoder_has_hpd_pulse(struct intel_encoder *encoder)
On Tue, Aug 06, 2019 at 03:55:48PM +0300, Stanislav Lisovskiy wrote:
This series introduce to drm a way to determine if something else except connection_status had changed during probing, which can be used by other drivers as well. Another i915 specific part uses this approach to determine if edid had changed without changing the connection status and send a hotplug event.
Did you read through the entire huge thread on all this (with I think Paul, Pekka, Ram and some others)? I feel like this is mostly taking that idea, but not taking a lot of the details we've discussed there. Specifically I'm not sure how userspace should handle this without also exposing the epoch counter, or at least generating a hotplug event for the specific connector. All that and more we discussed there.
And then there's the follow-up question: What's the userspace for this? Existing expectations are a minefield here, so even if you don't plan to change userspace we need to know what this is aimed for, and see above I don't think this is possible to use without userspace changes ... -Daniel
Stanislav Lisovskiy (3): drm: Add helper to compare edids. drm: Introduce change counter to drm_connector drm/i915: Send hotplug event if edid had changed.
drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/drm_edid.c | 33 ++++++++++++++++++++ drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++-- drivers/gpu/drm/i915/display/intel_dp.c | 16 +++++++++- drivers/gpu/drm/i915/display/intel_hdmi.c | 16 ++++++++-- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 ++++++++++--- include/drm/drm_connector.h | 3 ++ include/drm/drm_edid.h | 9 ++++++ 8 files changed, 117 insertions(+), 11 deletions(-)
-- 2.17.1
On Tue, 2019-08-06 at 15:51 +0200, Daniel Vetter wrote:
On Tue, Aug 06, 2019 at 03:55:48PM +0300, Stanislav Lisovskiy wrote:
This series introduce to drm a way to determine if something else except connection_status had changed during probing, which can be used by other drivers as well. Another i915 specific part uses this approach to determine if edid had changed without changing the connection status and send a hotplug event.
Did you read through the entire huge thread on all this (with I think Paul, Pekka, Ram and some others)? I feel like this is mostly taking that idea, but not taking a lot of the details we've discussed there. Specifically I'm not sure how userspace should handle this without also exposing the epoch counter, or at least generating a hotplug event for the specific connector. All that and more we discussed there.
And then there's the follow-up question: What's the userspace for this? Existing expectations are a minefield here, so even if you don't plan to change userspace we need to know what this is aimed for, and see above I don't think this is possible to use without userspace changes ...
Yes, sure I agree about userspace. However I guess we must start from something at least. I think I have seen some discussion regarding exposing this epoch counter as a property. Was even implementing this at some point but didn't dare to send to mailing list :)
My idea was just to expose this epoch counter as a drm property, to let userspace then to figure out, what has happened, as imho adding different events for this and that is a bit of an overkill and brings additional complexity..
However, please let me know, what do you think we should do for userspace.
-Stanislav
-Daniel
Stanislav Lisovskiy (3): drm: Add helper to compare edids. drm: Introduce change counter to drm_connector drm/i915: Send hotplug event if edid had changed.
drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/drm_edid.c | 33 ++++++++++++++++++++ drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++-
drivers/gpu/drm/i915/display/intel_dp.c | 16 +++++++++- drivers/gpu/drm/i915/display/intel_hdmi.c | 16 ++++++++-- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 ++++++++++--- include/drm/drm_connector.h | 3 ++ include/drm/drm_edid.h | 9 ++++++ 8 files changed, 117 insertions(+), 11 deletions(-)
-- 2.17.1
On Tue, Aug 6, 2019 at 4:06 PM Lisovskiy, Stanislav stanislav.lisovskiy@intel.com wrote:
On Tue, 2019-08-06 at 15:51 +0200, Daniel Vetter wrote:
On Tue, Aug 06, 2019 at 03:55:48PM +0300, Stanislav Lisovskiy wrote:
This series introduce to drm a way to determine if something else except connection_status had changed during probing, which can be used by other drivers as well. Another i915 specific part uses this approach to determine if edid had changed without changing the connection status and send a hotplug event.
Did you read through the entire huge thread on all this (with I think Paul, Pekka, Ram and some others)? I feel like this is mostly taking that idea, but not taking a lot of the details we've discussed there. Specifically I'm not sure how userspace should handle this without also exposing the epoch counter, or at least generating a hotplug event for the specific connector. All that and more we discussed there.
And then there's the follow-up question: What's the userspace for this? Existing expectations are a minefield here, so even if you don't plan to change userspace we need to know what this is aimed for, and see above I don't think this is possible to use without userspace changes ...
Yes, sure I agree about userspace. However I guess we must start from something at least. I think I have seen some discussion regarding exposing this epoch counter as a property. Was even implementing this at some point but didn't dare to send to mailing list :)
My idea was just to expose this epoch counter as a drm property, to let userspace then to figure out, what has happened, as imho adding different events for this and that is a bit of an overkill and brings additional complexity..
Adding Ram and link to the (warning, huge!) thread:
https://patchwork.freedesktop.org/patch/303905/?series=57232&rev=9
However, please let me know, what do you think we should do for userspace.
That seems backwards, since this is an uapi change I'd expect you're solving some specific issue in some specific userspace? If we're doing this just because I'm not really following.
Cheers, Daniel
-Stanislav
-Daniel
Stanislav Lisovskiy (3): drm: Add helper to compare edids. drm: Introduce change counter to drm_connector drm/i915: Send hotplug event if edid had changed.
drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/drm_edid.c | 33 ++++++++++++++++++++ drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++-
drivers/gpu/drm/i915/display/intel_dp.c | 16 +++++++++- drivers/gpu/drm/i915/display/intel_hdmi.c | 16 ++++++++-- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 ++++++++++--- include/drm/drm_connector.h | 3 ++ include/drm/drm_edid.h | 9 ++++++ 8 files changed, 117 insertions(+), 11 deletions(-)
-- 2.17.1
On Tue, 2019-08-06 at 19:43 +0200, Daniel Vetter wrote:
On Tue, Aug 6, 2019 at 4:06 PM Lisovskiy, Stanislav stanislav.lisovskiy@intel.com wrote:
On Tue, 2019-08-06 at 15:51 +0200, Daniel Vetter wrote:
On Tue, Aug 06, 2019 at 03:55:48PM +0300, Stanislav Lisovskiy wrote:
This series introduce to drm a way to determine if something else except connection_status had changed during probing, which can be used by other drivers as well. Another i915 specific part uses this approach to determine if edid had changed without changing the connection status and send a hotplug event.
Did you read through the entire huge thread on all this (with I think Paul, Pekka, Ram and some others)? I feel like this is mostly taking that idea, but not taking a lot of the details we've discussed there. Specifically I'm not sure how userspace should handle this without also exposing the epoch counter, or at least generating a hotplug event for the specific connector. All that and more we discussed there.
And then there's the follow-up question: What's the userspace for this? Existing expectations are a minefield here, so even if you don't plan to change userspace we need to know what this is aimed for, and see above I don't think this is possible to use without userspace changes ...
Yes, sure I agree about userspace. However I guess we must start from something at least. I think I have seen some discussion regarding exposing this epoch counter as a property. Was even implementing this at some point but didn't dare to send to mailing list :)
My idea was just to expose this epoch counter as a drm property, to let userspace then to figure out, what has happened, as imho adding different events for this and that is a bit of an overkill and brings additional complexity..
Adding Ram and link to the (warning, huge!) thread:
https://patchwork.freedesktop.org/patch/303905/?series=57232&rev=9
However, please let me know, what do you think we should do for userspace.
That seems backwards, since this is an uapi change I'd expect you're solving some specific issue in some specific userspace? If we're doing this just because I'm not really following.
Specifically, I'm solving an issue of changed edid during suspend, like we for example have in kms_chamelium hotplug tests(some of which now fail because of that). So even if connection status hasn't changed, we still need to send hotplug event and userspace needs to be able to understand that something had changed and whether we need to do a full reprobe or not. Epoch counter property would be responsible for this. As I understand in general there might be other connector updates, except edid which we need propagate in a similar way.
-Stanislav
Cheers, Daniel
-Stanislav
-Daniel
Stanislav Lisovskiy (3): drm: Add helper to compare edids. drm: Introduce change counter to drm_connector drm/i915: Send hotplug event if edid had changed.
drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/drm_edid.c | 33 ++++++++++++++++++++ drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++-
drivers/gpu/drm/i915/display/intel_dp.c | 16 +++++++++- drivers/gpu/drm/i915/display/intel_hdmi.c | 16 ++++++++-- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 ++++++++++
include/drm/drm_connector.h | 3 ++ include/drm/drm_edid.h | 9 ++++++ 8 files changed, 117 insertions(+), 11 deletions(-)
-- 2.17.1
On Wed, Aug 07, 2019 at 07:43:18AM +0000, Lisovskiy, Stanislav wrote:
On Tue, 2019-08-06 at 19:43 +0200, Daniel Vetter wrote:
On Tue, Aug 6, 2019 at 4:06 PM Lisovskiy, Stanislav stanislav.lisovskiy@intel.com wrote:
On Tue, 2019-08-06 at 15:51 +0200, Daniel Vetter wrote:
On Tue, Aug 06, 2019 at 03:55:48PM +0300, Stanislav Lisovskiy wrote:
This series introduce to drm a way to determine if something else except connection_status had changed during probing, which can be used by other drivers as well. Another i915 specific part uses this approach to determine if edid had changed without changing the connection status and send a hotplug event.
Did you read through the entire huge thread on all this (with I think Paul, Pekka, Ram and some others)? I feel like this is mostly taking that idea, but not taking a lot of the details we've discussed there. Specifically I'm not sure how userspace should handle this without also exposing the epoch counter, or at least generating a hotplug event for the specific connector. All that and more we discussed there.
And then there's the follow-up question: What's the userspace for this? Existing expectations are a minefield here, so even if you don't plan to change userspace we need to know what this is aimed for, and see above I don't think this is possible to use without userspace changes ...
Yes, sure I agree about userspace. However I guess we must start from something at least. I think I have seen some discussion regarding exposing this epoch counter as a property. Was even implementing this at some point but didn't dare to send to mailing list :)
My idea was just to expose this epoch counter as a drm property, to let userspace then to figure out, what has happened, as imho adding different events for this and that is a bit of an overkill and brings additional complexity..
Adding Ram and link to the (warning, huge!) thread:
https://patchwork.freedesktop.org/patch/303905/?series=57232&rev=9
However, please let me know, what do you think we should do for userspace.
That seems backwards, since this is an uapi change I'd expect you're solving some specific issue in some specific userspace? If we're doing this just because I'm not really following.
Specifically, I'm solving an issue of changed edid during suspend, like we for example have in kms_chamelium hotplug tests(some of which now fail because of that). So even if connection status hasn't changed, we still need to send hotplug event and userspace needs to be able to understand that something had changed and whether we need to do a full reprobe or not. Epoch counter property would be responsible for this. As I understand in general there might be other connector updates, except edid which we need propagate in a similar way.
So igt isn't valid userspace (it's just good testcases). Can we repro the same on real userspace? Does this work with real userspace? We've had userspace which tries to be clever and filters out what looks like redundant hotplug events. And then gets it wrong in cases like this.
Also, we've had forever an unconditional uevent on resume, exactly because anything could have changed. Did we loose this one on the way somewhere? Or maybe I misremember ...
If all we care about is resume re-adding that uncondtional uevent on resume is going to be a lot easier than this here. -Daniel
-Stanislav
Cheers, Daniel
-Stanislav
-Daniel
Stanislav Lisovskiy (3): drm: Add helper to compare edids. drm: Introduce change counter to drm_connector drm/i915: Send hotplug event if edid had changed.
drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/drm_edid.c | 33 ++++++++++++++++++++ drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++-
drivers/gpu/drm/i915/display/intel_dp.c | 16 +++++++++- drivers/gpu/drm/i915/display/intel_hdmi.c | 16 ++++++++-- drivers/gpu/drm/i915/display/intel_hotplug.c | 21 ++++++++++
include/drm/drm_connector.h | 3 ++ include/drm/drm_edid.h | 9 ++++++ 8 files changed, 117 insertions(+), 11 deletions(-)
-- 2.17.1
On Wed, 2019-08-07 at 23:07 +0200, Daniel Vetter wrote:
On Wed, Aug 07, 2019 at 07:43:18AM +0000, Lisovskiy, Stanislav wrote:
On Tue, 2019-08-06 at 19:43 +0200, Daniel Vetter wrote:
On Tue, Aug 6, 2019 at 4:06 PM Lisovskiy, Stanislav stanislav.lisovskiy@intel.com wrote:
On Tue, 2019-08-06 at 15:51 +0200, Daniel Vetter wrote:
On Tue, Aug 06, 2019 at 03:55:48PM +0300, Stanislav Lisovskiy wrote:
This series introduce to drm a way to determine if something else except connection_status had changed during probing, which can be used by other drivers as well. Another i915 specific part uses this approach to determine if edid had changed without changing the connection status and send a hotplug event.
Did you read through the entire huge thread on all this (with I think Paul, Pekka, Ram and some others)? I feel like this is mostly taking that idea, but not taking a lot of the details we've discussed there. Specifically I'm not sure how userspace should handle this without also exposing the epoch counter, or at least generating a hotplug event for the specific connector. All that and more we discussed there.
And then there's the follow-up question: What's the userspace for this? Existing expectations are a minefield here, so even if you don't plan to change userspace we need to know what this is aimed for, and see above I don't think this is possible to use without userspace changes ...
Yes, sure I agree about userspace. However I guess we must start from something at least. I think I have seen some discussion regarding exposing this epoch counter as a property. Was even implementing this at some point but didn't dare to send to mailing list :)
My idea was just to expose this epoch counter as a drm property, to let userspace then to figure out, what has happened, as imho adding different events for this and that is a bit of an overkill and brings additional complexity..
Adding Ram and link to the (warning, huge!) thread:
https://patchwork.freedesktop.org/patch/303905/?series=57232&rev=9
However, please let me know, what do you think we should do for userspace.
That seems backwards, since this is an uapi change I'd expect you're solving some specific issue in some specific userspace? If we're doing this just because I'm not really following.
Specifically, I'm solving an issue of changed edid during suspend, like we for example have in kms_chamelium hotplug tests(some of which now fail because of that). So even if connection status hasn't changed, we still need to send hotplug event and userspace needs to be able to understand that something had changed and whether we need to do a full reprobe or not. Epoch counter property would be responsible for this. As I understand in general there might be other connector updates, except edid which we need propagate in a similar way.
So igt isn't valid userspace (it's just good testcases). Can we repro the same on real userspace? Does this work with real userspace? We've had userspace which tries to be clever and filters out what looks like redundant hotplug events. And then gets it wrong in cases like this.
Also, we've had forever an unconditional uevent on resume, exactly because anything could have changed. Did we loose this one on the way somewhere? Or maybe I misremember ...
If all we care about is resume re-adding that uncondtional uevent on resume is going to be a lot easier than this here. -Daniel
Sorry for long reply(was on vacation), that is a good question regarding reproducing this in real life scenario. My obvious guess was to suspend the machine and meanwhile change connected display to another one. However this situation seems to be already handled by kernel nicely(tried few times and we always get a hotplug event). So that edid change during suspend chamelium test case seems to be a bit different. I will talk to our guys who wrote this about what is the real life scenario for this, because I'm now curious as well.
- Stanislav
-Stanislav
Cheers, Daniel
-Stanislav
-Daniel
Stanislav Lisovskiy (3): drm: Add helper to compare edids. drm: Introduce change counter to drm_connector drm/i915: Send hotplug event if edid had changed.
drivers/gpu/drm/drm_connector.c | 1 + drivers/gpu/drm/drm_edid.c | 33 ++++++++++++++++++++ drivers/gpu/drm/drm_probe_helper.c | 29 +++++++++++++++-
drivers/gpu/drm/i915/display/intel_dp.c | 16 +++++++++- drivers/gpu/drm/i915/display/intel_hdmi.c | 16 ++++++++-- drivers/gpu/drm/i915/display/intel_hotplug.c | 21
++++++++++
include/drm/drm_connector.h | 3 ++ include/drm/drm_edid.h | 9 ++++++ 8 files changed, 117 insertions(+), 11 deletions(-)
-- 2.17.1
On 19/08/2019 10:23, Lisovskiy, Stanislav wrote:
On Wed, 2019-08-07 at 23:07 +0200, Daniel Vetter wrote:
On Wed, Aug 07, 2019 at 07:43:18AM +0000, Lisovskiy, Stanislav wrote:
On Tue, 2019-08-06 at 19:43 +0200, Daniel Vetter wrote:
On Tue, Aug 6, 2019 at 4:06 PM Lisovskiy, Stanislav stanislav.lisovskiy@intel.com wrote:
On Tue, 2019-08-06 at 15:51 +0200, Daniel Vetter wrote:
On Tue, Aug 06, 2019 at 03:55:48PM +0300, Stanislav Lisovskiy wrote: > This series introduce to drm a way to determine if > something > else > except connection_status had changed during probing, which > can be used by other drivers as well. Another i915 specific > part > uses this approach to determine if edid had changed without > changing the connection status and send a hotplug event.
Did you read through the entire huge thread on all this (with I think Paul, Pekka, Ram and some others)? I feel like this is mostly taking that idea, but not taking a lot of the details we've discussed there. Specifically I'm not sure how userspace should handle this without also exposing the epoch counter, or at least generating a hotplug event for the specific connector. All that and more we discussed there.
And then there's the follow-up question: What's the userspace for this? Existing expectations are a minefield here, so even if you don't plan to change userspace we need to know what this is aimed for, and see above I don't think this is possible to use without userspace changes ...
Yes, sure I agree about userspace. However I guess we must start from something at least. I think I have seen some discussion regarding exposing this epoch counter as a property. Was even implementing this at some point but didn't dare to send to mailing list :)
My idea was just to expose this epoch counter as a drm property, to let userspace then to figure out, what has happened, as imho adding different events for this and that is a bit of an overkill and brings additional complexity..
Adding Ram and link to the (warning, huge!) thread:
https://patchwork.freedesktop.org/patch/303905/?series=57232&rev=9
However, please let me know, what do you think we should do for userspace.
That seems backwards, since this is an uapi change I'd expect you're solving some specific issue in some specific userspace? If we're doing this just because I'm not really following.
Specifically, I'm solving an issue of changed edid during suspend, like we for example have in kms_chamelium hotplug tests(some of which now fail because of that). So even if connection status hasn't changed, we still need to send hotplug event and userspace needs to be able to understand that something had changed and whether we need to do a full reprobe or not. Epoch counter property would be responsible for this. As I understand in general there might be other connector updates, except edid which we need propagate in a similar way.
So igt isn't valid userspace (it's just good testcases). Can we repro the same on real userspace? Does this work with real userspace? We've had userspace which tries to be clever and filters out what looks like redundant hotplug events. And then gets it wrong in cases like this.
Also, we've had forever an unconditional uevent on resume, exactly because anything could have changed. Did we loose this one on the way somewhere? Or maybe I misremember ...
If all we care about is resume re-adding that uncondtional uevent on resume is going to be a lot easier than this here. -Daniel
Sorry for long reply(was on vacation), that is a good question regarding reproducing this in real life scenario. My obvious guess was to suspend the machine and meanwhile change connected display to another one. However this situation seems to be already handled by kernel nicely(tried few times and we always get a hotplug event). So that edid change during suspend chamelium test case seems to be a bit different. I will talk to our guys who wrote this about what is the real life scenario for this, because I'm now curious as well.
Thanks Daniel for the feedback.
I also now wonder why our IGT test (chamelium-based) does not pass if a uevent is sent on resume automatically and all the test is expecting is a uevent...
Martin
- Stanislav
-Stanislav
Cheers, Daniel
-Stanislav
-Daniel
> > Stanislav Lisovskiy (3): > drm: Add helper to compare edids. > drm: Introduce change counter to drm_connector > drm/i915: Send hotplug event if edid had changed. > > drivers/gpu/drm/drm_connector.c | 1 + > drivers/gpu/drm/drm_edid.c | 33 > ++++++++++++++++++++ > drivers/gpu/drm/drm_probe_helper.c | 29 > +++++++++++++++- > - > drivers/gpu/drm/i915/display/intel_dp.c | 16 > +++++++++- > drivers/gpu/drm/i915/display/intel_hdmi.c | 16 > ++++++++-- > drivers/gpu/drm/i915/display/intel_hotplug.c | 21 > ++++++++++ > --- > include/drm/drm_connector.h | 3 ++ > include/drm/drm_edid.h | 9 ++++++ > 8 files changed, 117 insertions(+), 11 deletions(-) > > -- > 2.17.1 >
On Mon, 2019-08-19 at 08:35 +0100, Peres, Martin wrote:
On 19/08/2019 10:23, Lisovskiy, Stanislav wrote:
On Wed, 2019-08-07 at 23:07 +0200, Daniel Vetter wrote:
So igt isn't valid userspace (it's just good testcases). Can we repro the same on real userspace? Does this work with real userspace? We've had userspace which tries to be clever and filters out what looks like redundant hotplug events. And then gets it wrong in cases like this.
Also, we've had forever an unconditional uevent on resume, exactly because anything could have changed. Did we loose this one on the way somewhere? Or maybe I misremember ...
If all we care about is resume re-adding that uncondtional uevent on resume is going to be a lot easier than this here. -Daniel
Sorry for long reply(was on vacation), that is a good question regarding reproducing this in real life scenario. My obvious guess was to suspend the machine and meanwhile change connected display to another one. However this situation seems to be already handled by kernel nicely(tried few times and we always get a hotplug event). So that edid change during suspend chamelium test case seems to be a bit different. I will talk to our guys who wrote this about what is the real life scenario for this, because I'm now curious as well.
Thanks Daniel for the feedback.
I also now wonder why our IGT test (chamelium-based) does not pass if a uevent is sent on resume automatically and all the test is expecting is a uevent...
Martin
In fact I was wrong - when it worked, it was using exactly those patches :). With clean drm-tip - it seems to work ocassionally and it doesn't update the actual display edid and other stuff, so even when displays are changed we still see the old info/edid from userspace.
We always get a hpd irq when suspend/resume however it doesn't always result in uevent being sent. So there is a real need in those patches.
- Stanislav
-Stanislav
Cheers, Daniel
-Stanislav
> -Daniel > > > > > Stanislav Lisovskiy (3): > > drm: Add helper to compare edids. > > drm: Introduce change counter to drm_connector > > drm/i915: Send hotplug event if edid had changed. > > > > drivers/gpu/drm/drm_connector.c | 1 + > > drivers/gpu/drm/drm_edid.c | 33 > > ++++++++++++++++++++ > > drivers/gpu/drm/drm_probe_helper.c | 29 > > +++++++++++++++- > > - > > drivers/gpu/drm/i915/display/intel_dp.c | 16 > > +++++++++- > > drivers/gpu/drm/i915/display/intel_hdmi.c | 16 > > ++++++++-- > > drivers/gpu/drm/i915/display/intel_hotplug.c | 21 > > ++++++++++ > > --- > > include/drm/drm_connector.h | 3 ++ > > include/drm/drm_edid.h | 9 > > ++++++ > > 8 files changed, 117 insertions(+), 11 deletions(-) > > > > -- > > 2.17.1 > > > >
On Mon, 2019-08-19 at 09:05 +0000, Lisovskiy, Stanislav wrote:
On Mon, 2019-08-19 at 08:35 +0100, Peres, Martin wrote:
On 19/08/2019 10:23, Lisovskiy, Stanislav wrote:
On Wed, 2019-08-07 at 23:07 +0200, Daniel Vetter wrote:
So igt isn't valid userspace (it's just good testcases). Can we repro the same on real userspace? Does this work with real userspace? We've had userspace which tries to be clever and filters out what looks like redundant hotplug events. And then gets it wrong in cases like this.
Also, we've had forever an unconditional uevent on resume, exactly because anything could have changed. Did we loose this one on the way somewhere? Or maybe I misremember ...
If all we care about is resume re-adding that uncondtional uevent on resume is going to be a lot easier than this here. -Daniel
Sorry for long reply(was on vacation), that is a good question regarding reproducing this in real life scenario. My obvious guess was to suspend the machine and meanwhile change connected display to another one. However this situation seems to be already handled by kernel nicely(tried few times and we always get a hotplug event). So that edid change during suspend chamelium test case seems to be a bit different. I will talk to our guys who wrote this about what is the real life scenario for this, because I'm now curious as well.
Thanks Daniel for the feedback.
I also now wonder why our IGT test (chamelium-based) does not pass if a uevent is sent on resume automatically and all the test is expecting is a uevent...
Martin
In fact I was wrong - when it worked, it was using exactly those patches :). With clean drm-tip - it seems to work ocassionally and it doesn't update the actual display edid and other stuff, so even when displays are changed we still see the old info/edid from userspace.
We always get a hpd irq when suspend/resume however it doesn't always result in uevent being sent. So there is a real need in those patches.
Just decided to "ping" this discussion again. The issue is already some years old and still nothing is fixed. I do agree that may be something needs to be fixed/changed here in those patches, but something must be agreed at least I guess, as discussions themself do not fix bugs. Currently those patches address a particular issue which occurs, if display is changed during suspend. On ocassional basis, userspace might not get a hotplug event at all, causing different kind of problems(like wrong mode set on display or diaply not working at all). Also some kms_chamelium hotplug tests fail because of that.
- Stanislav
-Stanislav
Cheers, Daniel
> > > -Stanislav > > > > -Daniel > > > > > > > > Stanislav Lisovskiy (3): > > > drm: Add helper to compare edids. > > > drm: Introduce change counter to drm_connector > > > drm/i915: Send hotplug event if edid had changed. > > > > > > drivers/gpu/drm/drm_connector.c | 1 + > > > drivers/gpu/drm/drm_edid.c | 33 > > > ++++++++++++++++++++ > > > drivers/gpu/drm/drm_probe_helper.c | 29 > > > +++++++++++++++- > > > - > > > drivers/gpu/drm/i915/display/intel_dp.c | 16 > > > +++++++++- > > > drivers/gpu/drm/i915/display/intel_hdmi.c | 16 > > > ++++++++-- > > > drivers/gpu/drm/i915/display/intel_hotplug.c | 21 > > > ++++++++++ > > > --- > > > include/drm/drm_connector.h | 3 ++ > > > include/drm/drm_edid.h | 9 > > > ++++++ > > > 8 files changed, 117 insertions(+), 11 deletions(-) > > > > > > -- > > > 2.17.1 > > > > > > >
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Sep 03, 2019 at 09:17:49AM +0000, Lisovskiy, Stanislav wrote:
On Mon, 2019-08-19 at 09:05 +0000, Lisovskiy, Stanislav wrote:
On Mon, 2019-08-19 at 08:35 +0100, Peres, Martin wrote:
On 19/08/2019 10:23, Lisovskiy, Stanislav wrote:
On Wed, 2019-08-07 at 23:07 +0200, Daniel Vetter wrote:
So igt isn't valid userspace (it's just good testcases). Can we repro the same on real userspace? Does this work with real userspace? We've had userspace which tries to be clever and filters out what looks like redundant hotplug events. And then gets it wrong in cases like this.
Also, we've had forever an unconditional uevent on resume, exactly because anything could have changed. Did we loose this one on the way somewhere? Or maybe I misremember ...
If all we care about is resume re-adding that uncondtional uevent on resume is going to be a lot easier than this here. -Daniel
Sorry for long reply(was on vacation), that is a good question regarding reproducing this in real life scenario. My obvious guess was to suspend the machine and meanwhile change connected display to another one. However this situation seems to be already handled by kernel nicely(tried few times and we always get a hotplug event). So that edid change during suspend chamelium test case seems to be a bit different. I will talk to our guys who wrote this about what is the real life scenario for this, because I'm now curious as well.
Thanks Daniel for the feedback.
I also now wonder why our IGT test (chamelium-based) does not pass if a uevent is sent on resume automatically and all the test is expecting is a uevent...
Martin
In fact I was wrong - when it worked, it was using exactly those patches :). With clean drm-tip - it seems to work ocassionally and it doesn't update the actual display edid and other stuff, so even when displays are changed we still see the old info/edid from userspace.
We always get a hpd irq when suspend/resume however it doesn't always result in uevent being sent. So there is a real need in those patches.
Just decided to "ping" this discussion again. The issue is already some years old and still nothing is fixed. I do agree that may be something needs to be fixed/changed here in those patches, but something must be agreed at least I guess, as discussions themself do not fix bugs. Currently those patches address a particular issue which occurs, if display is changed during suspend. On ocassional basis, userspace might not get a hotplug event at all, causing different kind of problems(like wrong mode set on display or diaply not working at all). Also some kms_chamelium hotplug tests fail because of that.
I still think we'll long-term regret this if we just duct-tape more stuff on top, instead of giving userspace a more informative uevent. This will send more uevents to userspace, so maybe then userspace tries to filter more and be clever, which never works, and we're back to tears.
Anyway, on the approach itself: It's extremely i915 specific, and it requires that all drivers roll out drm_edid_equal checks and not forget to increment the epoch counter.
What I had in mind is that when we set the edid for a connector with drm_connector_update_edid_property() or whatever, then the epoch counter would auto-increment if anything has changed. Similarly (long-term idea at least) if anything important with DP registers has changed.
Can't we do that, instead of this sub-optimal solution of requiring all drivers to roll out lots of code? -Daniel
- Stanislav
-Stanislav
> > Cheers, Daniel > > > > > > > -Stanislav > > > > > > > -Daniel > > > > > > > > > > > Stanislav Lisovskiy (3): > > > > drm: Add helper to compare edids. > > > > drm: Introduce change counter to drm_connector > > > > drm/i915: Send hotplug event if edid had changed. > > > > > > > > drivers/gpu/drm/drm_connector.c | 1 + > > > > drivers/gpu/drm/drm_edid.c | 33 > > > > ++++++++++++++++++++ > > > > drivers/gpu/drm/drm_probe_helper.c | 29 > > > > +++++++++++++++- > > > > - > > > > drivers/gpu/drm/i915/display/intel_dp.c | 16 > > > > +++++++++- > > > > drivers/gpu/drm/i915/display/intel_hdmi.c | 16 > > > > ++++++++-- > > > > drivers/gpu/drm/i915/display/intel_hotplug.c | 21 > > > > ++++++++++ > > > > --- > > > > include/drm/drm_connector.h | 3 ++ > > > > include/drm/drm_edid.h | 9 > > > > ++++++ > > > > 8 files changed, 117 insertions(+), 11 deletions(-) > > > > > > > > -- > > > > 2.17.1 > > > > > > > > > > > > >
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 2019-09-03 at 11:40 +0200, Daniel Vetter wrote:
In fact I was wrong - when it worked, it was using exactly those patches :). With clean drm-tip - it seems to work ocassionally and it doesn't update the actual display edid and other stuff, so even when displays are changed we still see the old info/edid from userspace.
We always get a hpd irq when suspend/resume however it doesn't always result in uevent being sent. So there is a real need in those patches.
Just decided to "ping" this discussion again. The issue is already some years old and still nothing is fixed. I do agree that may be something needs to be fixed/changed here in those patches, but something must be agreed at least I guess, as discussions themself do not fix bugs. Currently those patches address a particular issue which occurs, if display is changed during suspend. On ocassional basis, userspace might not get a hotplug event at all, causing different kind of problems(like wrong mode set on display or diaply not working at all). Also some kms_chamelium hotplug tests fail because of that.
I still think we'll long-term regret this if we just duct-tape more stuff on top, instead of giving userspace a more informative uevent. This will send more uevents to userspace, so maybe then userspace tries to filter more and be clever, which never works, and we're back to tears.
But here we actually do need a uevent as currently we don't get any at all, if edid changes during suspend. If userspace will try to filter this out - it's just stupid, however we still need to do things correctly.
Anyway, on the approach itself: It's extremely i915 specific, and it requires that all drivers roll out drm_edid_equal checks and not forget to increment the epoch counter.
What I had in mind is that when we set the edid for a connector with drm_connector_update_edid_property() or whatever, then the epoch counter would auto-increment if anything has changed. Similarly (long-term idea at least) if anything important with DP registers has changed.
Can't we do that, instead of this sub-optimal solution of requiring all drivers to roll out lots of code?
1) We update edid in intel_dp_set_edid, which is called from intel_dp_detect(drm_connector_helper_funcs->detect_ctx hook) which is called from drm_helper_probe_detect. That one is called either from specific intel_encoder->hotplug hook in i915_hotplug_work_func or by userspace request during reprobe.
2) Previously we were simply updating edid in intel_dp_set_edid without caring if it is the same or not and hotplug event was sent only once connection_status had changed.
3) drm_connector_update_edid_property is called from connector-
get_modes hook(lets say intel_dp_get_modes fo dp) however it simply
uses results of drm_helper_probe_detect so without actual comparison it would not be able to detect if we really need to update epoch_counter or not.
Because as I said currently intel_dp_set_edid simply assigns it without checking, so that way you will get epoch_counter updated every time, i.e exactly what you wanted to avoid here.
So we really need someway to determine if edid had changed, instead of simply assigning it all the time - that is why I had to make this function.
Cheers,
Stanislav
-Daniel
- Stanislav
> > -Stanislav > > > > > Cheers, Daniel > > > > > > > > > > > -Stanislav > > > > > > > > > > -Daniel > > > > > > > > > > > > > > Stanislav Lisovskiy (3): > > > > > drm: Add helper to compare edids. > > > > > drm: Introduce change counter to drm_connector > > > > > drm/i915: Send hotplug event if edid had > > > > > changed. > > > > > > > > > > drivers/gpu/drm/drm_connector.c | > > > > > 1 + > > > > > drivers/gpu/drm/drm_edid.c | > > > > > 33 > > > > > ++++++++++++++++++++ > > > > > drivers/gpu/drm/drm_probe_helper.c | > > > > > 29 > > > > > +++++++++++++++- > > > > > - > > > > > drivers/gpu/drm/i915/display/intel_dp.c | > > > > > 16 > > > > > +++++++++- > > > > > drivers/gpu/drm/i915/display/intel_hdmi.c | > > > > > 16 > > > > > ++++++++-- > > > > > drivers/gpu/drm/i915/display/intel_hotplug.c | > > > > > 21 > > > > > ++++++++++ > > > > > --- > > > > > include/drm/drm_connector.h | > > > > > 3 ++ > > > > > include/drm/drm_edid.h | > > > > > 9 > > > > > ++++++ > > > > > 8 files changed, 117 insertions(+), 11 > > > > > deletions(-) > > > > > > > > > > -- > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > >
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 2019-09-03 at 11:49 +0000, Lisovskiy, Stanislav wrote:
On Tue, 2019-09-03 at 11:40 +0200, Daniel Vetter wrote:
In fact I was wrong - when it worked, it was using exactly those patches :). With clean drm-tip - it seems to work ocassionally and it doesn't update the actual display edid and other stuff, so even when displays are changed we still see the old info/edid from userspace.
We always get a hpd irq when suspend/resume however it doesn't always result in uevent being sent. So there is a real need in those patches.
Just decided to "ping" this discussion again. The issue is already some years old and still nothing is fixed. I do agree that may be something needs to be fixed/changed here in those patches, but something must be agreed at least I guess, as discussions themself do not fix bugs. Currently those patches address a particular issue which occurs, if display is changed during suspend. On ocassional basis, userspace might not get a hotplug event at all, causing different kind of problems(like wrong mode set on display or diaply not working at all). Also some kms_chamelium hotplug tests fail because of that.
I still think we'll long-term regret this if we just duct-tape more stuff on top, instead of giving userspace a more informative uevent. This will send more uevents to userspace, so maybe then userspace tries to filter more and be clever, which never works, and we're back to tears.
But here we actually do need a uevent as currently we don't get any at all, if edid changes during suspend. If userspace will try to filter this out - it's just stupid, however we still need to do things correctly.
Anyway, on the approach itself: It's extremely i915 specific, and it requires that all drivers roll out drm_edid_equal checks and not forget to increment the epoch counter.
What I had in mind is that when we set the edid for a connector with drm_connector_update_edid_property() or whatever, then the epoch counter would auto-increment if anything has changed. Similarly (long-term idea at least) if anything important with DP registers has changed.
Can't we do that, instead of this sub-optimal solution of requiring all drivers to roll out lots of code?
So once again, just to summarize things:
1) We update edid in intel_dp_set_edid, which is called from intel_dp_detect(drm_connector_helper_funcs->detect_ctx hook) which is called from drm_helper_probe_detect. That one is called either from specific intel_encoder->hotplug hook in i915_hotplug_work_func or by userspace request during reprobe.
2) Currently we are simply updating edid in intel_dp_set_edid without caring if it is the same or not and hotplug event is sent only once connection_status had changed.
3) drm_connector_update_edid_property is called from connector-
get_modes hook(lets say intel_dp_get_modes fo dp) however it simply
uses results of drm_helper_probe_detect so without actual comparison it would not be able to detect if we really need to update epoch_counter or not.
Because as I said currently intel_dp_set_edid simply assigns it without checking, so that way you will get epoch_counter updated every time, i.e exactly what you wanted to avoid here.
So we really need someway to determine if edid had changed, instead of simply assigning it all the time - that is why I had to implement drm_edid_equal function.
- Stanislav
-Daniel
- Stanislav
> > > > > > -Stanislav > > > > > > > > Cheers, Daniel > > > > > > > > > > > > > > > -Stanislav > > > > > > > > > > > > > -Daniel > > > > > > > > > > > > > > > > > Stanislav Lisovskiy (3): > > > > > > drm: Add helper to compare edids. > > > > > > drm: Introduce change counter to > > > > > > drm_connector > > > > > > drm/i915: Send hotplug event if edid had > > > > > > changed. > > > > > > > > > > > > drivers/gpu/drm/drm_connector.c | > > > > > > > > > > > > 1 + > > > > > > drivers/gpu/drm/drm_edid.c | > > > > > > 33 > > > > > > ++++++++++++++++++++ > > > > > > drivers/gpu/drm/drm_probe_helper.c | > > > > > > 29 > > > > > > +++++++++++++++- > > > > > > - > > > > > > drivers/gpu/drm/i915/display/intel_dp.c | > > > > > > 16 > > > > > > +++++++++- > > > > > > drivers/gpu/drm/i915/display/intel_hdmi.c | > > > > > > 16 > > > > > > ++++++++-- > > > > > > drivers/gpu/drm/i915/display/intel_hotplug.c | > > > > > > 21 > > > > > > ++++++++++ > > > > > > --- > > > > > > include/drm/drm_connector.h | > > > > > > > > > > > > 3 ++ > > > > > > include/drm/drm_edid.h | > > > > > > > > > > > > 9 > > > > > > ++++++ > > > > > > 8 files changed, 117 insertions(+), 11 > > > > > > deletions(-) > > > > > > > > > > > > -- > > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > > > > > > > > > >
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Sep 03, 2019 at 11:49:23AM +0000, Lisovskiy, Stanislav wrote:
On Tue, 2019-09-03 at 11:40 +0200, Daniel Vetter wrote:
In fact I was wrong - when it worked, it was using exactly those patches :). With clean drm-tip - it seems to work ocassionally and it doesn't update the actual display edid and other stuff, so even when displays are changed we still see the old info/edid from userspace.
We always get a hpd irq when suspend/resume however it doesn't always result in uevent being sent. So there is a real need in those patches.
Just decided to "ping" this discussion again. The issue is already some years old and still nothing is fixed. I do agree that may be something needs to be fixed/changed here in those patches, but something must be agreed at least I guess, as discussions themself do not fix bugs. Currently those patches address a particular issue which occurs, if display is changed during suspend. On ocassional basis, userspace might not get a hotplug event at all, causing different kind of problems(like wrong mode set on display or diaply not working at all). Also some kms_chamelium hotplug tests fail because of that.
I still think we'll long-term regret this if we just duct-tape more stuff on top, instead of giving userspace a more informative uevent. This will send more uevents to userspace, so maybe then userspace tries to filter more and be clever, which never works, and we're back to tears.
But here we actually do need a uevent as currently we don't get any at all, if edid changes during suspend. If userspace will try to filter this out - it's just stupid, however we still need to do things correctly.
Anyway, on the approach itself: It's extremely i915 specific, and it requires that all drivers roll out drm_edid_equal checks and not forget to increment the epoch counter.
What I had in mind is that when we set the edid for a connector with drm_connector_update_edid_property() or whatever, then the epoch counter would auto-increment if anything has changed. Similarly (long-term idea at least) if anything important with DP registers has changed.
Can't we do that, instead of this sub-optimal solution of requiring all drivers to roll out lots of code?
- We update edid in intel_dp_set_edid, which is called from
intel_dp_detect(drm_connector_helper_funcs->detect_ctx hook) which is called from drm_helper_probe_detect. That one is called either from specific intel_encoder->hotplug hook in i915_hotplug_work_func or by userspace request during reprobe.
- Previously we were simply updating edid in intel_dp_set_edid without
caring if it is the same or not and hotplug event was sent only once connection_status had changed.
- drm_connector_update_edid_property is called from connector-
get_modes hook(lets say intel_dp_get_modes fo dp) however it simply
uses results of drm_helper_probe_detect so without actual comparison it would not be able to detect if we really need to update epoch_counter or not.
Because as I said currently intel_dp_set_edid simply assigns it without checking, so that way you will get epoch_counter updated every time, i.e exactly what you wanted to avoid here.
So we really need someway to determine if edid had changed, instead of simply assigning it all the time - that is why I had to make this function.
update_edid is the thing which changes the userspace visible edid. We can check there with the previous userspace visible edid, and if it's different, increase the epoch counter. If we never call update_edid then userspace won't see the changed edid (it might see the changed mode list or whatever), so doing that is pretty much a requirement for correctness.
The higher levels should notice the epoch counter change (you might not have captured all of them, there's a bunch between ioctl, poll worker and sysfs iirc), and send out the uevent.
Why does that not work? -Daniel
Cheers,
Stanislav
-Daniel
- Stanislav
> > > > > > -Stanislav > > > > > > > > Cheers, Daniel > > > > > > > > > > > > > > > -Stanislav > > > > > > > > > > > > > -Daniel > > > > > > > > > > > > > > > > > Stanislav Lisovskiy (3): > > > > > > drm: Add helper to compare edids. > > > > > > drm: Introduce change counter to drm_connector > > > > > > drm/i915: Send hotplug event if edid had > > > > > > changed. > > > > > > > > > > > > drivers/gpu/drm/drm_connector.c | > > > > > > 1 + > > > > > > drivers/gpu/drm/drm_edid.c | > > > > > > 33 > > > > > > ++++++++++++++++++++ > > > > > > drivers/gpu/drm/drm_probe_helper.c | > > > > > > 29 > > > > > > +++++++++++++++- > > > > > > - > > > > > > drivers/gpu/drm/i915/display/intel_dp.c | > > > > > > 16 > > > > > > +++++++++- > > > > > > drivers/gpu/drm/i915/display/intel_hdmi.c | > > > > > > 16 > > > > > > ++++++++-- > > > > > > drivers/gpu/drm/i915/display/intel_hotplug.c | > > > > > > 21 > > > > > > ++++++++++ > > > > > > --- > > > > > > include/drm/drm_connector.h | > > > > > > 3 ++ > > > > > > include/drm/drm_edid.h | > > > > > > 9 > > > > > > ++++++ > > > > > > 8 files changed, 117 insertions(+), 11 > > > > > > deletions(-) > > > > > > > > > > > > -- > > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > > > > > > > > > >
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, 2019-09-03 at 17:49 +0200, Daniel Vetter wrote:
On Tue, Sep 03, 2019 at 11:49:23AM +0000, Lisovskiy, Stanislav wrote:
On Tue, 2019-09-03 at 11:40 +0200, Daniel Vetter wrote:
In fact I was wrong - when it worked, it was using exactly those patches :). With clean drm-tip - it seems to work ocassionally and it doesn't update the actual display edid and other stuff, so even when displays are changed we still see the old info/edid from userspace.
We always get a hpd irq when suspend/resume however it doesn't always result in uevent being sent. So there is a real need in those patches.
Just decided to "ping" this discussion again. The issue is already some years old and still nothing is fixed. I do agree that may be something needs to be fixed/changed here in those patches, but something must be agreed at least I guess, as discussions themself do not fix bugs. Currently those patches address a particular issue which occurs, if display is changed during suspend. On ocassional basis, userspace might not get a hotplug event at all, causing different kind of problems(like wrong mode set on display or diaply not working at all). Also some kms_chamelium hotplug tests fail because of that.
I still think we'll long-term regret this if we just duct-tape more stuff on top, instead of giving userspace a more informative uevent. This will send more uevents to userspace, so maybe then userspace tries to filter more and be clever, which never works, and we're back to tears.
But here we actually do need a uevent as currently we don't get any at all, if edid changes during suspend. If userspace will try to filter this out - it's just stupid, however we still need to do things correctly.
Anyway, on the approach itself: It's extremely i915 specific, and it requires that all drivers roll out drm_edid_equal checks and not forget to increment the epoch counter.
What I had in mind is that when we set the edid for a connector with drm_connector_update_edid_property() or whatever, then the epoch counter would auto-increment if anything has changed. Similarly (long- term idea at least) if anything important with DP registers has changed.
Can't we do that, instead of this sub-optimal solution of requiring all drivers to roll out lots of code?
- We update edid in intel_dp_set_edid, which is called from
intel_dp_detect(drm_connector_helper_funcs->detect_ctx hook) which is called from drm_helper_probe_detect. That one is called either from specific intel_encoder->hotplug hook in i915_hotplug_work_func or by userspace request during reprobe.
- Previously we were simply updating edid in intel_dp_set_edid
without caring if it is the same or not and hotplug event was sent only once connection_status had changed.
- drm_connector_update_edid_property is called from connector-
get_modes hook(lets say intel_dp_get_modes fo dp) however it simply
uses results of drm_helper_probe_detect so without actual comparison it would not be able to detect if we really need to update epoch_counter or not.
Because as I said currently intel_dp_set_edid simply assigns it without checking, so that way you will get epoch_counter updated every time, i.e exactly what you wanted to avoid here.
So we really need someway to determine if edid had changed, instead of simply assigning it all the time - that is why I had to make this function.
update_edid is the thing which changes the userspace visible edid. We can check there with the previous userspace visible edid, and if it's different, increase the epoch counter. If we never call update_edid then userspace won't see the changed edid (it might see the changed mode list or whatever), so doing that is pretty much a requirement for correctness.
The higher levels should notice the epoch counter change (you might not have captured all of them, there's a bunch between ioctl, poll worker and sysfs iirc), and send out the uevent.
Why does that not work?
Sure this will work, but still we need somehow to be able to determine this "if it's different" state. In your solution we just move that comparison to drm_connector_update_edid_property, which is quite fine for me.
I would say that yes, this idea may be is even better because drivers won't need to implement this comparison in encoder->hotplug in each driver. However: we still need a comparison in drm_connector_update_edid_property(drm_edid_equal) and also I'm not sure we can send a hotplug event based on that as drm_connector_update_edid_property seems to get called only during connector init or during reprobe from userspace from connector->get_modes hook. Also it is called from drm_kms_helper_hotplug_event from, but this one is called from i915 only if connection status had changed.
- Stanislav
-Daniel
Cheers,
Stanislav
-Daniel
> > - Stanislav > > > > > > > > > > > -Stanislav > > > > > > > > > > > Cheers, Daniel > > > > > > > > > > > > > > > > > > > -Stanislav > > > > > > > > > > > > > > > > -Daniel > > > > > > > > > > > > > > > > > > > > Stanislav Lisovskiy (3): > > > > > > > drm: Add helper to compare edids. > > > > > > > drm: Introduce change counter to > > > > > > > drm_connector > > > > > > > drm/i915: Send hotplug event if edid had > > > > > > > changed. > > > > > > > > > > > > > > drivers/gpu/drm/drm_connector.c > > > > > > > | > > > > > > > 1 + > > > > > > > drivers/gpu/drm/drm_edid.c > > > > > > > | > > > > > > > 33 > > > > > > > ++++++++++++++++++++ > > > > > > > drivers/gpu/drm/drm_probe_helper.c > > > > > > > | > > > > > > > 29 > > > > > > > +++++++++++++++- > > > > > > > - > > > > > > > drivers/gpu/drm/i915/display/intel_dp.c > > > > > > > | > > > > > > > 16 > > > > > > > +++++++++- > > > > > > > drivers/gpu/drm/i915/display/intel_hdmi.c > > > > > > > | > > > > > > > 16 > > > > > > > ++++++++-- > > > > > > > drivers/gpu/drm/i915/display/intel_hotplug.c > > > > > > > | > > > > > > > 21 > > > > > > > ++++++++++ > > > > > > > --- > > > > > > > include/drm/drm_connector.h > > > > > > > | > > > > > > > 3 ++ > > > > > > > include/drm/drm_edid.h > > > > > > > | > > > > > > > 9 > > > > > > > ++++++ > > > > > > > 8 files changed, 117 insertions(+), 11 > > > > > > > deletions(-) > > > > > > > > > > > > > > -- > > > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, Sep 04, 2019 at 08:36:46AM +0000, Lisovskiy, Stanislav wrote:
On Tue, 2019-09-03 at 17:49 +0200, Daniel Vetter wrote:
On Tue, Sep 03, 2019 at 11:49:23AM +0000, Lisovskiy, Stanislav wrote:
On Tue, 2019-09-03 at 11:40 +0200, Daniel Vetter wrote:
In fact I was wrong - when it worked, it was using exactly those patches :). With clean drm-tip - it seems to work ocassionally and it doesn't update the actual display edid and other stuff, so even when displays are changed we still see the old info/edid from userspace.
We always get a hpd irq when suspend/resume however it doesn't always result in uevent being sent. So there is a real need in those patches.
Just decided to "ping" this discussion again. The issue is already some years old and still nothing is fixed. I do agree that may be something needs to be fixed/changed here in those patches, but something must be agreed at least I guess, as discussions themself do not fix bugs. Currently those patches address a particular issue which occurs, if display is changed during suspend. On ocassional basis, userspace might not get a hotplug event at all, causing different kind of problems(like wrong mode set on display or diaply not working at all). Also some kms_chamelium hotplug tests fail because of that.
I still think we'll long-term regret this if we just duct-tape more stuff on top, instead of giving userspace a more informative uevent. This will send more uevents to userspace, so maybe then userspace tries to filter more and be clever, which never works, and we're back to tears.
But here we actually do need a uevent as currently we don't get any at all, if edid changes during suspend. If userspace will try to filter this out - it's just stupid, however we still need to do things correctly.
Anyway, on the approach itself: It's extremely i915 specific, and it requires that all drivers roll out drm_edid_equal checks and not forget to increment the epoch counter.
What I had in mind is that when we set the edid for a connector with drm_connector_update_edid_property() or whatever, then the epoch counter would auto-increment if anything has changed. Similarly (long- term idea at least) if anything important with DP registers has changed.
Can't we do that, instead of this sub-optimal solution of requiring all drivers to roll out lots of code?
- We update edid in intel_dp_set_edid, which is called from
intel_dp_detect(drm_connector_helper_funcs->detect_ctx hook) which is called from drm_helper_probe_detect. That one is called either from specific intel_encoder->hotplug hook in i915_hotplug_work_func or by userspace request during reprobe.
- Previously we were simply updating edid in intel_dp_set_edid
without caring if it is the same or not and hotplug event was sent only once connection_status had changed.
- drm_connector_update_edid_property is called from connector-
get_modes hook(lets say intel_dp_get_modes fo dp) however it simply
uses results of drm_helper_probe_detect so without actual comparison it would not be able to detect if we really need to update epoch_counter or not.
Because as I said currently intel_dp_set_edid simply assigns it without checking, so that way you will get epoch_counter updated every time, i.e exactly what you wanted to avoid here.
So we really need someway to determine if edid had changed, instead of simply assigning it all the time - that is why I had to make this function.
update_edid is the thing which changes the userspace visible edid. We can check there with the previous userspace visible edid, and if it's different, increase the epoch counter. If we never call update_edid then userspace won't see the changed edid (it might see the changed mode list or whatever), so doing that is pretty much a requirement for correctness.
The higher levels should notice the epoch counter change (you might not have captured all of them, there's a bunch between ioctl, poll worker and sysfs iirc), and send out the uevent.
Why does that not work?
Sure this will work, but still we need somehow to be able to determine this "if it's different" state. In your solution we just move that comparison to drm_connector_update_edid_property, which is quite fine for me.
Yes we need to compare edid somewhere, that much is clear. I'm not disputing that. I just want something where we don't have to roll this out over all the drivers, because that's a hopeless endeavour.
I would say that yes, this idea may be is even better because drivers won't need to implement this comparison in encoder->hotplug in each driver. However: we still need a comparison in drm_connector_update_edid_property(drm_edid_equal) and also I'm not sure we can send a hotplug event based on that as drm_connector_update_edid_property seems to get called only during connector init or during reprobe from userspace from connector->get_modes hook. Also it is called from drm_kms_helper_hotplug_event from, but this one is called from i915 only if connection status had changed.
So ditch the optimization to only call ->get_modes when called from userspace? We've been talking about this one too in the past ...
I'd really like a solution where it will work for most drivers out of the box. -Daniel
- Stanislav
-Daniel
Cheers,
Stanislav
-Daniel
> > > > > - Stanislav > > > > > > > > > > > > > > > > -Stanislav > > > > > > > > > > > > > > Cheers, Daniel > > > > > > > > > > > > > > > > > > > > > > > -Stanislav > > > > > > > > > > > > > > > > > > > -Daniel > > > > > > > > > > > > > > > > > > > > > > > Stanislav Lisovskiy (3): > > > > > > > > drm: Add helper to compare edids. > > > > > > > > drm: Introduce change counter to > > > > > > > > drm_connector > > > > > > > > drm/i915: Send hotplug event if edid had > > > > > > > > changed. > > > > > > > > > > > > > > > > drivers/gpu/drm/drm_connector.c > > > > > > > > | > > > > > > > > 1 + > > > > > > > > drivers/gpu/drm/drm_edid.c > > > > > > > > | > > > > > > > > 33 > > > > > > > > ++++++++++++++++++++ > > > > > > > > drivers/gpu/drm/drm_probe_helper.c > > > > > > > > | > > > > > > > > 29 > > > > > > > > +++++++++++++++- > > > > > > > > - > > > > > > > > drivers/gpu/drm/i915/display/intel_dp.c > > > > > > > > | > > > > > > > > 16 > > > > > > > > +++++++++- > > > > > > > > drivers/gpu/drm/i915/display/intel_hdmi.c > > > > > > > > | > > > > > > > > 16 > > > > > > > > ++++++++-- > > > > > > > > drivers/gpu/drm/i915/display/intel_hotplug.c > > > > > > > > | > > > > > > > > 21 > > > > > > > > ++++++++++ > > > > > > > > --- > > > > > > > > include/drm/drm_connector.h > > > > > > > > | > > > > > > > > 3 ++ > > > > > > > > include/drm/drm_edid.h > > > > > > > > | > > > > > > > > 9 > > > > > > > > ++++++ > > > > > > > > 8 files changed, 117 insertions(+), 11 > > > > > > > > deletions(-) > > > > > > > > > > > > > > > > -- > > > > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Wed, 2019-09-04 at 11:23 +0200, Daniel Vetter wrote:
Sure this will work, but still we need somehow to be able to determine this "if it's different" state. In your solution we just move that comparison to drm_connector_update_edid_property, which is quite fine for me.
Yes we need to compare edid somewhere, that much is clear. I'm not disputing that. I just want something where we don't have to roll this out over all the drivers, because that's a hopeless endeavour.
I would say that yes, this idea may be is even better because drivers won't need to implement this comparison in encoder->hotplug in each driver. However: we still need a comparison in drm_connector_update_edid_property(drm_edid_equal) and also I'm not sure we can send a hotplug event based on that as drm_connector_update_edid_property seems to get called only during connector init or during reprobe from userspace from connector->get_modes hook. Also it is called from drm_kms_helper_hotplug_event from, but this one is called from i915 only if connection status had changed.
So ditch the optimization to only call ->get_modes when called from userspace? We've been talking about this one too in the past ...
I'd really like a solution where it will work for most drivers out of the box.
So I guess the conclusion would be to try to use drm_connector_update_edid_property that way we will avoid duplicating drm_edid_equal code in all drivers. However this might require ensuring that drm_connector_update_edid_property is always called when we get a hotplug, so there we can check if edid had changed and send uevent, if needed.
- Stanislav
-Daniel
- Stanislav
-Daniel
Cheers,
Stanislav
-Daniel
> > > > > > > > - Stanislav > > > > > > > > > > > > > > > > > > > > > -Stanislav > > > > > > > > > > > > > > > > > Cheers, Daniel > > > > > > > > > > > > > > > > > > > > > > > > > > > -Stanislav > > > > > > > > > > > > > > > > > > > > > > -Daniel > > > > > > > > > > > > > > > > > > > > > > > > > > Stanislav Lisovskiy (3): > > > > > > > > > drm: Add helper to compare edids. > > > > > > > > > drm: Introduce change counter to > > > > > > > > > drm_connector > > > > > > > > > drm/i915: Send hotplug event if edid > > > > > > > > > had > > > > > > > > > changed. > > > > > > > > > > > > > > > > > > drivers/gpu/drm/drm_connector.c > > > > > > > > > > > > > > > > > > | > > > > > > > > > 1 + > > > > > > > > > drivers/gpu/drm/drm_edid.c > > > > > > > > > > > > > > > > > > | > > > > > > > > > 33 > > > > > > > > > ++++++++++++++++++++ > > > > > > > > > drivers/gpu/drm/drm_probe_helper.c > > > > > > > > > > > > > > > > > > | > > > > > > > > > 29 > > > > > > > > > +++++++++++++++- > > > > > > > > > - > > > > > > > > > drivers/gpu/drm/i915/display/intel_dp.c > > > > > > > > > > > > > > > > > > | > > > > > > > > > 16 > > > > > > > > > +++++++++- > > > > > > > > > drivers/gpu/drm/i915/display/intel_hdmi. > > > > > > > > > c > > > > > > > > > | > > > > > > > > > 16 > > > > > > > > > ++++++++-- > > > > > > > > > drivers/gpu/drm/i915/display/intel_hotpl > > > > > > > > > ug.c > > > > > > > > > > > > > > > > > > > > > > > > > > > > 21 > > > > > > > > > ++++++++++ > > > > > > > > > --- > > > > > > > > > include/drm/drm_connector.h > > > > > > > > > > > > > > > > > > | > > > > > > > > > 3 ++ > > > > > > > > > include/drm/drm_edid.h > > > > > > > > > > > > > > > > > > | > > > > > > > > > 9 > > > > > > > > > ++++++ > > > > > > > > > 8 files changed, 117 insertions(+), 11 > > > > > > > > > deletions(-) > > > > > > > > > > > > > > > > > > -- > > > > > > > > > 2.17.1 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Lisovskiy, Stanislav -
Peres, Martin -
Stanislav Lisovskiy