diff mbox series

[v5,05/11] drm/fb-helper: Remove drm_fb_helper_crtc

Message ID 20190506180139.6913-6-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 May 6, 2019, 6:01 p.m. UTC
It now only contains the modeset so use that directly instead and attach
a modeset array to drm_client_dev. drm_fb_helper will use this array.
Code will later be moved to drm_client, so add code there in a new file
drm_client_modeset.c with MIT license to match drm_fb_helper.c.

The modeset connector array size is hardcoded for the cloned case to avoid
having to pass in a value from the driver. A value of 8 is chosen to err
on the safe side. This means that the max connector argument for
drm_fb_helper_init() and drm_fb_helper_fbdev_setup() isn't used anymore,
a todo entry for this is added.

In pan_display_atomic() restore_fbdev_mode_force() is used instead of
restore_fbdev_mode_atomic() because that one will later become internal
to drm_client_modeset.

Locking order:
1. drm_fb_helper->lock
2. drm_master_internal_acquire
3. drm_client_dev->modeset_mutex

v3:
- Use full drm_client_init/release for the modesets (Daniel Vetter)
- drm_client_for_each_modeset: use lockdep_assert_held (Daniel Vetter)
- Hook up to Documentation/gpu/drm-client.rst (Daniel Vetter)

v2:
- Add modesets array to drm_client (Daniel Vetter)
- Use a new file for the modeset code (Daniel Vetter)
- File has to be MIT licensed (Emmanuel Vadot)
- Add copyrights from drm_fb_helper.c

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 Documentation/gpu/drm-client.rst     |   3 +
 Documentation/gpu/todo.rst           |   3 +
 drivers/gpu/drm/Makefile             |   2 +-
 drivers/gpu/drm/drm_client.c         |  10 +-
 drivers/gpu/drm/drm_client_modeset.c | 104 +++++++++
 drivers/gpu/drm/drm_fb_helper.c      | 301 +++++++++++----------------
 include/drm/drm_client.h             |  30 +++
 include/drm/drm_fb_helper.h          |   8 -
 8 files changed, 274 insertions(+), 187 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_client_modeset.c

Comments

Sam Ravnborg May 15, 2019, 9:04 a.m. UTC | #1
Hi Noralf.

I have read through the cahnes a copuple of times not and feel confident
to add my r-b if the comments are considered.

On Mon, May 06, 2019 at 08:01:33PM +0200, Noralf Trønnes wrote:
> It now only contains the modeset so use that directly instead and attach
> a modeset array to drm_client_dev. drm_fb_helper will use this array.
> Code will later be moved to drm_client, so add code there in a new file
> drm_client_modeset.c with MIT license to match drm_fb_helper.c.

The first part of this commit log could use some re-pharsing.
What is "It" etc.

> @@ -92,14 +91,20 @@ int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
>  	client->name = name;
>  	client->funcs = funcs;
>  
> -	ret = drm_client_open(client);
> +	ret = drm_client_modeset_create(client);
>  	if (ret)
>  		goto err_put_module;
>  
> +	ret = drm_client_open(client);
> +	if (ret)
> +		goto err_free;
> +
>  	drm_dev_get(dev);
>  
>  	return 0;
>  
> +err_free:
> +	drm_client_modeset_free(client);
>  err_put_module:
>  	if (funcs)
>  		module_put(funcs->owner);
> @@ -148,6 +153,7 @@ void drm_client_release(struct drm_client_dev *client)
>  
>  	DRM_DEV_DEBUG_KMS(dev->dev, "%s\n", client->name);
>  
> +	drm_client_modeset_free(client);
>  	drm_client_close(client);
>  	drm_dev_put(dev);

If the order should be the opposite of init then call drm_client_close() before
drm_client_modeset_free()

>  	if (client->funcs)

