diff mbox series

[v2,08/12] drm/fb-helper: Move out commit code

Message ID 20190407165243.54043-9-noralf@tronnes.org (mailing list archive)
State New, archived
Headers show
Series drm/fb-helper: Move modesetting code to drm_client | expand

Commit Message

Noralf Trønnes April 7, 2019, 4:52 p.m. UTC
Move the modeset commit code to drm_client_modeset.
No changes except exporting API.

v2: Move to drm_client_modeset.c instead of drm_client.c

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_client_modeset.c | 287 +++++++++++++++++++++++++++
 drivers/gpu/drm/drm_fb_helper.c      | 282 --------------------------
 include/drm/drm_client.h             |   4 +
 3 files changed, 291 insertions(+), 282 deletions(-)

Comments

Daniel Vetter April 16, 2019, 8:38 a.m. UTC | #1
On Sun, Apr 07, 2019 at 06:52:39PM +0200, Noralf Trønnes wrote:
> Move the modeset commit code to drm_client_modeset.
> No changes except exporting API.
> 
> v2: Move to drm_client_modeset.c instead of drm_client.c
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/drm_client_modeset.c | 287 +++++++++++++++++++++++++++

Please add a new chapter in Documentation/gpu to pull in all the nice new
kerneldoc you've typed.

>  drivers/gpu/drm/drm_fb_helper.c      | 282 --------------------------
>  include/drm/drm_client.h             |   4 +
>  3 files changed, 291 insertions(+), 282 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
> index bb34222c9db8..94b52f97c06b 100644
> --- a/drivers/gpu/drm/drm_client_modeset.c
> +++ b/drivers/gpu/drm/drm_client_modeset.c
> @@ -11,9 +11,14 @@
>  #include <linux/mutex.h>
>  #include <linux/slab.h>
>  
> +#include <drm/drm_atomic.h>
>  #include <drm/drm_client.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_device.h>
> +#include <drm/drm_drv.h>
> +
> +#include "drm_crtc_internal.h"
> +#include "drm_internal.h"
>  
>  /* Used by drm_client and drm_fb_helper */
>  int drm_client_modeset_create(struct drm_client_dev *client)
> @@ -100,3 +105,285 @@ struct drm_mode_set *drm_client_find_modeset(struct drm_client_dev *client, stru
>  }
>  /* TODO: Remove export when modeset code has been moved over */
>  EXPORT_SYMBOL(drm_client_find_modeset);
> +
> +/**
> + * drm_client_panel_rotation() - Check panel orientation
> + * @modeset: DRM modeset
> + * @rotation: Returned rotation value
> + *
> + * This function checks if the primary plane in @modeset can hw rotate to match
> + * the panel orientation on its connector.
> + *
> + * Note: Currently only 0 and 180 degrees are supported.
> + *
> + * Return:
> + * True if the plane can do the rotation, false otherwise.
> + */
> +bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation)
> +{
> +	struct drm_connector *connector = modeset->connectors[0];
> +	struct drm_plane *plane = modeset->crtc->primary;
> +	u64 valid_mask = 0;
> +	unsigned int i;
> +
> +	if (!modeset->num_connectors)
> +		return false;
> +
> +	switch (connector->display_info.panel_orientation) {
> +	case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
> +		*rotation = DRM_MODE_ROTATE_180;
> +		break;
> +	case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
> +		*rotation = DRM_MODE_ROTATE_90;
> +		break;
> +	case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
> +		*rotation = DRM_MODE_ROTATE_270;
> +		break;
> +	default:
> +		*rotation = DRM_MODE_ROTATE_0;
> +	}
> +
> +	/*
> +	 * TODO: support 90 / 270 degree hardware rotation,
> +	 * depending on the hardware this may require the framebuffer
> +	 * to be in a specific tiling format.
> +	 */
> +	if (*rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property)
> +		return false;
> +
> +	for (i = 0; i < plane->rotation_property->num_values; i++)
> +		valid_mask |= (1ULL << plane->rotation_property->values[i]);
> +
> +	if (!(*rotation & valid_mask))
> +		return false;
> +
> +	return true;
> +}
> +
> +static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, bool active)
> +{
> +	struct drm_device *dev = client->dev;
> +	struct drm_plane_state *plane_state;
> +	struct drm_plane *plane;
> +	struct drm_atomic_state *state;
> +	struct drm_modeset_acquire_ctx ctx;
> +	struct drm_mode_set *mode_set;
> +	int ret;
> +
> +	drm_modeset_acquire_init(&ctx, 0);
> +
> +	state = drm_atomic_state_alloc(dev);
> +	if (!state) {
> +		ret = -ENOMEM;
> +		goto out_ctx;
> +	}
> +
> +	state->acquire_ctx = &ctx;
> +retry:
> +	drm_for_each_plane(plane, dev) {
> +		plane_state = drm_atomic_get_plane_state(state, plane);
> +		if (IS_ERR(plane_state)) {
> +			ret = PTR_ERR(plane_state);
> +			goto out_state;
> +		}
> +
> +		plane_state->rotation = DRM_MODE_ROTATE_0;
> +
> +		/* disable non-primary: */
> +		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> +			continue;
> +
> +		ret = __drm_atomic_helper_disable_plane(plane, plane_state);
> +		if (ret != 0)
> +			goto out_state;
> +	}
> +
> +	drm_client_for_each_modeset(mode_set, client) {
> +		struct drm_plane *primary = mode_set->crtc->primary;
> +		unsigned int rotation;
> +
> +		if (drm_client_panel_rotation(mode_set, &rotation)) {
> +			/* Cannot fail as we've already gotten the plane state above */
> +			plane_state = drm_atomic_get_new_plane_state(state, primary);
> +			plane_state->rotation = rotation;
> +		}
> +
> +		ret = __drm_atomic_helper_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
> +		 */
> +		if (!active) {
> +			struct drm_crtc *crtc = mode_set->crtc;
> +			struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> +
> +			crtc_state->active = false;
> +		}
> +	}
> +
> +	ret = drm_atomic_commit(state);
> +
> +out_state:
> +	if (ret == -EDEADLK)
> +		goto backoff;
> +
> +	drm_atomic_state_put(state);
> +out_ctx:
> +	drm_modeset_drop_locks(&ctx);
> +	drm_modeset_acquire_fini(&ctx);
> +
> +	return ret;
> +
> +backoff:
> +	drm_atomic_state_clear(state);
> +	drm_modeset_backoff(&ctx);
> +
> +	goto retry;
> +}
> +
> +static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
> +{
> +	struct drm_device *dev = client->dev;
> +	struct drm_mode_set *mode_set;
> +	struct drm_plane *plane;
> +	int ret = 0;
> +
> +	drm_modeset_lock_all(dev);
> +	drm_for_each_plane(plane, dev) {
> +		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> +			drm_plane_force_disable(plane);
> +
> +		if (plane->rotation_property)
> +			drm_mode_plane_set_obj_prop(plane,
> +						    plane->rotation_property,
> +						    DRM_MODE_ROTATE_0);
> +	}
> +
> +	drm_client_for_each_modeset(mode_set, client) {
> +		struct drm_crtc *crtc = mode_set->crtc;
> +
> +		if (crtc->funcs->cursor_set2) {
> +			ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0);
> +			if (ret)
> +				goto out;
> +		} else if (crtc->funcs->cursor_set) {
> +			ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0);
> +			if (ret)
> +				goto out;
> +		}
> +
> +		ret = drm_mode_set_config_internal(mode_set);
> +		if (ret)
> +			goto out;
> +	}
> +out:
> +	drm_modeset_unlock_all(dev);
> +
> +	return ret;
> +}
> +
> +/**
> + * drm_client_modeset_commit_force() - Force commit CRTC configuration
> + * @client: DRM client
> + *
> + * Commit modeset configuration to crtcs without checking if there is a DRM master.
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_client_modeset_commit_force(struct drm_client_dev *client)
> +{
> +	struct drm_device *dev = client->dev;
> +	int ret;
> +
> +	mutex_lock(&client->modeset_mutex);
> +	if (drm_drv_uses_atomic_modeset(dev))
> +		ret = drm_client_modeset_commit_atomic(client, true);
> +	else
> +		ret = drm_client_modeset_commit_legacy(client);
> +	mutex_unlock(&client->modeset_mutex);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_client_modeset_commit_force);
> +
> +/**
> + * drm_client_modeset_commit() - Commit CRTC configuration
> + * @client: DRM client
> + *
> + * Commit modeset configuration to crtcs.
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_client_modeset_commit(struct drm_client_dev *client)
> +{
> +	struct drm_device *dev = client->dev;
> +	int ret;
> +
> +	if (!drm_master_internal_acquire(dev))
> +		return -EBUSY;
> +
> +	ret = drm_client_modeset_commit_force(client);
> +
> +	drm_master_internal_release(dev);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_client_modeset_commit);
> +
> +static void drm_client_modeset_dpms_legacy(struct drm_client_dev *client, int dpms_mode)
> +{
> +	struct drm_device *dev = client->dev;
> +	struct drm_connector *connector;
> +	struct drm_mode_set *modeset;
> +	int j;
> +
> +	drm_modeset_lock_all(dev);
> +	drm_client_for_each_modeset(modeset, client) {
> +		if (!modeset->crtc->enabled)
> +			continue;
> +
> +		for (j = 0; j < modeset->num_connectors; j++) {
> +			connector = modeset->connectors[j];
> +			connector->funcs->dpms(connector, dpms_mode);
> +			drm_object_property_set_value(&connector->base,
> +				dev->mode_config.dpms_property, dpms_mode);
> +		}
> +	}
> +	drm_modeset_unlock_all(dev);
> +}
> +
> +/**
> + * drm_client_modeset_dpms() - Set DPMS mode
> + * @client: DRM client
> + * @mode: DPMS mode
> + *
> + * Note: For atomic drivers @mode is reduced to on/off.
> + *
> + * Returns:
> + * Zero on success or negative error code on failure.
> + */
> +int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
> +{
> +	struct drm_device *dev = client->dev;
> +	int ret = 0;
> +
> +	if (!drm_master_internal_acquire(dev))
> +		return -EBUSY;
> +
> +	mutex_lock(&client->modeset_mutex);
> +	if (drm_drv_uses_atomic_modeset(dev))
> +		ret = drm_client_modeset_commit_atomic(client, mode == DRM_MODE_DPMS_ON);
> +	else
> +		drm_client_modeset_dpms_legacy(client, mode);
> +	mutex_unlock(&client->modeset_mutex);
> +
> +	drm_master_internal_release(dev);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_client_modeset_dpms);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0229e187e6de..73fbcd748ddd 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -40,10 +40,7 @@
>  #include <drm/drm_fb_helper.h>
>  #include <drm/drm_crtc_helper.h>
>  #include <drm/drm_atomic.h>
> -#include <drm/drm_atomic_helper.h>
>  
> -#include "drm_crtc_internal.h"
> -#include "drm_crtc_helper_internal.h"
>  #include "drm_internal.h"
>  
>  static bool drm_fbdev_emulation = true;
> @@ -388,233 +385,6 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
>  }
>  EXPORT_SYMBOL(drm_fb_helper_debug_leave);
>  
> -/**
> - * drm_client_panel_rotation() - Check panel orientation
> - * @modeset: DRM modeset
> - * @rotation: Returned rotation value
> - *
> - * This function checks if the primary plane in @modeset can hw rotate to match
> - * the panel orientation on its connector.
> - *
> - * Note: Currently only 0 and 180 degrees are supported.
> - *
> - * Return:
> - * True if the plane can do the rotation, false otherwise.
> - */
> -bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation)

Why not static? Doesn't seem to be used by anything outside of
drm_client_modeset.c.

> -{
> -	struct drm_connector *connector = modeset->connectors[0];
> -	struct drm_plane *plane = modeset->crtc->primary;
> -	u64 valid_mask = 0;
> -	unsigned int i;
> -
> -	if (!modeset->num_connectors)
> -		return false;
> -
> -	switch (connector->display_info.panel_orientation) {
> -	case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
> -		*rotation = DRM_MODE_ROTATE_180;
> -		break;
> -	case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
> -		*rotation = DRM_MODE_ROTATE_90;
> -		break;
> -	case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
> -		*rotation = DRM_MODE_ROTATE_270;
> -		break;
> -	default:
> -		*rotation = DRM_MODE_ROTATE_0;
> -	}
> -
> -	/*
> -	 * TODO: support 90 / 270 degree hardware rotation,
> -	 * depending on the hardware this may require the framebuffer
> -	 * to be in a specific tiling format.
> -	 */
> -	if (*rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property)
> -		return false;
> -
> -	for (i = 0; i < plane->rotation_property->num_values; i++)
> -		valid_mask |= (1ULL << plane->rotation_property->values[i]);
> -
> -	if (!(*rotation & valid_mask))
> -		return false;
> -
> -	return true;
> -}
> -
> -static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, bool active)
> -{
> -	struct drm_device *dev = client->dev;
> -	struct drm_plane_state *plane_state;
> -	struct drm_plane *plane;
> -	struct drm_atomic_state *state;
> -	struct drm_modeset_acquire_ctx ctx;
> -	struct drm_mode_set *mode_set;
> -	int ret;
> -
> -	drm_modeset_acquire_init(&ctx, 0);
> -
> -	state = drm_atomic_state_alloc(dev);
> -	if (!state) {
> -		ret = -ENOMEM;
> -		goto out_ctx;
> -	}
> -
> -	state->acquire_ctx = &ctx;
> -retry:
> -	drm_for_each_plane(plane, dev) {
> -		plane_state = drm_atomic_get_plane_state(state, plane);
> -		if (IS_ERR(plane_state)) {
> -			ret = PTR_ERR(plane_state);
> -			goto out_state;
> -		}
> -
> -		plane_state->rotation = DRM_MODE_ROTATE_0;
> -
> -		/* disable non-primary: */
> -		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
> -			continue;
> -
> -		ret = __drm_atomic_helper_disable_plane(plane, plane_state);
> -		if (ret != 0)
> -			goto out_state;
> -	}
> -
> -	drm_client_for_each_modeset(mode_set, client) {
> -		struct drm_plane *primary = mode_set->crtc->primary;
> -		unsigned int rotation;
> -
> -		if (drm_client_panel_rotation(mode_set, &rotation)) {
> -			/* Cannot fail as we've already gotten the plane state above */
> -			plane_state = drm_atomic_get_new_plane_state(state, primary);
> -			plane_state->rotation = rotation;
> -		}
> -
> -		ret = __drm_atomic_helper_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
> -		 */
> -		if (!active) {
> -			struct drm_crtc *crtc = mode_set->crtc;
> -			struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> -
> -			crtc_state->active = false;
> -		}
> -	}
> -
> -	ret = drm_atomic_commit(state);
> -
> -out_state:
> -	if (ret == -EDEADLK)
> -		goto backoff;
> -
> -	drm_atomic_state_put(state);
> -out_ctx:
> -	drm_modeset_drop_locks(&ctx);
> -	drm_modeset_acquire_fini(&ctx);
> -
> -	return ret;
> -
> -backoff:
> -	drm_atomic_state_clear(state);
> -	drm_modeset_backoff(&ctx);
> -
> -	goto retry;
> -}
> -
> -static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
> -{
> -	struct drm_device *dev = client->dev;
> -	struct drm_mode_set *mode_set;
> -	struct drm_plane *plane;
> -	int ret = 0;
> -
> -	drm_modeset_lock_all(dev);
> -	drm_for_each_plane(plane, dev) {
> -		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
> -			drm_plane_force_disable(plane);
> -
> -		if (plane->rotation_property)
> -			drm_mode_plane_set_obj_prop(plane,
> -						    plane->rotation_property,
> -						    DRM_MODE_ROTATE_0);
> -	}
> -
> -	drm_client_for_each_modeset(mode_set, client) {
> -		struct drm_crtc *crtc = mode_set->crtc;
> -
> -		if (crtc->funcs->cursor_set2) {
> -			ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0);
> -			if (ret)
> -				goto out;
> -		} else if (crtc->funcs->cursor_set) {
> -			ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0);
> -			if (ret)
> -				goto out;
> -		}
> -
> -		ret = drm_mode_set_config_internal(mode_set);
> -		if (ret)
> -			goto out;
> -	}
> -out:
> -	drm_modeset_unlock_all(dev);
> -
> -	return ret;
> -}
> -
> -/**
> - * drm_client_modeset_commit_force() - Force commit CRTC configuration
> - * @client: DRM client
> - *
> - * Commit modeset configuration to crtcs without checking if there is a DRM master.
> - *
> - * Returns:
> - * Zero on success or negative error code on failure.
> - */
> -int drm_client_modeset_commit_force(struct drm_client_dev *client)
> -{
> -	struct drm_device *dev = client->dev;
> -	int ret;
> -
> -	mutex_lock(&client->modeset_mutex);
> -	if (drm_drv_uses_atomic_modeset(dev))
> -		ret = drm_client_modeset_commit_atomic(client, true);
> -	else
> -		ret = drm_client_modeset_commit_legacy(client);
> -	mutex_unlock(&client->modeset_mutex);
> -
> -	return ret;
> -}
> -
> -/**
> - * drm_client_modeset_commit() - Commit CRTC configuration
> - * @client: DRM client
> - *
> - * Commit modeset configuration to crtcs.
> - *
> - * Returns:
> - * Zero on success or negative error code on failure.
> - */
> -int drm_client_modeset_commit(struct drm_client_dev *client)
> -{
> -	struct drm_device *dev = client->dev;
> -	int ret;
> -
> -	if (!drm_master_internal_acquire(dev))
> -		return -EBUSY;
> -
> -	ret = drm_client_modeset_commit_force(client);
> -
> -	drm_master_internal_release(dev);
> -
> -	return ret;
> -}
> -
>  /**
>   * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
>   * @fb_helper: driver-allocated fbdev helper, can be NULL
> @@ -704,58 +474,6 @@ static struct sysrq_key_op sysrq_drm_fb_helper_restore_op = {
>  static struct sysrq_key_op sysrq_drm_fb_helper_restore_op = { };
>  #endif
>  
> -static void drm_client_modeset_dpms_legacy(struct drm_client_dev *client, int dpms_mode)
> -{
> -	struct drm_device *dev = client->dev;
> -	struct drm_connector *connector;
> -	struct drm_mode_set *modeset;
> -	int j;
> -
> -	drm_modeset_lock_all(dev);
> -	drm_client_for_each_modeset(modeset, client) {
> -		if (!modeset->crtc->enabled)
> -			continue;
> -
> -		for (j = 0; j < modeset->num_connectors; j++) {
> -			connector = modeset->connectors[j];
> -			connector->funcs->dpms(connector, dpms_mode);
> -			drm_object_property_set_value(&connector->base,
> -				dev->mode_config.dpms_property, dpms_mode);
> -		}
> -	}
> -	drm_modeset_unlock_all(dev);
> -}
> -
> -/**
> - * drm_client_modeset_dpms() - Set DPMS mode
> - * @client: DRM client
> - * @mode: DPMS mode
> - *
> - * Note: For atomic drivers @mode is reduced to on/off.
> - *
> - * Returns:
> - * Zero on success or negative error code on failure.
> - */
> -int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
> -{
> -	struct drm_device *dev = client->dev;
> -	int ret = 0;
> -
> -	if (!drm_master_internal_acquire(dev))
> -		return -EBUSY;
> -
> -	mutex_lock(&client->modeset_mutex);
> -	if (drm_drv_uses_atomic_modeset(dev))
> -		ret = drm_client_modeset_commit_atomic(client, mode == DRM_MODE_DPMS_ON);
> -	else
> -		drm_client_modeset_dpms_legacy(client, mode);
> -	mutex_unlock(&client->modeset_mutex);
> -
> -	drm_master_internal_release(dev);
> -
> -	return ret;
> -}
> -
>  static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 858c8be70870..64b725b318f2 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -154,6 +154,10 @@ int drm_client_modeset_create(struct drm_client_dev *client);
>  void drm_client_modeset_free(struct drm_client_dev *client);
>  void drm_client_modeset_release(struct drm_client_dev *client);
>  struct drm_mode_set *drm_client_find_modeset(struct drm_client_dev *client, struct drm_crtc *crtc);
> +bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation);
> +int drm_client_modeset_commit_force(struct drm_client_dev *client);

