Re: [PATCH 03/16] drm/atomic: Move __drm_atomic_helper_disable_plane/set_config()

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

 




Den 26.03.2019 21.48, skrev Daniel Vetter:
> On Tue, Mar 26, 2019 at 06:55:33PM +0100, Noralf Trønnes wrote:
>> Prepare for moving drm_fb_helper modesetting code to drm_client.
>> drm_client will be linked to drm.ko, so move
>> __drm_atomic_helper_disable_plane() and __drm_atomic_helper_set_config()
>> out of drm_kms_helper.ko.
>>
>> While at it, fix two checkpatch complaints:
>> - WARNING: Block comments use a trailing */ on a separate line
>> - CHECK: Alignment should match open parenthesis
>>
>> Signed-off-by: Noralf Trønnes <noralf@xxxxxxxxxxx>
> 
> Triggers a bit my midlayer ocd, but I can't come up with a better idea
> either.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
> 

The Intel CI informed me that this patch didn't apply.

Turns out there's a patch in drm-next by Sean:

drm: Merge __drm_atomic_helper_disable_all() into
drm_atomic_helper_disable_all()

https://cgit.freedesktop.org/drm/drm/commit?id=37406a60fac7de336dec331a8707a94462ac5a5e

Should I move the whole function with the kerneldoc to drm_atomic.c, or
should I keep the doc and function in drm_atomic_helper.c and just
recreate __drm_atomic_helper_disable_all() and call that ?

Noralf.

>> ---
>>  drivers/gpu/drm/drm_atomic.c        | 168 ++++++++++++++++++++++++++++
>>  drivers/gpu/drm/drm_atomic_helper.c | 164 ---------------------------
>>  drivers/gpu/drm/drm_crtc_internal.h |   5 +
>>  include/drm/drm_atomic_helper.h     |   4 -
>>  4 files changed, 173 insertions(+), 168 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 5eb40130fafb..c3a9ffbf2310 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -1130,6 +1130,174 @@ int drm_atomic_nonblocking_commit(struct drm_atomic_state *state)
>>  }
>>  EXPORT_SYMBOL(drm_atomic_nonblocking_commit);
>>  
>> +/* just used from drm-client and atomic-helper: */
>> +int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
>> +				      struct drm_plane_state *plane_state)
>> +{
>> +	int ret;
>> +
>> +	ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
>> +	if (ret != 0)
>> +		return ret;
>> +
>> +	drm_atomic_set_fb_for_plane(plane_state, NULL);
>> +	plane_state->crtc_x = 0;
>> +	plane_state->crtc_y = 0;
>> +	plane_state->crtc_w = 0;
>> +	plane_state->crtc_h = 0;
>> +	plane_state->src_x = 0;
>> +	plane_state->src_y = 0;
>> +	plane_state->src_w = 0;
>> +	plane_state->src_h = 0;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(__drm_atomic_helper_disable_plane);
>> +
>> +static int update_output_state(struct drm_atomic_state *state,
>> +			       struct drm_mode_set *set)
>> +{
>> +	struct drm_device *dev = set->crtc->dev;
>> +	struct drm_crtc *crtc;
>> +	struct drm_crtc_state *new_crtc_state;
>> +	struct drm_connector *connector;
>> +	struct drm_connector_state *new_conn_state;
>> +	int ret, i;
>> +
>> +	ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
>> +			       state->acquire_ctx);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* First disable all connectors on the target crtc. */
>> +	ret = drm_atomic_add_affected_connectors(state, set->crtc);
>> +	if (ret)
>> +		return ret;
>> +
>> +	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
>> +		if (new_conn_state->crtc == set->crtc) {
>> +			ret = drm_atomic_set_crtc_for_connector(new_conn_state,
>> +								NULL);
>> +			if (ret)
>> +				return ret;
>> +
>> +			/* Make sure legacy setCrtc always re-trains */
>> +			new_conn_state->link_status = DRM_LINK_STATUS_GOOD;
>> +		}
>> +	}
>> +
>> +	/* Then set all connectors from set->connectors on the target crtc */
>> +	for (i = 0; i < set->num_connectors; i++) {
>> +		new_conn_state = drm_atomic_get_connector_state(state,
>> +								set->connectors[i]);
>> +		if (IS_ERR(new_conn_state))
>> +			return PTR_ERR(new_conn_state);
>> +
>> +		ret = drm_atomic_set_crtc_for_connector(new_conn_state,
>> +							set->crtc);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>> +		/*
>> +		 * Don't update ->enable for the CRTC in the set_config request,
>> +		 * since a mismatch would indicate a bug in the upper layers.
>> +		 * The actual modeset code later on will catch any
>> +		 * inconsistencies here.
>> +		 */
>> +		if (crtc == set->crtc)
>> +			continue;
>> +
>> +		if (!new_crtc_state->connector_mask) {
>> +			ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state,
>> +								NULL);
>> +			if (ret < 0)
>> +				return ret;
>> +
>> +			new_crtc_state->active = false;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/* just used from drm-client and atomic-helper: */
>> +int __drm_atomic_helper_set_config(struct drm_mode_set *set,
>> +				   struct drm_atomic_state *state)
>> +{
>> +	struct drm_crtc_state *crtc_state;
>> +	struct drm_plane_state *primary_state;
>> +	struct drm_crtc *crtc = set->crtc;
>> +	int hdisplay, vdisplay;
>> +	int ret;
>> +
>> +	crtc_state = drm_atomic_get_crtc_state(state, crtc);
>> +	if (IS_ERR(crtc_state))
>> +		return PTR_ERR(crtc_state);
>> +
>> +	primary_state = drm_atomic_get_plane_state(state, crtc->primary);
>> +	if (IS_ERR(primary_state))
>> +		return PTR_ERR(primary_state);
>> +
>> +	if (!set->mode) {
>> +		WARN_ON(set->fb);
>> +		WARN_ON(set->num_connectors);
>> +
>> +		ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL);
>> +		if (ret != 0)
>> +			return ret;
>> +
>> +		crtc_state->active = false;
>> +
>> +		ret = drm_atomic_set_crtc_for_plane(primary_state, NULL);
>> +		if (ret != 0)
>> +			return ret;
>> +
>> +		drm_atomic_set_fb_for_plane(primary_state, NULL);
>> +
>> +		goto commit;
>> +	}
>> +
>> +	WARN_ON(!set->fb);
>> +	WARN_ON(!set->num_connectors);
>> +
>> +	ret = drm_atomic_set_mode_for_crtc(crtc_state, set->mode);
>> +	if (ret != 0)
>> +		return ret;
>> +
>> +	crtc_state->active = true;
>> +
>> +	ret = drm_atomic_set_crtc_for_plane(primary_state, crtc);
>> +	if (ret != 0)
>> +		return ret;
>> +
>> +	drm_mode_get_hv_timing(set->mode, &hdisplay, &vdisplay);
>> +
>> +	drm_atomic_set_fb_for_plane(primary_state, set->fb);
>> +	primary_state->crtc_x = 0;
>> +	primary_state->crtc_y = 0;
>> +	primary_state->crtc_w = hdisplay;
>> +	primary_state->crtc_h = vdisplay;
>> +	primary_state->src_x = set->x << 16;
>> +	primary_state->src_y = set->y << 16;
>> +	if (drm_rotation_90_or_270(primary_state->rotation)) {
>> +		primary_state->src_w = vdisplay << 16;
>> +		primary_state->src_h = hdisplay << 16;
>> +	} else {
>> +		primary_state->src_w = hdisplay << 16;
>> +		primary_state->src_h = vdisplay << 16;
>> +	}
>> +
>> +commit:
>> +	ret = update_output_state(state, set);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(__drm_atomic_helper_set_config);
>> +
>>  void drm_atomic_print_state(const struct drm_atomic_state *state)
>>  {
>>  	struct drm_printer p = drm_info_printer(state->dev->dev);
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 2453678d1186..b0d960da53cb 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -2831,95 +2831,6 @@ int drm_atomic_helper_disable_plane(struct drm_plane *plane,
>>  }
>>  EXPORT_SYMBOL(drm_atomic_helper_disable_plane);
>>  
>> -/* just used from fb-helper and atomic-helper: */
>> -int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
>> -		struct drm_plane_state *plane_state)
>> -{
>> -	int ret;
>> -
>> -	ret = drm_atomic_set_crtc_for_plane(plane_state, NULL);
>> -	if (ret != 0)
>> -		return ret;
>> -
>> -	drm_atomic_set_fb_for_plane(plane_state, NULL);
>> -	plane_state->crtc_x = 0;
>> -	plane_state->crtc_y = 0;
>> -	plane_state->crtc_w = 0;
>> -	plane_state->crtc_h = 0;
>> -	plane_state->src_x = 0;
>> -	plane_state->src_y = 0;
>> -	plane_state->src_w = 0;
>> -	plane_state->src_h = 0;
>> -
>> -	return 0;
>> -}
>> -
>> -static int update_output_state(struct drm_atomic_state *state,
>> -			       struct drm_mode_set *set)
>> -{
>> -	struct drm_device *dev = set->crtc->dev;
>> -	struct drm_crtc *crtc;
>> -	struct drm_crtc_state *new_crtc_state;
>> -	struct drm_connector *connector;
>> -	struct drm_connector_state *new_conn_state;
>> -	int ret, i;
>> -
>> -	ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
>> -			       state->acquire_ctx);
>> -	if (ret)
>> -		return ret;
>> -
>> -	/* First disable all connectors on the target crtc. */
>> -	ret = drm_atomic_add_affected_connectors(state, set->crtc);
>> -	if (ret)
>> -		return ret;
>> -
>> -	for_each_new_connector_in_state(state, connector, new_conn_state, i) {
>> -		if (new_conn_state->crtc == set->crtc) {
>> -			ret = drm_atomic_set_crtc_for_connector(new_conn_state,
>> -								NULL);
>> -			if (ret)
>> -				return ret;
>> -
>> -			/* Make sure legacy setCrtc always re-trains */
>> -			new_conn_state->link_status = DRM_LINK_STATUS_GOOD;
>> -		}
>> -	}
>> -
>> -	/* Then set all connectors from set->connectors on the target crtc */
>> -	for (i = 0; i < set->num_connectors; i++) {
>> -		new_conn_state = drm_atomic_get_connector_state(state,
>> -							    set->connectors[i]);
>> -		if (IS_ERR(new_conn_state))
>> -			return PTR_ERR(new_conn_state);
>> -
>> -		ret = drm_atomic_set_crtc_for_connector(new_conn_state,
>> -							set->crtc);
>> -		if (ret)
>> -			return ret;
>> -	}
>> -
>> -	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>> -		/* Don't update ->enable for the CRTC in the set_config request,
>> -		 * since a mismatch would indicate a bug in the upper layers.
>> -		 * The actual modeset code later on will catch any
>> -		 * inconsistencies here. */
>> -		if (crtc == set->crtc)
>> -			continue;
>> -
>> -		if (!new_crtc_state->connector_mask) {
>> -			ret = drm_atomic_set_mode_prop_for_crtc(new_crtc_state,
>> -								NULL);
>> -			if (ret < 0)
>> -				return ret;
>> -
>> -			new_crtc_state->active = false;
>> -		}
>> -	}
>> -
>> -	return 0;
>> -}
>> -
>>  /**
>>   * drm_atomic_helper_set_config - set a new config from userspace
>>   * @set: mode set configuration
>> @@ -2964,81 +2875,6 @@ int drm_atomic_helper_set_config(struct drm_mode_set *set,
>>  }
>>  EXPORT_SYMBOL(drm_atomic_helper_set_config);
>>  
>> -/* just used from fb-helper and atomic-helper: */
>> -int __drm_atomic_helper_set_config(struct drm_mode_set *set,
>> -		struct drm_atomic_state *state)
>> -{
>> -	struct drm_crtc_state *crtc_state;
>> -	struct drm_plane_state *primary_state;
>> -	struct drm_crtc *crtc = set->crtc;
>> -	int hdisplay, vdisplay;
>> -	int ret;
>> -
>> -	crtc_state = drm_atomic_get_crtc_state(state, crtc);
>> -	if (IS_ERR(crtc_state))
>> -		return PTR_ERR(crtc_state);
>> -
>> -	primary_state = drm_atomic_get_plane_state(state, crtc->primary);
>> -	if (IS_ERR(primary_state))
>> -		return PTR_ERR(primary_state);
>> -
>> -	if (!set->mode) {
>> -		WARN_ON(set->fb);
>> -		WARN_ON(set->num_connectors);
>> -
>> -		ret = drm_atomic_set_mode_for_crtc(crtc_state, NULL);
>> -		if (ret != 0)
>> -			return ret;
>> -
>> -		crtc_state->active = false;
>> -
>> -		ret = drm_atomic_set_crtc_for_plane(primary_state, NULL);
>> -		if (ret != 0)
>> -			return ret;
>> -
>> -		drm_atomic_set_fb_for_plane(primary_state, NULL);
>> -
>> -		goto commit;
>> -	}
>> -
>> -	WARN_ON(!set->fb);
>> -	WARN_ON(!set->num_connectors);
>> -
>> -	ret = drm_atomic_set_mode_for_crtc(crtc_state, set->mode);
>> -	if (ret != 0)
>> -		return ret;
>> -
>> -	crtc_state->active = true;
>> -
>> -	ret = drm_atomic_set_crtc_for_plane(primary_state, crtc);
>> -	if (ret != 0)
>> -		return ret;
>> -
>> -	drm_mode_get_hv_timing(set->mode, &hdisplay, &vdisplay);
>> -
>> -	drm_atomic_set_fb_for_plane(primary_state, set->fb);
>> -	primary_state->crtc_x = 0;
>> -	primary_state->crtc_y = 0;
>> -	primary_state->crtc_w = hdisplay;
>> -	primary_state->crtc_h = vdisplay;
>> -	primary_state->src_x = set->x << 16;
>> -	primary_state->src_y = set->y << 16;
>> -	if (drm_rotation_90_or_270(primary_state->rotation)) {
>> -		primary_state->src_w = vdisplay << 16;
>> -		primary_state->src_h = hdisplay << 16;
>> -	} else {
>> -		primary_state->src_w = hdisplay << 16;
>> -		primary_state->src_h = vdisplay << 16;
>> -	}
>> -
>> -commit:
>> -	ret = update_output_state(state, set);
>> -	if (ret)
>> -		return ret;
>> -
>> -	return 0;
>> -}
>> -
>>  static int __drm_atomic_helper_disable_all(struct drm_device *dev,
>>  					   struct drm_modeset_acquire_ctx *ctx,
>>  					   bool clean_old_fbs)
>> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
>> index 216f2a9ee3d4..22a63a544ec7 100644
>> --- a/drivers/gpu/drm/drm_crtc_internal.h
>> +++ b/drivers/gpu/drm/drm_crtc_internal.h
>> @@ -207,6 +207,11 @@ struct drm_minor;
>>  int drm_atomic_debugfs_init(struct drm_minor *minor);
>>  #endif
>>  
>> +int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
>> +				      struct drm_plane_state *plane_state);
>> +int __drm_atomic_helper_set_config(struct drm_mode_set *set,
>> +				   struct drm_atomic_state *state);
>> +
>>  void drm_atomic_print_state(const struct drm_atomic_state *state);
>>  
>>  /* drm_atomic_uapi.c */
>> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
>> index 58214be3bf3d..bf4e07141d81 100644
>> --- a/include/drm/drm_atomic_helper.h
>> +++ b/include/drm/drm_atomic_helper.h
>> @@ -117,12 +117,8 @@ int drm_atomic_helper_update_plane(struct drm_plane *plane,
>>  				   struct drm_modeset_acquire_ctx *ctx);
>>  int drm_atomic_helper_disable_plane(struct drm_plane *plane,
>>  				    struct drm_modeset_acquire_ctx *ctx);
>> -int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
>> -		struct drm_plane_state *plane_state);
>>  int drm_atomic_helper_set_config(struct drm_mode_set *set,
>>  				 struct drm_modeset_acquire_ctx *ctx);
>> -int __drm_atomic_helper_set_config(struct drm_mode_set *set,
>> -		struct drm_atomic_state *state);
>>  
>>  int drm_atomic_helper_disable_all(struct drm_device *dev,
>>  				  struct drm_modeset_acquire_ctx *ctx);
>> -- 
>> 2.20.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux