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

[Thomas Renninger <trenn@suse.de>]

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