All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: patch for video.c driver
       [not found] <20090312154741.GA1055@hygelac>
@ 2009-03-13  2:08 ` Zhang Rui
  2009-03-13  3:00   ` Matthew Garrett
  2009-03-14  7:55   ` Len Brown
  0 siblings, 2 replies; 10+ messages in thread
From: Zhang Rui @ 2009-03-13  2:08 UTC (permalink / raw)
  To: Terence Ripperda; +Cc: lenb, linux-acpi

[-- Attachment #1: Type: text/plain, Size: 2920 bytes --]

cc linux-acpi mail list to get more comments. :)
the patch in the original email is also attached.

On Thu, 2009-03-12 at 23:47 +0800, Terence Ripperda wrote:
> Hello,
> 
> I work on the Linux driver team at nvidia and we've been working on improving
> our notebook support. One area that we're working on is improved hotkey
> support for display switching.
> 
> As you may know, notebooks tend to vary quite a bit in architecture. For the
> majority of notebooks, the current video.ko driver works great for routing
> hotkey events to userspace for higher level drivers to handle. But there are
> some notebooks that use different mechanisms to support hotkeys. For nvidia,
> this is via an NVIF ACPI extension (my understanding is that other vendors
> have similar extensions).

I saw NVIF method implemented on some laptops, but not one is using it.
so do you mean there will be a Nvidia platform specific driver using
NVIF for hotkeys?

> 
> NVIDIA would like to be able to support these extensions,

that's a good news. :)

> using the standard
> video.ko driver. To support this (in a neutral manner), our team has put
> together a patch to allow registration of generic ACPI methods to deliver
> events back to acpid.

I'm a little confused here.
First, NVIF is usually defined as a control method of an ACPI video bus
device. And this is why you want to support this in the generic ACPI
video driver, rather than a platform specific driver like the others,
right?
But I didn't see this piece of code(poking NVIF) in the patch below.
If you do this in another patch, please send the full patch set here. :)

>  The intent was to follow the same architecture that the
> video.ko driver already follows and to make the support generic enough for
> many different users. For example, above I mentioned that both NVIDIA and
> other vendors have ACPI extension methods, the support we're adding would be
> usable by all such vendors.

Makes sense, although the other vendors usually offers a platform
specific device rather than a platform specific control method.

> 
> The idea is that a higher level driver (for example, acpid) can request that a
> new method be enabled via the procfs interface to the video.ko driver.

If we need such an I/F, we'd better introduce a sysfs I/F rather than a
procfs one, because the ACPI procfs I/F is marked as deprecated and will
be removed sooner or later.

>  The
> video.ko driver would then register this method for incoming events.

But I see that only user space can register the customized method and
events, and ACPI video driver do nothing but sending the events to user
space again.
So my question is that, why would we need "char method_name[5];" in
"struct custom_method_data"?

>  Upon
> receiving these events, the video.ko would pass them on to the higher level
> drivers for processing.

the higher level drivers means user space here, right?

Thanks,
rui


[-- Attachment #2: video.c.3.patch --]
[-- Type: text/x-patch, Size: 5631 bytes --]

--- linux-2.6.28/drivers/acpi/video.c.orig	2009-03-06 08:40:07.000000000 -0600
+++ linux-2.6.28/drivers/acpi/video.c	2009-03-06 08:40:07.000000000 -0600
@@ -57,6 +57,7 @@
 #define ACPI_VIDEO_NOTIFY_DISPLAY_OFF		0x89
 
 #define MAX_NAME_LEN	20
+#define MAX_CUSTOM_METHODS		10
 
 #define ACPI_VIDEO_DISPLAY_CRT	1
 #define ACPI_VIDEO_DISPLAY_TV	2
@@ -132,9 +133,15 @@ struct acpi_video_enumerated_device {
 	struct acpi_video_device *bind_info;
 };
 
+struct custom_method_data {
+	char method_name[5]; 
+	u32  method_event_id;
+};
+
 struct acpi_video_bus {
 	struct acpi_device *device;
 	u8 dos_setting;
+	struct custom_method_data method_data[MAX_CUSTOM_METHODS];
 	struct acpi_video_enumerated_device *attached_array;
 	u8 attached_count;
 	struct acpi_video_bus_cap cap;
@@ -233,6 +240,15 @@ static struct file_operations acpi_video
 	.release = single_release,
 };
 
+static int acpi_video_bus_custom_METHODS_open_fs(struct inode *inode,
+						 struct file *file);
+static struct file_operations acpi_video_bus_custom_METHODS_fops = {
+	.open = acpi_video_bus_custom_METHODS_open_fs,
+	.read = seq_read,
+	.llseek = seq_lseek,
+	.release = single_release,
+};
+
 /* device */
 static int acpi_video_device_info_open_fs(struct inode *inode,
 					  struct file *file);
@@ -1291,6 +1307,24 @@ static int acpi_video_bus_DOS_seq_show(s
 	return 0;
 }
 
+static int acpi_video_bus_custom_METHODS_seq_show(struct seq_file *seq,
+						  void *offset)
+{
+	struct acpi_video_bus *video = seq->private;
+	int ctr = 0;
+
+	seq_printf(seq, "Custom methods:	Event-IDs\n");
+	for (ctr = 0; (video->method_data[ctr].method_name[0] != '\0') 
+			&& (ctr < MAX_CUSTOM_METHODS); ctr++) {
+		seq_printf(seq, "%s		0x%x\n", 
+			         video->method_data[ctr].method_name,
+			         video->method_data[ctr].method_event_id);
+	}
+
+	return 0;
+}
+
+
 static int acpi_video_bus_POST_open_fs(struct inode *inode, struct file *file)
 {
 	return single_open(file, acpi_video_bus_POST_seq_show,
@@ -1302,6 +1336,13 @@ static int acpi_video_bus_DOS_open_fs(st
 	return single_open(file, acpi_video_bus_DOS_seq_show, PDE(inode)->data);
 }
 
+static int acpi_video_bus_custom_METHODS_open_fs(struct inode *inode, 
+						 struct file *file)
+{
+	return single_open(file, acpi_video_bus_custom_METHODS_seq_show, 
+				 PDE(inode)->data);
+}
+
 static ssize_t
 acpi_video_bus_write_POST(struct file *file,
 			  const char __user * buffer,
@@ -1373,6 +1414,46 @@ acpi_video_bus_write_DOS(struct file *fi
 	return count;
 }
 
+static ssize_t
+acpi_video_bus_write_custom_METHODS(struct file *file,
+			 const char __user * buffer,
+			 size_t count, loff_t * data)
+{
+	struct seq_file *m = file->private_data;
+	struct acpi_video_bus *video = m->private;
+	char str[12] = { 0 };
+	int data_ctr = 0;
+	int ctr = 0;
+	
+
+	if (!video || count + 1 > sizeof(str))
+		return -EINVAL;
+
+	if (copy_from_user(str, buffer, count))
+		return -EFAULT;
+	
+	str[count] = 0;
+
+	while (video->method_data[ctr].method_name[0] != '\0') 
+		ctr++;
+
+	if (ctr+1 == MAX_CUSTOM_METHODS)
+		return -EFAULT;
+
+	while ((str[data_ctr] != '\0') && !(isspace(str[data_ctr])))
+		data_ctr++;
+
+	if (data_ctr > 4)
+		return -EFAULT;
+
+	strncpy(video->method_data[ctr].method_name, str, data_ctr);
+	video->method_data[ctr].method_name[data_ctr] = '\0';
+	video->method_data[ctr].method_event_id = strtoul(&str[data_ctr+1],
+							  NULL, 0);
+
+	return count;
+}
+
 static int acpi_video_bus_add_fs(struct acpi_device *device)
 {
 	struct acpi_video_bus *video = acpi_driver_data(device);
@@ -1424,9 +1505,20 @@ static int acpi_video_bus_add_fs(struct 
 	if (!entry)
 		goto err_remove_post;
 
+	/* 'custom_METHODS' [R/W] */
+	acpi_video_bus_custom_METHODS_fops.write = acpi_video_bus_write_custom_METHODS;
+	entry = proc_create_data("custom_METHODS", S_IFREG | S_IRUGO | S_IRUSR,
+				 device_dir,
+				 &acpi_video_bus_custom_METHODS_fops,
+				 acpi_driver_data(device));
+	if (!entry)
+		goto err_remove_dos;
+
 	video->dir = acpi_device_dir(device) = device_dir;
 	return 0;
 
+ err_remove_dos:
+	remove_proc_entry("DOS", device_dir);
  err_remove_post:
 	remove_proc_entry("POST", device_dir);
  err_remove_post_info:
@@ -1450,6 +1542,7 @@ static int acpi_video_bus_remove_fs(stru
 		remove_proc_entry("POST_info", device_dir);
 		remove_proc_entry("POST", device_dir);
 		remove_proc_entry("DOS", device_dir);
+		remove_proc_entry("custom_METHODS", device_dir);
 		remove_proc_entry(acpi_device_bid(device), acpi_video_dir);
 		acpi_device_dir(device) = NULL;
 	}
@@ -1836,6 +1929,8 @@ static void acpi_video_bus_notify(acpi_h
 	struct acpi_device *device = NULL;
 	struct input_dev *input;
 	int keycode;
+	int ctr = 0;
+	int custom_event = 0;
 
 	if (!video)
 		return;
@@ -1872,8 +1967,18 @@ static void acpi_video_bus_notify(acpi_h
 		break;
 
 	default:
+		while ((video->method_data[ctr].method_name[0] != '\0') &&
+				ctr < MAX_CUSTOM_METHODS) {  
+ 			if (event == video->method_data[ctr].method_event_id) { 
+				acpi_bus_generate_proc_event(device, event, 0);
+				custom_event = 1;
+				break;
+			}
+			ctr++;
+		}
 		keycode = KEY_UNKNOWN;
-		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
+		if (!custom_event)
+			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "Unsupported event [0x%x]\n", event));
 		break;
 	}
@@ -1990,6 +2095,7 @@ static int acpi_video_bus_add(struct acp
 	video->device = device;
 	strcpy(acpi_device_name(device), ACPI_VIDEO_BUS_NAME);
 	strcpy(acpi_device_class(device), ACPI_VIDEO_CLASS);
+	memset(video->method_data, 0, MAX_CUSTOM_METHODS * sizeof(struct custom_method_data));
 	device->driver_data = video;
 
 	acpi_video_bus_find_cap(video);

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: patch for video.c driver
  2009-03-13  2:08 ` patch for video.c driver Zhang Rui
