Re: [LOR] ACPI related on 9-current

On Saturday 26 September 2009 10:41:36 am Munehiro Matsuda wrote:
> Hi all,
> 
> I've got two LORs related to ACPI on 9-current system from few days old 
world.
> The second one with "ACPI AC adapter", may relate to using sysutils/xbattbar
> from ports.
> If you need more info, please let me know.

Try this patch.  It avoids holding the ACPI adapter lock while calling all the 
power manager eventhandler routines, and it splits the ACPI video lock up 
into two locks: one to manage the "bus" of video output devices, and one that 
serves as the softc lock for each of the video output devices.

Index: dev/acpica/acpi_acad.c
===================================================================
--- dev/acpica/acpi_acad.c	(revision 197430)
+++ dev/acpica/acpi_acad.c	(working copy)
_at__at_ -109,13 +109,14 _at__at_
     ACPI_SERIAL_BEGIN(acad);
     if (newstatus != -1 && sc->status != newstatus) {
 	sc->status = newstatus;
+	ACPI_SERIAL_END(acad);
 	power_profile_set_state(newstatus ? POWER_PROFILE_PERFORMANCE :
 	    POWER_PROFILE_ECONOMY);
 	ACPI_VPRINT(dev, acpi_device_get_parent_softc(dev),
 	    "%s Line\n", newstatus ? "On" : "Off");
 	acpi_UserNotify("ACAD", h, newstatus);
-    }
-    ACPI_SERIAL_END(acad);
+    } else
+	ACPI_SERIAL_END(acad);
 }
 
 static void
Index: dev/acpica/acpi_video.c
===================================================================
--- dev/acpica/acpi_video.c	(revision 197438)
+++ dev/acpica/acpi_video.c	(working copy)
_at__at_ -170,7 +170,13 _at__at_
 static struct acpi_video_output_queue crt_units, tv_units,
     ext_units, lcd_units, other_units;
 
+/*
+ * The 'video' lock protects the hierarchy of video output devices
+ * (the video "bus").  The 'video_output' lock protects per-output
+ * data is equivalent to a softc lock for each video output.
+ */
 ACPI_SERIAL_DECL(video, "ACPI video");
+ACPI_SERIAL_DECL(video_output, "ACPI video output");
 MALLOC_DEFINE(M_ACPIVIDEO, "acpivideo", "ACPI video extension");
 
 static int
_at__at_ -236,12 +242,14 _at__at_
 	acpi_sc = devclass_get_softc(devclass_find("acpi"), 0);
 	if (acpi_sc == NULL)
 		return (ENXIO);
+	ACPI_SERIAL_BEGIN(video);
 	if (acpi_video_sysctl_tree == NULL) {
 		acpi_video_sysctl_tree = SYSCTL_ADD_NODE(&acpi_video_sysctl_ctx,
 				    SYSCTL_CHILDREN(acpi_sc->acpi_sysctl_tree),
 				    OID_AUTO, "video", CTLFLAG_RD, 0,
 				    "video extension control");
 	}
+	ACPI_SERIAL_END(video);
 
 	sc->device = dev;
 	sc->handle = acpi_get_handle(dev);
_at__at_ -316,6 +324,7 _at__at_
 		dss_p = 0;
 		lasthand = NULL;
 		ACPI_SERIAL_BEGIN(video);
+		ACPI_SERIAL_BEGIN(video_output);
 		STAILQ_FOREACH(vo, &sc->vid_outputs, vo_next) {
 			dss = vo_get_graphics_state(vo->handle);
 			dcs = vo_get_device_status(vo->handle);
_at__at_ -331,6 +340,7 _at__at_
 		}
 		if (lasthand != NULL)
 			vo_set_device_state(lasthand, dss_p|DSS_COMMIT);
+		ACPI_SERIAL_END(video_output);
 		ACPI_SERIAL_END(video);
 		break;
 	case VID_NOTIFY_REPROBE:
_at__at_ -367,12 +377,14 _at__at_
 		return;
 
 	ACPI_SERIAL_BEGIN(video);
+	ACPI_SERIAL_BEGIN(video_output);
 	STAILQ_FOREACH(vo, &sc->vid_outputs, vo_next) {
 		if (vo->vo_levels != NULL && vo->vo_brightness == -1)
 			vo_set_brightness(vo->handle,
 			    state == POWER_PROFILE_ECONOMY ?
 			    vo->vo_economy : vo->vo_fullpower);
 	}
+	ACPI_SERIAL_END(video_output);
 	ACPI_SERIAL_END(video);
 }
 
_at__at_ -550,7 +562,7 _at__at_
 acpi_video_vo_bind(struct acpi_video_output *vo, ACPI_HANDLE handle)
 {
 
-	ACPI_SERIAL_ASSERT(video);
+	ACPI_SERIAL_BEGIN(video_output);
 	if (vo->vo_levels != NULL)
 		AcpiOsFree(vo->vo_levels);
 	vo->handle = handle;
_at__at_ -565,6 +577,7 _at__at_
 			/* XXX - see above. */
 			vo->vo_economy = vo->vo_levels[BCL_ECONOMY];
 	}
+	ACPI_SERIAL_END(video_output);
 }
 
 static void
_at__at_ -605,7 +618,7 _at__at_
 {
 	int i;
 
-	ACPI_SERIAL_ASSERT(video);
+	ACPI_SERIAL_ASSERT(video_output);
 	if (vo->vo_levels == NULL)
 		return (ENODEV);
 	for (i = 0; i < vo->vo_numlevels; i++)
_at__at_ -624,7 +637,7 _at__at_
 	vo = (struct acpi_video_output *)arg1;
 	if (vo->handle == NULL)
 		return (ENXIO);
-	ACPI_SERIAL_BEGIN(video);
+	ACPI_SERIAL_BEGIN(video_output);
 	state = (vo_get_device_status(vo->handle) & DCS_ACTIVE) ? 1 : 0;
 	err = sysctl_handle_int(oidp, &state, 0, req);
 	if (err != 0 || req->newptr == NULL)
_at__at_ -632,7 +645,7 _at__at_
 	vo_set_device_state(vo->handle,
 	    DSS_COMMIT | (state ? DSS_ACTIVE : DSS_INACTIVE));
 out:
-	ACPI_SERIAL_END(video);
+	ACPI_SERIAL_END(video_output);
 	return (err);
 }
 
_at__at_ -644,7 +657,7 _at__at_
 	int level, preset, err;
 
 	vo = (struct acpi_video_output *)arg1;
-	ACPI_SERIAL_BEGIN(video);
+	ACPI_SERIAL_BEGIN(video_output);
 	if (vo->handle == NULL) {
 		err = ENXIO;
 		goto out;
_at__at_ -674,7 +687,7 _at__at_
 	vo_set_brightness(vo->handle, (level == -1) ? preset : level);
 
 out:
-	ACPI_SERIAL_END(video);
+	ACPI_SERIAL_END(video_output);
 	return (err);
 }
 
_at__at_ -686,7 +699,7 _at__at_
 
 	err = 0;
 	vo = (struct acpi_video_output *)arg1;
-	ACPI_SERIAL_BEGIN(video);
+	ACPI_SERIAL_BEGIN(video_output);
 	if (vo->handle == NULL) {
 		err = ENXIO;
 		goto out;
_at__at_ -717,7 +730,7 _at__at_
 	*preset = level;
 
 out:
-	ACPI_SERIAL_END(video);
+	ACPI_SERIAL_END(video_output);
 	return (err);
 }
 
_at__at_ -729,7 +742,7 _at__at_
 	int err;
 
 	vo = (struct acpi_video_output *)arg1;
-	ACPI_SERIAL_BEGIN(video);
+	ACPI_SERIAL_BEGIN(video_output);
 	if (vo->vo_levels == NULL) {
 		err = ENODEV;
 		goto out;
_at__at_ -742,7 +755,7 _at__at_
 	    vo->vo_numlevels * sizeof(*vo->vo_levels), req);
 
 out:
-	ACPI_SERIAL_END(video);
+	ACPI_SERIAL_END(video_output);
 	return (err);
 }
 
_at__at_ -892,6 +905,7 _at__at_
 {
 	ACPI_STATUS status;
 
+	ACPI_SERIAL_ASSERT(video_output);
 	status = acpi_SetInteger(handle, "_BCM", level);
 	if (ACPI_FAILURE(status))
 		printf("can't evaluate %s._BCM - %s\n",
_at__at_ -904,6 +918,7 _at__at_
 	UINT32 dcs;
 	ACPI_STATUS status;
 
+	ACPI_SERIAL_ASSERT(video_output);
 	dcs = 0;
 	status = acpi_GetInteger(handle, "_DCS", &dcs);
 	if (ACPI_FAILURE(status))
_at__at_ -933,6 +948,7 _at__at_
 {
 	ACPI_STATUS status;
 
+	ACPI_SERIAL_ASSERT(video_output);
 	status = acpi_SetInteger(handle, "_DSS", state);
 	if (ACPI_FAILURE(status))
 		printf("can't evaluate %s._DSS - %s\n",

-- 
John Baldwin