[PATCH 1/2] drm/i915/icl: Add TBT checks for PLL calculations

[PATCH 1/2] drm/i915/icl: Add TBT checks for PLL calculations

[Anusha Srivatsa]

Add missing TBT check in the Pll calculation.

v2: do not use a auxiliary function to check if status is
TBT or not. (Paulo)

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 7e5e6eb..ba49078 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -428,6 +428,7 @@ ibx_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	     struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
 	struct intel_shared_dpll *pll;
 	enum intel_dpll_id i;
 
@@ -2866,6 +2867,9 @@ static struct intel_shared_dpll *
 icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	     struct intel_encoder *encoder)
 {
+	struct intel_digital_port *intel_dig_port =
+			enc_to_dig_port(&encoder->base);
+
 	struct intel_shared_dpll *pll;
 	struct intel_dpll_hw_state pll_state = {};
 	enum port port = encoder->port;
@@ -2885,7 +2889,7 @@ icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state,
 	case PORT_D:
 	case PORT_E:
 	case PORT_F:
-		if (0 /* TODO: TBT PLLs */) {
+		if (intel_dig_port->tc_type == TC_PORT_TBT) {
 			min = DPLL_ID_ICL_TBTPLL;
 			max = min;
 			ret = icl_calc_dpll_state(crtc_state, encoder, clock,
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/2] drm/i915/icl: Set TBT IO in Aux transaction

[Anusha Srivatsa]

For a TBT sequence, we need to set the IO type to TBT
in  DDI_AUX_CTL.

Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h |  1 +
 drivers/gpu/drm/i915/intel_dp.c | 34 +++++++++++++++++++++++++---------
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 5530c47..7bdc214 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5558,6 +5558,7 @@ enum {
 #define   DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL	(1 << 14)
 #define   DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL	(1 << 13)
 #define   DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL	(1 << 12)
+#define   DP_AUX_CH_CTL_TBT_IO			(1 << 11)
 #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (0x1f << 5)
 #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
 #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index cc33d7c..90a8e2f 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1208,15 +1208,31 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp,
 				      int send_bytes,
 				      uint32_t unused)
 {
-	return DP_AUX_CH_CTL_SEND_BUSY |
-	       DP_AUX_CH_CTL_DONE |
-	       DP_AUX_CH_CTL_INTERRUPT |
-	       DP_AUX_CH_CTL_TIME_OUT_ERROR |
-	       DP_AUX_CH_CTL_TIME_OUT_MAX |
-	       DP_AUX_CH_CTL_RECEIVE_ERROR |
-	       (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
-	       DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
-	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
+	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
+	struct drm_i915_private *dev_priv = to_i915(intel_dig_port->base.base.dev);
+
+	if (INTEL_GEN(dev_priv) >= 11 && intel_dig_port->tc_type == TC_PORT_TBT) {
+		return DP_AUX_CH_CTL_SEND_BUSY |
+		       DP_AUX_CH_CTL_DONE |
+		       DP_AUX_CH_CTL_INTERRUPT |
+		       DP_AUX_CH_CTL_TIME_OUT_ERROR |
+		       DP_AUX_CH_CTL_TIME_OUT_MAX |
+		       DP_AUX_CH_CTL_RECEIVE_ERROR |
+		       (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
+		       DP_AUX_CH_CTL_TBT_IO |
+		       DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
+		       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
+	} else {
+		return DP_AUX_CH_CTL_SEND_BUSY |
+		       DP_AUX_CH_CTL_DONE |
+		       DP_AUX_CH_CTL_INTERRUPT |
+		       DP_AUX_CH_CTL_TIME_OUT_ERROR |
+		       DP_AUX_CH_CTL_TIME_OUT_MAX |
+		       DP_AUX_CH_CTL_RECEIVE_ERROR |
+		       (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
+		       DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
+		       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
+	}
 }
 
 static int
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/icl: Add TBT checks for PLL calculations

[Paulo Zanoni]

Em Qua, 2018-07-25 às 14:28 -0700, Anusha Srivatsa escreveu:
> Add missing TBT check in the Pll calculation.
> 
> v2: do not use a auxiliary function to check if status is
> TBT or not. (Paulo)
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 7e5e6eb..ba49078 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -428,6 +428,7 @@ ibx_get_dpll(struct intel_crtc *crtc, struct
> intel_crtc_state *crtc_state,
>  	     struct intel_encoder *encoder)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +
>  	struct intel_shared_dpll *pll;
>  	enum intel_dpll_id i;
>  

This chunk does nothing and the space introduced is even against our
coding style. Please review your own patches before submitting them to
the mailing list: this is the kind of error that's very easy to catch
by looking at the patch file, saving everybody's time.



> @@ -2866,6 +2867,9 @@ static struct intel_shared_dpll *
>  icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state
> *crtc_state,
>  	     struct intel_encoder *encoder)
>  {
> +	struct intel_digital_port *intel_dig_port =
> +			enc_to_dig_port(&encoder->base);
> +

This chunk also adds a blank line that goes against our coding style.



>  	struct intel_shared_dpll *pll;
>  	struct intel_dpll_hw_state pll_state = {};
>  	enum port port = encoder->port;
> @@ -2885,7 +2889,7 @@ icl_get_dpll(struct intel_crtc *crtc, struct
> intel_crtc_state *crtc_state,
>  	case PORT_D:
>  	case PORT_E:
>  	case PORT_F:
> -		if (0 /* TODO: TBT PLLs */) {
> +		if (intel_dig_port->tc_type == TC_PORT_TBT) {
>  			min = DPLL_ID_ICL_TBTPLL;
>  			max = min;
>  			ret = icl_calc_dpll_state(crtc_state,
> encoder, clock,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/icl: Set TBT IO in Aux transaction

[Paulo Zanoni]

Em Qua, 2018-07-25 às 14:28 -0700, Anusha Srivatsa escreveu:
> For a TBT sequence, we need to set the IO type to TBT
> in  DDI_AUX_CTL.
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h |  1 +
>  drivers/gpu/drm/i915/intel_dp.c | 34 +++++++++++++++++++++++++----
> -----
>  2 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 5530c47..7bdc214 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -5558,6 +5558,7 @@ enum {
>  #define   DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL	(1 << 14)
>  #define   DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL	(1 << 13)
>  #define   DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL	(1 << 12)
> +#define   DP_AUX_CH_CTL_TBT_IO			(1 << 11)
>  #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (0x1f << 5)
>  #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
>  #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index cc33d7c..90a8e2f 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1208,15 +1208,31 @@ static uint32_t skl_get_aux_send_ctl(struct
> intel_dp *intel_dp,
>  				      int send_bytes,
>  				      uint32_t unused)
>  {
> -	return DP_AUX_CH_CTL_SEND_BUSY |
> -	       DP_AUX_CH_CTL_DONE |
> -	       DP_AUX_CH_CTL_INTERRUPT |
> -	       DP_AUX_CH_CTL_TIME_OUT_ERROR |
> -	       DP_AUX_CH_CTL_TIME_OUT_MAX |
> -	       DP_AUX_CH_CTL_RECEIVE_ERROR |
> -	       (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> -	       DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
> -	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> +	struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
> >base.base.dev);
> +
> +	if (INTEL_GEN(dev_priv) >= 11

There's no need for this check, since intel_dig_port->tc_type can't be
TC_PORT_TBT on platforms earlier than gen11.

If we were to add a check, I'd suggest to use intel_port_is_tc(), but I
don't see the need.

There's a lot of avoidable duplicate code. Please avoid it with
something like:

---
uint32_t ret;

ret = DP_AUX_CH_CTL_SEND_BUSY |
      DP_AUX_CH_CTL_DONE |
      etc;

if (intel_dig_port->tc_type == TC_PORT_TBT)
	ret |= DP_AUX_CH_CTL_TBT_IO;

return ret;
---


Thanks,
Paulo

>  && intel_dig_port->tc_type == TC_PORT_TBT) {
> +		return DP_AUX_CH_CTL_SEND_BUSY |
> +		       DP_AUX_CH_CTL_DONE |
> +		       DP_AUX_CH_CTL_INTERRUPT |
> +		       DP_AUX_CH_CTL_TIME_OUT_ERROR |
> +		       DP_AUX_CH_CTL_TIME_OUT_MAX |
> +		       DP_AUX_CH_CTL_RECEIVE_ERROR |
> +		       (send_bytes <<
> DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> +		       DP_AUX_CH_CTL_TBT_IO |
> +		       DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
> +		       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> +	} else {
> +		return DP_AUX_CH_CTL_SEND_BUSY |
> +		       DP_AUX_CH_CTL_DONE |
> +		       DP_AUX_CH_CTL_INTERRUPT |
> +		       DP_AUX_CH_CTL_TIME_OUT_ERROR |
> +		       DP_AUX_CH_CTL_TIME_OUT_MAX |
> +		       DP_AUX_CH_CTL_RECEIVE_ERROR |
> +		       (send_bytes <<
> DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
> +		       DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
> +		       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
> +	}
>  }
>  
>  static int
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/icl: Add TBT checks for PLL calculations

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/icl: Add TBT checks for PLL calculations
URL   : https://patchwork.freedesktop.org/series/47248/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4547 -> Patchwork_9770 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9770 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9770, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47248/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9770:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_objects:
      fi-cnl-psr:         NOTRUN -> DMESG-FAIL

    
== Known issues ==

  Here are the changes found in Patchwork_9770 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@debugfs_test@read_all_entries:
      fi-snb-2520m:       PASS -> INCOMPLETE (fdo#103713)

    igt@drv_module_reload@basic-no-display:
      fi-cnl-psr:         NOTRUN -> DMESG-WARN (fdo#105395) +5

    igt@drv_selftest@live_hangcheck:
      {fi-cfl-8109u}:     PASS -> DMESG-FAIL (fdo#106560)

    igt@drv_selftest@live_workarounds:
      fi-cnl-psr:         NOTRUN -> DMESG-FAIL (fdo#107292)

    igt@gem_exec_suspend@basic-s3:
      {fi-skl-caroline}:  NOTRUN -> INCOMPLETE (fdo#104108)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#106000)

    {igt@kms_psr@cursor_plane_move}:
      fi-cnl-psr:         NOTRUN -> DMESG-FAIL (fdo#107372) +1

    {igt@kms_psr@primary_mmap_gtt}:
      fi-cnl-psr:         NOTRUN -> DMESG-WARN (fdo#107372)

    igt@prime_vgem@basic-fence-wait-default:
      fi-glk-j4005:       PASS -> DMESG-WARN (fdo#105719)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      fi-bxt-dsi:         DMESG-FAIL (fdo#106560) -> PASS

    igt@gem_exec_create@basic:
      fi-glk-j4005:       DMESG-WARN (fdo#106745) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-glk-j4005:       DMESG-WARN (fdo#106000) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
  fdo#105395 https://bugs.freedesktop.org/show_bug.cgi?id=105395
  fdo#105719 https://bugs.freedesktop.org/show_bug.cgi?id=105719
  fdo#106000 https://bugs.freedesktop.org/show_bug.cgi?id=106000
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106745 https://bugs.freedesktop.org/show_bug.cgi?id=106745
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (48 -> 45) ==

  Additional (3): fi-byt-j1900 fi-skl-caroline fi-cnl-psr 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper 


== Build changes ==

    * Linux: CI_DRM_4547 -> Patchwork_9770

  CI_DRM_4547: 0a7ab192a697e951b2404f3c1ce42c5fa74f9ed1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4575: fe908a01012c9daafafb3410b9407725ca9d4f21 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9770: 3dc2bc1d7aac475f7510af216b435d1f8ed87fc1 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

3dc2bc1d7aac drm/i915/icl: Set TBT IO in Aux transaction
2362a9dca728 drm/i915/icl: Add TBT checks for PLL calculations

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9770/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915/icl: Set TBT IO in Aux transaction

[Srivatsa, Anusha]

>-----Original Message-----
>From: Zanoni, Paulo R
>Sent: Wednesday, July 25, 2018 4:08 PM
>To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel-
>gfx@lists.freedesktop.org
>Subject: Re: [PATCH 2/2] drm/i915/icl: Set TBT IO in Aux transaction
>
>Em Qua, 2018-07-25 às 14:28 -0700, Anusha Srivatsa escreveu:
>> For a TBT sequence, we need to set the IO type to TBT in  DDI_AUX_CTL.
>>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h |  1 +
>> drivers/gpu/drm/i915/intel_dp.c | 34 +++++++++++++++++++++++++----
>> -----
>>  2 files changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index 5530c47..7bdc214 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -5558,6 +5558,7 @@ enum {
>>  #define   DP_AUX_CH_CTL_PSR_DATA_AUX_REG_SKL	(1 << 14)
>>  #define   DP_AUX_CH_CTL_FS_DATA_AUX_REG_SKL	(1 << 13)
>>  #define   DP_AUX_CH_CTL_GTC_DATA_AUX_REG_SKL	(1 << 12)
>> +#define   DP_AUX_CH_CTL_TBT_IO			(1 << 11)
>>  #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL_MASK (0x1f << 5)
>>  #define   DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(c) (((c) - 1) << 5)
>>  #define   DP_AUX_CH_CTL_SYNC_PULSE_SKL(c)   ((c) - 1)
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c
>> b/drivers/gpu/drm/i915/intel_dp.c index cc33d7c..90a8e2f 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -1208,15 +1208,31 @@ static uint32_t skl_get_aux_send_ctl(struct
>> intel_dp *intel_dp,
>>  				      int send_bytes,
>>  				      uint32_t unused)
>>  {
>> -	return DP_AUX_CH_CTL_SEND_BUSY |
>> -	       DP_AUX_CH_CTL_DONE |
>> -	       DP_AUX_CH_CTL_INTERRUPT |
>> -	       DP_AUX_CH_CTL_TIME_OUT_ERROR |
>> -	       DP_AUX_CH_CTL_TIME_OUT_MAX |
>> -	       DP_AUX_CH_CTL_RECEIVE_ERROR |
>> -	       (send_bytes << DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
>> -	       DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
>> -	       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
>> +	struct intel_digital_port *intel_dig_port =
>> dp_to_dig_port(intel_dp);
>> +	struct drm_i915_private *dev_priv = to_i915(intel_dig_port-
>> >base.base.dev);
>> +
>> +	if (INTEL_GEN(dev_priv) >= 11
>
>There's no need for this check, since intel_dig_port->tc_type can't be
>TC_PORT_TBT on platforms earlier than gen11.
 That is true.

>If we were to add a check, I'd suggest to use intel_port_is_tc(), but I don't see the
>need.
>There's a lot of avoidable duplicate code. Please avoid it with something like:
>
>---
>uint32_t ret;
>
>ret = DP_AUX_CH_CTL_SEND_BUSY |
>      DP_AUX_CH_CTL_DONE |
>      etc;
>
>if (intel_dig_port->tc_type == TC_PORT_TBT)
>	ret |= DP_AUX_CH_CTL_TBT_IO;
>
>return ret;
That’s a much better approach.
Thanks for the feedback.

Anusha 

>---
>
>
>Thanks,
>Paulo
>
>>  && intel_dig_port->tc_type == TC_PORT_TBT) {
>> +		return DP_AUX_CH_CTL_SEND_BUSY |
>> +		       DP_AUX_CH_CTL_DONE |
>> +		       DP_AUX_CH_CTL_INTERRUPT |
>> +		       DP_AUX_CH_CTL_TIME_OUT_ERROR |
>> +		       DP_AUX_CH_CTL_TIME_OUT_MAX |
>> +		       DP_AUX_CH_CTL_RECEIVE_ERROR |
>> +		       (send_bytes <<
>> DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
>> +		       DP_AUX_CH_CTL_TBT_IO |
>> +		       DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
>> +		       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
>> +	} else {
>> +		return DP_AUX_CH_CTL_SEND_BUSY |
>> +		       DP_AUX_CH_CTL_DONE |
>> +		       DP_AUX_CH_CTL_INTERRUPT |
>> +		       DP_AUX_CH_CTL_TIME_OUT_ERROR |
>> +		       DP_AUX_CH_CTL_TIME_OUT_MAX |
>> +		       DP_AUX_CH_CTL_RECEIVE_ERROR |
>> +		       (send_bytes <<
>> DP_AUX_CH_CTL_MESSAGE_SIZE_SHIFT) |
>> +		       DP_AUX_CH_CTL_FW_SYNC_PULSE_SKL(32) |
>> +		       DP_AUX_CH_CTL_SYNC_PULSE_SKL(32);
>> +	}
>>  }
>>
>>  static int
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/2] drm/i915/icl: Add TBT checks for PLL calculations

[Srivatsa, Anusha]

>-----Original Message-----
>From: Zanoni, Paulo R
>Sent: Wednesday, July 25, 2018 3:41 PM
>To: Srivatsa, Anusha <anusha.srivatsa@intel.com>; intel-
>gfx@lists.freedesktop.org
>Cc: De Marchi, Lucas <lucas.demarchi@intel.com>
>Subject: Re: [PATCH 1/2] drm/i915/icl: Add TBT checks for PLL calculations
>
>Em Qua, 2018-07-25 às 14:28 -0700, Anusha Srivatsa escreveu:
>> Add missing TBT check in the Pll calculation.
>>
>> v2: do not use a auxiliary function to check if status is TBT or not.
>> (Paulo)
>>
>> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>> Signed-off-by: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> index 7e5e6eb..ba49078 100644
>> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
>> @@ -428,6 +428,7 @@ ibx_get_dpll(struct intel_crtc *crtc, struct
>> intel_crtc_state *crtc_state,
>>  	     struct intel_encoder *encoder)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +
>>  	struct intel_shared_dpll *pll;
>>  	enum intel_dpll_id i;
>>
>
>This chunk does nothing and the space introduced is even against our coding
>style. Please review your own patches before submitting them to the mailing list:
>this is the kind of error that's very easy to catch by looking at the patch file,
>saving everybody's time.
>
Oops :(
Agreed. 
>
>> @@ -2866,6 +2867,9 @@ static struct intel_shared_dpll *
>> icl_get_dpll(struct intel_crtc *crtc, struct intel_crtc_state
>> *crtc_state,
>>  	     struct intel_encoder *encoder)
>>  {
>> +	struct intel_digital_port *intel_dig_port =
>> +			enc_to_dig_port(&encoder->base);
>> +
>
>This chunk also adds a blank line that goes against our coding style.
>
Will take care!
Thanks,
Anusha
>
>>  	struct intel_shared_dpll *pll;
>>  	struct intel_dpll_hw_state pll_state = {};
>>  	enum port port = encoder->port;
>> @@ -2885,7 +2889,7 @@ icl_get_dpll(struct intel_crtc *crtc, struct
>> intel_crtc_state *crtc_state,
>>  	case PORT_D:
>>  	case PORT_E:
>>  	case PORT_F:
>> -		if (0 /* TODO: TBT PLLs */) {
>> +		if (intel_dig_port->tc_type == TC_PORT_TBT) {
>>  			min = DPLL_ID_ICL_TBTPLL;
>>  			max = min;
>>  			ret = icl_calc_dpll_state(crtc_state, encoder, clock,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/2] drm/i915/icl: Add TBT checks for PLL calculations

[Patchwork]

== Series Details ==

Series: series starting with [1/2] drm/i915/icl: Add TBT checks for PLL calculations
URL   : https://patchwork.freedesktop.org/series/47309/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4554 -> Patchwork_9786 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_9786 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9786, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47309/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9786:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_getparams_basic@basic-eu-total:
      fi-kbl-7560u:       PASS -> INCOMPLETE

    
== Known issues ==

  Here are the changes found in Patchwork_9786 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_workarounds:
      fi-skl-6700hq:      PASS -> DMESG-FAIL (fdo#107292)

    igt@kms_busy@basic-flip-a:
      fi-kbl-r:           PASS -> DMESG-WARN (fdo#105602)

    
    ==== Possible fixes ====

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       FAIL (fdo#103841) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-skl-6700hq:      DMESG-WARN (fdo#105998) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#107292 https://bugs.freedesktop.org/show_bug.cgi?id=107292


== Participating hosts (50 -> 43) ==

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-icl-u fi-byt-clapper 


== Build changes ==

    * Linux: CI_DRM_4554 -> Patchwork_9786

  CI_DRM_4554: 5ce2e0fe88bd5a3615abb7289ab98d487201c450 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4576: bcb37a9b20eeec97f15fac2222408cc2e0b77631 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9786: d10692dfab80c00fe4aca1c39f38ffeacb5962f3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d10692dfab80 drm/i915/icl: Set TBT IO in Aux transaction
f3a6b3c581eb drm/i915/icl: Add TBT checks for PLL calculations

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9786/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx