Re: [PATCH] xserver: add monitor Option ZoomModes

2013-03-26 Thread Keith Packard 
Aaron Plattner aplatt...@nvidia.com writes:

  The revised patch v2 is here below.  Servaas.
 Thanks for making these changes.  Version 2 looks good to me:

Merged.
   2967391..ac4c2ab  master - master

-- 
keith.pack...@intel.com


pgpGsU8ZJL6TG.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] xserver: add monitor Option ZoomModes

2013-03-19 Thread Aaron Plattner 

On 03/07/13 07:07, v...@picaros.org wrote:

On 11/21/2012 04:12 AM, v...@picaros.org wrote:

Add support for the Option ZoomModes in a monitor section:

Section Monitor
Identifier a21inch
Option PreferredMode 1600x1200
Option ZoomModes 1600x1200 1280x1024 1280x1024 640x480
EndSection


ZoomModes seems like an unfortunate name to me, but there's precedent
for it in the DontZoom option so I won't object to it too strongly.


Well, I settled on ZoomModes since xserver/hw/xfree86 functions with
'Zoom' in their name relate to the Ctrl+Alt+Keypad-{Plus,Minus} mode
switch.  The 'Modes' part comes from the Section Screen/Subsection
Display/Modes statement.


That's fair.


diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
index 5d92bbe..729d301 100644
--- a/hw/xfree86/man/xorg.conf.man
+++ b/hw/xfree86/man/xorg.conf.man
@@ -1676,6 +1676,16 @@ This optional entry specifies a mode to be marked as the 
preferred initial mode
   of the monitor.
   (RandR 1.2-supporting drivers only)
   .TP 7
+.BI Option \*qZoomModes\*q \*q name   name   ... \*q
+This optional entry specifies modes to be marked as zoom modes.
+It is possible to switch to the next and previous mode via
+.BR Ctrl+Alt+Keypad\-Plus  and  Ctrl+Alt+Keypad\-Minus .
+All these keypad available modes are selected from the screen mode
+list.  This list is a copy of the compatibility output monitor mode


roff requires each sentence to begin on its own line, though you can add
additional line breaks in the middle of sentences for readability.


+list.  Since this output is the output connected to the lowest
+dot\-area monitor, as determined from its largest size mode, that


Should this be a hyphen ('-') rather than a literal dash ('\-')?


+monitor defines the available zoom modes.


This only applies to RandR 1.2 drivers, so it should probably get the
RandR 1.2-supporting drivers only text.


Indeed, text modified per suggestions, thank you.


diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index 154f684..2e46885 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c



@@ -1424,6 +1426,88 @@ preferredMode(ScrnInfoPtr pScrn, xf86OutputPtr output)
   return preferred_mode;
   }

+/** identify a token
+ * args
+ *   *src a string with zero or more tokens, e.g. tok0 tok1,
+ *   **token  stores a pointer to the first token character,
+ *   *len stores the token length.
+ * return
+ *   a pointer into src[] at the token terminating character, or
+ *   NULL if no token is found.
+ */
+static const char *
+gettoken(const char *src, const char **token, int *len)
+{
+const char *next, *delim =  \t;
+int skip;
+
+if (!src)
+return NULL;
+
+skip = strspn(src, delim);
+*token = src[skip];
+
+*len = strcspn(*token, delim);
+/* Support for backslash escaped delimiters could be implemented
+ * here.
+ */
+
+/* (*token)[0] != '\0'  ==  *len  0 */
+next = *len  0 ? (*token)[*len] : NULL;


This would probably be clearer written

if (*len  0)
return (*token)[*len];
else
return NULL;


+
+return next;
+}


Agreed, code changed per suggestion.


It seems surprising that there isn't already a function to do this.


There used to be a function in xf86-video-ati/ to process the
Option MetaModes for the driver's internal multiple output support.

I checked xserver/hw/xfree86/{ common, parser} but couldn't find
anything reusable.  An option statement with a list value, for example

   Option ZoomModes 1600x1200 1280x1024 1280x1024 640x480

would be ideal.  It's almost there: in common/xf86Opt.h ValueUnion
add a pointer to a list, add support in parser/Flag.c
xf86parseOption() for list values and extend common/xf86Option.c
ParseOptionValue().

But this added complexity doesn't seem worthwhile for a single Option
user.  So I settled for a string of mode names and a gettoken()
function.


Sounds fine.


+/** Check for a user configured zoom mode list, Option ZoomModes:
+ *
+ * Section Monitor
+ *   Identifier a21inch
+ *   Option ZoomModes 1600x1200 1280x1024 1280x1024 640x480
+ * EndSection
+ *
+ * Each user mode name is searched for independently so the list
+ * specification order is free.  An output mode is matched at most
+ * once, a mode with an already set M_T_USERDEF type bit is skipped.
+ * Thus a repeat mode name specificaton matches the next output mode.


s/specificaton/specification/

This took me a few reads to make sense.  Maybe add with the same name
to the end of this sentence?


Indeed, text amended per suggestion, thank you.


+ * Ctrl+Alt+Keypad-{Plus,Minus} zooms {in,out} by selecting the
+ * {next,previous} M_T_USERDEF mode in the screen modes list, itself
+ * sorted toward lower dot area or lower dot clock frequency, see
+ *   modes/xf86Crtc.c: xf86SortModes() xf86SetScrnInfoModes(), and
+ *   common/xf86Cursor.c: xf86ZoomViewport().
+ */
+static int
+processZoomModes(xf86OutputPtr output)
+{
+

Re: [PATCH] xserver: add monitor Option ZoomModes

2013-03-18 Thread Keith Packard 
v...@picaros.org writes:


 The revised patch v2 is here below.  Servaas.

Aaron -- you reviewed the previous version of this patch; any chance you
might read through this second version and see if it looks reasonable now?

-- 
keith.pack...@intel.com


pgpYpv0HUIHTT.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] xserver: add monitor Option ZoomModes

2013-03-07 Thread vdb 
 On 11/21/2012 04:12 AM, v...@picaros.org wrote:
  Add support for the Option ZoomModes in a monitor section:
 
  Section Monitor
 Identifier a21inch
 Option PreferredMode 1600x1200
 Option ZoomModes 1600x1200 1280x1024 1280x1024 640x480
  EndSection
 
 ZoomModes seems like an unfortunate name to me, but there's precedent 
 for it in the DontZoom option so I won't object to it too strongly.

Well, I settled on ZoomModes since xserver/hw/xfree86 functions with 
'Zoom' in their name relate to the Ctrl+Alt+Keypad-{Plus,Minus} mode 
switch.  The 'Modes' part comes from the Section Screen/Subsection 
Display/Modes statement.  

  diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
  index 5d92bbe..729d301 100644
  --- a/hw/xfree86/man/xorg.conf.man
  +++ b/hw/xfree86/man/xorg.conf.man
  @@ -1676,6 +1676,16 @@ This optional entry specifies a mode to be marked as 
  the preferred initial mode
of the monitor.
(RandR 1.2-supporting drivers only)
.TP 7
  +.BI Option \*qZoomModes\*q \*q name   name   ... \*q
  +This optional entry specifies modes to be marked as zoom modes.
  +It is possible to switch to the next and previous mode via
  +.BR Ctrl+Alt+Keypad\-Plus  and  Ctrl+Alt+Keypad\-Minus .
  +All these keypad available modes are selected from the screen mode
  +list.  This list is a copy of the compatibility output monitor mode
 
 roff requires each sentence to begin on its own line, though you can add 
 additional line breaks in the middle of sentences for readability.
 
  +list.  Since this output is the output connected to the lowest
  +dot\-area monitor, as determined from its largest size mode, that
 
 Should this be a hyphen ('-') rather than a literal dash ('\-')?
 
  +monitor defines the available zoom modes.
 
 This only applies to RandR 1.2 drivers, so it should probably get the 
 RandR 1.2-supporting drivers only text.

Indeed, text modified per suggestions, thank you.

  diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
  index 154f684..2e46885 100644
  --- a/hw/xfree86/modes/xf86Crtc.c
  +++ b/hw/xfree86/modes/xf86Crtc.c

  @@ -1424,6 +1426,88 @@ preferredMode(ScrnInfoPtr pScrn, xf86OutputPtr 
  output)
return preferred_mode;
}
 
  +/** identify a token
  + * args
  + *   *src a string with zero or more tokens, e.g. tok0 tok1,
  + *   **token  stores a pointer to the first token character,
  + *   *len stores the token length.
  + * return
  + *   a pointer into src[] at the token terminating character, or
  + *   NULL if no token is found.
  + */
  +static const char *
  +gettoken(const char *src, const char **token, int *len)
  +{
  +const char *next, *delim =  \t;
  +int skip;
  +
  +if (!src)
  +return NULL;
  +
  +skip = strspn(src, delim);
  +*token = src[skip];
  +
  +*len = strcspn(*token, delim);
  +/* Support for backslash escaped delimiters could be implemented
  + * here.
  + */
  +
  +/* (*token)[0] != '\0'  ==  *len  0 */
  +next = *len  0 ? (*token)[*len] : NULL;
 
 This would probably be clearer written
 
if (*len  0)
return (*token)[*len];
else
return NULL;
 
  +
  +return next;
  +}

Agreed, code changed per suggestion.

 It seems surprising that there isn't already a function to do this.

There used to be a function in xf86-video-ati/ to process the
Option MetaModes for the driver's internal multiple output support.  

I checked xserver/hw/xfree86/{ common, parser} but couldn't find 
anything reusable.  An option statement with a list value, for example

  Option ZoomModes 1600x1200 1280x1024 1280x1024 640x480

would be ideal.  It's almost there: in common/xf86Opt.h ValueUnion 
add a pointer to a list, add support in parser/Flag.c
xf86parseOption() for list values and extend common/xf86Option.c
ParseOptionValue().

