From 7118fd9bd975a9f3093239d4c0f4e15356b57fab Mon Sep 17 00:00:00 2001 From: Matt Roper Date: Fri, 18 Dec 2015 17:27:01 -0800 Subject: drm/fb-helper: Use proper plane mask for fb cleanup pan_display_atomic() calls drm_atomic_clean_old_fb() to sanitize the legacy FB fields (plane->fb and plane->old_fb). However it was building the plane mask to pass to this function incorrectly (the bitwise OR was using plane indices rather than plane masks). The end result was that sometimes the legacy pointers would become out of sync with the atomic pointers. If another operation tried to re-set the same FB onto the plane, we might end up with the pointers back in sync, but improper reference counts, which would eventually lead to system crashes when we accessed a pointer to a prematurely-destroyed FB. The cause here was a very subtle bug introduced in commit: commit 07d3bad6c1210bd21e85d084807ef4ee4ac43a78 Author: Maarten Lankhorst Date: Wed Nov 11 11:29:11 2015 +0100 drm/core: Fix old_fb handling in pan_display_atomic. I found the crashes were most easily reproduced (on i915 at least) by starting X and then VT switching to a VT that wasn't running a console instance...the sequence of vt/fbcon entries that happen in that case trigger a reference count mismatch and crash the system. Cc: Maarten Lankhorst Cc: Daniel Vetter Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93313 Signed-off-by: Matt Roper Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 69cbab5..1e103c4 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1251,7 +1251,7 @@ retry: goto fail; plane = mode_set->crtc->primary; - plane_mask |= drm_plane_index(plane); + plane_mask |= (1 << drm_plane_index(plane)); plane->old_fb = plane->fb; } -- cgit v0.10.2 From d21b02af63fd90d5fd6b8042fa5fb25f5911128d Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Sun, 27 Dec 2015 18:45:58 +0800 Subject: drm/gma500: use to_pci_dev() Use to_pci_dev() instead of open-coding it. Signed-off-by: Geliang Tang Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/gma500/power.c b/drivers/gpu/drm/gma500/power.c index b6b135f..bea8578 100644 --- a/drivers/gpu/drm/gma500/power.c +++ b/drivers/gpu/drm/gma500/power.c @@ -187,7 +187,7 @@ static bool gma_resume_pci(struct pci_dev *pdev) */ int gma_power_suspend(struct device *_dev) { - struct pci_dev *pdev = container_of(_dev, struct pci_dev, dev); + struct pci_dev *pdev = to_pci_dev(_dev); struct drm_device *dev = pci_get_drvdata(pdev); struct drm_psb_private *dev_priv = dev->dev_private; @@ -214,7 +214,7 @@ int gma_power_suspend(struct device *_dev) */ int gma_power_resume(struct device *_dev) { - struct pci_dev *pdev = container_of(_dev, struct pci_dev, dev); + struct pci_dev *pdev = to_pci_dev(_dev); struct drm_device *dev = pci_get_drvdata(pdev); mutex_lock(&power_mutex); -- cgit v0.10.2 From 69a0f89c0641668d402573a05b327ac8ed6d2560 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Wed, 30 Dec 2015 22:20:30 +0100 Subject: drm/dp/mst: constify drm_dp_mst_topology_cbs structures The drm_dp_mst_topology_cbs structures are never modified, so declare them as const. Done with the help of Coccinelle. Signed-off-by: Julia Lawall Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index e8d369d..9ae1a4f 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -512,7 +512,7 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr) drm_kms_helper_hotplug_event(dev); } -static struct drm_dp_mst_topology_cbs mst_cbs = { +static const struct drm_dp_mst_topology_cbs mst_cbs = { .add_connector = intel_dp_add_mst_connector, .register_connector = intel_dp_register_mst_connector, .destroy_connector = intel_dp_destroy_mst_connector, diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c index 94323f5..8a02225 100644 --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c @@ -329,7 +329,7 @@ static void radeon_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr) drm_kms_helper_hotplug_event(dev); } -struct drm_dp_mst_topology_cbs mst_cbs = { +const struct drm_dp_mst_topology_cbs mst_cbs = { .add_connector = radeon_dp_add_mst_connector, .register_connector = radeon_dp_register_mst_connector, .destroy_connector = radeon_dp_destroy_mst_connector, diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 5340099..1666371 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -420,7 +420,7 @@ struct drm_dp_payload { struct drm_dp_mst_topology_mgr { struct device *dev; - struct drm_dp_mst_topology_cbs *cbs; + const struct drm_dp_mst_topology_cbs *cbs; int max_dpcd_transaction_bytes; struct drm_dp_aux *aux; /* auxch for this topology mgr to use */ int max_payloads; -- cgit v0.10.2 From 3a848662c75118e1ee4653e08e25943d8b4ab8b7 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Sat, 2 Jan 2016 18:47:17 +0100 Subject: vga_switcheroo: Prettify documentation Fix indentation of vga_switcheroo sections in gpu.tmpl. Change section type of API documentation from "chapter" to "sect1" so that the individual functions no longer clutter up the ToC. Group together under a new "API" chapter. Fix wording "heretoforth" -> "henceforth". Signed-off-by: Lukas Wunner Signed-off-by: Daniel Vetter diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index 6c6e81a..225a246 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -3625,37 +3625,37 @@ int num_ioctls; Modes of Use - - Manual switching and manual power control + + Manual switching and manual power control !Pdrivers/gpu/vga/vga_switcheroo.c Manual switching and manual power control - - - Driver power control + + + Driver power control !Pdrivers/gpu/vga/vga_switcheroo.c Driver power control - + - - Public functions + + API + + Public functions !Edrivers/gpu/vga/vga_switcheroo.c - - - - Public structures + + + Public structures !Finclude/linux/vga_switcheroo.h vga_switcheroo_handler !Finclude/linux/vga_switcheroo.h vga_switcheroo_client_ops - - - - Public constants + + + Public constants !Finclude/linux/vga_switcheroo.h vga_switcheroo_client_id !Finclude/linux/vga_switcheroo.h vga_switcheroo_state - - - - Private structures + + + Private structures !Fdrivers/gpu/vga/vga_switcheroo.c vgasr_priv !Fdrivers/gpu/vga/vga_switcheroo.c vga_switcheroo_client + !Cdrivers/gpu/vga/vga_switcheroo.c diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c index 41edd5a..d64d905 100644 --- a/drivers/gpu/vga/vga_switcheroo.c +++ b/drivers/gpu/vga/vga_switcheroo.c @@ -63,7 +63,7 @@ * for the inactive GPU.) Also, muxes are often used to cut power to the * discrete GPU while it is not used. * - * DRM drivers register GPUs with vga_switcheroo, these are heretoforth called + * DRM drivers register GPUs with vga_switcheroo, these are henceforth called * clients. The mux is called the handler. Muxless machines also register a * handler to control the power state of the discrete GPU, its ->switchto * callback is a no-op for obvious reasons. The discrete GPU is often equipped -- cgit v0.10.2 From 6984128d01cf935820a0563f3a00c6623ba58109 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 4 Jan 2016 10:10:59 +0000 Subject: drm: Balance error path for GEM handle allocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current error path for failure when establishing a handle for a GEM object is unbalance, e.g. we call object_close() without calling first object_open(). Use the typical onion structure to only undo what has been set up prior to the error. Signed-off-by: Chris Wilson Reviewed-by: Ville Syrjälä Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 2e10bba..a08176d 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -343,27 +343,32 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, spin_unlock(&file_priv->table_lock); idr_preload_end(); mutex_unlock(&dev->object_name_lock); - if (ret < 0) { - drm_gem_object_handle_unreference_unlocked(obj); - return ret; - } + if (ret < 0) + goto err_unref; + *handlep = ret; ret = drm_vma_node_allow(&obj->vma_node, file_priv->filp); - if (ret) { - drm_gem_handle_delete(file_priv, *handlep); - return ret; - } + if (ret) + goto err_remove; if (dev->driver->gem_open_object) { ret = dev->driver->gem_open_object(obj, file_priv); - if (ret) { - drm_gem_handle_delete(file_priv, *handlep); - return ret; - } + if (ret) + goto err_revoke; } return 0; + +err_revoke: + drm_vma_node_revoke(&obj->vma_node, file_priv->filp); +err_remove: + spin_lock(&file_priv->table_lock); + idr_remove(&file_priv->object_idr, *handlep); + spin_unlock(&file_priv->table_lock); +err_unref: + drm_gem_object_handle_unreference_unlocked(obj); + return ret; } /** -- cgit v0.10.2 From 98a8883ad419128493b8cfab3982d58785deabda Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 4 Jan 2016 10:11:00 +0000 Subject: drm: Only bump object-reference count when adding first handle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We only need a single reference count for all handles (i.e. non-zero obj->handle_count) and so can trim a few atomic operations by only taking the reference on the first handle and dropping it after the last. Signed-off-by: Chris Wilson Reviewed-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1451902261-25380-2-git-send-email-chris@chris-wilson.co.uk Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index a08176d..ad955d7 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -220,6 +220,9 @@ static void drm_gem_object_exported_dma_buf_free(struct drm_gem_object *obj) static void drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj) { + struct drm_device *dev = obj->dev; + bool final = false; + if (WARN_ON(obj->handle_count == 0)) return; @@ -229,14 +232,16 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj) * checked for a name */ - mutex_lock(&obj->dev->object_name_lock); + mutex_lock(&dev->object_name_lock); if (--obj->handle_count == 0) { drm_gem_object_handle_free(obj); drm_gem_object_exported_dma_buf_free(obj); + final = true; } - mutex_unlock(&obj->dev->object_name_lock); + mutex_unlock(&dev->object_name_lock); - drm_gem_object_unreference_unlocked(obj); + if (final) + drm_gem_object_unreference_unlocked(obj); } /** @@ -329,6 +334,8 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, int ret; WARN_ON(!mutex_is_locked(&dev->object_name_lock)); + if (obj->handle_count++ == 0) + drm_gem_object_reference(obj); /* * Get the user-visible handle using idr. Preload and perform @@ -338,10 +345,10 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, spin_lock(&file_priv->table_lock); ret = idr_alloc(&file_priv->object_idr, obj, 1, 0, GFP_NOWAIT); - drm_gem_object_reference(obj); - obj->handle_count++; + spin_unlock(&file_priv->table_lock); idr_preload_end(); + mutex_unlock(&dev->object_name_lock); if (ret < 0) goto err_unref; -- cgit v0.10.2 From 0f646425b94a01606450eb03b3a8f3b5e03ea9b8 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 4 Jan 2016 10:11:01 +0000 Subject: drm: Use a normal idr allocation for the obj->name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unlike the handle, the name table uses a sleeping mutex rather than a spinlock. The allocation is in a normal context, and we can use the simpler sleeping gfp_t, rather than have to take from the atomic reserves. Signed-off-by: Chris Wilson Reviewed-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1451902261-25380-3-git-send-email-chris@chris-wilson.co.uk Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index ad955d7..1b0c2c1 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -642,7 +642,6 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, return -ENOENT; mutex_lock(&dev->object_name_lock); - idr_preload(GFP_KERNEL); /* prevent races with concurrent gem_close. */ if (obj->handle_count == 0) { ret = -ENOENT; @@ -650,7 +649,7 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, } if (!obj->name) { - ret = idr_alloc(&dev->object_name_idr, obj, 1, 0, GFP_NOWAIT); + ret = idr_alloc(&dev->object_name_idr, obj, 1, 0, GFP_KERNEL); if (ret < 0) goto err; @@ -661,7 +660,6 @@ drm_gem_flink_ioctl(struct drm_device *dev, void *data, ret = 0; err: - idr_preload_end(); mutex_unlock(&dev->object_name_lock); drm_gem_object_unreference_unlocked(obj); return ret; -- cgit v0.10.2 From 5350a031247680590724a1badc112d1217f721d6 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 4 Jan 2016 12:53:15 +0100 Subject: drm/i915: Set connector_state->connector using the helper. The atomic helper sets connector_state->connector, which the i915 code didn't. This will become a problem when we start using it. Signed-off-by: Maarten Lankhorst Acked-by: Thierry Reding Link: http://patchwork.freedesktop.org/patch/msgid/1451908400-25147-1-git-send-email-maarten.lankhorst@linux.intel.com Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bda6b9c..e8f76be 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -6475,13 +6475,11 @@ static void intel_connector_check_state(struct intel_connector *connector) int intel_connector_init(struct intel_connector *connector) { - struct drm_connector_state *connector_state; + drm_atomic_helper_connector_reset(&connector->base); - connector_state = kzalloc(sizeof *connector_state, GFP_KERNEL); - if (!connector_state) + if (!connector->base.state) return -ENOMEM; - connector->base.state = connector_state; return 0; } -- cgit v0.10.2 From 4cd39917ddb2fb5691e05b13b13f1f2398343b3e Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 4 Jan 2016 12:53:16 +0100 Subject: drm/atomic: Add __drm_atomic_helper_connector_reset, v2. This is useful for drivers that subclass connector_state, like tegra. Changes since v1: - Docbook updates. Signed-off-by: Maarten Lankhorst Link: http://patchwork.freedesktop.org/patch/msgid/1451908400-25147-2-git-send-email-maarten.lankhorst@linux.intel.com Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 268d37f..26d258d 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2606,6 +2606,28 @@ void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane, EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state); /** + * __drm_atomic_helper_connector_reset - reset state on connector + * @connector: drm connector + * @conn_state: connector state to assign + * + * Initializes the newly allocated @conn_state and assigns it to + * #connector ->state, usually required when initializing the drivers + * or when called from the ->reset hook. + * + * This is useful for drivers that subclass the connector state. + */ +void +__drm_atomic_helper_connector_reset(struct drm_connector *connector, + struct drm_connector_state *conn_state) +{ + if (conn_state) + conn_state->connector = connector; + + connector->state = conn_state; +} +EXPORT_SYMBOL(__drm_atomic_helper_connector_reset); + +/** * drm_atomic_helper_connector_reset - default ->reset hook for connectors * @connector: drm connector * @@ -2615,11 +2637,11 @@ EXPORT_SYMBOL(drm_atomic_helper_plane_destroy_state); */ void drm_atomic_helper_connector_reset(struct drm_connector *connector) { - kfree(connector->state); - connector->state = kzalloc(sizeof(*connector->state), GFP_KERNEL); + struct drm_connector_state *conn_state = + kzalloc(sizeof(*conn_state), GFP_KERNEL); - if (connector->state) - connector->state->connector = connector; + kfree(connector->state); + __drm_atomic_helper_connector_reset(connector, conn_state); } EXPORT_SYMBOL(drm_atomic_helper_connector_reset); diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h index a286cce..89d008d 100644 --- a/include/drm/drm_atomic_helper.h +++ b/include/drm/drm_atomic_helper.h @@ -126,6 +126,8 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane *plane, void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane, struct drm_plane_state *state); +void __drm_atomic_helper_connector_reset(struct drm_connector *connector, + struct drm_connector_state *conn_state); void drm_atomic_helper_connector_reset(struct drm_connector *connector); void __drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector, -- cgit v0.10.2 From 5459a2ad9da0c93161f31cfda5d39e23b64d50f8 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 4 Jan 2016 12:53:17 +0100 Subject: drm/tegra: Use __drm_atomic_helper_reset_connector for subclassing connector state, v2. Changes since v1: - Do not reset if state allocation fails. Signed-off-by: Maarten Lankhorst Acked-by: Thierry Reding #irc Link: http://patchwork.freedesktop.org/patch/msgid/1451908400-25147-3-git-send-email-maarten.lankhorst@linux.intel.com Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c index 50d46ae..44e1027 100644 --- a/drivers/gpu/drm/tegra/dsi.c +++ b/drivers/gpu/drm/tegra/dsi.c @@ -745,14 +745,13 @@ static void tegra_dsi_soft_reset(struct tegra_dsi *dsi) static void tegra_dsi_connector_reset(struct drm_connector *connector) { - struct tegra_dsi_state *state; - - kfree(connector->state); - connector->state = NULL; + struct tegra_dsi_state *state = + kzalloc(sizeof(*state), GFP_KERNEL); - state = kzalloc(sizeof(*state), GFP_KERNEL); - if (state) - connector->state = &state->base; + if (state) { + kfree(connector->state); + __drm_atomic_helper_connector_reset(connector, &state->base); + } } static struct drm_connector_state * -- cgit v0.10.2 From 4cd9fa529d77dde8f760adb3d934dfac6e169b1e Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 4 Jan 2016 12:53:18 +0100 Subject: drm/atomic: add connector mask to drm_crtc_state. It can be useful to iterate over connectors without grabbing connection_mutex. It can also be used to see how many connectors are on a crtc without iterating over the list. Signed-off-by: Maarten Lankhorst Link: http://patchwork.freedesktop.org/patch/msgid/1451908400-25147-4-git-send-email-maarten.lankhorst@linux.intel.com Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 6a21e5c..14b3215 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1063,10 +1063,21 @@ drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state, { struct drm_crtc_state *crtc_state; + if (conn_state->crtc && conn_state->crtc != crtc) { + crtc_state = drm_atomic_get_existing_crtc_state(conn_state->state, + conn_state->crtc); + + crtc_state->connector_mask &= + ~(1 << drm_connector_index(conn_state->connector)); + } + if (crtc) { crtc_state = drm_atomic_get_crtc_state(conn_state->state, crtc); if (IS_ERR(crtc_state)) return PTR_ERR(crtc_state); + + crtc_state->connector_mask |= + 1 << drm_connector_index(conn_state->connector); } conn_state->crtc = crtc; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 3b040b3..e3c4d48 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -306,6 +306,7 @@ struct drm_plane_helper_funcs; * @active_changed: crtc_state->active has been toggled. * @connectors_changed: connectors to this crtc have been updated * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes + * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors * @last_vblank_count: for helpers and drivers to capture the vblank of the * update to ensure framebuffer cleanup isn't done too early * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings @@ -339,6 +340,8 @@ struct drm_crtc_state { */ u32 plane_mask; + u32 connector_mask; + /* last_vblank_count: for vblank waits before cleanup */ u32 last_vblank_count; -- cgit v0.10.2 From 4cba68507cf58db99752cf79198beb4a85a9f8ce Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 8 Dec 2015 09:49:20 +0100 Subject: drm/atomic-helper: Reject legacy flips on a disabled pipe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We want this for consistency with existing page_flip semantics. Since this spurred quite a discussion on IRC also document why we reject event generation when the pipe is off: It's not that it's hard to implement, but userspace has a track recording which proves that it's way too easy to accidentally abuse and cause havoc. We want to make sure userspace doesn't get away with that. v2: Somehow thought we do reject events already, but that code only existed in my imagination ... Also suggestions from Thierry. Cc: Daniel Stone Cc: Ville Syrjälä Cc: Thierry Reding Reviewed-by: Daniel Stone Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1449564561-3896-4-git-send-email-daniel.vetter@ffwll.ch diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 14b3215..6050c80f 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -508,6 +508,22 @@ static int drm_atomic_crtc_check(struct drm_crtc *crtc, return -EINVAL; } + /* + * Reject event generation for when a CRTC is off and stays off. + * It wouldn't be hard to implement this, but userspace has a track + * record of happily burning through 100% cpu (or worse, crash) when the + * display pipe is suspended. To avoid all that fun just reject updates + * that ask for events since likely that indicates a bug in the + * compositor's drawing loop. This is consistent with the vblank IOCTL + * and legacy page_flip IOCTL which also reject service on a disabled + * pipe. + */ + if (state->event && !state->active && !crtc->state->active) { + DRM_DEBUG_ATOMIC("[CRTC:%d] requesting event but off\n", + crtc->base.id); + return -EINVAL; + } + return 0; } diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 26d258d..738104b 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2284,6 +2284,15 @@ retry: goto fail; drm_atomic_set_fb_for_plane(plane_state, fb); + /* Make sure we don't accidentally do a full modeset. */ + state->allow_modeset = false; + if (!crtc_state->active) { + DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n", + crtc->base.id); + ret = -EINVAL; + goto fail; + } + ret = drm_atomic_async_commit(state); if (ret != 0) goto fail; diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index e3c4d48..c65a212 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -551,7 +551,8 @@ struct drm_crtc_funcs { * ->page_flip() operation is already pending the callback should return * -EBUSY. Pageflips on a disabled CRTC (either by setting a NULL mode * or just runtime disabled through DPMS respectively the new atomic - * "ACTIVE" state) should result in an -EINVAL error code. + * "ACTIVE" state) should result in an -EINVAL error code. Note that + * drm_atomic_helper_page_flip() checks this already for atomic drivers. */ int (*page_flip)(struct drm_crtc *crtc, struct drm_framebuffer *fb, -- cgit v0.10.2 From df7d678bea8ba8904bdb293c8e96aa9488f7dbee Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Mon, 4 Jan 2016 07:53:36 +0100 Subject: drm/docs: more leftovers from the big vtable documentation pile Another pile of vfuncs from the old gpu.tmpl xml documentation that I've forgotten to delete. I spotted a few more things to clarify/extend in the new kerneldoc while going through this once more. v2: Spelling fixes (Thierry). v3: More spelling fixes and use Thierry's proposal to clarify why drivers need to validate modes both in ->mode_fixup and ->mode_valid. Cc: Laurent Pinchart Cc: Thierry Reding Acked-by: Thierry Reding Signed-off-by: Daniel Vetter diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index 225a246..faa5e0d 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -1579,194 +1579,6 @@ void intel_crt_init(struct drm_device *dev) entities. - Legacy CRTC Helper Operations - - - bool (*mode_fixup)(struct drm_crtc *crtc, - const struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode); - - Let CRTCs adjust the requested mode or reject it completely. This - operation returns true if the mode is accepted (possibly after being - adjusted) or false if it is rejected. - - - The mode_fixup operation should reject the - mode if it can't reasonably use it. The definition of "reasonable" - is currently fuzzy in this context. One possible behaviour would be - to set the adjusted mode to the panel timings when a fixed-mode - panel is used with hardware capable of scaling. Another behaviour - would be to accept any input mode and adjust it to the closest mode - supported by the hardware (FIXME: This needs to be clarified). - - - - int (*mode_set_base)(struct drm_crtc *crtc, int x, int y, - struct drm_framebuffer *old_fb) - - Move the CRTC on the current frame buffer (stored in - crtc->fb) to position (x,y). Any of the frame - buffer, x position or y position may have been modified. - - - This helper operation is optional. If not provided, the - drm_crtc_helper_set_config function will fall - back to the mode_set helper operation. - - - FIXME: Why are x and y passed as arguments, as they can be accessed - through crtc->x and - crtc->y? - - - - void (*prepare)(struct drm_crtc *crtc); - - Prepare the CRTC for mode setting. This operation is called after - validating the requested mode. Drivers use it to perform - device-specific operations required before setting the new mode. - - - - int (*mode_set)(struct drm_crtc *crtc, struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode, int x, int y, - struct drm_framebuffer *old_fb); - - Set a new mode, position and frame buffer. Depending on the device - requirements, the mode can be stored internally by the driver and - applied in the commit operation, or - programmed to the hardware immediately. - - - The mode_set operation returns 0 on success - or a negative error code if an error occurs. - - - - void (*commit)(struct drm_crtc *crtc); - - Commit a mode. This operation is called after setting the new mode. - Upon return the device must use the new mode and be fully - operational. - - - - - - Encoder Helper Operations - - - bool (*mode_fixup)(struct drm_encoder *encoder, - const struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode); - - Let encoders adjust the requested mode or reject it completely. This - operation returns true if the mode is accepted (possibly after being - adjusted) or false if it is rejected. See the - mode_fixup CRTC helper - operation for an explanation of the allowed adjustments. - - - - void (*prepare)(struct drm_encoder *encoder); - - Prepare the encoder for mode setting. This operation is called after - validating the requested mode. Drivers use it to perform - device-specific operations required before setting the new mode. - - - - void (*mode_set)(struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode); - - Set a new mode. Depending on the device requirements, the mode can - be stored internally by the driver and applied in the - commit operation, or programmed to the - hardware immediately. - - - - void (*commit)(struct drm_encoder *encoder); - - Commit a mode. This operation is called after setting the new mode. - Upon return the device must use the new mode and be fully - operational. - - - - - - Connector Helper Operations - - - struct drm_encoder *(*best_encoder)(struct drm_connector *connector); - - Return a pointer to the best encoder for the connecter. Device that - map connectors to encoders 1:1 simply return the pointer to the - associated encoder. This operation is mandatory. - - - - int (*get_modes)(struct drm_connector *connector); - - Fill the connector's probed_modes list - by parsing EDID data with drm_add_edid_modes, - adding standard VESA DMT modes with drm_add_modes_noedid, - or calling drm_mode_probed_add directly for every - supported mode and return the number of modes it has detected. This - operation is mandatory. - - - Note that the caller function will automatically add standard VESA - DMT modes up to 1024x768 if the get_modes - helper operation returns no mode and if the connector status is - connector_status_connected. There is no need to call - drm_add_edid_modes manually in that case. - - - The vrefresh value is computed by - drm_helper_probe_single_connector_modes. - - - When parsing EDID data, drm_add_edid_modes fills the - connector display_info - width_mm and - height_mm fields. When creating modes - manually the get_modes helper operation must - set the display_info - width_mm and - height_mm fields if they haven't been set - already (for instance at initialization time when a fixed-size panel is - attached to the connector). The mode width_mm - and height_mm fields are only used internally - during EDID parsing and should not be set when creating modes manually. - - - - int (*mode_valid)(struct drm_connector *connector, - struct drm_display_mode *mode); - - Verify whether a mode is valid for the connector. Return MODE_OK for - supported modes and one of the enum drm_mode_status values (MODE_*) - for unsupported modes. This operation is optional. - - - As the mode rejection reason is currently not used beside for - immediately removing the unsupported mode, an implementation can - return MODE_BAD regardless of the exact reason why the mode is not - valid. - - - Note that the mode_valid helper operation is - only called for modes detected by the device, and - not for modes set by the user through the CRTC - set_config operation. - - - - - Atomic Modeset Helper Functions Reference Overview diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 29e0dc5..a126a0d 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -131,6 +131,20 @@ struct drm_crtc_helper_funcs { * Atomic drivers which need to inspect and adjust more state should * instead use the @atomic_check callback. * + * Also beware that neither core nor helpers filter modes before + * passing them to the driver: While the list of modes that is + * advertised to userspace is filtered using the connector's + * ->mode_valid() callback, neither the core nor the helpers do any + * filtering on modes passed in from userspace when setting a mode. It + * is therefore possible for userspace to pass in a mode that was + * previously filtered out using ->mode_valid() or add a custom mode + * that wasn't probed from EDID or similar to begin with. Even though + * this is an advanced feature and rarely used nowadays, some users rely + * on being able to specify modes manually so drivers must be prepared + * to deal with it. Specifically this means that all drivers need not + * only validate modes in ->mode_valid() but also in ->mode_fixup() to + * make sure invalid modes passed in from userspace are rejected. + * * RETURNS: * * True if an acceptable configuration is possible, false if the modeset @@ -188,7 +202,9 @@ struct drm_crtc_helper_funcs { * This callback is used by the legacy CRTC helpers to set a new * framebuffer and scanout position. It is optional and used as an * optimized fast-path instead of a full mode set operation with all the - * resulting flickering. Since it can't update other planes it's + * resulting flickering. If it is not present + * drm_crtc_helper_set_config() will fall back to a full modeset, using + * the ->mode_set() callback. Since it can't update other planes it's * incompatible with atomic modeset support. * * This callback is only used by the CRTC helpers and deprecated. @@ -439,6 +455,20 @@ struct drm_encoder_helper_funcs { * Atomic drivers which need to inspect and adjust more state should * instead use the @atomic_check callback. * + * Also beware that neither core nor helpers filter modes before + * passing them to the driver: While the list of modes that is + * advertised to userspace is filtered using the connector's + * ->mode_valid() callback, neither the core nor the helpers do any + * filtering on modes passed in from userspace when setting a mode. It + * is therefore possible for userspace to pass in a mode that was + * previously filtered out using ->mode_valid() or add a custom mode + * that wasn't probed from EDID or similar to begin with. Even though + * this is an advanced feature and rarely used nowadays, some users rely + * on being able to specify modes manually so drivers must be prepared + * to deal with it. Specifically this means that all drivers need not + * only validate modes in ->mode_valid() but also in ->mode_fixup() to + * make sure invalid modes passed in from userspace are rejected. + * * RETURNS: * * True if an acceptable configuration is possible, false if the modeset @@ -640,8 +670,16 @@ struct drm_connector_helper_funcs { * In this function drivers then parse the modes in the EDID and add * them by calling drm_add_edid_modes(). But connectors that driver a * fixed panel can also manually add specific modes using - * drm_mode_probed_add(). Finally drivers that support audio probably - * want to update the ELD data, too, using drm_edid_to_eld(). + * drm_mode_probed_add(). Drivers which manually add modes should also + * make sure that the @display_info, @width_mm and @height_mm fields of the + * struct #drm_connector are filled in. + * + * Virtual drivers that just want some standard VESA mode with a given + * resolution can call drm_add_modes_noedid(), and mark the preferred + * one using drm_set_preferred_mode(). + * + * Finally drivers that support audio probably want to update the ELD + * data, too, using drm_edid_to_eld(). * * This function is only called after the ->detect() hook has indicated * that a sink is connected and when the EDID isn't overridden through -- cgit v0.10.2 From 9649399e918f61788a6302a0f7d3c5ed34c2930c Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 5 Jan 2016 09:42:30 +0000 Subject: drm: Do not set outparam on error during GEM handle allocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Good practice dictates that we do not leak stale information to our callers, and should avoid overwriting an outparam on an error path. Reported-by: Ville Syrjälä Signed-off-by: Chris Wilson Cc: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1451986951-3703-1-git-send-email-chris@chris-wilson.co.uk Reviewed-by: Thierry Reding Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 1b0c2c1..eeee320 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -331,6 +331,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, u32 *handlep) { struct drm_device *dev = obj->dev; + u32 handle; int ret; WARN_ON(!mutex_is_locked(&dev->object_name_lock)); @@ -353,7 +354,7 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, if (ret < 0) goto err_unref; - *handlep = ret; + handle = ret; ret = drm_vma_node_allow(&obj->vma_node, file_priv->filp); if (ret) @@ -365,13 +366,14 @@ drm_gem_handle_create_tail(struct drm_file *file_priv, goto err_revoke; } + *handlep = handle; return 0; err_revoke: drm_vma_node_revoke(&obj->vma_node, file_priv->filp); err_remove: spin_lock(&file_priv->table_lock); - idr_remove(&file_priv->object_idr, *handlep); + idr_remove(&file_priv->object_idr, handle); spin_unlock(&file_priv->table_lock); err_unref: drm_gem_object_handle_unreference_unlocked(obj); -- cgit v0.10.2 From 8815b23aa0933653812e9c13c506f6f60fac474e Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Tue, 5 Jan 2016 09:42:31 +0000 Subject: drm: Remove opencoded drm_gem_object_release_handle() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drm_gem_handle_delete() contains its own version of drm_gem_object_release_handle(), so lets just call the release method instead. Reported-by: Ville Syrjälä Signed-off-by: Chris Wilson Cc: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1451986951-3703-2-git-send-email-chris@chris-wilson.co.uk Reviewed-by: Thierry Reding Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index eeee320..2e8c77e 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -244,6 +244,29 @@ drm_gem_object_handle_unreference_unlocked(struct drm_gem_object *obj) drm_gem_object_unreference_unlocked(obj); } +/* + * Called at device or object close to release the file's + * handle references on objects. + */ +static int +drm_gem_object_release_handle(int id, void *ptr, void *data) +{ + struct drm_file *file_priv = data; + struct drm_gem_object *obj = ptr; + struct drm_device *dev = obj->dev; + + if (drm_core_check_feature(dev, DRIVER_PRIME)) + drm_gem_remove_prime_handles(obj, file_priv); + drm_vma_node_revoke(&obj->vma_node, file_priv->filp); + + if (dev->driver->gem_close_object) + dev->driver->gem_close_object(obj, file_priv); + + drm_gem_object_handle_unreference_unlocked(obj); + + return 0; +} + /** * drm_gem_handle_delete - deletes the given file-private handle * @filp: drm file-private structure to use for the handle look up @@ -282,14 +305,7 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) idr_remove(&filp->object_idr, handle); spin_unlock(&filp->table_lock); - if (drm_core_check_feature(dev, DRIVER_PRIME)) - drm_gem_remove_prime_handles(obj, filp); - drm_vma_node_revoke(&obj->vma_node, filp->filp); - - if (dev->driver->gem_close_object) - dev->driver->gem_close_object(obj, filp); - drm_gem_object_handle_unreference_unlocked(obj); - + drm_gem_object_release_handle(handle, obj, filp); return 0; } EXPORT_SYMBOL(drm_gem_handle_delete); @@ -726,29 +742,6 @@ drm_gem_open(struct drm_device *dev, struct drm_file *file_private) spin_lock_init(&file_private->table_lock); } -/* - * Called at device close to release the file's - * handle references on objects. - */ -static int -drm_gem_object_release_handle(int id, void *ptr, void *data) -{ - struct drm_file *file_priv = data; - struct drm_gem_object *obj = ptr; - struct drm_device *dev = obj->dev; - - if (drm_core_check_feature(dev, DRIVER_PRIME)) - drm_gem_remove_prime_handles(obj, file_priv); - drm_vma_node_revoke(&obj->vma_node, file_priv->filp); - - if (dev->driver->gem_close_object) - dev->driver->gem_close_object(obj, file_priv); - - drm_gem_object_handle_unreference_unlocked(obj); - - return 0; -} - /** * drm_gem_release - release file-private GEM resources * @dev: drm_device which is being closed by userspace -- cgit v0.10.2 From 2aa974c92b9d0bb092a8f308d83dfe3c69041b01 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Wed, 6 Jan 2016 14:53:25 +0100 Subject: drm/i915: Update connector_mask during readout, v2. drm/i915: Update connector_mask during readout, v2. The connector_mask may be used any time during the non-atomic .crtc_disable which is called before the full atomic state is set up and needs to be accurate for that reason. Changes since v1: - Update connector_mask in readout_hw_state and add a comment. Signed-off-by: Maarten Lankhorst Link: http://patchwork.freedesktop.org/patch/msgid/568D1C55.8010001@linux.intel.com Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e8f76be..1ef7153 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15355,6 +15355,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc) WARN_ON(drm_atomic_set_mode_for_crtc(crtc->base.state, NULL) < 0); crtc->base.state->active = crtc->active; crtc->base.enabled = crtc->active; + crtc->base.state->connector_mask = 0; /* Because we only establish the connector -> encoder -> * crtc links if something is active, this means the @@ -15557,7 +15558,21 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) for_each_intel_connector(dev, connector) { if (connector->get_hw_state(connector)) { connector->base.dpms = DRM_MODE_DPMS_ON; - connector->base.encoder = &connector->encoder->base; + + encoder = connector->encoder; + connector->base.encoder = &encoder->base; + + if (encoder->base.crtc && + encoder->base.crtc->state->active) { + /* + * This has to be done during hardware readout + * because anything calling .crtc_disable may + * rely on the connector_mask being accurate. + */ + encoder->base.crtc->state->connector_mask |= + 1 << drm_connector_index(&connector->base); + } + } else { connector->base.dpms = DRM_MODE_DPMS_OFF; connector->base.encoder = NULL; -- cgit v0.10.2 From 14de6c44d149c68df1800ded42bbab51485ef67a Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 4 Jan 2016 12:53:20 +0100 Subject: drm/atomic: Remove drm_atomic_connectors_for_crtc. Now that connector_mask is reliable there's no need for this function any more. Signed-off-by: Maarten Lankhorst Link: http://patchwork.freedesktop.org/patch/msgid/1451908400-25147-6-git-send-email-maarten.lankhorst@linux.intel.com Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 6050c80f..3f74193 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1199,36 +1199,6 @@ drm_atomic_add_affected_planes(struct drm_atomic_state *state, EXPORT_SYMBOL(drm_atomic_add_affected_planes); /** - * drm_atomic_connectors_for_crtc - count number of connected outputs - * @state: atomic state - * @crtc: DRM crtc - * - * This function counts all connectors which will be connected to @crtc - * according to @state. Useful to recompute the enable state for @crtc. - */ -int -drm_atomic_connectors_for_crtc(struct drm_atomic_state *state, - struct drm_crtc *crtc) -{ - struct drm_connector *connector; - struct drm_connector_state *conn_state; - - int i, num_connected_connectors = 0; - - for_each_connector_in_state(state, connector, conn_state, i) { - if (conn_state->crtc == crtc) - num_connected_connectors++; - } - - DRM_DEBUG_ATOMIC("State %p has %i connectors for [CRTC:%d:%s]\n", - state, num_connected_connectors, - crtc->base.id, crtc->name); - - return num_connected_connectors; -} -EXPORT_SYMBOL(drm_atomic_connectors_for_crtc); - -/** * drm_atomic_legacy_backoff - locking backoff for legacy ioctls * @state: atomic state * diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 738104b..57cccd6 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -463,7 +463,8 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, * crtc only changed its mode but has the same set of connectors. */ for_each_crtc_in_state(state, crtc, crtc_state, i) { - int num_connectors; + bool has_connectors = + !!crtc_state->connector_mask; /* * We must set ->active_changed after walking connectors for @@ -492,10 +493,7 @@ drm_atomic_helper_check_modeset(struct drm_device *dev, if (ret != 0) return ret; - num_connectors = drm_atomic_connectors_for_crtc(state, - crtc); - - if (crtc_state->enable != !!num_connectors) { + if (crtc_state->enable != has_connectors) { DRM_DEBUG_ATOMIC("[CRTC:%d:%s] enabled/connectors mismatch\n", crtc->base.id, crtc->name); @@ -1754,7 +1752,7 @@ static int update_output_state(struct drm_atomic_state *state, if (crtc == set->crtc) continue; - if (!drm_atomic_connectors_for_crtc(state, crtc)) { + if (!crtc_state->connector_mask) { ret = drm_atomic_set_mode_prop_for_crtc(crtc_state, NULL); if (ret < 0) diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 8d0d70e..018145e 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -328,7 +328,7 @@ static int vc4_crtc_atomic_check(struct drm_crtc *crtc, /* The pixelvalve can only feed one encoder (and encoders are * 1:1 with connectors.) */ - if (drm_atomic_connectors_for_crtc(state->state, crtc) > 1) + if (hweight32(state->connector_mask) > 1) return -EINVAL; drm_atomic_crtc_state_for_each_plane(plane, state) { diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h index d8576ac..d3eaa5d 100644 --- a/include/drm/drm_atomic.h +++ b/include/drm/drm_atomic.h @@ -130,10 +130,6 @@ int __must_check drm_atomic_add_affected_planes(struct drm_atomic_state *state, struct drm_crtc *crtc); -int -drm_atomic_connectors_for_crtc(struct drm_atomic_state *state, - struct drm_crtc *crtc); - void drm_atomic_legacy_backoff(struct drm_atomic_state *state); void -- cgit v0.10.2 From d9278b4c2c31603474eec19d9ea1dea6b3a81087 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Fri, 8 Jan 2016 13:21:51 +0200 Subject: drm/edid: index CEA/HDMI mode tables using the VIC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a dummy entry to CEA/HDMI mode tables so they can be indexed directly using the VIC, avoiding a +1/-1 dance here and there. This adds clarity to the error checking for various functions that return the VIC on success and zero on failure; we can now explicitly check for 0 instead of just subtracting one from an unsigned type. Also add drm_valid_cea_vic() and drm_valid_hdmi_vic() helpers for checking valid VICs. v2: add drm_valid_cea_vic and drm_valid_hdmi_vic helpers (Ville) use { } instead of { 0 } for initializing the dummy modes Cc: Ville Syrjälä Cc: Alex Deucher Signed-off-by: Jani Nikula Reviewed-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1452252111-6439-1-git-send-email-jani.nikula@intel.com Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c214f12..04cb487 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -637,8 +637,12 @@ static const struct minimode extra_modes[] = { /* * Probably taken from CEA-861 spec. * This table is converted from xorg's hw/xfree86/modes/xf86EdidModes.c. + * + * Index using the VIC. */ static const struct drm_display_mode edid_cea_modes[] = { + /* 0 - dummy, VICs start at 1 */ + { }, /* 1 - 640x480@60Hz */ { DRM_MODE("640x480", DRM_MODE_TYPE_DRIVER, 25175, 640, 656, 752, 800, 0, 480, 490, 492, 525, 0, @@ -987,9 +991,11 @@ static const struct drm_display_mode edid_cea_modes[] = { }; /* - * HDMI 1.4 4k modes. + * HDMI 1.4 4k modes. Index using the VIC. */ static const struct drm_display_mode edid_4k_modes[] = { + /* 0 - dummy, VICs start at 1 */ + { }, /* 1 - 3840x2160@30Hz */ { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000, 3840, 4016, 4104, 4400, 0, @@ -2548,13 +2554,13 @@ cea_mode_alternate_clock(const struct drm_display_mode *cea_mode) static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) { - u8 mode; + u8 vic; if (!to_match->clock) return 0; - for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) { - const struct drm_display_mode *cea_mode = &edid_cea_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) { + const struct drm_display_mode *cea_mode = &edid_cea_modes[vic]; unsigned int clock1, clock2; /* Check both 60Hz and 59.94Hz */ @@ -2566,7 +2572,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m continue; if (drm_mode_equal_no_clocks(to_match, cea_mode)) - return mode + 1; + return vic; } return 0; @@ -2581,13 +2587,13 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m */ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) { - u8 mode; + u8 vic; if (!to_match->clock) return 0; - for (mode = 0; mode < ARRAY_SIZE(edid_cea_modes); mode++) { - const struct drm_display_mode *cea_mode = &edid_cea_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) { + const struct drm_display_mode *cea_mode = &edid_cea_modes[vic]; unsigned int clock1, clock2; /* Check both 60Hz and 59.94Hz */ @@ -2597,12 +2603,17 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, cea_mode)) - return mode + 1; + return vic; } return 0; } EXPORT_SYMBOL(drm_match_cea_mode); +static bool drm_valid_cea_vic(u8 vic) +{ + return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes); +} + /** * drm_get_cea_aspect_ratio - get the picture aspect ratio corresponding to * the input VIC from the CEA mode list @@ -2612,10 +2623,7 @@ EXPORT_SYMBOL(drm_match_cea_mode); */ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code) { - /* return picture aspect ratio for video_code - 1 to access the - * right array element - */ - return edid_cea_modes[video_code-1].picture_aspect_ratio; + return edid_cea_modes[video_code].picture_aspect_ratio; } EXPORT_SYMBOL(drm_get_cea_aspect_ratio); @@ -2639,13 +2647,13 @@ hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode) static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) { - u8 mode; + u8 vic; if (!to_match->clock) return 0; - for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) { - const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { + const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic]; unsigned int clock1, clock2; /* Make sure to also match alternate clocks */ @@ -2657,7 +2665,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ continue; if (drm_mode_equal_no_clocks(to_match, hdmi_mode)) - return mode + 1; + return vic; } return 0; @@ -2673,13 +2681,13 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ */ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) { - u8 mode; + u8 vic; if (!to_match->clock) return 0; - for (mode = 0; mode < ARRAY_SIZE(edid_4k_modes); mode++) { - const struct drm_display_mode *hdmi_mode = &edid_4k_modes[mode]; + for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) { + const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic]; unsigned int clock1, clock2; /* Make sure to also match alternate clocks */ @@ -2689,11 +2697,16 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) if ((KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock1) || KHZ2PICOS(to_match->clock) == KHZ2PICOS(clock2)) && drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) - return mode + 1; + return vic; } return 0; } +static bool drm_valid_hdmi_vic(u8 vic) +{ + return vic > 0 && vic < ARRAY_SIZE(edid_4k_modes); +} + static int add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) { @@ -2713,16 +2726,16 @@ add_alternate_cea_modes(struct drm_connector *connector, struct edid *edid) list_for_each_entry(mode, &connector->probed_modes, head) { const struct drm_display_mode *cea_mode = NULL; struct drm_display_mode *newmode; - u8 mode_idx = drm_match_cea_mode(mode) - 1; + u8 vic = drm_match_cea_mode(mode); unsigned int clock1, clock2; - if (mode_idx < ARRAY_SIZE(edid_cea_modes)) { - cea_mode = &edid_cea_modes[mode_idx]; + if (drm_valid_cea_vic(vic)) { + cea_mode = &edid_cea_modes[vic]; clock2 = cea_mode_alternate_clock(cea_mode); } else { - mode_idx = drm_match_hdmi_mode(mode) - 1; - if (mode_idx < ARRAY_SIZE(edid_4k_modes)) { - cea_mode = &edid_4k_modes[mode_idx]; + vic = drm_match_hdmi_mode(mode); + if (drm_valid_hdmi_vic(vic)) { + cea_mode = &edid_4k_modes[vic]; clock2 = hdmi_mode_alternate_clock(cea_mode); } } @@ -2773,17 +2786,17 @@ drm_display_mode_from_vic_index(struct drm_connector *connector, { struct drm_device *dev = connector->dev; struct drm_display_mode *newmode; - u8 cea_mode; + u8 vic; if (video_db == NULL || video_index >= video_len) return NULL; /* CEA modes are numbered 1..127 */ - cea_mode = (video_db[video_index] & 127) - 1; - if (cea_mode >= ARRAY_SIZE(edid_cea_modes)) + vic = (video_db[video_index] & 127); + if (!drm_valid_cea_vic(vic)) return NULL; - newmode = drm_mode_duplicate(dev, &edid_cea_modes[cea_mode]); + newmode = drm_mode_duplicate(dev, &edid_cea_modes[vic]); if (!newmode) return NULL; @@ -2878,8 +2891,7 @@ static int add_hdmi_mode(struct drm_connector *connector, u8 vic) struct drm_device *dev = connector->dev; struct drm_display_mode *newmode; - vic--; /* VICs start at 1 */ - if (vic >= ARRAY_SIZE(edid_4k_modes)) { + if (!drm_valid_hdmi_vic(vic)) { DRM_ERROR("Unknown HDMI VIC: %d\n", vic); return 0; } @@ -3170,24 +3182,24 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) { const struct drm_display_mode *cea_mode; int clock1, clock2, clock; - u8 mode_idx; + u8 vic; const char *type; /* * allow 5kHz clock difference either way to account for * the 10kHz clock resolution limit of detailed timings. */ - mode_idx = drm_match_cea_mode_clock_tolerance(mode, 5) - 1; - if (mode_idx < ARRAY_SIZE(edid_cea_modes)) { + vic = drm_match_cea_mode_clock_tolerance(mode, 5); + if (drm_valid_cea_vic(vic)) { type = "CEA"; - cea_mode = &edid_cea_modes[mode_idx]; + cea_mode = &edid_cea_modes[vic]; clock1 = cea_mode->clock; clock2 = cea_mode_alternate_clock(cea_mode); } else { - mode_idx = drm_match_hdmi_mode_clock_tolerance(mode, 5) - 1; - if (mode_idx < ARRAY_SIZE(edid_4k_modes)) { + vic = drm_match_hdmi_mode_clock_tolerance(mode, 5); + if (drm_valid_hdmi_vic(vic)) { type = "HDMI"; - cea_mode = &edid_4k_modes[mode_idx]; + cea_mode = &edid_4k_modes[vic]; clock1 = cea_mode->clock; clock2 = hdmi_mode_alternate_clock(cea_mode); } else { @@ -3205,7 +3217,7 @@ static void fixup_detailed_cea_mode_clock(struct drm_display_mode *mode) return; DRM_DEBUG("detailed mode matches %s VIC %d, adjusting clock %d -> %d\n", - type, mode_idx + 1, mode->clock, clock); + type, vic, mode->clock, clock); mode->clock = clock; } -- cgit v0.10.2 From d9d8c4cf23e8badfa8d49fe66c6a49970f896689 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Fri, 8 Jan 2016 14:00:45 +0300 Subject: drm: move MODULE_PARM_DESC to other file We moved the module options from drm_drv.c to drm_irq.c in 18882995713d ('drm: Move vblank related module options into drm_irq.c'). Let's move the MODULE_PARM_DESC()s as well so they're together. Signed-off-by: Dan Carpenter Link: http://patchwork.freedesktop.org/patch/msgid/20160108110045.GF32195@mwanda Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index bf934cde..167c8d3 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -44,10 +44,6 @@ MODULE_AUTHOR(CORE_AUTHOR); MODULE_DESCRIPTION(CORE_DESC); MODULE_LICENSE("GPL and additional rights"); MODULE_PARM_DESC(debug, "Enable debug output"); -MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs] (0: never disable, <0: disable immediately)"); -MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]"); -MODULE_PARM_DESC(timestamp_monotonic, "Use monotonic timestamps"); - module_param_named(debug, drm_debug, int, 0600); static DEFINE_SPINLOCK(drm_minor_lock); diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 607f493..d12a4ef 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -73,6 +73,9 @@ static int drm_vblank_offdelay = 5000; /* Default to 5000 msecs. */ module_param_named(vblankoffdelay, drm_vblank_offdelay, int, 0600); module_param_named(timestamp_precision_usec, drm_timestamp_precision, int, 0600); module_param_named(timestamp_monotonic, drm_timestamp_monotonic, int, 0600); +MODULE_PARM_DESC(vblankoffdelay, "Delay until vblank irq auto-disable [msecs] (0: never disable, <0: disable immediately)"); +MODULE_PARM_DESC(timestamp_precision_usec, "Max. error on timestamps [usecs]"); +MODULE_PARM_DESC(timestamp_monotonic, "Use monotonic timestamps"); static void store_vblank(struct drm_device *dev, unsigned int pipe, u32 vblank_count_inc, -- cgit v0.10.2 From 3d7b75fdae9c81dd71c7573dbc285af90e0924fa Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Mon, 11 Jan 2016 00:08:35 +0100 Subject: apple-gmux: Add initial documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document what I've learned so far about the gmux so that we can collaboratively reverse-engineer its remaining unknown bits without everyone having to start from scratch. The DOC sections are bound together in the gpu.tmpl DocBook under a new vga_switcheroo "Handlers" chapter. Eventually this should be amended with documentation about the four other handlers that exist so far (nouveau v1 DSM, nouveau Optimus DSM, radeon ATPX, amdgpu ATPX). Requires kernel-doc with asciidoc support. The EFI variable was reverse-engineered by Bruno Bierbaumer and Andreas Heider . Some of the remaining open questions: * How are vblank intervals synchronized on retinas to achieve seamless switching? Is the DP mux capable of this? It's not mentioned in the data sheets. Or is it done at the OS level, i.e. do we have to synchronize vblank intervals between DRM drivers? There's a signal coming from the panel connector and going into gmux, could this be the vblank signal as received by the panel to properly time the switch? * On retinas there's an I2C bus between gmux and the connector of the right I/O board, apparently leading to the Parade PS8401A HDMI repeater. What is this for, is it controlled via gmux registers? Data sheet: http://www.paradetech.com/products/jitter-cleaning-repeaters/ps8401/ * On retinas there's an I2C bus between gmux and the LED driver. Pre-retinas connected the LED driver to SMBUS. Are there additional gmux registers on retinas to control it? * The MacPro6,1 2013 also has a gmux, the same Renesas R4F2113 as the retina MacBook Pro. The Mac Pro doesn't have a built-in display, so what is its purpose? Power control of the dual FirePro GPUs? Switching of the external DP/Thunderbolt ports? The iFixit teardown clearly shows one TI HD3SS212 DisplayPort mux on the logic board next to one of the three Thunderbolt controllers. However six muxes would be necessary to switch all six ports between GPUs. The mux is probably necessary for one of the display configurations allowed by Apple, but which one? https://www.ifixit.com/Teardown/Mac+Pro+Late+2013+Teardown/20778 https://d3nevzfk7ii3be.cloudfront.net/igi/fELBTnt31QhnDsqq.huge https://support.apple.com/en-us/HT202801 * Registers we haven't decoded yet: 0x700 32 Bit configmap? 0x708 32 Bit power sequence? 0x712 8 Bit status of clock from panel on retinas? 0x713 8 Bit dito? 0x724 16 Bit backlight, raw value? 0x760 32 Bit backlight 0x764 32 Bit backlight 0x768 8 Bit backlight 0x76a 16 Bit backlight 0x76c 16 Bit backlight 0x76e 16 Bit backlight 0x77f edp/dp/hdmi probe? retina only. * Addition by Bruno Prémont : "Missing is also precise knowledge as to what the gmux depends on. From behavioral reports, it is somehow sensitive to VGA IO/MEM routing (it apparently needs the routing to go to integrated GPU, not discrete GPU). When the routing is inappropriate backlight control IO just reads back as 0xFF (and eventually gmux IO in general does so)." Signed-off-by: Lukas Wunner Acked-by: Darren Hart Link: http://patchwork.freedesktop.org/patch/msgid/da309e436fbeac886477d80376457b7d83ea4b2d.1452431795.git.lukas@wunner.de Signed-off-by: Daniel Vetter diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index faa5e0d..a866933 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -3470,8 +3470,30 @@ int num_ioctls; + + Handlers + + apple-gmux Handler +!Pdrivers/platform/x86/apple-gmux.c Overview +!Pdrivers/platform/x86/apple-gmux.c Interrupt + + Graphics mux +!Pdrivers/platform/x86/apple-gmux.c Graphics mux + + + Power control +!Pdrivers/platform/x86/apple-gmux.c Power control + + + Backlight control +!Pdrivers/platform/x86/apple-gmux.c Backlight control + + + + !Cdrivers/gpu/vga/vga_switcheroo.c !Cinclude/linux/vga_switcheroo.h +!Cdrivers/platform/x86/apple-gmux.c diff --git a/drivers/platform/x86/apple-gmux.c b/drivers/platform/x86/apple-gmux.c index 976efeb..2b921de 100644 --- a/drivers/platform/x86/apple-gmux.c +++ b/drivers/platform/x86/apple-gmux.c @@ -3,6 +3,7 @@ * * Copyright (C) Canonical Ltd. * Copyright (C) 2010-2012 Andreas Heider + * Copyright (C) 2015 Lukas Wunner * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 as @@ -26,6 +27,24 @@ #include #include +/** + * DOC: Overview + * + * :1: http://www.latticesemi.com/en/Products/FPGAandCPLD/LatticeXP2.aspx + * :2: http://www.renesas.com/products/mpumcu/h8s/h8s2100/h8s2113/index.jsp + * + * gmux is a microcontroller built into the MacBook Pro to support dual GPUs: + * A {1}[Lattice XP2] on pre-retinas, a {2}[Renesas R4F2113] on retinas. + * + * (The MacPro6,1 2013 also has a gmux, however it is unclear why since it has + * dual GPUs but no built-in display.) + * + * gmux is connected to the LPC bus of the southbridge. Its I/O ports are + * accessed differently depending on the microcontroller: Driver functions + * to access a pre-retina gmux are infixed `_pio_`, those for a retina gmux + * are infixed `_index_`. + */ + struct apple_gmux_data { unsigned long iostart; unsigned long iolen; @@ -247,6 +266,20 @@ static bool gmux_is_indexed(struct apple_gmux_data *gmux_data) return false; } +/** + * DOC: Backlight control + * + * :3: http://www.ti.com/lit/ds/symlink/lp8543.pdf + * :4: http://www.ti.com/lit/ds/symlink/lp8545.pdf + * + * On single GPU MacBooks, the PWM signal for the backlight is generated by + * the GPU. On dual GPU MacBook Pros by contrast, either GPU may be suspended + * to conserve energy. Hence the PWM signal needs to be generated by a separate + * backlight driver which is controlled by gmux. The earliest generation + * MBP5 2008/09 uses a {3}[TI LP8543] backlight driver. All newer models + * use a {4}[TI LP8545]. + */ + static int gmux_get_brightness(struct backlight_device *bd) { struct apple_gmux_data *gmux_data = bl_get_data(bd); @@ -273,6 +306,68 @@ static const struct backlight_ops gmux_bl_ops = { .update_status = gmux_update_status, }; +/** + * DOC: Graphics mux + * + * :5: http://pimg-fpiw.uspto.gov/fdd/07/870/086/0.pdf + * :6: http://www.nxp.com/documents/data_sheet/CBTL06141.pdf + * :7: http://www.ti.com/lit/ds/symlink/hd3ss212.pdf + * :8: https://www.pericom.com/assets/Datasheets/PI3VDP12412.pdf + * :9: http://www.ti.com/lit/ds/symlink/sn74lv4066a.pdf + * :10: http://pdf.datasheetarchive.com/indexerfiles/Datasheets-SW16/DSASW00308511.pdf + * :11: http://www.ti.com/lit/ds/symlink/ts3ds10224.pdf + * + * On pre-retinas, the LVDS outputs of both GPUs feed into gmux which muxes + * either of them to the panel. One of the tricks gmux has up its sleeve is + * to lengthen the blanking interval of its output during a switch to + * synchronize it with the GPU switched to. This allows for a flicker-free + * switch that is imperceptible by the user ({5}[US 8,687,007 B2]). + * + * On retinas, muxing is no longer done by gmux itself, but by a separate + * chip which is controlled by gmux. The chip is triple sourced, it is + * either an {6}[NXP CBTL06142], {7}[TI HD3SS212] or {8}[Pericom PI3VDP12412]. + * The panel is driven with eDP instead of LVDS since the pixel clock + * required for retina resolution exceeds LVDS' limits. + * + * Pre-retinas are able to switch the panel's DDC pins separately. + * This is handled by a {9}[TI SN74LV4066A] which is controlled by gmux. + * The inactive GPU can thus probe the panel's EDID without switching over + * the entire panel. Retinas lack this functionality as the chips used for + * eDP muxing are incapable of switching the AUX channel separately (see + * the linked data sheets, Pericom would be capable but this is unused). + * However the retina panel has the NO_AUX_HANDSHAKE_LINK_TRAINING bit set + * in its DPCD, allowing the inactive GPU to skip the AUX handshake and + * set up the output with link parameters pre-calibrated by the active GPU. + * + * The external DP port is only fully switchable on the first two unibody + * MacBook Pro generations, MBP5 2008/09 and MBP6 2010. This is done by an + * {6}[NXP CBTL06141] which is controlled by gmux. It's the predecessor of the + * eDP mux on retinas, the difference being support for 2.7 versus 5.4 Gbit/s. + * + * The following MacBook Pro generations replaced the external DP port with a + * combined DP/Thunderbolt port and lost the ability to switch it between GPUs, + * connecting it either to the discrete GPU or the Thunderbolt controller. + * Oddly enough, while the full port is no longer switchable, AUX and HPD + * are still switchable by way of an {10}[NXP CBTL03062] (on pre-retinas + * MBP8 2011 and MBP9 2012) or two {11}[TI TS3DS10224] (on retinas) under the + * control of gmux. Since the integrated GPU is missing the main link, + * external displays appear to it as phantoms which fail to link-train. + * + * gmux receives the HPD signal of all display connectors and sends an + * interrupt on hotplug. On generations which cannot switch external ports, + * the discrete GPU can then be woken to drive the newly connected display. + * The ability to switch AUX on these generations could be used to improve + * reliability of hotplug detection by having the integrated GPU poll the + * ports while the discrete GPU is asleep, but currently we do not make use + * of this feature. + * + * gmux' initial switch state on bootup is user configurable via the EFI + * variable `gpu-power-prefs-fa4ce28d-b62f-4c99-9cc3-6815686e30f9` (5th byte, + * 1 = IGD, 0 = DIS). Based on this setting, the EFI firmware tells gmux to + * switch the panel and the external DP connector and allocates a framebuffer + * for the selected GPU. + */ + static int gmux_switchto(enum vga_switcheroo_client_id id) { if (id == VGA_SWITCHEROO_IGD) { @@ -288,6 +383,14 @@ static int gmux_switchto(enum vga_switcheroo_client_id id) return 0; } +/** + * DOC: Power control + * + * gmux is able to cut power to the discrete GPU. It automatically takes care + * of the correct sequence to tear down and bring up the power rails for + * core voltage, VRAM and PCIe. + */ + static int gmux_set_discrete_state(struct apple_gmux_data *gmux_data, enum vga_switcheroo_state state) { @@ -352,6 +455,16 @@ static const struct vga_switcheroo_handler gmux_handler = { .get_client_id = gmux_get_client_id, }; +/** + * DOC: Interrupt + * + * gmux is also connected to a GPIO pin of the southbridge and thereby is able + * to trigger an ACPI GPE. On the MBP5 2008/09 it's GPIO pin 22 of the Nvidia + * MCP79, on all following generations it's GPIO pin 6 of the Intel PCH. + * The GPE merely signals that an interrupt occurred, the actual type of event + * is identified by reading a gmux register. + */ + static inline void gmux_disable_interrupts(struct apple_gmux_data *gmux_data) { gmux_write8(gmux_data, GMUX_PORT_INTERRUPT_ENABLE, -- cgit v0.10.2 From eb47fe8033d6c2013ce47ec44f39fa0092aa8551 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Mon, 16 Nov 2015 18:19:53 +0100 Subject: drm: Do not set connector->encoder in drivers An encoder is associated with a connector by the DRM core as a result of setting up a configuration. Drivers using the atomic or legacy helpers should never set up this link, even if it is a static one. While at it, try to catch this kind of error in the future by adding a WARN_ON() in drm_mode_connector_attach_encoder(). Note that this doesn't cover all the cases, since drivers could set this up after attaching. Drivers that use the atomic helpers will get a warning later on, though, so hopefully the two combined cover enough to help people avoid this in the future. Cc: Russell King Cc: Philipp Zabel Cc: Laurent Pinchart Cc: Liviu Dudau Cc: Mark yao Signed-off-by: Thierry Reding Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1447694393-24700-1-git-send-email-thierry.reding@gmail.com diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c index 210d8df..f224945 100644 --- a/drivers/gpu/drm/bridge/dw_hdmi.c +++ b/drivers/gpu/drm/bridge/dw_hdmi.c @@ -1648,8 +1648,6 @@ static int dw_hdmi_register(struct drm_device *drm, struct dw_hdmi *hdmi) drm_connector_init(drm, &hdmi->connector, &dw_hdmi_connector_funcs, DRM_MODE_CONNECTOR_HDMIA); - hdmi->connector.encoder = encoder; - drm_mode_connector_attach_encoder(&hdmi->connector, encoder); return 0; diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 62fa95f..d40bab2 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5054,6 +5054,20 @@ int drm_mode_connector_attach_encoder(struct drm_connector *connector, { int i; + /* + * In the past, drivers have attempted to model the static association + * of connector to encoder in simple connector/encoder devices using a + * direct assignment of connector->encoder = encoder. This connection + * is a logical one and the responsibility of the core, so drivers are + * expected not to mess with this. + * + * Note that the error return should've been enough here, but a large + * majority of drivers ignores the return value, so add in a big WARN + * to get people's attention. + */ + if (WARN_ON(connector->encoder)) + return -EINVAL; + for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) { if (connector->encoder_ids[i] == 0) { connector->encoder_ids[i] = encoder->base.id; diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index a46248f..7885859 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -1439,7 +1439,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) if (ret) goto err_sysfs; - priv->connector.encoder = &priv->encoder; drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder); return 0; diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c index b74bf8e..0ffef17 100644 --- a/drivers/gpu/drm/imx/parallel-display.c +++ b/drivers/gpu/drm/imx/parallel-display.c @@ -204,8 +204,6 @@ static int imx_pd_register(struct drm_device *drm, drm_mode_connector_attach_encoder(&imxpd->connector, &imxpd->encoder); - imxpd->connector.encoder = &imxpd->encoder; - return 0; } diff --git a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c index b80802f..db07637 100644 --- a/drivers/gpu/drm/shmobile/shmob_drm_crtc.c +++ b/drivers/gpu/drm/shmobile/shmob_drm_crtc.c @@ -739,8 +739,6 @@ int shmob_drm_connector_create(struct shmob_drm_device *sdev, if (ret < 0) goto err_backlight; - connector->encoder = encoder; - drm_helper_connector_dpms(connector, DRM_MODE_DPMS_OFF); drm_object_property_set_value(&connector->base, sdev->ddev->mode_config.dpms_property, DRM_MODE_DPMS_OFF); -- cgit v0.10.2 From f5949141a21ee16edf1beaf95cbae7e419171ab5 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 13 Jan 2016 11:55:28 +0100 Subject: drm/i915: Init power domains early in driver load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since commit ac9b8236551d1177fd07b56aef9b565d1864420d Author: Ville Syrjälä Date: Fri Nov 27 18:55:26 2015 +0200 drm/i915: Introduce a gmbus power domain gmbus also needs the power domain infrastructure right from the start, since as soon as we register the i2c controllers someone can use them. v2: Adjust cleanup paths too (Chris). v3: Rebase onto -nightly (totally bogus tree I had lying around) and also move dpio init head (Ville). v4: Ville instead suggested to move gmbus setup later in the sequence, since it's only needed by the modeset code. v5: Move even close to the actual user, right next to the comment that states where we really need gmbus (and interrupts!). Cc: Ville Syrjälä Cc: Patrik Jakobsson Cc: Imre Deak Cc: Jani Nikula Cc: Meelis Roos Cc: Chris Wilson Fixes: ac9b8236551d ("drm/i915: Introduce a gmbus power domain") Cc: stable@vger.kernel.org References: http://www.spinics.net/lists/intel-gfx/msg83075.html Signed-off-by: Daniel Vetter Reviewed-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/1452682528-19437-1-git-send-email-daniel.vetter@ffwll.ch Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index a81c766..aace060 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -403,6 +403,8 @@ static int i915_load_modeset_init(struct drm_device *dev) if (ret) goto cleanup_gem_stolen; + intel_setup_gmbus(dev); + /* Important: The output setup functions called by modeset_init need * working irqs for e.g. gmbus and dp aux transfers. */ intel_modeset_init(dev); @@ -452,6 +454,7 @@ cleanup_gem: cleanup_irq: intel_guc_ucode_fini(dev); drm_irq_uninstall(dev); + intel_teardown_gmbus(dev); cleanup_gem_stolen: i915_gem_cleanup_stolen(dev); cleanup_vga_switcheroo: @@ -1026,7 +1029,6 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) /* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev); - intel_setup_gmbus(dev); intel_opregion_setup(dev); i915_gem_load(dev); @@ -1097,7 +1099,6 @@ out_gem_unload: if (dev->pdev->msi_enabled) pci_disable_msi(dev->pdev); - intel_teardown_gmbus(dev); intel_teardown_mchbar(dev); pm_qos_remove_request(&dev_priv->pm_qos); destroy_workqueue(dev_priv->gpu_error.hangcheck_wq); @@ -1196,7 +1197,6 @@ int i915_driver_unload(struct drm_device *dev) intel_csr_ucode_fini(dev_priv); - intel_teardown_gmbus(dev); intel_teardown_mchbar(dev); destroy_workqueue(dev_priv->hotplug.dp_wq); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1ef7153..1e3461f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15821,6 +15821,8 @@ void intel_modeset_cleanup(struct drm_device *dev) mutex_lock(&dev->struct_mutex); intel_cleanup_gt_powersave(dev); mutex_unlock(&dev->struct_mutex); + + intel_teardown_gmbus(dev); } /* -- cgit v0.10.2 From d122cbf1a310d962cef45c2f161b8dc3ec1475a3 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Wed, 13 Jan 2016 22:48:41 +0800 Subject: drm/sysfs: use kobj_to_dev() Use kobj_to_dev() instead of open-coding it. Link: http://patchwork.freedesktop.org/patch/msgid/3fea991541fbfc4ffece2c174adeb02cb9436c90.1452696179.git.geliangtang@163.com Signed-off-by: Geliang Tang diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index 0ca6410..d503f8e 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -240,7 +240,7 @@ static ssize_t edid_show(struct file *filp, struct kobject *kobj, struct bin_attribute *attr, char *buf, loff_t off, size_t count) { - struct device *connector_dev = container_of(kobj, struct device, kobj); + struct device *connector_dev = kobj_to_dev(kobj); struct drm_connector *connector = to_drm_connector(connector_dev); unsigned char *edid; size_t size; -- cgit v0.10.2 From 4314e19ef4ae0ba8872bd8610f6fef5e8743e236 Mon Sep 17 00:00:00 2001 From: Maxime Ripard Date: Thu, 14 Jan 2016 16:24:56 +0100 Subject: drm/fb_cma_helper: Remove implicit call to disable_unused_functions The drm_fbdev_cma_init function always calls the drm_helper_disable_unused_functions. Since it's part of the usual probe process, all the drivers using that helper will end up having their encoder and CRTC disable functions called at probe if their device has not been reported as enabled. This could be fixed by reading out from the registers the current state of the device if it is enabled, but even that will not handle the case where the device is actually disabled. Moreover, the drivers using the atomic modesetting expect that their enable and disable callback to be called when the device is already enabled or disabled (respectively). We can however fix this issue by moving the call to drm_helper_disable_unused_functions out of drm_fbdev_cma_init and make the drivers needing it (all the drivers calling drm_fbdev_cma_init and not using the atomic modesetting) explicitly call it. Signed-off-by: Maxime Ripard Link: http://patchwork.freedesktop.org/patch/msgid/1452785109-6172-14-git-send-email-maxime.ripard@free-electrons.com Acked-by: Laurent Pinchart Signed-off-by: Daniel Vetter diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c b/drivers/gpu/drm/drm_fb_cma_helper.c index 5543fa8..c895b6f 100644 --- a/drivers/gpu/drm/drm_fb_cma_helper.c +++ b/drivers/gpu/drm/drm_fb_cma_helper.c @@ -348,9 +348,6 @@ struct drm_fbdev_cma *drm_fbdev_cma_init(struct drm_device *dev, } - /* disable all the possible outputs/crtcs before entering KMS mode */ - drm_helper_disable_unused_functions(dev); - ret = drm_fb_helper_initial_config(helper, preferred_bpp); if (ret < 0) { dev_err(dev->dev, "Failed to set initial hw configuration.\n"); diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 7be7ac8..2f57d79 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -305,6 +305,7 @@ static int imx_drm_driver_load(struct drm_device *drm, unsigned long flags) dev_warn(drm->dev, "Invalid legacyfb_depth. Defaulting to 16bpp\n"); legacyfb_depth = 16; } + drm_helper_disable_unused_functions(drm); imxdrm->fbhelper = drm_fbdev_cma_init(drm, legacyfb_depth, drm->mode_config.num_crtc, MAX_CRTC); if (IS_ERR(imxdrm->fbhelper)) { diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c index 1469987..506b562 100644 --- a/drivers/gpu/drm/sti/sti_drv.c +++ b/drivers/gpu/drm/sti/sti_drv.c @@ -160,6 +160,7 @@ static int sti_load(struct drm_device *dev, unsigned long flags) drm_mode_config_reset(dev); + drm_helper_disable_unused_functions(dev); drm_fbdev_cma_init(dev, 32, dev->mode_config.num_crtc, dev->mode_config.num_connector); diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 4ddb21e..d7f5b89 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -294,6 +294,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags) break; } + drm_helper_disable_unused_functions(dev); priv->fbdev = drm_fbdev_cma_init(dev, bpp, dev->mode_config.num_crtc, dev->mode_config.num_connector); -- cgit v0.10.2