Skip to content

Commit

Permalink
drm/atomic: Make atomic iterators less surprising
Browse files Browse the repository at this point in the history 
Commit 669c921 ("drm/atomic: Make async plane update checks work as
intended, v2.") assumed incorrectly that if only 1 plane is matched in
the loop, the variables will be set to that plane. In reality we reset
them to NULL every time a new plane was iterated. This behavior is
surprising, so fix this by making the for loops only assign the
variables on a match.

When we have not added all the planes/crtc/connector to the state, and
there's a few NULL ones after the last one we iterated, te assumption
is broken that the pointers will hold the values from the last loop
iteration, which holds true for all other for_each macros we're using.
Except of course the iterator pointer itself, but that one really is
entirely internal.

Cc: Dmitry Osipenko <digetx@gmail.com>
Fixes: 669c921 ("drm/atomic: Make async plane update checks work as intended, v2.")
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20170927083532.5756-2-maarten.lankhorst@linux.intel.com
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Tested-by: Dmitry Osipenko <digetx@gmail.com>
  • Loading branch information
@mlankhorst
mlankhorst committed Oct 6, 2017
1 parent f0d2e86 commit 331494e
Showing 1 changed file with 42 additions and 43 deletions.
85 changes: 42 additions & 43 deletions include/drm/drm_atomic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -585,12 +585,12 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#define for_each_oldnew_connector_in_state(__state, connector, old_connector_state, new_connector_state, __i) \
for ((__i) = 0; \
(__i) < (__state)->num_connector && \
((connector) = (__state)->connectors[__i].ptr, \
(old_connector_state) = (__state)->connectors[__i].old_state, \
(new_connector_state) = (__state)->connectors[__i].new_state, 1); \
(__i)++) \
for_each_if (connector)
(__i) < (__state)->num_connector; \
(__i)++) \
for_each_if ((__state)->connectors[__i].ptr && \
((connector) = (__state)->connectors[__i].ptr, \
(old_connector_state) = (__state)->connectors[__i].old_state, \
(new_connector_state) = (__state)->connectors[__i].new_state, 1))

/**
* for_each_old_connector_in_state - iterate over all connectors in an atomic update
Expand All @@ -606,11 +606,11 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#define for_each_old_connector_in_state(__state, connector, old_connector_state, __i) \
for ((__i) = 0; \
(__i) < (__state)->num_connector && \
((connector) = (__state)->connectors[__i].ptr, \
(old_connector_state) = (__state)->connectors[__i].old_state, 1); \
(__i)++) \
for_each_if (connector)
(__i) < (__state)->num_connector; \
(__i)++) \
for_each_if ((__state)->connectors[__i].ptr && \
((connector) = (__state)->connectors[__i].ptr, \
(old_connector_state) = (__state)->connectors[__i].old_state, 1))

/**
* for_each_new_connector_in_state - iterate over all connectors in an atomic update
Expand All @@ -626,11 +626,11 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#define for_each_new_connector_in_state(__state, connector, new_connector_state, __i) \
for ((__i) = 0; \
(__i) < (__state)->num_connector && \
((connector) = (__state)->connectors[__i].ptr, \
(new_connector_state) = (__state)->connectors[__i].new_state, 1); \
(__i)++) \
for_each_if (connector)
(__i) < (__state)->num_connector; \
(__i)++) \
for_each_if ((__state)->connectors[__i].ptr && \
((connector) = (__state)->connectors[__i].ptr, \
(new_connector_state) = (__state)->connectors[__i].new_state, 1))

/**
* for_each_oldnew_crtc_in_state - iterate over all CRTCs in an atomic update
Expand All @@ -646,12 +646,12 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#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 && \
((crtc) = (__state)->crtcs[__i].ptr, \
(old_crtc_state) = (__state)->crtcs[__i].old_state, \
(new_crtc_state) = (__state)->crtcs[__i].new_state, 1); \
(__i) < (__state)->dev->mode_config.num_crtc; \
(__i)++) \
for_each_if (crtc)
for_each_if ((__state)->crtcs[__i].ptr && \
((crtc) = (__state)->crtcs[__i].ptr, \
(old_crtc_state) = (__state)->crtcs[__i].old_state, \
(new_crtc_state) = (__state)->crtcs[__i].new_state, 1))

/**
* for_each_old_crtc_in_state - iterate over all CRTCs in an atomic update
Expand All @@ -666,11 +666,11 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#define for_each_old_crtc_in_state(__state, crtc, old_crtc_state, __i) \
for ((__i) = 0; \
(__i) < (__state)->dev->mode_config.num_crtc && \
((crtc) = (__state)->crtcs[__i].ptr, \
(old_crtc_state) = (__state)->crtcs[__i].old_state, 1); \
(__i) < (__state)->dev->mode_config.num_crtc; \
(__i)++) \
for_each_if (crtc)
for_each_if ((__state)->crtcs[__i].ptr && \
((crtc) = (__state)->crtcs[__i].ptr, \
(old_crtc_state) = (__state)->crtcs[__i].old_state, 1))

/**
* for_each_new_crtc_in_state - iterate over all CRTCs in an atomic update
Expand All @@ -685,11 +685,11 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#define for_each_new_crtc_in_state(__state, crtc, new_crtc_state, __i) \
for ((__i) = 0; \
(__i) < (__state)->dev->mode_config.num_crtc && \
((crtc) = (__state)->crtcs[__i].ptr, \
(new_crtc_state) = (__state)->crtcs[__i].new_state, 1); \
(__i) < (__state)->dev->mode_config.num_crtc; \
(__i)++) \
for_each_if (crtc)
for_each_if ((__state)->crtcs[__i].ptr && \
((crtc) = (__state)->crtcs[__i].ptr, \
(new_crtc_state) = (__state)->crtcs[__i].new_state, 1))

/**
* for_each_oldnew_plane_in_state - iterate over all planes in an atomic update
Expand All @@ -705,12 +705,12 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#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 && \
((plane) = (__state)->planes[__i].ptr, \
(old_plane_state) = (__state)->planes[__i].old_state, \
(new_plane_state) = (__state)->planes[__i].new_state, 1); \
(__i) < (__state)->dev->mode_config.num_total_plane; \
(__i)++) \
for_each_if (plane)
for_each_if ((__state)->planes[__i].ptr && \
((plane) = (__state)->planes[__i].ptr, \
(old_plane_state) = (__state)->planes[__i].old_state,\
(new_plane_state) = (__state)->planes[__i].new_state, 1))

/**
* for_each_old_plane_in_state - iterate over all planes in an atomic update
Expand All @@ -725,12 +725,11 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#define for_each_old_plane_in_state(__state, plane, old_plane_state, __i) \
for ((__i) = 0; \
(__i) < (__state)->dev->mode_config.num_total_plane && \
((plane) = (__state)->planes[__i].ptr, \
(old_plane_state) = (__state)->planes[__i].old_state, 1); \
(__i) < (__state)->dev->mode_config.num_total_plane; \
(__i)++) \
for_each_if (plane)

for_each_if ((__state)->planes[__i].ptr && \
((plane) = (__state)->planes[__i].ptr, \
(old_plane_state) = (__state)->planes[__i].old_state, 1))
/**
* for_each_new_plane_in_state - iterate over all planes in an atomic update
* @__state: &struct drm_atomic_state pointer
Expand All @@ -744,11 +743,11 @@ void drm_state_dump(struct drm_device *dev, struct drm_printer *p);
*/
#define for_each_new_plane_in_state(__state, plane, new_plane_state, __i) \
for ((__i) = 0; \
(__i) < (__state)->dev->mode_config.num_total_plane && \
((plane) = (__state)->planes[__i].ptr, \
(new_plane_state) = (__state)->planes[__i].new_state, 1); \
(__i) < (__state)->dev->mode_config.num_total_plane; \
(__i)++) \
for_each_if (plane)
for_each_if ((__state)->planes[__i].ptr && \
((plane) = (__state)->planes[__i].ptr, \
(new_plane_state) = (__state)->planes[__i].new_state, 1))

/**
* for_each_oldnew_private_obj_in_state - iterate over all private objects in an atomic update
Expand Down

0 comments on commit 331494e

Please sign in to comment.