> +	drm_client_for_each_modeset(mode_set, client) {
>  		struct drm_plane *primary = mode_set->crtc->primary;
>  		unsigned int rotation;
>  
> @@ -517,9 +518,11 @@ static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ
>  
>  static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
>  {
> +	struct drm_client_dev *client = &fb_helper->client;
>  	struct drm_device *dev = fb_helper->dev;
> +	struct drm_mode_set *mode_set;
>  	struct drm_plane *plane;
> -	int i, ret = 0;
> +	int ret = 0;
>  
>  	drm_modeset_lock_all(fb_helper->dev);
>  	drm_for_each_plane(plane, dev) {
> @@ -532,8 +535,7 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
>  						    DRM_MODE_ROTATE_0);
>  	}
>  
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
> +	drm_client_for_each_modeset(mode_set, client) {
>  		struct drm_crtc *crtc = mode_set->crtc;
>  
>  		if (crtc->funcs->cursor_set2) {
This function requires modeset_mutex to be held. Maybe add comment?

> @@ -682,15 +689,14 @@ static struct sysrq_key_op sysrq_drm_fb_helper_restore_op = { };
>  
>  static void dpms_legacy(struct drm_fb_helper *fb_helper, int dpms_mode)
>  {
> +	struct drm_client_dev *client = &fb_helper->client;
>  	struct drm_device *dev = fb_helper->dev;
>  	struct drm_connector *connector;
>  	struct drm_mode_set *modeset;
> -	int i, j;
> +	int j;
>  
>  	drm_modeset_lock_all(dev);
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		modeset = &fb_helper->crtc_info[i].mode_set;
> -
> +	drm_client_for_each_modeset(modeset, client) {
>  		if (!modeset->crtc->enabled)
>  			continue;
>
This function needs modeset_mutex - maybe add comment?

> @@ -1390,13 +1353,14 @@ static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info)
>  static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info)
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
> +	struct drm_mode_set *modeset;
>  	struct drm_crtc *crtc;
>  	u16 *r, *g, *b;
> -	int i, ret = 0;
> +	int ret = 0;
>  
>  	drm_modeset_lock_all(fb_helper->dev);
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> +	drm_client_for_each_modeset(modeset, &fb_helper->client) {
> +		crtc = modeset->crtc;
>  		if (!crtc->funcs->gamma_set || !crtc->gamma_size)
>  			return -EINVAL;
>  
This function needs modeset_mutex - maybe add comment?

> @@ -1472,10 +1436,11 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
>  	struct drm_modeset_acquire_ctx ctx;
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_atomic_state *state;
> +	struct drm_mode_set *modeset;
>  	struct drm_crtc *crtc;
>  	u16 *r, *g, *b;
> -	int i, ret = 0;
>  	bool replaced;
> +	int ret = 0;
>  
>  	drm_modeset_acquire_init(&ctx, 0);
>  
> @@ -1487,8 +1452,8 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
>  
>  	state->acquire_ctx = &ctx;
>  retry:
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> +	drm_client_for_each_modeset(modeset, &fb_helper->client) {
> +		crtc = modeset->crtc;
>  
>  		if (!gamma_lut)
>  			gamma_lut = setcmap_new_gamma_lut(crtc, cmap);
> @@ -1516,8 +1481,8 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
>  	if (ret)
>  		goto out_state;
>  
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		crtc = fb_helper->crtc_info[i].mode_set.crtc;
> +	drm_client_for_each_modeset(modeset, &fb_helper->client) {
> +		crtc = modeset->crtc;
>  
>  		r = crtc->gamma_store;
>  		g = r + crtc->gamma_size;
This function needs modeset_mutex - maybe add comment?

> @@ -1567,12 +1532,14 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
>  		goto unlock;
>  	}
>  
> +	mutex_lock(&fb_helper->client.modeset_mutex);
>  	if (info->fix.visual == FB_VISUAL_TRUECOLOR)
>  		ret = setcmap_pseudo_palette(cmap, info);
>  	else if (drm_drv_uses_atomic_modeset(fb_helper->dev))
>  		ret = setcmap_atomic(cmap, info);
>  	else
>  		ret = setcmap_legacy(cmap, info);
> +	mutex_unlock(&fb_helper->client.modeset_mutex);
>  
>  	drm_master_internal_release(dev);
>  unlock:
> @@ -1596,7 +1563,6 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
>  	struct drm_device *dev = fb_helper->dev;
> -	struct drm_mode_set *mode_set;
>  	struct drm_crtc *crtc;
>  	int ret = 0;
>  
> @@ -1624,8 +1590,7 @@ int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
>  		 * make. If we're not smart enough here, one should
>  		 * just consider switch the userspace to KMS.
>  		 */
> -		mode_set = &fb_helper->crtc_info[0].mode_set;
> -		crtc = mode_set->crtc;
> +		crtc = fb_helper->client.modesets[0].crtc;
>  
>  		/*
>  		 * Only wait for a vblank event if the CRTC is
> @@ -1822,16 +1787,14 @@ EXPORT_SYMBOL(drm_fb_helper_set_par);
>  
>  static void pan_set(struct drm_fb_helper *fb_helper, int x, int y)
>  {
> -	int i;
> -
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		struct drm_mode_set *mode_set;
> -
> -		mode_set = &fb_helper->crtc_info[i].mode_set;
> +	struct drm_mode_set *mode_set;
>  
> +	mutex_lock(&fb_helper->client.modeset_mutex);
> +	drm_client_for_each_modeset(mode_set, &fb_helper->client) {
>  		mode_set->x = x;
>  		mode_set->y = y;
>  	}
> +	mutex_unlock(&fb_helper->client.modeset_mutex);
>  }
>  
>  static int pan_display_atomic(struct fb_var_screeninfo *var,
> @@ -1842,7 +1805,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
>  
>  	pan_set(fb_helper, var->xoffset, var->yoffset);
>  
> -	ret = restore_fbdev_mode_atomic(fb_helper, true);
> +	ret = restore_fbdev_mode_force(fb_helper);
This change looks alien compared to other changes.
Does it belong to this patchset?

>  	if (!ret) {
>  		info->var.xoffset = var->xoffset;
>  		info->var.yoffset = var->yoffset;
> @@ -1856,14 +1819,13 @@ static int pan_display_legacy(struct fb_var_screeninfo *var,
>  			      struct fb_info *info)
>  {
>  	struct drm_fb_helper *fb_helper = info->par;
> +	struct drm_client_dev *client = &fb_helper->client;
>  	struct drm_mode_set *modeset;
>  	int ret = 0;
> -	int i;
>  
>  	drm_modeset_lock_all(fb_helper->dev);
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		modeset = &fb_helper->crtc_info[i].mode_set;
> -
> +	mutex_lock(&client->modeset_mutex);
> +	drm_client_for_each_modeset(modeset, client) {
>  		modeset->x = var->xoffset;
>  		modeset->y = var->yoffset;
>  
> @@ -1875,6 +1837,7 @@ static int pan_display_legacy(struct fb_var_screeninfo *var,
>  			}
>  		}
>  	}
> +	mutex_unlock(&client->modeset_mutex);
>  	drm_modeset_unlock_all(fb_helper->dev);
>  
>  	return ret;
> @@ -1921,10 +1884,12 @@ EXPORT_SYMBOL(drm_fb_helper_pan_display);
>  static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  					 int preferred_bpp)
>  {
> +	struct drm_client_dev *client = &fb_helper->client;
>  	int ret = 0;
>  	int crtc_count = 0;
>  	int i;
>  	struct drm_fb_helper_surface_size sizes;
> +	struct drm_mode_set *mode_set;
>  	int best_depth = 0;
>  
>  	memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
> @@ -1975,13 +1940,13 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  	 * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, depth
>  	 * 16) we need to scale down the depth of the sizes we request.
>  	 */
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
> +	mutex_lock(&client->modeset_mutex);
> +	drm_client_for_each_modeset(mode_set, client) {
>  		struct drm_crtc *crtc = mode_set->crtc;
>  		struct drm_plane *plane = crtc->primary;
>  		int j;
>  
> -		DRM_DEBUG("test CRTC %d primary plane\n", i);
> +		DRM_DEBUG("test CRTC %u primary plane\n", drm_crtc_index(crtc));
>  
>  		for (j = 0; j < plane->format_count; j++) {
>  			const struct drm_format_info *fmt;
> @@ -2021,9 +1986,8 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  
>  	/* first up get a count of crtcs now in use and new min/maxes width/heights */
>  	crtc_count = 0;
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> +	drm_client_for_each_modeset(mode_set, client) {
>  		struct drm_display_mode *desired_mode;
> -		struct drm_mode_set *mode_set;
>  		int x, y, j;
>  		/* in case of tile group, are we the last tile vert or horiz?
>  		 * If no tile group you are always the last one both vertically
> @@ -2031,7 +1995,6 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  		 */
>  		bool lastv = true, lasth = true;
>  
> -		mode_set = &fb_helper->crtc_info[i].mode_set;
>  		desired_mode = mode_set->mode;
>  
>  		if (!desired_mode)
> @@ -2061,6 +2024,7 @@ static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
>  		if (lastv)
>  			sizes.fb_height = min_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
>  	}
> +	mutex_unlock(&client->modeset_mutex);
>  
>  	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
>  		DRM_INFO("Cannot find any crtc or sizes\n");
> @@ -2292,7 +2256,7 @@ static bool drm_target_cloned(struct drm_fb_helper *fb_helper,
>  	struct drm_display_mode *dmt_mode, *mode;
>  
>  	/* only contemplate cloning in the single crtc case */
> -	if (fb_helper->crtc_count > 1)
> +	if (fb_helper->dev->mode_config.num_crtc > 1)
>  		return false;
>  
>  	count = 0;
> @@ -2481,15 +2445,17 @@ static bool connector_has_possible_crtc(struct drm_connector *connector,
>  }
>  
>  static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
> -			  struct drm_fb_helper_crtc **best_crtcs,
> +			  struct drm_crtc **best_crtcs,
>  			  struct drm_display_mode **modes,
>  			  int n, int width, int height)
>  {
> -	int c, o;
> +	struct drm_client_dev *client = &fb_helper->client;
>  	struct drm_connector *connector;
>  	int my_score, best_score, score;
> -	struct drm_fb_helper_crtc **crtcs, *crtc;
> +	struct drm_crtc **crtcs, *crtc;
> +	struct drm_mode_set *modeset;
>  	struct drm_fb_helper_connector *fb_helper_conn;
> +	int o;
>  
>  	if (n == fb_helper->connector_count)
>  		return 0;

drm_pick_crtcs() needs the modeset mutex.
And due considering the complexity of the function it would be nice
to have this fact documented.

> @@ -2502,8 +2468,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>  	if (modes[n] == NULL)
>  		return best_score;
>  
> -	crtcs = kcalloc(fb_helper->connector_count,
> -			sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
> +	crtcs = kcalloc(fb_helper->connector_count, sizeof(*crtcs), GFP_KERNEL);
>  	if (!crtcs)
>  		return best_score;
>  
> @@ -2519,11 +2484,10 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>  	 * select a crtc for this connector and then attempt to configure
>  	 * remaining connectors
>  	 */
> -	for (c = 0; c < fb_helper->crtc_count; c++) {
> -		crtc = &fb_helper->crtc_info[c];
> +	drm_client_for_each_modeset(modeset, client) {
> +		crtc = modeset->crtc;
>  
> -		if (!connector_has_possible_crtc(connector,
> -						 crtc->mode_set.crtc))
> +		if (!connector_has_possible_crtc(connector, crtc))
>  			continue;
>  
>  		for (o = 0; o < n; o++)
> @@ -2532,7 +2496,7 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>  
>  		if (o < n) {
>  			/* ignore cloning unless only a single crtc */
> -			if (fb_helper->crtc_count > 1)
> +			if (fb_helper->dev->mode_config.num_crtc > 1)
>  				continue;
>  
>  			if (!drm_mode_equal(modes[o], modes[n]))
> @@ -2540,14 +2504,13 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>  		}
>  
>  		crtcs[n] = crtc;
> -		memcpy(crtcs, best_crtcs, n * sizeof(struct drm_fb_helper_crtc *));
> +		memcpy(crtcs, best_crtcs, n * sizeof(*crtcs));
>  		score = my_score + drm_pick_crtcs(fb_helper, crtcs, modes, n + 1,
>  						  width, height);
>  		if (score > best_score) {
>  			best_score = score;
>  			memcpy(best_crtcs, crtcs,
> -			       fb_helper->connector_count *
> -			       sizeof(struct drm_fb_helper_crtc *));
> +			       fb_helper->connector_count * sizeof(*crtcs));
>  		}
>  	}
>  
> @@ -2555,21 +2518,9 @@ static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
>  	return best_score;
>  }
>  
> -static struct drm_fb_helper_crtc *
> -drm_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
> -{
> -	int i;
> -
> -	for (i = 0; i < fb_helper->crtc_count; i++)
> -		if (fb_helper->crtc_info[i].mode_set.crtc == crtc)
> -			return &fb_helper->crtc_info[i];
> -
> -	return NULL;
> -}
> -
>  /* Try to read the BIOS display configuration and use it for the initial config */
>  static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper,
> -					  struct drm_fb_helper_crtc **crtcs,
> +					  struct drm_crtc **crtcs,
>  					  struct drm_display_mode **modes,
>  					  struct drm_fb_offset *offsets,
>  					  bool *enabled, int width, int height)
> @@ -2605,7 +2556,7 @@ static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper,
>  		struct drm_fb_helper_connector *fb_conn;
>  		struct drm_connector *connector;
>  		struct drm_encoder *encoder;
> -		struct drm_fb_helper_crtc *new_crtc;
> +		struct drm_crtc *new_crtc;
>  
>  		fb_conn = fb_helper->connector_info[i];
>  		connector = fb_conn->connector;
> @@ -2647,7 +2598,7 @@ static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper,
>  
>  		num_connectors_enabled++;
>  
> -		new_crtc = drm_fb_helper_crtc(fb_helper, connector->state->crtc);
> +		new_crtc = connector->state->crtc;
>  
>  		/*
>  		 * Make sure we're not trying to drive multiple connectors
> @@ -2747,10 +2698,11 @@ static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper,
>  static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>  			    u32 width, u32 height)
>  {
> +	struct drm_client_dev *client = &fb_helper->client;
>  	struct drm_device *dev = fb_helper->dev;
> -	struct drm_fb_helper_crtc **crtcs;
>  	struct drm_display_mode **modes;
>  	struct drm_fb_offset *offsets;
> +	struct drm_crtc **crtcs;
>  	bool *enabled;
>  	int i;
>  
> @@ -2758,8 +2710,7 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>  	/* prevent concurrent modification of connector_count by hotplug */
>  	lockdep_assert_held(&fb_helper->lock);
>  
> -	crtcs = kcalloc(fb_helper->connector_count,
> -			sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
> +	crtcs = kcalloc(fb_helper->connector_count, sizeof(*crtcs), GFP_KERNEL);
>  	modes = kcalloc(fb_helper->connector_count,
>  			sizeof(struct drm_display_mode *), GFP_KERNEL);
>  	offsets = kcalloc(fb_helper->connector_count,
> @@ -2771,6 +2722,8 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>  		goto out;
>  	}
>  
> +	mutex_lock(&client->modeset_mutex);
> +
>  	mutex_lock(&fb_helper->dev->mode_config.mutex);
>  	if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
>  		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
> @@ -2795,24 +2748,24 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>  	}
>  	mutex_unlock(&fb_helper->dev->mode_config.mutex);
>  
> -	/* need to set the modesets up here for use later */
> -	/* fill out the connector<->crtc mappings into the modesets */
> -	for (i = 0; i < fb_helper->crtc_count; i++)
> -		drm_fb_helper_modeset_release(fb_helper,
> -					      &fb_helper->crtc_info[i].mode_set);
> +	drm_client_modeset_release(client);
>  
>  	drm_fb_helper_for_each_connector(fb_helper, i) {
>  		struct drm_display_mode *mode = modes[i];
> -		struct drm_fb_helper_crtc *fb_crtc = crtcs[i];
> +		struct drm_crtc *crtc = crtcs[i];
>  		struct drm_fb_offset *offset = &offsets[i];
>  
> -		if (mode && fb_crtc) {
> -			struct drm_mode_set *modeset = &fb_crtc->mode_set;
> +		if (mode && crtc) {
> +			struct drm_mode_set *modeset = drm_client_find_modeset(client, crtc);
>  			struct drm_connector *connector =
>  				fb_helper->connector_info[i]->connector;
>  
>  			DRM_DEBUG_KMS("desired mode %s set on crtc %d (%d,%d)\n",
> -				      mode->name, fb_crtc->mode_set.crtc->base.id, offset->x, offset->y);
> +				      mode->name, crtc->base.id, offset->x, offset->y);
> +
> +			if (WARN_ON_ONCE(modeset->num_connectors == DRM_CLIENT_MAX_CLONED_CONNECTORS ||
> +					 (dev->mode_config.num_crtc > 1 && modeset->num_connectors == 1)))
> +				break;
>  
>  			modeset->mode = drm_mode_duplicate(dev, mode);
>  			drm_connector_get(connector);
> @@ -2821,6 +2774,8 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>  			modeset->y = offset->y;
>  		}
>  	}
> +
> +	mutex_unlock(&client->modeset_mutex);
>  out:
>  	kfree(crtcs);
>  	kfree(modes);
> @@ -2837,13 +2792,14 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
>   */
>  static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
>  {
> +	struct drm_client_dev *client = &fb_helper->client;
>  	struct fb_info *info = fb_helper->fbdev;
>  	unsigned int rotation, sw_rotations = 0;
> +	struct drm_mode_set *modeset;
>  	int i;
>  
> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> -		struct drm_mode_set *modeset = &fb_helper->crtc_info[i].mode_set;
> -
> +	mutex_lock(&client->modeset_mutex);
> +	drm_client_for_each_modeset(modeset, client) {
>  		if (!modeset->num_connectors)
>  			continue;
>  
> @@ -2855,6 +2811,7 @@ static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
>  		else
>  			sw_rotations |= rotation;
>  	}
> +	mutex_unlock(&client->modeset_mutex);
>  
>  	mutex_lock(&fb_helper->dev->mode_config.mutex);
>  	drm_fb_helper_for_each_connector(fb_helper, i) {
> @@ -3071,8 +3028,7 @@ EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
>   * @funcs: fbdev helper functions
>   * @preferred_bpp: Preferred bits per pixel for the device.
>   *                 @dev->mode_config.preferred_depth is used if this is zero.
> - * @max_conn_count: Maximum number of connectors.
> - *                  @dev->mode_config.num_connector is used if this is zero.
> + * @max_conn_count: Maximum number of connectors (not used)
>   *
>   * This function sets up fbdev emulation and registers fbdev for access by
>   * userspace. If all connectors are disconnected, setup is deferred to the next
> @@ -3100,16 +3056,9 @@ int drm_fb_helper_fbdev_setup(struct drm_device *dev,
>  	if (!preferred_bpp)
>  		preferred_bpp = 32;
>  
> -	if (!max_conn_count)
> -		max_conn_count = dev->mode_config.num_connector;
> -	if (!max_conn_count) {
> -		DRM_DEV_ERROR(dev->dev, "fbdev: No connectors\n");
> -		return -EINVAL;
> -	}
> -
>  	drm_fb_helper_prepare(dev, fb_helper, funcs);
>  
> -	ret = drm_fb_helper_init(dev, fb_helper, max_conn_count);
> +	ret = drm_fb_helper_init(dev, fb_helper, 0);
>  	if (ret < 0) {
>  		DRM_DEV_ERROR(dev->dev, "fbdev: Failed to initialize (ret=%d)\n", ret);
>  		return ret;
> @@ -3422,7 +3371,7 @@ static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
>  
>  	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
>  
> -	ret = drm_fb_helper_init(dev, fb_helper, dev->mode_config.num_connector);
> +	ret = drm_fb_helper_init(dev, fb_helper, 0);
>  	if (ret)
>  		goto err;
>
Familien Trønnes May 15, 2019, 2:35 p.m. UTC | #2
Hi Sam,

Den 15.05.2019 11.04, skrev Sam Ravnborg:
> Hi Noralf.
> 
> I have read through the cahnes a copuple of times not and feel confident
> to add my r-b if the comments are considered.
> 
> On Mon, May 06, 2019 at 08:01:33PM +0200, Noralf Trønnes wrote:
>> It now only contains the modeset so use that directly instead and attach
>> a modeset array to drm_client_dev. drm_fb_helper will use this array.
>> Code will later be moved to drm_client, so add code there in a new file
>> drm_client_modeset.c with MIT license to match drm_fb_helper.c.
> 
> The first part of this commit log could use some re-pharsing.
> What is "It" etc.
> 

I could do this:

struct drm_fb_helper_crtc is now just a wrapper around drm_mode_set so
use that directly instead and attach it as a modeset array onto
drm_client_dev. drm_fb_helper will use this array to store its modesets
which means it will always initialize a drm_client, but it will not
register the client (callbacks) unless it's the generic fbdev emulation.

>> @@ -532,8 +535,7 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
>>  						    DRM_MODE_ROTATE_0);
>>  	}
>>  
>> -	for (i = 0; i < fb_helper->crtc_count; i++) {
>> -		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
>> +	drm_client_for_each_modeset(mode_set, client) {
>>  		struct drm_crtc *crtc = mode_set->crtc;
>>  
>>  		if (crtc->funcs->cursor_set2) {
> This function requires modeset_mutex to be held. Maybe add comment?
> 

drm_client_for_each_modeset() warns if it's not held (courtesy of Daniel
Vetter):

#define drm_client_for_each_modeset(modeset, client) \
	for (({ lockdep_assert_held(&(client)->modeset_mutex); }), \
	     modeset = (client)->modesets; modeset->crtc; modeset++)

>> @@ -1842,7 +1805,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
>>  
>>  	pan_set(fb_helper, var->xoffset, var->yoffset);
>>  
>> -	ret = restore_fbdev_mode_atomic(fb_helper, true);
>> +	ret = restore_fbdev_mode_force(fb_helper);
> This change looks alien compared to other changes.
> Does it belong to this patchset?
> 

It's explained in the commit message:

In pan_display_atomic() restore_fbdev_mode_force() is used instead of
restore_fbdev_mode_atomic() because that one will later become internal
to drm_client_modeset.

Noralf.
Noralf Trønnes May 15, 2019, 2:51 p.m. UTC | #3
Hi Sam,

[looks like Thundebird decided to throw away my reply, so I'll try again]

Den 15.05.2019 11.04, skrev Sam Ravnborg:
> Hi Noralf.
> 
> I have read through the cahnes a copuple of times not and feel confident
> to add my r-b if the comments are considered.
> 
> On Mon, May 06, 2019 at 08:01:33PM +0200, Noralf Trønnes wrote:
>> It now only contains the modeset so use that directly instead and attach
>> a modeset array to drm_client_dev. drm_fb_helper will use this array.
>> Code will later be moved to drm_client, so add code there in a new file
>> drm_client_modeset.c with MIT license to match drm_fb_helper.c.
> 
> The first part of this commit log could use some re-pharsing.
> What is "It" etc.
> 

I could do this:

struct drm_fb_helper_crtc is now just a wrapper around drm_mode_set so
use that directly instead and attach it as a modeset array onto
drm_client_dev. drm_fb_helper will use this array to store its modesets
which means it will always initialize a drm_client, but it will not
register the client (callbacks) unless it's the generic fbdev emulation.

>> @@ -532,8 +535,7 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
>>  						    DRM_MODE_ROTATE_0);
>>  	}
>>  
>> -	for (i = 0; i < fb_helper->crtc_count; i++) {
>> -		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
>> +	drm_client_for_each_modeset(mode_set, client) {
>>  		struct drm_crtc *crtc = mode_set->crtc;
>>  
>>  		if (crtc->funcs->cursor_set2) {
> This function requires modeset_mutex to be held. Maybe add comment?
> 

drm_client_for_each_modeset() has a lockdep warn (courtesy of Daniel
Vetter):

#define drm_client_for_each_modeset(modeset, client) \
	for (({ lockdep_assert_held(&(client)->modeset_mutex); }), \
	     modeset = (client)->modesets; modeset->crtc; modeset++)

>> @@ -1842,7 +1805,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
>>  
>>  	pan_set(fb_helper, var->xoffset, var->yoffset);
>>  
>> -	ret = restore_fbdev_mode_atomic(fb_helper, true);
>> +	ret = restore_fbdev_mode_force(fb_helper);
> This change looks alien compared to other changes.
> Does it belong to this patchset?
> 

It's mentioned in the commit message:

In pan_display_atomic() restore_fbdev_mode_force() is used instead of
restore_fbdev_mode_atomic() because that one will later become internal
to drm_client_modeset.

Thanks for looking at this, I'll spin a new version.

Noralf.
Sam Ravnborg May 15, 2019, 3:01 p.m. UTC | #4
Hi Noralf.

On Wed, May 15, 2019 at 04:51:04PM +0200, Noralf Trønnes wrote:
> Hi Sam,
> 
> [looks like Thundebird decided to throw away my reply, so I'll try again]
I guess it awaits moderator approval as the from address was different
from what you otherwise used - I got both mails.

> 
> Den 15.05.2019 11.04, skrev Sam Ravnborg:
> > Hi Noralf.
> > 
> > I have read through the cahnes a copuple of times not and feel confident
> > to add my r-b if the comments are considered.
> > 
> > On Mon, May 06, 2019 at 08:01:33PM +0200, Noralf Trønnes wrote:
> >> It now only contains the modeset so use that directly instead and attach
> >> a modeset array to drm_client_dev. drm_fb_helper will use this array.
> >> Code will later be moved to drm_client, so add code there in a new file
> >> drm_client_modeset.c with MIT license to match drm_fb_helper.c.
> > 
> > The first part of this commit log could use some re-pharsing.
> > What is "It" etc.
> > 
> 
> I could do this:
> 
> struct drm_fb_helper_crtc is now just a wrapper around drm_mode_set so
> use that directly instead and attach it as a modeset array onto
> drm_client_dev. drm_fb_helper will use this array to store its modesets
> which means it will always initialize a drm_client, but it will not
> register the client (callbacks) unless it's the generic fbdev emulation.
This is much better - thanks.

> 
> >> @@ -532,8 +535,7 @@ static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
> >>  						    DRM_MODE_ROTATE_0);
> >>  	}
> >>  
> >> -	for (i = 0; i < fb_helper->crtc_count; i++) {
> >> -		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
> >> +	drm_client_for_each_modeset(mode_set, client) {
> >>  		struct drm_crtc *crtc = mode_set->crtc;
> >>  
> >>  		if (crtc->funcs->cursor_set2) {
> > This function requires modeset_mutex to be held. Maybe add comment?
> > 
> 
> drm_client_for_each_modeset() has a lockdep warn (courtesy of Daniel
> Vetter):
> 
> #define drm_client_for_each_modeset(modeset, client) \
> 	for (({ lockdep_assert_held(&(client)->modeset_mutex); }), \
> 	     modeset = (client)->modesets; modeset->crtc; modeset++)
> 
> >> @@ -1842,7 +1805,7 @@ static int pan_display_atomic(struct fb_var_screeninfo *var,
> >>  
> >>  	pan_set(fb_helper, var->xoffset, var->yoffset);
> >>  
> >> -	ret = restore_fbdev_mode_atomic(fb_helper, true);
> >> +	ret = restore_fbdev_mode_force(fb_helper);
> > This change looks alien compared to other changes.
> > Does it belong to this patchset?
> > 
> 
> It's mentioned in the commit message:
> 
> In pan_display_atomic() restore_fbdev_mode_force() is used instead of
> restore_fbdev_mode_atomic() because that one will later become internal
> to drm_client_modeset.
Right, I forgot about this detail from the commit log.

	Sam
diff mbox series

Patch

diff --git a/Documentation/gpu/drm-client.rst b/Documentation/gpu/drm-client.rst
index 7e672063e7eb..58b5a1d1219d 100644
--- a/Documentation/gpu/drm-client.rst
+++ b/Documentation/gpu/drm-client.rst
@@ -10,3 +10,6 @@  Kernel clients
 
 .. kernel-doc:: drivers/gpu/drm/drm_client.c
    :export:
+
+.. kernel-doc:: drivers/gpu/drm/drm_client_modeset.c
+   :export:
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 66f05f4e469f..9d4038c50013 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -289,6 +289,9 @@  drm_fb_helper tasks
   these igt tests need to be fixed: kms_fbcon_fbt@psr and
   kms_fbcon_fbt@psr-suspend.
 
+- The max connector argument for drm_fb_helper_init() and
+  drm_fb_helper_fbdev_setup() isn't used anymore and can be removed.
+
 Core refactorings
 =================
 
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 72f5036d9bfa..ae0e8adb2d73 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -17,7 +17,7 @@  drm-y       :=	drm_auth.o drm_cache.o \
 		drm_plane.o drm_color_mgmt.o drm_print.o \
 		drm_dumb_buffers.o drm_mode_config.o drm_vblank.o \
 		drm_syncobj.o drm_lease.o drm_writeback.o drm_client.o \
-		drm_atomic_uapi.o
+		drm_client_modeset.o drm_atomic_uapi.o
 
 drm-$(CONFIG_DRM_LEGACY) += drm_legacy_misc.o drm_bufs.o drm_context.o drm_dma.o drm_scatter.o drm_lock.o
 drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index f20d1dda3961..3dd08c6b264d 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -27,7 +27,6 @@ 
  * DOC: overview
  *
  * This library provides support for clients running in the kernel like fbdev and bootsplash.
- * Currently it's only partially implemented, just enough to support fbdev.
  *
  * GEM drivers which provide a GEM based dumb buffer with a virtual address are supported.
  */
@@ -92,14 +91,20 @@  int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
 	client->name = name;
 	client->funcs = funcs;
 
-	ret = drm_client_open(client);
+	ret = drm_client_modeset_create(client);
 	if (ret)
 		goto err_put_module;
 
+	ret = drm_client_open(client);
+	if (ret)
+		goto err_free;
+
 	drm_dev_get(dev);
 
 	return 0;
 
+err_free:
+	drm_client_modeset_free(client);
 err_put_module:
 	if (funcs)
 		module_put(funcs->owner);
@@ -148,6 +153,7 @@  void drm_client_release(struct drm_client_dev *client)
 
 	DRM_DEV_DEBUG_KMS(dev->dev, "%s\n", client->name);
 
+	drm_client_modeset_free(client);
 	drm_client_close(client);
 	drm_dev_put(dev);
 	if (client->funcs)
diff --git a/drivers/gpu/drm/drm_client_modeset.c b/drivers/gpu/drm/drm_client_modeset.c
new file mode 100644
index 000000000000..66770ed3299e
--- /dev/null
+++ b/drivers/gpu/drm/drm_client_modeset.c
@@ -0,0 +1,104 @@ 
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright 2018 Noralf Trønnes
+ * Copyright (c) 2006-2009 Red Hat Inc.
+ * Copyright (c) 2006-2008 Intel Corporation
+ *   Jesse Barnes <jesse.barnes@intel.com>
+ * Copyright (c) 2007 Dave Airlie <airlied@linux.ie>
+ */
+
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/slab.h>
+
+#include <drm/drm_client.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_device.h>
+
+int drm_client_modeset_create(struct drm_client_dev *client)
+{
+	struct drm_device *dev = client->dev;
+	unsigned int num_crtc = dev->mode_config.num_crtc;
+	unsigned int max_connector_count = 1;
+	struct drm_mode_set *modeset;
+	struct drm_crtc *crtc;
+	unsigned int i = 0;
+
+	/* Add terminating zero entry to enable index less iteration */
+	client->modesets = kcalloc(num_crtc + 1, sizeof(*client->modesets), GFP_KERNEL);
+	if (!client->modesets)
+		return -ENOMEM;
+
+	mutex_init(&client->modeset_mutex);
+
+	drm_for_each_crtc(crtc, dev)
+		client->modesets[i++].crtc = crtc;
+
+	/* Cloning is only supported in the single crtc case. */
+	if (num_crtc == 1)
+		max_connector_count = DRM_CLIENT_MAX_CLONED_CONNECTORS;
+
+	for (modeset = client->modesets; modeset->crtc; modeset++) {
+		modeset->connectors = kcalloc(max_connector_count,
+					      sizeof(*modeset->connectors), GFP_KERNEL);
+		if (!modeset->connectors)
+			goto err_free;
+	}
+
+	return 0;
+
+err_free:
+	drm_client_modeset_free(client);
+
+	return -ENOMEM;
+}
+
+void drm_client_modeset_release(struct drm_client_dev *client)
+{
+	struct drm_mode_set *modeset;
+	unsigned int i;
+
+	drm_client_for_each_modeset(modeset, client) {
+		drm_mode_destroy(client->dev, modeset->mode);
+		modeset->mode = NULL;
+		modeset->fb = NULL;
+
+		for (i = 0; i < modeset->num_connectors; i++) {
+			drm_connector_put(modeset->connectors[i]);
+			modeset->connectors[i] = NULL;
+		}
+		modeset->num_connectors = 0;
+	}
+}
+/* TODO: Remove export when modeset code has been moved over */
+EXPORT_SYMBOL(drm_client_modeset_release);
+
+void drm_client_modeset_free(struct drm_client_dev *client)
+{
+	struct drm_mode_set *modeset;
+
+	mutex_lock(&client->modeset_mutex);
+
+	drm_client_modeset_release(client);
+
+	drm_client_for_each_modeset(modeset, client)
+		kfree(modeset->connectors);
+
+	mutex_unlock(&client->modeset_mutex);
+
+	mutex_destroy(&client->modeset_mutex);
+	kfree(client->modesets);
+}
+
+struct drm_mode_set *drm_client_find_modeset(struct drm_client_dev *client, struct drm_crtc *crtc)
+{
+	struct drm_mode_set *modeset;
+
+	drm_client_for_each_modeset(modeset, client)
+		if (modeset->crtc == crtc)
+			return modeset;
+
+	return NULL;
+}
+/* TODO: Remove export when modeset code has been moved over */
+EXPORT_SYMBOL(drm_client_find_modeset);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index af2e74c742a4..6637858bf530 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -317,13 +317,11 @@  int drm_fb_helper_debug_enter(struct fb_info *info)
 {
 	struct drm_fb_helper *helper = info->par;
 	const struct drm_crtc_helper_funcs *funcs;
-	int i;
+	struct drm_mode_set *mode_set;
 
 	list_for_each_entry(helper, &kernel_fb_helper_list, kernel_fb_list) {
-		for (i = 0; i < helper->crtc_count; i++) {
-			struct drm_mode_set *mode_set =
-				&helper->crtc_info[i].mode_set;
-
+		mutex_lock(&helper->client.modeset_mutex);
+		drm_client_for_each_modeset(mode_set, &helper->client) {
 			if (!mode_set->crtc->enabled)
 				continue;
 
@@ -340,6 +338,7 @@  int drm_fb_helper_debug_enter(struct fb_info *info)
 						    mode_set->y,
 						    ENTER_ATOMIC_MODE_SET);
 		}
+		mutex_unlock(&helper->client.modeset_mutex);
 	}
 
 	return 0;
@@ -353,14 +352,14 @@  EXPORT_SYMBOL(drm_fb_helper_debug_enter);
 int drm_fb_helper_debug_leave(struct fb_info *info)
 {
 	struct drm_fb_helper *helper = info->par;
+	struct drm_client_dev *client = &helper->client;
 	struct drm_crtc *crtc;
 	const struct drm_crtc_helper_funcs *funcs;
+	struct drm_mode_set *mode_set;
 	struct drm_framebuffer *fb;
-	int i;
-
-	for (i = 0; i < helper->crtc_count; i++) {
-		struct drm_mode_set *mode_set = &helper->crtc_info[i].mode_set;
 
+	mutex_lock(&client->modeset_mutex);
+	drm_client_for_each_modeset(mode_set, client) {
 		crtc = mode_set->crtc;
 		if (drm_drv_uses_atomic_modeset(crtc->dev))
 			continue;
@@ -383,6 +382,7 @@  int drm_fb_helper_debug_leave(struct fb_info *info)
 		funcs->mode_set_base_atomic(mode_set->crtc, fb, crtc->x,
 					    crtc->y, LEAVE_ATOMIC_MODE_SET);
 	}
+	mutex_unlock(&client->modeset_mutex);
 
 	return 0;
 }
@@ -433,12 +433,14 @@  static bool drm_fb_helper_panel_rotation(struct drm_mode_set *modeset,
 
 static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool active)
 {
+	struct drm_client_dev *client = &fb_helper->client;
 	struct drm_device *dev = fb_helper->dev;
 	struct drm_plane_state *plane_state;
 	struct drm_plane *plane;
 	struct drm_atomic_state *state;
-	int i, ret;
 	struct drm_modeset_acquire_ctx ctx;
+	struct drm_mode_set *mode_set;
+	int ret;
 
 	drm_modeset_acquire_init(&ctx, 0);
 
@@ -468,8 +470,7 @@  static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ
 			goto out_state;
 	}
 
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
+	drm_client_for_each_modeset(mode_set, client) {
 		struct drm_plane *primary = mode_set->crtc->primary;
 		unsigned int rotation;
 
@@ -517,9 +518,11 @@  static int restore_fbdev_mode_atomic(struct drm_fb_helper *fb_helper, bool activ
 
 static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
 {
+	struct drm_client_dev *client = &fb_helper->client;
 	struct drm_device *dev = fb_helper->dev;
+	struct drm_mode_set *mode_set;
 	struct drm_plane *plane;
-	int i, ret = 0;
+	int ret = 0;
 
 	drm_modeset_lock_all(fb_helper->dev);
 	drm_for_each_plane(plane, dev) {
@@ -532,8 +535,7 @@  static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
 						    DRM_MODE_ROTATE_0);
 	}
 
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
+	drm_client_for_each_modeset(mode_set, client) {
 		struct drm_crtc *crtc = mode_set->crtc;
 
 		if (crtc->funcs->cursor_set2) {
@@ -559,11 +561,16 @@  static int restore_fbdev_mode_legacy(struct drm_fb_helper *fb_helper)
 static int restore_fbdev_mode_force(struct drm_fb_helper *fb_helper)
 {
 	struct drm_device *dev = fb_helper->dev;
+	int ret;
 
+	mutex_lock(&fb_helper->client.modeset_mutex);
 	if (drm_drv_uses_atomic_modeset(dev))
-		return restore_fbdev_mode_atomic(fb_helper, true);
+		ret = restore_fbdev_mode_atomic(fb_helper, true);
 	else
-		return restore_fbdev_mode_legacy(fb_helper);
+		ret = restore_fbdev_mode_legacy(fb_helper);
+	mutex_unlock(&fb_helper->client.modeset_mutex);
+
+	return ret;
 }
 
 static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
@@ -682,15 +689,14 @@  static struct sysrq_key_op sysrq_drm_fb_helper_restore_op = { };
 
 static void dpms_legacy(struct drm_fb_helper *fb_helper, int dpms_mode)
 {
+	struct drm_client_dev *client = &fb_helper->client;
 	struct drm_device *dev = fb_helper->dev;
 	struct drm_connector *connector;
 	struct drm_mode_set *modeset;
-	int i, j;
+	int j;
 
 	drm_modeset_lock_all(dev);
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		modeset = &fb_helper->crtc_info[i].mode_set;
-
+	drm_client_for_each_modeset(modeset, client) {
 		if (!modeset->crtc->enabled)
 			continue;
 
@@ -707,6 +713,7 @@  static void dpms_legacy(struct drm_fb_helper *fb_helper, int dpms_mode)
 static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 {
 	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_client_dev *client = &fb_helper->client;
 	struct drm_device *dev = fb_helper->dev;
 
 	/*
@@ -716,10 +723,12 @@  static void drm_fb_helper_dpms(struct fb_info *info, int dpms_mode)
 	if (!drm_master_internal_acquire(dev))
 		goto unlock;
 
+	mutex_lock(&client->modeset_mutex);
 	if (drm_drv_uses_atomic_modeset(dev))
 		restore_fbdev_mode_atomic(fb_helper, dpms_mode == DRM_MODE_DPMS_ON);
 	else
 		dpms_legacy(fb_helper, dpms_mode);
+	mutex_unlock(&client->modeset_mutex);
 
 	drm_master_internal_release(dev);
 unlock:
@@ -762,43 +771,6 @@  int drm_fb_helper_blank(int blank, struct fb_info *info)
 }
 EXPORT_SYMBOL(drm_fb_helper_blank);
 
-static void drm_fb_helper_modeset_release(struct drm_fb_helper *helper,
-					  struct drm_mode_set *modeset)
-{
-	int i;
-
-	for (i = 0; i < modeset->num_connectors; i++) {
-		drm_connector_put(modeset->connectors[i]);
-		modeset->connectors[i] = NULL;
-	}
-	modeset->num_connectors = 0;
-
-	drm_mode_destroy(helper->dev, modeset->mode);
-	modeset->mode = NULL;
-
-	/* FIXME should hold a ref? */
-	modeset->fb = NULL;
-}
-
-static void drm_fb_helper_crtc_free(struct drm_fb_helper *helper)
-{
-	int i;
-
-	for (i = 0; i < helper->connector_count; i++) {
-		drm_connector_put(helper->connector_info[i]->connector);
-		kfree(helper->connector_info[i]);
-	}
-	kfree(helper->connector_info);
-
-	for (i = 0; i < helper->crtc_count; i++) {
-		struct drm_mode_set *modeset = &helper->crtc_info[i].mode_set;
-
-		drm_fb_helper_modeset_release(helper, modeset);
-		kfree(modeset->connectors);
-	}
-	kfree(helper->crtc_info);
-}
-
 static void drm_fb_helper_resume_worker(struct work_struct *work)
 {
 	struct drm_fb_helper *helper = container_of(work, struct drm_fb_helper,
@@ -877,7 +849,7 @@  EXPORT_SYMBOL(drm_fb_helper_prepare);
  * drm_fb_helper_init - initialize a &struct drm_fb_helper
  * @dev: drm device
  * @fb_helper: driver-allocated fbdev helper structure to initialize
- * @max_conn_count: max connector count
+ * @max_conn_count: max connector count (not used)
  *
  * This allocates the structures for the fbdev helper with the given limits.
  * Note that this won't yet touch the hardware (through the driver interfaces)
@@ -893,53 +865,36 @@  int drm_fb_helper_init(struct drm_device *dev,
 		       struct drm_fb_helper *fb_helper,
 		       int max_conn_count)
 {
-	struct drm_crtc *crtc;
-	struct drm_mode_config *config = &dev->mode_config;
-	int i;
+	int ret;
 
 	if (!drm_fbdev_emulation) {
 		dev->fb_helper = fb_helper;
 		return 0;
 	}
 
-	if (!max_conn_count)
-		return -EINVAL;
-
-	fb_helper->crtc_info = kcalloc(config->num_crtc, sizeof(struct drm_fb_helper_crtc), GFP_KERNEL);
-	if (!fb_helper->crtc_info)
-		return -ENOMEM;
-
-	fb_helper->crtc_count = config->num_crtc;
-	fb_helper->connector_info = kcalloc(dev->mode_config.num_connector, sizeof(struct drm_fb_helper_connector *), GFP_KERNEL);
-	if (!fb_helper->connector_info) {
-		kfree(fb_helper->crtc_info);
-		return -ENOMEM;
+	/*
+	 * If this is not the generic fbdev client, initialize a drm_client
+	 * without callbacks so we can use the modesets.
+	 */
+	if (!fb_helper->client.funcs) {
+		ret = drm_client_init(dev, &fb_helper->client, "drm_fb_helper", NULL);
+		if (ret)
+			return ret;
 	}
+
+	fb_helper->connector_info = kcalloc(dev->mode_config.num_connector, sizeof(struct drm_fb_helper_connector *), GFP_KERNEL);
+	if (!fb_helper->connector_info)
+		goto out_free;
+
 	fb_helper->connector_info_alloc_count = dev->mode_config.num_connector;
 	fb_helper->connector_count = 0;
 
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		fb_helper->crtc_info[i].mode_set.connectors =
-			kcalloc(max_conn_count,
-				sizeof(struct drm_connector *),
-				GFP_KERNEL);
-
-		if (!fb_helper->crtc_info[i].mode_set.connectors)
-			goto out_free;
-		fb_helper->crtc_info[i].mode_set.num_connectors = 0;
-	}
-
-	i = 0;
-	drm_for_each_crtc(crtc, dev) {
-		fb_helper->crtc_info[i].mode_set.crtc = crtc;
-		i++;
-	}
-
 	dev->fb_helper = fb_helper;
 
 	return 0;
 out_free:
-	drm_fb_helper_crtc_free(fb_helper);
+	drm_client_release(&fb_helper->client);
+
 	return -ENOMEM;
 }
 EXPORT_SYMBOL(drm_fb_helper_init);
@@ -1015,6 +970,7 @@  EXPORT_SYMBOL(drm_fb_helper_unregister_fbi);
 void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
 {
 	struct fb_info *info;
+	int i;
 
 	if (!fb_helper)
 		return;
@@ -1044,8 +1000,15 @@  void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
 	mutex_unlock(&kernel_fb_helper_lock);
 
 	mutex_destroy(&fb_helper->lock);
-	drm_fb_helper_crtc_free(fb_helper);
 
+	if (!fb_helper->client.funcs)
+		drm_client_release(&fb_helper->client);
+
+	for (i = 0; i < fb_helper->connector_count; i++) {
+		drm_connector_put(fb_helper->connector_info[i]->connector);
+		kfree(fb_helper->connector_info[i]);
+	}
+	kfree(fb_helper->connector_info);
 }
 EXPORT_SYMBOL(drm_fb_helper_fini);
 
@@ -1390,13 +1353,14 @@  static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info)
 static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info)
 {
 	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_mode_set *modeset;
 	struct drm_crtc *crtc;
 	u16 *r, *g, *b;
-	int i, ret = 0;
+	int ret = 0;
 
 	drm_modeset_lock_all(fb_helper->dev);
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		crtc = fb_helper->crtc_info[i].mode_set.crtc;
+	drm_client_for_each_modeset(modeset, &fb_helper->client) {
+		crtc = modeset->crtc;
 		if (!crtc->funcs->gamma_set || !crtc->gamma_size)
 			return -EINVAL;
 
@@ -1472,10 +1436,11 @@  static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
 	struct drm_modeset_acquire_ctx ctx;
 	struct drm_crtc_state *crtc_state;
 	struct drm_atomic_state *state;
+	struct drm_mode_set *modeset;
 	struct drm_crtc *crtc;
 	u16 *r, *g, *b;
-	int i, ret = 0;
 	bool replaced;
+	int ret = 0;
 
 	drm_modeset_acquire_init(&ctx, 0);
 
@@ -1487,8 +1452,8 @@  static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
 
 	state->acquire_ctx = &ctx;
 retry:
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		crtc = fb_helper->crtc_info[i].mode_set.crtc;
+	drm_client_for_each_modeset(modeset, &fb_helper->client) {
+		crtc = modeset->crtc;
 
 		if (!gamma_lut)
 			gamma_lut = setcmap_new_gamma_lut(crtc, cmap);
@@ -1516,8 +1481,8 @@  static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
 	if (ret)
 		goto out_state;
 
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		crtc = fb_helper->crtc_info[i].mode_set.crtc;
+	drm_client_for_each_modeset(modeset, &fb_helper->client) {
+		crtc = modeset->crtc;
 
 		r = crtc->gamma_store;
 		g = r + crtc->gamma_size;
@@ -1567,12 +1532,14 @@  int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
 		goto unlock;
 	}
 
+	mutex_lock(&fb_helper->client.modeset_mutex);
 	if (info->fix.visual == FB_VISUAL_TRUECOLOR)
 		ret = setcmap_pseudo_palette(cmap, info);
 	else if (drm_drv_uses_atomic_modeset(fb_helper->dev))
 		ret = setcmap_atomic(cmap, info);
 	else
 		ret = setcmap_legacy(cmap, info);
+	mutex_unlock(&fb_helper->client.modeset_mutex);
 
 	drm_master_internal_release(dev);
 unlock:
@@ -1596,7 +1563,6 @@  int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 {
 	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_device *dev = fb_helper->dev;
-	struct drm_mode_set *mode_set;
 	struct drm_crtc *crtc;
 	int ret = 0;
 
@@ -1624,8 +1590,7 @@  int drm_fb_helper_ioctl(struct fb_info *info, unsigned int cmd,
 		 * make. If we're not smart enough here, one should
 		 * just consider switch the userspace to KMS.
 		 */
-		mode_set = &fb_helper->crtc_info[0].mode_set;
-		crtc = mode_set->crtc;
+		crtc = fb_helper->client.modesets[0].crtc;
 
 		/*
 		 * Only wait for a vblank event if the CRTC is
@@ -1822,16 +1787,14 @@  EXPORT_SYMBOL(drm_fb_helper_set_par);
 
 static void pan_set(struct drm_fb_helper *fb_helper, int x, int y)
 {
-	int i;
-
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		struct drm_mode_set *mode_set;
-
-		mode_set = &fb_helper->crtc_info[i].mode_set;
+	struct drm_mode_set *mode_set;
 
+	mutex_lock(&fb_helper->client.modeset_mutex);
+	drm_client_for_each_modeset(mode_set, &fb_helper->client) {
 		mode_set->x = x;
 		mode_set->y = y;
 	}
+	mutex_unlock(&fb_helper->client.modeset_mutex);
 }
 
 static int pan_display_atomic(struct fb_var_screeninfo *var,
@@ -1842,7 +1805,7 @@  static int pan_display_atomic(struct fb_var_screeninfo *var,
 
 	pan_set(fb_helper, var->xoffset, var->yoffset);
 
-	ret = restore_fbdev_mode_atomic(fb_helper, true);
+	ret = restore_fbdev_mode_force(fb_helper);
 	if (!ret) {
 		info->var.xoffset = var->xoffset;
 		info->var.yoffset = var->yoffset;
@@ -1856,14 +1819,13 @@  static int pan_display_legacy(struct fb_var_screeninfo *var,
 			      struct fb_info *info)
 {
 	struct drm_fb_helper *fb_helper = info->par;
+	struct drm_client_dev *client = &fb_helper->client;
 	struct drm_mode_set *modeset;
 	int ret = 0;
-	int i;
 
 	drm_modeset_lock_all(fb_helper->dev);
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		modeset = &fb_helper->crtc_info[i].mode_set;
-
+	mutex_lock(&client->modeset_mutex);
+	drm_client_for_each_modeset(modeset, client) {
 		modeset->x = var->xoffset;
 		modeset->y = var->yoffset;
 
@@ -1875,6 +1837,7 @@  static int pan_display_legacy(struct fb_var_screeninfo *var,
 			}
 		}
 	}
+	mutex_unlock(&client->modeset_mutex);
 	drm_modeset_unlock_all(fb_helper->dev);
 
 	return ret;
@@ -1921,10 +1884,12 @@  EXPORT_SYMBOL(drm_fb_helper_pan_display);
 static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 					 int preferred_bpp)
 {
+	struct drm_client_dev *client = &fb_helper->client;
 	int ret = 0;
 	int crtc_count = 0;
 	int i;
 	struct drm_fb_helper_surface_size sizes;
+	struct drm_mode_set *mode_set;
 	int best_depth = 0;
 
 	memset(&sizes, 0, sizeof(struct drm_fb_helper_surface_size));
@@ -1975,13 +1940,13 @@  static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 	 * supports RGBA5551 (16 bpp, depth 15) but not RGB565 (16 bpp, depth
 	 * 16) we need to scale down the depth of the sizes we request.
 	 */
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		struct drm_mode_set *mode_set = &fb_helper->crtc_info[i].mode_set;
+	mutex_lock(&client->modeset_mutex);
+	drm_client_for_each_modeset(mode_set, client) {
 		struct drm_crtc *crtc = mode_set->crtc;
 		struct drm_plane *plane = crtc->primary;
 		int j;
 
-		DRM_DEBUG("test CRTC %d primary plane\n", i);
+		DRM_DEBUG("test CRTC %u primary plane\n", drm_crtc_index(crtc));
 
 		for (j = 0; j < plane->format_count; j++) {
 			const struct drm_format_info *fmt;
@@ -2021,9 +1986,8 @@  static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 
 	/* first up get a count of crtcs now in use and new min/maxes width/heights */
 	crtc_count = 0;
-	for (i = 0; i < fb_helper->crtc_count; i++) {
+	drm_client_for_each_modeset(mode_set, client) {
 		struct drm_display_mode *desired_mode;
-		struct drm_mode_set *mode_set;
 		int x, y, j;
 		/* in case of tile group, are we the last tile vert or horiz?
 		 * If no tile group you are always the last one both vertically
@@ -2031,7 +1995,6 @@  static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 		 */
 		bool lastv = true, lasth = true;
 
-		mode_set = &fb_helper->crtc_info[i].mode_set;
 		desired_mode = mode_set->mode;
 
 		if (!desired_mode)
@@ -2061,6 +2024,7 @@  static int drm_fb_helper_single_fb_probe(struct drm_fb_helper *fb_helper,
 		if (lastv)
 			sizes.fb_height = min_t(u32, desired_mode->vdisplay + y, sizes.fb_height);
 	}
+	mutex_unlock(&client->modeset_mutex);
 
 	if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
 		DRM_INFO("Cannot find any crtc or sizes\n");
@@ -2292,7 +2256,7 @@  static bool drm_target_cloned(struct drm_fb_helper *fb_helper,
 	struct drm_display_mode *dmt_mode, *mode;
 
 	/* only contemplate cloning in the single crtc case */
-	if (fb_helper->crtc_count > 1)
+	if (fb_helper->dev->mode_config.num_crtc > 1)
 		return false;
 
 	count = 0;
@@ -2481,15 +2445,17 @@  static bool connector_has_possible_crtc(struct drm_connector *connector,
 }
 
 static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
-			  struct drm_fb_helper_crtc **best_crtcs,
+			  struct drm_crtc **best_crtcs,
 			  struct drm_display_mode **modes,
 			  int n, int width, int height)
 {
-	int c, o;
+	struct drm_client_dev *client = &fb_helper->client;
 	struct drm_connector *connector;
 	int my_score, best_score, score;
-	struct drm_fb_helper_crtc **crtcs, *crtc;
+	struct drm_crtc **crtcs, *crtc;
+	struct drm_mode_set *modeset;
 	struct drm_fb_helper_connector *fb_helper_conn;
+	int o;
 
 	if (n == fb_helper->connector_count)
 		return 0;
@@ -2502,8 +2468,7 @@  static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 	if (modes[n] == NULL)
 		return best_score;
 
-	crtcs = kcalloc(fb_helper->connector_count,
-			sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
+	crtcs = kcalloc(fb_helper->connector_count, sizeof(*crtcs), GFP_KERNEL);
 	if (!crtcs)
 		return best_score;
 
@@ -2519,11 +2484,10 @@  static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 	 * select a crtc for this connector and then attempt to configure
 	 * remaining connectors
 	 */
-	for (c = 0; c < fb_helper->crtc_count; c++) {
-		crtc = &fb_helper->crtc_info[c];
+	drm_client_for_each_modeset(modeset, client) {
+		crtc = modeset->crtc;
 
-		if (!connector_has_possible_crtc(connector,
-						 crtc->mode_set.crtc))
+		if (!connector_has_possible_crtc(connector, crtc))
 			continue;
 
 		for (o = 0; o < n; o++)
@@ -2532,7 +2496,7 @@  static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 
 		if (o < n) {
 			/* ignore cloning unless only a single crtc */
-			if (fb_helper->crtc_count > 1)
+			if (fb_helper->dev->mode_config.num_crtc > 1)
 				continue;
 
 			if (!drm_mode_equal(modes[o], modes[n]))
@@ -2540,14 +2504,13 @@  static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 		}
 
 		crtcs[n] = crtc;
-		memcpy(crtcs, best_crtcs, n * sizeof(struct drm_fb_helper_crtc *));
+		memcpy(crtcs, best_crtcs, n * sizeof(*crtcs));
 		score = my_score + drm_pick_crtcs(fb_helper, crtcs, modes, n + 1,
 						  width, height);
 		if (score > best_score) {
 			best_score = score;
 			memcpy(best_crtcs, crtcs,
-			       fb_helper->connector_count *
-			       sizeof(struct drm_fb_helper_crtc *));
+			       fb_helper->connector_count * sizeof(*crtcs));
 		}
 	}
 
@@ -2555,21 +2518,9 @@  static int drm_pick_crtcs(struct drm_fb_helper *fb_helper,
 	return best_score;
 }
 
-static struct drm_fb_helper_crtc *
-drm_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
-{
-	int i;
-
-	for (i = 0; i < fb_helper->crtc_count; i++)
-		if (fb_helper->crtc_info[i].mode_set.crtc == crtc)
-			return &fb_helper->crtc_info[i];
-
-	return NULL;
-}
-
 /* Try to read the BIOS display configuration and use it for the initial config */
 static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper,
-					  struct drm_fb_helper_crtc **crtcs,
+					  struct drm_crtc **crtcs,
 					  struct drm_display_mode **modes,
 					  struct drm_fb_offset *offsets,
 					  bool *enabled, int width, int height)
@@ -2605,7 +2556,7 @@  static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper,
 		struct drm_fb_helper_connector *fb_conn;
 		struct drm_connector *connector;
 		struct drm_encoder *encoder;
-		struct drm_fb_helper_crtc *new_crtc;
+		struct drm_crtc *new_crtc;
 
 		fb_conn = fb_helper->connector_info[i];
 		connector = fb_conn->connector;
@@ -2647,7 +2598,7 @@  static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper,
 
 		num_connectors_enabled++;
 
-		new_crtc = drm_fb_helper_crtc(fb_helper, connector->state->crtc);
+		new_crtc = connector->state->crtc;
 
 		/*
 		 * Make sure we're not trying to drive multiple connectors
@@ -2747,10 +2698,11 @@  static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper,
 static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 			    u32 width, u32 height)
 {
+	struct drm_client_dev *client = &fb_helper->client;
 	struct drm_device *dev = fb_helper->dev;
-	struct drm_fb_helper_crtc **crtcs;
 	struct drm_display_mode **modes;
 	struct drm_fb_offset *offsets;
+	struct drm_crtc **crtcs;
 	bool *enabled;
 	int i;
 
@@ -2758,8 +2710,7 @@  static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 	/* prevent concurrent modification of connector_count by hotplug */
 	lockdep_assert_held(&fb_helper->lock);
 
-	crtcs = kcalloc(fb_helper->connector_count,
-			sizeof(struct drm_fb_helper_crtc *), GFP_KERNEL);
+	crtcs = kcalloc(fb_helper->connector_count, sizeof(*crtcs), GFP_KERNEL);
 	modes = kcalloc(fb_helper->connector_count,
 			sizeof(struct drm_display_mode *), GFP_KERNEL);
 	offsets = kcalloc(fb_helper->connector_count,
@@ -2771,6 +2722,8 @@  static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 		goto out;
 	}
 
+	mutex_lock(&client->modeset_mutex);
+
 	mutex_lock(&fb_helper->dev->mode_config.mutex);
 	if (drm_fb_helper_probe_connector_modes(fb_helper, width, height) == 0)
 		DRM_DEBUG_KMS("No connectors reported connected with modes\n");
@@ -2795,24 +2748,24 @@  static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 	}
 	mutex_unlock(&fb_helper->dev->mode_config.mutex);
 
-	/* need to set the modesets up here for use later */
-	/* fill out the connector<->crtc mappings into the modesets */
-	for (i = 0; i < fb_helper->crtc_count; i++)
-		drm_fb_helper_modeset_release(fb_helper,
-					      &fb_helper->crtc_info[i].mode_set);
+	drm_client_modeset_release(client);
 
 	drm_fb_helper_for_each_connector(fb_helper, i) {
 		struct drm_display_mode *mode = modes[i];
-		struct drm_fb_helper_crtc *fb_crtc = crtcs[i];
+		struct drm_crtc *crtc = crtcs[i];
 		struct drm_fb_offset *offset = &offsets[i];
 
-		if (mode && fb_crtc) {
-			struct drm_mode_set *modeset = &fb_crtc->mode_set;
+		if (mode && crtc) {
+			struct drm_mode_set *modeset = drm_client_find_modeset(client, crtc);
 			struct drm_connector *connector =
 				fb_helper->connector_info[i]->connector;
 
 			DRM_DEBUG_KMS("desired mode %s set on crtc %d (%d,%d)\n",
-				      mode->name, fb_crtc->mode_set.crtc->base.id, offset->x, offset->y);
+				      mode->name, crtc->base.id, offset->x, offset->y);
+
+			if (WARN_ON_ONCE(modeset->num_connectors == DRM_CLIENT_MAX_CLONED_CONNECTORS ||
+					 (dev->mode_config.num_crtc > 1 && modeset->num_connectors == 1)))
+				break;
 
 			modeset->mode = drm_mode_duplicate(dev, mode);
 			drm_connector_get(connector);
@@ -2821,6 +2774,8 @@  static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
 			modeset->y = offset->y;
 		}
 	}
+
+	mutex_unlock(&client->modeset_mutex);
 out:
 	kfree(crtcs);
 	kfree(modes);
@@ -2837,13 +2792,14 @@  static void drm_setup_crtcs(struct drm_fb_helper *fb_helper,
  */
 static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
 {
+	struct drm_client_dev *client = &fb_helper->client;
 	struct fb_info *info = fb_helper->fbdev;
 	unsigned int rotation, sw_rotations = 0;
+	struct drm_mode_set *modeset;
 	int i;
 
-	for (i = 0; i < fb_helper->crtc_count; i++) {
-		struct drm_mode_set *modeset = &fb_helper->crtc_info[i].mode_set;
-
+	mutex_lock(&client->modeset_mutex);
+	drm_client_for_each_modeset(modeset, client) {
 		if (!modeset->num_connectors)
 			continue;
 
@@ -2855,6 +2811,7 @@  static void drm_setup_crtcs_fb(struct drm_fb_helper *fb_helper)
 		else
 			sw_rotations |= rotation;
 	}
+	mutex_unlock(&client->modeset_mutex);
 
 	mutex_lock(&fb_helper->dev->mode_config.mutex);
 	drm_fb_helper_for_each_connector(fb_helper, i) {
@@ -3071,8 +3028,7 @@  EXPORT_SYMBOL(drm_fb_helper_hotplug_event);
  * @funcs: fbdev helper functions
  * @preferred_bpp: Preferred bits per pixel for the device.
  *                 @dev->mode_config.preferred_depth is used if this is zero.
- * @max_conn_count: Maximum number of connectors.
- *                  @dev->mode_config.num_connector is used if this is zero.
+ * @max_conn_count: Maximum number of connectors (not used)
  *
  * This function sets up fbdev emulation and registers fbdev for access by
  * userspace. If all connectors are disconnected, setup is deferred to the next
@@ -3100,16 +3056,9 @@  int drm_fb_helper_fbdev_setup(struct drm_device *dev,
 	if (!preferred_bpp)
 		preferred_bpp = 32;
 
-	if (!max_conn_count)
-		max_conn_count = dev->mode_config.num_connector;
-	if (!max_conn_count) {
-		DRM_DEV_ERROR(dev->dev, "fbdev: No connectors\n");
-		return -EINVAL;
-	}
-
 	drm_fb_helper_prepare(dev, fb_helper, funcs);
 
-	ret = drm_fb_helper_init(dev, fb_helper, max_conn_count);
+	ret = drm_fb_helper_init(dev, fb_helper, 0);
 	if (ret < 0) {
 		DRM_DEV_ERROR(dev->dev, "fbdev: Failed to initialize (ret=%d)\n", ret);
 		return ret;
@@ -3422,7 +3371,7 @@  static int drm_fbdev_client_hotplug(struct drm_client_dev *client)
 
 	drm_fb_helper_prepare(dev, fb_helper, &drm_fb_helper_generic_funcs);
 
-	ret = drm_fb_helper_init(dev, fb_helper, dev->mode_config.num_connector);
+	ret = drm_fb_helper_init(dev, fb_helper, 0);
 	if (ret)
 		goto err;
 
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 268b2cf0052a..87be9aeb1fe0 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -3,8 +3,12 @@ 
 #ifndef _DRM_CLIENT_H_
 #define _DRM_CLIENT_H_
 
+#include <linux/lockdep.h>
+#include <linux/mutex.h>
 #include <linux/types.h>
 
+#include <drm/drm_crtc.h>
+
 struct drm_client_dev;
 struct drm_device;
 struct drm_file;
@@ -13,6 +17,8 @@  struct drm_gem_object;
 struct drm_minor;
 struct module;
 
+#define DRM_CLIENT_MAX_CLONED_CONNECTORS	8
+
 /**
  * struct drm_client_funcs - DRM client callbacks
  */
@@ -85,6 +91,16 @@  struct drm_client_dev {
 	 * @file: DRM file
 	 */
 	struct drm_file *file;
+
+	/**
+	 * @modeset_mutex: Protects @modesets.
+	 */
+	struct mutex modeset_mutex;
+
+	/**
+	 * @modesets: CRTC configurations
+	 */
+	struct drm_mode_set *modesets;
 };
 
 int drm_client_init(struct drm_device *dev, struct drm_client_dev *client,
@@ -135,6 +151,20 @@  struct drm_client_buffer *
 drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32 height, u32 format);
 void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
 
+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);
+
+/**
+ * drm_client_for_each_modeset() - Iterate over client modesets
+ * @modeset: &drm_mode_set loop cursor
+ * @client: DRM client
+ */
+#define drm_client_for_each_modeset(modeset, client) \
+	for (({ lockdep_assert_held(&(client)->modeset_mutex); }), \
+	     modeset = (client)->modesets; modeset->crtc; modeset++)
+
 int drm_client_debugfs_init(struct drm_minor *minor);
 
 #endif
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 2af1c6d3e147..6b334f4d8a22 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -47,10 +47,6 @@  struct drm_fb_offset {
 	int x, y;
 };
 
-struct drm_fb_helper_crtc {
-	struct drm_mode_set mode_set;
-};
-
 /**
  * struct drm_fb_helper_surface_size - describes fbdev size and scanout surface size
  * @fb_width: fbdev width
@@ -109,8 +105,6 @@  struct drm_fb_helper_connector {
  * struct drm_fb_helper - main structure to emulate fbdev on top of KMS
  * @fb: Scanout framebuffer object
  * @dev: DRM device
- * @crtc_count: number of possible CRTCs
- * @crtc_info: per-CRTC helper state (mode, x/y offset, etc)
  * @connector_count: number of connected connectors
  * @connector_info_alloc_count: size of connector_info
  * @funcs: driver callbacks for fb helper
@@ -144,8 +138,6 @@  struct drm_fb_helper {
 
 	struct drm_framebuffer *fb;
 	struct drm_device *dev;
-	int crtc_count;
-	struct drm_fb_helper_crtc *crtc_info;
 	int connector_count;
 	int connector_info_alloc_count;
 	/**