[Qemu-devel] [PATCH] Add resolutions via the command-line

[Qemu-devel] [PATCH] Add resolutions via the command-line

[G 3]

Add the ability to add resolutions from the command-line. This patch  
works by
looking for a property called 'resolutions' in the options node of  
OpenBIOS.
If it is found all the resolutions are parsed and loaded.

Example command-line:

-prom-env resolutions=512x342,640x480,800x600,1024x600,1200x700,1440x900

Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
---
  QemuVGADriver/src/QemuVga.c | 227 ++++++++++++++++++++++++++++++++++ 
+++++++++-
  1 file changed, 225 insertions(+), 2 deletions(-)

diff --git a/QemuVGADriver/src/QemuVga.c b/QemuVGADriver/src/QemuVga.c
index 4584242..d74fa41 100644
--- a/QemuVGADriver/src/QemuVga.c
+++ b/QemuVGADriver/src/QemuVga.c
@@ -3,6 +3,7 @@
  #include "DriverQDCalls.h"
  #include "QemuVga.h"
  #include <Timer.h>
+#include <stdlib.h>

  /* List of supported modes */
  struct vMode {
@@ -18,7 +19,21 @@ static struct vMode vModes[] =  {
  	{ 1600, 1200 },
  	{ 1920, 1080 },
  	{ 1920, 1200 },
-	{ 0,0 }
+	
+	/* The rest are place holders */
+	{ 0,0 },
+	{ 0,0 },
+	{ 0,0 },
+	{ 0,0 },
+	{ 0,0 },
+	{ 0,0 },
+	{ 0,0 },
+	{ 0,0 },
+	{ 0,0 },
+	{ 0,0 },
+	{ 0,0 },
+	{ 0,0 },
+	{ 0,0 },
  };

  static void VgaWriteB(UInt16 port, UInt8 val)
@@ -147,11 +162,219 @@ static InterruptMemberNumber  
PCIInterruptHandler(InterruptSetMember ISTmember,
  }
  #endif

+/*
+ * Get the resolution set at the specified index.
+ * The string returned needs to be freed when no longer used.
+ */
+static char *get_set(const char *resolution_set_str, int set_number)
+{
+	const int max_buf_size = 100;
+	char c, *buffer;
+	int index, comma_count;
+
+	buffer = (char *) malloc(max_buf_size);
+	comma_count = 0;
+	index = 0;
+	set_number++; /* Makes things easier to understand */
+
+	c = *(resolution_set_str++);
+	while (c != '\0') {
+		buffer[index++] = c;
+		c = *(resolution_set_str++);
+		if (c == ',') {
+			comma_count++;
+			if (comma_count == set_number || index >= max_buf_size) {
+				buffer[index] = '\0';
+				return buffer;
+			}
+			
+			else {
+				/* reset the buffer */
+				index = 0;
+				c = *(resolution_set_str++);
+			}
+		}
+	}
+	
+	buffer[index] = '\0';
+
+	return buffer;
+}
+
+/*
+ * Get the number of resolution sets
+ */
+
+static int get_set_count(const char *resolution_sets_str)
+{
+	char c;
+	int count;
+	
+	/* Count the number of commas */
+	count = 0;
+	c = *(resolution_sets_str++);
+	while (c != '\0') {
+		if (c == ',') {
+			count++;
+		}
+		c = *(resolution_sets_str++);
+	}
+
+	return count + 1;
+}
+
+/*
+ * Returns the width value of a resolution set
+ * Example:
+ * input: 16000x9000
+ * output: 16000
+ */
+
+static int get_width(const char *resolution_str)
+{
+	int index;
+	char c;
+	const int max_buf_size = 25;
+	char buffer[max_buf_size];
+	c = *(resolution_str++);
+	index = 0;
+	while (c != 'x' && index < max_buf_size) {
+		buffer[index++] = c;
+		c = *(resolution_str++);
+	}
+	
+	/* Terminate string */
+	buffer[index] = '\0';
+	
+	return atoi(buffer);
+}
+
+/*
+ * Returns the height value of a resolution set
+ * Example
+ * input: 16000x9000
+ * output: 9000
+ */
+
+static int get_height(const char *resolution_str)
+{
+	int index;
+	char c;
+	const int max_buf_size = 25;
+	char buffer[max_buf_size];
+	
+	/* skip to character after x */
+	while (*resolution_str != 'x') {
+		resolution_str++;
+	}
+	resolution_str++;
+	
+	/* Add digits of the height to the buffer */
+	index = 0;
+	c = *(resolution_str++);
+	while (c != '\0') {
+		buffer[index++] = c;
+		c = *(resolution_str++);
+		if(index >= max_buf_size) {
+			break;
+		}
+	}
+	
+	/* Terminate string */
+	buffer[index] = '\0';
+	
+	return atoi(buffer);
+}
+
+/*
+ * Looks in the /chosen node for the value of the resolutions property
+ */
+static int add_user_resolutions(void)
+{	
+	RegEntryID *entry_id;
+	OSErr err;
+	Boolean is_done;
+	void *value;
+	RegPropertyValueSize property_size;
+	int index, res_set_count;
+	char *set_str;
+		
+	#define PROPERTY_NAME "resolutions"
+	#define NODE_PATH "Devices:device-tree:options"
+
+	/* init the entry variable */
+	err = RegistryEntryIDInit(entry_id);
+	if (err != noErr) {
+		lprintf("Error: Failed to init entry variable! (%d)\n", err);
+		return err;
+	}
+	is_done = false;
+
+	/* Get the entry ID value */
+	err = RegistryCStrEntryLookup(NULL /* start root */, NODE_PATH,  
entry_id);
+	if (err != noErr) {
+		lprintf("RegistryCStrEntryLookup() failure (%d)\n", err);
+		return err;
+	}
+
+	/* Get the size of the property */
+	err = RegistryPropertyGetSize(entry_id, PROPERTY_NAME,  
&property_size);
+	if (err != noErr) {
+		lprintf("Error: Failed to get property size! (%d)\n", err);
+		return err;
+	}
+
+	/* allocate memory to the value variable */
+	value = (void *) malloc(property_size);
+	
+	/* Get the value of the property */
+	err = RegistryPropertyGet(entry_id, PROPERTY_NAME, value,  
&property_size);
+	if (err != noErr) {
+		lprintf("Error: Failed to find property value %s!\n", PROPERTY_NAME);
+		return err;
+	}
+
+	/* Limit the number of resolutions to 20 */
+	#define sane_number_of_resolutions 20
+	res_set_count = get_set_count(value);
+	res_set_count = (res_set_count > sane_number_of_resolutions ?  
sane_number_of_resolutions : res_set_count);
+
+	/* Add each resolution set */
+	for(index = 0; index < res_set_count; index++) {
+		set_str = get_set(value, index);
+		vModes[index].width = get_width(set_str);
+		vModes[index].height = get_height(set_str);
+		free(set_str);
+	}
+
+	free(value);
+}
+
+/* Returns the number of resolutions */
+static int get_number_of_resolutions()
+{
+	int size_of_array, num_of_resolutions, index;
+	
+	num_of_resolutions = 0;
+	size_of_array = sizeof(vModes) / sizeof(struct vMode);
+	
+	for(index = 0; index < size_of_array; index++)
+	{
+		/* Don't count any place holders */
+		if (vModes[index].width != 0) {
+			num_of_resolutions++;
+		}
+	}
+	
+	return num_of_resolutions;
+}

  OSStatus QemuVga_Init(void)
  {
  	UInt16 id, i;
  	UInt32 mem, width, height, depth;
+	
+	add_user_resolutions();

  	lprintf("First MMIO read...\n");
  	id = DispiReadW(VBE_DISPI_INDEX_ID);
@@ -183,7 +406,7 @@ OSStatus QemuVga_Init(void)
  		i = 0;
  	}
  	GLOBAL.bootMode = i;
-	GLOBAL.numModes = sizeof(vModes) / sizeof(struct vMode) - 1;
+	GLOBAL.numModes = get_number_of_resolutions();

  	QemuVga_SetMode(GLOBAL.bootMode, depth, 0);

-- 
2.6.0

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[Benjamin Herrenschmidt]

On Sat, 2016-09-17 at 23:31 -0400, G 3 wrote:
> Add the ability to add resolutions from the command-line. This
> patch  
> works by
> looking for a property called 'resolutions' in the options node of  
> OpenBIOS.
> If it is found all the resolutions are parsed and loaded.
> 
> Example command-line:

You must not use the C library in the ndrv (malloc, atoi, ...)

Stick to what's in DriverServicesLib and NameRegistryLib.

> -prom-env
> resolutions=512x342,640x480,800x600,1024x600,1200x700,1440x900
> 
> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> ---
>   QemuVGADriver/src/QemuVga.c | 227
> ++++++++++++++++++++++++++++++++++ 
> +++++++++-
>   1 file changed, 225 insertions(+), 2 deletions(-)
> 
> diff --git a/QemuVGADriver/src/QemuVga.c
> b/QemuVGADriver/src/QemuVga.c
> index 4584242..d74fa41 100644
> --- a/QemuVGADriver/src/QemuVga.c
> +++ b/QemuVGADriver/src/QemuVga.c
> @@ -3,6 +3,7 @@
>   #include "DriverQDCalls.h"
>   #include "QemuVga.h"
>   #include <Timer.h>
> +#include <stdlib.h>
> 
>   /* List of supported modes */
>   struct vMode {
> @@ -18,7 +19,21 @@ static struct vMode vModes[] =  {
>   	{ 1600, 1200 },
>   	{ 1920, 1080 },
>   	{ 1920, 1200 },
> -	{ 0,0 }
> +	
> +	/* The rest are place holders */
> +	{ 0,0 },
> +	{ 0,0 },
> +	{ 0,0 },
> +	{ 0,0 },
> +	{ 0,0 },
> +	{ 0,0 },
> +	{ 0,0 },
> +	{ 0,0 },
> +	{ 0,0 },
> +	{ 0,0 },
> +	{ 0,0 },
> +	{ 0,0 },
> +	{ 0,0 },
>   };
> 
>   static void VgaWriteB(UInt16 port, UInt8 val)
> @@ -147,11 +162,219 @@ static InterruptMemberNumber  
> PCIInterruptHandler(InterruptSetMember ISTmember,
>   }
>   #endif
> 
> +/*
> + * Get the resolution set at the specified index.
> + * The string returned needs to be freed when no longer used.
> + */
> +static char *get_set(const char *resolution_set_str, int set_number)
> +{
> +	const int max_buf_size = 100;
> +	char c, *buffer;
> +	int index, comma_count;
> +
> +	buffer = (char *) malloc(max_buf_size);
> +	comma_count = 0;
> +	index = 0;
> +	set_number++; /* Makes things easier to understand */
> +
> +	c = *(resolution_set_str++);
> +	while (c != '\0') {
> +		buffer[index++] = c;
> +		c = *(resolution_set_str++);
> +		if (c == ',') {
> +			comma_count++;
> +			if (comma_count == set_number || index >=
> max_buf_size) {
> +				buffer[index] = '\0';
> +				return buffer;
> +			}
> +			
> +			else {
> +				/* reset the buffer */
> +				index = 0;
> +				c = *(resolution_set_str++);
> +			}
> +		}
> +	}
> +	
> +	buffer[index] = '\0';
> +
> +	return buffer;
> +}
> +
> +/*
> + * Get the number of resolution sets
> + */
> +
> +static int get_set_count(const char *resolution_sets_str)
> +{
> +	char c;
> +	int count;
> +	
> +	/* Count the number of commas */
> +	count = 0;
> +	c = *(resolution_sets_str++);
> +	while (c != '\0') {
> +		if (c == ',') {
> +			count++;
> +		}
> +		c = *(resolution_sets_str++);
> +	}
> +
> +	return count + 1;
> +}
> +
> +/*
> + * Returns the width value of a resolution set
> + * Example:
> + * input: 16000x9000
> + * output: 16000
> + */
> +
> +static int get_width(const char *resolution_str)
> +{
> +	int index;
> +	char c;
> +	const int max_buf_size = 25;
> +	char buffer[max_buf_size];
> +	c = *(resolution_str++);
> +	index = 0;
> +	while (c != 'x' && index < max_buf_size) {
> +		buffer[index++] = c;
> +		c = *(resolution_str++);
> +	}
> +	
> +	/* Terminate string */
> +	buffer[index] = '\0';
> +	
> +	return atoi(buffer);
> +}
> +
> +/*
> + * Returns the height value of a resolution set
> + * Example
> + * input: 16000x9000
> + * output: 9000
> + */
> +
> +static int get_height(const char *resolution_str)
> +{
> +	int index;
> +	char c;
> +	const int max_buf_size = 25;
> +	char buffer[max_buf_size];
> +	
> +	/* skip to character after x */
> +	while (*resolution_str != 'x') {
> +		resolution_str++;
> +	}
> +	resolution_str++;
> +	
> +	/* Add digits of the height to the buffer */
> +	index = 0;
> +	c = *(resolution_str++);
> +	while (c != '\0') {
> +		buffer[index++] = c;
> +		c = *(resolution_str++);
> +		if(index >= max_buf_size) {
> +			break;
> +		}
> +	}
> +	
> +	/* Terminate string */
> +	buffer[index] = '\0';
> +	
> +	return atoi(buffer);
> +}
> +
> +/*
> + * Looks in the /chosen node for the value of the resolutions
> property
> + */
> +static int add_user_resolutions(void)
> +{	
> +	RegEntryID *entry_id;
> +	OSErr err;
> +	Boolean is_done;
> +	void *value;
> +	RegPropertyValueSize property_size;
> +	int index, res_set_count;
> +	char *set_str;
> +		
> +	#define PROPERTY_NAME "resolutions"
> +	#define NODE_PATH "Devices:device-tree:options"
> +
> +	/* init the entry variable */
> +	err = RegistryEntryIDInit(entry_id);
> +	if (err != noErr) {
> +		lprintf("Error: Failed to init entry variable!
> (%d)\n", err);
> +		return err;
> +	}
> +	is_done = false;
> +
> +	/* Get the entry ID value */
> +	err = RegistryCStrEntryLookup(NULL /* start root */,
> NODE_PATH,  
> entry_id);
> +	if (err != noErr) {
> +		lprintf("RegistryCStrEntryLookup() failure (%d)\n",
> err);
> +		return err;
> +	}
> +
> +	/* Get the size of the property */
> +	err = RegistryPropertyGetSize(entry_id, PROPERTY_NAME,  
> &property_size);
> +	if (err != noErr) {
> +		lprintf("Error: Failed to get property size!
> (%d)\n", err);
> +		return err;
> +	}
> +
> +	/* allocate memory to the value variable */
> +	value = (void *) malloc(property_size);
> +	
> +	/* Get the value of the property */
> +	err = RegistryPropertyGet(entry_id, PROPERTY_NAME, value,  
> &property_size);
> +	if (err != noErr) {
> +		lprintf("Error: Failed to find property value
> %s!\n", PROPERTY_NAME);
> +		return err;
> +	}
> +
> +	/* Limit the number of resolutions to 20 */
> +	#define sane_number_of_resolutions 20
> +	res_set_count = get_set_count(value);
> +	res_set_count = (res_set_count > sane_number_of_resolutions
> ?  
> sane_number_of_resolutions : res_set_count);
> +
> +	/* Add each resolution set */
> +	for(index = 0; index < res_set_count; index++) {
> +		set_str = get_set(value, index);
> +		vModes[index].width = get_width(set_str);
> +		vModes[index].height = get_height(set_str);
> +		free(set_str);
> +	}
> +
> +	free(value);
> +}
> +
> +/* Returns the number of resolutions */
> +static int get_number_of_resolutions()
> +{
> +	int size_of_array, num_of_resolutions, index;
> +	
> +	num_of_resolutions = 0;
> +	size_of_array = sizeof(vModes) / sizeof(struct vMode);
> +	
> +	for(index = 0; index < size_of_array; index++)
> +	{
> +		/* Don't count any place holders */
> +		if (vModes[index].width != 0) {
> +			num_of_resolutions++;
> +		}
> +	}
> +	
> +	return num_of_resolutions;
> +}
> 
>   OSStatus QemuVga_Init(void)
>   {
>   	UInt16 id, i;
>   	UInt32 mem, width, height, depth;
> +	
> +	add_user_resolutions();
> 
>   	lprintf("First MMIO read...\n");
>   	id = DispiReadW(VBE_DISPI_INDEX_ID);
> @@ -183,7 +406,7 @@ OSStatus QemuVga_Init(void)
>   		i = 0;
>   	}
>   	GLOBAL.bootMode = i;
> -	GLOBAL.numModes = sizeof(vModes) / sizeof(struct vMode) - 1;
> +	GLOBAL.numModes = get_number_of_resolutions();
> 
>   	QemuVga_SetMode(GLOBAL.bootMode, depth, 0);
> 

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[G 3]

On Sep 19, 2016, at 2:24 AM, Benjamin Herrenschmidt wrote:

> On Sat, 2016-09-17 at 23:31 -0400, G 3 wrote:
>> Add the ability to add resolutions from the command-line. This
>> patch
>> works by
>> looking for a property called 'resolutions' in the options node of
>> OpenBIOS.
>> If it is found all the resolutions are parsed and loaded.
>>
>> Example command-line:
>
> You must not use the C library in the ndrv (malloc, atoi, ...)
>
> Stick to what's in DriverServicesLib and NameRegistryLib.

I originally thought that, but was wrong. Those C library functions  
work.


>
>> -prom-env
>> resolutions=512x342,640x480,800x600,1024x600,1200x700,1440x900
>>
>> Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
>> ---
>>   QemuVGADriver/src/QemuVga.c | 227
>> ++++++++++++++++++++++++++++++++++
>> +++++++++-
>>   1 file changed, 225 insertions(+), 2 deletions(-)
>>
>> diff --git a/QemuVGADriver/src/QemuVga.c
>> b/QemuVGADriver/src/QemuVga.c
>> index 4584242..d74fa41 100644
>> --- a/QemuVGADriver/src/QemuVga.c
>> +++ b/QemuVGADriver/src/QemuVga.c
>> @@ -3,6 +3,7 @@
>>   #include "DriverQDCalls.h"
>>   #include "QemuVga.h"
>>   #include <Timer.h>
>> +#include <stdlib.h>
>>
>>   /* List of supported modes */
>>   struct vMode {
>> @@ -18,7 +19,21 @@ static struct vMode vModes[] =  {
>>   	{ 1600, 1200 },
>>   	{ 1920, 1080 },
>>   	{ 1920, 1200 },
>> -	{ 0,0 }
>> +	
>> +	/* The rest are place holders */
>> +	{ 0,0 },
>> +	{ 0,0 },
>> +	{ 0,0 },
>> +	{ 0,0 },
>> +	{ 0,0 },
>> +	{ 0,0 },
>> +	{ 0,0 },
>> +	{ 0,0 },
>> +	{ 0,0 },
>> +	{ 0,0 },
>> +	{ 0,0 },
>> +	{ 0,0 },
>> +	{ 0,0 },
>>   };
>>
>>   static void VgaWriteB(UInt16 port, UInt8 val)
>> @@ -147,11 +162,219 @@ static InterruptMemberNumber
>> PCIInterruptHandler(InterruptSetMember ISTmember,
>>   }
>>   #endif
>>
>> +/*
>> + * Get the resolution set at the specified index.
>> + * The string returned needs to be freed when no longer used.
>> + */
>> +static char *get_set(const char *resolution_set_str, int set_number)
>> +{
>> +	const int max_buf_size = 100;
>> +	char c, *buffer;
>> +	int index, comma_count;
>> +
>> +	buffer = (char *) malloc(max_buf_size);
>> +	comma_count = 0;
>> +	index = 0;
>> +	set_number++; /* Makes things easier to understand */
>> +
>> +	c = *(resolution_set_str++);
>> +	while (c != '\0') {
>> +		buffer[index++] = c;
>> +		c = *(resolution_set_str++);
>> +		if (c == ',') {
>> +			comma_count++;
>> +			if (comma_count == set_number || index >=
>> max_buf_size) {
>> +				buffer[index] = '\0';
>> +				return buffer;
>> +			}
>> +			
>> +			else {
>> +				/* reset the buffer */
>> +				index = 0;
>> +				c = *(resolution_set_str++);
>> +			}
>> +		}
>> +	}
>> +	
>> +	buffer[index] = '\0';
>> +
>> +	return buffer;
>> +}
>> +
>> +/*
>> + * Get the number of resolution sets
>> + */
>> +
>> +static int get_set_count(const char *resolution_sets_str)
>> +{
>> +	char c;
>> +	int count;
>> +	
>> +	/* Count the number of commas */
>> +	count = 0;
>> +	c = *(resolution_sets_str++);
>> +	while (c != '\0') {
>> +		if (c == ',') {
>> +			count++;
>> +		}
>> +		c = *(resolution_sets_str++);
>> +	}
>> +
>> +	return count + 1;
>> +}
>> +
>> +/*
>> + * Returns the width value of a resolution set
>> + * Example:
>> + * input: 16000x9000
>> + * output: 16000
>> + */
>> +
>> +static int get_width(const char *resolution_str)
>> +{
>> +	int index;
>> +	char c;
>> +	const int max_buf_size = 25;
>> +	char buffer[max_buf_size];
>> +	c = *(resolution_str++);
>> +	index = 0;
>> +	while (c != 'x' && index < max_buf_size) {
>> +		buffer[index++] = c;
>> +		c = *(resolution_str++);
>> +	}
>> +	
>> +	/* Terminate string */
>> +	buffer[index] = '\0';
>> +	
>> +	return atoi(buffer);
>> +}
>> +
>> +/*
>> + * Returns the height value of a resolution set
>> + * Example
>> + * input: 16000x9000
>> + * output: 9000
>> + */
>> +
>> +static int get_height(const char *resolution_str)
>> +{
>> +	int index;
>> +	char c;
>> +	const int max_buf_size = 25;
>> +	char buffer[max_buf_size];
>> +	
>> +	/* skip to character after x */
>> +	while (*resolution_str != 'x') {
>> +		resolution_str++;
>> +	}
>> +	resolution_str++;
>> +	
>> +	/* Add digits of the height to the buffer */
>> +	index = 0;
>> +	c = *(resolution_str++);
>> +	while (c != '\0') {
>> +		buffer[index++] = c;
>> +		c = *(resolution_str++);
>> +		if(index >= max_buf_size) {
>> +			break;
>> +		}
>> +	}
>> +	
>> +	/* Terminate string */
>> +	buffer[index] = '\0';
>> +	
>> +	return atoi(buffer);
>> +}
>> +
>> +/*
>> + * Looks in the /chosen node for the value of the resolutions
>> property
>> + */
>> +static int add_user_resolutions(void)
>> +{	
>> +	RegEntryID *entry_id;
>> +	OSErr err;
>> +	Boolean is_done;
>> +	void *value;
>> +	RegPropertyValueSize property_size;
>> +	int index, res_set_count;
>> +	char *set_str;
>> +		
>> +	#define PROPERTY_NAME "resolutions"
>> +	#define NODE_PATH "Devices:device-tree:options"
>> +
>> +	/* init the entry variable */
>> +	err = RegistryEntryIDInit(entry_id);
>> +	if (err != noErr) {
>> +		lprintf("Error: Failed to init entry variable!
>> (%d)\n", err);
>> +		return err;
>> +	}
>> +	is_done = false;
>> +
>> +	/* Get the entry ID value */
>> +	err = RegistryCStrEntryLookup(NULL /* start root */,
>> NODE_PATH,
>> entry_id);
>> +	if (err != noErr) {
>> +		lprintf("RegistryCStrEntryLookup() failure (%d)\n",
>> err);
>> +		return err;
>> +	}
>> +
>> +	/* Get the size of the property */
>> +	err = RegistryPropertyGetSize(entry_id, PROPERTY_NAME,
>> &property_size);
>> +	if (err != noErr) {
>> +		lprintf("Error: Failed to get property size!
>> (%d)\n", err);
>> +		return err;
>> +	}
>> +
>> +	/* allocate memory to the value variable */
>> +	value = (void *) malloc(property_size);
>> +	
>> +	/* Get the value of the property */
>> +	err = RegistryPropertyGet(entry_id, PROPERTY_NAME, value,
>> &property_size);
>> +	if (err != noErr) {
>> +		lprintf("Error: Failed to find property value
>> %s!\n", PROPERTY_NAME);
>> +		return err;
>> +	}
>> +
>> +	/* Limit the number of resolutions to 20 */
>> +	#define sane_number_of_resolutions 20
>> +	res_set_count = get_set_count(value);
>> +	res_set_count = (res_set_count > sane_number_of_resolutions
>> ?
>> sane_number_of_resolutions : res_set_count);
>> +
>> +	/* Add each resolution set */
>> +	for(index = 0; index < res_set_count; index++) {
>> +		set_str = get_set(value, index);
>> +		vModes[index].width = get_width(set_str);
>> +		vModes[index].height = get_height(set_str);
>> +		free(set_str);
>> +	}
>> +
>> +	free(value);
>> +}
>> +
>> +/* Returns the number of resolutions */
>> +static int get_number_of_resolutions()
>> +{
>> +	int size_of_array, num_of_resolutions, index;
>> +	
>> +	num_of_resolutions = 0;
>> +	size_of_array = sizeof(vModes) / sizeof(struct vMode);
>> +	
>> +	for(index = 0; index < size_of_array; index++)
>> +	{
>> +		/* Don't count any place holders */
>> +		if (vModes[index].width != 0) {
>> +			num_of_resolutions++;
>> +		}
>> +	}
>> +	
>> +	return num_of_resolutions;
>> +}
>>
>>   OSStatus QemuVga_Init(void)
>>   {
>>   	UInt16 id, i;
>>   	UInt32 mem, width, height, depth;
>> +	
>> +	add_user_resolutions();
>>
>>   	lprintf("First MMIO read...\n");
>>   	id = DispiReadW(VBE_DISPI_INDEX_ID);
>> @@ -183,7 +406,7 @@ OSStatus QemuVga_Init(void)
>>   		i = 0;
>>   	}
>>   	GLOBAL.bootMode = i;
>> -	GLOBAL.numModes = sizeof(vModes) / sizeof(struct vMode) - 1;
>> +	GLOBAL.numModes = get_number_of_resolutions();
>>
>>   	QemuVga_SetMode(GLOBAL.bootMode, depth, 0);
>>

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[Benjamin Herrenschmidt]

On Mon, 2016-09-19 at 08:44 -0400, G 3 wrote:
> On Sep 19, 2016, at 2:24 AM, Benjamin Herrenschmidt wrote:
> 
> > 
> > On Sat, 2016-09-17 at 23:31 -0400, G 3 wrote:
> > > 
> > > Add the ability to add resolutions from the command-line. This
> > > patch
> > > works by
> > > looking for a property called 'resolutions' in the options node
> > > of
> > > OpenBIOS.
> > > If it is found all the resolutions are parsed and loaded.
> > > 
> > > Example command-line:
> > 
> > You must not use the C library in the ndrv (malloc, atoi, ...)
> > 
> > Stick to what's in DriverServicesLib and NameRegistryLib.
> 
> I originally thought that, but was wrong. Those C library functions  
> work.

No, don't use them. They "seem" to work but bad thing will happen,
especially on OS X.

Cheers,
Ben.

> 
> > 
> > 
> > > 
> > > -prom-env
> > > resolutions=512x342,640x480,800x600,1024x600,1200x700,1440x900
> > > 
> > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> > > ---
> > >   QemuVGADriver/src/QemuVga.c | 227
> > > ++++++++++++++++++++++++++++++++++
> > > +++++++++-
> > >   1 file changed, 225 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/QemuVGADriver/src/QemuVga.c
> > > b/QemuVGADriver/src/QemuVga.c
> > > index 4584242..d74fa41 100644
> > > --- a/QemuVGADriver/src/QemuVga.c
> > > +++ b/QemuVGADriver/src/QemuVga.c
> > > @@ -3,6 +3,7 @@
> > >   #include "DriverQDCalls.h"
> > >   #include "QemuVga.h"
> > >   #include <Timer.h>
> > > +#include <stdlib.h>
> > > 
> > >   /* List of supported modes */
> > >   struct vMode {
> > > @@ -18,7 +19,21 @@ static struct vMode vModes[] =  {
> > >   	{ 1600, 1200 },
> > >   	{ 1920, 1080 },
> > >   	{ 1920, 1200 },
> > > -	{ 0,0 }
> > > +	
> > > +	/* The rest are place holders */
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > > +	{ 0,0 },
> > >   };
> > > 
> > >   static void VgaWriteB(UInt16 port, UInt8 val)
> > > @@ -147,11 +162,219 @@ static InterruptMemberNumber
> > > PCIInterruptHandler(InterruptSetMember ISTmember,
> > >   }
> > >   #endif
> > > 
> > > +/*
> > > + * Get the resolution set at the specified index.
> > > + * The string returned needs to be freed when no longer used.
> > > + */
> > > +static char *get_set(const char *resolution_set_str, int
> > > set_number)
> > > +{
> > > +	const int max_buf_size = 100;
> > > +	char c, *buffer;
> > > +	int index, comma_count;
> > > +
> > > +	buffer = (char *) malloc(max_buf_size);
> > > +	comma_count = 0;
> > > +	index = 0;
> > > +	set_number++; /* Makes things easier to understand */
> > > +
> > > +	c = *(resolution_set_str++);
> > > +	while (c != '\0') {
> > > +		buffer[index++] = c;
> > > +		c = *(resolution_set_str++);
> > > +		if (c == ',') {
> > > +			comma_count++;
> > > +			if (comma_count == set_number || index
> > > >=
> > > max_buf_size) {
> > > +				buffer[index] = '\0';
> > > +				return buffer;
> > > +			}
> > > +			
> > > +			else {
> > > +				/* reset the buffer */
> > > +				index = 0;
> > > +				c = *(resolution_set_str++);
> > > +			}
> > > +		}
> > > +	}
> > > +	
> > > +	buffer[index] = '\0';
> > > +
> > > +	return buffer;
> > > +}
> > > +
> > > +/*
> > > + * Get the number of resolution sets
> > > + */
> > > +
> > > +static int get_set_count(const char *resolution_sets_str)
> > > +{
> > > +	char c;
> > > +	int count;
> > > +	
> > > +	/* Count the number of commas */
> > > +	count = 0;
> > > +	c = *(resolution_sets_str++);
> > > +	while (c != '\0') {
> > > +		if (c == ',') {
> > > +			count++;
> > > +		}
> > > +		c = *(resolution_sets_str++);
> > > +	}
> > > +
> > > +	return count + 1;
> > > +}
> > > +
> > > +/*
> > > + * Returns the width value of a resolution set
> > > + * Example:
> > > + * input: 16000x9000
> > > + * output: 16000
> > > + */
> > > +
> > > +static int get_width(const char *resolution_str)
> > > +{
> > > +	int index;
> > > +	char c;
> > > +	const int max_buf_size = 25;
> > > +	char buffer[max_buf_size];
> > > +	c = *(resolution_str++);
> > > +	index = 0;
> > > +	while (c != 'x' && index < max_buf_size) {
> > > +		buffer[index++] = c;
> > > +		c = *(resolution_str++);
> > > +	}
> > > +	
> > > +	/* Terminate string */
> > > +	buffer[index] = '\0';
> > > +	
> > > +	return atoi(buffer);
> > > +}
> > > +
> > > +/*
> > > + * Returns the height value of a resolution set
> > > + * Example
> > > + * input: 16000x9000
> > > + * output: 9000
> > > + */
> > > +
> > > +static int get_height(const char *resolution_str)
> > > +{
> > > +	int index;
> > > +	char c;
> > > +	const int max_buf_size = 25;
> > > +	char buffer[max_buf_size];
> > > +	
> > > +	/* skip to character after x */
> > > +	while (*resolution_str != 'x') {
> > > +		resolution_str++;
> > > +	}
> > > +	resolution_str++;
> > > +	
> > > +	/* Add digits of the height to the buffer */
> > > +	index = 0;
> > > +	c = *(resolution_str++);
> > > +	while (c != '\0') {
> > > +		buffer[index++] = c;
> > > +		c = *(resolution_str++);
> > > +		if(index >= max_buf_size) {
> > > +			break;
> > > +		}
> > > +	}
> > > +	
> > > +	/* Terminate string */
> > > +	buffer[index] = '\0';
> > > +	
> > > +	return atoi(buffer);
> > > +}
> > > +
> > > +/*
> > > + * Looks in the /chosen node for the value of the resolutions
> > > property
> > > + */
> > > +static int add_user_resolutions(void)
> > > +{	
> > > +	RegEntryID *entry_id;
> > > +	OSErr err;
> > > +	Boolean is_done;
> > > +	void *value;
> > > +	RegPropertyValueSize property_size;
> > > +	int index, res_set_count;
> > > +	char *set_str;
> > > +		
> > > +	#define PROPERTY_NAME "resolutions"
> > > +	#define NODE_PATH "Devices:device-tree:options"
> > > +
> > > +	/* init the entry variable */
> > > +	err = RegistryEntryIDInit(entry_id);
> > > +	if (err != noErr) {
> > > +		lprintf("Error: Failed to init entry variable!
> > > (%d)\n", err);
> > > +		return err;
> > > +	}
> > > +	is_done = false;
> > > +
> > > +	/* Get the entry ID value */
> > > +	err = RegistryCStrEntryLookup(NULL /* start root */,
> > > NODE_PATH,
> > > entry_id);
> > > +	if (err != noErr) {
> > > +		lprintf("RegistryCStrEntryLookup() failure
> > > (%d)\n",
> > > err);
> > > +		return err;
> > > +	}
> > > +
> > > +	/* Get the size of the property */
> > > +	err = RegistryPropertyGetSize(entry_id, PROPERTY_NAME,
> > > &property_size);
> > > +	if (err != noErr) {
> > > +		lprintf("Error: Failed to get property size!
> > > (%d)\n", err);
> > > +		return err;
> > > +	}
> > > +
> > > +	/* allocate memory to the value variable */
> > > +	value = (void *) malloc(property_size);
> > > +	
> > > +	/* Get the value of the property */
> > > +	err = RegistryPropertyGet(entry_id, PROPERTY_NAME,
> > > value,
> > > &property_size);
> > > +	if (err != noErr) {
> > > +		lprintf("Error: Failed to find property value
> > > %s!\n", PROPERTY_NAME);
> > > +		return err;
> > > +	}
> > > +
> > > +	/* Limit the number of resolutions to 20 */
> > > +	#define sane_number_of_resolutions 20
> > > +	res_set_count = get_set_count(value);
> > > +	res_set_count = (res_set_count >
> > > sane_number_of_resolutions
> > > ?
> > > sane_number_of_resolutions : res_set_count);
> > > +
> > > +	/* Add each resolution set */
> > > +	for(index = 0; index < res_set_count; index++) {
> > > +		set_str = get_set(value, index);
> > > +		vModes[index].width = get_width(set_str);
> > > +		vModes[index].height = get_height(set_str);
> > > +		free(set_str);
> > > +	}
> > > +
> > > +	free(value);
> > > +}
> > > +
> > > +/* Returns the number of resolutions */
> > > +static int get_number_of_resolutions()
> > > +{
> > > +	int size_of_array, num_of_resolutions, index;
> > > +	
> > > +	num_of_resolutions = 0;
> > > +	size_of_array = sizeof(vModes) / sizeof(struct vMode);
> > > +	
> > > +	for(index = 0; index < size_of_array; index++)
> > > +	{
> > > +		/* Don't count any place holders */
> > > +		if (vModes[index].width != 0) {
> > > +			num_of_resolutions++;
> > > +		}
> > > +	}
> > > +	
> > > +	return num_of_resolutions;
> > > +}
> > > 
> > >   OSStatus QemuVga_Init(void)
> > >   {
> > >   	UInt16 id, i;
> > >   	UInt32 mem, width, height, depth;
> > > +	
> > > +	add_user_resolutions();
> > > 
> > >   	lprintf("First MMIO read...\n");
> > >   	id = DispiReadW(VBE_DISPI_INDEX_ID);
> > > @@ -183,7 +406,7 @@ OSStatus QemuVga_Init(void)
> > >   		i = 0;
> > >   	}
> > >   	GLOBAL.bootMode = i;
> > > -	GLOBAL.numModes = sizeof(vModes) / sizeof(struct vMode)
> > > - 1;
> > > +	GLOBAL.numModes = get_number_of_resolutions();
> > > 
> > >   	QemuVga_SetMode(GLOBAL.bootMode, depth, 0);
> > > 

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[Alfonso Gamboa]

John,

http://mirror.informatimago.com/next/developer.apple.com/qa/dv/dv43.html
perhaps yields some insight:

*Note:*
An example of this is the video 'ndrv' support in Mac OS X. Mac OS X can
load and run native video drivers from the ROM of PCI video cards. This
allows Mac OS X to provide basic video functionality on any card that
supports traditional Mac OS boot-time video, without having a real Mac OS X
video driver available. In order for this to work your video driver must:

   - link with only the standard libraries (DriverServicesLib,
   NameRegistryLib, VideoServicesLib, and PCILib)
   - not access low-memory globals
   - recognize that it's possible for the logical and physical addresses of
   its PCI hardware to be different, and always access the appropriate
   addresses through the appropriate Name Registry properties.


On Mon, Sep 19, 2016 at 3:44 PM, Benjamin Herrenschmidt <
benh@kernel.crashing.org> wrote:

> On Mon, 2016-09-19 at 08:44 -0400, G 3 wrote:
> > On Sep 19, 2016, at 2:24 AM, Benjamin Herrenschmidt wrote:
> >
> > >
> > > On Sat, 2016-09-17 at 23:31 -0400, G 3 wrote:
> > > >
> > > > Add the ability to add resolutions from the command-line. This
> > > > patch
> > > > works by
> > > > looking for a property called 'resolutions' in the options node
> > > > of
> > > > OpenBIOS.
> > > > If it is found all the resolutions are parsed and loaded.
> > > >
> > > > Example command-line:
> > >
> > > You must not use the C library in the ndrv (malloc, atoi, ...)
> > >
> > > Stick to what's in DriverServicesLib and NameRegistryLib.
> >
> > I originally thought that, but was wrong. Those C library functions
> > work.
>
> No, don't use them. They "seem" to work but bad thing will happen,
> especially on OS X.
>
> Cheers,
> Ben.
>
> >
> > >
> > >
> > > >
> > > > -prom-env
> > > > resolutions=512x342,640x480,800x600,1024x600,1200x700,1440x900
> > > >
> > > > Signed-off-by: John Arbuckle <programmingkidx@gmail.com>
> > > > ---
> > > >   QemuVGADriver/src/QemuVga.c | 227
> > > > ++++++++++++++++++++++++++++++++++
> > > > +++++++++-
> > > >   1 file changed, 225 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/QemuVGADriver/src/QemuVga.c
> > > > b/QemuVGADriver/src/QemuVga.c
> > > > index 4584242..d74fa41 100644
> > > > --- a/QemuVGADriver/src/QemuVga.c
> > > > +++ b/QemuVGADriver/src/QemuVga.c
> > > > @@ -3,6 +3,7 @@
> > > >   #include "DriverQDCalls.h"
> > > >   #include "QemuVga.h"
> > > >   #include <Timer.h>
> > > > +#include <stdlib.h>
> > > >
> > > >   /* List of supported modes */
> > > >   struct vMode {
> > > > @@ -18,7 +19,21 @@ static struct vMode vModes[] =  {
> > > >           { 1600, 1200 },
> > > >           { 1920, 1080 },
> > > >           { 1920, 1200 },
> > > > - { 0,0 }
> > > > +
> > > > + /* The rest are place holders */
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > > + { 0,0 },
> > > >   };
> > > >
> > > >   static void VgaWriteB(UInt16 port, UInt8 val)
> > > > @@ -147,11 +162,219 @@ static InterruptMemberNumber
> > > > PCIInterruptHandler(InterruptSetMember ISTmember,
> > > >   }
> > > >   #endif
> > > >
> > > > +/*
> > > > + * Get the resolution set at the specified index.
> > > > + * The string returned needs to be freed when no longer used.
> > > > + */
> > > > +static char *get_set(const char *resolution_set_str, int
> > > > set_number)
> > > > +{
> > > > + const int max_buf_size = 100;
> > > > + char c, *buffer;
> > > > + int index, comma_count;
> > > > +
> > > > + buffer = (char *) malloc(max_buf_size);
> > > > + comma_count = 0;
> > > > + index = 0;
> > > > + set_number++; /* Makes things easier to understand */
> > > > +
> > > > + c = *(resolution_set_str++);
> > > > + while (c != '\0') {
> > > > +         buffer[index++] = c;
> > > > +         c = *(resolution_set_str++);
> > > > +         if (c == ',') {
> > > > +                 comma_count++;
> > > > +                 if (comma_count == set_number || index
> > > > >=
> > > > max_buf_size) {
> > > > +                         buffer[index] = '\0';
> > > > +                         return buffer;
> > > > +                 }
> > > > +
> > > > +                 else {
> > > > +                         /* reset the buffer */
> > > > +                         index = 0;
> > > > +                         c = *(resolution_set_str++);
> > > > +                 }
> > > > +         }
> > > > + }
> > > > +
> > > > + buffer[index] = '\0';
> > > > +
> > > > + return buffer;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Get the number of resolution sets
> > > > + */
> > > > +
> > > > +static int get_set_count(const char *resolution_sets_str)
> > > > +{
> > > > + char c;
> > > > + int count;
> > > > +
> > > > + /* Count the number of commas */
> > > > + count = 0;
> > > > + c = *(resolution_sets_str++);
> > > > + while (c != '\0') {
> > > > +         if (c == ',') {
> > > > +                 count++;
> > > > +         }
> > > > +         c = *(resolution_sets_str++);
> > > > + }
> > > > +
> > > > + return count + 1;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Returns the width value of a resolution set
> > > > + * Example:
> > > > + * input: 16000x9000
> > > > + * output: 16000
> > > > + */
> > > > +
> > > > +static int get_width(const char *resolution_str)
> > > > +{
> > > > + int index;
> > > > + char c;
> > > > + const int max_buf_size = 25;
> > > > + char buffer[max_buf_size];
> > > > + c = *(resolution_str++);
> > > > + index = 0;
> > > > + while (c != 'x' && index < max_buf_size) {
> > > > +         buffer[index++] = c;
> > > > +         c = *(resolution_str++);
> > > > + }
> > > > +
> > > > + /* Terminate string */
> > > > + buffer[index] = '\0';
> > > > +
> > > > + return atoi(buffer);
> > > > +}
> > > > +
> > > > +/*
> > > > + * Returns the height value of a resolution set
> > > > + * Example
> > > > + * input: 16000x9000
> > > > + * output: 9000
> > > > + */
> > > > +
> > > > +static int get_height(const char *resolution_str)
> > > > +{
> > > > + int index;
> > > > + char c;
> > > > + const int max_buf_size = 25;
> > > > + char buffer[max_buf_size];
> > > > +
> > > > + /* skip to character after x */
> > > > + while (*resolution_str != 'x') {
> > > > +         resolution_str++;
> > > > + }
> > > > + resolution_str++;
> > > > +
> > > > + /* Add digits of the height to the buffer */
> > > > + index = 0;
> > > > + c = *(resolution_str++);
> > > > + while (c != '\0') {
> > > > +         buffer[index++] = c;
> > > > +         c = *(resolution_str++);
> > > > +         if(index >= max_buf_size) {
> > > > +                 break;
> > > > +         }
> > > > + }
> > > > +
> > > > + /* Terminate string */
> > > > + buffer[index] = '\0';
> > > > +
> > > > + return atoi(buffer);
> > > > +}
> > > > +
> > > > +/*
> > > > + * Looks in the /chosen node for the value of the resolutions
> > > > property
> > > > + */
> > > > +static int add_user_resolutions(void)
> > > > +{
> > > > + RegEntryID *entry_id;
> > > > + OSErr err;
> > > > + Boolean is_done;
> > > > + void *value;
> > > > + RegPropertyValueSize property_size;
> > > > + int index, res_set_count;
> > > > + char *set_str;
> > > > +
> > > > + #define PROPERTY_NAME "resolutions"
> > > > + #define NODE_PATH "Devices:device-tree:options"
> > > > +
> > > > + /* init the entry variable */
> > > > + err = RegistryEntryIDInit(entry_id);
> > > > + if (err != noErr) {
> > > > +         lprintf("Error: Failed to init entry variable!
> > > > (%d)\n", err);
> > > > +         return err;
> > > > + }
> > > > + is_done = false;
> > > > +
> > > > + /* Get the entry ID value */
> > > > + err = RegistryCStrEntryLookup(NULL /* start root */,
> > > > NODE_PATH,
> > > > entry_id);
> > > > + if (err != noErr) {
> > > > +         lprintf("RegistryCStrEntryLookup() failure
> > > > (%d)\n",
> > > > err);
> > > > +         return err;
> > > > + }
> > > > +
> > > > + /* Get the size of the property */
> > > > + err = RegistryPropertyGetSize(entry_id, PROPERTY_NAME,
> > > > &property_size);
> > > > + if (err != noErr) {
> > > > +         lprintf("Error: Failed to get property size!
> > > > (%d)\n", err);
> > > > +         return err;
> > > > + }
> > > > +
> > > > + /* allocate memory to the value variable */
> > > > + value = (void *) malloc(property_size);
> > > > +
> > > > + /* Get the value of the property */
> > > > + err = RegistryPropertyGet(entry_id, PROPERTY_NAME,
> > > > value,
> > > > &property_size);
> > > > + if (err != noErr) {
> > > > +         lprintf("Error: Failed to find property value
> > > > %s!\n", PROPERTY_NAME);
> > > > +         return err;
> > > > + }
> > > > +
> > > > + /* Limit the number of resolutions to 20 */
> > > > + #define sane_number_of_resolutions 20
> > > > + res_set_count = get_set_count(value);
> > > > + res_set_count = (res_set_count >
> > > > sane_number_of_resolutions
> > > > ?
> > > > sane_number_of_resolutions : res_set_count);
> > > > +
> > > > + /* Add each resolution set */
> > > > + for(index = 0; index < res_set_count; index++) {
> > > > +         set_str = get_set(value, index);
> > > > +         vModes[index].width = get_width(set_str);
> > > > +         vModes[index].height = get_height(set_str);
> > > > +         free(set_str);
> > > > + }
> > > > +
> > > > + free(value);
> > > > +}
> > > > +
> > > > +/* Returns the number of resolutions */
> > > > +static int get_number_of_resolutions()
> > > > +{
> > > > + int size_of_array, num_of_resolutions, index;
> > > > +
> > > > + num_of_resolutions = 0;
> > > > + size_of_array = sizeof(vModes) / sizeof(struct vMode);
> > > > +
> > > > + for(index = 0; index < size_of_array; index++)
> > > > + {
> > > > +         /* Don't count any place holders */
> > > > +         if (vModes[index].width != 0) {
> > > > +                 num_of_resolutions++;
> > > > +         }
> > > > + }
> > > > +
> > > > + return num_of_resolutions;
> > > > +}
> > > >
> > > >   OSStatus QemuVga_Init(void)
> > > >   {
> > > >           UInt16 id, i;
> > > >           UInt32 mem, width, height, depth;
> > > > +
> > > > + add_user_resolutions();
> > > >
> > > >           lprintf("First MMIO read...\n");
> > > >           id = DispiReadW(VBE_DISPI_INDEX_ID);
> > > > @@ -183,7 +406,7 @@ OSStatus QemuVga_Init(void)
> > > >                   i = 0;
> > > >           }
> > > >           GLOBAL.bootMode = i;
> > > > - GLOBAL.numModes = sizeof(vModes) / sizeof(struct vMode)
> > > > - 1;
> > > > + GLOBAL.numModes = get_number_of_resolutions();
> > > >
> > > >           QemuVga_SetMode(GLOBAL.bootMode, depth, 0);
> > > >
>
>


-- 
Alfonso Gamboa
Gran Turismo Enterprises, Inc.   |   Director, Global Operations
4660 La Jolla Village Drive    -     La Jolla, CA 92122  -  USA
www.gtxent.com  -=-  www.blue-foundation.org -=- profiles.gtxent.com/aag

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[G 3]

On Sep 19, 2016, at 7:56 PM, Alfonso Gamboa wrote:

> John,
>
> http://mirror.informatimago.com/next/developer.apple.com/qa/dv/ 
> dv43.html perhaps yields some insight:
>
> Note:
> An example of this is the video 'ndrv' support in Mac OS X. Mac OS  
> X can load and run native video drivers from the ROM of PCI video  
> cards. This allows Mac OS X to provide basic video functionality on  
> any card that supports traditional Mac OS boot-time video, without  
> having a real Mac OS X video driver available. In order for this to  
> work your video driver must:
>
> link with only the standard libraries (DriverServicesLib,  
> NameRegistryLib, VideoServicesLib, and PCILib)
> not access low-memory globals
> recognize that it's possible for the logical and physical addresses  
> of its PCI hardware to be different, and always access the  
> appropriate addresses through the appropriate Name Registry  
> properties.
>
Thank you very much for this.

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[G 3]

On Sep 19, 2016, at 6:44 PM, Benjamin Herrenschmidt wrote:

> On Mon, 2016-09-19 at 08:44 -0400, G 3 wrote:
>> On Sep 19, 2016, at 2:24 AM, Benjamin Herrenschmidt wrote:
>>
>>>
>>> On Sat, 2016-09-17 at 23:31 -0400, G 3 wrote:
>>>>
>>>> Add the ability to add resolutions from the command-line. This
>>>> patch
>>>> works by
>>>> looking for a property called 'resolutions' in the options node
>>>> of
>>>> OpenBIOS.
>>>> If it is found all the resolutions are parsed and loaded.
>>>>
>>>> Example command-line:
>>>
>>> You must not use the C library in the ndrv (malloc, atoi, ...)
>>>
>>> Stick to what's in DriverServicesLib and NameRegistryLib.
>>
>> I originally thought that, but was wrong. Those C library functions
>> work.
>
> No, don't use them. They "seem" to work but bad thing will happen,
> especially on OS X.
>
> Cheers,
> Ben.

Ok, they have been replaced in my next patch.

Something is wrong with the options node in OpenBIOS. It is  
inaccessible from Mac OS X. When trying to access the options node,  
IORegistryExplorer always crashes. The other nodes are accessible.  
I'm not certain what the problem could be.

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[Benjamin Herrenschmidt]

On Tue, 2016-09-20 at 00:01 -0400, G 3 wrote:
> 
> Something is wrong with the options node in OpenBIOS. It is  
> inaccessible from Mac OS X. When trying to access the options node,  
> IORegistryExplorer always crashes. The other nodes are accessible.  
> I'm not certain what the problem could be.

Try using /chosen instead of /options.

The latter is special, it's nvram stuff, it could be that OS X treats
it specially too.

Cheers,
Ben.

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[G 3]

On Sep 20, 2016, at 8:18 PM, Benjamin Herrenschmidt wrote:

> On Tue, 2016-09-20 at 00:01 -0400, G 3 wrote:
>>
>> Something is wrong with the options node in OpenBIOS. It is
>> inaccessible from Mac OS X. When trying to access the options node,
>> IORegistryExplorer always crashes. The other nodes are accessible.
>> I'm not certain what the problem could be.
>
> Try using /chosen instead of /options.
>
> The latter is special, it's nvram stuff, it could be that OS X treats
> it specially too.
>
> Cheers,
> Ben.


When I use the -prom-env option I can easily add properties to the  
options node.
Can something like this be done with the chosen node?

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[Benjamin Herrenschmidt]

On Tue, 2016-09-20 at 20:35 -0400, G 3 wrote:
> 
> When I use the -prom-env option I can easily add properties to the  
> options node.
> Can something like this be done with the chosen node?

We can make it so. That or we can add code to OpenBIOS to copy the list
of resolutions into the device-node of the VGA device. The latter is
probably the best, that way we can also have all the default ones in
OpenBIOS and remove the hard coded list completely.

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[G 3]

On Sep 20, 2016, at 8:41 PM, Benjamin Herrenschmidt wrote:

> On Tue, 2016-09-20 at 20:35 -0400, G 3 wrote:
>>
>> When I use the -prom-env option I can easily add properties to the
>> options node.
>> Can something like this be done with the chosen node?
>
> We can make it so. That or we can add code to OpenBIOS to copy the  
> list
> of resolutions into the device-node of the VGA device. The latter is
> probably the best, that way we can also have all the default ones in
> OpenBIOS and remove the hard coded list completely.

You really want to remove the included list of resolutions? I was  
thinking about adding a lot more built-in resolutions in another  
patch. A built-in list is very convenient.

As for the options node problem. We need a way to add resolutions  
from the command-line for a Mac OS X guest. Using "-prom-env" isn't  
an option because of some unknown bug. I suppose it is possible to  
fix this bug - if it is a bug. Another idea I have is to add a  
completely different command-line option to QEMU just for  
resolutions. The user could specify resolutions like this: -video- 
resolutions 640x480,1024x768,1280x700.... This feature could place  
the list of resolutions inside a property of the VGA device node. I  
am betting the VGA driver could access this node and parse the list  
of resolutions. So this: qemu -video-resolutions ---> OpenBIOS VGA  
device node ---> VGA driver

Would anyone have another idea they would like to share?

Re: [Qemu-devel] [OpenBIOS] [PATCH] Add resolutions via the command-line

[Alfonso Gamboa]

Perhaps include a base set of resolutions that the actual mac99 machine
supported, implemented in OpenBIOS, and also adding the --video-resolutions
option so the user may add custom entries that would be read by the VGA
device if so desired.  I believe that is the best compromise vs. having a
huge list of resolutions hard coded in QEMU and presented to MacOS and OSX.

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[Benjamin Herrenschmidt]

On Tue, 2016-09-20 at 22:54 -0400, G 3 wrote:
> You really want to remove the included list of resolutions? I was  
> thinking about adding a lot more built-in resolutions in another  
> patch. A built-in list is very convenient.

I mean remove it from the driver and put it in OpenBIOS instead.

Ie. have openbios build the resolution list from the combination of what you put
in the options node and its own built-in list.

> As for the options node problem. We need a way to add resolutions  
> from the command-line for a Mac OS X guest. Using "-prom-env" isn't  
> an option because of some unknown bug.

Maybe, I can look into it later, but the above would fix it nicely.

>  I suppose it is possible to  
> fix this bug - if it is a bug. Another idea I have is to add a  
> completely different command-line option to QEMU just for  
> resolutions. The user could specify resolutions like this: -video- 
> resolutions 640x480,1024x768,1280x700.... This feature could place  
> the list of resolutions inside a property of the VGA device node. I  
> am betting the VGA driver could access this node and parse the list  
> of resolutions. So this: qemu -video-resolutions ---> OpenBIOS VGA  
> device node ---> VGA driver
> 
> Would anyone have another idea they would like to share?

As I said. Have OpenBIOS build the list of resolutions and put it in a
property (in binary, not ASCII form so the driver doesn't have to do
the mess it does now) in the DT node of the device itself.

Cheers,
Ben.

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[G 3]

On Sep 21, 2016, at 5:01 AM, Benjamin Herrenschmidt wrote:

> On Tue, 2016-09-20 at 22:54 -0400, G 3 wrote:
>> You really want to remove the included list of resolutions? I was
>> thinking about adding a lot more built-in resolutions in another
>> patch. A built-in list is very convenient.
>
> I mean remove it from the driver and put it in OpenBIOS instead.
>
> Ie. have openbios build the resolution list from the combination of  
> what you put
> in the options node and its own built-in list.

That idea is shared with others. OpenBIOS might be considered easier  
to alter than the VGA driver, so this might be best.

>
>> As for the options node problem. We need a way to add resolutions
>> from the command-line for a Mac OS X guest. Using "-prom-env" isn't
>> an option because of some unknown bug.
>
> Maybe, I can look into it later, but the above would fix it nicely.

Nodes like chose, aliases, openprom are of class IOService. options  
is of class IODTNVRAM. It looks like this class has problems. I'm  
thinking since Alexander Graf did work in the mac_nvram.c file, he  
might know what is wrong. I found another way to access the options  
node in Mac OS X. Use this command: ioreg -c IODTNVRAM. It stops at  
the options node and prints this error message: "ioreg: error: can't  
obtain properties" in QEMU. On a real Power Mac the command prints  
all the properties of the options node just fine.

>
>>  I suppose it is possible to
>> fix this bug - if it is a bug. Another idea I have is to add a
>> completely different command-line option to QEMU just for
>> resolutions. The user could specify resolutions like this: -video-
>> resolutions 640x480,1024x768,1280x700.... This feature could place
>> the list of resolutions inside a property of the VGA device node. I
>> am betting the VGA driver could access this node and parse the list
>> of resolutions. So this: qemu -video-resolutions ---> OpenBIOS VGA
>> device node ---> VGA driver
>>
>> Would anyone have another idea they would like to share?
>
> As I said. Have OpenBIOS build the list of resolutions and put it in a
> property (in binary, not ASCII form so the driver doesn't have to do
> the mess it does now) in the DT node of the device itself.

I don't know about using binary. The way we do it now seems just fine.

I just need to figure out how to write to the /chosen node from the  
QEMU command line. I found a function called OpenBIOS_set_var() in  
openbios_firmware_abi.h, but it would require an address to write to.  
So where do I find the address of the /chosen node....

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[Benjamin Herrenschmidt]

On Wed, 2016-09-21 at 14:26 -0400, G 3 wrote:
> 
> Nodes like chose, aliases, openprom are of class IOService. options  
> is of class IODTNVRAM. It looks like this class has problems. I'm  
> thinking since Alexander Graf did work in the mac_nvram.c file, he  
> might know what is wrong.

What is wrong is purely that MacOS X limits how you do things on that
node. Have a look at the OS X kernel source :-) The implementation of
the support libraries to run NDRVs is all there (module
IOGraphicsFamily iirc).

>  I found another way to access the options  
> node in Mac OS X. Use this command: ioreg -c IODTNVRAM. It stops at  
> the options node and prints this error message: "ioreg: error: can't  
> obtain properties" in QEMU. On a real Power Mac the command prints  
> all the properties of the options node just fine.

Correct, part of the problem is that I think we don't emulate the NVRAM
in a format that OS X understands, a problem for another day.

> I don't know about using binary. The way we do it now seems just fine.

ASCII parsing in a driver sucks.

> I just need to figure out how to write to the /chosen node from the  
> QEMU command line. I found a function called OpenBIOS_set_var() in  
> openbios_firmware_abi.h, but it would require an address to write to.  
> So where do I find the address of the /chosen node....

No, you don't. Do as I said. Have OpenBIOS construct the list of
resolutions and put it in the node of the device itself.

Cheers,
Ben.

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[G 3]

On Sep 21, 2016, at 5:53 PM, Benjamin Herrenschmidt wrote:

> On Wed, 2016-09-21 at 14:26 -0400, G 3 wrote:
>>
>> Nodes like chose, aliases, openprom are of class IOService. options
>> is of class IODTNVRAM. It looks like this class has problems. I'm
>> thinking since Alexander Graf did work in the mac_nvram.c file, he
>> might know what is wrong.
>
> What is wrong is purely that MacOS X limits how you do things on that
> node. Have a look at the OS X kernel source :-) The implementation of
> the support libraries to run NDRVs is all there (module
> IOGraphicsFamily iirc).

I will check the IOGraphicsFamily kernel extension source code.

>
>>  I found another way to access the options
>> node in Mac OS X. Use this command: ioreg -c IODTNVRAM. It stops at
>> the options node and prints this error message: "ioreg: error: can't
>> obtain properties" in QEMU. On a real Power Mac the command prints
>> all the properties of the options node just fine.
>
> Correct, part of the problem is that I think we don't emulate the  
> NVRAM
> in a format that OS X understands, a problem for another day.

If you know any more info please share. I would like to help fix this  
problem.

>
>> I don't know about using binary. The way we do it now seems just  
>> fine.
>
> ASCII parsing in a driver sucks.

All the code needed parse ASCII is already in the patch. But I will
try to see things your way.

>
>> I just need to figure out how to write to the /chosen node from the
>> QEMU command line. I found a function called OpenBIOS_set_var() in
>> openbios_firmware_abi.h, but it would require an address to write to.
>> So where do I find the address of the /chosen node....
>
> No, you don't. Do as I said. Have OpenBIOS construct the list of
> resolutions and put it in the node of the device itself.

That can be done, but I was hoping to be able to do this via a  
command-line switch.
That would make things so much easier on the user. Altering  
properties in
OpenBIOS can be challenging using forth. OpenBIOS does support using  
startup
scripts so maybe someone could make a script that adds user  
resolutions to the
QEMU,VGA node from the options node.

I'm hoping you had a look at version three of my patch. If there are  
any other issues
with it, I would like to know before making version 4.

Re: [Qemu-devel] [PATCH] Add resolutions via the command-line

[Benjamin Herrenschmidt]

On Wed, 2016-09-21 at 19:18 -0400, G 3 wrote:
> 
> That can be done, but I was hoping to be able to do this via a  
> command-line switch.

But you still do that !

Your switch puts stuff in /options which OpenBIOS then picks up to
construct the final list that it puts in the driver node.

> That would make things so much easier on the user. Altering  
> properties in OpenBIOS can be challenging using forth. OpenBIOS does
> support using  
> startup
> scripts so maybe someone could make a script that adds user  
> resolutions to the
> QEMU,VGA node from the options node.
> 
> I'm hoping you had a look at version three of my patch. If there
> are  
> any other issues
> with it, I would like to know before making version 4.

I'm really not fan of the whole ASCII parsing business. In any case,
I'm a bit too busy right now, I'll try to get back onto this next week.

Re: [Qemu-devel] [OpenBIOS] [PATCH] Add resolutions via the command-line

[Mark Cave-Ayland]

On 21/09/16 22:53, Benjamin Herrenschmidt wrote:

> On Wed, 2016-09-21 at 14:26 -0400, G 3 wrote:
>>
>> Nodes like chose, aliases, openprom are of class IOService. options  
>> is of class IODTNVRAM. It looks like this class has problems. I'm  
>> thinking since Alexander Graf did work in the mac_nvram.c file, he  
>> might know what is wrong.
> 
> What is wrong is purely that MacOS X limits how you do things on that
> node. Have a look at the OS X kernel source :-) The implementation of
> the support libraries to run NDRVs is all there (module
> IOGraphicsFamily iirc).
> 
>>  I found another way to access the options  
>> node in Mac OS X. Use this command: ioreg -c IODTNVRAM. It stops at  
>> the options node and prints this error message: "ioreg: error: can't  
>> obtain properties" in QEMU. On a real Power Mac the command prints  
>> all the properties of the options node just fine.
> 
> Correct, part of the problem is that I think we don't emulate the NVRAM
> in a format that OS X understands, a problem for another day.
> 
>> I don't know about using binary. The way we do it now seems just fine.
> 
> ASCII parsing in a driver sucks.
> 
>> I just need to figure out how to write to the /chosen node from the  
>> QEMU command line. I found a function called OpenBIOS_set_var() in  
>> openbios_firmware_abi.h, but it would require an address to write to.  
>> So where do I find the address of the /chosen node....
> 
> No, you don't. Do as I said. Have OpenBIOS construct the list of
> resolutions and put it in the node of the device itself.

Yeah. My take would be to add a "modes" property to the VGA device which
is a set of encode-int triplets of (width, height, depth) respectively.

Even better, does QEMU provide any preset resolutions via EDID? I'd much
rather get OpenBIOS's VGA to read those and put them in the "modes"
device property, including adding in any custom resolution provided on
the command line via -g wxhxd.


ATB,

Mark.