From owner-freebsd-acpi@FreeBSD.ORG Thu Oct 18 19:59:57 2012 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 65601250; Thu, 18 Oct 2012 19:59:57 +0000 (UTC) (envelope-from nox@jelal.kn-bremen.de) Received: from smtp.kn-bremen.de (gelbbaer.kn-bremen.de [78.46.108.116]) by mx1.freebsd.org (Postfix) with ESMTP id 9013E8FC17; Thu, 18 Oct 2012 19:59:56 +0000 (UTC) Received: by smtp.kn-bremen.de (Postfix, from userid 10) id 7275C1E000E3; Thu, 18 Oct 2012 21:59:55 +0200 (CEST) Received: from triton8.kn-bremen.de (noident@localhost [127.0.0.1]) by triton8.kn-bremen.de (8.14.4/8.14.4) with ESMTP id q9IJvOA5010060; Thu, 18 Oct 2012 21:57:24 +0200 (CEST) (envelope-from nox@triton8.kn-bremen.de) Received: (from nox@localhost) by triton8.kn-bremen.de (8.14.4/8.14.3/Submit) id q9IJvNTF010059; Thu, 18 Oct 2012 21:57:23 +0200 (CEST) (envelope-from nox) From: Juergen Lock Date: Thu, 18 Oct 2012 21:57:23 +0200 To: John Baldwin Subject: Re: Dell acpi_video patch Message-ID: <20121018195723.GA10042@triton8.kn-bremen.de> References: <20121005215316.GA38707@triton8.kn-bremen.de> <201210121006.17276.jhb@freebsd.org> <20121012163349.GA63588@triton8.kn-bremen.de> <201210180859.14457.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201210180859.14457.jhb@freebsd.org> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: avilla@freebsd.org, freebsd-acpi@freebsd.org, Juergen Lock , mobile@freebsd.org X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Oct 2012 19:59:57 -0000 On Thu, Oct 18, 2012 at 08:59:14AM -0400, John Baldwin wrote: > 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 That still works the same: % sysctl hw.acpi.video. hw.acpi.video.crt0.active: 0 hw.acpi.video.lcd0.active: 0 hw.acpi.video.lcd0.brightness: 100 hw.acpi.video.lcd0.fullpower: 100 hw.acpi.video.lcd0.economy: 46 hw.acpi.video.lcd0.levels: 100 46 0 6 13 20 26 33 40 46 53 60 66 73 80 86 93 100 hw.acpi.video.ext0.active: 0 hw.acpi.video.ext1.active: 0 hw.acpi.video.ext2.active: 0 hw.acpi.video.ext3.active: 0 % Thanx, :) Juergen