Dell acpi_video patch

[Juergen Lock]

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:
> 
> Index: acpi_video.c
> ===================================================================
> --- acpi_video.c	(revision 241382)
> +++ 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);
> @@ -906,7 +913,8 @@ vid_enum_outputs_subr(ACPI_HANDLE handle, UINT32 l
>  
>  	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_SCHEME_STD | DOD_DEVID_MASK_FULL)) ==
> +		    adr) {
>  			argset->callback(handle, val, argset->context);
>  			argset->count++;
>  		}
> 
> -- 
> John Baldwin

Thank you, yes that still works for me the same as my original patch:

% 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
%

	Juergen