Hi Daniel, 2017-01-25 Daniel Vetter <daniel.vetter@xxxxxxxx>: > 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@xxxxxxxxxxxxxxx> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxx> Gustavo _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
