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

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@tronnes.org

Triggers a bit my midlayer ocd, but I can't come up with a better idea either.

Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch

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=37406a60fac7de336dec331a8707a...

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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx