Am 07.09.2016 um 19:38 schrieb Mark Fortescue: > > 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.
I'm not an expert on this, but that is really odd cause even LCD Panels should have a DDC interface. > > 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 Especially since the BIOS claims that this is a displayport connector and there is no physical way to have a DP without an DDC as far as I know. Please open a bug report on FDO and attach you BIOS image. Alex can probably take a look when he's back from vacation. Regards, Christian. > [ 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] [<ffffffffc02024da>] 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 <mark at thurning-instruments.co.uk> > Tested-by: Mark Fortescue <mark at thurning-instruments.co.uk> > > --- > > 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: > 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); > } else { > _______________________________________________ > dri-devel mailing list > dri-devel at lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
