Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 18 Oct 2012 08:59:14 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        Juergen Lock <nox@jelal.kn-bremen.de>
Cc:        avilla@freebsd.org, freebsd-acpi@freebsd.org, mobile@freebsd.org
Subject:   Re: Dell acpi_video patch
Message-ID:  <201210180859.14457.jhb@freebsd.org>
In-Reply-To: <20121012163349.GA63588@triton8.kn-bremen.de>
References:  <20121005215316.GA38707@triton8.kn-bremen.de> <201210121006.17276.jhb@freebsd.org> <20121012163349.GA63588@triton8.kn-bremen.de>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201210180859.14457.jhb>