Date: Thu, 18 Oct 2012 21:57:23 +0200 From: Juergen Lock <nox@jelal.kn-bremen.de> To: John Baldwin <jhb@freebsd.org> Cc: avilla@freebsd.org, freebsd-acpi@freebsd.org, Juergen Lock <nox@jelal.kn-bremen.de>, mobile@freebsd.org Subject: Re: Dell acpi_video patch Message-ID: <20121018195723.GA10042@triton8.kn-bremen.de> In-Reply-To: <201210180859.14457.jhb@freebsd.org> References: <20121005215316.GA38707@triton8.kn-bremen.de> <201210121006.17276.jhb@freebsd.org> <20121012163349.GA63588@triton8.kn-bremen.de> <201210180859.14457.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121018195723.GA10042>