diff mbox

[3/4] drm/i915: use ACPI LID status for LVDS ->detect hook

Message ID 1247695886-18432-4-git-send-email-jbarnes@virtuousgeek.org (mailing list archive)
State RFC, archived
Headers show

Commit Message

Jesse Barnes July 15, 2009, 10:11 p.m. UTC
We can't load or hotplug detect LVDS like we can other outputs, but if
there's a lid device present we can use it as a proxy.  This allows the
LFP state to be determined at ->detect time, making configurations
requiring manual intervention today "just work" assuming the lid device
status is correct.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_lvds.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

Comments

Matthew Garrett July 15, 2009, 10:55 p.m. UTC | #1
On Wed, Jul 15, 2009 at 03:11:25PM -0700, Jesse Barnes wrote:
> We can't load or hotplug detect LVDS like we can other outputs, but if
> there's a lid device present we can use it as a proxy.  This allows the
> LFP state to be determined at ->detect time, making configurations
> requiring manual intervention today "just work" assuming the lid device
> status is correct.

I'm a bit unhappy with this appearing to be generic functionality but 
ending up implemented in the driver rather than in the KMS core. If this 
is going in then can we at least document this behaviour and that other 
KMS drivers are expected to implement it?
Jesse Barnes July 15, 2009, 11:09 p.m. UTC | #2
On Wed, 15 Jul 2009 23:55:58 +0100
Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Wed, Jul 15, 2009 at 03:11:25PM -0700, Jesse Barnes wrote:
> > We can't load or hotplug detect LVDS like we can other outputs, but
> > if there's a lid device present we can use it as a proxy.  This
> > allows the LFP state to be determined at ->detect time, making
> > configurations requiring manual intervention today "just work"
> > assuming the lid device status is correct.
> 
> I'm a bit unhappy with this appearing to be generic functionality but 
> ending up implemented in the driver rather than in the KMS core. If
> this is going in then can we at least document this behaviour and
> that other KMS drivers are expected to implement it?

Well, we don't really have generic LVDS helpers where we could put
this, but yeah we can document this as preferred for all LVDS outputs.
Zhao, Yakui July 16, 2009, 1:54 a.m. UTC | #3
On Thu, 2009-07-16 at 06:11 +0800, Jesse Barnes wrote:
> We can't load or hotplug detect LVDS like we can other outputs, but if
> there's a lid device present we can use it as a proxy.  This allows the
> LFP state to be determined at ->detect time, making configurations
> requiring manual intervention today "just work" assuming the lid device
> status is correct.
It is ok that the LID status is to decide whether the LVDS is
connected/disconnected.

But on some boxes the initial LID status is incorrect. It reports that
LID is closed although it is open.
    For example: 
    http://bugzilla.kernel.org/show_bug.cgi?id=5809
    http://bugzilla.kernel.org/show_bug.cgi?id=13263
    http://bugzilla.kernel.org/show_bug.cgi?id=5904
    https://bugs.launchpad.net/ubuntu/+bug/34389
Maybe this feature is not supported on the above laptops. That means
that LVDS is always connected regardless of LID status.

At the same time on some boxes there is no ACPI LID event when the LID
is reopened. In such case we can't send the hotplug event to user space
when the LID is reopened. How about disable this feature on such box?
    For example: Aspire One.


I have another issue about the hotplug event.
On some boxes it will continue to report the LID event  when the LID is
closed. In such case we had better not send the hotplug event to user
space. Otherwise the user space will receive too many hotplug event
about LVDS. For example:
     http://bugzilla.kernel.org/show_bug.cgi?id=10485
     http://bugzilla.kernel.org/show_bug.cgi?id=5853

So it will be better that the feature of hotplug/LID status is not
supported on such boxes.

