[Intel-gfx] [PATCH] drm/i915/dp: DP PHY compliance for JSL

Almahallawy, Khaled khaled.almahallawy at intel.com
Thu Jun 4 20:01:03 UTC 2020


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 at intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas at intel.com>
> > ---
> >  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.

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 at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 


More information about the Intel-gfx mailing list