Op 27-09-17 om 14:53 schreef Dmitry Osipenko:
On 27.09.2017 11:35, Maarten Lankhorst wrote:
Commit 669c9215afea ("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.
Cc: Dmitry Osipenko digetx@gmail.com Fixes: 669c9215afea ("drm/atomic: Make async plane update checks work as intended, v2.") Signed-off-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
include/drm/drm_atomic.h | 85 ++++++++++++++++++++++++------------------------ 1 file changed, 42 insertions(+), 43 deletions(-)
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index 6fae95f28e10..5afd6e364fb6 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
This variant also works.
Tested-by: Dmitry Osipenko digetx@gmail.com
Forgot to hit push, pushed now. :)