From owner-freebsd-mobile@FreeBSD.ORG Thu Oct 18 13:47:35 2012 Return-Path: Delivered-To: mobile@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id D5B887B3; Thu, 18 Oct 2012 13:47:35 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigknife-pt.tunnel.tserv9.chi1.ipv6.he.net [IPv6:2001:470:1f10:75::2]) by mx1.freebsd.org (Postfix) with ESMTP id 95DF68FC16; Thu, 18 Oct 2012 13:47:35 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id E0D38B98A; Thu, 18 Oct 2012 09:47:34 -0400 (EDT) From: John Baldwin To: Juergen Lock Subject: Re: Dell acpi_video patch Date: Thu, 18 Oct 2012 08:59:14 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p20; KDE/4.5.5; amd64; ; ) References: <20121005215316.GA38707@triton8.kn-bremen.de> <201210121006.17276.jhb@freebsd.org> <20121012163349.GA63588@triton8.kn-bremen.de> In-Reply-To: <20121012163349.GA63588@triton8.kn-bremen.de> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201210180859.14457.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Thu, 18 Oct 2012 09:47:35 -0400 (EDT) Cc: avilla@freebsd.org, freebsd-acpi@freebsd.org, mobile@freebsd.org X-BeenThere: freebsd-mobile@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Mobile computing with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Oct 2012 13:47:35 -0000 On Friday, October 12, 2012 12:33:49 pm Juergen Lock wrote: > On Fri, Oct 12, 2012 at 10:06:17AM -0400, John Baldwin wrote: > > On Friday, October 05, 2012 5:53:16 pm Juergen Lock wrote: > > > Hi! > > > > > > I finally took a closer look why acpi_video found nothing on my > > > Dell laptop (Precision M4500), and came up with this patch: > > > > > > --- sys/dev/acpica/acpi_video.c.orig > > > +++ sys/dev/acpica/acpi_video.c > > > @@ -906,7 +906,7 @@ vid_enum_outputs_subr(ACPI_HANDLE handle > > > > > > for (i = 0; i < argset->dod_pkg->Package.Count; i++) { > > > if (acpi_PkgInt32(argset->dod_pkg, i, &val) == 0 && > > > - (val & DOD_DEVID_MASK_FULL) == adr) { > > > + (val & (DOD_DEVID_MASK_FULL | 0x80000000)) == adr) { > > > argset->callback(handle, val, argset->context); > > > argset->count++; > > > } > > > > > > which gives me: > > > > I think this is correct, but in we need to do more to properly handle that > > flag (DOD_DEVID_SCHEME_STD). Specifically, we shouldn't trust any bits in the > > device ID unless that bit is set (except for the special case of > > DOD_DEVID_LCD) as per my reading of the _DOD description in the ACPI 3.0b > > spec. I think this larger patch will do that while also fixing your case: > > Thank you, yes that still works for me the same as my original patch: Can you please test this updated patch as well: Index: acpi_video.c =================================================================== --- acpi_video.c (revision 241683) +++ acpi_video.c (working copy) @@ -320,7 +320,8 @@ acpi_video_resume(device_t dev) ACPI_SERIAL_BEGIN(video_output); STAILQ_FOREACH_SAFE(vo, &sc->vid_outputs, vo_next, vn) { if ((vo->adr & DOD_DEVID_MASK_FULL) != DOD_DEVID_LCD && - (vo->adr & DOD_DEVID_MASK) != DOD_DEVID_INTDFP) + (vo->adr & (DOD_DEVID_SCHEME_STD | DOD_DEVID_MASK)) != + (DOD_DEVID_SCHEME_STD | DOD_DEVID_INTDFP)) continue; if ((vo_get_device_status(vo->handle) & DCS_ACTIVE) == 0) @@ -467,38 +468,40 @@ acpi_video_vo_init(UINT32 adr) ACPI_SERIAL_ASSERT(video); - switch (adr & DOD_DEVID_MASK) { + /* Assume an unknown unit by default. */ + desc = "unknown output"; + type = "out"; + voqh = &other_units; + + switch (adr & (DOD_DEVID_SCHEME_STD | DOD_DEVID_MASK)) { case DOD_DEVID_MONITOR: if ((adr & DOD_DEVID_MASK_FULL) == DOD_DEVID_LCD) { /* DOD_DEVID_LCD is a common, backward compatible ID */ desc = "Internal/Integrated Digital Flat Panel"; type = "lcd"; voqh = &lcd_units; - } else { - desc = "VGA CRT or VESA Compatible Analog Monitor"; - type = "crt"; - voqh = &crt_units; } break; - case DOD_DEVID_TV: + case DOD_DEVID_SCHEME_STD | DOD_DEVID_MONITOR: + desc = "VGA CRT or VESA Compatible Analog Monitor"; + type = "crt"; + voqh = &crt_units; + break; + case DOD_DEVID_SCHEME_STD | DOD_DEVID_TV: desc = "TV/HDTV or Analog-Video Monitor"; type = "tv"; voqh = &tv_units; break; - case DOD_DEVID_EXT: + case DOD_DEVID_SCHEME_STD | DOD_DEVID_EXT: desc = "External Digital Monitor"; type = "ext"; voqh = &ext_units; break; - case DOD_DEVID_INTDFP: + case DOD_DEVID_SCHEME_STD | DOD_DEVID_INTDFP: desc = "Internal/Integrated Digital Flat Panel"; type = "lcd"; voqh = &lcd_units; break; - default: - desc = "unknown output"; - type = "out"; - voqh = &other_units; } n = 0; @@ -633,21 +636,25 @@ acpi_video_vo_destroy(struct acpi_video_output *vo AcpiOsFree(vo->vo_levels); } - switch (vo->adr & DOD_DEVID_MASK) { + voqh = &other_units; + + switch (vo->adr & (DOD_DEVID_SCHEME_STD | DOD_DEVID_MASK)) { case DOD_DEVID_MONITOR: + if ((vo->adr & DOD_DEVID_MASK_FULL) == DOD_DEVID_LCD) + voqh = &lcd_units; + break; + case DOD_DEVID_SCHEME_STD | DOD_DEVID_MONITOR: voqh = &crt_units; break; - case DOD_DEVID_TV: + case DOD_DEVID_SCHEME_STD | DOD_DEVID_TV: voqh = &tv_units; break; - case DOD_DEVID_EXT: + case DOD_DEVID_SCHEME_STD | DOD_DEVID_EXT: voqh = &ext_units; break; - case DOD_DEVID_INTDFP: + case DOD_DEVID_SCHEME_STD | DOD_DEVID_INTDFP: voqh = &lcd_units; break; - default: - voqh = &other_units; } STAILQ_REMOVE(voqh, vo, acpi_video_output, vo_unit.next); free(vo, M_ACPIVIDEO); @@ -905,8 +912,14 @@ vid_enum_outputs_subr(ACPI_HANDLE handle, UINT32 l return (AE_OK); for (i = 0; i < argset->dod_pkg->Package.Count; i++) { + /* + * Some systems do not include DOD_DEVID_SCHEME_STD in + * the _ADR of output devices and some do, so just + * ignore DOD_DEVID_SCHEME_STD. + */ if (acpi_PkgInt32(argset->dod_pkg, i, &val) == 0 && - (val & DOD_DEVID_MASK_FULL) == adr) { + (val & DOD_DEVID_MASK_FULL) == + (adr & ~DOD_DEVID_SCHEME_STD)) { argset->callback(handle, val, argset->context); argset->count++; } -- John Baldwin