Date: Sun, 16 Jul 2006 02:47:41 -0400 (EDT) From: john@utzweb.net To: "Bruno Ducrot" <ducrot@poupinou.org> 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 Message-ID: <50159.69.93.78.27.1153032461.squirrel@69.93.78.27> In-Reply-To: <20060716032130.GP17014@poupinou.org> References: <Pine.GSO.4.64.0607130824240.6165@sea.ntplx.net> <44B6401F.8050507@centtech.com> <Pine.GSO.4.64.0607130848190.6165@sea.ntplx.net> <44B641F2.2020500@centtech.com> <Pine.GSO.4.64.0607130900460.6165@sea.ntplx.net> <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>
next in thread | previous in thread | raw e-mail | index | archive | help
> 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.
ohh!
> 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.
ok, i get it, my approach met my immediate needs but would fail in the
general v3 compliant case.
> 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.
i have much to learn, tnx for taking the time to explain it! reading the
spec should greatly diminish my clue deficit
> 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:
i'll try this out after i get some sleep.
> 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,
>
> --
> Bruno Ducrot
>
> -- Which is worse: ignorance or apathy?
> -- Don't know. Don't care.
>
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?50159.69.93.78.27.1153032461.squirrel>