From owner-freebsd-acpi@FreeBSD.ORG Thu Aug 10 10:29:59 2006 Return-Path: X-Original-To: freebsd-acpi@freebsd.org Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 0E48116A4DE; Thu, 10 Aug 2006 10:29:59 +0000 (UTC) (envelope-from ducrot@poupinou.org) Received: from poup.poupinou.org (poup.poupinou.org [195.101.94.96]) by mx1.FreeBSD.org (Postfix) with ESMTP id 61E6343D45; Thu, 10 Aug 2006 10:29:58 +0000 (GMT) (envelope-from ducrot@poupinou.org) Received: from ducrot by poup.poupinou.org with local (Exim) id 1GB7nH-0003Mt-00; Thu, 10 Aug 2006 12:29:39 +0200 Date: Thu, 10 Aug 2006 12:29:39 +0200 To: Eric Anderson Message-ID: <20060810102939.GA4945@poupinou.org> References: <44B641F2.2020500@centtech.com> <32884.69.93.78.27.1152831695.squirrel@69.93.78.27> <34247.69.93.78.27.1152835592.squirrel@69.93.78.27> <39062.69.93.78.27.1152857140.squirrel@69.93.78.27> <20060715183804.GN17014@poupinou.org> <46050.69.93.78.27.1153011683.squirrel@69.93.78.27> <20060716032130.GP17014@poupinou.org> <44D781AA.3000508@centtech.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <44D781AA.3000508@centtech.com> User-Agent: Mutt/1.5.9i From: Bruno Ducrot Cc: freebsd-acpi@freebsd.org, freebsd-mobile@freebsd.org Subject: Re: ch to fix this Re: Dell/acpi_video hw.acpi.video.out0 is probably a bug, and an important one. Re: Dell laptops X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Aug 2006 10:29:59 -0000 On Mon, Aug 07, 2006 at 01:08:42PM -0500, Eric Anderson wrote: > On 07/15/06 22:21, Bruno Ducrot wrote: > >On Sat, Jul 15, 2006 at 09:01:23PM -0400, john@utzweb.net wrote: > >>>Hi John, > >>Hello Bruno > >> > >>>On Fri, Jul 14, 2006 at 02:05:40AM -0400, john@utzweb.net wrote: > >>>>acpi_video.c expects the lcd to be identified as 0x0110, but my Dell > >>>>Latitude C400 (and probably others) id's the lcd at 0x0400: > >>>> > >>>>Device (LCD) > >>>> { > >>>> Method (_ADR, 0, NotSerialized) > >>>> { > >>>> Return (0x0400) > >>>> } > >>>> > >>>> > >>>>so, acpi_video needs to account for this. > >>>> > >>>> > >>>>got this sorted, and now the display turns back on, here's the patch, i > >>>>already send-pr'd it > >>>Youre somewhat right, but your patch is wrong. > >>Thankyou for taking interest and reviewing my analysis and patch. > >> > >>I would beg to differ with your assertions concerning the patch's > >>correctness, because the operation you mention below is handled a few > >>lines above the patch. > >> > >>> Actually you have to check if ((adr & 0x0400) == 0x0400). > >>the & occurs at the top of the switch, here's the destroy case: > > > >But with the *WRONG* mask. It used to be 0xffff with ACPI v2, but > >shall now be 0x0f00 with ACPI v3. > > > >If for example the _ADR is 0x0401, then your patch won't work. Same > >if for example the _ADR is 0x0101, which identify a CRT monitor, etc. > > > >The only one value that must be kept for backward compatility is > >(adr & 0xffff) == 0x0110 which is for an internal Flat Panel, instead > >of a CRT monitor if we take the new specification into account without > >this very specific value. > > > >BTW I compiled and found some stupid mistakes. I also changed my DSDT > >such that I'm pretty sure it will work for you, and for others where > >the _ADR may be 0x04xx as well. > > > >Please consider that one: > > > >Index: acpi_video.c > >=================================================================== > >RCS file: /home/ncvs/src/sys/dev/acpica/acpi_video.c,v > >retrieving revision 1.12 > >diff -u -p -r1.12 acpi_video.c > >--- acpi_video.c 20 Dec 2005 22:42:16 -0000 1.12 > >+++ acpi_video.c 16 Jul 2006 03:11:24 -0000 > >@@ -110,9 +110,12 @@ static void vo_set_device_state(ACPI_HAN > > > > /* _DOD and subdev's _ADR */ > > #define DOD_DEVID_MASK 0xffff > >+#define DOD_DEVID_TYPE 0x0f00 > > #define DOD_DEVID_MONITOR 0x0100 > >-#define DOD_DEVID_PANEL 0x0110 > > #define DOD_DEVID_TV 0x0200 > >+#define DOD_DEVID_DIGITAL 0x0300 > >+#define DOD_DEVID_PANEL 0x0400 > >+#define DOD_DEVID_PANEL_COMPAT 0x0110 > > #define DOD_BIOS (1 << 16) > > #define DOD_NONVGA (1 << 17) > > #define DOD_HEAD_ID_SHIFT 18 > >@@ -409,27 +412,37 @@ acpi_video_vo_init(UINT32 adr) > > struct acpi_video_output_queue *voqh; > > > > ACPI_SERIAL_ASSERT(video); > >- switch (adr & DOD_DEVID_MASK) { > >- case DOD_DEVID_MONITOR: > >- desc = "CRT monitor"; > >- type = "crt"; > >- voqh = &crt_units; > >- break; > >- case DOD_DEVID_PANEL: > >+ if ((adr & DOD_DEVID_MASK) == DOD_DEVID_PANEL_COMPAT) { > > desc = "LCD panel"; > > type = "lcd"; > > voqh = &lcd_units; > >- break; > >- case DOD_DEVID_TV: > >- desc = "TV"; > >- type = "tv"; > >- voqh = &tv_units; > >- break; > >- default: > >- desc = "unknown output"; > >- type = "out"; > >- voqh = &other_units; > >- } > >+ } else > >+ switch (adr & DOD_DEVID_TYPE) { > >+ case DOD_DEVID_MONITOR: > >+ desc = "CRT monitor"; > >+ type = "crt"; > >+ voqh = &crt_units; > >+ break; > >+ case DOD_DEVID_DIGITAL: > >+ desc = "Digital monitor"; > >+ type = "crt"; > >+ voqh = &crt_units; > >+ break; > >+ case DOD_DEVID_PANEL: > >+ desc = "LCD panel"; > >+ type = "lcd"; > >+ voqh = &lcd_units; > >+ break; > >+ case DOD_DEVID_TV: > >+ desc = "TV"; > >+ type = "tv"; > >+ voqh = &tv_units; > >+ break; > >+ default: > >+ desc = "unknown output"; > >+ type = "out"; > >+ voqh = &other_units; > >+ } > > > > n = 0; > > vn = vp = NULL; > >@@ -553,19 +566,25 @@ acpi_video_vo_destroy(struct acpi_video_ > > if (vo->vo_levels != NULL) > > AcpiOsFree(vo->vo_levels); > > > >- switch (vo->adr & DOD_DEVID_MASK) { > >- case DOD_DEVID_MONITOR: > >- voqh = &crt_units; > >- break; > >- case DOD_DEVID_PANEL: > >+ if ((vo->adr & 0xffff) == DOD_DEVID_PANEL_COMPAT) > > voqh = &lcd_units; > >- break; > >- case DOD_DEVID_TV: > >- voqh = &tv_units; > >- break; > >- default: > >- voqh = &other_units; > >- } > >+ else > >+ switch (vo->adr & DOD_DEVID_TYPE) { > >+ case DOD_DEVID_MONITOR: > >+ voqh = &crt_units; > >+ break; > >+ case DOD_DEVID_DIGITAL: > >+ voqh = &crt_units; > >+ break; > >+ case DOD_DEVID_PANEL: > >+ voqh = &lcd_units; > >+ break; > >+ case DOD_DEVID_TV: > >+ voqh = &tv_units; > >+ break; > >+ default: > >+ voqh = &other_units; > >+ } > > STAILQ_REMOVE(voqh, vo, acpi_video_output, vo_unit.next); > > free(vo, M_ACPIVIDEO); > > } > > > > > >Cheers, > > > > > Did this ever get committed? If so, MFC'ed? > > Not yet. I thought to add more acpi 3.0 stuff onto acpi_video first, but it might be better indeed to first commit those bits first. Actually this will be the patch written by Hiroki Sato (see PR 100271). -- Bruno Ducrot -- Which is worse: ignorance or apathy? -- Don't know. Don't care.