Re: [RFC v4 06/25] drm/atomic: Move __drm_atomic_helper_disable_plane/set_config()

On Sat, Apr 14, 2018 at 01:52:59PM +0200, 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.

Signed-off-by: Noralf Trønnes noralf@tronnes.org

drivers/gpu/drm/drm_atomic.c | 168 ++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_atomic_helper.c | 168 +----------------------------------- drivers/gpu/drm/drm_fb_helper.c | 8 +- include/drm/drm_atomic.h | 5 ++ include/drm/drm_atomic_helper.h | 4 - 5 files changed, 179 insertions(+), 174 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 7d25c42f22db..1fb602b6c8b1 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -2060,6 +2060,174 @@ void drm_atomic_clean_old_fb(struct drm_device *dev, } EXPORT_SYMBOL(drm_atomic_clean_old_fb);

+/* just used from drm_client and atomic-helper: */

In that case please put the prototype for these into the internal header drm_crtc_internal.h. Just to avoid drivers abusing this (they really shouldn't). -Daniel

+int drm_atomic_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_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_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_set_config);

/**

• DOC: explicit fencing properties

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index ee03c1ed2521..4f20eb58dd4f 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2673,7 +2673,7 @@ int drm_atomic_helper_disable_plane(struct drm_plane *plane, if (plane_state->crtc && plane_state->crtc->cursor == plane) plane_state->state->legacy_cursor_update = true;

• ret = __drm_atomic_helper_disable_plane(plane, plane_state);

• ret = drm_atomic_disable_plane(plane, plane_state); if (ret != 0) goto fail;

@@ -2684,95 +2684,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

@@ -2801,7 +2712,7 @@ int drm_atomic_helper_set_config(struct drm_mode_set *set, return -ENOMEM;

state->acquire_ctx = ctx;

• ret = __drm_atomic_helper_set_config(set, state);

• ret = drm_atomic_set_config(set, state); if (ret != 0) goto fail;

@@ -2817,81 +2728,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_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index d0936671a9a6..2eef24db21f8 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -358,7 +358,7 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ if (plane->type == DRM_PLANE_TYPE_PRIMARY) continue;

• ret = __drm_atomic_helper_disable_plane(plane, plane_state);
  

• ret = drm_atomic_disable_plane(plane, plane_state);
  

  if (ret != 0) goto out_state; }

@@ -374,13 +374,13 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ plane_state->rotation = rotation; }

• ret = __drm_atomic_helper_set_config(mode_set, state);
  

• ret = drm_atomic_set_config(mode_set, state);
  

  if (ret != 0) goto out_state;

  /*

•  * __drm_atomic_helper_set_config() sets active when a
  

•  * mode is set, unconditionally clear it if we force DPMS off
  

•  * drm_atomic_set_config() sets active when a mode is set,
  

•  * unconditionally clear it if we force DPMS off
  

  */ if (!active) { struct drm_crtc *crtc = mode_set->crtc;

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index a57a8aa90ffb..0b0827ff2169 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -604,6 +604,11 @@ drm_atomic_add_affected_planes(struct drm_atomic_state *state, void drm_atomic_clean_old_fb(struct drm_device *dev, unsigned plane_mask, int ret);

+int drm_atomic_disable_plane(struct drm_plane *plane,

• 	     struct drm_plane_state *plane_state);
  

+int drm_atomic_set_config(struct drm_mode_set *set,

• 	  struct drm_atomic_state *state);
  

int __must_check drm_atomic_check_only(struct drm_atomic_state *state); int __must_check drm_atomic_commit(struct drm_atomic_state *state); int __must_check drm_atomic_nonblocking_commit(struct drm_atomic_state *state); diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index 26aaba58d6ce..8d88a8b2ad52 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -114,12 +114,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.15.1

---

dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel