| Message ID | 20170328155349.5972-3-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 28, 2017 at 11:53 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > Mostly because I want the links from the newly-added @state functions > to work. But I think explaining when they're useful and that the > implicit one is deprecated is good either way. Slightly repetitive > unfortunately. > > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Series is: Reviewed-by: Alex Deucher <alexander.deucher@amd.com> > --- > include/drm/drm_atomic.h | 159 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 158 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 0147a047878d..fd33ed5eaeb4 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -498,6 +498,23 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); > > void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > +/** > + * for_each_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @connector_state: &struct drm_connector_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_connector_in_state(__state, connector, connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -506,6 +523,20 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_oldnew_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > + * old state > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > + * new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -515,6 +546,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_old_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > + * old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -523,6 +566,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_new_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > + * new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -531,6 +586,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_crtc_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @crtc_state: &struct drm_crtc_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_crtc_in_state(__state, crtc, crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -539,6 +611,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc_state) > > +/** > + * for_each_oldnew_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -548,6 +632,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_old_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -556,6 +651,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_new_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -564,6 +670,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @plane_state: &struct drm_plane_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_plane_in_state(__state, plane, plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -572,6 +695,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane_state) > > +/** > + * for_each_oldnew_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -581,6 +716,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane) > > +/** > + * for_each_old_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -589,6 +735,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane) > > +/** > + * for_each_new_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -603,7 +760,7 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > * > * To give drivers flexibility &struct drm_crtc_state has 3 booleans to track > * whether the state CRTC changed enough to need a full modeset cycle: > - * connectors_changed, mode_changed and active_changed. This helper simply > + * planes_changed, mode_changed and active_changed. This helper simply > * combines these three to compute the overall need for a modeset for @state. > * > * The atomic helper code sets these booleans, but drivers can and should > -- > 2.11.0 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Patches 1-3 are Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry On 2017-03-28 11:53 AM, Daniel Vetter wrote: > Mostly because I want the links from the newly-added @state functions > to work. But I think explaining when they're useful and that the > implicit one is deprecated is good either way. Slightly repetitive > unfortunately. > > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > include/drm/drm_atomic.h | 159 ++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 158 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index 0147a047878d..fd33ed5eaeb4 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -498,6 +498,23 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); > > void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > +/** > + * for_each_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @connector_state: &struct drm_connector_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_connector_in_state(__state, connector, connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -506,6 +523,20 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_oldnew_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > + * old state > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > + * new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -515,6 +546,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_old_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > + * old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -523,6 +566,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_new_connector_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @connector: &struct drm_connector iteration cursor > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > + * new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all connectors in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->num_connector && \ > @@ -531,6 +586,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (connector) > > +/** > + * for_each_crtc_in_state - iterate over all connectors in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @crtc_state: &struct drm_crtc_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_crtc_in_state(__state, crtc, crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -539,6 +611,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc_state) > > +/** > + * for_each_oldnew_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -548,6 +632,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_old_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -556,6 +651,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_new_crtc_in_state - iterate over all CRTCs in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @crtc: &struct drm_crtc iteration cursor > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all CRTCs in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_crtc && \ > @@ -564,6 +670,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (crtc) > > +/** > + * for_each_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @plane_state: &struct drm_plane_state iteration cursor > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update. Note that before the > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > + * points to the new state, while afterwards it points to the old state. Due to > + * this tricky confusion this macro is deprecated. > + * > + * FIXME: > + * > + * Replace all usage of this with one of the explicit iterators below and then > + * remove this macro. > + */ > #define for_each_plane_in_state(__state, plane, plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -572,6 +695,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane_state) > > +/** > + * for_each_oldnew_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking both old and > + * new state. This is useful in places where the state delta needs to be > + * considered, for example in atomic check functions. > + */ > #define for_each_oldnew_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -581,6 +716,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane) > > +/** > + * for_each_old_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking only the old > + * state. This is useful in disable functions, where we need the old state the > + * hardware is still in. > + */ > #define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -589,6 +735,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > (__i)++) \ > for_each_if (plane) > > +/** > + * for_each_new_plane_in_state - iterate over all planes in an atomic update > + * @__state: &struct drm_atomic_state pointer > + * @plane: &struct drm_plane iteration cursor > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > + * @__i: int iteration cursor, for macro-internal use > + * > + * This iterates over all planes in an atomic update, tracking only the new > + * state. This is useful in enable functions, where we need the new state the > + * hardware should be in when the atomic commit operation has completed. > + */ > #define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \ > for ((__i) = 0; \ > (__i) < (__state)->dev->mode_config.num_total_plane && \ > @@ -603,7 +760,7 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > * > * To give drivers flexibility &struct drm_crtc_state has 3 booleans to track > * whether the state CRTC changed enough to need a full modeset cycle: > - * connectors_changed, mode_changed and active_changed. This helper simply > + * planes_changed, mode_changed and active_changed. This helper simply > * combines these three to compute the overall need for a modeset for @state. > * > * The atomic helper code sets these booleans, but drivers can and should >
On Tue, Mar 28, 2017 at 12:10:37PM -0400, Alex Deucher wrote: > On Tue, Mar 28, 2017 at 11:53 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote: > > Mostly because I want the links from the newly-added @state functions > > to work. But I think explaining when they're useful and that the > > implicit one is deprecated is good either way. Slightly repetitive > > unfortunately. > > > > Cc: Harry Wentland <harry.wentland@amd.com> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > Series is: > Reviewed-by: Alex Deucher <alexander.deucher@amd.com> Thanks a lot for your reviews, all applied to -misc. -Daniel > > > --- > > include/drm/drm_atomic.h | 159 ++++++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 158 insertions(+), 1 deletion(-) > > > > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > > index 0147a047878d..fd33ed5eaeb4 100644 > > --- a/include/drm/drm_atomic.h > > +++ b/include/drm/drm_atomic.h > > @@ -498,6 +498,23 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); > > > > void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > > > +/** > > + * for_each_connector_in_state - iterate over all connectors in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @connector: &struct drm_connector iteration cursor > > + * @connector_state: &struct drm_connector_state iteration cursor > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all connectors in an atomic update. Note that before the > > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > > + * points to the new state, while afterwards it points to the old state. Due to > > + * this tricky confusion this macro is deprecated. > > + * > > + * FIXME: > > + * > > + * Replace all usage of this with one of the explicit iterators below and then > > + * remove this macro. > > + */ > > #define for_each_connector_in_state(__state, connector, connector_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->num_connector && \ > > @@ -506,6 +523,20 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (connector) > > > > +/** > > + * for_each_oldnew_connector_in_state - iterate over all connectors in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @connector: &struct drm_connector iteration cursor > > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > > + * old state > > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > > + * new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all connectors in an atomic update, tracking both old and > > + * new state. This is useful in places where the state delta needs to be > > + * considered, for example in atomic check functions. > > + */ > > #define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->num_connector && \ > > @@ -515,6 +546,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (connector) > > > > +/** > > + * for_each_old_connector_in_state - iterate over all connectors in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @connector: &struct drm_connector iteration cursor > > + * @old_connector_state: &struct drm_connector_state iteration cursor for the > > + * old state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all connectors in an atomic update, tracking only the old > > + * state. This is useful in disable functions, where we need the old state the > > + * hardware is still in. > > + */ > > #define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->num_connector && \ > > @@ -523,6 +566,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (connector) > > > > +/** > > + * for_each_new_connector_in_state - iterate over all connectors in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @connector: &struct drm_connector iteration cursor > > + * @new_connector_state: &struct drm_connector_state iteration cursor for the > > + * new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all connectors in an atomic update, tracking only the new > > + * state. This is useful in enable functions, where we need the new state the > > + * hardware should be in when the atomic commit operation has completed. > > + */ > > #define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->num_connector && \ > > @@ -531,6 +586,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (connector) > > > > +/** > > + * for_each_crtc_in_state - iterate over all connectors in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @crtc: &struct drm_crtc iteration cursor > > + * @crtc_state: &struct drm_crtc_state iteration cursor > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all CRTCs in an atomic update. Note that before the > > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > > + * points to the new state, while afterwards it points to the old state. Due to > > + * this tricky confusion this macro is deprecated. > > + * > > + * FIXME: > > + * > > + * Replace all usage of this with one of the explicit iterators below and then > > + * remove this macro. > > + */ > > #define for_each_crtc_in_state(__state, crtc, crtc_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_crtc && \ > > @@ -539,6 +611,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (crtc_state) > > > > +/** > > + * for_each_oldnew_crtc_in_state - iterate over all CRTCs in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @crtc: &struct drm_crtc iteration cursor > > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all CRTCs in an atomic update, tracking both old and > > + * new state. This is useful in places where the state delta needs to be > > + * considered, for example in atomic check functions. > > + */ > > #define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_crtc && \ > > @@ -548,6 +632,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (crtc) > > > > +/** > > + * for_each_old_crtc_in_state - iterate over all CRTCs in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @crtc: &struct drm_crtc iteration cursor > > + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all CRTCs in an atomic update, tracking only the old > > + * state. This is useful in disable functions, where we need the old state the > > + * hardware is still in. > > + */ > > #define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_crtc && \ > > @@ -556,6 +651,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (crtc) > > > > +/** > > + * for_each_new_crtc_in_state - iterate over all CRTCs in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @crtc: &struct drm_crtc iteration cursor > > + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all CRTCs in an atomic update, tracking only the new > > + * state. This is useful in enable functions, where we need the new state the > > + * hardware should be in when the atomic commit operation has completed. > > + */ > > #define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_crtc && \ > > @@ -564,6 +670,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (crtc) > > > > +/** > > + * for_each_plane_in_state - iterate over all planes in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @plane: &struct drm_plane iteration cursor > > + * @plane_state: &struct drm_plane_state iteration cursor > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all planes in an atomic update. Note that before the > > + * software state is committed (by calling drm_atomic_helper_swap_state(), this > > + * points to the new state, while afterwards it points to the old state. Due to > > + * this tricky confusion this macro is deprecated. > > + * > > + * FIXME: > > + * > > + * Replace all usage of this with one of the explicit iterators below and then > > + * remove this macro. > > + */ > > #define for_each_plane_in_state(__state, plane, plane_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_total_plane && \ > > @@ -572,6 +695,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (plane_state) > > > > +/** > > + * for_each_oldnew_plane_in_state - iterate over all planes in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @plane: &struct drm_plane iteration cursor > > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all planes in an atomic update, tracking both old and > > + * new state. This is useful in places where the state delta needs to be > > + * considered, for example in atomic check functions. > > + */ > > #define for_each_oldnew_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_total_plane && \ > > @@ -581,6 +716,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (plane) > > > > +/** > > + * for_each_old_plane_in_state - iterate over all planes in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @plane: &struct drm_plane iteration cursor > > + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all planes in an atomic update, tracking only the old > > + * state. This is useful in disable functions, where we need the old state the > > + * hardware is still in. > > + */ > > #define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_total_plane && \ > > @@ -589,6 +735,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > (__i)++) \ > > for_each_if (plane) > > > > +/** > > + * for_each_new_plane_in_state - iterate over all planes in an atomic update > > + * @__state: &struct drm_atomic_state pointer > > + * @plane: &struct drm_plane iteration cursor > > + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state > > + * @__i: int iteration cursor, for macro-internal use > > + * > > + * This iterates over all planes in an atomic update, tracking only the new > > + * state. This is useful in enable functions, where we need the new state the > > + * hardware should be in when the atomic commit operation has completed. > > + */ > > #define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \ > > for ((__i) = 0; \ > > (__i) < (__state)->dev->mode_config.num_total_plane && \ > > @@ -603,7 +760,7 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); > > * > > * To give drivers flexibility &struct drm_crtc_state has 3 booleans to track > > * whether the state CRTC changed enough to need a full modeset cycle: > > - * connectors_changed, mode_changed and active_changed. This helper simply > > + * planes_changed, mode_changed and active_changed. This helper simply > > * combines these three to compute the overall need for a modeset for @state. > > * > > * The atomic helper code sets these booleans, but drivers can and should > > -- > > 2.11.0 > > > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 0147a047878d..fd33ed5eaeb4 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -498,6 +498,23 @@ int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); void drm_state_dump(struct drm_device *dev, struct drm_printer *p); +/** + * for_each_connector_in_state - iterate over all connectors in an atomic update + * @__state: &struct drm_atomic_state pointer + * @connector: &struct drm_connector iteration cursor + * @connector_state: &struct drm_connector_state iteration cursor + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all connectors in an atomic update. Note that before the + * software state is committed (by calling drm_atomic_helper_swap_state(), this + * points to the new state, while afterwards it points to the old state. Due to + * this tricky confusion this macro is deprecated. + * + * FIXME: + * + * Replace all usage of this with one of the explicit iterators below and then + * remove this macro. + */ #define for_each_connector_in_state(__state, connector, connector_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->num_connector && \ @@ -506,6 +523,20 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (connector) +/** + * for_each_oldnew_connector_in_state - iterate over all connectors in an atomic update + * @__state: &struct drm_atomic_state pointer + * @connector: &struct drm_connector iteration cursor + * @old_connector_state: &struct drm_connector_state iteration cursor for the + * old state + * @new_connector_state: &struct drm_connector_state iteration cursor for the + * new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all connectors in an atomic update, tracking both old and + * new state. This is useful in places where the state delta needs to be + * considered, for example in atomic check functions. + */ #define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->num_connector && \ @@ -515,6 +546,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (connector) +/** + * for_each_old_connector_in_state - iterate over all connectors in an atomic update + * @__state: &struct drm_atomic_state pointer + * @connector: &struct drm_connector iteration cursor + * @old_connector_state: &struct drm_connector_state iteration cursor for the + * old state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all connectors in an atomic update, tracking only the old + * state. This is useful in disable functions, where we need the old state the + * hardware is still in. + */ #define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->num_connector && \ @@ -523,6 +566,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (connector) +/** + * for_each_new_connector_in_state - iterate over all connectors in an atomic update + * @__state: &struct drm_atomic_state pointer + * @connector: &struct drm_connector iteration cursor + * @new_connector_state: &struct drm_connector_state iteration cursor for the + * new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all connectors in an atomic update, tracking only the new + * state. This is useful in enable functions, where we need the new state the + * hardware should be in when the atomic commit operation has completed. + */ #define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->num_connector && \ @@ -531,6 +586,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (connector) +/** + * for_each_crtc_in_state - iterate over all connectors in an atomic update + * @__state: &struct drm_atomic_state pointer + * @crtc: &struct drm_crtc iteration cursor + * @crtc_state: &struct drm_crtc_state iteration cursor + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all CRTCs in an atomic update. Note that before the + * software state is committed (by calling drm_atomic_helper_swap_state(), this + * points to the new state, while afterwards it points to the old state. Due to + * this tricky confusion this macro is deprecated. + * + * FIXME: + * + * Replace all usage of this with one of the explicit iterators below and then + * remove this macro. + */ #define for_each_crtc_in_state(__state, crtc, crtc_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_crtc && \ @@ -539,6 +611,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (crtc_state) +/** + * for_each_oldnew_crtc_in_state - iterate over all CRTCs in an atomic update + * @__state: &struct drm_atomic_state pointer + * @crtc: &struct drm_crtc iteration cursor + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all CRTCs in an atomic update, tracking both old and + * new state. This is useful in places where the state delta needs to be + * considered, for example in atomic check functions. + */ #define for_each_oldnew_crtc_in_state(__state, crtc, old_crtc_state, new_crtc_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_crtc && \ @@ -548,6 +632,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (crtc) +/** + * for_each_old_crtc_in_state - iterate over all CRTCs in an atomic update + * @__state: &struct drm_atomic_state pointer + * @crtc: &struct drm_crtc iteration cursor + * @old_crtc_state: &struct drm_crtc_state iteration cursor for the old state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all CRTCs in an atomic update, tracking only the old + * state. This is useful in disable functions, where we need the old state the + * hardware is still in. + */ #define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_crtc && \ @@ -556,6 +651,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (crtc) +/** + * for_each_new_crtc_in_state - iterate over all CRTCs in an atomic update + * @__state: &struct drm_atomic_state pointer + * @crtc: &struct drm_crtc iteration cursor + * @new_crtc_state: &struct drm_crtc_state iteration cursor for the new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all CRTCs in an atomic update, tracking only the new + * state. This is useful in enable functions, where we need the new state the + * hardware should be in when the atomic commit operation has completed. + */ #define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_crtc && \ @@ -564,6 +670,23 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (crtc) +/** + * for_each_plane_in_state - iterate over all planes in an atomic update + * @__state: &struct drm_atomic_state pointer + * @plane: &struct drm_plane iteration cursor + * @plane_state: &struct drm_plane_state iteration cursor + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all planes in an atomic update. Note that before the + * software state is committed (by calling drm_atomic_helper_swap_state(), this + * points to the new state, while afterwards it points to the old state. Due to + * this tricky confusion this macro is deprecated. + * + * FIXME: + * + * Replace all usage of this with one of the explicit iterators below and then + * remove this macro. + */ #define for_each_plane_in_state(__state, plane, plane_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_total_plane && \ @@ -572,6 +695,18 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (plane_state) +/** + * for_each_oldnew_plane_in_state - iterate over all planes in an atomic update + * @__state: &struct drm_atomic_state pointer + * @plane: &struct drm_plane iteration cursor + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all planes in an atomic update, tracking both old and + * new state. This is useful in places where the state delta needs to be + * considered, for example in atomic check functions. + */ #define for_each_oldnew_plane_in_state(__state, plane, old_plane_state, new_plane_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_total_plane && \ @@ -581,6 +716,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (plane) +/** + * for_each_old_plane_in_state - iterate over all planes in an atomic update + * @__state: &struct drm_atomic_state pointer + * @plane: &struct drm_plane iteration cursor + * @old_plane_state: &struct drm_plane_state iteration cursor for the old state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all planes in an atomic update, tracking only the old + * state. This is useful in disable functions, where we need the old state the + * hardware is still in. + */ #define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_total_plane && \ @@ -589,6 +735,17 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); (__i)++) \ for_each_if (plane) +/** + * for_each_new_plane_in_state - iterate over all planes in an atomic update + * @__state: &struct drm_atomic_state pointer + * @plane: &struct drm_plane iteration cursor + * @new_plane_state: &struct drm_plane_state iteration cursor for the new state + * @__i: int iteration cursor, for macro-internal use + * + * This iterates over all planes in an atomic update, tracking only the new + * state. This is useful in enable functions, where we need the new state the + * hardware should be in when the atomic commit operation has completed. + */ #define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \ for ((__i) = 0; \ (__i) < (__state)->dev->mode_config.num_total_plane && \ @@ -603,7 +760,7 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p); * * To give drivers flexibility &struct drm_crtc_state has 3 booleans to track * whether the state CRTC changed enough to need a full modeset cycle: - * connectors_changed, mode_changed and active_changed. This helper simply + * planes_changed, mode_changed and active_changed. This helper simply * combines these three to compute the overall need for a modeset for @state. * * The atomic helper code sets these booleans, but drivers can and should
Mostly because I want the links from the newly-added @state functions to work. But I think explaining when they're useful and that the implicit one is deprecated is good either way. Slightly repetitive unfortunately. Cc: Harry Wentland <harry.wentland@amd.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- include/drm/drm_atomic.h | 159 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 158 insertions(+), 1 deletion(-)