[Intel-gfx] [PATCH 01/15] drm/kms-helpers: Use recommened kerneldoc for struct member refs
Gustavo Padovan
gustavo at padovan.org
Wed Jan 25 12:48:53 UTC 2017
Hi Daniel,
2017-01-25 Daniel Vetter <daniel.vetter at ffwll.ch>:
> I just learned that &struct_name.member_name works and looks pretty
> even. It doesn't (yet) link to the member directly though, which would
> be really good for big structures or vfunc tables (where the
> per-member kerneldoc tends to be long).
>
> Also some minor drive-by polish where it makes sense, I read a lot
> of docs ...
>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 97 ++++++++++----------
> drivers/gpu/drm/drm_crtc_helper.c | 28 +++---
> drivers/gpu/drm/drm_dp_helper.c | 2 +-
> drivers/gpu/drm/drm_fb_helper.c | 48 +++++-----
> drivers/gpu/drm/drm_plane_helper.c | 9 +-
> drivers/gpu/drm/drm_probe_helper.c | 14 +--
> include/drm/drm_atomic_helper.h | 13 +--
> include/drm/drm_dp_mst_helper.h | 7 +-
> include/drm/drm_flip_work.h | 2 +-
> include/drm/drm_modeset_helper_vtables.h | 146 ++++++++++++++++---------------
> include/drm/drm_simple_kms_helper.h | 12 +--
> 11 files changed, 197 insertions(+), 181 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 1f0cd7e715c9..5e512dd3a2c4 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -458,22 +458,25 @@ mode_fixup(struct drm_atomic_state *state)
> * Check the state object to see if the requested state is physically possible.
> * This does all the crtc and connector related computations for an atomic
> * update and adds any additional connectors needed for full modesets and calls
> - * down into ->mode_fixup functions of the driver backend.
> - *
> - * crtc_state->mode_changed is set when the input mode is changed.
> - * crtc_state->connectors_changed is set when a connector is added or
> - * removed from the crtc.
> - * crtc_state->active_changed is set when crtc_state->active changes,
> - * which is used for dpms.
> + * down into &drm_crtc_helper_funcs.mode_fixup and
> + * &drm_encoder_helper_funcs.mode_fixup or
> + * &drm_encoder_helper_funcs.atomic_check functions of the driver backend.
> + *
> + * &drm_crtc_state.mode_changed is set when the input mode is changed.
> + * &drm_crtc_state.connectors_changed is set when a connector is added or
> + * removed from the crtc. &drm_crtc_state.active_changed is set when
> + * &drm_crtc_state.active changes, which is used for DPMS.
> * See also: drm_atomic_crtc_needs_modeset()
> *
> * IMPORTANT:
> *
> - * Drivers which set ->mode_changed (e.g. in their ->atomic_check hooks if a
> - * plane update can't be done without a full modeset) _must_ call this function
> - * afterwards after that change. It is permitted to call this function multiple
> - * times for the same update, e.g. when the ->atomic_check functions depend upon
> - * the adjusted dotclock for fifo space allocation and watermark computation.
> + * Drivers which set &drm_crtc_state.mode_changed (e.g. in their
> + * &drm_plane_helper_funcs.atomic_check hooks if a plane update can't be done
> + * without a full modeset) _must_ call this function afterwards after that
> + * change. It is permitted to call this function multiple times for the same
> + * update, e.g. when the &drm_crtc_helper_funcs.atomic_check functions depend
> + * upon the adjusted dotclock for fifo space allocation and watermark
> + * computation.
> *
> * RETURNS:
> * Zero for success or -errno
> @@ -584,9 +587,10 @@ EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
> *
> * Check the state object to see if the requested state is physically possible.
> * This does all the plane update related checks using by calling into the
> - * ->atomic_check hooks provided by the driver.
> + * &drm_crtc_helper_funcs.atomic_check and &drm_plane_helper_funcs.atomic_check
> + * hooks provided by the driver.
> *
> - * It also sets crtc_state->planes_changed to indicate that a crtc has
> + * It also sets &drm_crtc_state.planes_changed to indicate that a crtc has
> * updated planes.
> *
> * RETURNS:
> @@ -648,14 +652,15 @@ EXPORT_SYMBOL(drm_atomic_helper_check_planes);
> * Check the state object to see if the requested state is physically possible.
> * Only crtcs and planes have check callbacks, so for any additional (global)
> * checking that a driver needs it can simply wrap that around this function.
> - * Drivers without such needs can directly use this as their ->atomic_check()
> - * callback.
> + * Drivers without such needs can directly use this as their
> + * &drm_mode_config_funcs.atomic_check callback.
> *
> * This just wraps the two parts of the state checking for planes and modeset
> * state in the default order: First it calls drm_atomic_helper_check_modeset()
> * and then drm_atomic_helper_check_planes(). The assumption is that the
> - * ->atomic_check functions depend upon an updated adjusted_mode.clock to
> - * e.g. properly compute watermarks.
> + * @drm_plane_helper_funcs.atomic_check and @drm_crtc_helper_funcs.atomic_check
> + * functions depend upon an updated adjusted_mode.clock to e.g. properly compute
> + * watermarks.
> *
> * RETURNS:
> * Zero for success or -errno
> @@ -1125,8 +1130,8 @@ EXPORT_SYMBOL(drm_atomic_helper_wait_for_vblanks);
> * drm_atomic_helper_commit_tail - commit atomic update to hardware
> * @old_state: atomic state object with old state structures
> *
> - * This is the default implemenation for the ->atomic_commit_tail() hook of the
> - * &drm_mode_config_helper_funcs vtable.
> + * This is the default implemenation for the
> + * &drm_mode_config_helper_funcs.atomic_commit_tail hook.
implementation
> *
> * Note that the default ordering of how the various stages are called is to
> * match the legacy modeset helper library closest. One peculiarity of that is
> @@ -1203,8 +1208,8 @@ static void commit_work(struct work_struct *work)
> * drm_atomic_helper_setup_commit() and related functions.
> *
> * Committing the actual hardware state is done through the
> - * ->atomic_commit_tail() callback of the &drm_mode_config_helper_funcs vtable,
> - * or it's default implementation drm_atomic_helper_commit_tail().
> + * &drm_mode_config_helper_funcs.atomic_commit_tail callback, or it's default
> + * implementation drm_atomic_helper_commit_tail().
> *
> * RETURNS:
> * Zero for success or -errno.
> @@ -1373,14 +1378,15 @@ static void release_crtc_commit(struct completion *completion)
> *
> * This function prepares @state to be used by the atomic helper's support for
> * nonblocking commits. Drivers using the nonblocking commit infrastructure
> - * should always call this function from their ->atomic_commit hook.
> + * should always call this function from their
> + * &drm_mode_config_funcs.atomic_commit hook.
> *
> * To be able to use this support drivers need to use a few more helper
> * functions. drm_atomic_helper_wait_for_dependencies() must be called before
> * actually committing the hardware state, and for nonblocking commits this call
> * must be placed in the async worker. See also drm_atomic_helper_swap_state()
> * and it's stall parameter, for when a driver's commit hooks look at the
> - * ->state pointers of &struct drm_crtc, &drm_plane or &drm_connector directly.
> + * &drm_crtc.state, &drm_plane.state or &drm_connector.state pointer directly.
> *
> * Completion of the hardware commit step must be signalled using
> * drm_atomic_helper_commit_hw_done(). After this step the driver is not allowed
> @@ -1489,8 +1495,7 @@ static struct drm_crtc_commit *preceeding_commit(struct drm_crtc *crtc)
> * This function waits for all preceeding commits that touch the same CRTC as
> * @old_state to both be committed to the hardware (as signalled by
> * drm_atomic_helper_commit_hw_done) and executed by the hardware (as signalled
> - * by calling drm_crtc_vblank_send_event on the event member of
> - * &drm_crtc_state).
> + * by calling drm_crtc_vblank_send_event on the &drm_crtc_state.event).
drm_crtc_vblank_send_event()
> *
> * This is part of the atomic helper support for nonblocking commits, see
> * drm_atomic_helper_setup_commit() for an overview.
> @@ -1627,8 +1632,9 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_cleanup_done);
> * @state: atomic state object with new state structures
> *
> * This function prepares plane state, specifically framebuffers, for the new
> - * configuration. If any failure is encountered this function will call
> - * ->cleanup_fb on any already successfully prepared framebuffer.
> + * configuration, by calling &drm_plane_helper_funcs.prepare_fb. If any failure
> + * is encountered this function will call &drm_plane_helper_funcs.cleanup_fb on
> + * any already successfully prepared framebuffer.
> *
> * Returns:
> * 0 on success, negative error code on failure.
> @@ -1708,10 +1714,10 @@ static bool plane_crtc_active(const struct drm_plane_state *state)
> *
> * Drivers may set the NO_DISABLE_AFTER_MODESET flag in @flags if the relevant
> * display controllers require to disable a CRTC's planes when the CRTC is
> - * disabled. This function would skip the ->atomic_disable call for a plane if
> - * the CRTC of the old plane state needs a modesetting operation. Of course,
> - * the drivers need to disable the planes in their CRTC disable callbacks
> - * since no one else would do that.
> + * disabled. This function would skip the &drm_plane_helper_funcs.atomic_disable
> + * call for a plane if the CRTC of the old plane state needs a modesetting
> + * operation. Of course, the drivers need to disable the planes in their CRTC
> + * disable callbacks since no one else would do that.
> *
> * The drm_atomic_helper_commit() default implementation doesn't set the
> * ACTIVE_ONLY flag to most closely match the behaviour of the legacy helpers.
> @@ -1874,7 +1880,7 @@ EXPORT_SYMBOL(drm_atomic_helper_commit_planes_on_crtc);
> * planes.
> *
> * It is a bug to call this function without having implemented the
> - * ->atomic_disable() plane hook.
> + * &drm_plane_helper_funcs.atomic_disable plane hook.
> */
> void
> drm_atomic_helper_disable_planes_on_crtc(struct drm_crtc_state *old_crtc_state,
> @@ -1961,8 +1967,8 @@ EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
> * contains the old state. Also do any other cleanup required with that state.
> *
> * @stall must be set when nonblocking commits for this driver directly access
> - * the ->state pointer of &drm_plane, &drm_crtc or &drm_connector. With the
> - * current atomic helpers this is almost always the case, since the helpers
> + * the &drm_plane.state, &drm_crtc.state or &drm_connector.state pointer. With
> + * the current atomic helpers this is almost always the case, since the helpers
> * don't pass the right state structures to the callbacks.
> */
> void drm_atomic_helper_swap_state(struct drm_atomic_state *state,
> @@ -2892,8 +2898,8 @@ EXPORT_SYMBOL(drm_atomic_helper_page_flip_target);
> *
> * This is the main helper function provided by the atomic helper framework for
> * implementing the legacy DPMS connector interface. It computes the new desired
> - * ->active state for the corresponding CRTC (if the connector is enabled) and
> - * updates it.
> + * &drm_crtc_state.active state for the corresponding CRTC (if the connector is
> + * enabled) and updates it.
> *
> * Returns:
> * Returns 0 on success, negative errno numbers on failure.
> @@ -2965,11 +2971,11 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector,
> EXPORT_SYMBOL(drm_atomic_helper_connector_dpms);
>
> /**
> - * drm_atomic_helper_best_encoder - Helper for &drm_connector_helper_funcs
> - * ->best_encoder callback
> + * drm_atomic_helper_best_encoder - Helper for
> + * &drm_connector_helper_funcs.best_encoder callback
> * @connector: Connector control structure
> *
> - * This is a &drm_connector_helper_funcs ->best_encoder callback helper for
> + * This is a &drm_connector_helper_funcs.best_encoder callback helper for
> * connectors that support exactly 1 encoder, statically determined at driver
> * init time.
> */
> @@ -3003,7 +3009,7 @@ EXPORT_SYMBOL(drm_atomic_helper_best_encoder);
> */
>
> /**
> - * drm_atomic_helper_crtc_reset - default ->reset hook for CRTCs
> + * drm_atomic_helper_crtc_reset - default &drm_crtc_funcs.reset hook for CRTCs
> * @crtc: drm CRTC
> *
> * Resets the atomic state for @crtc by freeing the state pointer (which might
> @@ -3110,7 +3116,7 @@ void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
> EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
>
> /**
> - * drm_atomic_helper_plane_reset - default ->reset hook for planes
> + * drm_atomic_helper_plane_reset - default &drm_plane_funcs.reset hook for planes
> * @plane: drm plane
> *
> * Resets the atomic state for @plane by freeing the state pointer (which might
> @@ -3214,8 +3220,9 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state);
> * @conn_state: connector state to assign
> *
> * Initializes the newly allocated @conn_state and assigns it to
> - * #connector ->state, usually required when initializing the drivers
> - * or when called from the ->reset hook.
> + * the &drm_conector->state pointer of @connector, usually required when
> + * initializing the drivers or when called from the &drm_connector_funcs.reset
> + * hook.
> *
> * This is useful for drivers that subclass the connector state.
> */
> @@ -3231,7 +3238,7 @@ __drm_atomic_helper_connector_reset(struct drm_connector *connector,
> EXPORT_SYMBOL(__drm_atomic_helper_connector_reset);
>
> /**
> - * drm_atomic_helper_connector_reset - default ->reset hook for connectors
> + * drm_atomic_helper_connector_reset - default &drm_connector_funcs.reset hook for connectors
> * @connector: drm connector
> *
> * Resets the atomic state for @connector by freeing the state pointer (which
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 1e281dd42e4b..8c1e4d93a4f4 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -53,9 +53,9 @@
> * configuration on resume with drm_helper_resume_force_mode().
> *
> * Note that this helper library doesn't track the current power state of CRTCs
> - * and encoders. It can call callbacks like ->dpms() even though the hardware is
> - * already in the desired state. This deficiency has been fixed in the atomic
> - * helpers.
> + * and encoders. It can call callbacks like &drm_encoder_helper_funcs.dpms even
> + * though the hardware is already in the desired state. This deficiency has been
> + * fixed in the atomic helpers.
> *
> * The driver callbacks are mostly compatible with the atomic modeset helpers,
> * except for the handling of the primary plane: Atomic helpers require that the
> @@ -477,12 +477,12 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
> * drm_crtc_helper_set_config - set a new config from userspace
> * @set: mode set configuration
> *
> - * The drm_crtc_helper_set_config() helper function implements the set_config
> - * callback of &struct drm_crtc_funcs for drivers using the legacy CRTC helpers.
> + * The drm_crtc_helper_set_config() helper function implements the of
> + * &drm_crtc_funcs.set_config callback for drivers using the legacy CRTC
> + * helpers.
> *
> * It first tries to locate the best encoder for each connector by calling the
> - * connector ->best_encoder() (&struct drm_connector_helper_funcs) helper
> - * operation.
> + * connector @drm_connector_helper_funcs.best_encoder helper operation.
> *
> * After locating the appropriate encoders, the helper function will call the
> * mode_fixup encoder and CRTC helper operations to adjust the requested mode,
> @@ -493,8 +493,7 @@ drm_crtc_helper_disable(struct drm_crtc *crtc)
> *
> * If the adjusted mode is identical to the current mode but changes to the
> * frame buffer need to be applied, the drm_crtc_helper_set_config() function
> - * will call the CRTC ->mode_set_base() (&struct drm_crtc_helper_funcs) helper
> - * operation.
> + * will call the CRTC &drm_crtc_helper_funcs.mode_set_base helper operation.
> *
> * If the adjusted mode differs from the current mode, or if the
> * ->mode_set_base() helper operation is not provided, the helper function
> @@ -851,14 +850,15 @@ static int drm_helper_choose_crtc_dpms(struct drm_crtc *crtc)
> * @connector: affected connector
> * @mode: DPMS mode
> *
> - * The drm_helper_connector_dpms() helper function implements the ->dpms()
> - * callback of &struct drm_connector_funcs for drivers using the legacy CRTC helpers.
> + * The drm_helper_connector_dpms() helper function implements the
> + * &drm_connector_funcs.dpms() callback for drivers using the legacy CRTC
> + * helpers.
You didn't use () in any of the others.
Otherwise looks good to me:
Rewiewed-by: Gustavo Padovan <gustavo.padovan at collabora.com>
Gustavo
More information about the dri-devel
mailing list