From patchwork Wed Sep 7 17:38:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Fortescue X-Patchwork-Id: 9320371 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3873D6077F for ; Thu, 8 Sep 2016 03:53:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2475E29494 for ; Thu, 8 Sep 2016 03:53:21 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 194BD29558; Thu, 8 Sep 2016 03:53:21 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 2FF1D29494 for ; Thu, 8 Sep 2016 03:53:20 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id ECF6B6ECDE; Thu, 8 Sep 2016 03:53:10 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org X-Greylist: delayed 6075 seconds by postgrey-1.35 at gabe; Wed, 07 Sep 2016 19:20:14 UTC Received: from smtp.hosts.co.uk (smtp.hosts.co.uk [85.233.160.19]) by gabe.freedesktop.org (Postfix) with ESMTPS id 6572C6EBDD for ; Wed, 7 Sep 2016 19:20:14 +0000 (UTC) Received: from [195.147.1.22] (helo=alpha.thurning-instruments.co.uk) by smtp.hosts.co.uk with esmtpsa (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.87) (envelope-from ) id 1bhgoL-0005Ev-9I; Wed, 07 Sep 2016 18:38:58 +0100 Received: from [10.1.1.7] (mtfhpc.demon.co.uk [83.104.139.140]) (authenticated bits=0) by alpha.thurning-instruments.co.uk (8.14.4/8.14.3/Debian-9.1ubuntu1) with ESMTP id u87HcnkO009277 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=OK); Wed, 7 Sep 2016 17:38:55 GMT From: Mark Fortescue Subject: [PATCH 001/001] drivers/gpu/radeon: NULL pointer deference workaround To: David Airlie Message-ID: <702d22fa-c229-3cc2-e1b3-d26f78dc0f52@thurning-instruments.co.uk> Date: Wed, 7 Sep 2016 18:38:44 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 X-Mailman-Approved-At: Thu, 08 Sep 2016 03:53:09 +0000 Cc: Alex Deucher , =?UTF-8?Q?Christian_K=c3=b6nig?= , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP On an LV-683 (AMD Dual-core G-T56N) Mini-ITX board, I get a Kernel Oops because Connector 0 (LCD Panel interface) does not have DDC. Ubuntu 16.04 LTS Kernel (4.4 series): ... [ 8.262990] [drm] ib test on ring 5 succeeded [ 8.288897] [drm] Radeon Display Connectors [ 8.293175] [drm] Connector 0: [ 8.296252] [drm] DP-1 [ 8.298791] [drm] Encoders: [ 8.301770] [drm] DFP1: INTERNAL_UNIPHY [ 8.305973] [drm] Connector 1: [ 8.309043] [drm] DP-2 [ 8.311598] [drm] HPD2 [ 8.314169] [drm] DDC: 0x6440 0x6440 0x6444 0x6444 0x6448 0x6448 0x644c 0x644c [ 8.321609] [drm] Encoders: [ 8.324589] [drm] DFP2: INTERNAL_UNIPHY [ 8.328793] [drm] Connector 2: [ 8.331856] [drm] VGA-1 [ 8.342947] [drm] DDC: 0x64d8 0x64d8 0x64dc 0x64dc 0x64e0 0x64e0 0x64e4 0x64e4 [ 8.350341] [drm] Encoders: [ 8.353310] [drm] CRT1: INTERNAL_KLDSCP_DAC1 [ 8.358195] BUG: unable to handle kernel NULL pointer dereference at 0000000000000409 [ 8.409733] [] radeon_dp_getsinktype+0x1a/0x30 [radeon] [ 8.416805] PGD 0 [ 8.418841] Oops: 0000 [#1] SMP ... This patch prevents Kernel failures due to a connector not having a DDC interface by changing the code so that ddc_bus is always checked before use. The problem was first identified using the uBuntu MATE 14.04 LTS (3.16 series kernels) but not dealt with at that time. On attempting to install uBuntu MATE 16.04 LTS (4.4 series kernels), it became clear that using various workarounds to allow the issue to be ignored were not viable so more effort was put in to sorting the issue resulting in this patch. See https://bugs.launchpad.net/bugs/1587885 for more details. Signed-off-by: Mark Fortescue Tested-by: Mark Fortescue --- Looks like Thunderbird may have made a mess of the patch when pasting the contents into the mail message - my alternate mail client (pine) also has strict line length handling and trashes non-MIME encoded patches. This may not be the correct approach to solving the issue but it is clean in that it ensures that ddc_bus is never used when NULL regardless of how the code ended up at the point of use. If it helps with back porting, I have patches for the uBuntu 14.04 LTS [3.13 series], uBuntu MATE 14.04 LTS [3.16 series] and uBuntu 16.04 LTS [4.4 series] kernels. Test Hardware: Commell LV-683 Mini-ITX with onboard AMD Dual-core G-T56N 4G Ram, 2x1TB Disk, HANNS-G HC194D 1280x1024 LCD (VGA). 4.8.0-rc5 with patch boots without error. drivers/gpu/drm/radeon/atombios_dp.c | 60 ++++++++++++------- drivers/gpu/drm/radeon/radeon_connectors.c | 46 +++++++------- drivers/gpu/drm/radeon/radeon_dp_mst.c | 9 ++ drivers/gpu/drm/radeon/radeon_i2c.c | 3 4 files changed, 73 insertions(+), 45 deletions(-) Patch: } else { diff --git a/drivers/gpu/drm/radeon/atombios_dp.c b/drivers/gpu/drm/radeon/atombios_dp.c index cead089a..98b3c0e 100644 --- a/drivers/gpu/drm/radeon/atombios_dp.c +++ b/drivers/gpu/drm/radeon/atombios_dp.c @@ -232,6 +232,9 @@ void radeon_dp_aux_init(struct radeon_connector *radeon_connector) struct radeon_device *rdev = dev->dev_private; int ret; + if (!radeon_connector->ddc_bus) + return; + radeon_connector->ddc_bus->rec.hpd = radeon_connector->hpd.hpd; radeon_connector->ddc_bus->aux.dev = radeon_connector->base.kdev; if (ASIC_IS_DCE5(rdev)) { @@ -364,6 +367,9 @@ u8 radeon_dp_getsinktype(struct radeon_connector *radeon_connector) struct drm_device *dev = radeon_connector->base.dev; struct radeon_device *rdev = dev->dev_private; + if (!radeon_connector->ddc_bus) + return 0; + return radeon_dp_encoder_service(rdev, ATOM_DP_ACTION_GET_SINK_TYPE, 0, radeon_connector->ddc_bus->rec.i2c_id, 0); } @@ -376,6 +382,9 @@ static void radeon_dp_probe_oui(struct radeon_connector *radeon_connector) if (!(dig_connector->dpcd[DP_DOWN_STREAM_PORT_COUNT] & DP_OUI_SUPPORT)) return; + if (!radeon_connector->ddc_bus) + return; + if (drm_dp_dpcd_read(&radeon_connector->ddc_bus->aux, DP_SINK_OUI, buf, 3) == 3) DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n", buf[0], buf[1], buf[2]); @@ -391,6 +400,9 @@ bool radeon_dp_getdpcd(struct radeon_connector *radeon_connector) u8 msg[DP_DPCD_SIZE]; int ret, i; + if (!radeon_connector->ddc_bus) + return false; + for (i = 0; i < 7; i++) { ret = drm_dp_dpcd_read(&radeon_connector->ddc_bus->aux, DP_DPCD_REV, msg, DP_DPCD_SIZE); @@ -428,24 +440,26 @@ int radeon_dp_get_panel_mode(struct drm_encoder *encoder, dig_connector = radeon_connector->con_priv; - if (dp_bridge != ENCODER_OBJECT_ID_NONE) { - /* DP bridge chips */ - if (drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux, - DP_EDP_CONFIGURATION_CAP, &tmp) == 1) { - if (tmp & 1) - panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE; - else if ((dp_bridge == ENCODER_OBJECT_ID_NUTMEG) || - (dp_bridge == ENCODER_OBJECT_ID_TRAVIS)) - panel_mode = DP_PANEL_MODE_INTERNAL_DP1_MODE; - else - panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE; - } - } else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) { - /* eDP */ - if (drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux, - DP_EDP_CONFIGURATION_CAP, &tmp) == 1) { - if (tmp & 1) - panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE; + if (radeon_connector->ddc_bus) { + if (dp_bridge != ENCODER_OBJECT_ID_NONE) { + /* DP bridge chips */ + if (drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux, + DP_EDP_CONFIGURATION_CAP, &tmp) == 1) { + if (tmp & 1) + panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE; + else if ((dp_bridge == ENCODER_OBJECT_ID_NUTMEG) || + (dp_bridge == ENCODER_OBJECT_ID_TRAVIS)) + panel_mode = DP_PANEL_MODE_INTERNAL_DP1_MODE; + else + panel_mode = DP_PANEL_MODE_EXTERNAL_DP_MODE; + } + } else if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) { + /* eDP */ + if (drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux, + DP_EDP_CONFIGURATION_CAP, &tmp) == 1) { + if (tmp & 1) + panel_mode = DP_PANEL_MODE_INTERNAL_DP2_MODE; + } } } @@ -511,6 +525,9 @@ bool radeon_dp_needs_link_train(struct radeon_connector *radeon_connector) u8 link_status[DP_LINK_STATUS_SIZE]; struct radeon_connector_atom_dig *dig = radeon_connector->con_priv; + if (!radeon_connector->ddc_bus) + return false; + if (drm_dp_dpcd_read_link_status(&radeon_connector->ddc_bus->aux, link_status) <= 0) return false; @@ -531,7 +548,7 @@ void radeon_dp_set_rx_power_state(struct drm_connector *connector, dig_connector = radeon_connector->con_priv; /* power up/down the sink */ - if (dig_connector->dpcd[0] >= 0x11) { + if (radeon_connector->ddc_bus && dig_connector->dpcd[0] >= 0x11) { drm_dp_dpcd_writeb(&radeon_connector->ddc_bus->aux, DP_SET_POWER, power_state); usleep_range(1000, 2000); @@ -834,7 +851,8 @@ void radeon_dp_link_train(struct drm_encoder *encoder, else dp_info.enc_id |= ATOM_DP_CONFIG_LINK_A; - if (drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux, DP_MAX_LANE_COUNT, &tmp) + if (radeon_connector->ddc_bus && + drm_dp_dpcd_readb(&radeon_connector->ddc_bus->aux, DP_MAX_LANE_COUNT, &tmp) == 1) { if (ASIC_IS_DCE5(rdev) && (tmp & DP_TPS3_SUPPORTED)) dp_info.tp3_supported = true; @@ -850,7 +868,7 @@ void radeon_dp_link_train(struct drm_encoder *encoder, dp_info.connector = connector; dp_info.dp_lane_count = dig_connector->dp_lane_count; dp_info.dp_clock = dig_connector->dp_clock; - dp_info.aux = &radeon_connector->ddc_bus->aux; + dp_info.aux = radeon_connector->ddc_bus ? &radeon_connector->ddc_bus->aux : 0; if (radeon_dp_link_train_init(&dp_info)) goto done; diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c index b79f3b0..cec30c9 100644 --- a/drivers/gpu/drm/radeon/radeon_connectors.c +++ b/drivers/gpu/drm/radeon/radeon_connectors.c @@ -328,31 +328,32 @@ static void radeon_connector_get_edid(struct drm_connector *connector) if (radeon_connector->router.ddc_valid) radeon_router_select_ddc_port(radeon_connector); - if ((radeon_connector_encoder_get_dp_bridge_encoder_id(connector) != - ENCODER_OBJECT_ID_NONE) && - radeon_connector->ddc_bus->has_aux) { - radeon_connector->edid = drm_get_edid(connector, - &radeon_connector->ddc_bus->aux.ddc); - } else if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) || - (connector->connector_type == DRM_MODE_CONNECTOR_eDP)) { - struct radeon_connector_atom_dig *dig = radeon_connector->con_priv; - - if ((dig->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT || - dig->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) && - radeon_connector->ddc_bus->has_aux) - radeon_connector->edid = drm_get_edid(&radeon_connector->base, + if (radeon_connector->ddc_bus) { + if ((radeon_connector_encoder_get_dp_bridge_encoder_id(connector) != + ENCODER_OBJECT_ID_NONE) && + radeon_connector->ddc_bus->has_aux) { + radeon_connector->edid = drm_get_edid(connector, &radeon_connector->ddc_bus->aux.ddc); - else if (radeon_connector->ddc_bus) + } else if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) || + (connector->connector_type == DRM_MODE_CONNECTOR_eDP)) { + struct radeon_connector_atom_dig *dig = radeon_connector->con_priv; + + if ((dig->dp_sink_type == CONNECTOR_OBJECT_ID_DISPLAYPORT || + dig->dp_sink_type == CONNECTOR_OBJECT_ID_eDP) && + radeon_connector->ddc_bus->has_aux) + radeon_connector->edid = drm_get_edid(&radeon_connector->base, + &radeon_connector->ddc_bus->aux.ddc); + else + radeon_connector->edid = drm_get_edid(&radeon_connector->base, + &radeon_connector->ddc_bus->adapter); + } else if (vga_switcheroo_handler_flags() & VGA_SWITCHEROO_CAN_SWITCH_DDC && + connector->connector_type == DRM_MODE_CONNECTOR_LVDS) { + radeon_connector->edid = drm_get_edid_switcheroo(&radeon_connector->base, + &radeon_connector->ddc_bus->adapter); + } else { radeon_connector->edid = drm_get_edid(&radeon_connector->base, &radeon_connector->ddc_bus->adapter); - } else if (vga_switcheroo_handler_flags() & VGA_SWITCHEROO_CAN_SWITCH_DDC && - connector->connector_type == DRM_MODE_CONNECTOR_LVDS && - radeon_connector->ddc_bus) { - radeon_connector->edid = drm_get_edid_switcheroo(&radeon_connector->base, - &radeon_connector->ddc_bus->adapter); - } else if (radeon_connector->ddc_bus) { - radeon_connector->edid = drm_get_edid(&radeon_connector->base, - &radeon_connector->ddc_bus->adapter); + } } if (!radeon_connector->edid) { @@ -1312,6 +1313,7 @@ radeon_dvi_detect(struct drm_connector *connector, bool force) continue; list_radeon_connector = to_radeon_connector(list_connector); if (list_radeon_connector->shared_ddc && + radeon_connector->ddc_bus && (list_radeon_connector->ddc_bus->rec.i2c_id == radeon_connector->ddc_bus->rec.i2c_id)) { /* cases where both connectors are digital */ diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c index de504ea..89e91f3 100644 --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c @@ -661,7 +661,7 @@ radeon_dp_mst_init(struct radeon_connector *radeon_connector) { struct drm_device *dev = radeon_connector->base.dev; - if (!radeon_connector->ddc_bus->has_aux) + if (!radeon_connector->ddc_bus || !radeon_connector->ddc_bus->has_aux) return 0; radeon_connector->mst_mgr.cbs = &mst_cbs; @@ -688,6 +688,9 @@ radeon_dp_mst_probe(struct radeon_connector *radeon_connector) if (dig_connector->dpcd[DP_DPCD_REV] < 0x12) return 0; + if (!radeon_connector->ddc_bus || !radeon_connector->ddc_bus->has_aux) + return 0; + ret = drm_dp_dpcd_read(&radeon_connector->ddc_bus->aux, DP_MSTM_CAP, msg, 1); if (ret) { @@ -711,7 +714,9 @@ radeon_dp_mst_check_status(struct radeon_connector *radeon_connector) struct radeon_connector_atom_dig *dig_connector = radeon_connector->con_priv; int retry; - if (dig_connector->is_mst) { + if (dig_connector->is_mst && + radeon_connector->ddc_bus && + radeon_connector->ddc_bus->has_aux) { u8 esi[16] = { 0 }; int dret; int ret = 0; diff --git a/drivers/gpu/drm/radeon/radeon_i2c.c b/drivers/gpu/drm/radeon/radeon_i2c.c index 9590bcd..c18f7ba 100644 --- a/drivers/gpu/drm/radeon/radeon_i2c.c +++ b/drivers/gpu/drm/radeon/radeon_i2c.c @@ -63,6 +63,9 @@ bool radeon_ddc_probe(struct radeon_connector *radeon_connector, bool use_aux) if (radeon_connector->router.ddc_valid) radeon_router_select_ddc_port(radeon_connector); + if (!radeon_connector->ddc_bus) + return false; + if (use_aux) { ret = i2c_transfer(&radeon_connector->ddc_bus->aux.ddc, msgs, 2);