@ 2009-03-13  3:00   ` Matthew Garrett
  2009-03-13 17:11     ` Rafael J. Wysocki
  2009-03-13 17:36     ` Terence Ripperda
  2009-03-14  7:55   ` Len Brown
  1 sibling, 2 replies; 10+ messages in thread
From: Matthew Garrett @ 2009-03-13  3:00 UTC (permalink / raw)
  To: Zhang Rui; +Cc: Terence Ripperda, lenb, linux-acpi

On Fri, Mar 13, 2009 at 10:08:12AM +0800, Zhang Rui wrote:
> On Thu, 2009-03-12 at 23:47 +0800, Terence Ripperda wrote:
> >  The intent was to follow the same architecture that the
> > video.ko driver already follows and to make the support generic enough for
> > many different users. For example, above I mentioned that both NVIDIA and
> > other vendors have ACPI extension methods, the support we're adding would be
> > usable by all such vendors.
> 
> Makes sense, although the other vendors usually offers a platform
> specific device rather than a platform specific control method.

Mm. I don't see any reason for video output switching to be handled in 
kernel. It's fundamentally behaviour that depends on the user's 
preferences, so the logical way to handle this is for the event to be 
sent to userland (as it is currently) and for a userspace agent to then 
turn this into an xrandr event. The only thing that currently prevents 
this from working with the binary nvidia drivers is the fact that they 
don't appear to support xrandr for output control. I'd prefer it if we 
didn't work around X driver shortcomings by adding interfaces to the 
kernel.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: patch for video.c driver
  2009-03-13  3:00   ` Matthew Garrett
@ 2009-03-13 17:11     ` Rafael J. Wysocki
  2009-03-13 17:36     ` Terence Ripperda
  1 sibling, 0 replies; 10+ messages in thread
From: Rafael J. Wysocki @ 2009-03-13 17:11 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Zhang Rui, Terence Ripperda, lenb, linux-acpi

On Friday 13 March 2009, Matthew Garrett wrote:
> On Fri, Mar 13, 2009 at 10:08:12AM +0800, Zhang Rui wrote:
> > On Thu, 2009-03-12 at 23:47 +0800, Terence Ripperda wrote:
> > >  The intent was to follow the same architecture that the
> > > video.ko driver already follows and to make the support generic enough for
> > > many different users. For example, above I mentioned that both NVIDIA and
> > > other vendors have ACPI extension methods, the support we're adding would be
> > > usable by all such vendors.
> > 
> > Makes sense, although the other vendors usually offers a platform
> > specific device rather than a platform specific control method.
> 
> Mm. I don't see any reason for video output switching to be handled in 
> kernel. It's fundamentally behaviour that depends on the user's 
> preferences, so the logical way to handle this is for the event to be 
> sent to userland (as it is currently) and for a userspace agent to then 
> turn this into an xrandr event. The only thing that currently prevents 
> this from working with the binary nvidia drivers is the fact that they 
> don't appear to support xrandr for output control. I'd prefer it if we 
> didn't work around X driver shortcomings by adding interfaces to the 
> kernel.

FWIW, agreed.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: patch for video.c driver
  2009-03-13  3:00   ` Matthew Garrett
  2009-03-13 17:11     ` Rafael J. Wysocki
@ 2009-03-13 17:36     ` Terence Ripperda
  2009-03-13 17:47       ` Matthew Garrett
  1 sibling, 1 reply; 10+ messages in thread
From: Terence Ripperda @ 2009-03-13 17:36 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Zhang Rui, Terence Ripperda, lenb, linux-acpi

On Thu, Mar 12, 2009 at 08:00:30PM -0700, mjg59@srcf.ucam.org wrote:
> On Fri, Mar 13, 2009 at 10:08:12AM +0800, Zhang Rui wrote:
> > On Thu, 2009-03-12 at 23:47 +0800, Terence Ripperda wrote:
> > >  The intent was to follow the same architecture that the
> > > video.ko driver already follows and to make the support generic enough for
> > > many different users. For example, above I mentioned that both NVIDIA and
> > > other vendors have ACPI extension methods, the support we're adding would be
> > > usable by all such vendors.
> > 
> > Makes sense, although the other vendors usually offers a platform
> > specific device rather than a platform specific control method.
> 
> Mm. I don't see any reason for video output switching to be handled in 
> kernel. It's fundamentally behaviour that depends on the user's 
> preferences, so the logical way to handle this is for the event to be 
> sent to userland (as it is currently) and for a userspace agent to then 
> turn this into an xrandr event. The only thing that currently prevents 
> this from working with the binary nvidia drivers is the fact that they 
> don't appear to support xrandr for output control. I'd prefer it if we 
> didn't work around X driver shortcomings by adding interfaces to the 
> kernel.

agreed, but that misunderstands the intent of this patch.

I consider hotkeys as 3 pieces:
- delivery of system event to a control mechanism (sbios/acpi/video.ko)
- control mechanism logic (to decide what to do) (userspace daemon)
- display change actions (xrandr support)

although the later 2 pieces can be common to all platforms, unfortunately
notebook vendors tend to be extremely customized, meaning mechanisms vary
greatly between vendors and even products of a single vendor. in a perfect
world, all vendors would follow the basic mechanism the video.c driver
implements (many do). unfortunately, we don't live in that world.

in the case I'm trying to fix here, a vendor is using the NVIF ACPI
extensions, rather than the DGS/DCS methods. not all vendors use these, but for
the ones that do, there's no other way to get the hotkey information. our fix
here is to update the video.c driver to acknowledge the NVIF methods and pass
the methods on to the userspace daemon.

we were attempting to make the patch more generic than just NVIF. no additional
kernel patches would be necessary. userspace logic to write the method name to
the video.ko is needed (the intent here is that the method name could be NVIF on
nvidia or something else for other vendors). if an NVIF specific approach is
desired, we could clean up the patch to specifically enable NVIF methods.


in terms of the higher level logic layers,

currently the nvidia X driver handles the control logic and display change
actions on our boards. this was written a couple of years ago and due to nothing
else in place at the time. we'd prefer to switch to a more generic mechanism in
line with what the community is working on. my understanding is that there is
work on a daemon that relies on X RandR 1.2. I would like nvidia to get involved
in and support that effort. (and I admit nvidia is behind on getting our X RandR
support up to 1.2)

even once that's done, you'd still need the patch (or a similar patch) I'm
suggesting for this infrastructure to work on NVIF-based platforms. userspace
logic for parsing the NVIF methods would also be needed; I'm working on getting
sign-off to release that IP for general linux support.



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

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: patch for video.c driver
  2009-03-13 17:36     ` Terence Ripperda
@ 2009-03-13 17:47       ` Matthew Garrett
  2009-03-13 17:58         ` Terence Ripperda
  0 siblings, 1 reply; 10+ messages in thread
From: Matthew Garrett @ 2009-03-13 17:47 UTC (permalink / raw)
  To: Terence Ripperda; +Cc: Zhang Rui, lenb, linux-acpi

On Fri, Mar 13, 2009 at 12:36:04PM -0500, Terence Ripperda wrote:

> in the case I'm trying to fix here, a vendor is using the NVIF ACPI
> extensions, rather than the DGS/DCS methods. not all vendors use these, but for
> the ones that do, there's no other way to get the hotkey information. our fix
> here is to update the video.c driver to acknowledge the NVIF methods and pass
> the methods on to the userspace daemon.

I'm not quite clear on what you mean by the hotkey information here. As 
far as I'm aware, we don't use DCS/DGS support for anything on Linux 
since it often seems to be either broken or just filled with incorrect 
information.

> currently the nvidia X driver handles the control logic and display change
> actions on our boards. this was written a couple of years ago and due to nothing
> else in place at the time. we'd prefer to switch to a more generic mechanism in
> line with what the community is working on. my understanding is that there is
> work on a daemon that relies on X RandR 1.2. I would like nvidia to get involved
> in and support that effort. (and I admit nvidia is behind on getting our X RandR
> support up to 1.2)
> 
> even once that's done, you'd still need the patch (or a similar patch) I'm
> suggesting for this infrastructure to work on NVIF-based platforms. userspace
> logic for parsing the NVIF methods would also be needed; I'm working on getting
> sign-off to release that IP for general linux support.

Yeah, this is the bit I don't understand. The design we've adopted for 
output switching ignores any BIOS provided information, and instead just 
enables outputs based on the user preferences. The X driver then has 
responsibility for performing the actual switch. Nouveau appears to be 
able to handle this on nvidia platforms without any problem. So, really, 
I guess I'm not clear on what functionality NVIF is intended to provide 
here. Is it impossible to enable outputs on some systems without it?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: patch for video.c driver
  2009-03-13 17:47       ` Matthew Garrett
@ 2009-03-13 17:58         ` Terence Ripperda
  2009-03-13 18:06           ` Matthew Garrett
  0 siblings, 1 reply; 10+ messages in thread
From: Terence Ripperda @ 2009-03-13 17:58 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Terence Ripperda, Zhang Rui, lenb, linux-acpi

On Fri, Mar 13, 2009 at 10:47:26AM -0700, mjg59@srcf.ucam.org wrote:
> On Fri, Mar 13, 2009 at 12:36:04PM -0500, Terence Ripperda wrote:
> 
> > in the case I'm trying to fix here, a vendor is using the NVIF ACPI
> > extensions, rather than the DGS/DCS methods. not all vendors use these, but for
> > the ones that do, there's no other way to get the hotkey information. our fix
> > here is to update the video.c driver to acknowledge the NVIF methods and pass
> > the methods on to the userspace daemon.
> 
> I'm not quite clear on what you mean by the hotkey information here. As 
> far as I'm aware, we don't use DCS/DGS support for anything on Linux 
> since it often seems to be either broken or just filled with incorrect 
> information.

heh, I suspect that "incorrect information" is some of the per-vendor
customization.

our X driver currently handles hotkeys itself, by registering with acpid and
interacting with the video.ko driver. the video.ko driver passes the DCS/DGS
information to us via acpid & procfs. acpid alerts us to the hotkey and we use
procfs to get the DCS/DGS info. the display device ids returned by the SBIOS
tend to follow the ACPI spec, but there are a lot of customized cases. Nvidia
opted to implement these to avoid OEM confusion between Linux & Windows (display
switching order).


> > currently the nvidia X driver handles the control logic and display change
> > actions on our boards. this was written a couple of years ago and due to nothing
> > else in place at the time. we'd prefer to switch to a more generic mechanism in
> > line with what the community is working on. my understanding is that there is
> > work on a daemon that relies on X RandR 1.2. I would like nvidia to get involved
> > in and support that effort. (and I admit nvidia is behind on getting our X RandR
> > support up to 1.2)
> > 
> > even once that's done, you'd still need the patch (or a similar patch) I'm
> > suggesting for this infrastructure to work on NVIF-based platforms. userspace
> > logic for parsing the NVIF methods would also be needed; I'm working on getting
> > sign-off to release that IP for general linux support.
> 
> Yeah, this is the bit I don't understand. The design we've adopted for 
> output switching ignores any BIOS provided information, and instead just 
> enables outputs based on the user preferences. The X driver then has 
> responsibility for performing the actual switch. Nouveau appears to be 
> able to handle this on nvidia platforms without any problem. So, really, 
> I guess I'm not clear on what functionality NVIF is intended to provide 
> here. Is it impossible to enable outputs on some systems without it?

yes, this is what I mean by platform customizations. some notebook platforms
will rely on the DCS/DGS methods to perform hotkeys, others will rely on other
mechanisms, such as NVIF methods. I suspect you'll find that nouveau's mechanism
will work on a lot of notebooks, but not work at all on other notebooks.

in your case, are you receiving the acpid hotkey event, then choosing the
display switch order based on user customization/distro defaults? or are you
even just looking for the keypress sequence, independent of the acpi subsystem?

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

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: patch for video.c driver
  2009-03-13 17:58         ` Terence Ripperda
@ 2009-03-13 18:06           ` Matthew Garrett
  2009-03-13 20:45             ` Terence Ripperda
  0 siblings, 1 reply; 10+ messages in thread
From: Matthew Garrett @ 2009-03-13 18:06 UTC (permalink / raw)
  To: Terence Ripperda; +Cc: Zhang Rui, lenb, linux-acpi

On Fri, Mar 13, 2009 at 12:58:12PM -0500, Terence Ripperda wrote:
> On Fri, Mar 13, 2009 at 10:47:26AM -0700, mjg59@srcf.ucam.org wrote:
> > Yeah, this is the bit I don't understand. The design we've adopted for 
> > output switching ignores any BIOS provided information, and instead just 
> > enables outputs based on the user preferences. The X driver then has 
> > responsibility for performing the actual switch. Nouveau appears to be 
> > able to handle this on nvidia platforms without any problem. So, really, 
> > I guess I'm not clear on what functionality NVIF is intended to provide 
> > here. Is it impossible to enable outputs on some systems without it?
> 
> yes, this is what I mean by platform customizations. some notebook platforms
> will rely on the DCS/DGS methods to perform hotkeys, others will rely on other
> mechanisms, such as NVIF methods. I suspect you'll find that nouveau's mechanism
> will work on a lot of notebooks, but not work at all on other notebooks.

Can we make sure we're clear on terminology here? I'm using hotkey 
events to refer purely to the process by which the user hits a key, the 
firmware generates a notification, the kernel catches this notification 
and generates a KEY_SWITCHVIDEOMODE which we then catch in the user 
session.

There's then the issue of actually performing a display output change in 
response to this. This is curently handled by generating an xrandr 
request. The X driver (potentially in combination with the kernel 
driver, in a kernel modesetting world) then enables or disables outputs 
as appropriate.

The actual output changing is related to but not dependent upon the 
hotkey press, as the user may want to change output configuration via 
the graphical UI rather than by hitting the hotkey. So is the NVIF 
functionality required for generating the event in the first place, 
performing the actual output switch or simply providing the firmware's 
preferred set of enabled outputs?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: patch for video.c driver
  2009-03-13 18:06           ` Matthew Garrett
@ 2009-03-13 20:45             ` Terence Ripperda
  2009-03-13 20:52               ` Matthew Garrett
  0 siblings, 1 reply; 10+ messages in thread
From: Terence Ripperda @ 2009-03-13 20:45 UTC (permalink / raw)
  To: Matthew Garrett; +Cc: Terence Ripperda, Zhang Rui, lenb, linux-acpi

On Fri, Mar 13, 2009 at 11:06:46AM -0700, mjg59@srcf.ucam.org wrote:
> On Fri, Mar 13, 2009 at 12:58:12PM -0500, Terence Ripperda wrote:
> > On Fri, Mar 13, 2009 at 10:47:26AM -0700, mjg59@srcf.ucam.org wrote:
> > > Yeah, this is the bit I don't understand. The design we've adopted for 
> > > output switching ignores any BIOS provided information, and instead just 
> > > enables outputs based on the user preferences. The X driver then has 
> > > responsibility for performing the actual switch. Nouveau appears to be 
> > > able to handle this on nvidia platforms without any problem. So, really, 
> > > I guess I'm not clear on what functionality NVIF is intended to provide 
> > > here. Is it impossible to enable outputs on some systems without it?
> > 
> > yes, this is what I mean by platform customizations. some notebook platforms
> > will rely on the DCS/DGS methods to perform hotkeys, others will rely on other
> > mechanisms, such as NVIF methods. I suspect you'll find that nouveau's mechanism
> > will work on a lot of notebooks, but not work at all on other notebooks.
> 
> Can we make sure we're clear on terminology here? I'm using hotkey 
> events to refer purely to the process by which the user hits a key, the 
> firmware generates a notification, the kernel catches this notification 
> and generates a KEY_SWITCHVIDEOMODE which we then catch in the user 
> session.
> 
> There's then the issue of actually performing a display output change in 
> response to this. This is curently handled by generating an xrandr 
> request. The X driver (potentially in combination with the kernel 
> driver, in a kernel modesetting world) then enables or disables outputs 
> as appropriate.
> 
> The actual output changing is related to but not dependent upon the 
> hotkey press, as the user may want to change output configuration via 
> the graphical UI rather than by hitting the hotkey. So is the NVIF 
> functionality required for generating the event in the first place, 
> performing the actual output switch or simply providing the firmware's 
> preferred set of enabled outputs?

sure, I sometimes use the wrong terminology; I mean a display change hotkey,
where a keypress generates a display change event via the sbios/acpi subsystem.

I do not mean a case where a userspace daemon detects keypresses independent of
the sbios/acpi/kernel. I was trying to clarify whether nouveau was using such a 
mechanism.

NVIF methods implement both notification and query commands. so it's required
for generating the event and querying the firmware's preferred set of enabled
outputs.

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

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: patch for video.c driver
  2009-03-13 20:45             ` Terence Ripperda
@ 2009-03-13 20:52               ` Matthew Garrett
  0 siblings, 0 replies; 10+ messages in thread
From: Matthew Garrett @ 2009-03-13 20:52 UTC (permalink / raw)
  To: Terence Ripperda; +Cc: Zhang Rui, lenb, linux-acpi

On Fri, Mar 13, 2009 at 03:45:44PM -0500, Terence Ripperda wrote:

> NVIF methods implement both notification and query commands. so it's required
> for generating the event and querying the firmware's preferred set of enabled
> outputs.

Ok, got you. In that case the best approach would be for you to add an 
input device to the kernel component of your driver which listens for 
the notification and then generates KEY_SWITCHVIDEOMODE. The right level 
of integration for the preferred set of screens would be to extend the 
video output class to support listing the outputs as well as just 
performing the switches, then register with that if the platform doesn't 
support DGS and co. We're trying to deprecate the proc interfaces, and 
in general platform-specific drivers should hook into generic interfaces 
rather than overloading the functionality of the existing ACPI drivers.

Do you have any examples of machines that have this requirement?
-- 
Matthew Garrett | mjg59@srcf.ucam.org

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: patch for video.c driver
  2009-03-13  2:08 ` patch for video.c driver Zhang Rui
  2009-03-13  3:00   ` Matthew Garrett
@ 2009-03-14  7:55   ` Len Brown
  1 sibling, 0 replies; 10+ messages in thread
From: Len Brown @ 2009-03-14  7:55 UTC (permalink / raw)
  To: Terence Ripperda; +Cc: Zhang Rui, linux-acpi

Terence,

First, I'm delighted to hear from nvidia -- thanks
for reaching out to the Linux community.

a couple of observations...

drivers/acpi/*.c in general, and video.c in particular,
are meant to implement what is documented in the ACPI specification.

Linux has moved the vendor specific drivers (that may use ACPI,
or use vendor-specific ACPI extensions)
to drivers/platform/x86/*.c

That is where I'd generally expect an nvidia-specific ACPI extension to 
live.

Is there any public documentationon how NVIF is supposed to operate?

I have a new snappy Sony laptop that has both nvidia and intel graphics,
and it has an NVIF.  I'm certain that Linux has no idea how to
switch between "Stamina" and "Speed" modes...

But I also have a Lenovo T61 with only Intel graphics,
and it still has an NVIF.  (presumably Lenovo ships one
BIOS for both their Intel and Nvidia based T61's...)
What happens with NVIF in this case?

thanks,
-Len


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2009-03-14  7:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20090312154741.GA1055@hygelac>
2009-03-13  2:08 ` patch for video.c driver Zhang Rui
2009-03-13  3:00   ` Matthew Garrett
2009-03-13 17:11     ` Rafael J. Wysocki
2009-03-13 17:36     ` Terence Ripperda
2009-03-13 17:47       ` Matthew Garrett
2009-03-13 17:58         ` Terence Ripperda
2009-03-13 18:06           ` Matthew Garrett
2009-03-13 20:45             ` Terence Ripperda
2009-03-13 20:52               ` Matthew Garrett
2009-03-14  7:55   ` Len Brown

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.