Thanks
    Yakui
    
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_lvds.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 1d0d30a..57c86fd 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -599,12 +599,18 @@ static void intel_lvds_mode_set(struct drm_encoder *encoder,
>  /**
>   * Detect the LVDS connection.
>   *
> - * This always returns CONNECTOR_STATUS_CONNECTED.  This connector should only have
> - * been set up if the LVDS was actually connected anyway.
> + * Since LVDS doesn't have hotlug, we use the lid as a proxy.  Open means
> + * connected and closed means disconnected.  We also send hotplug events as
> + * needed, using lid status notification from the input layer.
>   */
>  static enum drm_connector_status intel_lvds_detect(struct drm_connector *connector)
>  {
> -	return connector_status_connected;
> +	enum drm_connector_status status = connector_status_connected;
> +
> +	if (!acpi_lid_open())
> +		status = connector_status_disconnected;
> +
> +	return status;
>  }
>  
>  /**

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jesse Barnes July 16, 2009, 4:32 p.m. UTC | #4
On Thu, 16 Jul 2009 09:54:44 +0800
ykzhao <yakui.zhao@intel.com> wrote:

> On Thu, 2009-07-16 at 06:11 +0800, Jesse Barnes wrote:
> > We can't load or hotplug detect LVDS like we can other outputs, but
> > if there's a lid device present we can use it as a proxy.  This
> > allows the LFP state to be determined at ->detect time, making
> > configurations requiring manual intervention today "just work"
> > assuming the lid device status is correct.
> It is ok that the LID status is to decide whether the LVDS is
> connected/disconnected.
> 
> But on some boxes the initial LID status is incorrect. It reports that
> LID is closed although it is open.
>     For example: 
>     http://bugzilla.kernel.org/show_bug.cgi?id=5809
>     http://bugzilla.kernel.org/show_bug.cgi?id=13263
>     http://bugzilla.kernel.org/show_bug.cgi?id=5904
>     https://bugs.launchpad.net/ubuntu/+bug/34389
> Maybe this feature is not supported on the above laptops. That means
> that LVDS is always connected regardless of LID status.
> 
> At the same time on some boxes there is no ACPI LID event when the LID
> is reopened. In such case we can't send the hotplug event to user
> space when the LID is reopened. How about disable this feature on
> such box? For example: Aspire One.
> 
> 
> I have another issue about the hotplug event.
> On some boxes it will continue to report the LID event  when the LID
> is closed. In such case we had better not send the hotplug event to
> user space. Otherwise the user space will receive too many hotplug
> event about LVDS. For example:
>      http://bugzilla.kernel.org/show_bug.cgi?id=10485
>      http://bugzilla.kernel.org/show_bug.cgi?id=5853
> 
> So it will be better that the feature of hotplug/LID status is not
> supported on such boxes.

Yeah those are all good points; thanks for the list of bugs.  Any
suggestions on how to do a blacklist?  Is there an ACPI or DMI
identifier we could use?
Zhao, Yakui July 17, 2009, 1:44 a.m. UTC | #5
On Fri, 2009-07-17 at 00:32 +0800, Jesse Barnes wrote:
> On Thu, 16 Jul 2009 09:54:44 +0800
> ykzhao <yakui.zhao@intel.com> wrote:
> 
> > On Thu, 2009-07-16 at 06:11 +0800, Jesse Barnes wrote:
> > > We can't load or hotplug detect LVDS like we can other outputs, but
> > > if there's a lid device present we can use it as a proxy.  This
> > > allows the LFP state to be determined at ->detect time, making
> > > configurations requiring manual intervention today "just work"
> > > assuming the lid device status is correct.
> > It is ok that the LID status is to decide whether the LVDS is
> > connected/disconnected.
> > 
> > But on some boxes the initial LID status is incorrect. It reports that
> > LID is closed although it is open.
> >     For example: 
> >     http://bugzilla.kernel.org/show_bug.cgi?id=5809
> >     http://bugzilla.kernel.org/show_bug.cgi?id=13263
> >     http://bugzilla.kernel.org/show_bug.cgi?id=5904
> >     https://bugs.launchpad.net/ubuntu/+bug/34389
> > Maybe this feature is not supported on the above laptops. That means
> > that LVDS is always connected regardless of LID status.
> > 
> > At the same time on some boxes there is no ACPI LID event when the LID
> > is reopened. In such case we can't send the hotplug event to user
> > space when the LID is reopened. How about disable this feature on
> > such box? For example: Aspire One.
> > 
> > 
> > I have another issue about the hotplug event.
> > On some boxes it will continue to report the LID event  when the LID
> > is closed. In such case we had better not send the hotplug event to
> > user space. Otherwise the user space will receive too many hotplug
> > event about LVDS. For example:
> >      http://bugzilla.kernel.org/show_bug.cgi?id=10485
> >      http://bugzilla.kernel.org/show_bug.cgi?id=5853
> > 
> > So it will be better that the feature of hotplug/LID status is not
> > supported on such boxes.
> 
> Yeah those are all good points; thanks for the list of bugs.  Any
> suggestions on how to do a blacklist?  Is there an ACPI or DMI
> identifier we could use?
How about not register the lid notifier callback function for the boxes
that fall into the backlist?
And the LVDS detection will always return connected when there is no LID
notifier callback.

Sorry that there is no dmidecode for the above laptops. I will ask the
bug reporter to attach them.

Thanks.
   Yakui
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 1d0d30a..57c86fd 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -599,12 +599,18 @@  static void intel_lvds_mode_set(struct drm_encoder *encoder,
 /**
  * Detect the LVDS connection.
  *
- * This always returns CONNECTOR_STATUS_CONNECTED.  This connector should only have
- * been set up if the LVDS was actually connected anyway.
+ * Since LVDS doesn't have hotlug, we use the lid as a proxy.  Open means
+ * connected and closed means disconnected.  We also send hotplug events as
+ * needed, using lid status notification from the input layer.
  */
 static enum drm_connector_status intel_lvds_detect(struct drm_connector *connector)
 {
-	return connector_status_connected;
+	enum drm_connector_status status = connector_status_connected;
+
+	if (!acpi_lid_open())
+		status = connector_status_disconnected;
+
+	return status;
 }
 
 /**