Hi Sam,
Thank you for the patch.
On Tue, Mar 31, 2020 at 09:53:18AM +0200, Daniel Vetter wrote:
On Sat, Mar 28, 2020 at 02:20:23PM +0100, Sam Ravnborg wrote:
Document the callbacks: drm_connector_helper_funcs.prepare_writeback_job drm_connector_helper_funcs.cleanup_writeback_job
The documentation was pulled from the changelong introducing the callbacks, originally written by Laurent.
Addign the missing documentation fixes the following warnings: drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'prepare_writeback_job' not described in 'drm_connector_helper_funcs' drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'cleanup_writeback_job' not described in 'drm_connector_helper_funcs'
Signed-off-by: Sam Ravnborg sam@ravnborg.org Cc: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com Cc: Liviu Dudau liviu.dudau@arm.com Cc: Daniel Vetter daniel.vetter@ffwll.ch Cc: Maarten Lankhorst maarten.lankhorst@linux.intel.com Cc: Maxime Ripard mripard@kernel.org Cc: Thomas Zimmermann tzimmermann@suse.de Cc: David Airlie airlied@linux.ie
include/drm/drm_modeset_helper_vtables.h | 31 ++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 7c20b1c8b6a7..c51bca1ffec7 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -1075,8 +1075,39 @@ struct drm_connector_helper_funcs { void (*atomic_commit)(struct drm_connector *connector, struct drm_connector_state *state);
- /**
* @prepare_writeback_job:Formatting looks funny, your linebreaks here won't go into the generated html and are a bit unusual. I'd remove them and just flow this as a full paragraph.
** As writeback jobs contain a framebuffer, drivers may need to* prepare and cleanup them the same way they can prepare and
"cleanup them" or "clean them up" ?
* cleanup framebuffers for planes.
This would be "clean up" too.
* This optional connector operation is used to support the* preparation of writeback jobs.* The job prepare operation is called from* drm_atomic_helper_prepare_planes() to avoid a new atomic commit* helper that would need to be called by all drivers not using* drm_atomic_helper_commit().I'd delete "to avoid a new ..." until the end of the sentence. That feels more like stuff in the commit message/review than kernel docs for driver writers.
Instead maybe add "... for struct &drm_writeback_connector connectors only." This gives us a nice link to the writeback docs, and makes it clear that this isn't some general prep/cleanup thing. Similar addition below.
** This hook is optional.** This callback is used by the atomic modeset helpers.
"hook" or "callback", you decide, but let's be consistent :-) I'd go for "operation" personally as that's what is used above. Same for the cleanup_writeback_job operation below.
*/- /**
* @cleanup_writeback_job:** This optional connector operation is used to support the* cleanup of writeback jobs.* The job cleanup operation is called from the existing* drm_writeback_cleanup_job() function, invoked both when* destroying the job as part of a aborted commit, or when
s/a aborted/an aborted/
* the job completes.** This hook is optional.** This callback is used by the atomic modeset helpers.*/With the bikesheds addressed as you see fit:
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
Reviewed-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
Also Laurent owes you one, I've been pestering to fill this gap in his docs since forever ...
I do. Sorry for letting you fix it.
};