But this added complexity doesn't seem worthwhile for a single Option 
user.  So I settled for a string of mode names and a gettoken() 
function.  

  +/** Check for a user configured zoom mode list, Option ZoomModes:
  + *
  + * Section Monitor
  + *   Identifier a21inch
  + *   Option ZoomModes 1600x1200 1280x1024 1280x1024 640x480
  + * EndSection
  + *
  + * Each user mode name is searched for independently so the list
  + * specification order is free.  An output mode is matched at most
  + * once, a mode with an already set M_T_USERDEF type bit is skipped.
  + * Thus a repeat mode name specificaton matches the next output mode.
 
 s/specificaton/specification/
 
 This took me a few reads to make sense.  Maybe add with the same name 
 to the end of this sentence?

Indeed, text amended per suggestion, thank you.

  + * Ctrl+Alt+Keypad-{Plus,Minus} zooms {in,out} by selecting the
  + * {next,previous} M_T_USERDEF mode in the screen modes list, itself
  + * sorted toward lower dot area or lower dot clock frequency, see
  + *   modes/xf86Crtc.c: xf86SortModes() xf86SetScrnInfoModes(), and
  + *

Re: [PATCH] xserver: add monitor Option ZoomModes

2013-03-04 Thread Aaron Plattner 

On 11/21/2012 04:12 AM, v...@picaros.org wrote:

Add support for the Option ZoomModes in a monitor section:

Section Monitor
   Identifier a21inch
   Option PreferredMode 1600x1200
   Option ZoomModes 1600x1200 1280x1024 1280x1024 640x480
EndSection


ZoomModes seems like an unfortunate name to me, but there's precedent 
for it in the DontZoom option so I won't object to it too strongly.



The option's effect is to search for and mark once each named mode in
the output modes list.  So the specification order is free and the zoom
modes sequence follows the order of the output modes list.  All marked
modes are available via the Ctrl+Alt+Keypad-{Plus,Minus} key
combination.

See also http://bugs.freedesktop.org/show_bug.cgi?id=17954.

This option has its use for combined monitor and television setups.
It allows for easy switching between 60 Hz and 50 Hz modes even when a
monitor refuses to display the input signal.

Is there any support for this enhancement ? The patch is here below.

Signed-off-by: Servaas Vandenberghe v...@picaros.org
---
  hw/xfree86/man/xorg.conf.man |   10 +
  hw/xfree86/modes/xf86Crtc.c  |   87 ++
  2 files changed, 97 insertions(+), 0 deletions(-)

diff --git a/hw/xfree86/man/xorg.conf.man b/hw/xfree86/man/xorg.conf.man
index 5d92bbe..729d301 100644
--- a/hw/xfree86/man/xorg.conf.man
+++ b/hw/xfree86/man/xorg.conf.man
@@ -1676,6 +1676,16 @@ This optional entry specifies a mode to be marked as the 
preferred initial mode
  of the monitor.
  (RandR 1.2-supporting drivers only)
  .TP 7
+.BI Option \*qZoomModes\*q \*q name   name   ... \*q
+This optional entry specifies modes to be marked as zoom modes.
+It is possible to switch to the next and previous mode via
+.BR Ctrl+Alt+Keypad\-Plus  and  Ctrl+Alt+Keypad\-Minus .
+All these keypad available modes are selected from the screen mode
+list.  This list is a copy of the compatibility output monitor mode


roff requires each sentence to begin on its own line, though you can add 
additional line breaks in the middle of sentences for readability.



+list.  Since this output is the output connected to the lowest
+dot\-area monitor, as determined from its largest size mode, that


Should this be a hyphen ('-') rather than a literal dash ('\-')?


+monitor defines the available zoom modes.


This only applies to RandR 1.2 drivers, so it should probably get the 
RandR 1.2-supporting drivers only text.



+.TP 7
  .BI Option \*qPosition\*q \*q x   y \*q
  This optional entry specifies the position of the monitor within the X
  screen.
diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index 154f684..2e46885 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c
@@ -427,6 +427,7 @@ extern XF86ConfigPtr xf86configptr;

  typedef enum {
  OPTION_PREFERRED_MODE,
+OPTION_ZOOM_MODES,
  OPTION_POSITION,
  OPTION_BELOW,
  OPTION_RIGHT_OF,
@@ -445,6 +446,7 @@ typedef enum {

  static OptionInfoRec xf86OutputOptions[] = {
  {OPTION_PREFERRED_MODE, PreferredMode, OPTV_STRING, {0}, FALSE},
+{OPTION_ZOOM_MODES, ZoomModes, OPTV_STRING, {0}, FALSE },
  {OPTION_POSITION, Position, OPTV_STRING, {0}, FALSE},
  {OPTION_BELOW, Below, OPTV_STRING, {0}, FALSE},
  {OPTION_RIGHT_OF, RightOf, OPTV_STRING, {0}, FALSE},
@@ -1424,6 +1426,88 @@ preferredMode(ScrnInfoPtr pScrn, xf86OutputPtr output)
  return preferred_mode;
  }

+/** identify a token
+ * args
+ *   *src a string with zero or more tokens, e.g. tok0 tok1,
+ *   **token  stores a pointer to the first token character,
+ *   *len stores the token length.
+ * return
+ *   a pointer into src[] at the token terminating character, or
+ *   NULL if no token is found.
+ */
+static const char *
+gettoken(const char *src, const char **token, int *len)
+{
+const char *next, *delim =  \t;
+int skip;
+
+if (!src)
+return NULL;
+
+skip = strspn(src, delim);
+*token = src[skip];
+
+*len = strcspn(*token, delim);
+/* Support for backslash escaped delimiters could be implemented
+ * here.
+ */
+
+/* (*token)[0] != '\0'  ==  *len  0 */
+next = *len  0 ? (*token)[*len] : NULL;


This would probably be clearer written

  if (*len  0)
  return (*token)[*len];
  else
  return NULL;


+
+return next;
+}


It seems surprising that there isn't already a function to do this.


+/** Check for a user configured zoom mode list, Option ZoomModes:
+ *
+ * Section Monitor
+ *   Identifier a21inch
+ *   Option ZoomModes 1600x1200 1280x1024 1280x1024 640x480
+ * EndSection
+ *
+ * Each user mode name is searched for independently so the list
+ * specification order is free.  An output mode is matched at most
+ * once, a mode with an already set M_T_USERDEF type bit is skipped.
+ * Thus a repeat mode name specificaton matches the next output mode.


s/specificaton/specification/

This took me a few reads to make sense.  Maybe add with the same