[Intel-gfx] [PATCH 26/28] drm: Move drm_display_mode an related docs into kerneldoc
Thierry Reding
thierry.reding at gmail.com
Mon Dec 7 07:02:38 PST 2015
On Fri, Dec 04, 2015 at 09:46:07AM +0100, Daniel Vetter wrote:
[...]
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
[...]
> @@ -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
[...]
> + * @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?!
These are missing a space between "mode" and the description.
> + *
> + * 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
"convenience"
> + * 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
"has never"
> + * 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.
"were"
> + * - 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;
I'm thinking that these could all use "unsigned", but that's definitely
something for a separate patch.
> + /**
> + * @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.
> + *
> + * 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: ?
^
Stray tab.
> + * 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.
"difference"
> + */
> + 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;
Off-topic: Any reasons why this is int * and not void *?
> +
> + /**
> + * @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.
"Field".
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20151207/398427ab/attachment-0001.sig>
More information about the Intel-gfx
mailing list