| Message ID | 1469717448-4297-2-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Jul 28, 2016 at 05:50:37PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Currently we try to check the lid status to see whether eDP is connected > or not. However we ignore the result anyway, and we have no lid notifier > to fire off uevents, so let's just ignore the lid stuff for eDP > entirely. You mean the OpRegion no longer contains lid_state? I don't see it being disabled... The lid notifier is from acpi/button.c. I haven't looked at why the result would be ignored, but you haven't convinced me why eDP is special. -Chris
On Thu, Jul 28, 2016 at 04:47:07PM +0100, Chris Wilson wrote: > On Thu, Jul 28, 2016 at 05:50:37PM +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Currently we try to check the lid status to see whether eDP is connected > > or not. However we ignore the result anyway, and we have no lid notifier > > to fire off uevents, so let's just ignore the lid stuff for eDP > > entirely. > > You mean the OpRegion no longer contains lid_state? I don't see it being > disabled... The lid notifier is from acpi/button.c. I haven't looked at > why the result would be ignored, but you haven't convinced me why eDP is > special. We don't register a lid notifier for eDP. So if the initial detect would happen with lid closed we'd mark eDP as disconnected, and when the lid gets opened we won't even notice, and thus the connector never becomes connected. IIRC on my ILK the lid state in OpRegion didn't work anyway. But I should probably double check.
On Thu, Jul 28, 2016 at 07:01:29PM +0300, Ville Syrjälä wrote: > On Thu, Jul 28, 2016 at 04:47:07PM +0100, Chris Wilson wrote: > > On Thu, Jul 28, 2016 at 05:50:37PM +0300, ville.syrjala@linux.intel.com wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Currently we try to check the lid status to see whether eDP is connected > > > or not. However we ignore the result anyway, and we have no lid notifier > > > to fire off uevents, so let's just ignore the lid stuff for eDP > > > entirely. > > > > You mean the OpRegion no longer contains lid_state? I don't see it being > > disabled... The lid notifier is from acpi/button.c. I haven't looked at > > why the result would be ignored, but you haven't convinced me why eDP is > > special. > > We don't register a lid notifier for eDP. So if the initial detect would > happen with lid closed we'd mark eDP as disconnected, and when the lid > gets opened we won't even notice, and thus the connector never becomes > connected. That was meant to be part of the intel_panel so that we have consistent behaviour on any laptop. I would still rather we have consistent behaviour, but eDP has been different for so long that any change now is going to be painful. > IIRC on my ILK the lid state in OpRegion didn't work anyway. But I > should probably double check. Yup, the lid button was unreliable as every other piece of hw. Looking at intel_lid_notify(), would it not be easier just to send the hotplug uevent. -Chris
On Thu, Jul 28, 2016 at 05:36:27PM +0100, Chris Wilson wrote: > On Thu, Jul 28, 2016 at 07:01:29PM +0300, Ville Syrjälä wrote: > > On Thu, Jul 28, 2016 at 04:47:07PM +0100, Chris Wilson wrote: > > > On Thu, Jul 28, 2016 at 05:50:37PM +0300, ville.syrjala@linux.intel.com wrote: > > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > > > Currently we try to check the lid status to see whether eDP is connected > > > > or not. However we ignore the result anyway, and we have no lid notifier > > > > to fire off uevents, so let's just ignore the lid stuff for eDP > > > > entirely. > > > > > > You mean the OpRegion no longer contains lid_state? I don't see it being > > > disabled... The lid notifier is from acpi/button.c. I haven't looked at > > > why the result would be ignored, but you haven't convinced me why eDP is > > > special. > > > > We don't register a lid notifier for eDP. So if the initial detect would > > happen with lid closed we'd mark eDP as disconnected, and when the lid > > gets opened we won't even notice, and thus the connector never becomes > > connected. > > That was meant to be part of the intel_panel so that we have consistent > behaviour on any laptop. I would still rather we have consistent > behaviour, but eDP has been different for so long that any change now is > going to be painful. > > > IIRC on my ILK the lid state in OpRegion didn't work anyway. But I > > should probably double check. > > Yup, the lid button was unreliable as every other piece of hw. Looking > at intel_lid_notify(), would it not be easier just to send the hotplug > uevent. I thought the main point of the lid notifier was to restore any state trampled by the VBIOS. I've been hoping that I'd find some magic thing to get the VBIOS to not do that. IIRC there are some semi-interesting looking things in OpRegion that might just do it. But to know for sure I'd first have to get my hands on a machine that suffers from this affliction.
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 21b04c3eda41..1fe9bb745de5 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3965,9 +3965,6 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) if (!intel_dp_get_dpcd(intel_dp)) return connector_status_disconnected; - if (is_edp(intel_dp)) - return connector_status_connected; - /* if there's no downstream port, we're done */ if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT)) return connector_status_connected; @@ -4003,19 +4000,6 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp) return connector_status_disconnected; } -static enum drm_connector_status -edp_detect(struct intel_dp *intel_dp) -{ - struct drm_device *dev = intel_dp_to_dev(intel_dp); - enum drm_connector_status status; - - status = intel_panel_detect(dev); - if (status == connector_status_unknown) - status = connector_status_connected; - - return status; -} - static bool ibx_digital_port_connected(struct drm_i915_private *dev_priv, struct intel_digital_port *port) { @@ -4223,9 +4207,8 @@ intel_dp_long_pulse(struct intel_connector *intel_connector) power_domain = intel_display_port_aux_power_domain(intel_encoder); intel_display_power_get(to_i915(dev), power_domain); - /* Can't disconnect eDP, but you can close the lid... */ if (is_edp(intel_dp)) - status = edp_detect(intel_dp); + status = connector_status_connected; else if (intel_digital_port_connected(to_i915(dev), dp_to_dig_port(intel_dp))) status = intel_dp_detect_dpcd(intel_dp);