I think latest here the _force postfix stopped making sense. It's just a
commit. Also I'm wondering whether we shouldn't pull the
master_acquire_internal into these helpers here, there's not really a
use-case I can think of where we should not check for other masters.

Only two kernel modeset requests want to ignore master status:
- debug enter/leave, which is utterly broken by design (and outright
  disable for any atomic driver)
- panic handling, for which we now have a really nice plan, plus first
  sketches of an implementation.

Cheers, Daniel

> +int drm_client_modeset_commit(struct drm_client_dev *client);
> +int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
>  
>  /**
>   * drm_client_for_each_modeset() - Iterate over client modesets
> -- 
> 2.20.1
>
Noralf Trønnes April 17, 2019, 5:56 p.m. UTC | #2
Den 16.04.2019 10.38, skrev Daniel Vetter:
> On Sun, Apr 07, 2019 at 06:52:39PM +0200, Noralf Trønnes wrote:
>> Move the modeset commit code to drm_client_modeset.
>> No changes except exporting API.
>>
>> v2: Move to drm_client_modeset.c instead of drm_client.c
>>
>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
>> ---
>>  drivers/gpu/drm/drm_client_modeset.c | 287 +++++++++++++++++++++++++++

<snip>

>> -/**
>> - * drm_client_panel_rotation() - Check panel orientation
>> - * @modeset: DRM modeset
>> - * @rotation: Returned rotation value
>> - *
>> - * This function checks if the primary plane in @modeset can hw rotate to match
>> - * the panel orientation on its connector.
>> - *
>> - * Note: Currently only 0 and 180 degrees are supported.
>> - *
>> - * Return:
>> - * True if the plane can do the rotation, false otherwise.
>> - */
>> -bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation)
> 
> Why not static? Doesn't seem to be used by anything outside of
> drm_client_modeset.c.
> 

It is used in drm_fb_helper.c:drm_setup_crtcs_fb() to set up any
rotation and do fbcon sw rotation if necessary. Clients that support
rotation need to call it.

>> -{
>> -	struct drm_connector *connector = modeset->connectors[0];
>> -	struct drm_plane *plane = modeset->crtc->primary;
>> -	u64 valid_mask = 0;
>> -	unsigned int i;
>> -
>> -	if (!modeset->num_connectors)
>> -		return false;
>> -
>> -	switch (connector->display_info.panel_orientation) {
>> -	case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
>> -		*rotation = DRM_MODE_ROTATE_180;
>> -		break;
>> -	case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
>> -		*rotation = DRM_MODE_ROTATE_90;
>> -		break;
>> -	case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
>> -		*rotation = DRM_MODE_ROTATE_270;
>> -		break;
>> -	default:
>> -		*rotation = DRM_MODE_ROTATE_0;
>> -	}
>> -
>> -	/*
>> -	 * TODO: support 90 / 270 degree hardware rotation,
>> -	 * depending on the hardware this may require the framebuffer
>> -	 * to be in a specific tiling format.
>> -	 */
>> -	if (*rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property)
>> -		return false;
>> -
>> -	for (i = 0; i < plane->rotation_property->num_values; i++)
>> -		valid_mask |= (1ULL << plane->rotation_property->values[i]);
>> -
>> -	if (!(*rotation & valid_mask))
>> -		return false;
>> -
>> -	return true;
>> -}
>> -

<snip>

>> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
>> index 858c8be70870..64b725b318f2 100644
>> --- a/include/drm/drm_client.h
>> +++ b/include/drm/drm_client.h
>> @@ -154,6 +154,10 @@ int drm_client_modeset_create(struct drm_client_dev *client);
>>  void drm_client_modeset_free(struct drm_client_dev *client);
>>  void drm_client_modeset_release(struct drm_client_dev *client);
>>  struct drm_mode_set *drm_client_find_modeset(struct drm_client_dev *client, struct drm_crtc *crtc);
>> +bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation);
>> +int drm_client_modeset_commit_force(struct drm_client_dev *client);
> 
> I think latest here the _force postfix stopped making sense. It's just a
> commit. Also I'm wondering whether we shouldn't pull the
> master_acquire_internal into these helpers here, there's not really a
> use-case I can think of where we should not check for other masters.
> 

drm_master_internal_acquire() is used in various places in drm_fb_helper
for functions that doesn't make sense to move to drm_client, like:
- drm_fb_helper_setcmap
- drm_fb_helper_ioctl
- drm_fb_helper_pan_display

Noralf.

> Only two kernel modeset requests want to ignore master status:
> - debug enter/leave, which is utterly broken by design (and outright
>   disable for any atomic driver)
> - panic handling, for which we now have a really nice plan, plus first
>   sketches of an implementation.
> 
> Cheers, Daniel
>
Daniel Vetter April 18, 2019, 8:30 a.m. UTC | #3
On Wed, Apr 17, 2019 at 07:56:20PM +0200, Noralf Trønnes wrote:
> 
> 
> Den 16.04.2019 10.38, skrev Daniel Vetter:
> > On Sun, Apr 07, 2019 at 06:52:39PM +0200, Noralf Trønnes wrote:
> >> Move the modeset commit code to drm_client_modeset.
> >> No changes except exporting API.
> >>
> >> v2: Move to drm_client_modeset.c instead of drm_client.c
> >>
> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> >> ---
> >>  drivers/gpu/drm/drm_client_modeset.c | 287 +++++++++++++++++++++++++++
> 
> <snip>
> 
> >> -/**
> >> - * drm_client_panel_rotation() - Check panel orientation
> >> - * @modeset: DRM modeset
> >> - * @rotation: Returned rotation value
> >> - *
> >> - * This function checks if the primary plane in @modeset can hw rotate to match
> >> - * the panel orientation on its connector.
> >> - *
> >> - * Note: Currently only 0 and 180 degrees are supported.
> >> - *
> >> - * Return:
> >> - * True if the plane can do the rotation, false otherwise.
> >> - */
> >> -bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation)
> > 
> > Why not static? Doesn't seem to be used by anything outside of
> > drm_client_modeset.c.
> > 
> 
> It is used in drm_fb_helper.c:drm_setup_crtcs_fb() to set up any
> rotation and do fbcon sw rotation if necessary. Clients that support
> rotation need to call it.
> 
> >> -{
> >> -	struct drm_connector *connector = modeset->connectors[0];
> >> -	struct drm_plane *plane = modeset->crtc->primary;
> >> -	u64 valid_mask = 0;
> >> -	unsigned int i;
> >> -
> >> -	if (!modeset->num_connectors)
> >> -		return false;
> >> -
> >> -	switch (connector->display_info.panel_orientation) {
> >> -	case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
> >> -		*rotation = DRM_MODE_ROTATE_180;
> >> -		break;
> >> -	case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
> >> -		*rotation = DRM_MODE_ROTATE_90;
> >> -		break;
> >> -	case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
> >> -		*rotation = DRM_MODE_ROTATE_270;
> >> -		break;
> >> -	default:
> >> -		*rotation = DRM_MODE_ROTATE_0;
> >> -	}
> >> -
> >> -	/*
> >> -	 * TODO: support 90 / 270 degree hardware rotation,
> >> -	 * depending on the hardware this may require the framebuffer
> >> -	 * to be in a specific tiling format.
> >> -	 */
> >> -	if (*rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property)
> >> -		return false;
> >> -
> >> -	for (i = 0; i < plane->rotation_property->num_values; i++)
> >> -		valid_mask |= (1ULL << plane->rotation_property->values[i]);
> >> -
> >> -	if (!(*rotation & valid_mask))
> >> -		return false;
> >> -
> >> -	return true;
> >> -}
> >> -
> 
> <snip>
> 
> >> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> >> index 858c8be70870..64b725b318f2 100644
> >> --- a/include/drm/drm_client.h
> >> +++ b/include/drm/drm_client.h
> >> @@ -154,6 +154,10 @@ int drm_client_modeset_create(struct drm_client_dev *client);
> >>  void drm_client_modeset_free(struct drm_client_dev *client);
> >>  void drm_client_modeset_release(struct drm_client_dev *client);
> >>  struct drm_mode_set *drm_client_find_modeset(struct drm_client_dev *client, struct drm_crtc *crtc);
> >> +bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation);
> >> +int drm_client_modeset_commit_force(struct drm_client_dev *client);
> > 
> > I think latest here the _force postfix stopped making sense. It's just a
> > commit. Also I'm wondering whether we shouldn't pull the
> > master_acquire_internal into these helpers here, there's not really a
> > use-case I can think of where we should not check for other masters.
> > 
> 
> drm_master_internal_acquire() is used in various places in drm_fb_helper
> for functions that doesn't make sense to move to drm_client, like:
> - drm_fb_helper_setcmap
> - drm_fb_helper_ioctl
> - drm_fb_helper_pan_display

See discussion on the earlier patches, I completely backtracked on this
after better understanding why we need _force.

And exporting/using master_acquire_internal by drm_clients makes total
sense to me.
-Daniel

> 
> Noralf.
> 
> > Only two kernel modeset requests want to ignore master status:
> > - debug enter/leave, which is utterly broken by design (and outright
> >   disable for any atomic driver)
> > - panic handling, for which we now have a really nice plan, plus first
> >   sketches of an implementation.
> > 
> > Cheers, Daniel
> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
index bb34222c9db8..94b52f97c06b 100644
--- a/drivers/gpu/drm/drm_client_modeset.c
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -11,9 +11,14 @@ 
 #include <linux/mutex.h>
 #include <linux/slab.h>
 
+#include <drm/drm_atomic.h>
 #include <drm/drm_client.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_device.h>
+#include <drm/drm_drv.h>
+
+#include "drm_crtc_internal.h"
+#include "drm_internal.h"
 
 /* Used by drm_client and drm_fb_helper */
 int drm_client_modeset_create(struct drm_client_dev *client)
@@ -100,3 +105,285 @@  struct drm_mode_set *drm_client_find_modeset(struct drm_client_dev *client, stru
 }
 /* TODO: Remove export when modeset code has been moved over */
 EXPORT_SYMBOL(drm_client_find_modeset);
+
+/**
+ * drm_client_panel_rotation() - Check panel orientation
+ * @modeset: DRM modeset
+ * @rotation: Returned rotation value
+ *
+ * This function checks if the primary plane in @modeset can hw rotate to match
+ * the panel orientation on its connector.
+ *
+ * Note: Currently only 0 and 180 degrees are supported.
+ *
+ * Return:
+ * True if the plane can do the rotation, false otherwise.
+ */
+bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation)
+{
+	struct drm_connector *connector = modeset->connectors[0];
+	struct drm_plane *plane = modeset->crtc->primary;
+	u64 valid_mask = 0;
+	unsigned int i;
+
+	if (!modeset->num_connectors)
+		return false;
+
+	switch (connector->display_info.panel_orientation) {
+	case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
+		*rotation = DRM_MODE_ROTATE_180;
+		break;
+	case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
+		*rotation = DRM_MODE_ROTATE_90;
+		break;
+	case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
+		*rotation = DRM_MODE_ROTATE_270;
+		break;
+	default:
+		*rotation = DRM_MODE_ROTATE_0;
+	}
+
+	/*
+	 * TODO: support 90 / 270 degree hardware rotation,
+	 * depending on the hardware this may require the framebuffer
+	 * to be in a specific tiling format.
+	 */
+	if (*rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property)
+		return false;
+
+	for (i = 0; i < plane->rotation_property->num_values; i++)
+		valid_mask |= (1ULL << plane->rotation_property->values[i]);
+
+	if (!(*rotation & valid_mask))
+		return false;
+
+	return true;
+}
+
+static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, bool active)
+{
+	struct drm_device *dev = client->dev;
+	struct drm_plane_state *plane_state;
+	struct drm_plane *plane;
+	struct drm_atomic_state *state;
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_mode_set *mode_set;
+	int ret;
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	state = drm_atomic_state_alloc(dev);
+	if (!state) {
+		ret = -ENOMEM;
+		goto out_ctx;
+	}
+
+	state->acquire_ctx = &ctx;
+retry:
+	drm_for_each_plane(plane, dev) {
+		plane_state = drm_atomic_get_plane_state(state, plane);
+		if (IS_ERR(plane_state)) {
+			ret = PTR_ERR(plane_state);
+			goto out_state;
+		}
+
+		plane_state->rotation = DRM_MODE_ROTATE_0;
+
+		/* disable non-primary: */
+		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
+			continue;
+
+		ret = __drm_atomic_helper_disable_plane(plane, plane_state);
+		if (ret != 0)
+			goto out_state;
+	}
+
+	drm_client_for_each_modeset(mode_set, client) {
+		struct drm_plane *primary = mode_set->crtc->primary;
+		unsigned int rotation;
+
+		if (drm_client_panel_rotation(mode_set, &rotation)) {
+			/* Cannot fail as we've already gotten the plane state above */
+			plane_state = drm_atomic_get_new_plane_state(state, primary);
+			plane_state->rotation = rotation;
+		}
+
+		ret = __drm_atomic_helper_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
+		 */
+		if (!active) {
+			struct drm_crtc *crtc = mode_set->crtc;
+			struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+
+			crtc_state->active = false;
+		}
+	}
+
+	ret = drm_atomic_commit(state);
+
+out_state:
+	if (ret == -EDEADLK)
+		goto backoff;
+
+	drm_atomic_state_put(state);
+out_ctx:
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	return ret;
+
+backoff:
+	drm_atomic_state_clear(state);
+	drm_modeset_backoff(&ctx);
+
+	goto retry;
+}
+
+static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
+{
+	struct drm_device *dev = client->dev;
+	struct drm_mode_set *mode_set;
+	struct drm_plane *plane;
+	int ret = 0;
+
+	drm_modeset_lock_all(dev);
+	drm_for_each_plane(plane, dev) {
+		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
+			drm_plane_force_disable(plane);
+
+		if (plane->rotation_property)
+			drm_mode_plane_set_obj_prop(plane,
+						    plane->rotation_property,
+						    DRM_MODE_ROTATE_0);
+	}
+
+	drm_client_for_each_modeset(mode_set, client) {
+		struct drm_crtc *crtc = mode_set->crtc;
+
+		if (crtc->funcs->cursor_set2) {
+			ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0);
+			if (ret)
+				goto out;
+		} else if (crtc->funcs->cursor_set) {
+			ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0);
+			if (ret)
+				goto out;
+		}
+
+		ret = drm_mode_set_config_internal(mode_set);
+		if (ret)
+			goto out;
+	}
+out:
+	drm_modeset_unlock_all(dev);
+
+	return ret;
+}
+
+/**
+ * drm_client_modeset_commit_force() - Force commit CRTC configuration
+ * @client: DRM client
+ *
+ * Commit modeset configuration to crtcs without checking if there is a DRM master.
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_client_modeset_commit_force(struct drm_client_dev *client)
+{
+	struct drm_device *dev = client->dev;
+	int ret;
+
+	mutex_lock(&client->modeset_mutex);
+	if (drm_drv_uses_atomic_modeset(dev))
+		ret = drm_client_modeset_commit_atomic(client, true);
+	else
+		ret = drm_client_modeset_commit_legacy(client);
+	mutex_unlock(&client->modeset_mutex);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_client_modeset_commit_force);
+
+/**
+ * drm_client_modeset_commit() - Commit CRTC configuration
+ * @client: DRM client
+ *
+ * Commit modeset configuration to crtcs.
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_client_modeset_commit(struct drm_client_dev *client)
+{
+	struct drm_device *dev = client->dev;
+	int ret;
+
+	if (!drm_master_internal_acquire(dev))
+		return -EBUSY;
+
+	ret = drm_client_modeset_commit_force(client);
+
+	drm_master_internal_release(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_client_modeset_commit);
+
+static void drm_client_modeset_dpms_legacy(struct drm_client_dev *client, int dpms_mode)
+{
+	struct drm_device *dev = client->dev;
+	struct drm_connector *connector;
+	struct drm_mode_set *modeset;
+	int j;
+
+	drm_modeset_lock_all(dev);
+	drm_client_for_each_modeset(modeset, client) {
+		if (!modeset->crtc->enabled)
+			continue;
+
+		for (j = 0; j < modeset->num_connectors; j++) {
+			connector = modeset->connectors[j];
+			connector->funcs->dpms(connector, dpms_mode);
+			drm_object_property_set_value(&connector->base,
+				dev->mode_config.dpms_property, dpms_mode);
+		}
+	}
+	drm_modeset_unlock_all(dev);
+}
+
+/**
+ * drm_client_modeset_dpms() - Set DPMS mode
+ * @client: DRM client
+ * @mode: DPMS mode
+ *
+ * Note: For atomic drivers @mode is reduced to on/off.
+ *
+ * Returns:
+ * Zero on success or negative error code on failure.
+ */
+int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
+{
+	struct drm_device *dev = client->dev;
+	int ret = 0;
+
+	if (!drm_master_internal_acquire(dev))
+		return -EBUSY;
+
+	mutex_lock(&client->modeset_mutex);
+	if (drm_drv_uses_atomic_modeset(dev))
+		ret = drm_client_modeset_commit_atomic(client, mode == DRM_MODE_DPMS_ON);
+	else
+		drm_client_modeset_dpms_legacy(client, mode);
+	mutex_unlock(&client->modeset_mutex);
+
+	drm_master_internal_release(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_client_modeset_dpms);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0229e187e6de..73fbcd748ddd 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -40,10 +40,7 @@ 
 #include <drm/drm_fb_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_atomic.h>
-#include <drm/drm_atomic_helper.h>
 
-#include "drm_crtc_internal.h"
-#include "drm_crtc_helper_internal.h"
 #include "drm_internal.h"
 
 static bool drm_fbdev_emulation = true;
@@ -388,233 +385,6 @@  int drm_fb_helper_debug_leave(struct fb_info *info)
 }
 EXPORT_SYMBOL(drm_fb_helper_debug_leave);
 
-/**
- * drm_client_panel_rotation() - Check panel orientation
- * @modeset: DRM modeset
- * @rotation: Returned rotation value
- *
- * This function checks if the primary plane in @modeset can hw rotate to match
- * the panel orientation on its connector.
- *
- * Note: Currently only 0 and 180 degrees are supported.
- *
- * Return:
- * True if the plane can do the rotation, false otherwise.
- */
-bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation)
-{
-	struct drm_connector *connector = modeset->connectors[0];
-	struct drm_plane *plane = modeset->crtc->primary;
-	u64 valid_mask = 0;
-	unsigned int i;
-
-	if (!modeset->num_connectors)
-		return false;
-
-	switch (connector->display_info.panel_orientation) {
-	case DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP:
-		*rotation = DRM_MODE_ROTATE_180;
-		break;
-	case DRM_MODE_PANEL_ORIENTATION_LEFT_UP:
-		*rotation = DRM_MODE_ROTATE_90;
-		break;
-	case DRM_MODE_PANEL_ORIENTATION_RIGHT_UP:
-		*rotation = DRM_MODE_ROTATE_270;
-		break;
-	default:
-		*rotation = DRM_MODE_ROTATE_0;
-	}
-
-	/*
-	 * TODO: support 90 / 270 degree hardware rotation,
-	 * depending on the hardware this may require the framebuffer
-	 * to be in a specific tiling format.
-	 */
-	if (*rotation != DRM_MODE_ROTATE_180 || !plane->rotation_property)
-		return false;
-
-	for (i = 0; i < plane->rotation_property->num_values; i++)
-		valid_mask |= (1ULL << plane->rotation_property->values[i]);
-
-	if (!(*rotation & valid_mask))
-		return false;
-
-	return true;
-}
-
-static int drm_client_modeset_commit_atomic(struct drm_client_dev *client, bool active)
-{
-	struct drm_device *dev = client->dev;
-	struct drm_plane_state *plane_state;
-	struct drm_plane *plane;
-	struct drm_atomic_state *state;
-	struct drm_modeset_acquire_ctx ctx;
-	struct drm_mode_set *mode_set;
-	int ret;
-
-	drm_modeset_acquire_init(&ctx, 0);
-
-	state = drm_atomic_state_alloc(dev);
-	if (!state) {
-		ret = -ENOMEM;
-		goto out_ctx;
-	}
-
-	state->acquire_ctx = &ctx;
-retry:
-	drm_for_each_plane(plane, dev) {
-		plane_state = drm_atomic_get_plane_state(state, plane);
-		if (IS_ERR(plane_state)) {
-			ret = PTR_ERR(plane_state);
-			goto out_state;
-		}
-
-		plane_state->rotation = DRM_MODE_ROTATE_0;
-
-		/* disable non-primary: */
-		if (plane->type == DRM_PLANE_TYPE_PRIMARY)
-			continue;
-
-		ret = __drm_atomic_helper_disable_plane(plane, plane_state);
-		if (ret != 0)
-			goto out_state;
-	}
-
-	drm_client_for_each_modeset(mode_set, client) {
-		struct drm_plane *primary = mode_set->crtc->primary;
-		unsigned int rotation;
-
-		if (drm_client_panel_rotation(mode_set, &rotation)) {
-			/* Cannot fail as we've already gotten the plane state above */
-			plane_state = drm_atomic_get_new_plane_state(state, primary);
-			plane_state->rotation = rotation;
-		}
-
-		ret = __drm_atomic_helper_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
-		 */
-		if (!active) {
-			struct drm_crtc *crtc = mode_set->crtc;
-			struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
-
-			crtc_state->active = false;
-		}
-	}
-
-	ret = drm_atomic_commit(state);
-
-out_state:
-	if (ret == -EDEADLK)
-		goto backoff;
-
-	drm_atomic_state_put(state);
-out_ctx:
-	drm_modeset_drop_locks(&ctx);
-	drm_modeset_acquire_fini(&ctx);
-
-	return ret;
-
-backoff:
-	drm_atomic_state_clear(state);
-	drm_modeset_backoff(&ctx);
-
-	goto retry;
-}
-
-static int drm_client_modeset_commit_legacy(struct drm_client_dev *client)
-{
-	struct drm_device *dev = client->dev;
-	struct drm_mode_set *mode_set;
-	struct drm_plane *plane;
-	int ret = 0;
-
-	drm_modeset_lock_all(dev);
-	drm_for_each_plane(plane, dev) {
-		if (plane->type != DRM_PLANE_TYPE_PRIMARY)
-			drm_plane_force_disable(plane);
-
-		if (plane->rotation_property)
-			drm_mode_plane_set_obj_prop(plane,
-						    plane->rotation_property,
-						    DRM_MODE_ROTATE_0);
-	}
-
-	drm_client_for_each_modeset(mode_set, client) {
-		struct drm_crtc *crtc = mode_set->crtc;
-
-		if (crtc->funcs->cursor_set2) {
-			ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0);
-			if (ret)
-				goto out;
-		} else if (crtc->funcs->cursor_set) {
-			ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0);
-			if (ret)
-				goto out;
-		}
-
-		ret = drm_mode_set_config_internal(mode_set);
-		if (ret)
-			goto out;
-	}
-out:
-	drm_modeset_unlock_all(dev);
-
-	return ret;
-}
-
-/**
- * drm_client_modeset_commit_force() - Force commit CRTC configuration
- * @client: DRM client
- *
- * Commit modeset configuration to crtcs without checking if there is a DRM master.
- *
- * Returns:
- * Zero on success or negative error code on failure.
- */
-int drm_client_modeset_commit_force(struct drm_client_dev *client)
-{
-	struct drm_device *dev = client->dev;
-	int ret;
-
-	mutex_lock(&client->modeset_mutex);
-	if (drm_drv_uses_atomic_modeset(dev))
-		ret = drm_client_modeset_commit_atomic(client, true);
-	else
-		ret = drm_client_modeset_commit_legacy(client);
-	mutex_unlock(&client->modeset_mutex);
-
-	return ret;
-}
-
-/**
- * drm_client_modeset_commit() - Commit CRTC configuration
- * @client: DRM client
- *
- * Commit modeset configuration to crtcs.
- *
- * Returns:
- * Zero on success or negative error code on failure.
- */
-int drm_client_modeset_commit(struct drm_client_dev *client)
-{
-	struct drm_device *dev = client->dev;
-	int ret;
-
-	if (!drm_master_internal_acquire(dev))
-		return -EBUSY;
-
-	ret = drm_client_modeset_commit_force(client);
-
-	drm_master_internal_release(dev);
-
-	return ret;
-}
-
 /**
  * drm_fb_helper_restore_fbdev_mode_unlocked - restore fbdev configuration
  * @fb_helper: driver-allocated fbdev helper, can be NULL
@@ -704,58 +474,6 @@  static struct sysrq_key_op sysrq_drm_fb_helper_restore_op = {
 static struct sysrq_key_op sysrq_drm_fb_helper_restore_op = { };
 #endif
 
-static void drm_client_modeset_dpms_legacy(struct drm_client_dev *client, int dpms_mode)
-{
-	struct drm_device *dev = client->dev;
-	struct drm_connector *connector;
-	struct drm_mode_set *modeset;
-	int j;
-
-	drm_modeset_lock_all(dev);
-	drm_client_for_each_modeset(modeset, client) {
-		if (!modeset->crtc->enabled)
-			continue;
-
-		for (j = 0; j < modeset->num_connectors; j++) {
-			connector = modeset->connectors[j];
-			connector->funcs->dpms(connector, dpms_mode);
-			drm_object_property_set_value(&connector->base,
-				dev->mode_config.dpms_property, dpms_mode);
-		}
-	}
-	drm_modeset_unlock_all(dev);
-}
-
-/**
- * drm_client_modeset_dpms() - Set DPMS mode
- * @client: DRM client
- * @mode: DPMS mode
- *
- * Note: For atomic drivers @mode is reduced to on/off.
- *
- * Returns:
- * Zero on success or negative error code on failure.
- */
-int drm_client_modeset_dpms(struct drm_client_dev *client, int mode)
-{
-	struct drm_device *dev = client->dev;
-	int ret = 0;
-
-	if (!drm_master_internal_acquire(dev))
-		return -EBUSY;
-
-	mutex_lock(&client->modeset_mutex);
-	if (drm_drv_uses_atomic_modeset(dev))
-		ret = drm_client_modeset_commit_atomic(client, mode == DRM_MODE_DPMS_ON);
-	else
-		drm_client_modeset_dpms_legacy(client, mode);
-	mutex_unlock(&client->modeset_mutex);
-
-	drm_master_internal_release(dev);
-
-	return ret;
-}
-
 static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 {
 	struct drm_fb_helper *fb_helper = info->par;
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 858c8be70870..64b725b318f2 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -154,6 +154,10 @@  int drm_client_modeset_create(struct drm_client_dev *client);
 void drm_client_modeset_free(struct drm_client_dev *client);
 void drm_client_modeset_release(struct drm_client_dev *client);
 struct drm_mode_set *drm_client_find_modeset(struct drm_client_dev *client, struct drm_crtc *crtc);
+bool drm_client_panel_rotation(struct drm_mode_set *modeset, unsigned int *rotation);
+int drm_client_modeset_commit_force(struct drm_client_dev *client);
+int drm_client_modeset_commit(struct drm_client_dev *client);
+int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
 
 /**
  * drm_client_for_each_modeset() - Iterate over client modesets