Re: [PATCH] Documentation: DocBook DRM framework documentation
Hi Laurent, I've used "behavior" when copying sections from the existing documentation. I'll unify that. Does kernel documentation favour one of the spellings ? Looking at v3.5, the American spelling is more common, but looking at how you spell favour, I think I know which one you favor :) linux-git$ grep -ri behaviour Documentation/ | wc -l 150 linux-git$ grep -ri behavior Documentation/ | wc -l 269 -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Documentation: DocBook DRM framework documentation
Hi Laurent, > I've used "behavior" when copying sections from the existing documentation. > I'll unify that. Does kernel documentation favour one of the spellings ? > Looking at v3.5, the American spelling is more common, but looking at how you spell favour, I think I know which one you favor :) linux-git$ grep -ri behaviour Documentation/ | wc -l 150 linux-git$ grep -ri behavior Documentation/ | wc -l 269 -Michael MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
[PATCH] Documentation: DocBook DRM framework documentation
Hi Michael, Thank you for the review. On Monday 23 July 2012 17:12:43 Michael Jones wrote: > Hi Laurent, > > At a quick glance I noticed a couple of things: > > On 07/13/2012 02:00 AM, Laurent Pinchart wrote: > > [snip] > > > + > > + The drm_driver structure contains static > > + information that describe the driver and features it supports, and > > s/describe/describes/ Fixed. > > + pointers to methods that the DRM core will call to implement the > > DRM API. > > + We will first go through the drm_driver > > static > > + information fields, and will then describe individual operations in > > + details as they get used in later sections. > > > > - > > > > - Driver private & performance counters > > - > > - The driver private hangs off the main drm_device structure and > > - can be used for tracking various device-specific bits of > > - information, like register offsets, command buffer status, > > - register state for suspend/resume, etc. At load time, a > > - driver may simply allocate one and set drm_device.dev_priv > > - appropriately; it should be freed and drm_device.dev_priv set > > - to NULL when the driver is unloaded. > > - > > + Driver Information > > + > > +Driver Features > > + > > + Drivers inform the DRM core about their requirements and > > supported > > + features by setting appropriate flags in the > > + driver_features field. Since those > > flags > > + influence the DRM core behaviour since registration time, most > > of them > > Elsewhere you use the American spelling "behavior". I've used "behavior" when copying sections from the existing documentation. I'll unify that. Does kernel documentation favour one of the spellings ? > [snip] > > > + > > +Major, Minor and Patchlevel > > +int major; > > + int minor; > > + int patchlevel; > > In my browser, "int minor" and "int patchlevel" look indented, whereas > "int major" does not. Looks like they _should_ be indented identically. > Don't know how you fix this or if you even see the same problem. Fixed. [snip] -- Regards, Laurent Pinchart
[PATCH] Documentation: DocBook DRM framework documentation
Hi Laurent, At a quick glance I noticed a couple of things: On 07/13/2012 02:00 AM, Laurent Pinchart wrote: [snip] > + > + The drm_driver structure contains static > + information that describe the driver and features it supports, and s/describe/describes/ > + pointers to methods that the DRM core will call to implement the DRM > API. > + We will first go through the drm_driver static > + information fields, and will then describe individual operations in > + details as they get used in later sections. > > - > > - Driver private & performance counters > - > - The driver private hangs off the main drm_device structure and > - can be used for tracking various device-specific bits of > - information, like register offsets, command buffer status, > - register state for suspend/resume, etc. At load time, a > - driver may simply allocate one and set drm_device.dev_priv > - appropriately; it should be freed and drm_device.dev_priv set > - to NULL when the driver is unloaded. > - > + Driver Information > + > +Driver Features > + > + Drivers inform the DRM core about their requirements and supported > + features by setting appropriate flags in the > + driver_features field. Since those flags > + influence the DRM core behaviour since registration time, most of > them Elsewhere you use the American spelling "behavior". [snip] > + > +Major, Minor and Patchlevel > +int major; > + int minor; > + int patchlevel; In my browser, "int minor" and "int patchlevel" look indented, whereas "int major" does not. Looks like they _should_ be indented identically. Don't know how you fix this or if you even see the same problem. > + > + The DRM core identifies driver versions by a major, minor and patch > + level triplet. The information is printed to the kernel log at > + initialization time and passed to userspace through the > + DRM_IOCTL_VERSION ioctl. > + > + > + The major and minor numbers are also used to verify the requested > driver > + API version passed to DRM_IOCTL_SET_VERSION. When the driver API > changes > + between minor versions, applications can call > DRM_IOCTL_SET_VERSION to > + select a specific version of the API. If the requested major isn't > equal > + to the driver major, or the requested minor is larger than the > driver > + minor, the DRM_IOCTL_SET_VERSION call will return an error. > Otherwise > + the driver's set_version() method will be called with the requested > + version. > + > + > + > +Name, Description and Date > +char *name; > + char *desc; > + char *date; Same indentation issue here. [snip] > + > +The mode_fixup operation should reject > the > +mode if it can't reasonably use it. The definition of > "reasonable" > +is currently fuzzy in this context. One possible behaviour would > be maybe s/behaviour/behavior/ again MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier
Re: [PATCH] Documentation: DocBook DRM framework documentation
Hi Michael, Thank you for the review. On Monday 23 July 2012 17:12:43 Michael Jones wrote: > Hi Laurent, > > At a quick glance I noticed a couple of things: > > On 07/13/2012 02:00 AM, Laurent Pinchart wrote: > > [snip] > > > + > > + The drm_driver structure contains static > > + information that describe the driver and features it supports, and > > s/describe/describes/ Fixed. > > + pointers to methods that the DRM core will call to implement the > > DRM API. > > + We will first go through the drm_driver > > static > > + information fields, and will then describe individual operations in > > + details as they get used in later sections. > > > > - > > > > - Driver private & performance counters > > - > > - The driver private hangs off the main drm_device structure and > > - can be used for tracking various device-specific bits of > > - information, like register offsets, command buffer status, > > - register state for suspend/resume, etc. At load time, a > > - driver may simply allocate one and set drm_device.dev_priv > > - appropriately; it should be freed and drm_device.dev_priv set > > - to NULL when the driver is unloaded. > > - > > + Driver Information > > + > > +Driver Features > > + > > + Drivers inform the DRM core about their requirements and > > supported > > + features by setting appropriate flags in the > > + driver_features field. Since those > > flags > > + influence the DRM core behaviour since registration time, most > > of them > > Elsewhere you use the American spelling "behavior". I've used "behavior" when copying sections from the existing documentation. I'll unify that. Does kernel documentation favour one of the spellings ? > [snip] > > > + > > +Major, Minor and Patchlevel > > +int major; > > + int minor; > > + int patchlevel; > > In my browser, "int minor" and "int patchlevel" look indented, whereas > "int major" does not. Looks like they _should_ be indented identically. > Don't know how you fix this or if you even see the same problem. Fixed. [snip] -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Documentation: DocBook DRM framework documentation
Hi Laurent, At a quick glance I noticed a couple of things: On 07/13/2012 02:00 AM, Laurent Pinchart wrote: [snip] + + The drm_driver structure contains static + information that describe the driver and features it supports, and s/describe/describes/ + pointers to methods that the DRM core will call to implement the DRM API. + We will first go through the drm_driver static + information fields, and will then describe individual operations in + details as they get used in later sections. - - Driver private & performance counters - - The driver private hangs off the main drm_device structure and - can be used for tracking various device-specific bits of - information, like register offsets, command buffer status, - register state for suspend/resume, etc. At load time, a - driver may simply allocate one and set drm_device.dev_priv - appropriately; it should be freed and drm_device.dev_priv set - to NULL when the driver is unloaded. - + Driver Information + +Driver Features + + Drivers inform the DRM core about their requirements and supported + features by setting appropriate flags in the + driver_features field. Since those flags + influence the DRM core behaviour since registration time, most of them Elsewhere you use the American spelling "behavior". [snip] + +Major, Minor and Patchlevel +int major; + int minor; + int patchlevel; In my browser, "int minor" and "int patchlevel" look indented, whereas "int major" does not. Looks like they _should_ be indented identically. Don't know how you fix this or if you even see the same problem. + + The DRM core identifies driver versions by a major, minor and patch + level triplet. The information is printed to the kernel log at + initialization time and passed to userspace through the + DRM_IOCTL_VERSION ioctl. + + + The major and minor numbers are also used to verify the requested driver + API version passed to DRM_IOCTL_SET_VERSION. When the driver API changes + between minor versions, applications can call DRM_IOCTL_SET_VERSION to + select a specific version of the API. If the requested major isn't equal + to the driver major, or the requested minor is larger than the driver + minor, the DRM_IOCTL_SET_VERSION call will return an error. Otherwise + the driver's set_version() method will be called with the requested + version. + + + +Name, Description and Date +char *name; + char *desc; + char *date; Same indentation issue here. [snip] + +The mode_fixup operation should reject the +mode if it can't reasonably use it. The definition of "reasonable" +is currently fuzzy in this context. One possible behaviour would be maybe s/behaviour/behavior/ again MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler Registergericht: Amtsgericht Stuttgart, HRB 271090 Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner, Erhard Meier ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Documentation: DocBook DRM framework documentation
On Friday 13 July 2012 02:00:23 Laurent Pinchart wrote: > Signed-off-by: Laurent Pinchart > --- > Documentation/DocBook/drm.tmpl | 2835 - > 1 files changed, 2226 insertions(+), 609 deletions(-) > > Hi everybody, > > Here's the DRM kernel framework documentation previously posted to the > dri-devel mailing list. The documentation has been reworked, converted to > DocBook and merged with the existing DocBook DRM documentation stub. The > result doesn't cover the whole DRM API but should hopefully be good enough > for a start. > > I've done my best to follow a natural flow starting at initialization and > covering the major DRM internal topics. As I'm not a native English speaker > I'm not totally happy with the result, so if anyone wants to edit the text > please feel free to do so. Review will as usual be appreciated, and acks > will be even more welcome (I've been working on this document for longer > than I feel comfortable with). Just for information, a compiled copy of the DRM documentation with this patch applied can be found at http://www.ideasonboard.org/media/drm/. That might be easier to review than the patch. -- Regards, Laurent Pinchart
Re: [PATCH] Documentation: DocBook DRM framework documentation
On Friday 13 July 2012 02:00:23 Laurent Pinchart wrote: > Signed-off-by: Laurent Pinchart > --- > Documentation/DocBook/drm.tmpl | 2835 - > 1 files changed, 2226 insertions(+), 609 deletions(-) > > Hi everybody, > > Here's the DRM kernel framework documentation previously posted to the > dri-devel mailing list. The documentation has been reworked, converted to > DocBook and merged with the existing DocBook DRM documentation stub. The > result doesn't cover the whole DRM API but should hopefully be good enough > for a start. > > I've done my best to follow a natural flow starting at initialization and > covering the major DRM internal topics. As I'm not a native English speaker > I'm not totally happy with the result, so if anyone wants to edit the text > please feel free to do so. Review will as usual be appreciated, and acks > will be even more welcome (I've been working on this document for longer > than I feel comfortable with). Just for information, a compiled copy of the DRM documentation with this patch applied can be found at http://www.ideasonboard.org/media/drm/. That might be easier to review than the patch. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Documentation: DocBook DRM framework documentation
On Thu, Jul 12, 2012 at 7:00 PM, Laurent Pinchart wrote: > Signed-off-by: Laurent Pinchart > --- > Documentation/DocBook/drm.tmpl | 2835 > +++- > 1 files changed, 2226 insertions(+), 609 deletions(-) > > Hi everybody, > > Here's the DRM kernel framework documentation previously posted to the > dri-devel mailing list. The documentation has been reworked, converted to > DocBook and merged with the existing DocBook DRM documentation stub. The > result doesn't cover the whole DRM API but should hopefully be good enough > for a start. > > I've done my best to follow a natural flow starting at initialization and > covering the major DRM internal topics. As I'm not a native English speaker > I'm not totally happy with the result, so if anyone wants to edit the text > please feel free to do so. Review will as usual be appreciated, and acks will > be even more welcome (I've been working on this document for longer than I > feel comfortable with). btw, thanks for this! One minor typo below.. with that, Reviewed-by: Rob Clark > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > index 196b8b9..44a2c66 100644 [snip] > + > + Output Polling > + void (*output_poll_changed)(struct drm_device > *dev); > + > +This operation notifies the driver that the status of one or more > +connectors has changed. Drivers that use the fbdev helper can just > call s/fbdev/fb/ > +the drm_fb_helper_hotplug_event function to > handle > +this operation. BR, -R
Re: [PATCH] Documentation: DocBook DRM framework documentation
On Thu, Jul 12, 2012 at 7:00 PM, Laurent Pinchart wrote: > Signed-off-by: Laurent Pinchart > --- > Documentation/DocBook/drm.tmpl | 2835 > +++- > 1 files changed, 2226 insertions(+), 609 deletions(-) > > Hi everybody, > > Here's the DRM kernel framework documentation previously posted to the > dri-devel mailing list. The documentation has been reworked, converted to > DocBook and merged with the existing DocBook DRM documentation stub. The > result doesn't cover the whole DRM API but should hopefully be good enough > for a start. > > I've done my best to follow a natural flow starting at initialization and > covering the major DRM internal topics. As I'm not a native English speaker > I'm not totally happy with the result, so if anyone wants to edit the text > please feel free to do so. Review will as usual be appreciated, and acks will > be even more welcome (I've been working on this document for longer than I > feel comfortable with). btw, thanks for this! One minor typo below.. with that, Reviewed-by: Rob Clark > diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl > index 196b8b9..44a2c66 100644 [snip] > + > + Output Polling > + void (*output_poll_changed)(struct drm_device > *dev); > + > +This operation notifies the driver that the status of one or more > +connectors has changed. Drivers that use the fbdev helper can just > call s/fbdev/fb/ > +the drm_fb_helper_hotplug_event function to > handle > +this operation. BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Documentation: DocBook DRM framework documentation
Signed-off-by: Laurent Pinchart --- Documentation/DocBook/drm.tmpl | 2835 +++- 1 files changed, 2226 insertions(+), 609 deletions(-) Hi everybody, Here's the DRM kernel framework documentation previously posted to the dri-devel mailing list. The documentation has been reworked, converted to DocBook and merged with the existing DocBook DRM documentation stub. The result doesn't cover the whole DRM API but should hopefully be good enough for a start. I've done my best to follow a natural flow starting at initialization and covering the major DRM internal topics. As I'm not a native English speaker I'm not totally happy with the result, so if anyone wants to edit the text please feel free to do so. Review will as usual be appreciated, and acks will be even more welcome (I've been working on this document for longer than I feel comfortable with). diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 196b8b9..44a2c66 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -6,11 +6,36 @@ Linux DRM Developer's Guide + + + Jesse + Barnes + Initial version + + Intel Corporation + + jesse.barnes at intel.com + + + + + Laurent + Pinchart + Driver internals + + Ideas on board SPRL + + laurent.pinchart at ideasonboard.com + + + + + 2008-2009 - - Intel Corporation (Jesse Barnes) - + 2012 + Intel Corporation + Laurent Pinchart @@ -20,6 +45,17 @@ the kernel source COPYING file. + + + + + 1.0 + 2012-07-13 + LP + Added extensive documentation about driver internals. + + + @@ -72,342 +108,361 @@ submission & fencing, suspend/resume support, and DMA services. - - The core of every DRM driver is struct drm_driver. Drivers - typically statically initialize a drm_driver structure, - then pass it to drm_init() at load time. - -Driver initialization - - Before calling the DRM initialization routines, the driver must - first create and fill out a struct drm_driver structure. - - - static struct drm_driver driver = { - /* Don't use MTRRs here; the Xserver or userspace app should -* deal with them for Intel hardware. -*/ - .driver_features = - DRIVER_USE_AGP | DRIVER_REQUIRE_AGP | - DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_MODESET, - .load = i915_driver_load, - .unload = i915_driver_unload, - .firstopen = i915_driver_firstopen, - .lastclose = i915_driver_lastclose, - .preclose = i915_driver_preclose, - .save = i915_save, - .restore = i915_restore, - .device_is_agp = i915_driver_device_is_agp, - .get_vblank_counter = i915_get_vblank_counter, - .enable_vblank = i915_enable_vblank, - .disable_vblank = i915_disable_vblank, - .irq_preinstall = i915_driver_irq_preinstall, - .irq_postinstall = i915_driver_irq_postinstall, - .irq_uninstall = i915_driver_irq_uninstall, - .irq_handler = i915_driver_irq_handler, - .reclaim_buffers = drm_core_reclaim_buffers, - .get_map_ofs = drm_core_get_map_ofs, - .get_reg_ofs = drm_core_get_reg_ofs, - .fb_probe = intelfb_probe, - .fb_remove = intelfb_remove, - .fb_resize = intelfb_resize, - .master_create = i915_master_create, - .master_destroy = i915_master_destroy, -#if defined(CONFIG_DEBUG_FS) - .debugfs_init = i915_debugfs_init, - .debugfs_cleanup = i915_debugfs_cleanup, -#endif - .gem_init_object = i915_gem_init_object, - .gem_free_object = i915_gem_free_object, - .gem_vm_ops = &i915_gem_vm_ops, - .ioctls = i915_ioctls, - .fops = { - .owner = THIS_MODULE, - .open = drm_open, - .release = drm_release, - .ioctl = drm_ioctl, - .mmap = drm_mmap, - .poll = drm_poll, - .fasync = drm_fasync, -#ifdef CONFIG_COMPAT - .compat_ioctl = i915_compat_ioctl, -#endif - .llseek = noop_llseek, - }, - .pci_driver = { - .name = DRIVER_NAME, - .id_table = pciidlist, - .probe = probe, - .remove = __devexit_p(drm_cleanup_pci), - }, - .name = DRIVER_NAME, - .desc = DRIVER_DESC, - .date = DRIVER_DATE, - .major = DRIVER_MAJOR, - .minor = DRIVER_MINOR, - .patchlevel = DRIVER_PATCHLEVEL, - }; - - - In the example above, taken from the i915 DRM driver, the driver - sets several flags indicating what core featur