From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 26/28] drm: Move drm_display_mode an related docs into kerneldoc Date: Mon, 7 Dec 2015 16:02:38 +0100 Message-ID: <20151207150238.GG13177@ulmo> References: <1449218769-16577-1-git-send-email-daniel.vetter@ffwll.ch> <1449218769-16577-27-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1957671691==" Return-path: In-Reply-To: <1449218769-16577-27-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: Daniel Vetter , Intel Graphics Development , DRI Development List-Id: dri-devel@lists.freedesktop.org --===============1957671691== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="6bKodGeYF53agbop" Content-Disposition: inline --6bKodGeYF53agbop Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 { > =20 > #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF > =20 > +/** > + * 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 diag= ram: > + * > + * > + * Active Front Sync B= ack > + * Region Porch Po= rch > + * <-----------------------><----------------><-------------><------= --------> > + * > + * //////////////////////| > + * ////////////////////// | > + * ////////////////////// |.................. .......= =2E........ > + * _______________ > + * > + * <----- [hv]display -----> > + * <------------- [hv]sync_start ------------> > + * <--------------------- [hv]sync_end ---------------------> > + * <-------------------------------- [hv]total ---------------------= -------->* > + * > + * This structure contains two copies of timings. First are the plain ti= mings, > + * 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; > =20 > + /** > + * @name: > + * > + * Human-readable name of the mode, filled out with drm_mode_set_name(). > + */ > char name[DRM_DISPLAY_MODE_LEN]; > =20 > + /** > + * @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; > =20 > - /* 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; > =20 > - /* 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; > =20 > - 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 --6bKodGeYF53agbop Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJWZZ+LAAoJEN0jrNd/PrOhMKoQAKK9maaqtEiwxrCgjNdKfdZX 8t2cGpXM4cjLjgAllYP8wbt4V3gj+2uQwWB+WSHn3XHE6+ZVKpuQYCOTBUVkZqwb n96eYtZdfRqRPPp2LdIGGmHARGCtO3DZNZ11SCvHkRvI6zMyRHXAXNDm0dfj+PBj ilq0AippAJPoMDjc1b3EFuiry1tGPUagYl1Gwj7HnJVTrxUrcTbSS++PtUNH1lWP la89y/DFAH+luX8wKnkT96G50hjOIIrB4I/uG1Jy+0DWlDpc0+IffGNZ9bLsJjpA BTrgWpxBAQlE19O5QFWMqGvaH8A/FuBb0HU/9KT0pOYocfc4Ms5i1G1nWaCUQBDu TURdCtbMHN+BNZOE6jxTNGym/m5YD0aC+7c1pM/NjWlwHOiYK6jvWWGvbyp5+PqE 7LTYnwt2frggQs/qIjBA1+MOptPCvSrYZ+lR/73uH4ljHogZNYAVwDVBySdHNXQL FhuYJmrguKIfX/9ucacR+vOPjvhYn0NWToRzga3o5pqQoa41O0P8/jHQ+gOFKi4m ZqEEAg+lvYdFtau6baQby9ihDzIycZoghXBrBFD3T9El0eFKJzytyH9RRRWNXFJv qJEl5YkC3xNcyE1ddmSBRKX9l1FQiovWutUXJHWDN/JX9XCumWu5EPi4ARTifiu8 JO7708hm9eyOXWg14QQT =Nr8u -----END PGP SIGNATURE----- --6bKodGeYF53agbop-- --===============1957671691== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK --===============1957671691==--