| Message ID | 1359308536-22124-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Daniel, Thanks for improving the documentation :-) On Sunday 27 January 2013 18:42:16 Daniel Vetter wrote: > Now that the fbdev helper interface for drivers is trimmed down, > update the kerneldoc for all the remaining exported functions. > > I've tried to beat the DocBook a bit by reordering the function > references a bit into a more sensible ordering. But that didn't work > out at all. Hence just extend the in-code DOC: section a bit. > > Also remove the LOCKING: sections - especially for the setup functions > they're totally bogus. But that's not a documentation problem, but > simply an artifact of the current rather hazardous locking around drm > init and even more so around fbdev setup ... Please see below for comments (I've reflowed the text to ease review). > v2: Some further improvements: > - Also add documentation for drm_fb_helper_single_add_all_connectors, > Dave Airlie didn't want me to kill this one from the fb helper > interface. > - Update docs for drm_fb_helper_fill_var/fix - they should be used > from the driver's ->fb_probe callback to setup the fbdev info > structure. > - Clarify what the ->fb_probe callback should all do - it needs to > setup both the fbdev info and allocate the drm framebuffer used as > backing storage. > - Add basic documentaation for the drm_fb_helper_funcs driver callback > vfunc. > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > moar kerneldoc > --- > Documentation/DocBook/drm.tmpl | 1 + > drivers/gpu/drm/drm_fb_helper.c | 146 ++++++++++++++++++++++++++++++++---- > include/drm/drm_fb_helper.h | 11 +++ > 3 files changed, 143 insertions(+), 15 deletions(-) > > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > index 6c11d77..14ad829 100644 > --- a/Documentation/DocBook/drm.tmpl > +++ b/Documentation/DocBook/drm.tmpl > @@ -2139,6 +2139,7 @@ void intel_crt_init(struct drm_device *dev) > <title>fbdev Helper Functions Reference</title> > !Pdrivers/gpu/drm/drm_fb_helper.c fbdev helpers > !Edrivers/gpu/drm/drm_fb_helper.c > +!Iinclude/drm/drm_fb_helper.h > </sect2> > <sect2> > <title>Display Port Helper Functions Reference</title> > diff --git a/drivers/gpu/drm/drm_fb_helper.c > b/drivers/gpu/drm/drm_fb_helper.c index 5a92470..a7538cc 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -52,9 +52,34 @@ static LIST_HEAD(kernel_fb_helper_list); > * mode setting driver. They can be used mostly independantely from the Now that you have removed one of the dependencies on the crtc helpers in your "drm/fb-helper: don't disable everything in initial_config" patch, are there others ? If not you can s/mostly //. > * crtc helper functions used by many drivers to implement the kernel mode > * setting interfaces. > + * > + * Initialization is done as a three-step process with > + * drm_fb_helper_init(), drm_fb_helper_single_add_all_connectors() and > + * drm_fb_helper_initial_config(). Drivers with fancier requirements than > + * the default beheviour can override the second step with their own code. > + * Teardown is done with drm_fb_helper_fini(). > + * > + * At runtime drivers should restore the fbdev console by calling > + * drm_fb_helper_restore_fbdev_mode() from their ->lastclose callback. They > + * can also notify the fb helper code from updates to the output Is it "can" or "must" ? If "can", in what conditions should they call drm_fb_helper_restore_fbdev_mode() and in what conditions shouldn't they ? > + * configuration by calling drm_fb_helper_hotplug_event(). > + * > + * All other functions exported by the fb helper library can be used to > + * implement the fbdev driver interface by the driver. > */ > > -/* simple single crtc case helper function */ > +/** > + * drm_fb_helper_single_add_all_connectors() - add all connectors to fbdev > + * emulation helper > + * @fb_helper: fbdev initialized with drm_fb_helper_init > + * > + * This functions adds all the available connectors for use with the given > + * fb_helper. This is a separate step to allow drivers to freely assign or > + * connectors to the fbdev, e.g. if some are reserved for special purposes > + * not adequate to be used for the fbcon. > + * > + * Since this is part of the initial setup before the fbdev is published, > + * no locking is required. > + */ > int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper > *fb_helper) > { > struct drm_device *dev = fb_helper->dev; > @@ -163,6 +188,10 @@ static void drm_fb_helper_restore_lut_atomic(struct > drm_crtc *crtc) crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, 0, > crtc->gamma_size); } > > +/** > + * drm_fb_helper_debug_enter - implementation for ->fb_debug_enter > + * @info: fbdev registered by the helper. > + */ > int drm_fb_helper_debug_enter(struct fb_info *info) > { > struct drm_fb_helper *helper = info->par; > @@ -208,6 +237,10 @@ static struct drm_framebuffer > *drm_mode_config_fb(struct drm_crtc *crtc) return NULL; > } > > +/** > + * drm_fb_helper_debug_leave - implementation for ->fb_debug_leave > + * @info: fbdev registered by the helper. > + */ > int drm_fb_helper_debug_leave(struct fb_info *info) > { > struct drm_fb_helper *helper = info->par; > @@ -243,9 +276,9 @@ EXPORT_SYMBOL(drm_fb_helper_debug_leave); > * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration > * @fb_helper: fbcon to restore > * > - * This should be called from driver's drm->lastclose callback when > - * implementing an fbcon on top of kms using this helper. This ensures that > - * the user isn't greeted with a black screen when e.g. X dies. > + * This should be called from driver's drm <code>->lastclose</code> The resulting HTML is <code>->lastclose</code> I'm not sure that's what you want :-) > + * callback when implementing an fbcon on top of kms using this helper. > + * This ensures that the user isn't greeted with a black screen when e.g. > + * X dies. > */ > bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) > { > @@ -378,6 +411,11 @@ static void drm_fb_helper_dpms(struct fb_info *info, > int dpms_mode) drm_modeset_unlock_all(dev); > } > > +/** > + * drm_fb_helper_blank - implementation for ->fb_blank > + * @blank: desired blanking state > + * @info: fbdev registered by the helper. Nitpicking here, shouldn't you either use a full stop at the end of each argument, or no full stop at all (this applies to the other functions as well) ? > + */ > int drm_fb_helper_blank(int blank, struct fb_info *info) > { > switch (blank) { > @@ -421,6 +459,24 @@ static void drm_fb_helper_crtc_free(struct > drm_fb_helper *helper) kfree(helper->crtc_info); > } > > +/** > + * drm_fb_helper_init - initialize a drm_fb_helper structure > + * @dev: drm device > + * @fb_helper: driver-allocated fbdev helper structure to initialize > + * @crtc_count: crtc count > + * @max_conn_count: max connector count > + * > + * This allocates the structures for the fbdev helper with the given > + * limits. Note that this won't yet touch the hw (through the driver s/hw/hardware/ ? It might be a good idea to add a small description of the crtc_count parameter. > + * interfaces) nor register the fbdev. This is only done in > + * drm_fb_helper_initial_config() to allow driver writes more control over > + * the exact init sequence. > + * > + * Drivers must also set fb_helper->funcs before calling s/also // ? > + * drm_fb_helper_initial_config(). > + * > + * RETURNS: > + * Zero if everything went ok, nonzero otherwise. > + */ > int drm_fb_helper_init(struct drm_device *dev, > struct drm_fb_helper *fb_helper, > int crtc_count, int max_conn_count) > @@ -549,6 +605,11 @@ static int setcolreg(struct drm_crtc *crtc, u16 red, > u16 green, return 0; > } > > +/** > + * drm_fb_helper_setcmap - implementation for ->fb_setcmap > + * @cmap: cmap to set > + * @info: fbdev registered by the helper. > + */ > int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info) > { > struct drm_fb_helper *fb_helper = info->par; > @@ -588,6 +649,11 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct > fb_info *info) } > EXPORT_SYMBOL(drm_fb_helper_setcmap); > > +/** > + * drm_fb_helper_check_var - implementation for ->fb_check_var > + * @var: screeninfo to check > + * @info: fbdev registered by the helper. > + */ > int drm_fb_helper_check_var(struct fb_var_screeninfo *var, > struct fb_info *info) > { > @@ -680,7 +746,14 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo > *var, } > EXPORT_SYMBOL(drm_fb_helper_check_var); > > -/* this will let fbcon do the mode init */ > +/** > + * drm_fb_helper_set_par - implementation for ->fb_set_par > + * @info: fbdev registered by the helper. > + * > + * This will let fbcon do the mode init and is called at initialization > + * time by the fbdev core when registering the driver, and later on > + * through the hotplug callback. > + */ > int drm_fb_helper_set_par(struct fb_info *info) > { > struct drm_fb_helper *fb_helper = info->par; > @@ -712,6 +785,11 @@ int drm_fb_helper_set_par(struct fb_info *info) > } > EXPORT_SYMBOL(drm_fb_helper_set_par); > > +/** > + * drm_fb_helper_pan_display - implementation for ->fb_pan_display > + * @var: updated screen information > + * @info: fbdev registered by the helper. > + */ > int drm_fb_helper_pan_display(struct fb_var_screeninfo *var, > struct fb_info *info) > { > @@ -750,8 +828,9 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo > *var, EXPORT_SYMBOL(drm_fb_helper_pan_display); > > /* > - * Allocates the backing storage through the ->fb_probe callback and then > - * registers the fbdev and sets up the panic notifier. > + * Allocates the backing storage and sets up the fbdev info structure > + * through the ->fb_probe callback and then registers the fbdev and sets > + * up the panic notifier. > */ > static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, > int preferred_bpp) > @@ -873,6 +952,19 @@ static int drm_fb_helper_single_fb_probe(struct > drm_fb_helper *fb_helper, return 0; > } > > +/** > + * drm_fb_helper_fill_fix - initializes fixed fbdev information > + * @info: fbdev registered by the helper. > + * @pitch: desired pitch > + * @depth: desired depth > + * > + * Helper to fill in the fixed fbdev information useful for a > + * non-accelerated fbdev emulations. Drivers which support acceleration > + * methods which impose additional constraints need to set up their own > + * limits. > + * > + * Drivers should call this (or their equivalent setup code) from their > + * <code>->fb_probe</code> callback. > + */ > void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch, > uint32_t depth) > { > @@ -893,6 +985,20 @@ void drm_fb_helper_fill_fix(struct fb_info *info, > uint32_t pitch, } > EXPORT_SYMBOL(drm_fb_helper_fill_fix); > > +/** > + * drm_fb_helper_fill_var - initalizes variable fbdev information > + * @info: fbdev instance to set up > + * @fb_helper: fb helper instance to use as template > + * @fb_width: desired fb width > + * @fb_height: desired fb height > + * > + * Sets up the variable fbdev metainformation from the given fb helper > + * instance and the drm framebuffer allocated in > + * <code>fb_helper->fb</code>. > + * > + * Drivers should call this (or their equivalent setup code) from their > + * <code>->fb_probe</code> callback after having allocated the fbdev > + * backing storage framebuffer. > + */ > void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper > *fb_helper, uint32_t fb_width, uint32_t fb_height) > { > @@ -1355,18 +1461,23 @@ out: > } > > /** > - * drm_helper_initial_config - setup a sane initial connector configuration > + * drm_fb_helper_initial_config - setup a sane initial connector > configuration > * @fb_helper: fb_helper device struct > * @bpp_sel: bpp value to use for the framebuffer configuration > * > - * LOCKING: > - * Called at init time by the driver to set up the @fb_helper initial > - * configuration, must take the mode config lock. > - * > * Scans the CRTCs and connectors and tries to put together an initial > * setup. At the moment, this is a cloned configuration across all heads > * with a new framebuffer object as the backing store. > * > + * Note that this also registers the fbdev and so allows userspace to call > + * into the driver through the fbdev interfaces. > + * > + * This function will call down into the <code>->fb_probe</code> callback <code> is displayed in the resulting HTML here as well. > + * to let the driver allocate and initialize the fbdev info structure and > + * the drm framebuffer used to back the fbdev. drm_fb_helper_fill_var() > + * and drm_fb_helper_fill_fix() are provided as helpers to setup simple > + * default values for the fbdev info structure. > + * > * RETURNS: > * Zero if everything went ok, nonzero otherwise. > */ > @@ -1397,12 +1508,17 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config); > * probing all the outputs attached to the fb > * @fb_helper: the drm_fb_helper > * > - * LOCKING: > - * Called at runtime, must take mode config lock. > - * > * Scan the connectors attached to the fb_helper and try to put together a > * setup after *notification of a change in output configuration. > * > + * Called at runtime, takes the mode config locks to be able to > + * check/change the modeset configuration. Must be run from process > + * context (which usually means either the output polling work or a work > + * item launch from the driver's hotplug interrupt). s/launch/launched/ > + * Note that the driver must ensure that this is only called _after_ the fb > + * has been fully set up, i.e. after the call to > + * drm_fb_helper_initial_config. > + * > * RETURNS: > * 0 on success and a non-zero error code otherwise. > */ > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h > index 973402d..3c30297 100644 > --- a/include/drm/drm_fb_helper.h > +++ b/include/drm/drm_fb_helper.h > @@ -48,6 +48,17 @@ struct drm_fb_helper_surface_size { > u32 surface_depth; > }; > > +/** > + * struct drm_fb_helper_funcs - driver callbacks for the fbdev emulation > library > + * @gamma_set: - Set the given lut register on the given crtc. > + * @gamma_get: - Read the given lut register on the given crtc, used to > safe the s/lut/gamma lut/ ? > + * current lut when force-restoring the fbdev for e.g. kdbg. > + * @fb_probe: - Driver callback to allocate and initialize the fbdev info > + * structure. Futhermore it also needs to allocate the drm > + * framebuffer used to back the fbdev. > + * > + * Driver callbacks used by the fbdev emulation helper library. > + */ > struct drm_fb_helper_funcs { > void (*gamma_set)(struct drm_crtc *crtc, u16 red, u16 green, > u16 blue, int regno);
On Fri, Feb 01, 2013 at 01:21:29PM +0100, Laurent Pinchart wrote: > Hi Daniel, > > Thanks for improving the documentation :-) > > On Sunday 27 January 2013 18:42:16 Daniel Vetter wrote: > > Now that the fbdev helper interface for drivers is trimmed down, > > update the kerneldoc for all the remaining exported functions. > > > > I've tried to beat the DocBook a bit by reordering the function > > references a bit into a more sensible ordering. But that didn't work > > out at all. Hence just extend the in-code DOC: section a bit. > > > > Also remove the LOCKING: sections - especially for the setup functions > > they're totally bogus. But that's not a documentation problem, but > > simply an artifact of the current rather hazardous locking around drm > > init and even more so around fbdev setup ... > > Please see below for comments (I've reflowed the text to ease review). > > > v2: Some further improvements: > > - Also add documentation for drm_fb_helper_single_add_all_connectors, > > Dave Airlie didn't want me to kill this one from the fb helper > > interface. > > - Update docs for drm_fb_helper_fill_var/fix - they should be used > > from the driver's ->fb_probe callback to setup the fbdev info > > structure. > > - Clarify what the ->fb_probe callback should all do - it needs to > > setup both the fbdev info and allocate the drm framebuffer used as > > backing storage. > > - Add basic documentaation for the drm_fb_helper_funcs driver callback > > vfunc. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > > > moar kerneldoc > > --- > > Documentation/DocBook/drm.tmpl | 1 + > > drivers/gpu/drm/drm_fb_helper.c | 146 ++++++++++++++++++++++++++++++++---- > > include/drm/drm_fb_helper.h | 11 +++ > > 3 files changed, 143 insertions(+), 15 deletions(-) > > > > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > > index 6c11d77..14ad829 100644 > > --- a/Documentation/DocBook/drm.tmpl > > +++ b/Documentation/DocBook/drm.tmpl > > @@ -2139,6 +2139,7 @@ void intel_crt_init(struct drm_device *dev) > > <title>fbdev Helper Functions Reference</title> > > !Pdrivers/gpu/drm/drm_fb_helper.c fbdev helpers > > !Edrivers/gpu/drm/drm_fb_helper.c > > +!Iinclude/drm/drm_fb_helper.h > > </sect2> > > <sect2> > > <title>Display Port Helper Functions Reference</title> > > diff --git a/drivers/gpu/drm/drm_fb_helper.c > > b/drivers/gpu/drm/drm_fb_helper.c index 5a92470..a7538cc 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -52,9 +52,34 @@ static LIST_HEAD(kernel_fb_helper_list); > > * mode setting driver. They can be used mostly independantely from the > > Now that you have removed one of the dependencies on the crtc helpers in your > "drm/fb-helper: don't disable everything in initial_config" patch, are there > others ? If not you can s/mostly //. It's getting better, but a few of the driver callbacks used by the fb helper are still in the crtc helper function tables. And there's the fb helper private way to safe/restore gamma tables. But at least semantically there's no dependency any longer after these patches I think. > > > * crtc helper functions used by many drivers to implement the kernel mode > > * setting interfaces. > > + * > > + * Initialization is done as a three-step process with > > + * drm_fb_helper_init(), drm_fb_helper_single_add_all_connectors() and > > + * drm_fb_helper_initial_config(). Drivers with fancier requirements than > > + * the default beheviour can override the second step with their own code. > > + * Teardown is done with drm_fb_helper_fini(). > > + * > > + * At runtime drivers should restore the fbdev console by calling > > + * drm_fb_helper_restore_fbdev_mode() from their ->lastclose callback. They > > + * can also notify the fb helper code from updates to the output > > Is it "can" or "must" ? If "can", in what conditions should they call > drm_fb_helper_restore_fbdev_mode() and in what conditions shouldn't they ? I've figured that hpd support is optional, hence no "must". I've opted for a should instead. Also added a bit of text to suggest a good place to put this call. > > > + * configuration by calling drm_fb_helper_hotplug_event(). > > + * > > + * All other functions exported by the fb helper library can be used to > > + * implement the fbdev driver interface by the driver. > > */ > > > > -/* simple single crtc case helper function */ > > +/** > > + * drm_fb_helper_single_add_all_connectors() - add all connectors to fbdev > > + * emulation helper > > + * @fb_helper: fbdev initialized with drm_fb_helper_init > > + * > > + * This functions adds all the available connectors for use with the given > > + * fb_helper. This is a separate step to allow drivers to freely assign or > > + * connectors to the fbdev, e.g. if some are reserved for special purposes > > + * not adequate to be used for the fbcon. > > + * > > + * Since this is part of the initial setup before the fbdev is published, > > + * no locking is required. > > + */ > > int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper > > *fb_helper) > > { > > struct drm_device *dev = fb_helper->dev; > > @@ -163,6 +188,10 @@ static void drm_fb_helper_restore_lut_atomic(struct > > drm_crtc *crtc) crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, 0, > > crtc->gamma_size); } > > > > +/** > > + * drm_fb_helper_debug_enter - implementation for ->fb_debug_enter > > + * @info: fbdev registered by the helper. > > + */ > > int drm_fb_helper_debug_enter(struct fb_info *info) > > { > > struct drm_fb_helper *helper = info->par; > > @@ -208,6 +237,10 @@ static struct drm_framebuffer > > *drm_mode_config_fb(struct drm_crtc *crtc) return NULL; > > } > > > > +/** > > + * drm_fb_helper_debug_leave - implementation for ->fb_debug_leave > > + * @info: fbdev registered by the helper. > > + */ > > int drm_fb_helper_debug_leave(struct fb_info *info) > > { > > struct drm_fb_helper *helper = info->par; > > @@ -243,9 +276,9 @@ EXPORT_SYMBOL(drm_fb_helper_debug_leave); > > * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration > > * @fb_helper: fbcon to restore > > * > > - * This should be called from driver's drm->lastclose callback when > > - * implementing an fbcon on top of kms using this helper. This ensures that > > - * the user isn't greeted with a black screen when e.g. X dies. > > + * This should be called from driver's drm <code>->lastclose</code> > > The resulting HTML is > > <code>->lastclose</code> > > I'm not sure that's what you want :-) Nope, killed them all. > > > + * callback when implementing an fbcon on top of kms using this helper. > > + * This ensures that the user isn't greeted with a black screen when e.g. > > + * X dies. > > */ > > bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) > > { > > @@ -378,6 +411,11 @@ static void drm_fb_helper_dpms(struct fb_info *info, > > int dpms_mode) drm_modeset_unlock_all(dev); > > } > > > > +/** > > + * drm_fb_helper_blank - implementation for ->fb_blank > > + * @blank: desired blanking state > > + * @info: fbdev registered by the helper. > > Nitpicking here, shouldn't you either use a full stop at the end of each > argument, or no full stop at all (this applies to the other functions as well) > ? Copy&pasta, full stop dropped everywhere. > > > + */ > > int drm_fb_helper_blank(int blank, struct fb_info *info) > > { > > switch (blank) { > > @@ -421,6 +459,24 @@ static void drm_fb_helper_crtc_free(struct > > drm_fb_helper *helper) kfree(helper->crtc_info); > > } > > > > +/** > > + * drm_fb_helper_init - initialize a drm_fb_helper structure > > + * @dev: drm device > > + * @fb_helper: driver-allocated fbdev helper structure to initialize > > + * @crtc_count: crtc count > > + * @max_conn_count: max connector count > > + * > > + * This allocates the structures for the fbdev helper with the given > > + * limits. Note that this won't yet touch the hw (through the driver > > s/hw/hardware/ ? > > It might be a good idea to add a small description of the crtc_count > parameter. Done. All the other corrections below I've also applied. Thanks, Daniel
diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 6c11d77..14ad829 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -2139,6 +2139,7 @@ void intel_crt_init(struct drm_device *dev) <title>fbdev Helper Functions Reference</title> !Pdrivers/gpu/drm/drm_fb_helper.c fbdev helpers !Edrivers/gpu/drm/drm_fb_helper.c +!Iinclude/drm/drm_fb_helper.h </sect2> <sect2> <title>Display Port Helper Functions Reference</title> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 5a92470..a7538cc 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -52,9 +52,34 @@ static LIST_HEAD(kernel_fb_helper_list); * mode setting driver. They can be used mostly independantely from the crtc * helper functions used by many drivers to implement the kernel mode setting * interfaces. + * + * Initialization is done as a three-step process with drm_fb_helper_init(), + * drm_fb_helper_single_add_all_connectors() and drm_fb_helper_initial_config(). + * Drivers with fancier requirements than the default beheviour can override the + * second step with their own code. Teardown is done with drm_fb_helper_fini(). + * + * At runtime drivers should restore the fbdev console by calling + * drm_fb_helper_restore_fbdev_mode() from their ->lastclose callback. They can + * also notify the fb helper code from updates to the output configuration by + * calling drm_fb_helper_hotplug_event(). + * + * All other functions exported by the fb helper library can be used to + * implement the fbdev driver interface by the driver. */ -/* simple single crtc case helper function */ +/** + * drm_fb_helper_single_add_all_connectors() - add all connectors to fbdev + * emulation helper + * @fb_helper: fbdev initialized with drm_fb_helper_init + * + * This functions adds all the available connectors for use with the given + * fb_helper. This is a separate step to allow drivers to freely assign + * connectors to the fbdev, e.g. if some are reserved for special purposes or + * not adequate to be used for the fbcon. + * + * Since this is part of the initial setup before the fbdev is published, no + * locking is required. + */ int drm_fb_helper_single_add_all_connectors(struct drm_fb_helper *fb_helper) { struct drm_device *dev = fb_helper->dev; @@ -163,6 +188,10 @@ static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc) crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, 0, crtc->gamma_size); } +/** + * drm_fb_helper_debug_enter - implementation for ->fb_debug_enter + * @info: fbdev registered by the helper. + */ int drm_fb_helper_debug_enter(struct fb_info *info) { struct drm_fb_helper *helper = info->par; @@ -208,6 +237,10 @@ static struct drm_framebuffer *drm_mode_config_fb(struct drm_crtc *crtc) return NULL; } +/** + * drm_fb_helper_debug_leave - implementation for ->fb_debug_leave + * @info: fbdev registered by the helper. + */ int drm_fb_helper_debug_leave(struct fb_info *info) { struct drm_fb_helper *helper = info->par; @@ -243,9 +276,9 @@ EXPORT_SYMBOL(drm_fb_helper_debug_leave); * drm_fb_helper_restore_fbdev_mode - restore fbdev configuration * @fb_helper: fbcon to restore * - * This should be called from driver's drm->lastclose callback when implementing - * an fbcon on top of kms using this helper. This ensures that the user isn't - * greeted with a black screen when e.g. X dies. + * This should be called from driver's drm <code>->lastclose</code> callback + * when implementing an fbcon on top of kms using this helper. This ensures that + * the user isn't greeted with a black screen when e.g. X dies. */ bool drm_fb_helper_restore_fbdev_mode(struct drm_fb_helper *fb_helper) { @@ -378,6 +411,11 @@ static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode) drm_modeset_unlock_all(dev); } +/** + * drm_fb_helper_blank - implementation for ->fb_blank + * @blank: desired blanking state + * @info: fbdev registered by the helper. + */ int drm_fb_helper_blank(int blank, struct fb_info *info) { switch (blank) { @@ -421,6 +459,24 @@ static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper) kfree(helper->crtc_info); } +/** + * drm_fb_helper_init - initialize a drm_fb_helper structure + * @dev: drm device + * @fb_helper: driver-allocated fbdev helper structure to initialize + * @crtc_count: crtc count + * @max_conn_count: max connector count + * + * This allocates the structures for the fbdev helper with the given limits. + * Note that this won't yet touch the hw (through the driver interfaces) nor + * register the fbdev. This is only done in drm_fb_helper_initial_config() to + * allow driver writes more control over the exact init sequence. + * + * Drivers must also set fb_helper->funcs before calling + * drm_fb_helper_initial_config(). + * + * RETURNS: + * Zero if everything went ok, nonzero otherwise. + */ int drm_fb_helper_init(struct drm_device *dev, struct drm_fb_helper *fb_helper, int crtc_count, int max_conn_count) @@ -549,6 +605,11 @@ static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green, return 0; } +/** + * drm_fb_helper_setcmap - implementation for ->fb_setcmap + * @cmap: cmap to set + * @info: fbdev registered by the helper. + */ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info) { struct drm_fb_helper *fb_helper = info->par; @@ -588,6 +649,11 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info) } EXPORT_SYMBOL(drm_fb_helper_setcmap); +/** + * drm_fb_helper_check_var - implementation for ->fb_check_var + * @var: screeninfo to check + * @info: fbdev registered by the helper. + */ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, struct fb_info *info) { @@ -680,7 +746,14 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var, } EXPORT_SYMBOL(drm_fb_helper_check_var); -/* this will let fbcon do the mode init */ +/** + * drm_fb_helper_set_par - implementation for ->fb_set_par + * @info: fbdev registered by the helper. + * + * This will let fbcon do the mode init and is called at initialization time by + * the fbdev core when registering the driver, and later on through the hotplug + * callback. + */ int drm_fb_helper_set_par(struct fb_info *info) { struct drm_fb_helper *fb_helper = info->par; @@ -712,6 +785,11 @@ int drm_fb_helper_set_par(struct fb_info *info) } EXPORT_SYMBOL(drm_fb_helper_set_par); +/** + * drm_fb_helper_pan_display - implementation for ->fb_pan_display + * @var: updated screen information + * @info: fbdev registered by the helper. + */ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var, struct fb_info *info) { @@ -750,8 +828,9 @@ int drm_fb_helper_pan_display(struct fb_var_screeninfo *var, EXPORT_SYMBOL(drm_fb_helper_pan_display); /* - * Allocates the backing storage through the ->fb_probe callback and then - * registers the fbdev and sets up the panic notifier. + * Allocates the backing storage and sets up the fbdev info structure through + * the ->fb_probe callback and then registers the fbdev and sets up the panic + * notifier. */ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, int preferred_bpp) @@ -873,6 +952,19 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper, return 0; } +/** + * drm_fb_helper_fill_fix - initializes fixed fbdev information + * @info: fbdev registered by the helper. + * @pitch: desired pitch + * @depth: desired depth + * + * Helper to fill in the fixed fbdev information useful for a non-accelerated + * fbdev emulations. Drivers which support acceleration methods which impose + * additional constraints need to set up their own limits. + * + * Drivers should call this (or their equivalent setup code) from their + * <code>->fb_probe</code> callback. + */ void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch, uint32_t depth) { @@ -893,6 +985,20 @@ void drm_fb_helper_fill_fix(struct fb_info *info, uint32_t pitch, } EXPORT_SYMBOL(drm_fb_helper_fill_fix); +/** + * drm_fb_helper_fill_var - initalizes variable fbdev information + * @info: fbdev instance to set up + * @fb_helper: fb helper instance to use as template + * @fb_width: desired fb width + * @fb_height: desired fb height + * + * Sets up the variable fbdev metainformation from the given fb helper instance + * and the drm framebuffer allocated in <code>fb_helper->fb</code>. + * + * Drivers should call this (or their equivalent setup code) from their + * <code>->fb_probe</code> callback after having allocated the fbdev backing + * storage framebuffer. + */ void drm_fb_helper_fill_var(struct fb_info *info, struct drm_fb_helper *fb_helper, uint32_t fb_width, uint32_t fb_height) { @@ -1355,18 +1461,23 @@ out: } /** - * drm_helper_initial_config - setup a sane initial connector configuration + * drm_fb_helper_initial_config - setup a sane initial connector configuration * @fb_helper: fb_helper device struct * @bpp_sel: bpp value to use for the framebuffer configuration * - * LOCKING: - * Called at init time by the driver to set up the @fb_helper initial - * configuration, must take the mode config lock. - * * Scans the CRTCs and connectors and tries to put together an initial setup. * At the moment, this is a cloned configuration across all heads with * a new framebuffer object as the backing store. * + * Note that this also registers the fbdev and so allows userspace to call into + * the driver through the fbdev interfaces. + * + * This function will call down into the <code>->fb_probe</code> callback to let + * the driver allocate and initialize the fbdev info structure and the drm + * framebuffer used to back the fbdev. drm_fb_helper_fill_var() and + * drm_fb_helper_fill_fix() are provided as helpers to setup simple default + * values for the fbdev info structure. + * * RETURNS: * Zero if everything went ok, nonzero otherwise. */ @@ -1397,12 +1508,17 @@ EXPORT_SYMBOL(drm_fb_helper_initial_config); * probing all the outputs attached to the fb * @fb_helper: the drm_fb_helper * - * LOCKING: - * Called at runtime, must take mode config lock. - * * Scan the connectors attached to the fb_helper and try to put together a * setup after *notification of a change in output configuration. * + * Called at runtime, takes the mode config locks to be able to check/change the + * modeset configuration. Must be run from process context (which usually means + * either the output polling work or a work item launch from the driver's + * hotplug interrupt). + * + * Note that the driver must ensure that this is only called _after_ the fb has + * been fully set up, i.e. after the call to drm_fb_helper_initial_config. + * * RETURNS: * 0 on success and a non-zero error code otherwise. */ diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 973402d..3c30297 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -48,6 +48,17 @@ struct drm_fb_helper_surface_size { u32 surface_depth; }; +/** + * struct drm_fb_helper_funcs - driver callbacks for the fbdev emulation library + * @gamma_set: - Set the given lut register on the given crtc. + * @gamma_get: - Read the given lut register on the given crtc, used to safe the + * current lut when force-restoring the fbdev for e.g. kdbg. + * @fb_probe: - Driver callback to allocate and initialize the fbdev info + * structure. Futhermore it also needs to allocate the drm + * framebuffer used to back the fbdev. + * + * Driver callbacks used by the fbdev emulation helper library. + */ struct drm_fb_helper_funcs { void (*gamma_set)(struct drm_crtc *crtc, u16 red, u16 green, u16 blue, int regno);
Now that the fbdev helper interface for drivers is trimmed down, update the kerneldoc for all the remaining exported functions. I've tried to beat the DocBook a bit by reordering the function references a bit into a more sensible ordering. But that didn't work out at all. Hence just extend the in-code DOC: section a bit. Also remove the LOCKING: sections - especially for the setup functions they're totally bogus. But that's not a documentation problem, but simply an artifact of the current rather hazardous locking around drm init and even more so around fbdev setup ... v2: Some further improvements: - Also add documentation for drm_fb_helper_single_add_all_connectors, Dave Airlie didn't want me to kill this one from the fb helper interface. - Update docs for drm_fb_helper_fill_var/fix - they should be used from the driver's ->fb_probe callback to setup the fbdev info structure. - Clarify what the ->fb_probe callback should all do - it needs to setup both the fbdev info and allocate the drm framebuffer used as backing storage. - Add basic documentaation for the drm_fb_helper_funcs driver callback vfunc. Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> moar kerneldoc --- Documentation/DocBook/drm.tmpl | 1 + drivers/gpu/drm/drm_fb_helper.c | 146 +++++++++++++++++++++++++++++++++++---- include/drm/drm_fb_helper.h | 11 +++ 3 files changed, 143 insertions(+), 15 deletions(-)