[PATCH 2/4] ACPI: video: prevent ACPI video actions upon display switch notifications

[PATCH 2/4] ACPI: video: prevent ACPI video actions upon display switch notifications

[Zhang Rui]

From: Zhang Rui <rui.zhang@intel.com>

Display switching via ACPI control methods are known to work on none
platform AFAIK.
And graphics people want to control the display switching all by themselves.
Prevent ACPI from handling display switch hotkey events in this patch.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 drivers/acpi/video.c |   63 ---------------------------------------------------
 1 file changed, 63 deletions(-)

Index: linux-2.6/drivers/acpi/video.c
===================================================================
--- linux-2.6.orig/drivers/acpi/video.c
+++ linux-2.6/drivers/acpi/video.c
@@ -276,7 +276,6 @@ static void acpi_video_device_rebind(str
 static void acpi_video_device_bind(struct acpi_video_bus *video,
 				   struct acpi_video_device *device);
 static int acpi_video_device_enumerate(struct acpi_video_bus *video);
-static int acpi_video_switch_output(struct acpi_video_bus *video, int event);
 static int acpi_video_device_lcd_set_level(struct acpi_video_device *device,
 			int level);
 static int acpi_video_device_lcd_get_level_current(
@@ -1583,64 +1582,6 @@ static int acpi_video_device_enumerate(s
 	return status;
 }
 
-/*
- *  Arg:
- *  	video	: video bus device 
- *  	event	: notify event
- *
- *  Return:
- *  	< 0	: error
- *  
- *	1. Find out the current active output device.
- *	2. Identify the next output device to switch to.
- *	3. call _DSS to do actual switch.
- */
-
-static int acpi_video_switch_output(struct acpi_video_bus *video, int event)
-{
-	struct list_head *node;
-	struct acpi_video_device *dev = NULL;
-	struct acpi_video_device *dev_next = NULL;
-	struct acpi_video_device *dev_prev = NULL;
-	unsigned long state;
-	int status = 0;
-
-	mutex_lock(&video->device_list_lock);
-
-	list_for_each(node, &video->video_device_list) {
-		dev = container_of(node, struct acpi_video_device, entry);
-		status = acpi_video_device_get_state(dev, &state);
-		if (state & 0x2) {
-			dev_next = container_of(node->next,
-					struct acpi_video_device, entry);
-			dev_prev = container_of(node->prev,
-					struct acpi_video_device, entry);
-			goto out;
-		}
-	}
-
-	dev_next = container_of(node->next, struct acpi_video_device, entry);
-	dev_prev = container_of(node->prev, struct acpi_video_device, entry);
-
- out:
-	mutex_unlock(&video->device_list_lock);
-
-	switch (event) {
-	case ACPI_VIDEO_NOTIFY_CYCLE:
-	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:
-		acpi_video_device_set_state(dev, 0);
-		acpi_video_device_set_state(dev_next, 0x80000001);
-		break;
-	case ACPI_VIDEO_NOTIFY_PREV_OUTPUT:
-		acpi_video_device_set_state(dev, 0);
-		acpi_video_device_set_state(dev_prev, 0x80000001);
-	default:
-		break;
-	}
-
-	return status;
-}
-
 static int
 acpi_video_get_next_level(struct acpi_video_device *device,
 			  u32 level_current, u32 event)
@@ -1800,23 +1741,19 @@ static void acpi_video_bus_notify(acpi_h
 					 * connector. */
 		acpi_video_device_enumerate(video);
 		acpi_video_device_rebind(video);
-		acpi_video_switch_output(video, event);
 		acpi_bus_generate_proc_event(device, event, 0);
 		keycode = KEY_SWITCHVIDEOMODE;
 		break;
 
 	case ACPI_VIDEO_NOTIFY_CYCLE:	/* Cycle Display output hotkey pressed. */
-		acpi_video_switch_output(video, event);
 		acpi_bus_generate_proc_event(device, event, 0);
 		keycode = KEY_SWITCHVIDEOMODE;
 		break;
 	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:	/* Next Display output hotkey pressed. */
-		acpi_video_switch_output(video, event);
 		acpi_bus_generate_proc_event(device, event, 0);
 		keycode = KEY_VIDEO_NEXT;
 		break;
 	case ACPI_VIDEO_NOTIFY_PREV_OUTPUT:	/* previous Display output hotkey pressed. */
-		acpi_video_switch_output(video, event);
 		acpi_bus_generate_proc_event(device, event, 0);
 		keycode = KEY_VIDEO_PREV;
 		break;

Re: [PATCH 2/4] ACPI: video: prevent ACPI video actions upon display switch notifications

[Len Brown]

Applied.

thanks,
-Len

On Friday 25 January 2008 01:47, Zhang Rui wrote:
> From: Zhang Rui <rui.zhang@intel.com>
> 
> Display switching via ACPI control methods are known to work on none
> platform AFAIK.
> And graphics people want to control the display switching all by themselves.
> Prevent ACPI from handling display switch hotkey events in this patch.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/video.c |   63 ---------------------------------------------------
>  1 file changed, 63 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/video.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/video.c
> +++ linux-2.6/drivers/acpi/video.c
> @@ -276,7 +276,6 @@ static void acpi_video_device_rebind(str
>  static void acpi_video_device_bind(struct acpi_video_bus *video,
>  				   struct acpi_video_device *device);
>  static int acpi_video_device_enumerate(struct acpi_video_bus *video);
> -static int acpi_video_switch_output(struct acpi_video_bus *video, int event);
>  static int acpi_video_device_lcd_set_level(struct acpi_video_device *device,
>  			int level);
>  static int acpi_video_device_lcd_get_level_current(
> @@ -1583,64 +1582,6 @@ static int acpi_video_device_enumerate(s
>  	return status;
>  }
>  
> -/*
> - *  Arg:
> - *  	video	: video bus device 
> - *  	event	: notify event
> - *
> - *  Return:
> - *  	< 0	: error
> - *  
> - *	1. Find out the current active output device.
> - *	2. Identify the next output device to switch to.
> - *	3. call _DSS to do actual switch.
> - */
> -
> -static int acpi_video_switch_output(struct acpi_video_bus *video, int event)
> -{
> -	struct list_head *node;
> -	struct acpi_video_device *dev = NULL;
> -	struct acpi_video_device *dev_next = NULL;
> -	struct acpi_video_device *dev_prev = NULL;
> -	unsigned long state;
> -	int status = 0;
> -
> -	mutex_lock(&video->device_list_lock);
> -
> -	list_for_each(node, &video->video_device_list) {
> -		dev = container_of(node, struct acpi_video_device, entry);
> -		status = acpi_video_device_get_state(dev, &state);
> -		if (state & 0x2) {
> -			dev_next = container_of(node->next,
> -					struct acpi_video_device, entry);
> -			dev_prev = container_of(node->prev,
> -					struct acpi_video_device, entry);
> -			goto out;
> -		}
> -	}
> -
> -	dev_next = container_of(node->next, struct acpi_video_device, entry);
> -	dev_prev = container_of(node->prev, struct acpi_video_device, entry);
> -
> - out:
> -	mutex_unlock(&video->device_list_lock);
> -
> -	switch (event) {
> -	case ACPI_VIDEO_NOTIFY_CYCLE:
> -	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:
> -		acpi_video_device_set_state(dev, 0);
> -		acpi_video_device_set_state(dev_next, 0x80000001);
> -		break;
> -	case ACPI_VIDEO_NOTIFY_PREV_OUTPUT:
> -		acpi_video_device_set_state(dev, 0);
> -		acpi_video_device_set_state(dev_prev, 0x80000001);
> -	default:
> -		break;
> -	}
> -
> -	return status;
> -}
> -
>  static int
>  acpi_video_get_next_level(struct acpi_video_device *device,
>  			  u32 level_current, u32 event)
> @@ -1800,23 +1741,19 @@ static void acpi_video_bus_notify(acpi_h
>  					 * connector. */
>  		acpi_video_device_enumerate(video);
>  		acpi_video_device_rebind(video);
> -		acpi_video_switch_output(video, event);
>  		acpi_bus_generate_proc_event(device, event, 0);
>  		keycode = KEY_SWITCHVIDEOMODE;
>  		break;
>  
>  	case ACPI_VIDEO_NOTIFY_CYCLE:	/* Cycle Display output hotkey pressed. */
> -		acpi_video_switch_output(video, event);
>  		acpi_bus_generate_proc_event(device, event, 0);
>  		keycode = KEY_SWITCHVIDEOMODE;
>  		break;
>  	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:	/* Next Display output hotkey pressed. */
> -		acpi_video_switch_output(video, event);
>  		acpi_bus_generate_proc_event(device, event, 0);
>  		keycode = KEY_VIDEO_NEXT;
>  		break;
>  	case ACPI_VIDEO_NOTIFY_PREV_OUTPUT:	/* previous Display output hotkey pressed. */
> -		acpi_video_switch_output(video, event);
>  		acpi_bus_generate_proc_event(device, event, 0);
>  		keycode = KEY_VIDEO_PREV;
>  		break;
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/4] ACPI: video: prevent ACPI video actions upon display switch notifications

[Thomas Renninger]

On Fri, 2008-01-25 at 14:47 +0800, Zhang Rui wrote:
> From: Zhang Rui <rui.zhang@intel.com>
> 
> Display switching via ACPI control methods are known to work on none
> platform AFAIK.
> And graphics people want to control the display switching all by themselves.
> Prevent ACPI from handling display switch hotkey events in this patch.

I expect this one has to be reverted.
I can double check tomorrow, but I am pretty sure I have a Compaq and
also saw more and I expect there are a lot more that work pretty fine
with Display Output Switching (_DOS) set to be handled by BIOS and not
graphics driver.

As said, I can double check, but this would be a regression.

IMO the logic should be:
  - Let the BIOS handle the switching (also works without X)
  - If a graphics driver gets active which can do the switching, it can
    take over control by:
    echo 0 >/proc/acpi/video/*/DOS


   Thomas

> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/acpi/video.c |   63 ---------------------------------------------------
>  1 file changed, 63 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/video.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/video.c
> +++ linux-2.6/drivers/acpi/video.c
> @@ -276,7 +276,6 @@ static void acpi_video_device_rebind(str
>  static void acpi_video_device_bind(struct acpi_video_bus *video,
>  				   struct acpi_video_device *device);
>  static int acpi_video_device_enumerate(struct acpi_video_bus *video);
> -static int acpi_video_switch_output(struct acpi_video_bus *video, int event);
>  static int acpi_video_device_lcd_set_level(struct acpi_video_device *device,
>  			int level);
>  static int acpi_video_device_lcd_get_level_current(
> @@ -1583,64 +1582,6 @@ static int acpi_video_device_enumerate(s
>  	return status;
>  }
>  
> -/*
> - *  Arg:
> - *  	video	: video bus device 
> - *  	event	: notify event
> - *
> - *  Return:
> - *  	< 0	: error
> - *  
> - *	1. Find out the current active output device.
> - *	2. Identify the next output device to switch to.
> - *	3. call _DSS to do actual switch.
> - */
> -
> -static int acpi_video_switch_output(struct acpi_video_bus *video, int event)
> -{
> -	struct list_head *node;
> -	struct acpi_video_device *dev = NULL;
> -	struct acpi_video_device *dev_next = NULL;
> -	struct acpi_video_device *dev_prev = NULL;
> -	unsigned long state;
> -	int status = 0;
> -
> -	mutex_lock(&video->device_list_lock);
> -
> -	list_for_each(node, &video->video_device_list) {
> -		dev = container_of(node, struct acpi_video_device, entry);
> -		status = acpi_video_device_get_state(dev, &state);
> -		if (state & 0x2) {
> -			dev_next = container_of(node->next,
> -					struct acpi_video_device, entry);
> -			dev_prev = container_of(node->prev,
> -					struct acpi_video_device, entry);
> -			goto out;
> -		}
> -	}
> -
> -	dev_next = container_of(node->next, struct acpi_video_device, entry);
> -	dev_prev = container_of(node->prev, struct acpi_video_device, entry);
> -
> - out:
> -	mutex_unlock(&video->device_list_lock);
> -
> -	switch (event) {
> -	case ACPI_VIDEO_NOTIFY_CYCLE:
> -	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:
> -		acpi_video_device_set_state(dev, 0);
> -		acpi_video_device_set_state(dev_next, 0x80000001);
> -		break;
> -	case ACPI_VIDEO_NOTIFY_PREV_OUTPUT:
> -		acpi_video_device_set_state(dev, 0);
> -		acpi_video_device_set_state(dev_prev, 0x80000001);
> -	default:
> -		break;
> -	}
> -
> -	return status;
> -}
> -
>  static int
>  acpi_video_get_next_level(struct acpi_video_device *device,
>  			  u32 level_current, u32 event)
> @@ -1800,23 +1741,19 @@ static void acpi_video_bus_notify(acpi_h
>  					 * connector. */
>  		acpi_video_device_enumerate(video);
>  		acpi_video_device_rebind(video);
> -		acpi_video_switch_output(video, event);
>  		acpi_bus_generate_proc_event(device, event, 0);
>  		keycode = KEY_SWITCHVIDEOMODE;
>  		break;
>  
>  	case ACPI_VIDEO_NOTIFY_CYCLE:	/* Cycle Display output hotkey pressed. */
> -		acpi_video_switch_output(video, event);
>  		acpi_bus_generate_proc_event(device, event, 0);
>  		keycode = KEY_SWITCHVIDEOMODE;
>  		break;
>  	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:	/* Next Display output hotkey pressed. */
> -		acpi_video_switch_output(video, event);
>  		acpi_bus_generate_proc_event(device, event, 0);
>  		keycode = KEY_VIDEO_NEXT;
>  		break;
>  	case ACPI_VIDEO_NOTIFY_PREV_OUTPUT:	/* previous Display output hotkey pressed. */
> -		acpi_video_switch_output(video, event);
>  		acpi_bus_generate_proc_event(device, event, 0);
>  		keycode = KEY_VIDEO_PREV;
>  		break;
> 
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] ACPI: video: prevent ACPI video actions upon display switch notifications

[Thomas Renninger]

On Wed, 2008-03-12 at 01:21 +0100, Thomas Renninger wrote:
> On Fri, 2008-01-25 at 14:47 +0800, Zhang Rui wrote:
> > From: Zhang Rui <rui.zhang@intel.com>
> > 
> > Display switching via ACPI control methods are known to work on none
> > platform AFAIK.
> > And graphics people want to control the display switching all by themselves.
> > Prevent ACPI from handling display switch hotkey events in this patch.
> 
> I expect this one has to be reverted.
> I can double check tomorrow, but I am pretty sure I have a Compaq and
> also saw more and I expect there are a lot more that work pretty fine
> with Display Output Switching (_DOS) set to be handled by BIOS and not
> graphics driver.
> 
> As said, I can double check, but this would be a regression.
> 
> IMO the logic should be:
>   - Let the BIOS handle the switching (also works without X)
>   - If a graphics driver gets active which can do the switching, it can
>     take over control by:
>     echo 0 >/proc/acpi/video/*/DOS
> 

I mixed this up with the default value for _DOS:
git commit a21101c46ca5b4320e31408853cdcbf7cb1ce4ed

Again, IMO the logic should be:
 - Let the BIOS handle the switching (also works without X)
 - If a graphics driver gets active which can do the switching, it can
   take over control by:
   echo 0 >/proc/acpi/video/*/DOS

Also the bug which above commit claims to fix has an interesting add-on
comment (after the bug got closed):
----------------
My HP 6710B also crashes when DOS=0, DOS=1,2 or 3 works just fine though. This
has an GM965 intel chipset. What does the ACPI spec say about values other then
0 or 1 ?

It seems that the change that was done from 1 to 0 causes some quit bad
regressions on some laptops, while fixing others. Otoh all reporters say that
DOS=2 works..
----------------

      Thomas
> 
>    Thomas
> 
> > 
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> >  drivers/acpi/video.c |   63 ---------------------------------------------------
> >  1 file changed, 63 deletions(-)
> > 
> > Index: linux-2.6/drivers/acpi/video.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/video.c
> > +++ linux-2.6/drivers/acpi/video.c
> > @@ -276,7 +276,6 @@ static void acpi_video_device_rebind(str
> >  static void acpi_video_device_bind(struct acpi_video_bus *video,
> >  				   struct acpi_video_device *device);
> >  static int acpi_video_device_enumerate(struct acpi_video_bus *video);
> > -static int acpi_video_switch_output(struct acpi_video_bus *video, int event);
> >  static int acpi_video_device_lcd_set_level(struct acpi_video_device *device,
> >  			int level);
> >  static int acpi_video_device_lcd_get_level_current(
> > @@ -1583,64 +1582,6 @@ static int acpi_video_device_enumerate(s
> >  	return status;
> >  }
> >  
> > -/*
> > - *  Arg:
> > - *  	video	: video bus device 
> > - *  	event	: notify event
> > - *
> > - *  Return:
> > - *  	< 0	: error
> > - *  
> > - *	1. Find out the current active output device.
> > - *	2. Identify the next output device to switch to.
> > - *	3. call _DSS to do actual switch.
> > - */
> > -
> > -static int acpi_video_switch_output(struct acpi_video_bus *video, int event)
> > -{
> > -	struct list_head *node;
> > -	struct acpi_video_device *dev = NULL;
> > -	struct acpi_video_device *dev_next = NULL;
> > -	struct acpi_video_device *dev_prev = NULL;
> > -	unsigned long state;
> > -	int status = 0;
> > -
> > -	mutex_lock(&video->device_list_lock);
> > -
> > -	list_for_each(node, &video->video_device_list) {
> > -		dev = container_of(node, struct acpi_video_device, entry);
> > -		status = acpi_video_device_get_state(dev, &state);
> > -		if (state & 0x2) {
> > -			dev_next = container_of(node->next,
> > -					struct acpi_video_device, entry);
> > -			dev_prev = container_of(node->prev,
> > -					struct acpi_video_device, entry);
> > -			goto out;
> > -		}
> > -	}
> > -
> > -	dev_next = container_of(node->next, struct acpi_video_device, entry);
> > -	dev_prev = container_of(node->prev, struct acpi_video_device, entry);
> > -
> > - out:
> > -	mutex_unlock(&video->device_list_lock);
> > -
> > -	switch (event) {
> > -	case ACPI_VIDEO_NOTIFY_CYCLE:
> > -	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:
> > -		acpi_video_device_set_state(dev, 0);
> > -		acpi_video_device_set_state(dev_next, 0x80000001);
> > -		break;
> > -	case ACPI_VIDEO_NOTIFY_PREV_OUTPUT:
> > -		acpi_video_device_set_state(dev, 0);
> > -		acpi_video_device_set_state(dev_prev, 0x80000001);
> > -	default:
> > -		break;
> > -	}
> > -
> > -	return status;
> > -}
> > -
> >  static int
> >  acpi_video_get_next_level(struct acpi_video_device *device,
> >  			  u32 level_current, u32 event)
> > @@ -1800,23 +1741,19 @@ static void acpi_video_bus_notify(acpi_h
> >  					 * connector. */
> >  		acpi_video_device_enumerate(video);
> >  		acpi_video_device_rebind(video);
> > -		acpi_video_switch_output(video, event);
> >  		acpi_bus_generate_proc_event(device, event, 0);
> >  		keycode = KEY_SWITCHVIDEOMODE;
> >  		break;
> >  
> >  	case ACPI_VIDEO_NOTIFY_CYCLE:	/* Cycle Display output hotkey pressed. */
> > -		acpi_video_switch_output(video, event);
> >  		acpi_bus_generate_proc_event(device, event, 0);
> >  		keycode = KEY_SWITCHVIDEOMODE;
> >  		break;
> >  	case ACPI_VIDEO_NOTIFY_NEXT_OUTPUT:	/* Next Display output hotkey pressed. */
> > -		acpi_video_switch_output(video, event);
> >  		acpi_bus_generate_proc_event(device, event, 0);
> >  		keycode = KEY_VIDEO_NEXT;
> >  		break;
> >  	case ACPI_VIDEO_NOTIFY_PREV_OUTPUT:	/* previous Display output hotkey pressed. */
> > -		acpi_video_switch_output(video, event);
> >  		acpi_bus_generate_proc_event(device, event, 0);
> >  		keycode = KEY_VIDEO_PREV;
> >  		break;
> > 
> > 
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] ACPI: video: prevent ACPI video actions upondisplay switch notifications

[Zhang, Rui]

On Wed, 2008-03-12 at 09:21 +0800, Thomas Renninger wrote:
> 
> On Wed, 2008-03-12 at 01:21 +0100, Thomas Renninger wrote:
> > On Fri, 2008-01-25 at 14:47 +0800, Zhang Rui wrote:
> > > From: Zhang Rui <rui.zhang@intel.com>
> > >
> > > Display switching via ACPI control methods are known to work on
> none
> > > platform AFAIK.
> > > And graphics people want to control the display switching all by
> themselves.
> > > Prevent ACPI from handling display switch hotkey events in this
> patch.
> >
> > I expect this one has to be reverted.
> > I can double check tomorrow, but I am pretty sure I have a Compaq
> and
> > also saw more and I expect there are a lot more that work pretty
> fine
> > with Display Output Switching (_DOS) set to be handled by BIOS and
> not
> > graphics driver.
> >
> > As said, I can double check, but this would be a regression.
> >
> > IMO the logic should be:
> >   - Let the BIOS handle the switching (also works without X)
> >   - If a graphics driver gets active which can do the switching, it
> can
> >     take over control by:
> >     echo 0 >/proc/acpi/video/*/DOS
> >
> 
> I mixed this up with the default value for _DOS:
> git commit a21101c46ca5b4320e31408853cdcbf7cb1ce4ed
> 
> Again, IMO the logic should be:
>  - Let the BIOS handle the switching (also works without X)
>  - If a graphics driver gets active which can do the switching, it can
>    take over control by:
>    echo 0 >/proc/acpi/video/*/DOS
That's what I thought at the beginning,
and the bug
http://bugzilla.kernel.org/show_bug.cgi?id=6001
suggested me to set _DOS to 0 by default.

> 
> Also the bug which above commit claims to fix has an interesting
> add-on
> comment (after the bug got closed):
> ----------------
> My HP 6710B also crashes when DOS=0, DOS=1,2 or 3 works just fine
> though.
which version of kernel are you using?
I get this problem before, but it can not be reproduced in the kernel
later than 2.6.23.

>  This
> has an GM965 intel chipset. What does the ACPI spec say about values
> other then
> 0 or 1 ?
nothing interesting.
And many laptops don't follow the ACPI spec on this. :(
> It seems that the change that was done from 1 to 0 causes some quit
> bad
> regressions on some laptops, while fixing others. Otoh all reporters
> say that
> DOS=2 works..
> ----------------
well, DOS=2 doesn't work either...
there is no solution that can work for all laptops.

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] ACPI: video: prevent ACPI video actions upondisplay switch notifications

[Thomas Renninger]

On Wed, 2008-03-12 at 10:53 +0800, Zhang, Rui wrote:
> On Wed, 2008-03-12 at 09:21 +0800, Thomas Renninger wrote:
> > 
> > On Wed, 2008-03-12 at 01:21 +0100, Thomas Renninger wrote:
> > > On Fri, 2008-01-25 at 14:47 +0800, Zhang Rui wrote:
> > > > From: Zhang Rui <rui.zhang@intel.com>
> > > >
> > > > Display switching via ACPI control methods are known to work on
> > > > none platform AFAIK.
> > > > And graphics people want to control the display switching all by
> > > > themselves.
> > > > Prevent ACPI from handling display switch hotkey events in this
> > > > patch.
> > >
> > > I expect this one has to be reverted.
> > > I can double check tomorrow, but I am pretty sure I have a Compaq
> > > and also saw more and I expect there are a lot more that work pretty
> > > fine with Display Output Switching (_DOS) set to be handled by BIOS and
> > > not graphics driver.
> > >
> > > As said, I can double check, but this would be a regression.
> > >
> > > IMO the logic should be:
> > >   - Let the BIOS handle the switching (also works without X)
> > >   - If a graphics driver gets active which can do the switching, it
> > >     can take over control by:
> > >     echo 0 >/proc/acpi/video/*/DOS
> > >
> > 
> > I mixed this up with the default value for _DOS:
> > git commit a21101c46ca5b4320e31408853cdcbf7cb1ce4ed
> > 
> > Again, IMO the logic should be:
> >  - Let the BIOS handle the switching (also works without X)
> >  - If a graphics driver gets active which can do the switching, it can
> >    take over control by:
> >    echo 0 >/proc/acpi/video/*/DOS
> That's what I thought at the beginning,
> and the bug
> http://bugzilla.kernel.org/show_bug.cgi?id=6001
> suggested me to set _DOS to 0 by default.
Yep. I think _DOS set to 0 by default is wrong and in the bug machines
were stated where this causes problems now.

> > Also the bug which above commit claims to fix has an interesting
> > add-on
> > comment (after the bug got closed):
> > ----------------
> > My HP 6710B also crashes when DOS=0, DOS=1,2 or 3 works just fine
> > though.
> which version of kernel are you using?
> I get this problem before, but it can not be reproduced in the kernel
> later than 2.6.23.
> 
> > This has an GM965 intel chipset. What does the ACPI spec say about values
> > other then 0 or 1 ?
> nothing interesting.
> And many laptops don't follow the ACPI spec on this. :(

> > It seems that the change that was done from 1 to 0 causes some quit bad
> > regressions on some laptops, while fixing others. Otoh all reporters
> > say that
> > DOS=2 works..
> > ----------------
> well, DOS=2 doesn't work either...
> there is no solution that can work for all laptops.
Above was not from myself, but copied out from the bug.
The guy(s) complained that DOS=0 now breaks their machines...
I try to find some time to look at this a bit deeper and will then
comment in the bug (can take some time...).
IMO recent HPs can/should be taken as a reference as they tend to stick
to the ACPI spec closely.
As said, it was late and I mixed up patches, the patch to switch _DOS
default value to 0 was already in 2.6.24?
I don't know now how much was tried already.
A) To switch DOS to 0 when e.g. intel graphics driver gets active
(should be done by the graphics driver package or if X is started if the
driver is aware of the hotkey and can do the switching).
B) Otherwise (e.g. if framebuffer or graphics driver which do not switch
the display) it should stay to be BIOS handled.
This is the correct way IMO this should get handled.

Puhh, this needs a lot testing (different graphics drivers, different
machines,...) and implementation to switch to DOS=0 in graphics
drivers...

Thanks,

  Thomas

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Linux-gfx] Re: [PATCH 2/4] ACPI: video: prevent ACPI video actions upondisplay switch notifications

[Jesse Barnes]

On Wednesday, March 12, 2008 9:06 am Thomas Renninger wrote:
> Above was not from myself, but copied out from the bug.
> The guy(s) complained that DOS=0 now breaks their machines...
> I try to find some time to look at this a bit deeper and will then
> comment in the bug (can take some time...).
> IMO recent HPs can/should be taken as a reference as they tend to stick
> to the ACPI spec closely.
> As said, it was late and I mixed up patches, the patch to switch _DOS
> default value to 0 was already in 2.6.24?
> I don't know now how much was tried already.
> A) To switch DOS to 0 when e.g. intel graphics driver gets active
> (should be done by the graphics driver package or if X is started if the
> driver is aware of the hotkey and can do the switching).
> B) Otherwise (e.g. if framebuffer or graphics driver which do not switch
> the display) it should stay to be BIOS handled.
> This is the correct way IMO this should get handled.
>
> Puhh, this needs a lot testing (different graphics drivers, different
> machines,...) and implementation to switch to DOS=0 in graphics
> drivers...

Yeah, you're right that userspace drivers should probably set the DOS value 
when they startup, though '3' may be a better value since userspace probably 
doesn't care about the _DGS stuff.

The trouble is that DOS=0 works for some machines but breaks others, does 
anyone have a summary of the reports against the various values?

Jesse

Re: [Linux-gfx] Re: [PATCH 2/4] ACPI: video: prevent ACPI video actions upondisplay switch notifications

[Henrique de Moraes Holschuh]

On Wed, 12 Mar 2008, Jesse Barnes wrote:
> Yeah, you're right that userspace drivers should probably set the DOS value 
> when they startup, though '3' may be a better value since userspace probably 
> doesn't care about the _DGS stuff.

Just punt it to userspace.  Really.  Never do any ACPI video display
switching actions automatically by default (unless you use a *whitelist*).
Don't set _DOS by default.  Don't touch anything related to video-display
switching by default, AT ALL.  Just propagate any video switching events we
get down to userspace, and that's it for default behaviour.

Add a sysfs control to let userspace ask ACPI video to handle the display
switching (as it used to).  Let userspace configure how ACPI video should
behave when it gets ACPI display switch notifications.

It is one of these areas where no matter what you do, if you touch anything
you will break a box somewhere.  So you just punt it to userspace, because
in userspace it is possible to have a massive DMI model database with the
proper parameters for a given userspace X.org capabilities, and a given box
AML quirks.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: [Linux-gfx] Re: [PATCH 2/4] ACPI: video: prevent ACPI video actions upondisplay switch notifications

[Zhang, Rui]

On Thu, 2008-03-13 at 01:35 +0800, Barnes, Jesse wrote:
> On Wednesday, March 12, 2008 9:06 am Thomas Renninger wrote:
> > Above was not from myself, but copied out from the bug.
> > The guy(s) complained that DOS=0 now breaks their machines...
> > I try to find some time to look at this a bit deeper and will then
> > comment in the bug (can take some time...).
> > IMO recent HPs can/should be taken as a reference as they tend to
> stick
> > to the ACPI spec closely.
> > As said, it was late and I mixed up patches, the patch to switch
> _DOS
> > default value to 0 was already in 2.6.24?
> > I don't know now how much was tried already.
> > A) To switch DOS to 0 when e.g. intel graphics driver gets active
> > (should be done by the graphics driver package or if X is started if
> the
> > driver is aware of the hotkey and can do the switching).
> > B) Otherwise (e.g. if framebuffer or graphics driver which do not
> switch
> > the display) it should stay to be BIOS handled.
> > This is the correct way IMO this should get handled.
> >
> > Puhh, this needs a lot testing (different graphics drivers,
> different
> > machines,...) and implementation to switch to DOS=0 in graphics
> > drivers...
> 
> Yeah, you're right that userspace drivers should probably set the DOS
> value
> when they startup, though '3' may be a better value since userspace
> probably
> doesn't care about the _DGS stuff.
Well, it doesn't work on many platforms.

And from the comments here:
http://bugzilla.kernel.org/show_bug.cgi?id=9159#c5
Maybe DOS values other than "1" is not well tested.


> The trouble is that DOS=0 works for some machines but breaks others,
> does
> anyone have a summary of the reports against the various values?
No. :(.
And I'm afraid that the symptom is different on different machines...

thanks,
rui

Re: [PATCH 2/4] ACPI: video: prevent ACPI video actions upondisplay switch notifications

[Matthew Garrett]

On Wed, Mar 12, 2008 at 05:06:45PM +0100, Thomas Renninger wrote:

> Above was not from myself, but copied out from the bug.
> The guy(s) complained that DOS=0 now breaks their machines...
> I try to find some time to look at this a bit deeper and will then
> comment in the bug (can take some time...).
> IMO recent HPs can/should be taken as a reference as they tend to stick
> to the ACPI spec closely.

Setting my HP 2510p to a DOS of 0 leaves it smashing CPU state on every 
other lid open event, so I suspect it's not tested at all. I'd lean 
towards saying that the kernel default should probably be 1, with 
userspace altering that as appropriate.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [PATCH 2/4] ACPI: video: prevent ACPI video actionsupondisplay switch notifications

[Zhang, Rui]

On Thu, 2008-03-13 at 00:06 +0800, Thomas Renninger wrote:
> On Wed, 2008-03-12 at 10:53 +0800, Zhang, Rui wrote:
> > On Wed, 2008-03-12 at 09:21 +0800, Thomas Renninger wrote:
> > >
> > > On Wed, 2008-03-12 at 01:21 +0100, Thomas Renninger wrote:
> > > > On Fri, 2008-01-25 at 14:47 +0800, Zhang Rui wrote:
> > > > > From: Zhang Rui <rui.zhang@intel.com>
> > > > >
> > > > > Display switching via ACPI control methods are known to work
> on
> > > > > none platform AFAIK.
> > > > > And graphics people want to control the display switching all
> by
> > > > > themselves.
> > > > > Prevent ACPI from handling display switch hotkey events in
> this
> > > > > patch.
> > > >
> > > > I expect this one has to be reverted.
> > > > I can double check tomorrow, but I am pretty sure I have a
> Compaq
> > > > and also saw more and I expect there are a lot more that work
> pretty
> > > > fine with Display Output Switching (_DOS) set to be handled by
> BIOS and
> > > > not graphics driver.
> > > >
> > > > As said, I can double check, but this would be a regression.
> > > >
> > > > IMO the logic should be:
> > > >   - Let the BIOS handle the switching (also works without X)
> > > >   - If a graphics driver gets active which can do the switching,
> it
> > > >     can take over control by:
> > > >     echo 0 >/proc/acpi/video/*/DOS
> > > >
> > >
> > > I mixed this up with the default value for _DOS:
> > > git commit a21101c46ca5b4320e31408853cdcbf7cb1ce4ed
> > >
> > > Again, IMO the logic should be:
> > >  - Let the BIOS handle the switching (also works without X)
> > >  - If a graphics driver gets active which can do the switching, it
> can
> > >    take over control by:
> > >    echo 0 >/proc/acpi/video/*/DOS
> > That's what I thought at the beginning,
> > and the bug
> > http://bugzilla.kernel.org/show_bug.cgi?id=6001
> > suggested me to set _DOS to 0 by default.
> Yep. I think _DOS set to 0 by default is wrong and in the bug machines
> were stated where this causes problems now.
> 
> > > Also the bug which above commit claims to fix has an interesting
> > > add-on
> > > comment (after the bug got closed):
> > > ----------------
> > > My HP 6710B also crashes when DOS=0, DOS=1,2 or 3 works just fine
> > > though.
> > which version of kernel are you using?
> > I get this problem before, but it can not be reproduced in the
> kernel
> > later than 2.6.23.
> >
> > > This has an GM965 intel chipset. What does the ACPI spec say about
> values
> > > other then 0 or 1 ?
> > nothing interesting.
> > And many laptops don't follow the ACPI spec on this. :(
> 
> > > It seems that the change that was done from 1 to 0 causes some
> quit bad
> > > regressions on some laptops, while fixing others. Otoh all
> reporters
> > > say that
> > > DOS=2 works..
> > > ----------------
> > well, DOS=2 doesn't work either...
> > there is no solution that can work for all laptops.
> Above was not from myself, but copied out from the bug.
> The guy(s) complained that DOS=0 now breaks their machines...
> I try to find some time to look at this a bit deeper and will then
> comment in the bug (can take some time...).
> IMO recent HPs can/should be taken as a reference as they tend to
> stick
> to the ACPI spec closely.
> As said, it was late and I mixed up patches, the patch to switch _DOS
> default value to 0 was already in 2.6.24?
Yes.
> I don't know now how much was tried already.
> A) To switch DOS to 0 when e.g. intel graphics driver gets active
> (should be done by the graphics driver package or if X is started if
> the
> driver is aware of the hotkey and can do the switching).
> B) Otherwise (e.g. if framebuffer or graphics driver which do not
> switch
> the display) it should stay to be BIOS handled.
> This is the correct way IMO this should get handled.
Well, I agree. If the patch causes regressions, we should revert it.
And I saw several bugs on this.

thanks,
rui
> Puhh, this needs a lot testing (different graphics drivers, different
> machines,...) and implementation to switch to DOS=0 in graphics
> drivers...


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html