| Message ID | 20170125062657.19270-16-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday, 2017-01-25 07:26:57 +0100, Daniel Vetter wrote: > After going through all the trouble of splitting out parts from > drm_crtc.[hc] and then properly documenting each I've entirely > forgotten to show the same TLC for CRTCs themselves! > > Let's make amends asap. > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > Documentation/gpu/drm-kms.rst | 6 ++++++ > drivers/gpu/drm/drm_crtc.c | 21 +++++++++++++++++++++ > include/drm/drm_crtc.h | 25 +++++++++++++++++++------ > 3 files changed, 46 insertions(+), 6 deletions(-) > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > index 35c41cf84a1b..979cee853bb1 100644 > --- a/Documentation/gpu/drm-kms.rst > +++ b/Documentation/gpu/drm-kms.rst > @@ -284,6 +284,12 @@ Atomic Mode Setting Function Reference > CRTC Abstraction > ================ > > +.. kernel-doc:: drivers/gpu/drm/drm_crtc.c > + :doc: overview > + > +CRTC Functions Reference > +-------------------------------- > + > .. kernel-doc:: include/drm/drm_crtc.h > :internal: > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 5f28e3a5a3e0..6915f897bd8e 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -47,6 +47,27 @@ > #include "drm_internal.h" > > /** > + * DOC: overview > + * > + * A CRTC represents the overall display pipeline. It receives pixel data from > + * &drm_plane and blends them together. The &drm_display_mode is also attached > + * to the CRTC, specifying display timings. On the output side the data is fed > + * to one or more &drm_encoder, which are then each connected to one > + * &drm_connector. > + * > + * To create a CRTC, a KMS drivers allocates and zeroes an instances of > + * &struct drm_crtc (possibly as part of a larger structure) and registers it > + * with a call to drm_crtc_init_with_planes(). > + * > + * The CRTC is also the entry point for legacy modeset operations, see > + * &drm_crtc_funcs.set_config, legacy plane operations, see > + * &drm_crtc_funcs.page_flip and &drm_crtc_funcs.cursor_set2, and other legacy > + * operations like &drm_crtc_funcs.gamma_set. For atomic drivers all these > + * features are controlled through &drm_property and > + * &drm_mode_config_funcs.atomic_check and &drm_mode_config_funcs.atomic_check. > + */ > + > +/** > * drm_crtc_from_index - find the registered CRTC at an index > * @dev: DRM device > * @idx: index of registered CRTC to find for > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 816edab054e6..d788c624f67a 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -641,7 +641,7 @@ struct drm_crtc { > * > * This provides a read lock for the overall crtc state (mode, dpms > * state, ...) and a write lock for everything which can be update > - * without a full modeset (fb, cursor data, crtc properties ...). Full > + * without a full modeset (fb, cursor data, crtc properties ...). A full > * modeset also need to grab &drm_mode_config.connection_mutex. > */ > struct drm_modeset_lock mutex; > @@ -774,10 +774,8 @@ struct drm_crtc { > * @connectors: array of connectors to drive with this CRTC if possible > * @num_connectors: size of @connectors array > * > - * Represents a single crtc the connectors that it drives with what mode > - * and from which framebuffer it scans out from. > - * > - * This is used to set modes. > + * This represents a modeset configuration for the legacy SETCRTC ioctl and is > + * also used internally. Atomic drivers instead use &drm_atomic_state. > */ > struct drm_mode_set { > struct drm_framebuffer *fb; > @@ -832,7 +830,15 @@ int drm_crtc_force_disable_all(struct drm_device *dev); > int drm_mode_set_config_internal(struct drm_mode_set *set); > struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx); > > -/* Helpers */ > +/** > + * drm_crtc_find - look up a CRTC object from its ID > + * @dev: DRM device > + * @id: &drm_mode_object ID > + * > + * This can be used to look up a CRTC from its userspace ID. Only used by > + * drivers for legacy IOCTLs and interface, nowadays extensions to the KMS > + * userspace interface should be done using &drm_property. > + */ > static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev, > uint32_t id) > { > @@ -841,6 +847,13 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev, > return mo ? obj_to_crtc(mo) : NULL; > } > > +/** > + * drm_for_each_crtc - iterate over all encoders s/encoder/crtc/ :) Other than that, this and #3 (btw I know you didn't introduce any of the typos in #3, but you can fix them :P) are: Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com> > + * @crtc: a &struct drm_crtc as the loop cursor > + * @dev: the &struct drm_device > + * > + * Iterate over all CRTCs of @dev. > + */ > #define drm_for_each_crtc(crtc, dev) \ > list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head) > > -- > 2.11.0 >
On Wed, Jan 25, 2017 at 02:26:09PM +0000, Eric Engestrom wrote: > On Wednesday, 2017-01-25 07:26:57 +0100, Daniel Vetter wrote: > > After going through all the trouble of splitting out parts from > > drm_crtc.[hc] and then properly documenting each I've entirely > > forgotten to show the same TLC for CRTCs themselves! > > > > Let's make amends asap. > > > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > Documentation/gpu/drm-kms.rst | 6 ++++++ > > drivers/gpu/drm/drm_crtc.c | 21 +++++++++++++++++++++ > > include/drm/drm_crtc.h | 25 +++++++++++++++++++------ > > 3 files changed, 46 insertions(+), 6 deletions(-) > > > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > > index 35c41cf84a1b..979cee853bb1 100644 > > --- a/Documentation/gpu/drm-kms.rst > > +++ b/Documentation/gpu/drm-kms.rst > > @@ -284,6 +284,12 @@ Atomic Mode Setting Function Reference > > CRTC Abstraction > > ================ > > > > +.. kernel-doc:: drivers/gpu/drm/drm_crtc.c > > + :doc: overview > > + > > +CRTC Functions Reference > > +-------------------------------- > > + > > .. kernel-doc:: include/drm/drm_crtc.h > > :internal: > > > > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > > index 5f28e3a5a3e0..6915f897bd8e 100644 > > --- a/drivers/gpu/drm/drm_crtc.c > > +++ b/drivers/gpu/drm/drm_crtc.c > > @@ -47,6 +47,27 @@ > > #include "drm_internal.h" > > > > /** > > + * DOC: overview > > + * > > + * A CRTC represents the overall display pipeline. It receives pixel data from > > + * &drm_plane and blends them together. The &drm_display_mode is also attached > > + * to the CRTC, specifying display timings. On the output side the data is fed > > + * to one or more &drm_encoder, which are then each connected to one > > + * &drm_connector. > > + * > > + * To create a CRTC, a KMS drivers allocates and zeroes an instances of > > + * &struct drm_crtc (possibly as part of a larger structure) and registers it > > + * with a call to drm_crtc_init_with_planes(). > > + * > > + * The CRTC is also the entry point for legacy modeset operations, see > > + * &drm_crtc_funcs.set_config, legacy plane operations, see > > + * &drm_crtc_funcs.page_flip and &drm_crtc_funcs.cursor_set2, and other legacy > > + * operations like &drm_crtc_funcs.gamma_set. For atomic drivers all these > > + * features are controlled through &drm_property and > > + * &drm_mode_config_funcs.atomic_check and &drm_mode_config_funcs.atomic_check. > > + */ > > + > > +/** > > * drm_crtc_from_index - find the registered CRTC at an index > > * @dev: DRM device > > * @idx: index of registered CRTC to find for > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > > index 816edab054e6..d788c624f67a 100644 > > --- a/include/drm/drm_crtc.h > > +++ b/include/drm/drm_crtc.h > > @@ -641,7 +641,7 @@ struct drm_crtc { > > * > > * This provides a read lock for the overall crtc state (mode, dpms > > * state, ...) and a write lock for everything which can be update > > - * without a full modeset (fb, cursor data, crtc properties ...). Full > > + * without a full modeset (fb, cursor data, crtc properties ...). A full > > * modeset also need to grab &drm_mode_config.connection_mutex. > > */ > > struct drm_modeset_lock mutex; > > @@ -774,10 +774,8 @@ struct drm_crtc { > > * @connectors: array of connectors to drive with this CRTC if possible > > * @num_connectors: size of @connectors array > > * > > - * Represents a single crtc the connectors that it drives with what mode > > - * and from which framebuffer it scans out from. > > - * > > - * This is used to set modes. > > + * This represents a modeset configuration for the legacy SETCRTC ioctl and is > > + * also used internally. Atomic drivers instead use &drm_atomic_state. > > */ > > struct drm_mode_set { > > struct drm_framebuffer *fb; > > @@ -832,7 +830,15 @@ int drm_crtc_force_disable_all(struct drm_device *dev); > > int drm_mode_set_config_internal(struct drm_mode_set *set); > > struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx); > > > > -/* Helpers */ > > +/** > > + * drm_crtc_find - look up a CRTC object from its ID > > + * @dev: DRM device > > + * @id: &drm_mode_object ID > > + * > > + * This can be used to look up a CRTC from its userspace ID. Only used by > > + * drivers for legacy IOCTLs and interface, nowadays extensions to the KMS > > + * userspace interface should be done using &drm_property. > > + */ > > static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev, > > uint32_t id) > > { > > @@ -841,6 +847,13 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev, > > return mo ? obj_to_crtc(mo) : NULL; > > } > > > > +/** > > + * drm_for_each_crtc - iterate over all encoders > > s/encoder/crtc/ :) > > Other than that, this and #3 (btw I know you didn't introduce any of the > typos in #3, but you can fix them :P) are: > Reviewed-by: Eric Engestrom <eric.engestrom@imgtec.com> Typos all fixed and both of these applied, thanks for your review. -Daniel > > > + * @crtc: a &struct drm_crtc as the loop cursor > > + * @dev: the &struct drm_device > > + * > > + * Iterate over all CRTCs of @dev. > > + */ > > #define drm_for_each_crtc(crtc, dev) \ > > list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head) > > > > -- > > 2.11.0 > >
diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 35c41cf84a1b..979cee853bb1 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -284,6 +284,12 @@ Atomic Mode Setting Function Reference CRTC Abstraction ================ +.. kernel-doc:: drivers/gpu/drm/drm_crtc.c + :doc: overview + +CRTC Functions Reference +-------------------------------- + .. kernel-doc:: include/drm/drm_crtc.h :internal: diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 5f28e3a5a3e0..6915f897bd8e 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -47,6 +47,27 @@ #include "drm_internal.h" /** + * DOC: overview + * + * A CRTC represents the overall display pipeline. It receives pixel data from + * &drm_plane and blends them together. The &drm_display_mode is also attached + * to the CRTC, specifying display timings. On the output side the data is fed + * to one or more &drm_encoder, which are then each connected to one + * &drm_connector. + * + * To create a CRTC, a KMS drivers allocates and zeroes an instances of + * &struct drm_crtc (possibly as part of a larger structure) and registers it + * with a call to drm_crtc_init_with_planes(). + * + * The CRTC is also the entry point for legacy modeset operations, see + * &drm_crtc_funcs.set_config, legacy plane operations, see + * &drm_crtc_funcs.page_flip and &drm_crtc_funcs.cursor_set2, and other legacy + * operations like &drm_crtc_funcs.gamma_set. For atomic drivers all these + * features are controlled through &drm_property and + * &drm_mode_config_funcs.atomic_check and &drm_mode_config_funcs.atomic_check. + */ + +/** * drm_crtc_from_index - find the registered CRTC at an index * @dev: DRM device * @idx: index of registered CRTC to find for diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 816edab054e6..d788c624f67a 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -641,7 +641,7 @@ struct drm_crtc { * * This provides a read lock for the overall crtc state (mode, dpms * state, ...) and a write lock for everything which can be update - * without a full modeset (fb, cursor data, crtc properties ...). Full + * without a full modeset (fb, cursor data, crtc properties ...). A full * modeset also need to grab &drm_mode_config.connection_mutex. */ struct drm_modeset_lock mutex; @@ -774,10 +774,8 @@ struct drm_crtc { * @connectors: array of connectors to drive with this CRTC if possible * @num_connectors: size of @connectors array * - * Represents a single crtc the connectors that it drives with what mode - * and from which framebuffer it scans out from. - * - * This is used to set modes. + * This represents a modeset configuration for the legacy SETCRTC ioctl and is + * also used internally. Atomic drivers instead use &drm_atomic_state. */ struct drm_mode_set { struct drm_framebuffer *fb; @@ -832,7 +830,15 @@ int drm_crtc_force_disable_all(struct drm_device *dev); int drm_mode_set_config_internal(struct drm_mode_set *set); struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx); -/* Helpers */ +/** + * drm_crtc_find - look up a CRTC object from its ID + * @dev: DRM device + * @id: &drm_mode_object ID + * + * This can be used to look up a CRTC from its userspace ID. Only used by + * drivers for legacy IOCTLs and interface, nowadays extensions to the KMS + * userspace interface should be done using &drm_property. + */ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev, uint32_t id) { @@ -841,6 +847,13 @@ static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev, return mo ? obj_to_crtc(mo) : NULL; } +/** + * drm_for_each_crtc - iterate over all encoders + * @crtc: a &struct drm_crtc as the loop cursor + * @dev: the &struct drm_device + * + * Iterate over all CRTCs of @dev. + */ #define drm_for_each_crtc(crtc, dev) \ list_for_each_entry(crtc, &(dev)->mode_config.crtc_list, head)
After going through all the trouble of splitting out parts from drm_crtc.[hc] and then properly documenting each I've entirely forgotten to show the same TLC for CRTCs themselves! Let's make amends asap. Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- Documentation/gpu/drm-kms.rst | 6 ++++++ drivers/gpu/drm/drm_crtc.c | 21 +++++++++++++++++++++ include/drm/drm_crtc.h | 25 +++++++++++++++++++------ 3 files changed, 46 insertions(+), 6 deletions(-)