From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH 07/28] drm: Update drm_plane_funcs kerneldoc Date: Mon, 7 Dec 2015 12:46:38 +0100 Message-ID: <20151207114638.GJ13177@ulmo> References: <1449218769-16577-1-git-send-email-daniel.vetter@ffwll.ch> <1449218769-16577-8-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1357959890==" Return-path: In-Reply-To: <1449218769-16577-8-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: Daniel Vetter , Intel Graphics Development , DRI Development List-Id: intel-gfx@lists.freedesktop.org --===============1357959890== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="gBdJBemW82xJqIAr" Content-Disposition: inline --gBdJBemW82xJqIAr Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Dec 04, 2015 at 09:45:48AM +0100, Daniel Vetter wrote: > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h [...] > + /** > + * @destroy: > + * > + * Clean up CRTC resources. This is only called at driver unload time Perhaps drop "only" because there are more than a single potential usage? > + /** > + * @set_property: > + * > + * This is the legacy entry point to update a property attach to the "attached to" > - /* atomic update handling */ > + /** > + * @atomic_duplicate_state: > + * > + * Duplicate the current atomic state for this CRTC and return it. > + * The core and helpers gurantee that any atomic state duplicated with > + * this hook and still owned by the caller (i.e. not transferred to the > + * driver by calling ->atomic_commit() from struct > + * &drm_mode_config_funcs) will be cleaned up by calling the > + * @atomic_destroy_state hook in this structure. > + * > + * Atomic drivers which don't subclass struct &drm_crtc should use > + * drm_atomic_helper_crtc_duplicate_state(). Drivers that subclass the > + * state structure to extend it with driver-private state should use > + * __drm_atomic_helper_crtc_duplicate_state() to make sure shared state is > + * duplicated in a consisten fashion across drivers. "consistent" > + /** > + * @atomic_set_property: > + * > + * Decode a driver-private property value and store the decoded value > + * into the passed-in state structure. Since the atomic core decodes all > + * standardized properties (even for extensions beyond the core set of > + * properties which might not be implemented by all drivers) this > + * requires drivers to subclass the state structure. > + * > + * Such driver-private properties should really only implemented for "be implemented" > + * This function is called in the state assembly phase of atomic > + * modesets, which can be aborted for any reason (including on > + * userspace's request to just check whether a configuration would be > + * possible). Drivers MUST NOT touch any persistent state (hardware or > + * software) or data structures except the passed in adjusted_mode Copy-paste error? Should be "... the passed in @state parameter." > + * > + * Also since userspace controls in which order properties are set this > + * function must not do any input validation (since the state update is > + * incompletely and hence likely inconsistent). Instead any such input "incomplete" > + * validation must be done in the various atomic_check callbacks. > + * > + * RETURNS: > + * > + * 0 if the property has been found, -EINVAL if the property isn't > + * implemented by the driver (which shouldn't ever happen, the core only s/shouldn't ever/should never/? > + * asks for properties attached to this CRTC). No other Seems like you could put more text on the above line. > + * validation is allowed by the driver. The core checks that the > + * property value is within the range (integer, valid enum value, ...) > + * the driver set when registering the property already. I'd put the "already" after "The core", otherwise it reads weird in my opinion. > + */ > int (*atomic_set_property)(struct drm_crtc *crtc, > struct drm_crtc_state *state, > struct drm_property *property, > uint64_t val); > + /** > + * @atomic_get_property: > + * > + * Reads out the decoded driver-private property. This is used to > + * implement the GETCRTC ioctl. > + * > + * Do not call this function directly, use > + * drm_atomic_crtc_get_property() instead. > + * > + * This callback is optional if the driver does not support any > + * driver-private atomic properties. > + * > + * RETURNS: > + * > + * 0 on success, -EINVAL if ther property isn't implemented by the s/ther/the/ > + * driver (which shouldn't ever happen, the core only asks for s/shouldn't ever/should never/? > + * properties attached on this CRTC). "attached to" > @@ -539,19 +662,142 @@ struct drm_connector_funcs { > enum drm_connector_status (*detect)(struct drm_connector *connector, > bool force); > int (*fill_modes)(struct drm_connector *connector, uint32_t max_width, uint32_t max_height); > + > + /** > + * @set_property: > + * > + * This is the legacy entry point to update a property attach to the "attached to" > + /** > + * @destroy: > + * > + * Clean up connector resources. This is only called at driver unload time > + * through drm_mode_config_cleanup(), but can also be called at runtime > + * when a connector is being hot-unplugged. Again, perhaps drop the "only" because it's inconsistent when followed by "but can also". Most of the comments on the CRTC helpers do apply to the connector helpers as well, so I haven't repeated them. Thierry --gBdJBemW82xJqIAr Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJWZXGbAAoJEN0jrNd/PrOhnqAP/2dcpLMxJt4uZhZOzc+KrcJt vbg0F6KHRgJaSRDWqx+OWUeAG64xf3Zqbyd1LgAfdhKtcxudfClwLihfGIoFSvef cIm8ohCw7izTDM9+6RYTCiclXkhHO1AxNRuLlrWRvg01aBx/RSqezksO38tLe82m Dw0Vl3R2AyHGUTRclLnRaIw4JMteBfXiV6YXcA3CmKTuiOqfiX3geS2g7Yn5PYIJ xqMZIt1sRovX/z5Jda9q87iEyAu3jib+WB7gjkY/XYVEmI9zekU/Kr2J3Okm9v5K No22CTf6aywkzyTrfD1sPc9kj11beDEp3V7bvZlbxU2ndmPirVvOXcOEp/WCSpzH 5WZDh43aR2yK1r2rRHgqwNI8dkKf+MBpPlLnxlVut227UrsOBDE9WZBPWJ9yF5kH RcIFiBYo2cQBG3zm7wAj5jPnPhL3zw75+pTLPGp4ego1ocY61q/JTPsqhSgRzVQV 77pSxe2dH76PY5izyDphgPSUG294o1UBBtfWTNhLKHJboskHtJDGXmx2XJjJCJXE 4La+gK70cm+4WRAIw2++L4uAz1yr8UlFdahH9Dda/axhR/x+MNi8hloESzi0rjaz Ee0Up/0RB1ZB3Bxr8W6MVaS/8sop0P2Bw+naxKiUqJ2PUKEphXai9/OKKYPNWedE k1VZkCjaT8xp6RdLjVgm =xJpS -----END PGP SIGNATURE----- --gBdJBemW82xJqIAr-- --===============1357959890== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============1357959890==--