<div dir="ltr">I will read the Documentation/SubmittingPatches,<div>sorry about my mistake.</div></div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Mar 16, 2015 at 4:17 PM, Daniel Vetter <<a href="mailto:daniel@ffwll.ch" target="_blank">daniel@ffwll.ch</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 <div class="HOEnZb"><div class="h5"> > --- > 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 > <a href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a> </div></div>-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - <a href="http://blog.ffwll.ch" target="_blank">http://blog.ffwll.ch</a> <div class="HOEnZb"><div class="h5">_______________________________________________ dri-devel mailing list <a href="mailto:dri-devel@lists.freedesktop.org">dri-devel@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/dri-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/dri-devel</a> </div></div></blockquote></div> <div> </div>-- <div class="gmail_signature"><div dir="ltr"><div>Best regards </div><div>Junwang Zhao</div><div>Microprocessor Research and Develop Center</div><div>Department of Computer Science &Technology</div><div>Peking University</div><div>Beijing, 100871, PRC</div></div></div> </div>