On Fri, Dec 04, 2015 at 09:46:07AM +0100, Daniel Vetter wrote:
> This was in the documentation for modeset helper hooks, where it is a
> bit misplaced.
> 
> v2: Reindent the drm_mode_status enum, inspired by Ville.
> 
> Cc: ville.syrj...@linux.intel.com
> Signed-off-by: Daniel Vetter <daniel.vet...@intel.com>
> ---
>  Documentation/DocBook/gpu.tmpl | 192 -----------------------
>  drivers/gpu/drm/drm_modes.c    |   3 +-
>  include/drm/drm_modes.h        | 342 
> +++++++++++++++++++++++++++++++++++------
>  3 files changed, 297 insertions(+), 240 deletions(-)
> 
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index 5312f5a05798..86e5b12a49ba 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -1758,198 +1758,6 @@ void intel_crt_init(struct drm_device *dev)
>              <function>drm_add_edid_modes</function> manually in that case.
>            </para>
>            <para>
> -            When adding modes manually the driver creates each mode with a 
> call to
> -            <function>drm_mode_create</function> and must fill the following 
> fields.
> -            <itemizedlist>
> -              <listitem>
> -                <synopsis>__u32 type;</synopsis>
> -                <para>
> -                  Mode type bitmask, a combination of
> -                  <variablelist>
> -                    <varlistentry>
> -                      <term>DRM_MODE_TYPE_BUILTIN</term>
> -                      <listitem><para>not used?</para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_TYPE_CLOCK_C</term>
> -                      <listitem><para>not used?</para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_TYPE_CRTC_C</term>
> -                      <listitem><para>not used?</para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>
> -        DRM_MODE_TYPE_PREFERRED - The preferred mode for the connector
> -                      </term>
> -                      <listitem>
> -                        <para>not used?</para>
> -                      </listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_TYPE_DEFAULT</term>
> -                      <listitem><para>not used?</para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_TYPE_USERDEF</term>
> -                      <listitem><para>not used?</para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_TYPE_DRIVER</term>
> -                      <listitem>
> -                        <para>
> -                          The mode has been created by the driver (as 
> opposed to
> -                          to user-created modes).
> -                        </para>
> -                      </listitem>
> -                    </varlistentry>
> -                  </variablelist>
> -                  Drivers must set the DRM_MODE_TYPE_DRIVER bit for all 
> modes they
> -                  create, and set the DRM_MODE_TYPE_PREFERRED bit for the 
> preferred
> -                  mode.
> -                </para>
> -              </listitem>
> -              <listitem>
> -                <synopsis>__u32 clock;</synopsis>
> -                <para>Pixel clock frequency in kHz unit</para>
> -              </listitem>
> -              <listitem>
> -                <synopsis>__u16 hdisplay, hsync_start, hsync_end, htotal;
> -    __u16 vdisplay, vsync_start, vsync_end, vtotal;</synopsis>
> -                <para>Horizontal and vertical timing information</para>
> -                <screen><![CDATA[
> -             Active                 Front           Sync           Back
> -             Region                 Porch                          Porch
> -    
> <-----------------------><----------------><-------------><-------------->
> -
> -      //////////////////////|
> -     ////////////////////// |
> -    //////////////////////  |..................               
> ................
> -                                               _______________
> -
> -    <----- [hv]display ----->
> -    <------------- [hv]sync_start ------------>
> -    <--------------------- [hv]sync_end --------------------->
> -    <-------------------------------- [hv]total 
> ----------------------------->
> -]]></screen>
> -              </listitem>
> -              <listitem>
> -                <synopsis>__u16 hskew;
> -    __u16 vscan;</synopsis>
> -                <para>Unknown</para>
> -              </listitem>
> -              <listitem>
> -                <synopsis>__u32 flags;</synopsis>
> -                <para>
> -                  Mode flags, a combination of
> -                  <variablelist>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_PHSYNC</term>
> -                      <listitem><para>
> -                        Horizontal sync is active high
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_NHSYNC</term>
> -                      <listitem><para>
> -                        Horizontal sync is active low
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_PVSYNC</term>
> -                      <listitem><para>
> -                        Vertical sync is active high
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_NVSYNC</term>
> -                      <listitem><para>
> -                        Vertical sync is active low
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_INTERLACE</term>
> -                      <listitem><para>
> -                        Mode is interlaced
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_DBLSCAN</term>
> -                      <listitem><para>
> -                        Mode uses doublescan
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_CSYNC</term>
> -                      <listitem><para>
> -                        Mode uses composite sync
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_PCSYNC</term>
> -                      <listitem><para>
> -                        Composite sync is active high
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_NCSYNC</term>
> -                      <listitem><para>
> -                        Composite sync is active low
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_HSKEW</term>
> -                      <listitem><para>
> -                        hskew provided (not used?)
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_BCAST</term>
> -                      <listitem><para>
> -                        not used?
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_PIXMUX</term>
> -                      <listitem><para>
> -                        not used?
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_DBLCLK</term>
> -                      <listitem><para>
> -                        not used?
> -                      </para></listitem>
> -                    </varlistentry>
> -                    <varlistentry>
> -                      <term>DRM_MODE_FLAG_CLKDIV2</term>
> -                      <listitem><para>
> -                        ?
> -                      </para></listitem>
> -                    </varlistentry>
> -                  </variablelist>
> -                </para>
> -                <para>
> -                  Note that modes marked with the INTERLACE or DBLSCAN flags 
> will be
> -                  filtered out by
> -                  
> <function>drm_helper_probe_single_connector_modes</function> if
> -                  the connector's 
> <structfield>interlace_allowed</structfield> or
> -                  <structfield>doublescan_allowed</structfield> field is set 
> to 0.
> -                </para>
> -              </listitem>
> -              <listitem>
> -                <synopsis>char name[DRM_DISPLAY_MODE_LEN];</synopsis>
> -                <para>
> -                  Mode name. The driver must call
> -                  <function>drm_mode_set_name</function> to fill the mode 
> name from
> -                  <structfield>hdisplay</structfield>,
> -                  <structfield>vdisplay</structfield> and interlace flag 
> after
> -                  filling the corresponding fields.
> -                </para>
> -              </listitem>
> -            </itemizedlist>
> -          </para>
> -          <para>
>              The <structfield>vrefresh</structfield> value is computed by
>              <function>drm_helper_probe_single_connector_modes</function>.
>            </para>
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index 5f79b9334a38..041138f5acc9 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -708,7 +708,8 @@ void drm_mode_set_name(struct drm_display_mode *mode)
>  }
>  EXPORT_SYMBOL(drm_mode_set_name);
>  
> -/** drm_mode_hsync - get the hsync of a mode
> +/**
> + * drm_mode_hsync - get the hsync of a mode
>   * @mode: mode
>   *
>   * Returns:
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index f9115aee43f4..cd3c42256ab8 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -35,46 +35,91 @@
>   * structures).
>   */
>  
> +/**
> + * enum drm_mode_status - hardware support status of a mode
> + * @MODE_OK: Mode OK
> + * @MODE_HSYNC: hsync out of range
> + * @MODE_VSYNC: vsync out of range
> + * @MODE_H_ILLEGAL: mode has illegal horizontal timings
> + * @MODE_V_ILLEGAL: mode has illegal horizontal timings
> + * @MODE_BAD_WIDTH: requires an unsupported linepitch
> + * @MODE_NOMODE: no mode with a matching name
> + * @MODE_NO_INTERLACE: interlaced mode not supported
> + * @MODE_NO_DBLESCAN: doublescan mode not supported
> + * @MODE_NO_VSCAN: multiscan mode not supported
> + * @MODE_MEM: insufficient video memory
> + * @MODE_VIRTUAL_X: mode width too large for specified virtual size
> + * @MODE_VIRTUAL_Y: mode height too large for specified virtual size
> + * @MODE_MEM_VIRT: insufficient video memory given virtual size
> + * @MODE_NOCLOCK: no fixed clock available
> + * @MODE_CLOCK_HIGH: clock required is too high
> + * @MODE_CLOCK_LOW: clock required is too low
> + * @MODE_CLOCK_RANGE: clock/mode isn't in a ClockRange
> + * @MODE_BAD_HVALUE: horizontal timing was out of range
> + * @MODE_BAD_VVALUE: vertical timing was out of range
> + * @MODE_BAD_VSCAN: VScan value out of range
> + * @MODE_HSYNC_NARROW: horizontal sync too narrow
> + * @MODE_HSYNC_WIDE: horizontal sync too wide
> + * @MODE_HBLANK_NARROW: horizontal blanking too narrow
> + * @MODE_HBLANK_WIDE: horizontal blanking too wide
> + * @MODE_VSYNC_NARROW: vertical sync too narrow
> + * @MODE_VSYNC_WIDE: vertical sync too wide
> + * @MODE_VBLANK_NARROW: vertical blanking too narrow
> + * @MODE_VBLANK_WIDE: vertical blanking too wide
> + * @MODE_PANEL: exceeds panel dimensions
> + * @MODE_INTERLACE_WIDTH: width too large for interlaced mode
> + * @MODE_ONE_WIDTH: only one width is supported
> + * @MODE_ONE_HEIGHT: only one height is supported
> + * @MODE_ONE_SIZE: only one resolution is supported
> + * @MODE_NO_REDUCED: monitor doesn't accept reduced blanking
> + * @MODE_NO_STEREO: stereo modes not supported
> + * @MODE_UNVERIFIED: mode needs to reverified
> + * @MODE_BAD: unspecified reason
> + * @MODE_ERROR: error condition
> + *
> + * This enum is used to filter out modes not supported by the driver/hardware
> + * combination.
> + */
>  enum drm_mode_status {
> -    MODE_OK  = 0,    /* Mode OK */
> -    MODE_HSYNC,              /* hsync out of range */
> -    MODE_VSYNC,              /* vsync out of range */
> -    MODE_H_ILLEGAL,  /* mode has illegal horizontal timings */
> -    MODE_V_ILLEGAL,  /* mode has illegal horizontal timings */
> -    MODE_BAD_WIDTH,  /* requires an unsupported linepitch */
> -    MODE_NOMODE,     /* no mode with a matching name */
> -    MODE_NO_INTERLACE,       /* interlaced mode not supported */
> -    MODE_NO_DBLESCAN,        /* doublescan mode not supported */
> -    MODE_NO_VSCAN,   /* multiscan mode not supported */
> -    MODE_MEM,                /* insufficient video memory */
> -    MODE_VIRTUAL_X,  /* mode width too large for specified virtual size */
> -    MODE_VIRTUAL_Y,  /* mode height too large for specified virtual size */
> -    MODE_MEM_VIRT,   /* insufficient video memory given virtual size */
> -    MODE_NOCLOCK,    /* no fixed clock available */
> -    MODE_CLOCK_HIGH, /* clock required is too high */
> -    MODE_CLOCK_LOW,  /* clock required is too low */
> -    MODE_CLOCK_RANGE,        /* clock/mode isn't in a ClockRange */
> -    MODE_BAD_HVALUE, /* horizontal timing was out of range */
> -    MODE_BAD_VVALUE, /* vertical timing was out of range */
> -    MODE_BAD_VSCAN,  /* VScan value out of range */
> -    MODE_HSYNC_NARROW,       /* horizontal sync too narrow */
> -    MODE_HSYNC_WIDE, /* horizontal sync too wide */
> -    MODE_HBLANK_NARROW,      /* horizontal blanking too narrow */
> -    MODE_HBLANK_WIDE,        /* horizontal blanking too wide */
> -    MODE_VSYNC_NARROW,       /* vertical sync too narrow */
> -    MODE_VSYNC_WIDE, /* vertical sync too wide */
> -    MODE_VBLANK_NARROW,      /* vertical blanking too narrow */
> -    MODE_VBLANK_WIDE,        /* vertical blanking too wide */
> -    MODE_PANEL,         /* exceeds panel dimensions */
> -    MODE_INTERLACE_WIDTH, /* width too large for interlaced mode */
> -    MODE_ONE_WIDTH,     /* only one width is supported */
> -    MODE_ONE_HEIGHT,    /* only one height is supported */
> -    MODE_ONE_SIZE,      /* only one resolution is supported */
> -    MODE_NO_REDUCED,    /* monitor doesn't accept reduced blanking */
> -    MODE_NO_STEREO,  /* stereo modes not supported */
> -    MODE_UNVERIFIED = -3, /* mode needs to reverified */
> -    MODE_BAD = -2,   /* unspecified reason */
> -    MODE_ERROR       = -1    /* error condition */
> +     MODE_OK = 0,
                 ^
Stray tab.

> +     MODE_HSYNC,
> +     MODE_VSYNC,
> +     MODE_H_ILLEGAL,
> +     MODE_V_ILLEGAL,
> +     MODE_BAD_WIDTH,
> +     MODE_NOMODE,
> +     MODE_NO_INTERLACE,
> +     MODE_NO_DBLESCAN,
> +     MODE_NO_VSCAN,
> +     MODE_MEM,
> +     MODE_VIRTUAL_X,
> +     MODE_VIRTUAL_Y,
> +     MODE_MEM_VIRT,
> +     MODE_NOCLOCK,
> +     MODE_CLOCK_HIGH,
> +     MODE_CLOCK_LOW,
> +     MODE_CLOCK_RANGE,
> +     MODE_BAD_HVALUE,
> +     MODE_BAD_VVALUE,
> +     MODE_BAD_VSCAN,
> +     MODE_HSYNC_NARROW,
> +     MODE_HSYNC_WIDE,
> +     MODE_HBLANK_NARROW,
> +     MODE_HBLANK_WIDE,
> +     MODE_VSYNC_NARROW,
> +     MODE_VSYNC_WIDE,
> +     MODE_VBLANK_NARROW,
> +     MODE_VBLANK_WIDE,
> +     MODE_PANEL,
> +     MODE_INTERLACE_WIDTH,
> +     MODE_ONE_WIDTH,
> +     MODE_ONE_HEIGHT,
> +     MODE_ONE_SIZE,
> +     MODE_NO_REDUCED,
> +     MODE_NO_STEREO,
> +     MODE_UNVERIFIED = -3,
> +     MODE_BAD = -2,
> +     MODE_ERROR      = -1
                  ^
and here too

>  };
>  
>  #define DRM_MODE_TYPE_CLOCK_CRTC_C (DRM_MODE_TYPE_CLOCK_C | \
> @@ -96,17 +141,124 @@ enum drm_mode_status {
>  
>  #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF
>  
> +/**
> + * struct drm_display_mode - DRM kernel-internal display mode structure
> + * @hdisplay: horizontal display size
> + * @hsync_start: horizontal sync start
> + * @hsync_end: horizontal sync end
> + * @htotal: horizontal total size
> + * @hskew: horizontal skew?!
> + * @vdisplay: vertical display size
> + * @vsync_start: vertical sync start
> + * @vsync_end: vertical sync end
> + * @vtotal: vertical total size
> + * @vscan: vertical scan?!
> + * @crtc_hdisplay: hardware modehorizontal display size
> + * @crtc_hsync_start: hardware modehorizontal sync start
> + * @crtc_hsync_end: hardware modehorizontal sync end
> + * @crtc_htotal: hardware modehorizontal total size
> + * @crtc_hskew: hardware modehorizontal skew?!
> + * @crtc_vdisplay: hardware modevertical display size
> + * @crtc_vsync_start: hardware modevertical sync start
> + * @crtc_vsync_end: hardware modevertical sync end
> + * @crtc_vtotal: hardware modevertical total size
> + * @crtc_vscan: hardware modevertical scan?!
> + *
> + * The horizontal an vertical timings are defined per the following diagram:
> + *
> + *
> + *               Active                 Front           Sync           Back
> + *              Region                 Porch                          Porch
> + *     
> <-----------------------><----------------><-------------><-------------->
> + *
> + *       //////////////////////|
> + *      ////////////////////// |
> + *     //////////////////////  |..................               
> ................
> + *                                                _______________
> + *
> + *     <----- [hv]display ----->
> + *     <------------- [hv]sync_start ------------>
> + *     <--------------------- [hv]sync_end --------------------->
> + *     <-------------------------------- [hv]total 
> ----------------------------->*
> + *
> + * This structure contains two copies of timings. First are the plain 
> timings,
> + * which specify the logical mode, as it would be for a progressive 1:1 
> scanout
> + * at the refresh rate userspace can observe through vblank timestamps. Then
> + * there's the hardware timings, which are corrected for interlacing,
> + * double-clocking and similar things. They are provided as a convience, and 
> can
> + * be appropriately computed using drm_mode_set_crtcinfo().
> + */
>  struct drm_display_mode {
> -     /* Header */
> +     /**
> +      * @head:
> +      *
> +      * struct list_head for mode lists.
> +      */
>       struct list_head head;
> +
> +     /**
> +      * @base:
> +      *
> +      * A display mode is a normal modeset object, possibly including public
> +      * userspace id.
> +      *
> +      * FIXME:
> +      *
> +      * This can probably be removed since the entire concept of userspace
> +      * managing modes explicitly hasn't ever landed in upstream kernel mode
> +      * setting support.
> +      */
>       struct drm_mode_object base;
>  
> +     /**
> +      * @name:
> +      *
> +      * Human-readable name of the mode, filled out with drm_mode_set_name().
> +      */
>       char name[DRM_DISPLAY_MODE_LEN];
>  
> +     /**
> +      * @status:
> +      *
> +      * Status of the mode, used to filter out modes not supported by the
> +      * hardware. See enum &drm_mode_status.
> +      */
>       enum drm_mode_status status;
> +
> +     /**
> +      * @type:
> +      *
> +      * A bitmask of flags, mostly about the source of a mode. Possible flags
> +      * are:
> +      *
> +      *  - DRM_MODE_TYPE_BUILTIN: Meant for hard-coded modes, effectively
> +      *    unused.
> +      *  - DRM_MODE_TYPE_PREFERRED: Preferred mode, usually the native
> +      *    resolution of an LCD panel. There should only be one preferred
> +      *    mode per connector at any given time.
> +      *  - DRM_MODE_TYPE_DRIVER: Mode created by the driver, which is all of
> +      *    them really. Drivers must set this bit for all modes they create
> +      *    and expose to userspace.
> +      *
> +      * Plus a big list of flags which shouldn't be used at all, but are
> +      * still around since these flags are also used in the userspace ABI:
> +      *
> +      *  - DRM_MODE_TYPE_DEFAULT: Again a leftover, use
> +      *    DRM_MODE_TYPE_PREFERRED instead.
> +      *  - DRM_MODE_TYPE_CLOCK_C and DRM_MODE_TYPE_CRTC_C: Define leftovers
> +      *    which are stuck around for hysterical raisins only. No one has an
> +      *    idea what they where meant for. Don't use.
> +      *  - DRM_MODE_TYPE_USERDEF: Mode defined by userspace, again a vestige
> +      *    from older kms designs where userspace had to first add a custom
> +      *    mode to the kernel's mode list before it could use it. Don't use.
> +      */
>       unsigned int type;
>  
> -     /* Proposed mode values */
> +     /**
> +      * @clock:
> +      *
> +      * Pixel clock in kHz.
> +      */
>       int clock;              /* in kHz */
>       int hdisplay;
>       int hsync_start;
> @@ -118,14 +270,74 @@ struct drm_display_mode {
>       int vsync_end;
>       int vtotal;
>       int vscan;
> +     /**
> +      * @flags:
> +      *
> +      * Sync and timing flags:
> +      *
> +      *  - DRM_MODE_FLAG_PHSYNC: horizontal sync is active high.
> +      *  - DRM_MODE_FLAG_NHSYNC: horizontal sync is active low.
> +      *  - DRM_MODE_FLAG_PVSYNC: vertical sync is active high.
> +      *  - DRM_MODE_FLAG_NVSYNC: vertical sync is active low.
> +      *  - DRM_MODE_FLAG_INTERLACE: mode is interlaced.
> +      *  - DRM_MODE_FLAG_DBLSCAN: mode uses doublescan.
> +      *  - DRM_MODE_FLAG_CSYNC: mode uses composite sync.
> +      *  - DRM_MODE_FLAG_PCSYNC: composite sync is active high.
> +      *  - DRM_MODE_FLAG_NCSYNC: composite sync is active low.
> +      *  - DRM_MODE_FLAG_HSKEW: hskew provided (not used?).
> +      *  - DRM_MODE_FLAG_BCAST: not used?
> +      *  - DRM_MODE_FLAG_PIXMUX: not used?
> +      *  - DRM_MODE_FLAG_DBLCLK: double-clocked mode.
> +      *  - DRM_MODE_FLAG_CLKDIV2: half-clocked mode.

Random rant: How I wish we didn't have have bunch of these flags.
No real reason why userspace should have to know how the pixels
get clocked out etc. But can't change that now.

> +      *
> +      * Additionally there's flags to specify how 3D modes are packed:
> +      *
> +      *  - DRM_MODE_FLAG_3D_NONE: normal, non-3D mode.
> +      *  - DRM_MODE_FLAG_3D_FRAME_PACKING: 2 full frames for left and right.
> +      *  - DRM_MODE_FLAG_3D_FIELD_ALTERNATIVE: interleaved like fields.
> +      *  - DRM_MODE_FLAG_3D_LINE_ALTERNATIVE: interleaved lines.
> +      *  - DRM_MODE_FLAG_3D_SIDE_BY_SIDE_FULL: side-by-side full frames.
> +      *  - DRM_MODE_FLAG_3D_L_DEPTH: ?
> +      *  - DRM_MODE_FLAG_3D_L_DEPTH_GFX_GFX_DEPTH: ?
> +      *  - DRM_MODE_FLAG_3D_TOP_AND_BOTTOM: frame split into top and bottom
> +      *    parts.
> +      *  - DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF: frame split into left and
> +      *    right parts.

I think these either need a proper explanation (ie. some diagrams probably),
or we just add a spec reference here. Can be done later once we figure
out which way we want to go.

Anyway, patch looks mostly OK, so
Reviewed-by: Ville Syrjälä <ville.syrj...@linux.intel.com>

> +      */
>       unsigned int flags;
>  
> -     /* Addressable image size (may be 0 for projectors, etc.) */
> +     /**
> +      * @width_mm:
> +      *
> +      * Addressable size of the output in mm, projectors should set this to
> +      * 0.
> +      */
>       int width_mm;
> +
> +     /**
> +      * @height_mm:
> +      *
> +      * Addressable size of the output in mm, projectors should set this to
> +      * 0.
> +      */
>       int height_mm;
>  
> -     /* Actual mode we give to hw */
> -     int crtc_clock;         /* in KHz */
> +     /**
> +      * @crtc_clock:
> +      *
> +      * Actual pixel or dot clock in the hardware. This differs from the
> +      * logical @clock when e.g. using interlacing, double-clocking, stereo
> +      * modes or other fancy stuff that changes the timings and signals
> +      * actually sent over the wire.
> +      *
> +      * This is again in kHz.
> +      *
> +      * Note that with digital outputs like HDMI or DP there's usually a
> +      * massive confusion between the dot clock and the signal clock at the
> +      * bit encoding level. Especially when a 8b/10b encoding is used and the
> +      * differences is exactly a factor of 10.
> +      */
> +     int crtc_clock;
>       int crtc_hdisplay;
>       int crtc_hblank_start;
>       int crtc_hblank_end;
> @@ -140,12 +352,48 @@ struct drm_display_mode {
>       int crtc_vsync_end;
>       int crtc_vtotal;
>  
> -     /* Driver private mode info */
> +     /**
> +      * @private:
> +      *
> +      * Pointer for driver private data. This can only be used for mode
> +      * objects passed to drivers in modeset operations. It shouldn't be used
> +      * by atomic drivers since they can store any additional data by
> +      * subclassing state structures.
> +      */
>       int *private;
> +
> +     /**
> +      * @private_flags:
> +      *
> +      * Similar to @private, but just an integer.
> +      */
>       int private_flags;
>  
> -     int vrefresh;           /* in Hz */
> -     int hsync;              /* in kHz */
> +     /**
> +      * @vrefresh:
> +      *
> +      * Vertical refresh rate, for debug output in human readable form. Not
> +      * used in a functional way.
> +      *
> +      * This value is in Hz.
> +      */
> +     int vrefresh;
> +
> +     /**
> +      * @hsync:
> +      *
> +      * Horizontal refresh rate, for debug output in human readable form. Not
> +      * used in a functional way.
> +      *
> +      * This value is in kHz.
> +      */
> +     int hsync;
> +
> +     /**
> +      * @picture_aspect_ratio:
> +      *
> +      * Filed for setting the HDMI picture aspect ratio of a mode.
> +      */
>       enum hdmi_picture_aspect picture_aspect_ratio;
>  };
>  
> -- 
> 2.5.1

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to