Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Almahallawy, Khaled" <khaled.almahallawy@intel.com>
To: "ville.syrjala@linux.intel.com" <ville.syrjala@linux.intel.com>,
	"Taylor,  Clinton A" <clinton.a.taylor@intel.com>,
	"Srinivas, Vidya" <vidya.srinivas@intel.com>
Cc: "intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/dp: DP PHY compliance for JSL
Date: Thu, 4 Jun 2020 20:01:03 +0000	[thread overview]
Message-ID: <fea323968324ceefe813d34d80fdd9779614aa01.camel@intel.com> (raw)
In-Reply-To: <20200604190612.GI6112@intel.com>

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@intel.com>
> > Signed-off-by: Vidya Srinivas <vidya.srinivas@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@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2020-06-04 20:02 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-04  5:03 [Intel-gfx] [PATCH] drm/i915/dp: DP PHY compliance for JSL Vidya Srinivas
2020-06-04  5:34 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2020-06-04  5:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-06-04 10:28 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-06-04 19:06 ` [Intel-gfx] [PATCH] " Ville Syrjälä
2020-06-04 20:01   ` Almahallawy, Khaled [this message]
2020-06-04 21:03     ` Ville Syrjälä
2020-06-12 18:25       ` Manasi Navare
2020-06-12 18:36         ` Ville Syrjälä
2020-06-12 18:44           ` Manasi Navare
2020-06-12 19:01             ` Ville Syrjälä
2020-06-12 19:12               ` Manasi Navare
2020-06-12 19:21                 ` Ville Syrjälä
2020-06-12 19:38                   ` Manasi Navare
2020-06-15 16:19                     ` Ville Syrjälä
2020-06-12 18:33     ` Manasi Navare
2020-06-12 18:39       ` Ville Syrjälä
2020-09-04  4:13 ` [Intel-gfx] [PATCH 1/3] drm/dp: Add PHY_TEST_PATTERN CP2520 Pattern 2 and 3 Vidya Srinivas
2020-09-04  4:13   ` [Intel-gfx] [PATCH] drm/i915/dp: DP PHY compliance for EHL/JSL Vidya Srinivas
2020-09-04  4:13   ` [Intel-gfx] [PATCH 2/3] drm/i915/dp: TPS4 PHY test pattern compliance support Vidya Srinivas
2020-09-04  4:13   ` [Intel-gfx] [PATCH 3/3] [RFC] drm/i915/dp: DP PHY compliance for EHL/JSL Vidya Srinivas
2020-09-04  4:14 ` [Intel-gfx] [PATCH 1/3] drm/dp: Add PHY_TEST_PATTERN CP2520 Pattern 2 and 3 Vidya Srinivas
2020-09-04  4:14   ` [Intel-gfx] [PATCH 2/3] drm/i915/dp: TPS4 PHY test pattern compliance support Vidya Srinivas
2020-09-04  4:14   ` [Intel-gfx] [PATCH 3/3] [RFC] drm/i915/dp: DP PHY compliance for EHL/JSL Vidya Srinivas
2020-09-04  4:29 ` [Intel-gfx] [PATCH] " Vidya Srinivas
2020-09-10  4:00   ` Vidya Srinivas
2020-09-10  4:10 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/dp: DP PHY compliance for JSL (rev11) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fea323968324ceefe813d34d80fdd9779614aa01.camel@intel.com \
    --to=khaled.almahallawy@intel.com \
    --cc=clinton.a.taylor@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=vidya.srinivas@intel.com \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox