On Sun, Mar 15, 2015 at 08:59:10PM +0800, John Hunter wrote:
> there are some duplication in the annotations
> add some empty line to make it easier to read
sob line missing. Also please split this up into individual parts, since I
don't agree with the additional empty lines bikeshed. See
Documentation/SubmittingPatches.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 48 +++++++++++++++++++++++++++++--------
> 1 file changed, 38 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index a745881..bc17019 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -346,7 +346,7 @@ needs_modeset(struct drm_crtc_state *state)
> }
>
> /**
> - * drm_atomic_helper_check - validate state object for modeset changes
> + * drm_atomic_helper_check_modeset - validate state object for modeset changes
> * @dev: DRM device
> * @state: the driver state object
> *
> @@ -461,7 +461,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> EXPORT_SYMBOL(drm_atomic_helper_check_modeset);
>
> /**
> - * drm_atomic_helper_check - validate state object for modeset changes
> + * drm_atomic_helper_check_planes - validate state object for planes changes
> * @dev: DRM device
> * @state: the driver state object
> *
> @@ -605,7 +605,7 @@ disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
>
> /*
> * Each encoder has at most one connector (since we always steal
> - * it away), so we won't call call disable hooks twice.
> + * it away), so we won't call disable hooks twice.
> */
> if (encoder->bridge)
> encoder->bridge->funcs->disable(encoder->bridge);
> @@ -757,7 +757,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
>
> /*
> * Each encoder has at most one connector (since we always steal
> - * it away), so we won't call call mode_set hooks twice.
> + * it away), so we won't call mode_set hooks twice.
> */
> if (funcs->mode_set)
> funcs->mode_set(encoder, mode, adjusted_mode);
> @@ -858,7 +858,7 @@ void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
>
> /*
> * Each encoder has at most one connector (since we always steal
> - * it away), so we won't call call enable hooks twice.
> + * it away), so we won't call enable hooks twice.
> */
> if (encoder->bridge)
> encoder->bridge->funcs->pre_enable(encoder->bridge);
> @@ -1025,7 +1025,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
>
> /*
> * Everything below can be run asynchronously without the need to grab
> - * any modeset locks at all under one conditions: It must be guaranteed
> + * any modeset locks at all under one condition: It must be guaranteed
> * that the asynchronous work has either been cancelled (if the driver
> * supports it, which at least requires that the framebuffers get
> * cleaned up with drm_atomic_helper_cleanup_planes()) or completed
> @@ -1151,7 +1151,6 @@ fail:
>
> if (fb && funcs->cleanup_fb)
> funcs->cleanup_fb(plane, fb, plane_state);
> -
> }
>
> return ret;
> @@ -1301,8 +1300,11 @@ void drm_atomic_helper_swap_state(struct drm_device *dev,
> struct drm_atomic_state *state)
> {
> int i;
> + int nconnectors = dev->mode_config.num_connector;
> + int ncrtcs = dev->mode_config.num_crtc;
> + int nplanes = dev->mode_config.num_total_plane;
>
> - for (i = 0; i < dev->mode_config.num_connector; i++) {
> + for (i = 0; i < nconnectors; i++) {
> struct drm_connector *connector = state->connectors[i];
>
> if (!connector)
> @@ -1313,7 +1315,7 @@ void drm_atomic_helper_swap_state(struct drm_device *dev,
> connector->state->state = NULL;
> }
>
> - for (i = 0; i < dev->mode_config.num_crtc; i++) {
> + for (i = 0; i < ncrtcs; i++) {
> struct drm_crtc *crtc = state->crtcs[i];
>
> if (!crtc)
> @@ -1324,7 +1326,7 @@ void drm_atomic_helper_swap_state(struct drm_device *dev,
> crtc->state->state = NULL;
> }
>
> - for (i = 0; i < dev->mode_config.num_total_plane; i++) {
> + for (i = 0; i < nplanes; i++) {
> struct drm_plane *plane = state->planes[i];
>
> if (!plane)
> @@ -1373,6 +1375,7 @@ int drm_atomic_helper_update_plane(struct drm_plane *plane,
> return -ENOMEM;
>
> state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> +
> retry:
> plane_state = drm_atomic_get_plane_state(state, plane);
> if (IS_ERR(plane_state)) {
> @@ -1402,6 +1405,7 @@ retry:
>
> /* Driver takes ownership of state on successful commit. */
> return 0;
> +
> fail:
> if (ret == -EDEADLK)
> goto backoff;
> @@ -1409,6 +1413,7 @@ fail:
> drm_atomic_state_free(state);
>
> return ret;
> +
> backoff:
> drm_atomic_state_clear(state);
> drm_atomic_legacy_backoff(state);
> @@ -1455,6 +1460,7 @@ int drm_atomic_helper_disable_plane(struct drm_plane *plane)
> return -ENOMEM;
>
> state->acquire_ctx = drm_modeset_legacy_acquire_ctx(plane->crtc);
> +
> retry:
> plane_state = drm_atomic_get_plane_state(state, plane);
> if (IS_ERR(plane_state)) {
> @@ -1465,6 +1471,7 @@ retry:
> ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
> if (ret != 0)
> goto fail;
> +
> drm_atomic_set_fb_for_plane(plane_state, NULL);
> plane_state->crtc_x = 0;
> plane_state->crtc_y = 0;
> @@ -1484,6 +1491,7 @@ retry:
>
> /* Driver takes ownership of state on successful commit. */
> return 0;
> +
> fail:
> if (ret == -EDEADLK)
> goto backoff;
> @@ -1491,6 +1499,7 @@ fail:
> drm_atomic_state_free(state);
>
> return ret;
> +
> backoff:
> drm_atomic_state_clear(state);
> drm_atomic_legacy_backoff(state);
> @@ -1609,6 +1618,7 @@ int drm_atomic_helper_set_config(struct drm_mode_set *set)
> return -ENOMEM;
>
> state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> +
> retry:
> crtc_state = drm_atomic_get_crtc_state(state, crtc);
> if (IS_ERR(crtc_state)) {
> @@ -1648,6 +1658,7 @@ retry:
> ret = drm_atomic_set_crtc_for_plane(primary_state, crtc);
> if (ret != 0)
> goto fail;
> +
> drm_atomic_set_fb_for_plane(primary_state, set->fb);
> primary_state->crtc_x = 0;
> primary_state->crtc_y = 0;
> @@ -1669,6 +1680,7 @@ commit:
>
> /* Driver takes ownership of state on successful commit. */
> return 0;
> +
> fail:
> if (ret == -EDEADLK)
> goto backoff;
> @@ -1676,6 +1688,7 @@ fail:
> drm_atomic_state_free(state);
>
> return ret;
> +
> backoff:
> drm_atomic_state_clear(state);
> drm_atomic_legacy_backoff(state);
> @@ -1718,6 +1731,7 @@ drm_atomic_helper_crtc_set_property(struct drm_crtc *crtc,
>
> /* ->set_property is always called with all locks held. */
> state->acquire_ctx = crtc->dev->mode_config.acquire_ctx;
> +
> retry:
> crtc_state = drm_atomic_get_crtc_state(state, crtc);
> if (IS_ERR(crtc_state)) {
> @@ -1736,6 +1750,7 @@ retry:
>
> /* Driver takes ownership of state on successful commit. */
> return 0;
> +
> fail:
> if (ret == -EDEADLK)
> goto backoff;
> @@ -1743,6 +1758,7 @@ fail:
> drm_atomic_state_free(state);
>
> return ret;
> +
> backoff:
> drm_atomic_state_clear(state);
> drm_atomic_legacy_backoff(state);
> @@ -1778,6 +1794,7 @@ drm_atomic_helper_plane_set_property(struct drm_plane *plane,
>
> /* ->set_property is always called with all locks held. */
> state->acquire_ctx = plane->dev->mode_config.acquire_ctx;
> +
> retry:
> plane_state = drm_atomic_get_plane_state(state, plane);
> if (IS_ERR(plane_state)) {
> @@ -1796,6 +1813,7 @@ retry:
>
> /* Driver takes ownership of state on successful commit. */
> return 0;
> +
> fail:
> if (ret == -EDEADLK)
> goto backoff;
> @@ -1803,6 +1821,7 @@ fail:
> drm_atomic_state_free(state);
>
> return ret;
> +
> backoff:
> drm_atomic_state_clear(state);
> drm_atomic_legacy_backoff(state);
> @@ -1838,6 +1857,7 @@ drm_atomic_helper_connector_set_property(struct drm_connector *connector,
>
> /* ->set_property is always called with all locks held. */
> state->acquire_ctx = connector->dev->mode_config.acquire_ctx;
> +
> retry:
> connector_state = drm_atomic_get_connector_state(state, connector);
> if (IS_ERR(connector_state)) {
> @@ -1856,6 +1876,7 @@ retry:
>
> /* Driver takes ownership of state on successful commit. */
> return 0;
> +
> fail:
> if (ret == -EDEADLK)
> goto backoff;
> @@ -1863,6 +1884,7 @@ fail:
> drm_atomic_state_free(state);
>
> return ret;
> +
> backoff:
> drm_atomic_state_clear(state);
> drm_atomic_legacy_backoff(state);
> @@ -1906,6 +1928,7 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
> return -ENOMEM;
>
> state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> +
> retry:
> crtc_state = drm_atomic_get_crtc_state(state, crtc);
> if (IS_ERR(crtc_state)) {
> @@ -1935,6 +1958,7 @@ retry:
>
> /* Driver takes ownership of state on successful async commit. */
> return 0;
> +
> fail:
> if (ret == -EDEADLK)
> goto backoff;
> @@ -1942,6 +1966,7 @@ fail:
> drm_atomic_state_free(state);
>
> return ret;
> +
> backoff:
> drm_atomic_state_clear(state);
> drm_atomic_legacy_backoff(state);
> @@ -1993,6 +2018,7 @@ void drm_atomic_helper_connector_dpms(struct drm_connector *connector,
> return;
>
> state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> +
> retry:
> crtc_state = drm_atomic_get_crtc_state(state, crtc);
> if (IS_ERR(crtc_state))
> @@ -2017,6 +2043,7 @@ retry:
>
> /* Driver takes ownership of state on successful async commit. */
> return;
> +
> fail:
> if (ret == -EDEADLK)
> goto backoff;
> @@ -2026,6 +2053,7 @@ fail:
> WARN(1, "Driver bug: Changing ->active failed with ret=%i\n", ret);
>
> return;
> +
> backoff:
> drm_atomic_state_clear(state);
> drm_atomic_legacy_backoff(state);
> --
> 1.9.1
>
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
