| Message ID | 20170228171319.1786-7-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Daniel Vetter <daniel.vetter@ffwll.ch> writes: > I want to split up a few more things and document some details better > (like how exactly to subclass drm_atomic_state). And maybe also split > up the helpers a bit per-topic, but this should be a ok-ish start for > better atomic overview. > > One thing I failed at is getting DOT to layout the overview graph how > I want it. The highlevel structure I want is: > > Free-standing State > > Current State > > i.e. one over the other. Currently it lays it out side-by-side, but > not even that really - "Current State" is somewhat offset below. Makes > the graph look like garbage, and also way too wide for proper > rendering. Ideas appreciated. > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Harry Wentland <harry.wentland@amd.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Thanks for writing these docs. I wish I had them back when I was starting vc4's atomic code! With the two little spelling nits fixed, 3-5 are: Acked-by: Eric Anholt <eric@anholt.net> A few copyedits on this one below, but it sounds like you don't want to push quite yet while you sort out the rendering. > --- > Documentation/gpu/drm-kms-helpers.rst | 2 + > Documentation/gpu/drm-kms.rst | 85 ++++++++++++++++++++++++++++++++++- > 2 files changed, 86 insertions(+), 1 deletion(-) > > diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst > index 050ebe81d256..ac53c0b893f6 100644 > --- a/Documentation/gpu/drm-kms-helpers.rst > +++ b/Documentation/gpu/drm-kms-helpers.rst > @@ -42,6 +42,8 @@ Modeset Helper Reference for Common Vtables > .. kernel-doc:: include/drm/drm_modeset_helper_vtables.h > :internal: > > +.. _drm_atomic_helper: > + > Atomic Modeset Helper Functions Reference > ========================================= > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > index 20378881445f..979cee853bb1 100644 > --- a/Documentation/gpu/drm-kms.rst > +++ b/Documentation/gpu/drm-kms.rst > @@ -189,8 +189,91 @@ multiple times to different objects using :c:func:`drm_object_attach_property() > .. kernel-doc:: drivers/gpu/drm/drm_mode_object.c > :export: > > +Atomic Mode Setting > +=================== > + > + > +.. FIXME: The I want the below graph to be laid out so that the 2 subgraph > + clusters are below each another. But I failed. > + > +.. kernel-render:: DOT > + :alt: Mode Objects and Properties > + :caption: Mode Objects and Properties > + > + digraph { > + node [shape=box] > + > + subgraph cluster_state { > + style=dashed > + label="Free-standing state" > + > + "drm_atomic_state" -> "duplicated drm_plane_state A" > + "drm_atomic_state" -> "duplicated drm_plane_state B" > + "drm_atomic_state" -> "duplicated drm_crtc_state" > + "drm_atomic_state" -> "duplicated drm_connector_state" > + "drm_atomic_state" -> "duplicated driver private state" > + } > + > + subgraph cluster_current { > + style=dashed > + label="Current state" > + > + "drm_device" -> "drm_plane A" > + "drm_device" -> "drm_plane B" > + "drm_device" -> "drm_crtc" > + "drm_device" -> "drm_connector" > + "drm_device" -> "driver private object" > + > + "drm_plane A" -> "drm_plane_state A" > + "drm_plane B" -> "drm_plane_state B" > + "drm_crtc" -> "drm_crtc_state" > + "drm_connector" -> "drm_connector_state" > + "driver private object" -> "driver private state" > + } > + > + "drm_atomic_state" -> "drm_device" [label="atomic_commit"] > + } > + > +Essentially atomic is transactional modeset (including planes) updates, but > +compared to the usual transactional approach of try-commit and rollback on > +failure atomic modesetting is a bit different: Maybe reword: "Atomic provides transactional modeset (including planes) updates, but a bit differently from the usual transactional approach of try-commit and rollback:" > +- Firstly, no hardware changes are allowed when the commit would fail. This > + allows us to implement the DRM_MODE_ATOMIC_TEST_ONLY mode, which allows > + userspace to explore whether certain configurations would work or not. > + > +- This would still allows setting and rollback of just the software state, "allow" > + simplifying conversion of existing drivers. But auditing drivers for > + correctness of the atomic_check code because really hard with that. s/because/becomes/? > +Taken all together there's two consequence for the atomic design: "consequences" > + > +- The overall state is split up into per-object state structures: > + :c:type:`struct drm_plane_state <drm_plane_state>` for planes, :c:type:`struct > + drm_crtc_state <drm_crtc_state>` for CRTCs and :c:type:`struct > + drm_connector_state <drm_connector_state` for connectors. These are the only > + objects with userspace-visible and settable state. For internal state drivers > + can subclass these structures through embeddeding, or add entirely new state > + structures for their globally shared hardware functions. > + > +- An atomic update is assembled and validated as an enterily free-standing pile > + of structures within the :c:type:`drm_atomic_state <drm_atomic_state>` > + container. Again drivers can subclass that container for their own state > + structure tracking needs. Only when a state is commit is it applied to the "is committed" > + driver and modeset objects. This way rolling back an update boils down to > + releasing memory and unreference objects like framebuffers. "unreferencing"
On Tue, Feb 28, 2017 at 03:48:47PM -0800, Eric Anholt wrote: > Daniel Vetter <daniel.vetter@ffwll.ch> writes: > > > I want to split up a few more things and document some details better > > (like how exactly to subclass drm_atomic_state). And maybe also split > > up the helpers a bit per-topic, but this should be a ok-ish start for > > better atomic overview. > > > > One thing I failed at is getting DOT to layout the overview graph how > > I want it. The highlevel structure I want is: > > > > Free-standing State > > > > Current State > > > > i.e. one over the other. Currently it lays it out side-by-side, but > > not even that really - "Current State" is somewhat offset below. Makes > > the graph look like garbage, and also way too wide for proper > > rendering. Ideas appreciated. > > > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Cc: Harry Wentland <harry.wentland@amd.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Thanks for writing these docs. I wish I had them back when I was > starting vc4's atomic code! With the two little spelling nits fixed, > 3-5 are: > > Acked-by: Eric Anholt <eric@anholt.net> > > A few copyedits on this one below, but it sounds like you don't want to > push quite yet while you sort out the rendering. I've spent quite some time trying to beat DOT into submission, this is the best I can do. The FIXME really is just a hint for someone with more clue to maybe make it better, or if not possible at all, what would look better when doing a proper diagram with .svg or something like that. Assuming no one knows how to fix this, I'd still like to push it - it's still better than nothing imo, you just need to look at the picture full-screen. -Daniel > > > --- > > Documentation/gpu/drm-kms-helpers.rst | 2 + > > Documentation/gpu/drm-kms.rst | 85 ++++++++++++++++++++++++++++++++++- > > 2 files changed, 86 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst > > index 050ebe81d256..ac53c0b893f6 100644 > > --- a/Documentation/gpu/drm-kms-helpers.rst > > +++ b/Documentation/gpu/drm-kms-helpers.rst > > @@ -42,6 +42,8 @@ Modeset Helper Reference for Common Vtables > > .. kernel-doc:: include/drm/drm_modeset_helper_vtables.h > > :internal: > > > > +.. _drm_atomic_helper: > > + > > Atomic Modeset Helper Functions Reference > > ========================================= > > > > diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst > > index 20378881445f..979cee853bb1 100644 > > --- a/Documentation/gpu/drm-kms.rst > > +++ b/Documentation/gpu/drm-kms.rst > > @@ -189,8 +189,91 @@ multiple times to different objects using :c:func:`drm_object_attach_property() > > .. kernel-doc:: drivers/gpu/drm/drm_mode_object.c > > :export: > > > > +Atomic Mode Setting > > +=================== > > + > > + > > +.. FIXME: The I want the below graph to be laid out so that the 2 subgraph > > + clusters are below each another. But I failed. > > + > > +.. kernel-render:: DOT > > + :alt: Mode Objects and Properties > > + :caption: Mode Objects and Properties > > + > > + digraph { > > + node [shape=box] > > + > > + subgraph cluster_state { > > + style=dashed > > + label="Free-standing state" > > + > > + "drm_atomic_state" -> "duplicated drm_plane_state A" > > + "drm_atomic_state" -> "duplicated drm_plane_state B" > > + "drm_atomic_state" -> "duplicated drm_crtc_state" > > + "drm_atomic_state" -> "duplicated drm_connector_state" > > + "drm_atomic_state" -> "duplicated driver private state" > > + } > > + > > + subgraph cluster_current { > > + style=dashed > > + label="Current state" > > + > > + "drm_device" -> "drm_plane A" > > + "drm_device" -> "drm_plane B" > > + "drm_device" -> "drm_crtc" > > + "drm_device" -> "drm_connector" > > + "drm_device" -> "driver private object" > > + > > + "drm_plane A" -> "drm_plane_state A" > > + "drm_plane B" -> "drm_plane_state B" > > + "drm_crtc" -> "drm_crtc_state" > > + "drm_connector" -> "drm_connector_state" > > + "driver private object" -> "driver private state" > > + } > > + > > + "drm_atomic_state" -> "drm_device" [label="atomic_commit"] > > + } > > + > > +Essentially atomic is transactional modeset (including planes) updates, but > > +compared to the usual transactional approach of try-commit and rollback on > > +failure atomic modesetting is a bit different: > > Maybe reword: > > "Atomic provides transactional modeset (including planes) updates, but a > bit differently from the usual transactional approach of try-commit and > rollback:" > > > +- Firstly, no hardware changes are allowed when the commit would fail. This > > + allows us to implement the DRM_MODE_ATOMIC_TEST_ONLY mode, which allows > > + userspace to explore whether certain configurations would work or not. > > + > > +- This would still allows setting and rollback of just the software state, > > "allow" > > > + simplifying conversion of existing drivers. But auditing drivers for > > + correctness of the atomic_check code because really hard with that. > > s/because/becomes/? > > > +Taken all together there's two consequence for the atomic design: > > "consequences" > > > + > > +- The overall state is split up into per-object state structures: > > + :c:type:`struct drm_plane_state <drm_plane_state>` for planes, :c:type:`struct > > + drm_crtc_state <drm_crtc_state>` for CRTCs and :c:type:`struct > > + drm_connector_state <drm_connector_state` for connectors. These are the only > > + objects with userspace-visible and settable state. For internal state drivers > > + can subclass these structures through embeddeding, or add entirely new state > > + structures for their globally shared hardware functions. > > + > > +- An atomic update is assembled and validated as an enterily free-standing pile > > + of structures within the :c:type:`drm_atomic_state <drm_atomic_state>` > > + container. Again drivers can subclass that container for their own state > > + structure tracking needs. Only when a state is commit is it applied to the > > "is committed" > > > + driver and modeset objects. This way rolling back an update boils down to > > + releasing memory and unreference objects like framebuffers. > > "unreferencing"
Daniel Vetter <daniel@ffwll.ch> writes: > I've spent quite some time trying to beat DOT into submission, this is the > best I can do. The FIXME really is just a hint for someone with more clue > to maybe make it better, or if not possible at all, what would look better > when doing a proper diagram with .svg or something like that. > > Assuming no one knows how to fix this, I'd still like to push it - it's > still better than nothing imo, you just need to look at the picture > full-screen. If you add a hidden edge from any of the "duplicated" nodes to the drm_device node, the second cluster will be pushed to the third rank and do what I think you want. Something like: "duplicated drm_plane_state A" -> "drm_device"[style=invis] This is the result for me: https://people.collabora.com/~krisman/atomic_modesetting.svg I think it got a little better.
On Wed, Mar 01, 2017 at 02:42:02PM -0300, Gabriel Krisman Bertazi wrote: > Daniel Vetter <daniel@ffwll.ch> writes: > > > I've spent quite some time trying to beat DOT into submission, this is the > > best I can do. The FIXME really is just a hint for someone with more clue > > to maybe make it better, or if not possible at all, what would look better > > when doing a proper diagram with .svg or something like that. > > > > Assuming no one knows how to fix this, I'd still like to push it - it's > > still better than nothing imo, you just need to look at the picture > > full-screen. > > If you add a hidden edge from any of the "duplicated" nodes to the > drm_device node, the second cluster will be pushed to the third rank > and do what I think you want. > > Something like: > > "duplicated drm_plane_state A" -> "drm_device"[style=invis] > > This is the result for me: > > https://people.collabora.com/~krisman/atomic_modesetting.svg > > I think it got a little better. Yeah, much better, thanks for the suggestion. Still rather small-ish, but it's a busy graph, so that can't be helped really. -Daniel
diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst index 050ebe81d256..ac53c0b893f6 100644 --- a/Documentation/gpu/drm-kms-helpers.rst +++ b/Documentation/gpu/drm-kms-helpers.rst @@ -42,6 +42,8 @@ Modeset Helper Reference for Common Vtables .. kernel-doc:: include/drm/drm_modeset_helper_vtables.h :internal: +.. _drm_atomic_helper: + Atomic Modeset Helper Functions Reference ========================================= diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index 20378881445f..979cee853bb1 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -189,8 +189,91 @@ multiple times to different objects using :c:func:`drm_object_attach_property() .. kernel-doc:: drivers/gpu/drm/drm_mode_object.c :export: +Atomic Mode Setting +=================== + + +.. FIXME: The I want the below graph to be laid out so that the 2 subgraph + clusters are below each another. But I failed. + +.. kernel-render:: DOT + :alt: Mode Objects and Properties + :caption: Mode Objects and Properties + + digraph { + node [shape=box] + + subgraph cluster_state { + style=dashed + label="Free-standing state" + + "drm_atomic_state" -> "duplicated drm_plane_state A" + "drm_atomic_state" -> "duplicated drm_plane_state B" + "drm_atomic_state" -> "duplicated drm_crtc_state" + "drm_atomic_state" -> "duplicated drm_connector_state" + "drm_atomic_state" -> "duplicated driver private state" + } + + subgraph cluster_current { + style=dashed + label="Current state" + + "drm_device" -> "drm_plane A" + "drm_device" -> "drm_plane B" + "drm_device" -> "drm_crtc" + "drm_device" -> "drm_connector" + "drm_device" -> "driver private object" + + "drm_plane A" -> "drm_plane_state A" + "drm_plane B" -> "drm_plane_state B" + "drm_crtc" -> "drm_crtc_state" + "drm_connector" -> "drm_connector_state" + "driver private object" -> "driver private state" + } + + "drm_atomic_state" -> "drm_device" [label="atomic_commit"] + } + +Essentially atomic is transactional modeset (including planes) updates, but +compared to the usual transactional approach of try-commit and rollback on +failure atomic modesetting is a bit different: + +- Firstly, no hardware changes are allowed when the commit would fail. This + allows us to implement the DRM_MODE_ATOMIC_TEST_ONLY mode, which allows + userspace to explore whether certain configurations would work or not. + +- This would still allows setting and rollback of just the software state, + simplifying conversion of existing drivers. But auditing drivers for + correctness of the atomic_check code because really hard with that. + +- Lastly, for backwards compatibility and to support all use-cases, atomic + updates need to be incremental and be able to execute in parallel. Hardware + doesn't always allow it, but where possible plane updates on different CRTCs + should not interfere, and not get stalled due to output routing changing on + different CRTCs. + +Taken all together there's two consequence for the atomic design: + +- The overall state is split up into per-object state structures: + :c:type:`struct drm_plane_state <drm_plane_state>` for planes, :c:type:`struct + drm_crtc_state <drm_crtc_state>` for CRTCs and :c:type:`struct + drm_connector_state <drm_connector_state` for connectors. These are the only + objects with userspace-visible and settable state. For internal state drivers + can subclass these structures through embeddeding, or add entirely new state + structures for their globally shared hardware functions. + +- An atomic update is assembled and validated as an enterily free-standing pile + of structures within the :c:type:`drm_atomic_state <drm_atomic_state>` + container. Again drivers can subclass that container for their own state + structure tracking needs. Only when a state is commit is it applied to the + driver and modeset objects. This way rolling back an update boils down to + releasing memory and unreference objects like framebuffers. + +Read on in this chapter, and also in :ref:`drm_atomic_helper` for more detailed +coverage of specific topics. + Atomic Mode Setting Function Reference -====================================== +-------------------------------------- .. kernel-doc:: include/drm/drm_atomic.h :internal:
I want to split up a few more things and document some details better (like how exactly to subclass drm_atomic_state). And maybe also split up the helpers a bit per-topic, but this should be a ok-ish start for better atomic overview. One thing I failed at is getting DOT to layout the overview graph how I want it. The highlevel structure I want is: Free-standing State Current State i.e. one over the other. Currently it lays it out side-by-side, but not even that really - "Current State" is somewhat offset below. Makes the graph look like garbage, and also way too wide for proper rendering. Ideas appreciated. Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Harry Wentland <harry.wentland@amd.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- Documentation/gpu/drm-kms-helpers.rst | 2 + Documentation/gpu/drm-kms.rst | 85 ++++++++++++++++++++++++++++++++++- 2 files changed, 86 insertions(+), 1 deletion(-)