On Fri, Jun 12, 2020 at 11:33:45AM -0700, Manasi Navare wrote: > On Thu, Jun 04, 2020 at 08:01:03PM +0000, Almahallawy, Khaled wrote: > > On Thu, 2020-06-04 at 22:06 +0300, Ville Syrjälä wrote: > > > On Thu, Jun 04, 2020 at 10:33:48AM +0530, Vidya Srinivas wrote: > > > > Signed-off-by: Khaled Almahallawy <khaled.almahallawy@xxxxxxxxx> > > > > Signed-off-by: Vidya Srinivas <vidya.srinivas@xxxxxxxxx> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_dp.c | 40 > > > > ++++++++++++++++++++++++++------- > > > > 1 file changed, 32 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > > index 7223367171d1..44663e8ac9a1 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > @@ -5470,22 +5470,32 @@ intel_dp_autotest_phy_ddi_disable(struct > > > > intel_dp *intel_dp) > > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > > struct intel_crtc *crtc = to_intel_crtc(intel_dig_port- > > > > >base.base.crtc); > > > > enum pipe pipe = crtc->pipe; > > > > - u32 trans_ddi_func_ctl_value, trans_conf_value, > > > > dp_tp_ctl_value; > > > > + u32 trans_ddi_func_ctl_value, trans_conf_value, > > > > dp_tp_ctl_value, trans_ddi_port_mask; > > > > + enum port port = intel_dig_port->base.port; > > > > + i915_reg_t dp_tp_reg; > > > > + > > > > + if (IS_ELKHARTLAKE(dev_priv)) { > > > > + dp_tp_reg = DP_TP_CTL(port); > > > > + trans_ddi_port_mask = TRANS_DDI_PORT_MASK; > > > > + } else if (IS_TIGERLAKE(dev_priv)) { > > > > + dp_tp_reg = TGL_DP_TP_CTL(pipe); > > > > + trans_ddi_port_mask = TGL_TRANS_DDI_PORT_MASK; > > > > + } > > > > > > > > trans_ddi_func_ctl_value = intel_de_read(dev_priv, > > > > TRANS_DDI_FUNC_CTL(pip > > > > e)); > > > > trans_conf_value = intel_de_read(dev_priv, PIPECONF(pipe)); > > > > - dp_tp_ctl_value = intel_de_read(dev_priv, TGL_DP_TP_CTL(pipe)); > > > > > > > > + dp_tp_ctl_value = intel_de_read(dev_priv, dp_tp_reg); > > > > trans_ddi_func_ctl_value &= ~(TRANS_DDI_FUNC_ENABLE | > > > > - TGL_TRANS_DDI_PORT_MASK); > > > > + trans_ddi_port_mask); > > > > trans_conf_value &= ~PIPECONF_ENABLE; > > > > dp_tp_ctl_value &= ~DP_TP_CTL_ENABLE; > > > > > > > > intel_de_write(dev_priv, PIPECONF(pipe), trans_conf_value); > > > > intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(pipe), > > > > trans_ddi_func_ctl_value); > > > > - intel_de_write(dev_priv, TGL_DP_TP_CTL(pipe), dp_tp_ctl_value); > > > > + intel_de_write(dev_priv, dp_tp_reg, dp_tp_ctl_value); > > > > > > All this ad-hoc modeset code really should not exist. It's going to > > > have different bugs than the norma modeset paths, so compliance > > > testing > > > this special code proves absolutely nothing about the normal modeset > > > code. IMO someone needs to take up the task of rewrtiting all this to > > > just perform normal modesets. > > But isnt that behaviour of the scope against the compliance spec? scope? > The PHY request as per the VESA compliance spec should only come through > a short pulse. > Yes if it comes through a long pulse, it will reset the link and this whole > code will fall apart. I am not saying anything about how the sink signals the requests. That's just an implementation detail that doesn't really matter. > > Manasi > > > > > Agree. I've just found that we get kernel NULL pointer dereference and > > panic when we try to access to_intel_crtc(intel_dig_port- > > >base.base.crtc). This is because we didn't realize when we developed > > the code that test scope has an option to send PHY test request on Long > > HPD. Current desing assume PHY test request on short HPD. Because of > > that we got the following error > > > > > > [ 106.810882] i915 0000:00:02.0: [drm:intel_hpd_irq_handler [i915]] > > digital hpd on [ENCODER:308:DDI F] - long > > [ 106.810916] i915 0000:00:02.0: [drm:intel_hpd_irq_handler [i915]] > > Received HPD interrupt on PIN 9 - cnt: 10 > > [ 106.811026] i915 0000:00:02.0: [drm:intel_dp_hpd_pulse [i915]] got > > hpd irq on [ENCODER:308:DDI F] - long > > [ 106.811095] i915 0000:00:02.0: [drm:i915_hotplug_work_func [i915]] > > running encoder hotplug functions > > [ 106.811184] i915 0000:00:02.0: [drm:i915_hotplug_work_func [i915]] > > Connector DP-3 (pin 9) received hotplug event. (retry 0) > > [ 106.811227] i915 0000:00:02.0: [drm:intel_dp_detect [i915]] > > [CONNECTOR:309:DP-3] > > [ 106.811292] i915 0000:00:02.0: [drm:intel_power_well_enable [i915]] > > enabling TC cold off > > [ 106.811365] i915 0000:00:02.0: [drm:tgl_tc_cold_request [i915]] TC > > cold block succeeded > > [ 106.811489] i915 0000:00:02.0: [drm:__intel_tc_port_lock [i915]] > > Port F/TC#3: TC port mode reset (tbt-alt -> dp-alt) > > [ 106.811663] i915 0000:00:02.0: [drm:intel_power_well_enable [i915]] > > enabling AUX F TC3 > > [ 106.812449] [drm:drm_dp_dpcd_read] AUX F/port F: 0x00000 AUX -> > > (ret= 15) 12 14 04 80 00 00 01 00 00 00 00 00 00 00 00 > > [ 106.812484] i915 0000:00:02.0: [drm:intel_dp_read_dpcd [i915]] DPCD: > > 12 14 04 80 00 00 01 00 00 00 00 00 00 00 00 > > [ 106.813266] [drm:drm_dp_dpcd_read] AUX F/port F: 0x00400 AUX -> > > (ret= 12) 00 00 00 00 00 00 00 00 00 00 00 00 > > [ 106.813271] [drm:drm_dp_read_desc] DP sink: OUI 00-00-00 dev-ID HW- > > rev 0.0 SW-rev 0.0 quirks 0x0000 > > [ 106.813891] [drm:drm_dp_dpcd_read] AUX F/port F: 0x00200 AUX -> > > (ret= 1) 01 > > [ 106.813940] i915 0000:00:02.0: [drm:intel_dp_print_rates [i915]] > > source rates: 162000, 216000, 270000, 324000, 432000, 540000, 648000, > > 810000 > > [ 106.813974] i915 0000:00:02.0: [drm:intel_dp_print_rates [i915]] > > sink rates: 162000, 270000, 540000 > > [ 106.814007] i915 0000:00:02.0: [drm:intel_dp_print_rates [i915]] > > common rates: 162000, 270000, 540000 > > [ 106.814550] [drm:drm_dp_dpcd_read] AUX F/port F: 0x00021 AUX -> > > (ret= 1) 00 > > [ 106.814583] i915 0000:00:02.0: [drm:intel_dp_detect [i915]] > > [ENCODER:308:DDI F] MST support: port: yes, sink: no, modparam: yes > > > > ..... > > > > [ 106.927291] i915 0000:00:02.0: [drm:intel_dp_check_service_irq > > [i915]] PHY_PATTERN test requested > > [ 106.927897] [drm:drm_dp_dpcd_read] AUX F/port F: 0x00219 AUX -> > > (ret= 1) 0a > > [ 106.928507] [drm:drm_dp_dpcd_read] AUX F/port F: 0x00220 AUX -> > > (ret= 1) 04 > > [ 106.929143] [drm:drm_dp_dpcd_read] AUX F/port F: 0x00248 AUX -> > > (ret= 1) 00 > > [ 106.929824] [drm:drm_dp_dpcd_read] AUX F/port F: 0x00202 AUX -> > > (ret= 6) 00 00 80 00 00 00 > > [ 106.929830] BUG: kernel NULL pointer dereference, address: > > 0000000000000578 > > [ 106.936809] #PF: supervisor read access in kernel mode > > [ 106.941953] #PF: error_code(0x0000) - not-present page > > [ 106.947082] PGD 0 P4D 0 > > [ 106.949643] Oops: 0000 [#1] PREEMPT SMP NOPTI > > [ 106.954010] CPU: 6 PID: 200 Comm: kworker/6:2 Not tainted 5.7.0-rc7- > > CI-CI_DRM_8566+ #5 > > [ 106.975251] Workqueue: events i915_hotplug_work_func [i915] > > [ 106.980887] RIP: 0010:intel_dp_process_phy_request+0x94/0x5a0 [i915] > > [ 106.987239] Code: 48 83 c4 20 5b 5d 41 5c 41 5d 41 5e 41 5f c3 48 8d > > 74 24 12 4c 89 f7 e8 3a 3e 00 00 49 8b 86 28 ff ff ff 49 8b 9e d8 fe ff > > ff <48> 63 80 78 05 00 00 8b 93 54 0d 00 00 48 8d ab e8 0e 00 00 48 89 > > [ 107.005890] RSP: 0018:ffffc9000046fb20 EFLAGS: 00010246 > > > > I plan to temporarily fix this issue by ignoreing scope request on long > > HPD, until we have modeset based implementation. > > > > > > } > > > > > > > > static void > > > > @@ -5497,20 +5507,28 @@ intel_dp_autotest_phy_ddi_enable(struct > > > > intel_dp *intel_dp, uint8_t lane_cnt) > > > > enum port port = intel_dig_port->base.port; > > > > struct intel_crtc *crtc = to_intel_crtc(intel_dig_port- > > > > >base.base.crtc); > > > > enum pipe pipe = crtc->pipe; > > > > - u32 trans_ddi_func_ctl_value, trans_conf_value, > > > > dp_tp_ctl_value; > > > > + u32 trans_ddi_func_ctl_value, trans_conf_value, > > > > dp_tp_ctl_value, trans_ddi_sel_port; > > > > + i915_reg_t dp_tp_reg; > > > > + > > > > + if (IS_ELKHARTLAKE(dev_priv)) { > > > > + dp_tp_reg = DP_TP_CTL(port); > > > > + trans_ddi_sel_port = TRANS_DDI_SELECT_PORT(port); > > > > + } else if (IS_TIGERLAKE(dev_priv)) { > > > > + dp_tp_reg = TGL_DP_TP_CTL(pipe); > > > > + trans_ddi_sel_port = TGL_TRANS_DDI_SELECT_PORT(port); > > > > + } > > > > > > > > trans_ddi_func_ctl_value = intel_de_read(dev_priv, > > > > TRANS_DDI_FUNC_CTL(pip > > > > e)); > > > > trans_conf_value = intel_de_read(dev_priv, PIPECONF(pipe)); > > > > dp_tp_ctl_value = intel_de_read(dev_priv, TGL_DP_TP_CTL(pipe)); > > > > - > > > > trans_ddi_func_ctl_value |= TRANS_DDI_FUNC_ENABLE | > > > > - TGL_TRANS_DDI_SELECT_PORT(port); > > > > + trans_ddi_sel_port; > > > > trans_conf_value |= PIPECONF_ENABLE; > > > > dp_tp_ctl_value |= DP_TP_CTL_ENABLE; > > > > > > > > intel_de_write(dev_priv, PIPECONF(pipe), trans_conf_value); > > > > - intel_de_write(dev_priv, TGL_DP_TP_CTL(pipe), dp_tp_ctl_value); > > > > + intel_de_write(dev_priv, dp_tp_reg, dp_tp_ctl_value); > > > > intel_de_write(dev_priv, TRANS_DDI_FUNC_CTL(pipe), > > > > trans_ddi_func_ctl_value); > > > > } > > > > @@ -5557,6 +5575,7 @@ static u8 > > > > intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp) > > > > static void intel_dp_handle_test_request(struct intel_dp > > > > *intel_dp) > > > > { > > > > struct drm_i915_private *i915 = dp_to_i915(intel_dp); > > > > + struct drm_i915_private *dev_priv = i915; > > > > u8 response = DP_TEST_NAK; > > > > u8 request = 0; > > > > int status; > > > > @@ -5582,6 +5601,11 @@ static void > > > > intel_dp_handle_test_request(struct intel_dp *intel_dp) > > > > response = intel_dp_autotest_edid(intel_dp); > > > > break; > > > > case DP_TEST_LINK_PHY_TEST_PATTERN: > > > > + if (!IS_ELKHARTLAKE(dev_priv) || > > > > !IS_TIGERLAKE(dev_priv)) { > > > > + drm_dbg_kms(&i915->drm, > > > > + "PHY compliance for platform not > > > > supported\n"); > > > > + return; > > > > + } > > > > drm_dbg_kms(&i915->drm, "PHY_PATTERN test > > > > requested\n"); > > > > response = intel_dp_autotest_phy_pattern(intel_dp); > > > > break; > > > > -- > > > > 2.7.4 > > > > > > > > _______________________________________________ > > > > Intel-gfx mailing list > > > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel