From 43cee314345ac9e34e744f8c335150a17ff9a6fb Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 21 Jun 2016 09:16:54 +0100 Subject: drm/i915/fbdev: Limit the global async-domain synchronization During cleanup we have to synchronise with the async task we are using to initialise and register our fbdev. Currently, we are using a full synchronisation on the global domain, but we can restrict this to just synchronising up to our task if we remember our cookie. Whilst there, streamline the function parameters. v2: async_synchronize_cookie() takes an exclusive upper bound, to synchronize with our task we have to pass in the next cookie. v3: Drop premature disregarding of the active cookie (we need to wait until the task is complete before continuing in the teardown). v4: Refactor waiting on async to incorporate a comment explaining why we need the +1. Signed-off-by: Chris Wilson Cc: Lukas Wunner Cc: Daniel Vetter Reviewed-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1466497015-8509-2-git-send-email-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_fbdev.c | 39 +++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_fbdev.c') diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 4babefc51eb2..45a03d87a659 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -538,8 +538,7 @@ static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { .fb_probe = intelfb_create, }; -static void intel_fbdev_destroy(struct drm_device *dev, - struct intel_fbdev *ifbdev) +static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) { /* We rely on the object-free to release the VMA pinning for * the info->screen_base mmaping. Leaking the VMA is simpler than @@ -552,12 +551,14 @@ static void intel_fbdev_destroy(struct drm_device *dev, drm_fb_helper_fini(&ifbdev->helper); if (ifbdev->fb) { - mutex_lock(&dev->struct_mutex); + mutex_lock(&ifbdev->helper.dev->struct_mutex); intel_unpin_fb_obj(&ifbdev->fb->base, BIT(DRM_ROTATE_0)); - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&ifbdev->helper.dev->struct_mutex); drm_framebuffer_remove(&ifbdev->fb->base); } + + kfree(ifbdev); } /* @@ -732,32 +733,44 @@ int intel_fbdev_init(struct drm_device *dev) static void intel_fbdev_initial_config(void *data, async_cookie_t cookie) { - struct drm_i915_private *dev_priv = data; - struct intel_fbdev *ifbdev = dev_priv->fbdev; + struct intel_fbdev *ifbdev = data; /* Due to peculiar init order wrt to hpd handling this is separate. */ if (drm_fb_helper_initial_config(&ifbdev->helper, ifbdev->preferred_bpp)) - intel_fbdev_fini(dev_priv->dev); + intel_fbdev_fini(ifbdev->helper.dev); } void intel_fbdev_initial_config_async(struct drm_device *dev) { - async_schedule(intel_fbdev_initial_config, to_i915(dev)); + struct intel_fbdev *ifbdev = to_i915(dev)->fbdev; + + ifbdev->cookie = async_schedule(intel_fbdev_initial_config, ifbdev); +} + +static void intel_fbdev_sync(struct intel_fbdev *ifbdev) +{ + if (!ifbdev->cookie) + return; + + /* Only serialises with all preceding async calls, hence +1 */ + async_synchronize_cookie(ifbdev->cookie + 1); + ifbdev->cookie = 0; } void intel_fbdev_fini(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - if (!dev_priv->fbdev) + struct intel_fbdev *ifbdev = dev_priv->fbdev; + + if (!ifbdev) return; flush_work(&dev_priv->fbdev_suspend_work); - if (!current_is_async()) - async_synchronize_full(); - intel_fbdev_destroy(dev, dev_priv->fbdev); - kfree(dev_priv->fbdev); + intel_fbdev_sync(ifbdev); + + intel_fbdev_destroy(ifbdev); dev_priv->fbdev = NULL; } -- cgit From e77018f7618960f7ec0e73e63868514ff16f8ddc Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 21 Jun 2016 09:16:55 +0100 Subject: drm/i915/fbdev: Flush mode configuration before lastclose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During lastclose, we call intel_fbdev_restore_mode() to switch back to the fbcon configuration on return to VT. However, if we have not yet finished the asynchronous fbdev initialisation, the current mode will be invalid and trigger WARNs upon application. Serialise with the outstanding initialisation if the first application exits quickly. Note that to hit this in practice requires using an unregistered async_domain as otherwise modprobe will force a full synchronisation prior to init() completing. v2: Reuse comment explaining the +1 by refactoring the wait on fbdev sync in the previous patch. Reported-by: Gustav Fägerlind Reported-by: "Li, Weinan Z" Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93580 Signed-off-by: Chris Wilson Cc: Daniel Vetter Reviewed-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1466497015-8509-3-git-send-email-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_fbdev.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/gpu/drm/i915/intel_fbdev.c') diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 45a03d87a659..16d73eaa46fb 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -840,6 +840,8 @@ void intel_fbdev_restore_mode(struct drm_device *dev) if (!ifbdev) return; + intel_fbdev_sync(ifbdev); + fb_helper = &ifbdev->helper; ret = drm_fb_helper_restore_fbdev_mode_unlocked(fb_helper); -- cgit From 86f40bb655ccde1d6ea2f1eaab7c1e62e9ee7742 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sat, 2 Jul 2016 15:36:02 +0100 Subject: drm/i915: Match bitmask size to types in intel_fb_initial_config() smatch complains of: drivers/gpu/drm/i915/intel_fbdev.c:403 intel_fb_initial_config() warn: should '1 << i' be a 64 bit type? drivers/gpu/drm/i915/intel_fbdev.c:422 intel_fb_initial_config() warn: should '1 << i' be a 64 bit type? drivers/gpu/drm/i915/intel_fbdev.c:501 intel_fb_initial_config() warn: should '1 << i' be a 64 bit type? We are prepared to iterate over a u64 but don't limit the number of connectors we try to configure to a maximum of 64. Signed-off-by: Chris Wilson Link: http://patchwork.freedesktop.org/patch/msgid/1467470166-31717-5-git-send-email-chris@chris-wilson.co.uk Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/intel_fbdev.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_fbdev.c') diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 16d73eaa46fb..224180e402be 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -362,23 +362,24 @@ static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, bool *enabled, int width, int height) { struct drm_device *dev = fb_helper->dev; + unsigned long conn_configured, mask; + unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG); int i, j; bool *save_enabled; bool fallback = true; int num_connectors_enabled = 0; int num_connectors_detected = 0; - uint64_t conn_configured = 0, mask; int pass = 0; - save_enabled = kcalloc(fb_helper->connector_count, sizeof(bool), - GFP_KERNEL); + save_enabled = kcalloc(count, sizeof(bool), GFP_KERNEL); if (!save_enabled) return false; - memcpy(save_enabled, enabled, fb_helper->connector_count); - mask = (1 << fb_helper->connector_count) - 1; + memcpy(save_enabled, enabled, count); + mask = BIT(count) - 1; + conn_configured = 0; retry: - for (i = 0; i < fb_helper->connector_count; i++) { + for (i = 0; i < count; i++) { struct drm_fb_helper_connector *fb_conn; struct drm_connector *connector; struct drm_encoder *encoder; @@ -388,7 +389,7 @@ retry: fb_conn = fb_helper->connector_info[i]; connector = fb_conn->connector; - if (conn_configured & (1 << i)) + if (conn_configured & BIT(i)) continue; if (pass == 0 && !connector->has_tile) @@ -400,7 +401,7 @@ retry: if (!enabled[i]) { DRM_DEBUG_KMS("connector %s not enabled, skipping\n", connector->name); - conn_configured |= (1 << i); + conn_configured |= BIT(i); continue; } @@ -419,7 +420,7 @@ retry: DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n", connector->name); enabled[i] = false; - conn_configured |= (1 << i); + conn_configured |= BIT(i); continue; } @@ -432,14 +433,15 @@ retry: intel_crtc->lut_b[j] = j; } - new_crtc = intel_fb_helper_crtc(fb_helper, connector->state->crtc); + new_crtc = intel_fb_helper_crtc(fb_helper, + connector->state->crtc); /* * Make sure we're not trying to drive multiple connectors * with a single CRTC, since our cloning support may not * match the BIOS. */ - for (j = 0; j < fb_helper->connector_count; j++) { + for (j = 0; j < count; j++) { if (crtcs[j] == new_crtc) { DRM_DEBUG_KMS("fallback: cloned configuration\n"); goto bail; @@ -498,7 +500,7 @@ retry: modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" :""); fallback = false; - conn_configured |= (1 << i); + conn_configured |= BIT(i); } if ((conn_configured & mask) != mask) { @@ -522,7 +524,7 @@ retry: if (fallback) { bail: DRM_DEBUG_KMS("Not using firmware configuration\n"); - memcpy(enabled, save_enabled, fb_helper->connector_count); + memcpy(enabled, save_enabled, count); kfree(save_enabled); return false; } -- cgit From fac5e23e3c385fde41aab4a23bc50c8c15ad4d00 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 4 Jul 2016 11:34:36 +0100 Subject: drm/i915: Mass convert dev->dev_private to to_i915(dev) Since we now subclass struct drm_device, we can save pointer dances by noting the equivalence of struct drm_device and struct drm_i915_private, i.e. by using to_i915(). text data bss dec hex filename 1073824 4562 416 1078802 107612 drivers/gpu/drm/i915/i915.ko 1068976 4562 416 1073954 106322 drivers/gpu/drm/i915/i915.ko Created by the coccinelle script: @@ expression E; identifier p; @@ - struct drm_i915_private *p = E->dev_private; + struct drm_i915_private *p = to_i915(E); Signed-off-by: Chris Wilson Reviewed-by: Dave Gordon Link: http://patchwork.freedesktop.org/patch/msgid/1467628477-25379-1-git-send-email-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_fbdev.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_fbdev.c') diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 224180e402be..5205afd69ff6 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -703,7 +703,7 @@ static void intel_fbdev_suspend_worker(struct work_struct *work) int intel_fbdev_init(struct drm_device *dev) { struct intel_fbdev *ifbdev; - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = to_i915(dev); int ret; if (WARN_ON(INTEL_INFO(dev)->num_pipes == 0)) @@ -762,7 +762,7 @@ static void intel_fbdev_sync(struct intel_fbdev *ifbdev) void intel_fbdev_fini(struct drm_device *dev) { - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = to_i915(dev); struct intel_fbdev *ifbdev = dev_priv->fbdev; if (!ifbdev) @@ -778,7 +778,7 @@ void intel_fbdev_fini(struct drm_device *dev) void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous) { - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = to_i915(dev); struct intel_fbdev *ifbdev = dev_priv->fbdev; struct fb_info *info; @@ -827,7 +827,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous void intel_fbdev_output_poll_changed(struct drm_device *dev) { - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = to_i915(dev); if (dev_priv->fbdev) drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper); } @@ -835,7 +835,7 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev) void intel_fbdev_restore_mode(struct drm_device *dev) { int ret; - struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = to_i915(dev); struct intel_fbdev *ifbdev = dev_priv->fbdev; struct drm_fb_helper *fb_helper; -- cgit From 91c8a326a192117219d5b9b980244c3662e35404 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 5 Jul 2016 10:40:23 +0100 Subject: drm/i915: Convert dev_priv->dev backpointers to dev_priv->drm Since drm_i915_private is now a subclass of drm_device we do not need to chase the drm_i915_private->dev backpointer and can instead simply access drm_i915_private->drm directly. text data bss dec hex filename 1068757 4565 416 1073738 10624a drivers/gpu/drm/i915/i915.ko 1066949 4565 416 1071930 105b3a drivers/gpu/drm/i915/i915.ko Created by the coccinelle script: @@ struct drm_i915_private *d; identifier i; @@ ( - d->dev->i + d->drm.i | - d->dev + &d->drm ) and for good measure the dev_priv->dev backpointer was removed entirely. Signed-off-by: Chris Wilson Reviewed-by: Matthew Auld Link: http://patchwork.freedesktop.org/patch/msgid/1467711623-2905-4-git-send-email-chris@chris-wilson.co.uk --- drivers/gpu/drm/i915/intel_fbdev.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_fbdev.c') diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 5205afd69ff6..86b00c6db1a6 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -693,9 +693,9 @@ out: static void intel_fbdev_suspend_worker(struct work_struct *work) { - intel_fbdev_set_suspend(container_of(work, - struct drm_i915_private, - fbdev_suspend_work)->dev, + intel_fbdev_set_suspend(&container_of(work, + struct drm_i915_private, + fbdev_suspend_work)->drm, FBINFO_STATE_RUNNING, true); } -- cgit