From e1083ff35157185b01bc0a99cb19b7cbae0fc9fa Mon Sep 17 00:00:00 2001 From: Lyude Date: Wed, 13 Apr 2016 10:58:30 -0400 Subject: [PATCH 01/19] drm/dp_helper: Always wait before retrying native aux transactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper. Some sinks need some time during the process of resuming the system from sleep before they're ready to handle transactions. While it would be nice if they responded with NACKs in these scenarios, this isn't always the case as a few sinks will just timeout on all of the transactions they receive until they're ready. Signed-off-by: Lyude Tested-by: Ville Syrjälä Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1460559513-32280-2-git-send-email-cpaul@redhat.com --- drivers/gpu/drm/drm_dp_helper.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index df64ed1c0139e..7dd330ae0e81c 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -196,6 +196,10 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, * sufficient, bump to 32 which makes Dell 4k monitors happier. */ for (retry = 0; retry < 32; retry++) { + if (err != 0 && err != -ETIMEDOUT) { + usleep_range(AUX_RETRY_INTERVAL, + AUX_RETRY_INTERVAL + 100); + } err = aux->transfer(aux, &msg); if (err < 0) { @@ -205,7 +209,6 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, goto unlock; } - switch (msg.reply & DP_AUX_NATIVE_REPLY_MASK) { case DP_AUX_NATIVE_REPLY_ACK: if (err < size) @@ -215,10 +218,6 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, case DP_AUX_NATIVE_REPLY_NACK: err = -EIO; goto unlock; - - case DP_AUX_NATIVE_REPLY_DEFER: - usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100); - break; } } From 82922da39190199260a726d7081a8ea4873e5fd6 Mon Sep 17 00:00:00 2001 From: Lyude Date: Wed, 13 Apr 2016 10:58:31 -0400 Subject: [PATCH 02/19] drm/dp_helper: Retry aux transactions on all errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper. We cannot rely on sinks NACKing or deferring when they can't receive transactions, nor can we rely on any other sort of consistent error to know when we should stop retrying. As such, we need to just retry unconditionally on errors. We also make sure here to return the error we encountered during the first transaction, since it's possible that retrying the transaction might return a different error then we had originally. This, along with the previous patch, work around a weird bug with the ThinkPad T560's and it's dock. When resuming the laptop, it appears that there's a short period of time where we're unable to complete any aux transactions, as they all immediately timeout. The only machine I'm able to reproduce this on is the T560 as other production Skylake models seem to be fine. The period during which AUX transactions fail appears to be around 22ms long. AFAIK, the dock for the T560 never actually turns off, the only difference is that it's in SST mode at the start of the resume process, so it's unclear as to why it would need so much time to come back up. There's been a discussion on this issue going on for a while on the intel-gfx mailing list about this that has, in addition to including developers from Intel, also had the correspondence of one of the hardware engineers for Intel: http://www.spinics.net/lists/intel-gfx/msg88831.html http://www.spinics.net/lists/intel-gfx/msg88410.html We've already looked into a couple of possible explanations for the problem: - Calling intel_dp_mst_resume() before right fix. intel_runtime_pm_enable_interrupts(). This was the first fix I tried, and while it worked it definitely wasn't the right fix. This worked because DP aux transactions don't actually require interrupts to work: static uint32_t intel_dp_aux_wait_done(struct intel_dp *intel_dp, bool has_aux_irq) { struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct drm_device *dev = intel_dig_port->base.base.dev; struct drm_i915_private *dev_priv = dev->dev_private; i915_reg_t ch_ctl = intel_dp->aux_ch_ctl_reg; uint32_t status; bool done; #define C (((status = I915_READ_NOTRACE(ch_ctl)) & DP_AUX_CH_CTL_SEND_BUSY) == 0) if (has_aux_irq) done = wait_event_timeout(dev_priv->gmbus_wait_queue, C, msecs_to_jiffies_timeout(10)); else done = wait_for_atomic(C, 10) == 0; if (!done) DRM_ERROR("dp aux hw did not signal timeout (has irq: %i)!\n", has_aux_irq); #undef C return status; } When there's no interrupts enabled, we end up timing out on the wait_event_timeout() call, which causes us to check the DP status register once to see if the transaction was successful or not. Since this adds a 10ms delay to each aux transaction, it ends up adding a long enough delay to the resume process for aux transactions to become functional again. This gave us the illusion that enabling interrupts had something to do with making things work again, and put me on the wrong track for a while. - Interrupts occurring when we try to perform the aux transactions required to put the dock back into MST mode. This isn't the problem, as the only interrupts I've observed that come during this timeout period are from the snd_hda_intel driver, and disabling that driver doesn't appear to change the behavior at all. - Skylake's PSR block causing issues by performing aux transactions while we try to bring the dock out of MST mode. Disabling PSR through i915's command line options doesn't seem to change the behavior either, nor does preventing the DMC firmware from being loaded. Since this investigation went on for about 2 weeks, we decided it would be better for the time being to just workaround this issue by making sure AUX transactions wait a short period of time before retrying. Signed-off-by: Lyude Tested-by: Ville Syrjälä Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1460559513-32280-3-git-send-email-cpaul@redhat.com --- drivers/gpu/drm/drm_dp_helper.c | 42 +++++++++++++++++---------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 7dd330ae0e81c..540c3e43a8ea9 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -178,8 +178,8 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, unsigned int offset, void *buffer, size_t size) { struct drm_dp_aux_msg msg; - unsigned int retry; - int err = 0; + unsigned int retry, native_reply; + int err = 0, ret = 0; memset(&msg, 0, sizeof(msg)); msg.address = offset; @@ -196,37 +196,39 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, * sufficient, bump to 32 which makes Dell 4k monitors happier. */ for (retry = 0; retry < 32; retry++) { - if (err != 0 && err != -ETIMEDOUT) { + if (ret != 0 && ret != -ETIMEDOUT) { usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100); } - err = aux->transfer(aux, &msg); - if (err < 0) { - if (err == -EBUSY) - continue; - - goto unlock; - } + ret = aux->transfer(aux, &msg); - switch (msg.reply & DP_AUX_NATIVE_REPLY_MASK) { - case DP_AUX_NATIVE_REPLY_ACK: - if (err < size) - err = -EPROTO; - goto unlock; + if (ret > 0) { + native_reply = msg.reply & DP_AUX_NATIVE_REPLY_MASK; + if (native_reply == DP_AUX_NATIVE_REPLY_ACK) { + if (ret == size) + goto unlock; - case DP_AUX_NATIVE_REPLY_NACK: - err = -EIO; - goto unlock; + ret = -EPROTO; + } else + ret = -EIO; } + + /* + * We want the error we return to be the error we received on + * the first transaction, since we may get a different error the + * next time we retry + */ + if (!err) + err = ret; } DRM_DEBUG_KMS("too many retries, giving up\n"); - err = -EIO; + ret = err; unlock: mutex_unlock(&aux->hw_mutex); - return err; + return ret; } /** From f808f63372cc47119f81d94ec92f9d1b4e21c562 Mon Sep 17 00:00:00 2001 From: Lyude Date: Fri, 15 Apr 2016 10:25:35 -0400 Subject: [PATCH 03/19] drm/dp_helper: Perform throw-away read before actual read in drm_dp_dpcd_read() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is part of a patch series to migrate all of the workarounds for commonly seen behavior from bad sinks in intel_dp_dpcd_read_wake() to drm's DP helper. Some sinks will just return garbage for the first aux tranaction they receive when coming out of sleep mode, so we need to perform an additional read before the actual read to workaround this. Changes since v5 - If the throwaway read in drm_dp_dpcd_read() fails, return the error from that instead of continuing. This follows the same logic we do in drm_dp_dpcd_access() (e.g. the error from the first transaction may differ from the errors that proceeding attempts might return). Signed-off-by: Lyude Tested-by: Ville Syrjälä Reviewed-by: Ville Syrjälä Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1460730335-5012-1-git-send-email-cpaul@redhat.com --- drivers/gpu/drm/drm_dp_helper.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c index 540c3e43a8ea9..eeaf5a7c3aa76 100644 --- a/drivers/gpu/drm/drm_dp_helper.c +++ b/drivers/gpu/drm/drm_dp_helper.c @@ -248,6 +248,25 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request, ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset, void *buffer, size_t size) { + int ret; + + /* + * HP ZR24w corrupts the first DPCD access after entering power save + * mode. Eg. on a read, the entire buffer will be filled with the same + * byte. Do a throw away read to avoid corrupting anything we care + * about. Afterwards things will work correctly until the monitor + * gets woken up and subsequently re-enters power save mode. + * + * The user pressing any button on the monitor is enough to wake it + * up, so there is no particularly good place to do the workaround. + * We just have to do it before any DPCD access and hope that the + * monitor doesn't power down exactly after the throw away read. + */ + ret = drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, DP_DPCD_REV, buffer, + 1); + if (ret != 1) + return ret; + return drm_dp_dpcd_access(aux, DP_AUX_NATIVE_READ, offset, buffer, size); } From 9f085ebb1a50f2728946028548c08860a9005c27 Mon Sep 17 00:00:00 2001 From: Lyude Date: Wed, 13 Apr 2016 10:58:33 -0400 Subject: [PATCH 04/19] drm/i915: Get rid of intel_dp_dpcd_read_wake() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since we've fixed up drm_dp_dpcd_read() to allow for retries when things timeout, there's no use for having this function anymore. Good riddens. Signed-off-by: Lyude Tested-by: Ville Syrjälä Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1460559513-32280-5-git-send-email-cpaul@redhat.com --- drivers/gpu/drm/i915/intel_dp.c | 81 ++++++++++----------------------- 1 file changed, 23 insertions(+), 58 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index da0c3d29fda8e..b52676a032f9b 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3103,37 +3103,6 @@ static void chv_dp_post_pll_disable(struct intel_encoder *encoder) chv_phy_powergate_lanes(encoder, false, 0x0); } -/* - * Native read with retry for link status and receiver capability reads for - * cases where the sink may still be asleep. - * - * Sinks are *supposed* to come up within 1ms from an off state, but we're also - * supposed to retry 3 times per the spec. - */ -static ssize_t -intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, - void *buffer, size_t size) -{ - ssize_t ret; - int i; - - /* - * Sometime we just get the same incorrect byte repeated - * over the entire buffer. Doing just one throw away read - * initially seems to "solve" it. - */ - drm_dp_dpcd_read(aux, DP_DPCD_REV, buffer, 1); - - for (i = 0; i < 3; i++) { - ret = drm_dp_dpcd_read(aux, offset, buffer, size); - if (ret == size) - return ret; - msleep(1); - } - - return ret; -} - /* * Fetch AUX CH registers 0x202 - 0x207 which contain * link status information @@ -3141,10 +3110,8 @@ intel_dp_dpcd_read_wake(struct drm_dp_aux *aux, unsigned int offset, bool intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_STATUS_SIZE]) { - return intel_dp_dpcd_read_wake(&intel_dp->aux, - DP_LANE0_1_STATUS, - link_status, - DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; + return drm_dp_dpcd_read(&intel_dp->aux, DP_LANE0_1_STATUS, link_status, + DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE; } /* These are source-specific values. */ @@ -3779,8 +3746,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) struct drm_i915_private *dev_priv = dev->dev_private; uint8_t rev; - if (intel_dp_dpcd_read_wake(&intel_dp->aux, 0x000, intel_dp->dpcd, - sizeof(intel_dp->dpcd)) < 0) + if (drm_dp_dpcd_read(&intel_dp->aux, 0x000, intel_dp->dpcd, + sizeof(intel_dp->dpcd)) < 0) return false; /* aux transfer failed */ DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), intel_dp->dpcd); @@ -3788,8 +3755,8 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) if (intel_dp->dpcd[DP_DPCD_REV] == 0) return false; /* DPCD not present */ - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_COUNT, - &intel_dp->sink_count, 1) < 0) + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT, + &intel_dp->sink_count, 1) < 0) return false; /* @@ -3812,9 +3779,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) /* Check if the panel supports PSR */ memset(intel_dp->psr_dpcd, 0, sizeof(intel_dp->psr_dpcd)); if (is_edp(intel_dp)) { - intel_dp_dpcd_read_wake(&intel_dp->aux, DP_PSR_SUPPORT, - intel_dp->psr_dpcd, - sizeof(intel_dp->psr_dpcd)); + drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, + intel_dp->psr_dpcd, + sizeof(intel_dp->psr_dpcd)); if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) { dev_priv->psr.sink_support = true; DRM_DEBUG_KMS("Detected EDP PSR Panel.\n"); @@ -3825,9 +3792,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) uint8_t frame_sync_cap; dev_priv->psr.sink_support = true; - intel_dp_dpcd_read_wake(&intel_dp->aux, - DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, - &frame_sync_cap, 1); + drm_dp_dpcd_read(&intel_dp->aux, + DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP, + &frame_sync_cap, 1); dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false; /* PSR2 needs frame sync as well */ dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync; @@ -3843,15 +3810,13 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) /* Intermediate frequency support */ if (is_edp(intel_dp) && (intel_dp->dpcd[DP_EDP_CONFIGURATION_CAP] & DP_DPCD_DISPLAY_CONTROL_CAPABLE) && - (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && + (drm_dp_dpcd_read(&intel_dp->aux, DP_EDP_DPCD_REV, &rev, 1) == 1) && (rev >= 0x03)) { /* eDp v1.4 or higher */ __le16 sink_rates[DP_MAX_SUPPORTED_RATES]; int i; - intel_dp_dpcd_read_wake(&intel_dp->aux, - DP_SUPPORTED_LINK_RATES, - sink_rates, - sizeof(sink_rates)); + drm_dp_dpcd_read(&intel_dp->aux, DP_SUPPORTED_LINK_RATES, + sink_rates, sizeof(sink_rates)); for (i = 0; i < ARRAY_SIZE(sink_rates); i++) { int val = le16_to_cpu(sink_rates[i]); @@ -3874,9 +3839,9 @@ intel_dp_get_dpcd(struct intel_dp *intel_dp) if (intel_dp->dpcd[DP_DPCD_REV] == 0x10) return true; /* no per-port downstream info */ - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, - intel_dp->downstream_ports, - DP_MAX_DOWNSTREAM_PORTS) < 0) + if (drm_dp_dpcd_read(&intel_dp->aux, DP_DOWNSTREAM_PORT_0, + intel_dp->downstream_ports, + DP_MAX_DOWNSTREAM_PORTS) < 0) return false; /* downstream port status fetch failed */ return true; @@ -3890,11 +3855,11 @@ intel_dp_probe_oui(struct intel_dp *intel_dp) if (!(intel_dp->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT)) return; - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) + if (drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_OUI, buf, 3) == 3) DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n", buf[0], buf[1], buf[2]); - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) + if (drm_dp_dpcd_read(&intel_dp->aux, DP_BRANCH_OUI, buf, 3) == 3) DRM_DEBUG_KMS("Branch OUI: %02hx%02hx%02hx\n", buf[0], buf[1], buf[2]); } @@ -3913,7 +3878,7 @@ intel_dp_probe_mst(struct intel_dp *intel_dp) if (intel_dp->dpcd[DP_DPCD_REV] < 0x12) return false; - if (intel_dp_dpcd_read_wake(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { + if (drm_dp_dpcd_read(&intel_dp->aux, DP_MSTM_CAP, buf, 1)) { if (buf[0] & DP_MST_CAP) { DRM_DEBUG_KMS("Sink is MST capable\n"); intel_dp->is_mst = true; @@ -4050,7 +4015,7 @@ int intel_dp_sink_crc(struct intel_dp *intel_dp, u8 *crc) static bool intel_dp_get_sink_irq(struct intel_dp *intel_dp, u8 *sink_irq_vector) { - return intel_dp_dpcd_read_wake(&intel_dp->aux, + return drm_dp_dpcd_read(&intel_dp->aux, DP_DEVICE_SERVICE_IRQ_VECTOR, sink_irq_vector, 1) == 1; } @@ -4060,7 +4025,7 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) { int ret; - ret = intel_dp_dpcd_read_wake(&intel_dp->aux, + ret = drm_dp_dpcd_read(&intel_dp->aux, DP_SINK_COUNT_ESI, sink_irq_vector, 14); if (ret != 14) From c251d85df1d537511a703feb71c6e7c999a95111 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Fri, 22 Apr 2016 17:54:39 +0300 Subject: [PATCH 05/19] drm: rcar-du: Fix compilation warning Commit d63c25e4245a ("drm: rcar-du: Use generic drm_connector_register_all() helper") left an unused local variable behind. Remove it. Fixes: d63c25e4245a ("drm: rcar-du: Use generic drm_connector_register_all() helper") Signed-off-by: Laurent Pinchart Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461336879-2469-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com --- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c b/drivers/gpu/drm/rcar-du/rcar_du_drv.c index 0f251dc11082f..fb9242d27883d 100644 --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c @@ -297,7 +297,6 @@ static int rcar_du_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node; struct rcar_du_device *rcdu; - struct drm_connector *connector; struct drm_device *ddev; struct resource *mem; int ret; From 5ff18e42ddfd9ab67088b2d7081380943b9f0cab Mon Sep 17 00:00:00 2001 From: Emil Velikov Date: Fri, 22 Apr 2016 00:03:52 +0100 Subject: [PATCH 06/19] MAINTAINERS: Update the files list for the GMA500 DRM driver Cc: Patrik Jakobsson Cc: dri-devel@lists.freedesktop.org Signed-off-by: Emil Velikov Acked-by: Patrik Jakobsson Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461279842-28695-5-git-send-email-emil.l.velikov@gmail.com --- MAINTAINERS | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 61a323a6b2cfe..fe87bbe9d2266 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3837,8 +3837,7 @@ M: Patrik Jakobsson L: dri-devel@lists.freedesktop.org T: git git://github.com/patjak/drm-gma500 S: Maintained -F: drivers/gpu/drm/gma500 -F: include/drm/gma500* +F: drivers/gpu/drm/gma500/ DRM DRIVERS FOR NVIDIA TEGRA M: Thierry Reding From 4eb9b945c69fe1ccd6235e21c52627e1b741474a Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 30 Mar 2016 11:45:13 +0200 Subject: [PATCH 07/19] drm/sysfs: Annote lockless show functions with READ_ONCE For documentation and paranoia. Acked-by: Chris Wilson Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1459331120-27864-4-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_sysfs.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c index d7d8cecfb0e69..fa7fadce8063f 100644 --- a/drivers/gpu/drm/drm_sysfs.c +++ b/drivers/gpu/drm/drm_sysfs.c @@ -208,9 +208,12 @@ static ssize_t status_show(struct device *device, char *buf) { struct drm_connector *connector = to_drm_connector(device); + enum drm_connector_status status; + + status = READ_ONCE(connector->status); return snprintf(buf, PAGE_SIZE, "%s\n", - drm_get_connector_status_name(connector->status)); + drm_get_connector_status_name(status)); } static ssize_t dpms_show(struct device *device, @@ -231,9 +234,11 @@ static ssize_t enabled_show(struct device *device, char *buf) { struct drm_connector *connector = to_drm_connector(device); + bool enabled; + + enabled = READ_ONCE(connector->encoder); - return snprintf(buf, PAGE_SIZE, "%s\n", connector->encoder ? "enabled" : - "disabled"); + return snprintf(buf, PAGE_SIZE, enabled ? "enabled\n" : "disabled\n"); } static ssize_t edid_show(struct file *filp, struct kobject *kobj, From 366884b17fae7d2a7517eea60e64d6d6754fa9db Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 26 Apr 2016 19:29:34 +0200 Subject: [PATCH 08/19] drm: Give drm_agp_clear drm_legacy_ prefix It has a DRIVER_MODESET check to sure make it's not creating havoc for drm drivers. Make that clear in the name too. v2: Move misplaced hunk, spotted by 0day and Thierry. Cc: Thierry Reding Reviewed-by: Chris Wilson Acked-by: Alex Deucher Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461691808-12414-2-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_agpsupport.c | 4 ++-- drivers/gpu/drm/drm_fops.c | 2 +- drivers/gpu/drm/drm_pci.c | 2 +- include/drm/drm_agpsupport.h | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_agpsupport.c b/drivers/gpu/drm/drm_agpsupport.c index a10ea6aec6291..605bd243fb36d 100644 --- a/drivers/gpu/drm/drm_agpsupport.c +++ b/drivers/gpu/drm/drm_agpsupport.c @@ -423,7 +423,7 @@ struct drm_agp_head *drm_agp_init(struct drm_device *dev) } /** - * drm_agp_clear - Clear AGP resource list + * drm_legacy_agp_clear - Clear AGP resource list * @dev: DRM device * * Iterate over all AGP resources and remove them. But keep the AGP head @@ -434,7 +434,7 @@ struct drm_agp_head *drm_agp_init(struct drm_device *dev) * resources from getting destroyed. Drivers are responsible of cleaning them up * during device shutdown. */ -void drm_agp_clear(struct drm_device *dev) +void drm_legacy_agp_clear(struct drm_device *dev) { struct drm_agp_mem *entry, *tempe; diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index aeef58ed359b7..7b5a13cda7a60 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -413,7 +413,7 @@ int drm_lastclose(struct drm_device * dev) mutex_lock(&dev->struct_mutex); - drm_agp_clear(dev); + drm_legacy_agp_clear(dev); drm_legacy_sg_cleanup(dev); drm_legacy_vma_flush(dev); diff --git a/drivers/gpu/drm/drm_pci.c b/drivers/gpu/drm/drm_pci.c index a1fff1179a97a..29d5a548d07a7 100644 --- a/drivers/gpu/drm/drm_pci.c +++ b/drivers/gpu/drm/drm_pci.c @@ -250,7 +250,7 @@ void drm_pci_agp_destroy(struct drm_device *dev) { if (dev->agp) { arch_phys_wc_del(dev->agp->agp_mtrr); - drm_agp_clear(dev); + drm_legacy_agp_clear(dev); kfree(dev->agp); dev->agp = NULL; } diff --git a/include/drm/drm_agpsupport.h b/include/drm/drm_agpsupport.h index 193ef19dfc5c5..b2d912670a7fb 100644 --- a/include/drm/drm_agpsupport.h +++ b/include/drm/drm_agpsupport.h @@ -37,7 +37,7 @@ struct agp_memory *drm_agp_bind_pages(struct drm_device *dev, uint32_t type); struct drm_agp_head *drm_agp_init(struct drm_device *dev); -void drm_agp_clear(struct drm_device *dev); +void drm_legacy_agp_clear(struct drm_device *dev); int drm_agp_acquire(struct drm_device *dev); int drm_agp_acquire_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv); @@ -93,7 +93,7 @@ static inline struct drm_agp_head *drm_agp_init(struct drm_device *dev) return NULL; } -static inline void drm_agp_clear(struct drm_device *dev) +static inline void drm_legacy_agp_clear(struct drm_device *dev) { } From 68dfbebab131146b460550cebf8c38667c490a04 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 26 Apr 2016 19:29:35 +0200 Subject: [PATCH 09/19] drm: Put legacy lastclose work into drm_legacy_dev_reinit Except for the ->lasclose driver callback evrything in drm_lastclose() is all legacy cruft and can be hidden. Which means another dev->struct_mutex site disappears entirely for modern drivers! Also while at it change the return value of drm_lastclose to void since it will always succeed. No one checks the return value of close() anyway, ever. v2: Move misplaced hunk, spotted by 0day. Cc: Thierry Reding Reviewed-by: Alex Deucher Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461691808-12414-3-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_fops.c | 42 +++++++++++++++------------------- drivers/gpu/drm/drm_internal.h | 2 +- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index 7b5a13cda7a60..c3d0aaac0669e 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -381,14 +381,26 @@ static void drm_events_release(struct drm_file *file_priv) */ static void drm_legacy_dev_reinit(struct drm_device *dev) { - if (drm_core_check_feature(dev, DRIVER_MODESET)) - return; + if (dev->irq_enabled) + drm_irq_uninstall(dev); + + mutex_lock(&dev->struct_mutex); + + drm_legacy_agp_clear(dev); + + drm_legacy_sg_cleanup(dev); + drm_legacy_vma_flush(dev); + drm_legacy_dma_takedown(dev); + + mutex_unlock(&dev->struct_mutex); dev->sigdata.lock = NULL; dev->context_flag = 0; dev->last_context = 0; dev->if_version = 0; + + DRM_DEBUG("lastclose completed\n"); } /* @@ -400,7 +412,7 @@ static void drm_legacy_dev_reinit(struct drm_device *dev) * * \sa drm_device */ -int drm_lastclose(struct drm_device * dev) +void drm_lastclose(struct drm_device * dev) { DRM_DEBUG("\n"); @@ -408,23 +420,8 @@ int drm_lastclose(struct drm_device * dev) dev->driver->lastclose(dev); DRM_DEBUG("driver lastclose completed\n"); - if (dev->irq_enabled && !drm_core_check_feature(dev, DRIVER_MODESET)) - drm_irq_uninstall(dev); - - mutex_lock(&dev->struct_mutex); - - drm_legacy_agp_clear(dev); - - drm_legacy_sg_cleanup(dev); - drm_legacy_vma_flush(dev); - drm_legacy_dma_takedown(dev); - - mutex_unlock(&dev->struct_mutex); - - drm_legacy_dev_reinit(dev); - - DRM_DEBUG("lastclose completed\n"); - return 0; + if (!drm_core_check_feature(dev, DRIVER_MODESET)) + drm_legacy_dev_reinit(dev); } /** @@ -445,7 +442,6 @@ int drm_release(struct inode *inode, struct file *filp) struct drm_file *file_priv = filp->private_data; struct drm_minor *minor = file_priv->minor; struct drm_device *dev = minor->dev; - int retcode = 0; mutex_lock(&drm_global_mutex); @@ -538,7 +534,7 @@ int drm_release(struct inode *inode, struct file *filp) */ if (!--dev->open_count) { - retcode = drm_lastclose(dev); + drm_lastclose(dev); if (drm_device_is_unplugged(dev)) drm_put_dev(dev); } @@ -546,7 +542,7 @@ int drm_release(struct inode *inode, struct file *filp) drm_minor_release(minor); - return retcode; + return 0; } EXPORT_SYMBOL(drm_release); diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index 43cbda3306ac0..c81ff4769e7be 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -26,7 +26,7 @@ extern unsigned int drm_timestamp_monotonic; /* drm_fops.c */ extern struct mutex drm_global_mutex; -int drm_lastclose(struct drm_device *dev); +void drm_lastclose(struct drm_device *dev); /* drm_pci.c */ int drm_pci_set_unique(struct drm_device *dev, From ec1f52efc0406a72870a1fcb859f18dea24bcd8b Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 26 Apr 2016 19:29:36 +0200 Subject: [PATCH 10/19] drm: Move drm_getmap into drm_bufs.c and give it a legacy prefix It belongs right next to the addmap and rmmap functions really. And for OCD consistency name it drm_legacy_getmap_ioctl. Reviewed-by: Chris Wilson Reviewed-by: Alex Deucher Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461691808-12414-4-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_bufs.c | 52 ++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_ioctl.c | 54 +----------------------------------- drivers/gpu/drm/drm_legacy.h | 2 ++ 3 files changed, 55 insertions(+), 53 deletions(-) diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index f1a204d253cce..d92db7007f62c 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -416,6 +416,58 @@ int drm_legacy_addmap_ioctl(struct drm_device *dev, void *data, return 0; } +/* + * Get a mapping information. + * + * \param inode device inode. + * \param file_priv DRM file private. + * \param cmd command. + * \param arg user argument, pointing to a drm_map structure. + * + * \return zero on success or a negative number on failure. + * + * Searches for the mapping with the specified offset and copies its information + * into userspace + */ +int drm_legacy_getmap_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_map *map = data; + struct drm_map_list *r_list = NULL; + struct list_head *list; + int idx; + int i; + + idx = map->offset; + if (idx < 0) + return -EINVAL; + + i = 0; + mutex_lock(&dev->struct_mutex); + list_for_each(list, &dev->maplist) { + if (i == idx) { + r_list = list_entry(list, struct drm_map_list, head); + break; + } + i++; + } + if (!r_list || !r_list->map) { + mutex_unlock(&dev->struct_mutex); + return -EINVAL; + } + + map->offset = r_list->map->offset; + map->size = r_list->map->size; + map->type = r_list->map->type; + map->flags = r_list->map->flags; + map->handle = (void *)(unsigned long) r_list->user_token; + map->mtrr = arch_phys_wc_index(r_list->map->mtrr); + + mutex_unlock(&dev->struct_mutex); + + return 0; +} + /** * Remove a map private from list and deallocate resources if the mapping * isn't in use. diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 8ce2a0c591165..b7a39771c152e 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -149,58 +149,6 @@ static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv) return 0; } -/* - * Get a mapping information. - * - * \param inode device inode. - * \param file_priv DRM file private. - * \param cmd command. - * \param arg user argument, pointing to a drm_map structure. - * - * \return zero on success or a negative number on failure. - * - * Searches for the mapping with the specified offset and copies its information - * into userspace - */ -static int drm_getmap(struct drm_device *dev, void *data, - struct drm_file *file_priv) -{ - struct drm_map *map = data; - struct drm_map_list *r_list = NULL; - struct list_head *list; - int idx; - int i; - - idx = map->offset; - if (idx < 0) - return -EINVAL; - - i = 0; - mutex_lock(&dev->struct_mutex); - list_for_each(list, &dev->maplist) { - if (i == idx) { - r_list = list_entry(list, struct drm_map_list, head); - break; - } - i++; - } - if (!r_list || !r_list->map) { - mutex_unlock(&dev->struct_mutex); - return -EINVAL; - } - - map->offset = r_list->map->offset; - map->size = r_list->map->size; - map->type = r_list->map->type; - map->flags = r_list->map->flags; - map->handle = (void *)(unsigned long) r_list->user_token; - map->mtrr = arch_phys_wc_index(r_list->map->mtrr); - - mutex_unlock(&dev->struct_mutex); - - return 0; -} - /* * Get client information. * @@ -558,7 +506,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_IOCTL_DEF(DRM_IOCTL_GET_UNIQUE, drm_getunique, 0), DRM_IOCTL_DEF(DRM_IOCTL_GET_MAGIC, drm_getmagic, 0), DRM_IOCTL_DEF(DRM_IOCTL_IRQ_BUSID, drm_irq_by_busid, DRM_MASTER|DRM_ROOT_ONLY), - DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_getmap, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_GET_MAP, drm_legacy_getmap_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_CLIENT, drm_getclient, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_STATS, drm_getstats, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_GET_CAP, drm_getcap, DRM_UNLOCKED|DRM_RENDER_ALLOW), diff --git a/drivers/gpu/drm/drm_legacy.h b/drivers/gpu/drm/drm_legacy.h index 9b731786e4db2..d3b6ee357a2b4 100644 --- a/drivers/gpu/drm/drm_legacy.h +++ b/drivers/gpu/drm/drm_legacy.h @@ -63,6 +63,8 @@ int drm_legacy_getsareactx(struct drm_device *d, void *v, struct drm_file *f); #define DRM_MAP_HASH_OFFSET 0x10000000 +int drm_legacy_getmap_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv); int drm_legacy_addmap_ioctl(struct drm_device *d, void *v, struct drm_file *f); int drm_legacy_rmmap_ioctl(struct drm_device *d, void *v, struct drm_file *f); int drm_legacy_addbufs(struct drm_device *d, void *v, struct drm_file *f); From 0d787b143d83de94decb801701a005ffd3ec6823 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 26 Apr 2016 19:29:38 +0200 Subject: [PATCH 11/19] drm: Push struct_mutex into ->master_destroy Only two drivers implement this hook. vmwgfx (which doesn't need it really) and legacy radeon (which since v1 has been nuked, yay). v1: Rebase over radeon ums removal. Cc: Thomas Hellstrom Cc: Alex Deucher Reviewed-by: Chris Wilson Reviewed-by: Alex Deucher Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461691808-12414-6-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index f8a7a6e66b7e2..55273f8f3acbc 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -123,10 +123,10 @@ static void drm_master_destroy(struct kref *kref) struct drm_device *dev = master->minor->dev; struct drm_map_list *r_list, *list_temp; - mutex_lock(&dev->struct_mutex); if (dev->driver->master_destroy) dev->driver->master_destroy(dev, master); + mutex_lock(&dev->struct_mutex); list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head) { if (r_list->master == master) { drm_legacy_rmmap_locked(dev, r_list->map); From e975eef07cbca53691f504318609bb95c7e5f1ac Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 26 Apr 2016 19:29:37 +0200 Subject: [PATCH 12/19] drm: Forbid legacy MAP functions for DRIVER_MODESET Like in commit 0e975980d435d58df2d430d688b8c18778b42218 Author: Peter Antoine Date: Tue Jun 23 08:18:49 2015 +0100 drm: Turn off Legacy Context Functions we need to again make an exception for nouveau, but everyone else really doesn't need this. Dave Airlie dug out again why we need this: The problem is the legacy dri1 open function the nouveau ddx called, and the problematic code is actually in the X server itself. It was only fixed in commit b1a630b48210d6a3c44994fce1b73273000ace5c Author: Dave Airlie Date: Wed Nov 7 14:45:14 2012 +1000 nouveau: drop DRI1 device open interface. Cc: Peter Antoine Cc: Ben Skeggs Acked-by: Alex Deucher Acked-by: Dave Airlie Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461691808-12414-5-git-send-email-daniel.vetter@ffwll.ch Link: http://patchwork.freedesktop.org/patch/msgid/1461691808-12414-6-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_bufs.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index d92db7007f62c..e8a12a4fd4007 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -396,6 +396,10 @@ int drm_legacy_addmap_ioctl(struct drm_device *dev, void *data, if (!(capable(CAP_SYS_ADMIN) || map->type == _DRM_AGP || map->type == _DRM_SHM)) return -EPERM; + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) && + drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + err = drm_addmap_core(dev, map->offset, map->size, map->type, map->flags, &maplist); @@ -438,6 +442,10 @@ int drm_legacy_getmap_ioctl(struct drm_device *dev, void *data, int idx; int i; + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) && + drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + idx = map->offset; if (idx < 0) return -EINVAL; @@ -569,6 +577,10 @@ int drm_legacy_rmmap_ioctl(struct drm_device *dev, void *data, struct drm_map_list *r_list; int ret; + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) && + drm_core_check_feature(dev, DRIVER_MODESET)) + return -EINVAL; + mutex_lock(&dev->struct_mutex); list_for_each_entry(r_list, &dev->maplist, head) { if (r_list->map && From 40647e45b92b1da599048332ec8fbd40d8d29457 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 27 Apr 2016 09:20:18 +0200 Subject: [PATCH 13/19] drm: Hide master MAP cleanup in drm_bufs.c And again make sure it's a no-op for modern drivers. Another case of dev->struct_mutex gone for modern drivers! Note that the entirety of the legacy addmap interface is now protected by DRIVER_MODESET. Note that just auditing kernel code is not enough, since userspace loves to set up legacy maps on it's own for various things - with ums userspace and kernel space share control over resources. v2: Also add a DRIVER_* check like for all other maps functions to really short-circuit the code. And give drm_legacy_rmmap used by the dev unregister code the same treatment. v3: - remove redundant return; (Alex, Chris) - don't special case nouveau with DRIVER_KMS_LEGACY_CONTEXT. v4: Again special case nouveau. The problem is not directly in the ddx, but that it calls dri1 functions from the X server. And those do call drmAddMap. Fixed only in commit b1a630b48210d6a3c44994fce1b73273000ace5c Author: Dave Airlie Date: Wed Nov 7 14:45:14 2012 +1000 nouveau: drop DRI1 device open interface. Acked-by: Alex Deucher Reviewed-by: Chris Wilson Cc: Alex Deucher Cc: Chris Wilson Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461741618-12679-1-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_bufs.c | 27 ++++++++++++++++++++++----- drivers/gpu/drm/drm_drv.c | 10 +--------- include/drm/drm_legacy.h | 4 +++- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/drm_bufs.c b/drivers/gpu/drm/drm_bufs.c index e8a12a4fd4007..9b34158c0f77b 100644 --- a/drivers/gpu/drm/drm_bufs.c +++ b/drivers/gpu/drm/drm_bufs.c @@ -542,18 +542,35 @@ int drm_legacy_rmmap_locked(struct drm_device *dev, struct drm_local_map *map) } EXPORT_SYMBOL(drm_legacy_rmmap_locked); -int drm_legacy_rmmap(struct drm_device *dev, struct drm_local_map *map) +void drm_legacy_rmmap(struct drm_device *dev, struct drm_local_map *map) { - int ret; + if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) && + drm_core_check_feature(dev, DRIVER_MODESET)) + return; mutex_lock(&dev->struct_mutex); - ret = drm_legacy_rmmap_locked(dev, map); + drm_legacy_rmmap_locked(dev, map); mutex_unlock(&dev->struct_mutex); - - return ret; } EXPORT_SYMBOL(drm_legacy_rmmap); +void drm_legacy_master_rmmaps(struct drm_device *dev, struct drm_master *master) +{ + struct drm_map_list *r_list, *list_temp; + + if (drm_core_check_feature(dev, DRIVER_MODESET)) + return; + + mutex_lock(&dev->struct_mutex); + list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head) { + if (r_list->master == master) { + drm_legacy_rmmap_locked(dev, r_list->map); + r_list = NULL; + } + } + mutex_unlock(&dev->struct_mutex); +} + /* The rmmap ioctl appears to be unnecessary. All mappings are torn down on * the last close of the device, and this is necessary for cleanup when things * exit uncleanly. Therefore, having userland manually remove mappings seems diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index 55273f8f3acbc..e1fb52d4f72cc 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -121,19 +121,11 @@ static void drm_master_destroy(struct kref *kref) { struct drm_master *master = container_of(kref, struct drm_master, refcount); struct drm_device *dev = master->minor->dev; - struct drm_map_list *r_list, *list_temp; if (dev->driver->master_destroy) dev->driver->master_destroy(dev, master); - mutex_lock(&dev->struct_mutex); - list_for_each_entry_safe(r_list, list_temp, &dev->maplist, head) { - if (r_list->master == master) { - drm_legacy_rmmap_locked(dev, r_list->map); - r_list = NULL; - } - } - mutex_unlock(&dev->struct_mutex); + drm_legacy_master_rmmaps(dev, master); idr_destroy(&master->magic_map); kfree(master->unique); diff --git a/include/drm/drm_legacy.h b/include/drm/drm_legacy.h index 3e698038dc7bf..a5ef2c7e40f8e 100644 --- a/include/drm/drm_legacy.h +++ b/include/drm/drm_legacy.h @@ -154,8 +154,10 @@ struct drm_map_list { int drm_legacy_addmap(struct drm_device *d, resource_size_t offset, unsigned int size, enum drm_map_type type, enum drm_map_flags flags, struct drm_local_map **map_p); -int drm_legacy_rmmap(struct drm_device *d, struct drm_local_map *map); +void drm_legacy_rmmap(struct drm_device *d, struct drm_local_map *map); int drm_legacy_rmmap_locked(struct drm_device *d, struct drm_local_map *map); +void drm_legacy_master_rmmaps(struct drm_device *dev, + struct drm_master *master); struct drm_local_map *drm_legacy_getsarea(struct drm_device *dev); int drm_legacy_mmap(struct file *filp, struct vm_area_struct *vma); From f47dbdd75bf3388ce8107c2e7e5a1eebd3700dd5 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 26 Apr 2016 19:29:40 +0200 Subject: [PATCH 14/19] drm: Make drm_vm_open/close_locked private to drm_vm.c It's only used for legacy mmaping support now. Reviewed-by: Alex Deucher Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461691808-12414-8-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/drm_internal.h | 2 -- drivers/gpu/drm/drm_vm.c | 16 ++++------------ 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index c81ff4769e7be..902cf6a152128 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -37,8 +37,6 @@ int drm_irq_by_busid(struct drm_device *dev, void *data, /* drm_vm.c */ int drm_vma_info(struct seq_file *m, void *data); -void drm_vm_open_locked(struct drm_device *dev, struct vm_area_struct *vma); -void drm_vm_close_locked(struct drm_device *dev, struct vm_area_struct *vma); /* drm_prime.c */ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/drm_vm.c b/drivers/gpu/drm/drm_vm.c index f90bd5fe35bab..ac9f4b3ec615c 100644 --- a/drivers/gpu/drm/drm_vm.c +++ b/drivers/gpu/drm/drm_vm.c @@ -395,16 +395,8 @@ static const struct vm_operations_struct drm_vm_sg_ops = { .close = drm_vm_close, }; -/** - * \c open method for shared virtual memory. - * - * \param vma virtual memory area. - * - * Create a new drm_vma_entry structure as the \p vma private data entry and - * add it to drm_device::vmalist. - */ -void drm_vm_open_locked(struct drm_device *dev, - struct vm_area_struct *vma) +static void drm_vm_open_locked(struct drm_device *dev, + struct vm_area_struct *vma) { struct drm_vma_entry *vma_entry; @@ -429,8 +421,8 @@ static void drm_vm_open(struct vm_area_struct *vma) mutex_unlock(&dev->struct_mutex); } -void drm_vm_close_locked(struct drm_device *dev, - struct vm_area_struct *vma) +static void drm_vm_close_locked(struct drm_device *dev, + struct vm_area_struct *vma) { struct drm_vma_entry *pt, *temp; From 1d2ac403ae3bfde7c50328ee0d39d3fb3d8d9823 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Tue, 26 Apr 2016 19:29:41 +0200 Subject: [PATCH 15/19] drm: Protect dev->filelist with its own mutex amdgpu gained dev->struct_mutex usage, and that's because it's walking the dev->filelist list. Protect that list with it's own lock to take one more step towards getting rid of struct_mutex usage in drivers once and for all. While doing the conversion I noticed that 2 debugfs files in i915 completely lacked appropriate locking. Fix that up too. v2: don't forget to switch to drm_gem_object_unreference_unlocked. Cc: Alex Deucher Reviewed-by: Alex Deucher Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461691808-12414-9-git-send-email-daniel.vetter@ffwll.ch --- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 10 +++++----- drivers/gpu/drm/drm_drv.c | 1 + drivers/gpu/drm/drm_fops.c | 9 ++++++--- drivers/gpu/drm/drm_info.c | 4 ++-- drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++++++++-- include/drm/drmP.h | 1 + 6 files changed, 25 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index fa6a27bff298b..a087b9638cded 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -93,7 +93,7 @@ void amdgpu_gem_force_release(struct amdgpu_device *adev) struct drm_device *ddev = adev->ddev; struct drm_file *file; - mutex_lock(&ddev->struct_mutex); + mutex_lock(&ddev->filelist_mutex); list_for_each_entry(file, &ddev->filelist, lhead) { struct drm_gem_object *gobj; @@ -103,13 +103,13 @@ void amdgpu_gem_force_release(struct amdgpu_device *adev) spin_lock(&file->table_lock); idr_for_each_entry(&file->object_idr, gobj, handle) { WARN_ONCE(1, "And also active allocations!\n"); - drm_gem_object_unreference(gobj); + drm_gem_object_unreference_unlocked(gobj); } idr_destroy(&file->object_idr); spin_unlock(&file->table_lock); } - mutex_unlock(&ddev->struct_mutex); + mutex_unlock(&ddev->filelist_mutex); } /* @@ -769,7 +769,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data) struct drm_file *file; int r; - r = mutex_lock_interruptible(&dev->struct_mutex); + r = mutex_lock_interruptible(&dev->filelist_mutex); if (r) return r; @@ -793,7 +793,7 @@ static int amdgpu_debugfs_gem_info(struct seq_file *m, void *data) spin_unlock(&file->table_lock); } - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&dev->filelist_mutex); return 0; } diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c index e1fb52d4f72cc..bff89226a344b 100644 --- a/drivers/gpu/drm/drm_drv.c +++ b/drivers/gpu/drm/drm_drv.c @@ -590,6 +590,7 @@ struct drm_device *drm_dev_alloc(struct drm_driver *driver, spin_lock_init(&dev->buf_lock); spin_lock_init(&dev->event_lock); mutex_init(&dev->struct_mutex); + mutex_init(&dev->filelist_mutex); mutex_init(&dev->ctxlist_mutex); mutex_init(&dev->master_mutex); diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c index c3d0aaac0669e..7af7f8bcb3558 100644 --- a/drivers/gpu/drm/drm_fops.c +++ b/drivers/gpu/drm/drm_fops.c @@ -297,9 +297,9 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor) } mutex_unlock(&dev->master_mutex); - mutex_lock(&dev->struct_mutex); + mutex_lock(&dev->filelist_mutex); list_add(&priv->lhead, &dev->filelist); - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&dev->filelist_mutex); #ifdef __alpha__ /* @@ -447,8 +447,11 @@ int drm_release(struct inode *inode, struct file *filp) DRM_DEBUG("open_count = %d\n", dev->open_count); - mutex_lock(&dev->struct_mutex); + mutex_lock(&dev->filelist_mutex); list_del(&file_priv->lhead); + mutex_unlock(&dev->filelist_mutex); + + mutex_lock(&dev->struct_mutex); if (file_priv->magic) idr_remove(&file_priv->master->magic_map, file_priv->magic); mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/drm_info.c b/drivers/gpu/drm/drm_info.c index cbb4fc0fc969e..5d469b2f26f46 100644 --- a/drivers/gpu/drm/drm_info.c +++ b/drivers/gpu/drm/drm_info.c @@ -174,7 +174,7 @@ int drm_clients_info(struct seq_file *m, void *data) /* dev->filelist is sorted youngest first, but we want to present * oldest first (i.e. kernel, servers, clients), so walk backwardss. */ - mutex_lock(&dev->struct_mutex); + mutex_lock(&dev->filelist_mutex); list_for_each_entry_reverse(priv, &dev->filelist, lhead) { struct task_struct *task; @@ -190,7 +190,7 @@ int drm_clients_info(struct seq_file *m, void *data) priv->magic); rcu_read_unlock(); } - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&dev->filelist_mutex); return 0; } diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 85933940fe9c7..c298312f83f6b 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -498,6 +498,10 @@ static int i915_gem_object_info(struct seq_file *m, void* data) seq_putc(m, '\n'); print_batch_pool_stats(m, dev_priv); + + mutex_unlock(&dev->struct_mutex); + + mutex_lock(&dev->filelist_mutex); list_for_each_entry_reverse(file, &dev->filelist, lhead) { struct file_stats stats; struct task_struct *task; @@ -518,8 +522,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data) print_file_stats(m, task ? task->comm : "", stats); rcu_read_unlock(); } - - mutex_unlock(&dev->struct_mutex); + mutex_unlock(&dev->filelist_mutex); return 0; } @@ -2325,6 +2328,7 @@ static int i915_ppgtt_info(struct seq_file *m, void *data) else if (INTEL_INFO(dev)->gen >= 6) gen6_ppgtt_info(m, dev); + mutex_lock(&dev->filelist_mutex); list_for_each_entry_reverse(file, &dev->filelist, lhead) { struct drm_i915_file_private *file_priv = file->driver_priv; struct task_struct *task; @@ -2339,6 +2343,7 @@ static int i915_ppgtt_info(struct seq_file *m, void *data) idr_for_each(&file_priv->context_idr, per_file_ctx, (void *)(unsigned long)m); } + mutex_unlock(&dev->filelist_mutex); out_put: intel_runtime_pm_put(dev_priv); @@ -2374,6 +2379,8 @@ static int i915_rps_boost_info(struct seq_file *m, void *data) intel_gpu_freq(dev_priv, dev_priv->rps.min_freq_softlimit), intel_gpu_freq(dev_priv, dev_priv->rps.max_freq_softlimit), intel_gpu_freq(dev_priv, dev_priv->rps.max_freq)); + + mutex_lock(&dev->filelist_mutex); spin_lock(&dev_priv->rps.client_lock); list_for_each_entry_reverse(file, &dev->filelist, lhead) { struct drm_i915_file_private *file_priv = file->driver_priv; @@ -2396,6 +2403,7 @@ static int i915_rps_boost_info(struct seq_file *m, void *data) list_empty(&dev_priv->rps.mmioflips.link) ? "" : ", active"); seq_printf(m, "Kernel boosts: %d\n", dev_priv->rps.boosts); spin_unlock(&dev_priv->rps.client_lock); + mutex_unlock(&dev->filelist_mutex); return 0; } diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 5de4cff05ac94..7901768bb47c2 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -769,6 +769,7 @@ struct drm_device { atomic_t buf_alloc; /**< Buffer allocation in progress */ /*@} */ + struct mutex filelist_mutex; struct list_head filelist; /** \name Memory management */ From 3849bef34d1a88e0775925f2d11d79717f4b082e Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Wed, 27 Apr 2016 11:07:02 +0100 Subject: [PATCH 16/19] drm: Quiet down drm_mode_getconnector Debug logging in this function does not provide any information apart that the userspace is calling an ioctl on the connector. There is not any info on the connector provided at all and since there are other ioctls userspace typically calls which do log useful things about the same connectors, remove this one to make things a little bit more readable when KMS debugging is turned on. Signed-off-by: Tvrtko Ursulin Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461751622-26927-10-git-send-email-tvrtko.ursulin@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index d078a5c34d48a..5c13f3127b566 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -2142,8 +2142,6 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, memset(&u_mode, 0, sizeof(struct drm_mode_modeinfo)); - DRM_DEBUG_KMS("[CONNECTOR:%d:?]\n", out_resp->connector_id); - mutex_lock(&dev->mode_config.mutex); connector = drm_connector_find(dev, out_resp->connector_id); From 676fb3240df3e12d55bbf29bff032bf9494bca28 Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Wed, 27 Apr 2016 12:11:47 +0100 Subject: [PATCH 17/19] drm: Quiet down drm_mode_getresources The debug logging here can be very verbose in the kernel logs and provides no information which userspace doesn't have the access to already. Turn it off so kernel logs become more manageable. Signed-off-by: Tvrtko Ursulin Cc: Daniel Vetter Cc: dri-devel@lists.freedesktop.org Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461755507-30453-1-git-send-email-tvrtko.ursulin@linux.intel.com --- drivers/gpu/drm/drm_crtc.c | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 5c13f3127b566..7d1960e4b23f0 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -1935,8 +1935,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, copied = 0; crtc_id = (uint32_t __user *)(unsigned long)card_res->crtc_id_ptr; drm_for_each_crtc(crtc, dev) { - DRM_DEBUG_KMS("[CRTC:%d:%s]\n", - crtc->base.id, crtc->name); if (put_user(crtc->base.id, crtc_id + copied)) { ret = -EFAULT; goto out; @@ -1951,8 +1949,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, copied = 0; encoder_id = (uint32_t __user *)(unsigned long)card_res->encoder_id_ptr; drm_for_each_encoder(encoder, dev) { - DRM_DEBUG_KMS("[ENCODER:%d:%s]\n", encoder->base.id, - encoder->name); if (put_user(encoder->base.id, encoder_id + copied)) { ret = -EFAULT; @@ -1968,9 +1964,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, copied = 0; connector_id = (uint32_t __user *)(unsigned long)card_res->connector_id_ptr; drm_for_each_connector(connector, dev) { - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", - connector->base.id, - connector->name); if (put_user(connector->base.id, connector_id + copied)) { ret = -EFAULT; @@ -1981,9 +1974,6 @@ int drm_mode_getresources(struct drm_device *dev, void *data, } card_res->count_connectors = connector_count; - DRM_DEBUG_KMS("CRTC[%d] CONNECTORS[%d] ENCODERS[%d]\n", card_res->count_crtcs, - card_res->count_connectors, card_res->count_encoders); - out: mutex_unlock(&dev->mode_config.mutex); return ret; From 36230cb5668c2633bee4ec87b58983eac3a5cb4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Wed, 27 Apr 2016 22:43:45 +0300 Subject: [PATCH 18/19] drm/dp: Allow signals to interrupt drm_aux-dev reads/writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's be nice and interrupt the dpcd aux-dev reads/writes when there's a signal pending. Much nicer if the user can hit ^C instead of having to sit around waiting for the read/write to finish. time dd if=/dev/drm_dp_aux0 bs=$((1024*1024)) ^C before: real 0m34.681s user 0m0.003s sys 0m6.880s after: real 0m0.222s user 0m0.006s sys 0m0.057s Cc: Rafael Antognolli Signed-off-by: Ville Syrjälä Reviewed-by: Jani Nikula Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461786225-7790-1-git-send-email-ville.syrjala@linux.intel.com --- drivers/gpu/drm/drm_dp_aux_dev.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c b/drivers/gpu/drm/drm_dp_aux_dev.c index f73b38b33a8eb..3334baacf43d8 100644 --- a/drivers/gpu/drm/drm_dp_aux_dev.c +++ b/drivers/gpu/drm/drm_dp_aux_dev.c @@ -159,6 +159,12 @@ static ssize_t auxdev_read(struct file *file, char __user *buf, size_t count, uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES]; ssize_t todo = min_t(size_t, bytes_pending, sizeof(localbuf)); + if (signal_pending(current)) { + res = num_bytes_processed ? + num_bytes_processed : -ERESTARTSYS; + goto out; + } + res = drm_dp_dpcd_read(aux_dev->aux, *offset, localbuf, todo); if (res <= 0) { res = num_bytes_processed ? num_bytes_processed : res; @@ -202,6 +208,12 @@ static ssize_t auxdev_write(struct file *file, const char __user *buf, uint8_t localbuf[DP_AUX_MAX_PAYLOAD_BYTES]; ssize_t todo = min_t(size_t, bytes_pending, sizeof(localbuf)); + if (signal_pending(current)) { + res = num_bytes_processed ? + num_bytes_processed : -ERESTARTSYS; + goto out; + } + if (__copy_from_user(localbuf, buf + num_bytes_processed, todo)) { res = num_bytes_processed ? From be35f94f5c83bfed9ded918162c9b622743ef520 Mon Sep 17 00:00:00 2001 From: Thierry Reding Date: Thu, 28 Apr 2016 15:19:56 +0200 Subject: [PATCH 19/19] drm/atomic: Add missing drm_crtc_internal.h include Some of the functions implemented are flagged as not having a prototype defined when building with W=1. Include the header to avoid these build warnings. Signed-off-by: Thierry Reding Signed-off-by: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/1461849596-12819-1-git-send-email-thierry.reding@gmail.com --- drivers/gpu/drm/drm_atomic.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 8ee1db866e800..2fabd8c4fd88f 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -31,6 +31,8 @@ #include #include +#include "drm_crtc_internal.h" + /** * drm_atomic_state_default_release - * release memory initialized by drm_atomic_state_init