Re: [PATCH 1/2] drm_atomic_helper.c: cosmetic: fix some typos and add some empty line

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux