[PATCH] drm/i915/ehl: Add support for DPLL4 (v2)

[PATCH] drm/i915/ehl: Add support for DPLL4 (v2)

[Vivek Kasireddy]

This patch adds support for DPLL4 on EHL that include the
following restrictions:

- DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
  DPLL4 can be used with other DDIs, including DDID
  (combo port A external usage).

- DPLL4 cannot be enabled when DC5 or DC6 are enabled.

- The DPLL4 enable, lock, power enabled, and power state are connected
  to the MGPLL1_ENABLE register.

v2: (suggestions from Bob Paauwe)
- Rework ehl_get_dpll() function to call intel_find_shared_dpll() and
  iterate twice: once for Combo plls and once for MG plls.

- Use MG pll funcs for DPLL4 instead of creating new ones and modify
  mg_pll_enable to include the restrictions for EHL.

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 60 ++++++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index e01c057ce50b..cb756acedc94 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2870,6 +2870,56 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
 	return pll;
 }
 
+static struct intel_shared_dpll *
+ehl_get_dpll(struct intel_crtc_state *crtc_state,
+	     struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+	struct intel_shared_dpll *pll;
+	enum port port = encoder->port;
+	enum intel_dpll_id min, max;
+	bool ret;
+
+	if (!intel_port_is_combophy(dev_priv, port)) {
+		MISSING_CASE(port);
+		return NULL;
+	}
+
+	min = DPLL_ID_ICL_DPLL0;
+	max = DPLL_ID_ICL_DPLL1;
+	ret = icl_calc_dpll_state(crtc_state, encoder);
+	if (ret) {
+		pll = intel_find_shared_dpll(crtc_state, min, max);
+		if (pll) {
+			intel_reference_shared_dpll(pll, crtc_state);
+			return pll;
+		}
+	} else {
+		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
+	}
+
+	if (encoder->type == INTEL_OUTPUT_EDP) {
+		DRM_DEBUG_KMS("Cannot use DPLL4 with EDP.\n");
+		return NULL;
+	}
+
+	min = max = DPLL_ID_ICL_MGPLL1;
+	ret = icl_calc_mg_pll_state(crtc_state, false);
+	if (!ret) {
+		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
+		return NULL;
+	}
+
+	pll = intel_find_shared_dpll(crtc_state, min, max);
+	if (!pll) {
+		DRM_DEBUG_KMS("No PLL selected\n");
+		return NULL;
+	}
+
+	intel_reference_shared_dpll(pll, crtc_state);
+	return pll;
+}
+
 static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				struct intel_shared_dpll *pll,
 				struct intel_dpll_hw_state *hw_state)
@@ -3115,6 +3165,13 @@ static void mg_pll_enable(struct drm_i915_private *dev_priv,
 	i915_reg_t enable_reg =
 		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
 
+	if (IS_ELKHARTLAKE(dev_priv) &&
+	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
+	    I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
+		DRM_ERROR("Cant enable DPLL4 when DC5 or DC6 are enabled\n");
+		return;
+	}
+
 	icl_pll_power_enable(dev_priv, pll, enable_reg);
 
 	icl_mg_pll_write(dev_priv, pll);
@@ -3249,12 +3306,13 @@ static const struct intel_dpll_mgr icl_pll_mgr = {
 static const struct dpll_info ehl_plls[] = {
 	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
 	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
+	{ "DPLL 4", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
 	{ },
 };
 
 static const struct intel_dpll_mgr ehl_pll_mgr = {
 	.dpll_info = ehl_plls,
-	.get_dpll = icl_get_dpll,
+	.get_dpll = ehl_get_dpll,
 	.dump_hw_state = icl_dump_hw_state,
 };
 
-- 
2.14.5

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

✗ Fi.CI.BAT: failure for drm/i915/ehl: Add support for DPLL4 (v2)

[Patchwork]

== Series Details ==

Series: drm/i915/ehl: Add support for DPLL4 (v2)
URL   : https://patchwork.freedesktop.org/series/59029/
State : failure

== Summary ==

CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  DESCEND  objtool
  CHK     include/generated/compile.h
  CC [M]  drivers/gpu/drm/i915/header_test_i915_active_types.o
  CC [M]  drivers/gpu/drm/i915/header_test_i915_gem_context_types.o
  CC [M]  drivers/gpu/drm/i915/header_test_i915_priolist_types.o
  CC [M]  drivers/gpu/drm/i915/header_test_i915_scheduler_types.o
  CC [M]  drivers/gpu/drm/i915/header_test_i915_timeline_types.o
  CC [M]  drivers/gpu/drm/i915/header_test_intel_context_types.o
  CC [M]  drivers/gpu/drm/i915/header_test_intel_engine_types.o
  CC [M]  drivers/gpu/drm/i915/header_test_intel_workarounds_types.o
  CC [M]  drivers/gpu/drm/i915/intel_dpll_mgr.o
drivers/gpu/drm/i915/intel_dpll_mgr.c: In function ‘ehl_get_dpll’:
drivers/gpu/drm/i915/intel_dpll_mgr.c:2907:8: error: too many arguments to function ‘icl_calc_mg_pll_state’
  ret = icl_calc_mg_pll_state(crtc_state, false);
        ^~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/intel_dpll_mgr.c:2649:13: note: declared here
 static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state)
             ^~~~~~~~~~~~~~~~~~~~~
scripts/Makefile.build:275: recipe for target 'drivers/gpu/drm/i915/intel_dpll_mgr.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_dpll_mgr.o] Error 1
scripts/Makefile.build:486: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:486: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:486: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1051: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v2)

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2901 bytes --]

Hi Vivek,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on next-20190404]
[cannot apply to v5.1-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Vivek-Kasireddy/drm-i915-ehl-Add-support-for-DPLL4-v2/20190405-110752
base:   git://anongit.freedesktop.org/drm-intel for-linux-next
config: x86_64-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/i915/intel_dpll_mgr.c: In function 'ehl_get_dpll':
>> drivers/gpu/drm/i915/intel_dpll_mgr.c:2907:8: error: too many arguments to function 'icl_calc_mg_pll_state'
     ret = icl_calc_mg_pll_state(crtc_state, false);
           ^~~~~~~~~~~~~~~~~~~~~
   drivers/gpu/drm/i915/intel_dpll_mgr.c:2649:13: note: declared here
    static bool icl_calc_mg_pll_state(struct intel_crtc_state *crtc_state)
                ^~~~~~~~~~~~~~~~~~~~~

vim +/icl_calc_mg_pll_state +2907 drivers/gpu/drm/i915/intel_dpll_mgr.c

  2872	
  2873	static struct intel_shared_dpll *
  2874	ehl_get_dpll(struct intel_crtc_state *crtc_state,
  2875		     struct intel_encoder *encoder)
  2876	{
  2877		struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
  2878		struct intel_shared_dpll *pll;
  2879		enum port port = encoder->port;
  2880		enum intel_dpll_id min, max;
  2881		bool ret;
  2882	
  2883		if (!intel_port_is_combophy(dev_priv, port)) {
  2884			MISSING_CASE(port);
  2885			return NULL;
  2886		}
  2887	
  2888		min = DPLL_ID_ICL_DPLL0;
  2889		max = DPLL_ID_ICL_DPLL1;
  2890		ret = icl_calc_dpll_state(crtc_state, encoder);
  2891		if (ret) {
  2892			pll = intel_find_shared_dpll(crtc_state, min, max);
  2893			if (pll) {
  2894				intel_reference_shared_dpll(pll, crtc_state);
  2895				return pll;
  2896			}
  2897		} else {
  2898			DRM_DEBUG_KMS("Could not calculate PLL state.\n");
  2899		}
  2900	
  2901		if (encoder->type == INTEL_OUTPUT_EDP) {
  2902			DRM_DEBUG_KMS("Cannot use DPLL4 with EDP.\n");
  2903			return NULL;
  2904		}
  2905	
  2906		min = max = DPLL_ID_ICL_MGPLL1;
> 2907		ret = icl_calc_mg_pll_state(crtc_state, false);
  2908		if (!ret) {
  2909			DRM_DEBUG_KMS("Could not calculate PLL state.\n");
  2910			return NULL;
  2911		}
  2912	
  2913		pll = intel_find_shared_dpll(crtc_state, min, max);
  2914		if (!pll) {
  2915			DRM_DEBUG_KMS("No PLL selected\n");
  2916			return NULL;
  2917		}
  2918	
  2919		intel_reference_shared_dpll(pll, crtc_state);
  2920		return pll;
  2921	}
  2922	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67142 bytes --]

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

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

[PATCH] drm/i915/ehl: Add support for DPLL4 (v3)

[Vivek Kasireddy]

This patch adds support for DPLL4 on EHL that include the
following restrictions:

- DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
  DPLL4 can be used with other DDIs, including DDID
  (combo port A external usage).

- DPLL4 cannot be enabled when DC5 or DC6 are enabled.

- The DPLL4 enable, lock, power enabled, and power state are connected
  to the MGPLL1_ENABLE register.

v2: (suggestions from Bob Paauwe)
- Rework ehl_get_dpll() function to call intel_find_shared_dpll() and
  iterate twice: once for Combo plls and once for MG plls.

- Use MG pll funcs for DPLL4 instead of creating new ones and modify
  mg_pll_enable to include the restrictions for EHL.

v3: Fix compilation error

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 60 ++++++++++++++++++++++++++++++++++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index e01c057ce50b..c3f0b9720c54 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2870,6 +2870,56 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
 	return pll;
 }
 
+static struct intel_shared_dpll *
+ehl_get_dpll(struct intel_crtc_state *crtc_state,
+	     struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+	struct intel_shared_dpll *pll;
+	enum port port = encoder->port;
+	enum intel_dpll_id min, max;
+	bool ret;
+
+	if (!intel_port_is_combophy(dev_priv, port)) {
+		MISSING_CASE(port);
+		return NULL;
+	}
+
+	min = DPLL_ID_ICL_DPLL0;
+	max = DPLL_ID_ICL_DPLL1;
+	ret = icl_calc_dpll_state(crtc_state, encoder);
+	if (ret) {
+		pll = intel_find_shared_dpll(crtc_state, min, max);
+		if (pll) {
+			intel_reference_shared_dpll(pll, crtc_state);
+			return pll;
+		}
+	} else {
+		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
+	}
+
+	if (encoder->type == INTEL_OUTPUT_EDP) {
+		DRM_DEBUG_KMS("Cannot use DPLL4 with EDP.\n");
+		return NULL;
+	}
+
+	min = max = DPLL_ID_ICL_MGPLL1;
+	ret = icl_calc_mg_pll_state(crtc_state);
+	if (!ret) {
+		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
+		return NULL;
+	}
+
+	pll = intel_find_shared_dpll(crtc_state, min, max);
+	if (!pll) {
+		DRM_DEBUG_KMS("No PLL selected\n");
+		return NULL;
+	}
+
+	intel_reference_shared_dpll(pll, crtc_state);
+	return pll;
+}
+
 static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				struct intel_shared_dpll *pll,
 				struct intel_dpll_hw_state *hw_state)
@@ -3115,6 +3165,13 @@ static void mg_pll_enable(struct drm_i915_private *dev_priv,
 	i915_reg_t enable_reg =
 		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
 
+	if (IS_ELKHARTLAKE(dev_priv) &&
+	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
+	    I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
+		DRM_ERROR("Cant enable DPLL4 when DC5 or DC6 are enabled\n");
+		return;
+	}
+
 	icl_pll_power_enable(dev_priv, pll, enable_reg);
 
 	icl_mg_pll_write(dev_priv, pll);
@@ -3249,12 +3306,13 @@ static const struct intel_dpll_mgr icl_pll_mgr = {
 static const struct dpll_info ehl_plls[] = {
 	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
 	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
+	{ "DPLL 4", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
 	{ },
 };
 
 static const struct intel_dpll_mgr ehl_pll_mgr = {
 	.dpll_info = ehl_plls,
-	.get_dpll = icl_get_dpll,
+	.get_dpll = ehl_get_dpll,
 	.dump_hw_state = icl_dump_hw_state,
 };
 
-- 
2.14.5

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v3)

[Ville Syrjälä]

On Fri, Apr 05, 2019 at 10:59:53AM -0700, Vivek Kasireddy wrote:
> This patch adds support for DPLL4 on EHL that include the
> following restrictions:
> 
> - DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
>   DPLL4 can be used with other DDIs, including DDID
>   (combo port A external usage).
> 
> - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> 
> - The DPLL4 enable, lock, power enabled, and power state are connected
>   to the MGPLL1_ENABLE register.
> 
> v2: (suggestions from Bob Paauwe)
> - Rework ehl_get_dpll() function to call intel_find_shared_dpll() and
>   iterate twice: once for Combo plls and once for MG plls.
> 
> - Use MG pll funcs for DPLL4 instead of creating new ones and modify
>   mg_pll_enable to include the restrictions for EHL.
> 
> v3: Fix compilation error
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 60 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index e01c057ce50b..c3f0b9720c54 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2870,6 +2870,56 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
>  	return pll;
>  }
>  
> +static struct intel_shared_dpll *
> +ehl_get_dpll(struct intel_crtc_state *crtc_state,
> +	     struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> +	struct intel_shared_dpll *pll;
> +	enum port port = encoder->port;
> +	enum intel_dpll_id min, max;
> +	bool ret;
> +
> +	if (!intel_port_is_combophy(dev_priv, port)) {
> +		MISSING_CASE(port);
> +		return NULL;
> +	}
> +
> +	min = DPLL_ID_ICL_DPLL0;
> +	max = DPLL_ID_ICL_DPLL1;
> +	ret = icl_calc_dpll_state(crtc_state, encoder);
> +	if (ret) {
> +		pll = intel_find_shared_dpll(crtc_state, min, max);
> +		if (pll) {
> +			intel_reference_shared_dpll(pll, crtc_state);
> +			return pll;
> +		}
> +	} else {
> +		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
> +	}
> +
> +	if (encoder->type == INTEL_OUTPUT_EDP) {
> +		DRM_DEBUG_KMS("Cannot use DPLL4 with EDP.\n");
> +		return NULL;
> +	}
> +
> +	min = max = DPLL_ID_ICL_MGPLL1;
> +	ret = icl_calc_mg_pll_state(crtc_state);
> +	if (!ret) {
> +		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
> +		return NULL;
> +	}
> +
> +	pll = intel_find_shared_dpll(crtc_state, min, max);
> +	if (!pll) {
> +		DRM_DEBUG_KMS("No PLL selected\n");
> +		return NULL;
> +	}
> +
> +	intel_reference_shared_dpll(pll, crtc_state);
> +	return pll;
> +}
> +
>  static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  				struct intel_shared_dpll *pll,
>  				struct intel_dpll_hw_state *hw_state)
> @@ -3115,6 +3165,13 @@ static void mg_pll_enable(struct drm_i915_private *dev_priv,
>  	i915_reg_t enable_reg =
>  		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
>  
> +	if (IS_ELKHARTLAKE(dev_priv) &&
> +	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
> +	    I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
> +		DRM_ERROR("Cant enable DPLL4 when DC5 or DC6 are enabled\n");
> +		return;
> +	}

This looks like dead code, or we messed up earlier already (in which
case it should just be some kind of assert IMO).

Also what is this EHL thing anyway? I can't even find it in the spec.

> +
>  	icl_pll_power_enable(dev_priv, pll, enable_reg);
>  
>  	icl_mg_pll_write(dev_priv, pll);
> @@ -3249,12 +3306,13 @@ static const struct intel_dpll_mgr icl_pll_mgr = {
>  static const struct dpll_info ehl_plls[] = {
>  	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
>  	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> +	{ "DPLL 4", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
>  	{ },
>  };
>  
>  static const struct intel_dpll_mgr ehl_pll_mgr = {
>  	.dpll_info = ehl_plls,
> -	.get_dpll = icl_get_dpll,
> +	.get_dpll = ehl_get_dpll,
>  	.dump_hw_state = icl_dump_hw_state,
>  };
>  
> -- 
> 2.14.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v3)

[Ville Syrjälä]

On Fri, Apr 05, 2019 at 09:33:56PM +0300, Ville Syrjälä wrote:
> On Fri, Apr 05, 2019 at 10:59:53AM -0700, Vivek Kasireddy wrote:
> > This patch adds support for DPLL4 on EHL that include the
> > following restrictions:
> > 
> > - DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
> >   DPLL4 can be used with other DDIs, including DDID
> >   (combo port A external usage).
> > 
> > - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> > 
> > - The DPLL4 enable, lock, power enabled, and power state are connected
> >   to the MGPLL1_ENABLE register.
> > 
> > v2: (suggestions from Bob Paauwe)
> > - Rework ehl_get_dpll() function to call intel_find_shared_dpll() and
> >   iterate twice: once for Combo plls and once for MG plls.
> > 
> > - Use MG pll funcs for DPLL4 instead of creating new ones and modify
> >   mg_pll_enable to include the restrictions for EHL.
> > 
> > v3: Fix compilation error
> > 
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 60 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 59 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > index e01c057ce50b..c3f0b9720c54 100644
> > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > @@ -2870,6 +2870,56 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
> >  	return pll;
> >  }
> >  
> > +static struct intel_shared_dpll *
> > +ehl_get_dpll(struct intel_crtc_state *crtc_state,
> > +	     struct intel_encoder *encoder)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > +	struct intel_shared_dpll *pll;
> > +	enum port port = encoder->port;
> > +	enum intel_dpll_id min, max;
> > +	bool ret;
> > +
> > +	if (!intel_port_is_combophy(dev_priv, port)) {
> > +		MISSING_CASE(port);
> > +		return NULL;
> > +	}
> > +
> > +	min = DPLL_ID_ICL_DPLL0;
> > +	max = DPLL_ID_ICL_DPLL1;
> > +	ret = icl_calc_dpll_state(crtc_state, encoder);
> > +	if (ret) {
> > +		pll = intel_find_shared_dpll(crtc_state, min, max);
> > +		if (pll) {
> > +			intel_reference_shared_dpll(pll, crtc_state);
> > +			return pll;
> > +		}
> > +	} else {
> > +		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
> > +	}
> > +
> > +	if (encoder->type == INTEL_OUTPUT_EDP) {
> > +		DRM_DEBUG_KMS("Cannot use DPLL4 with EDP.\n");
> > +		return NULL;
> > +	}
> > +
> > +	min = max = DPLL_ID_ICL_MGPLL1;
> > +	ret = icl_calc_mg_pll_state(crtc_state);
> > +	if (!ret) {
> > +		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
> > +		return NULL;
> > +	}
> > +
> > +	pll = intel_find_shared_dpll(crtc_state, min, max);
> > +	if (!pll) {
> > +		DRM_DEBUG_KMS("No PLL selected\n");
> > +		return NULL;
> > +	}
> > +
> > +	intel_reference_shared_dpll(pll, crtc_state);
> > +	return pll;
> > +}
> > +
> >  static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
> >  				struct intel_shared_dpll *pll,
> >  				struct intel_dpll_hw_state *hw_state)
> > @@ -3115,6 +3165,13 @@ static void mg_pll_enable(struct drm_i915_private *dev_priv,
> >  	i915_reg_t enable_reg =
> >  		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
> >  
> > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > +	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
> > +	    I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
> > +		DRM_ERROR("Cant enable DPLL4 when DC5 or DC6 are enabled\n");
> > +		return;
> > +	}
> 
> This looks like dead code, or we messed up earlier already (in which
> case it should just be some kind of assert IMO).

Does DMC handle other MG PLLs? I would have thought not, so if we keep
the assert then it should perhaps be unconditional. Hmm. But aren't we
still hodling the modeset power domain when this gets called? If so this
is definitely dead code.

> 
> Also what is this EHL thing anyway? I can't even find it in the spec.
> 
> > +
> >  	icl_pll_power_enable(dev_priv, pll, enable_reg);
> >  
> >  	icl_mg_pll_write(dev_priv, pll);
> > @@ -3249,12 +3306,13 @@ static const struct intel_dpll_mgr icl_pll_mgr = {
> >  static const struct dpll_info ehl_plls[] = {
> >  	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> >  	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> > +	{ "DPLL 4", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
> >  	{ },
> >  };
> >  
> >  static const struct intel_dpll_mgr ehl_pll_mgr = {
> >  	.dpll_info = ehl_plls,
> > -	.get_dpll = icl_get_dpll,
> > +	.get_dpll = ehl_get_dpll,
> >  	.dump_hw_state = icl_dump_hw_state,
> >  };
> >  
> > -- 
> > 2.14.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915/ehl: Add support for DPLL4 (v3)

[Patchwork]

== Series Details ==

Series: drm/i915/ehl: Add support for DPLL4 (v3)
URL   : https://patchwork.freedesktop.org/series/59078/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
946fdfff865f drm/i915/ehl: Add support for DPLL4 (v3)
-:72: CHECK:MULTIPLE_ASSIGNMENTS: multiple assignments should be avoided
#72: FILE: drivers/gpu/drm/i915/intel_dpll_mgr.c:2906:
+	min = max = DPLL_ID_ICL_MGPLL1;

-:97: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#97: FILE: drivers/gpu/drm/i915/intel_dpll_mgr.c:3169:
+	if (IS_ELKHARTLAKE(dev_priv) &&
+	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||

-:110: ERROR:SPACING: space required after that ',' (ctx:VxV)
#110: FILE: drivers/gpu/drm/i915/intel_dpll_mgr.c:3309:
+	{ "DPLL 4", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
 	                                             ^

total: 1 errors, 0 warnings, 2 checks, 83 lines checked

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

✓ Fi.CI.BAT: success for drm/i915/ehl: Add support for DPLL4 (v3)

[Patchwork]

== Series Details ==

Series: drm/i915/ehl: Add support for DPLL4 (v3)
URL   : https://patchwork.freedesktop.org/series/59078/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5881 -> Patchwork_12706
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       PASS -> INCOMPLETE [fdo#108602] / [fdo#108744]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

  * igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
    - fi-byt-clapper:     PASS -> FAIL [fdo#103191] / [fdo#107362]

  * igt@runner@aborted:
    - fi-skl-iommu:       NOTRUN -> FAIL [fdo#104108] / [fdo#108602]

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      FAIL [fdo#108511] -> PASS

  * igt@i915_selftest@live_uncore:
    - fi-ivb-3770:        DMESG-FAIL [fdo#110210] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

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

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210


Participating hosts (49 -> 45)
------------------------------

  Missing    (4): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5881 -> Patchwork_12706

  CI_DRM_5881: b070175c76da1440a747fd023ee6253e573055f8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4931: 019f892e5d1a0a9643cb726c47ce2d99c14b444f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12706: 946fdfff865f08d6c9cae4c6328733b4a8e407f6 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

946fdfff865f drm/i915/ehl: Add support for DPLL4 (v3)

== Logs ==

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v3)

[Vivek Kasireddy]

On Fri, 5 Apr 2019 21:39:11 +0300
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
Hi Ville,

> On Fri, Apr 05, 2019 at 09:33:56PM +0300, Ville Syrjälä wrote:
> > On Fri, Apr 05, 2019 at 10:59:53AM -0700, Vivek Kasireddy wrote:  
> > > This patch adds support for DPLL4 on EHL that include the
> > > following restrictions:
> > > 
> > > - DPLL4 cannot be used with DDIA (combo port A internal eDP
> > > usage). DPLL4 can be used with other DDIs, including DDID
> > >   (combo port A external usage).
> > > 
> > > - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> > > 
> > > - The DPLL4 enable, lock, power enabled, and power state are
> > > connected to the MGPLL1_ENABLE register.
> > > 
> > > v2: (suggestions from Bob Paauwe)
> > > - Rework ehl_get_dpll() function to call intel_find_shared_dpll()
> > > and iterate twice: once for Combo plls and once for MG plls.
> > > 
> > > - Use MG pll funcs for DPLL4 instead of creating new ones and
> > > modify mg_pll_enable to include the restrictions for EHL.
> > > 
> > > v3: Fix compilation error
> > > 
> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > > Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 60
> > > ++++++++++++++++++++++++++++++++++- 1 file changed, 59
> > > insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c index
> > > e01c057ce50b..c3f0b9720c54 100644 ---
> > > a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++
> > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -2870,6 +2870,56 @@
> > > icl_get_dpll(struct intel_crtc_state *crtc_state, return pll;
> > >  }
> > >  
> > > +static struct intel_shared_dpll *
> > > +ehl_get_dpll(struct intel_crtc_state *crtc_state,
> > > +	     struct intel_encoder *encoder)
> > > +{
> > > +	struct drm_i915_private *dev_priv =
> > > to_i915(crtc_state->base.crtc->dev);
> > > +	struct intel_shared_dpll *pll;
> > > +	enum port port = encoder->port;
> > > +	enum intel_dpll_id min, max;
> > > +	bool ret;
> > > +
> > > +	if (!intel_port_is_combophy(dev_priv, port)) {
> > > +		MISSING_CASE(port);
> > > +		return NULL;
> > > +	}
> > > +
> > > +	min = DPLL_ID_ICL_DPLL0;
> > > +	max = DPLL_ID_ICL_DPLL1;
> > > +	ret = icl_calc_dpll_state(crtc_state, encoder);
> > > +	if (ret) {
> > > +		pll = intel_find_shared_dpll(crtc_state, min,
> > > max);
> > > +		if (pll) {
> > > +			intel_reference_shared_dpll(pll,
> > > crtc_state);
> > > +			return pll;
> > > +		}
> > > +	} else {
> > > +		DRM_DEBUG_KMS("Could not calculate PLL
> > > state.\n");
> > > +	}
> > > +
> > > +	if (encoder->type == INTEL_OUTPUT_EDP) {
> > > +		DRM_DEBUG_KMS("Cannot use DPLL4 with EDP.\n");
> > > +		return NULL;
> > > +	}
> > > +
> > > +	min = max = DPLL_ID_ICL_MGPLL1;
> > > +	ret = icl_calc_mg_pll_state(crtc_state);
> > > +	if (!ret) {
> > > +		DRM_DEBUG_KMS("Could not calculate PLL
> > > state.\n");
> > > +		return NULL;
> > > +	}
> > > +
> > > +	pll = intel_find_shared_dpll(crtc_state, min, max);
> > > +	if (!pll) {
> > > +		DRM_DEBUG_KMS("No PLL selected\n");
> > > +		return NULL;
> > > +	}
> > > +
> > > +	intel_reference_shared_dpll(pll, crtc_state);
> > > +	return pll;
> > > +}
> > > +
> > >  static bool mg_pll_get_hw_state(struct drm_i915_private
> > > *dev_priv, struct intel_shared_dpll *pll,
> > >  				struct intel_dpll_hw_state
> > > *hw_state) @@ -3115,6 +3165,13 @@ static void
> > > mg_pll_enable(struct drm_i915_private *dev_priv, i915_reg_t
> > > enable_reg = MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
> > >  
> > > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > > +	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
> > > +	    I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
> > > +		DRM_ERROR("Cant enable DPLL4 when DC5 or DC6 are
> > > enabled\n");
> > > +		return;
> > > +	}  
> > 
> > This looks like dead code, or we messed up earlier already (in which
> > case it should just be some kind of assert IMO).  
Are you suggesting that I put this in a ->prepare() hook or much earlier
in the atomic check/prepare phase? FYI, I am just adding a restriction 
mentioned in the spec concerning this DPLL.

> 
> Does DMC handle other MG PLLs? I would have thought not, so if we keep
> the assert then it should perhaps be unconditional. Hmm. But aren't we
> still hodling the modeset power domain when this gets called? If so
> this is definitely dead code.
I am not sure whether DMC handles other MG PLLs but the spec explicitly
says that DMC will not handle this PLL. Where do you suggest is an
appropriate place to put this assert?

> 
> > 
> > Also what is this EHL thing anyway? I can't even find it in the
> > spec. 
EHL = Elkhart Lake, a new Gen 11 platform.

Thanks,
Vivek

> > > +
> > >  	icl_pll_power_enable(dev_priv, pll, enable_reg);
> > >  
> > >  	icl_mg_pll_write(dev_priv, pll);
> > > @@ -3249,12 +3306,13 @@ static const struct intel_dpll_mgr
> > > icl_pll_mgr = { static const struct dpll_info ehl_plls[] = {
> > >  	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> > >  	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> > > +	{ "DPLL 4", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
> > >  	{ },
> > >  };
> > >  
> > >  static const struct intel_dpll_mgr ehl_pll_mgr = {
> > >  	.dpll_info = ehl_plls,
> > > -	.get_dpll = icl_get_dpll,
> > > +	.get_dpll = ehl_get_dpll,
> > >  	.dump_hw_state = icl_dump_hw_state,
> > >  };
> > >  
> > > -- 
> > > 2.14.5
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx  
> > 
> > -- 
> > Ville Syrjälä
> > Intel  
> 

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v3)

[Lucas De Marchi]

On Fri, Apr 05, 2019 at 10:59:53AM -0700, Vivek Kasireddy wrote:
>This patch adds support for DPLL4 on EHL that include the
>following restrictions:
>
>- DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
>  DPLL4 can be used with other DDIs, including DDID
>  (combo port A external usage).
>
>- DPLL4 cannot be enabled when DC5 or DC6 are enabled.
>
>- The DPLL4 enable, lock, power enabled, and power state are connected
>  to the MGPLL1_ENABLE register.

ok

>
>v2: (suggestions from Bob Paauwe)
>- Rework ehl_get_dpll() function to call intel_find_shared_dpll() and
>  iterate twice: once for Combo plls and once for MG plls.
>
>- Use MG pll funcs for DPLL4 instead of creating new ones and modify
>  mg_pll_enable to include the restrictions for EHL.

these 2 don't match spec.

"3rd PLL for use with combo PHY (DPLL4) and 3rd combo PHY DDI clocks (DDIC clock)"

This is a combophy pll, not a mg phy pll. The only thing that is hooked to
mg registers is the enable. So my understanding is that what you need:

  - use the dpll calculations
  - make sure intel_find_shared_dpll doesn't this if it's for eDP
  - setup the enable/disable to use MG_ENABLE register

>
>v3: Fix compilation error
>
>Cc: Lucas De Marchi <lucas.demarchi@intel.com>
>Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
>Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
>---
> drivers/gpu/drm/i915/intel_dpll_mgr.c | 60 ++++++++++++++++++++++++++++++++++-
> 1 file changed, 59 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
>index e01c057ce50b..c3f0b9720c54 100644
>--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
>+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
>@@ -2870,6 +2870,56 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
> 	return pll;
> }
>
>+static struct intel_shared_dpll *
>+ehl_get_dpll(struct intel_crtc_state *crtc_state,
>+	     struct intel_encoder *encoder)
>+{
>+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>+	struct intel_shared_dpll *pll;
>+	enum port port = encoder->port;
>+	enum intel_dpll_id min, max;
>+	bool ret;
>+
>+	if (!intel_port_is_combophy(dev_priv, port)) {
>+		MISSING_CASE(port);
>+		return NULL;
>+	}
>+
>+	min = DPLL_ID_ICL_DPLL0;
>+	max = DPLL_ID_ICL_DPLL1;
>+	ret = icl_calc_dpll_state(crtc_state, encoder);
>+	if (ret) {
>+		pll = intel_find_shared_dpll(crtc_state, min, max);
>+		if (pll) {
>+			intel_reference_shared_dpll(pll, crtc_state);
>+			return pll;
>+		}
>+	} else {
>+		DRM_DEBUG_KMS("Could not calculate PLL state.\n");

the check for ret is swapped and you are missing a return here.

But given the comments above, I think it would be better to reuse
icl_get_dpll() rather than what you are doing here.

>+	}
>+
>+	if (encoder->type == INTEL_OUTPUT_EDP) {
>+		DRM_DEBUG_KMS("Cannot use DPLL4 with EDP.\n");
>+		return NULL;
>+	}

this is already too late

>+
>+	min = max = DPLL_ID_ICL_MGPLL1;
>+	ret = icl_calc_mg_pll_state(crtc_state);
>+	if (!ret) {
>+		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
>+		return NULL;

again... ret == 0 is success, not otherwise

Lucas De Marchi

>+	}
>+
>+	pll = intel_find_shared_dpll(crtc_state, min, max);
>+	if (!pll) {
>+		DRM_DEBUG_KMS("No PLL selected\n");
>+		return NULL;
>+	}
>+
>+	intel_reference_shared_dpll(pll, crtc_state);
>+	return pll;
>+}
>+
> static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
> 				struct intel_shared_dpll *pll,
> 				struct intel_dpll_hw_state *hw_state)
>@@ -3115,6 +3165,13 @@ static void mg_pll_enable(struct drm_i915_private *dev_priv,
> 	i915_reg_t enable_reg =
> 		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
>
>+	if (IS_ELKHARTLAKE(dev_priv) &&
>+	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
>+	    I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
>+		DRM_ERROR("Cant enable DPLL4 when DC5 or DC6 are enabled\n");
>+		return;
>+	}
>+
> 	icl_pll_power_enable(dev_priv, pll, enable_reg);
>
> 	icl_mg_pll_write(dev_priv, pll);
>@@ -3249,12 +3306,13 @@ static const struct intel_dpll_mgr icl_pll_mgr = {
> static const struct dpll_info ehl_plls[] = {
> 	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> 	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
>+	{ "DPLL 4", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
> 	{ },
> };
>
> static const struct intel_dpll_mgr ehl_pll_mgr = {
> 	.dpll_info = ehl_plls,
>-	.get_dpll = icl_get_dpll,
>+	.get_dpll = ehl_get_dpll,
> 	.dump_hw_state = icl_dump_hw_state,
> };
>
>-- 
>2.14.5
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for drm/i915/ehl: Add support for DPLL4 (v3)

[Patchwork]

== Series Details ==

Series: drm/i915/ehl: Add support for DPLL4 (v3)
URL   : https://patchwork.freedesktop.org/series/59078/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5881_full -> Patchwork_12706_full
====================================================

Summary
-------

  **FAILURE**

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

  

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_pm_rpm@cursor-dpms:
    - shard-glk:          PASS -> DMESG-WARN

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_create@create-clear:
    - shard-snb:          PASS -> INCOMPLETE [fdo#105411]

  * igt@gem_ctx_isolation@rcs0-dirty-switch:
    - shard-glk:          PASS -> INCOMPLETE [fdo#103359] / [k.org#198133]

  * igt@gem_exec_schedule@preempt-other-chain-bsd2:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +3

  * igt@gem_exec_store@cachelines-bsd1:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +80

  * igt@gem_mmap_gtt@hang:
    - shard-iclb:         PASS -> FAIL [fdo#109677]

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#109801]

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         PASS -> FAIL [fdo#108686]

  * igt@i915_pm_rpm@debugfs-forcewake-user:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#107807]

  * igt@i915_pm_rpm@dpms-mode-unset-non-lpsp:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103558] / [fdo#105602] +2

  * igt@i915_pm_rpm@gem-execbuf:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103313] / [fdo#103558]

  * igt@kms_atomic_transition@4x-modeset-transitions-nonblocking-fencing:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +9

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-apl:          NOTRUN -> DMESG-WARN [fdo#110222]
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#110222] +2

  * igt@kms_busy@extended-modeset-hang-oldfb-render-f:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +1

  * igt@kms_busy@extended-pageflip-hang-newfb-render-c:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +6

  * igt@kms_busy@extended-pageflip-hang-oldfb-render-f:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_ccs@pipe-c-missing-ccs-buffer:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +36

  * igt@kms_chamelium@dp-crc-single:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284] +1

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@kms_cursor_edge_walk@pipe-a-64x64-left-edge:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103313] / [fdo#103558] / [fdo#105602]

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274]

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-iclb:         PASS -> FAIL [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-iclb:         PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +8

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render:
    - shard-iclb:         NOTRUN -> FAIL [fdo#109247] +3

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-shrfb-fliptrack:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +3

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-mmap-gtt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +22

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-move:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +99

  * igt@kms_lease@atomic_implicit_crtc:
    - shard-skl:          NOTRUN -> FAIL [fdo#110279]
    - shard-apl:          NOTRUN -> FAIL [fdo#110279]

  * igt@kms_lease@setcrtc_implicit_plane:
    - shard-skl:          NOTRUN -> FAIL [fdo#110281]

  * igt@kms_pipe_crc_basic@read-crc-pipe-e:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590]

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145] +1

  * igt@kms_psr@cursor_render:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215] +2

  * igt@kms_psr@no_drrs:
    - shard-iclb:         PASS -> FAIL [fdo#108341]

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +1

  * igt@kms_psr@psr2_sprite_plane_onoff:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109441]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_setmode@basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#99912]

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-snb:          PASS -> DMESG-WARN [fdo#102365]

  * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
    - shard-apl:          PASS -> FAIL [fdo#104894] +1

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-iclb:         PASS -> FAIL [fdo#104894]

  * igt@perf@polling:
    - shard-iclb:         PASS -> FAIL [fdo#108587]

  * igt@runner@aborted:
    - shard-kbl:          NOTRUN -> FAIL [fdo#109383]

  
#### Possible fixes ####

  * igt@gem_ppgtt@blt-vs-render-ctx0:
    - shard-iclb:         INCOMPLETE [fdo#109801] -> PASS

  * igt@i915_pm_rpm@universal-planes-dpms:
    - shard-iclb:         INCOMPLETE [fdo#107713] / [fdo#108840] -> PASS

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-iclb:         DMESG-WARN [fdo#110222] -> PASS

  * igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          FAIL [fdo#106509] / [fdo#107409] -> PASS

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic:
    - shard-iclb:         FAIL [fdo#103355] -> PASS

  * igt@kms_fbcon_fbt@fbc:
    - shard-iclb:         DMESG-WARN [fdo#109593] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          FAIL [fdo#105363] -> PASS

  * igt@kms_flip@flip-vs-suspend:
    - shard-kbl:          DMESG-WARN [fdo#108566] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-iclb:         FAIL [fdo#103375] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +2

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +16

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-pwrite:
    - shard-iclb:         FAIL [fdo#105682] / [fdo#109247] -> PASS

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] -> PASS

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_psr@primary_mmap_cpu:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS +1

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +3

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  * igt@kms_vblank@pipe-a-ts-continuation-modeset-rpm:
    - shard-apl:          FAIL [fdo#104894] -> PASS +1

  * igt@perf@oa-exponents:
    - shard-glk:          FAIL [fdo#105483] -> PASS

  
#### Warnings ####

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-c-planes:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> DMESG-WARN [fdo#103558] / [fdo#103841] / [fdo#105079] / [fdo#105602]

  * igt@runner@aborted:
    - shard-glk:          FAIL [fdo#109373] / [k.org#202321] -> ( 2 FAIL ) [fdo#109373] / [k.org#202321]

  
  [fdo#102365]: https://bugs.freedesktop.org/show_bug.cgi?id=102365
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103313]: https://bugs.freedesktop.org/show_bug.cgi?id=103313
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103841]: https://bugs.freedesktop.org/show_bug.cgi?id=103841
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105483]: https://bugs.freedesktop.org/show_bug.cgi?id=105483
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108587]: https://bugs.freedesktop.org/show_bug.cgi?id=108587
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373
  [fdo#109383]: https://bugs.freedesktop.org/show_bug.cgi?id=109383
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
  [fdo#109677]: https://bugs.freedesktop.org/show_bug.cgi?id=109677
  [fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
  [fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279
  [fdo#110281]: https://bugs.freedesktop.org/show_bug.cgi?id=110281
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

    * Linux: CI_DRM_5881 -> Patchwork_12706

  CI_DRM_5881: b070175c76da1440a747fd023ee6253e573055f8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4931: 019f892e5d1a0a9643cb726c47ce2d99c14b444f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12706: 946fdfff865f08d6c9cae4c6328733b4a8e407f6 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v3)

[Ville Syrjälä]

On Fri, Apr 05, 2019 at 04:33:30PM -0700, Vivek Kasireddy wrote:
> On Fri, 5 Apr 2019 21:39:11 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> Hi Ville,
> 
> > On Fri, Apr 05, 2019 at 09:33:56PM +0300, Ville Syrjälä wrote:
> > > On Fri, Apr 05, 2019 at 10:59:53AM -0700, Vivek Kasireddy wrote:  
> > > > This patch adds support for DPLL4 on EHL that include the
> > > > following restrictions:
> > > > 
> > > > - DPLL4 cannot be used with DDIA (combo port A internal eDP
> > > > usage). DPLL4 can be used with other DDIs, including DDID
> > > >   (combo port A external usage).
> > > > 
> > > > - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> > > > 
> > > > - The DPLL4 enable, lock, power enabled, and power state are
> > > > connected to the MGPLL1_ENABLE register.
> > > > 
> > > > v2: (suggestions from Bob Paauwe)
> > > > - Rework ehl_get_dpll() function to call intel_find_shared_dpll()
> > > > and iterate twice: once for Combo plls and once for MG plls.
> > > > 
> > > > - Use MG pll funcs for DPLL4 instead of creating new ones and
> > > > modify mg_pll_enable to include the restrictions for EHL.
> > > > 
> > > > v3: Fix compilation error
> > > > 
> > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > > > Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 60
> > > > ++++++++++++++++++++++++++++++++++- 1 file changed, 59
> > > > insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c index
> > > > e01c057ce50b..c3f0b9720c54 100644 ---
> > > > a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++
> > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -2870,6 +2870,56 @@
> > > > icl_get_dpll(struct intel_crtc_state *crtc_state, return pll;
> > > >  }
> > > >  
> > > > +static struct intel_shared_dpll *
> > > > +ehl_get_dpll(struct intel_crtc_state *crtc_state,
> > > > +	     struct intel_encoder *encoder)
> > > > +{
> > > > +	struct drm_i915_private *dev_priv =
> > > > to_i915(crtc_state->base.crtc->dev);
> > > > +	struct intel_shared_dpll *pll;
> > > > +	enum port port = encoder->port;
> > > > +	enum intel_dpll_id min, max;
> > > > +	bool ret;
> > > > +
> > > > +	if (!intel_port_is_combophy(dev_priv, port)) {
> > > > +		MISSING_CASE(port);
> > > > +		return NULL;
> > > > +	}
> > > > +
> > > > +	min = DPLL_ID_ICL_DPLL0;
> > > > +	max = DPLL_ID_ICL_DPLL1;
> > > > +	ret = icl_calc_dpll_state(crtc_state, encoder);
> > > > +	if (ret) {
> > > > +		pll = intel_find_shared_dpll(crtc_state, min,
> > > > max);
> > > > +		if (pll) {
> > > > +			intel_reference_shared_dpll(pll,
> > > > crtc_state);
> > > > +			return pll;
> > > > +		}
> > > > +	} else {
> > > > +		DRM_DEBUG_KMS("Could not calculate PLL
> > > > state.\n");
> > > > +	}
> > > > +
> > > > +	if (encoder->type == INTEL_OUTPUT_EDP) {
> > > > +		DRM_DEBUG_KMS("Cannot use DPLL4 with EDP.\n");
> > > > +		return NULL;
> > > > +	}
> > > > +
> > > > +	min = max = DPLL_ID_ICL_MGPLL1;
> > > > +	ret = icl_calc_mg_pll_state(crtc_state);
> > > > +	if (!ret) {
> > > > +		DRM_DEBUG_KMS("Could not calculate PLL
> > > > state.\n");
> > > > +		return NULL;
> > > > +	}
> > > > +
> > > > +	pll = intel_find_shared_dpll(crtc_state, min, max);
> > > > +	if (!pll) {
> > > > +		DRM_DEBUG_KMS("No PLL selected\n");
> > > > +		return NULL;
> > > > +	}
> > > > +
> > > > +	intel_reference_shared_dpll(pll, crtc_state);
> > > > +	return pll;
> > > > +}
> > > > +
> > > >  static bool mg_pll_get_hw_state(struct drm_i915_private
> > > > *dev_priv, struct intel_shared_dpll *pll,
> > > >  				struct intel_dpll_hw_state
> > > > *hw_state) @@ -3115,6 +3165,13 @@ static void
> > > > mg_pll_enable(struct drm_i915_private *dev_priv, i915_reg_t
> > > > enable_reg = MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
> > > >  
> > > > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > > > +	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
> > > > +	    I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
> > > > +		DRM_ERROR("Cant enable DPLL4 when DC5 or DC6 are
> > > > enabled\n");
> > > > +		return;
> > > > +	}  
> > > 
> > > This looks like dead code, or we messed up earlier already (in which
> > > case it should just be some kind of assert IMO).  
> Are you suggesting that I put this in a ->prepare() hook or much earlier
> in the atomic check/prepare phase? FYI, I am just adding a restriction 
> mentioned in the spec concerning this DPLL.
> 
> > 
> > Does DMC handle other MG PLLs? I would have thought not, so if we keep
> > the assert then it should perhaps be unconditional. Hmm. But aren't we
> > still hodling the modeset power domain when this gets called? If so
> > this is definitely dead code.
> I am not sure whether DMC handles other MG PLLs but the spec explicitly
> says that DMC will not handle this PLL. Where do you suggest is an
> appropriate place to put this assert?

DC5/6 enable probably is the best, if we even want the assert. And I'm
not convinced that we do since we don't have it for any other PLL
either.

> 
> > 
> > > 
> > > Also what is this EHL thing anyway? I can't even find it in the
> > > spec. 
> EHL = Elkhart Lake, a new Gen 11 platform.
> 
> Thanks,
> Vivek
> 
> > > > +
> > > >  	icl_pll_power_enable(dev_priv, pll, enable_reg);
> > > >  
> > > >  	icl_mg_pll_write(dev_priv, pll);
> > > > @@ -3249,12 +3306,13 @@ static const struct intel_dpll_mgr
> > > > icl_pll_mgr = { static const struct dpll_info ehl_plls[] = {
> > > >  	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> > > >  	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> > > > +	{ "DPLL 4", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
> > > >  	{ },
> > > >  };
> > > >  
> > > >  static const struct intel_dpll_mgr ehl_pll_mgr = {
> > > >  	.dpll_info = ehl_plls,
> > > > -	.get_dpll = icl_get_dpll,
> > > > +	.get_dpll = ehl_get_dpll,
> > > >  	.dump_hw_state = icl_dump_hw_state,
> > > >  };
> > > >  
> > > > -- 
> > > > 2.14.5
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx  
> > > 
> > > -- 
> > > Ville Syrjälä
> > > Intel  
> > 

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v3)

[Vivek Kasireddy]

On Fri, 5 Apr 2019 17:46:38 -0700
Lucas De Marchi <lucas.demarchi@intel.com> wrote:
Hi,

> On Fri, Apr 05, 2019 at 10:59:53AM -0700, Vivek Kasireddy wrote:
> >This patch adds support for DPLL4 on EHL that include the
> >following restrictions:
> >
> >- DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
> >  DPLL4 can be used with other DDIs, including DDID
> >  (combo port A external usage).
> >
> >- DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> >
> >- The DPLL4 enable, lock, power enabled, and power state are
> >connected
> >  to the MGPLL1_ENABLE register.  
> 
> ok
> 
> >
> >v2: (suggestions from Bob Paauwe)
> >- Rework ehl_get_dpll() function to call intel_find_shared_dpll() and
> >  iterate twice: once for Combo plls and once for MG plls.
> >
> >- Use MG pll funcs for DPLL4 instead of creating new ones and modify
> >  mg_pll_enable to include the restrictions for EHL.  
> 
> these 2 don't match spec.
> 
> "3rd PLL for use with combo PHY (DPLL4) and 3rd combo PHY DDI clocks
> (DDIC clock)"
> 
> This is a combophy pll, not a mg phy pll. The only thing that is
> hooked to mg registers is the enable. So my understanding is that
> what you need:
> 
>   - use the dpll calculations
>   - make sure intel_find_shared_dpll doesn't this if it's for eDP
>   - setup the enable/disable to use MG_ENABLE register
Looks like my interpretation of the spec is different from yours but
your comments make sense. Should I create a new ID for this DPLL
or juse re-use DPLL_ID_ICL_MGPLL1?

> 
> >
> >v3: Fix compilation error
> >
> >Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> >Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
> >---
> > drivers/gpu/drm/i915/intel_dpll_mgr.c | 60
> > ++++++++++++++++++++++++++++++++++- 1 file changed, 59
> > insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> >b/drivers/gpu/drm/i915/intel_dpll_mgr.c index
> >e01c057ce50b..c3f0b9720c54 100644 ---
> >a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++
> >b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -2870,6 +2870,56 @@
> >icl_get_dpll(struct intel_crtc_state *crtc_state,
> > 	return pll;
> > }
> >
> >+static struct intel_shared_dpll *
> >+ehl_get_dpll(struct intel_crtc_state *crtc_state,
> >+	     struct intel_encoder *encoder)
> >+{
> >+	struct drm_i915_private *dev_priv =
> >to_i915(crtc_state->base.crtc->dev);
> >+	struct intel_shared_dpll *pll;
> >+	enum port port = encoder->port;
> >+	enum intel_dpll_id min, max;
> >+	bool ret;
> >+
> >+	if (!intel_port_is_combophy(dev_priv, port)) {
> >+		MISSING_CASE(port);
> >+		return NULL;
> >+	}
> >+
> >+	min = DPLL_ID_ICL_DPLL0;
> >+	max = DPLL_ID_ICL_DPLL1;
> >+	ret = icl_calc_dpll_state(crtc_state, encoder);
> >+	if (ret) {
> >+		pll = intel_find_shared_dpll(crtc_state, min, max);
> >+		if (pll) {
> >+			intel_reference_shared_dpll(pll,
> >crtc_state);
> >+			return pll;
> >+		}
> >+	} else {
> >+		DRM_DEBUG_KMS("Could not calculate PLL state.\n");  
> 
> the check for ret is swapped and you are missing a return here.
Unless I am reading it utterly wrong, icl_get_dpll has this:
if (!ret) {
                DRM_DEBUG_KMS("Could not calculate PLL state.\n");
                return NULL;

> 
> But given the comments above, I think it would be better to reuse
> icl_get_dpll() rather than what you are doing here.
I could have used icl_get_dpll() but thought it would be much cleaner
to have a separate function for EHL; otherwise, I guess I need to
sprinkle icl_get_dpll with many if(EHL) statements.

> 
> >+	}
> >+
> >+	if (encoder->type == INTEL_OUTPUT_EDP) {
> >+		DRM_DEBUG_KMS("Cannot use DPLL4 with EDP.\n");
> >+		return NULL;
> >+	}  
> 
> this is already too late
The idea was if we have EDP being used, then we first try to find if
one of the combo PHY DPLLs are available to be used. If they are
not, then we come here and return as we cannot use this one either.

> 
> >+
> >+	min = max = DPLL_ID_ICL_MGPLL1;
> >+	ret = icl_calc_mg_pll_state(crtc_state);
> >+	if (!ret) {
> >+		DRM_DEBUG_KMS("Could not calculate PLL state.\n");
> >+		return NULL;  
> 
> again... ret == 0 is success, not otherwise
I'll send out a v4 with your suggestions soon.

Thanks,
Vivek
> 
> Lucas De Marchi
> 
> >+	}
> >+
> >+	pll = intel_find_shared_dpll(crtc_state, min, max);
> >+	if (!pll) {
> >+		DRM_DEBUG_KMS("No PLL selected\n");
> >+		return NULL;
> >+	}
> >+
> >+	intel_reference_shared_dpll(pll, crtc_state);
> >+	return pll;
> >+}
> >+
> > static bool mg_pll_get_hw_state(struct drm_i915_private *dev_priv,
> > 				struct intel_shared_dpll *pll,
> > 				struct intel_dpll_hw_state
> > *hw_state)
> >@@ -3115,6 +3165,13 @@ static void mg_pll_enable(struct
> >drm_i915_private *dev_priv,
> > 	i915_reg_t enable_reg =
> > 		MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id));
> >
> >+	if (IS_ELKHARTLAKE(dev_priv) &&
> >+	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
> >+	    I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
> >+		DRM_ERROR("Cant enable DPLL4 when DC5 or DC6 are
> >enabled\n");
> >+		return;
> >+	}
> >+
> > 	icl_pll_power_enable(dev_priv, pll, enable_reg);
> >
> > 	icl_mg_pll_write(dev_priv, pll);
> >@@ -3249,12 +3306,13 @@ static const struct intel_dpll_mgr
> >icl_pll_mgr = {
> > static const struct dpll_info ehl_plls[] = {
> > 	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> > 	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> >+	{ "DPLL 4", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
> > 	{ },
> > };
> >
> > static const struct intel_dpll_mgr ehl_pll_mgr = {
> > 	.dpll_info = ehl_plls,
> >-	.get_dpll = icl_get_dpll,
> >+	.get_dpll = ehl_get_dpll,
> > 	.dump_hw_state = icl_dump_hw_state,
> > };
> >
> >-- 
> >2.14.5
> >  

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v3)

[Vivek Kasireddy]

On Mon, 8 Apr 2019 12:11:15 +0300
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
Hi,

> On Fri, Apr 05, 2019 at 04:33:30PM -0700, Vivek Kasireddy wrote:
> > On Fri, 5 Apr 2019 21:39:11 +0300
> > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > Hi Ville,
> >   
> > > On Fri, Apr 05, 2019 at 09:33:56PM +0300, Ville Syrjälä wrote:  
> > > > On Fri, Apr 05, 2019 at 10:59:53AM -0700, Vivek Kasireddy
> > > > wrote:    
> > > > > This patch adds support for DPLL4 on EHL that include the
> > > > > following restrictions:
> > > > > 
> > > > > - DPLL4 cannot be used with DDIA (combo port A internal eDP
> > > > > usage). DPLL4 can be used with other DDIs, including DDID
> > > > >   (combo port A external usage).
> > > > > 
> > > > > - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> > > > > 
> > > > > - The DPLL4 enable, lock, power enabled, and power state are
> > > > > connected to the MGPLL1_ENABLE register.
> > > > > 
> > > > > v2: (suggestions from Bob Paauwe)
> > > > > - Rework ehl_get_dpll() function to call
> > > > > intel_find_shared_dpll() and iterate twice: once for Combo
> > > > > plls and once for MG plls.
> > > > > 
> > > > > - Use MG pll funcs for DPLL4 instead of creating new ones and
> > > > > modify mg_pll_enable to include the restrictions for EHL.
> > > > > 
> > > > > v3: Fix compilation error
> > > > > 
> > > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > > > > Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 60
> > > > > ++++++++++++++++++++++++++++++++++- 1 file changed, 59
> > > > > insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c index
> > > > > e01c057ce50b..c3f0b9720c54 100644 ---
> > > > > a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++
> > > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -2870,6 +2870,56 @@
> > > > > icl_get_dpll(struct intel_crtc_state *crtc_state, return pll;
> > > > >  }
> > > > >  
> > > > > +static struct intel_shared_dpll *
> > > > > +ehl_get_dpll(struct intel_crtc_state *crtc_state,
> > > > > +	     struct intel_encoder *encoder)
> > > > > +{
> > > > > +	struct drm_i915_private *dev_priv =
> > > > > to_i915(crtc_state->base.crtc->dev);
> > > > > +	struct intel_shared_dpll *pll;
> > > > > +	enum port port = encoder->port;
> > > > > +	enum intel_dpll_id min, max;
> > > > > +	bool ret;
> > > > > +
> > > > > +	if (!intel_port_is_combophy(dev_priv, port)) {
> > > > > +		MISSING_CASE(port);
> > > > > +		return NULL;
> > > > > +	}
> > > > > +
> > > > > +	min = DPLL_ID_ICL_DPLL0;
> > > > > +	max = DPLL_ID_ICL_DPLL1;
> > > > > +	ret = icl_calc_dpll_state(crtc_state, encoder);
> > > > > +	if (ret) {
> > > > > +		pll = intel_find_shared_dpll(crtc_state, min,
> > > > > max);
> > > > > +		if (pll) {
> > > > > +			intel_reference_shared_dpll(pll,
> > > > > crtc_state);
> > > > > +			return pll;
> > > > > +		}
> > > > > +	} else {
> > > > > +		DRM_DEBUG_KMS("Could not calculate PLL
> > > > > state.\n");
> > > > > +	}
> > > > > +
> > > > > +	if (encoder->type == INTEL_OUTPUT_EDP) {
> > > > > +		DRM_DEBUG_KMS("Cannot use DPLL4 with
> > > > > EDP.\n");
> > > > > +		return NULL;
> > > > > +	}
> > > > > +
> > > > > +	min = max = DPLL_ID_ICL_MGPLL1;
> > > > > +	ret = icl_calc_mg_pll_state(crtc_state);
> > > > > +	if (!ret) {
> > > > > +		DRM_DEBUG_KMS("Could not calculate PLL
> > > > > state.\n");
> > > > > +		return NULL;
> > > > > +	}
> > > > > +
> > > > > +	pll = intel_find_shared_dpll(crtc_state, min, max);
> > > > > +	if (!pll) {
> > > > > +		DRM_DEBUG_KMS("No PLL selected\n");
> > > > > +		return NULL;
> > > > > +	}
> > > > > +
> > > > > +	intel_reference_shared_dpll(pll, crtc_state);
> > > > > +	return pll;
> > > > > +}
> > > > > +
> > > > >  static bool mg_pll_get_hw_state(struct drm_i915_private
> > > > > *dev_priv, struct intel_shared_dpll *pll,
> > > > >  				struct intel_dpll_hw_state
> > > > > *hw_state) @@ -3115,6 +3165,13 @@ static void
> > > > > mg_pll_enable(struct drm_i915_private *dev_priv, i915_reg_t
> > > > > enable_reg =
> > > > > MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id)); 
> > > > > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > > > > +	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
> > > > > +	    I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
> > > > > +		DRM_ERROR("Cant enable DPLL4 when DC5 or DC6
> > > > > are enabled\n");
> > > > > +		return;
> > > > > +	}    
> > > > 
> > > > This looks like dead code, or we messed up earlier already (in
> > > > which case it should just be some kind of assert IMO).    
> > Are you suggesting that I put this in a ->prepare() hook or much
> > earlier in the atomic check/prepare phase? FYI, I am just adding a
> > restriction mentioned in the spec concerning this DPLL.
> >   
> > > 
> > > Does DMC handle other MG PLLs? I would have thought not, so if we
> > > keep the assert then it should perhaps be unconditional. Hmm. But
> > > aren't we still hodling the modeset power domain when this gets
> > > called? If so this is definitely dead code.  
> > I am not sure whether DMC handles other MG PLLs but the spec
> > explicitly says that DMC will not handle this PLL. Where do you
> > suggest is an appropriate place to put this assert?  
> 
> DC5/6 enable probably is the best, if we even want the assert. And I'm
> not convinced that we do since we don't have it for any other PLL
> either.
As I mentioned earlier, the spec says we cannot enable this DPLL for EHL
if DC5 or DC6 is enabled. Do you think we should disable DC5/DC6 on EHL
in order to enable this DPLL on EHL? Should the user/system integrator 
be making that decision via a module parameter?

Thanks,
Vivek
> 
> >   
> > >   
> > > > 
> > > > Also what is this EHL thing anyway? I can't even find it in the
> > > > spec.   
> > EHL = Elkhart Lake, a new Gen 11 platform.
> > 
> > Thanks,
> > Vivek
> >   
> > > > > +
> > > > >  	icl_pll_power_enable(dev_priv, pll, enable_reg);
> > > > >  
> > > > >  	icl_mg_pll_write(dev_priv, pll);
> > > > > @@ -3249,12 +3306,13 @@ static const struct intel_dpll_mgr
> > > > > icl_pll_mgr = { static const struct dpll_info ehl_plls[] = {
> > > > >  	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> > > > >  	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> > > > > +	{ "DPLL 4", &mg_pll_funcs, DPLL_ID_ICL_MGPLL1,0 },
> > > > >  	{ },
> > > > >  };
> > > > >  
> > > > >  static const struct intel_dpll_mgr ehl_pll_mgr = {
> > > > >  	.dpll_info = ehl_plls,
> > > > > -	.get_dpll = icl_get_dpll,
> > > > > +	.get_dpll = ehl_get_dpll,
> > > > >  	.dump_hw_state = icl_dump_hw_state,
> > > > >  };
> > > > >  
> > > > > -- 
> > > > > 2.14.5
> > > > > 
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx    
> > > > 
> > > > -- 
> > > > Ville Syrjälä
> > > > Intel    
> > >   
> 

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v3)

[Ville Syrjälä]

On Tue, Apr 09, 2019 at 05:00:44PM -0700, Vivek Kasireddy wrote:
> On Mon, 8 Apr 2019 12:11:15 +0300
> Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> Hi,
> 
> > On Fri, Apr 05, 2019 at 04:33:30PM -0700, Vivek Kasireddy wrote:
> > > On Fri, 5 Apr 2019 21:39:11 +0300
> > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > > Hi Ville,
> > >   
> > > > On Fri, Apr 05, 2019 at 09:33:56PM +0300, Ville Syrjälä wrote:  
> > > > > On Fri, Apr 05, 2019 at 10:59:53AM -0700, Vivek Kasireddy
> > > > > wrote:    
> > > > > > This patch adds support for DPLL4 on EHL that include the
> > > > > > following restrictions:
> > > > > > 
> > > > > > - DPLL4 cannot be used with DDIA (combo port A internal eDP
> > > > > > usage). DPLL4 can be used with other DDIs, including DDID
> > > > > >   (combo port A external usage).
> > > > > > 
> > > > > > - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> > > > > > 
> > > > > > - The DPLL4 enable, lock, power enabled, and power state are
> > > > > > connected to the MGPLL1_ENABLE register.
> > > > > > 
> > > > > > v2: (suggestions from Bob Paauwe)
> > > > > > - Rework ehl_get_dpll() function to call
> > > > > > intel_find_shared_dpll() and iterate twice: once for Combo
> > > > > > plls and once for MG plls.
> > > > > > 
> > > > > > - Use MG pll funcs for DPLL4 instead of creating new ones and
> > > > > > modify mg_pll_enable to include the restrictions for EHL.
> > > > > > 
> > > > > > v3: Fix compilation error
> > > > > > 
> > > > > > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > > > > > Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 60
> > > > > > ++++++++++++++++++++++++++++++++++- 1 file changed, 59
> > > > > > insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > > > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c index
> > > > > > e01c057ce50b..c3f0b9720c54 100644 ---
> > > > > > a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++
> > > > > > b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -2870,6 +2870,56 @@
> > > > > > icl_get_dpll(struct intel_crtc_state *crtc_state, return pll;
> > > > > >  }
> > > > > >  
> > > > > > +static struct intel_shared_dpll *
> > > > > > +ehl_get_dpll(struct intel_crtc_state *crtc_state,
> > > > > > +	     struct intel_encoder *encoder)
> > > > > > +{
> > > > > > +	struct drm_i915_private *dev_priv =
> > > > > > to_i915(crtc_state->base.crtc->dev);
> > > > > > +	struct intel_shared_dpll *pll;
> > > > > > +	enum port port = encoder->port;
> > > > > > +	enum intel_dpll_id min, max;
> > > > > > +	bool ret;
> > > > > > +
> > > > > > +	if (!intel_port_is_combophy(dev_priv, port)) {
> > > > > > +		MISSING_CASE(port);
> > > > > > +		return NULL;
> > > > > > +	}
> > > > > > +
> > > > > > +	min = DPLL_ID_ICL_DPLL0;
> > > > > > +	max = DPLL_ID_ICL_DPLL1;
> > > > > > +	ret = icl_calc_dpll_state(crtc_state, encoder);
> > > > > > +	if (ret) {
> > > > > > +		pll = intel_find_shared_dpll(crtc_state, min,
> > > > > > max);
> > > > > > +		if (pll) {
> > > > > > +			intel_reference_shared_dpll(pll,
> > > > > > crtc_state);
> > > > > > +			return pll;
> > > > > > +		}
> > > > > > +	} else {
> > > > > > +		DRM_DEBUG_KMS("Could not calculate PLL
> > > > > > state.\n");
> > > > > > +	}
> > > > > > +
> > > > > > +	if (encoder->type == INTEL_OUTPUT_EDP) {
> > > > > > +		DRM_DEBUG_KMS("Cannot use DPLL4 with
> > > > > > EDP.\n");
> > > > > > +		return NULL;
> > > > > > +	}
> > > > > > +
> > > > > > +	min = max = DPLL_ID_ICL_MGPLL1;
> > > > > > +	ret = icl_calc_mg_pll_state(crtc_state);
> > > > > > +	if (!ret) {
> > > > > > +		DRM_DEBUG_KMS("Could not calculate PLL
> > > > > > state.\n");
> > > > > > +		return NULL;
> > > > > > +	}
> > > > > > +
> > > > > > +	pll = intel_find_shared_dpll(crtc_state, min, max);
> > > > > > +	if (!pll) {
> > > > > > +		DRM_DEBUG_KMS("No PLL selected\n");
> > > > > > +		return NULL;
> > > > > > +	}
> > > > > > +
> > > > > > +	intel_reference_shared_dpll(pll, crtc_state);
> > > > > > +	return pll;
> > > > > > +}
> > > > > > +
> > > > > >  static bool mg_pll_get_hw_state(struct drm_i915_private
> > > > > > *dev_priv, struct intel_shared_dpll *pll,
> > > > > >  				struct intel_dpll_hw_state
> > > > > > *hw_state) @@ -3115,6 +3165,13 @@ static void
> > > > > > mg_pll_enable(struct drm_i915_private *dev_priv, i915_reg_t
> > > > > > enable_reg =
> > > > > > MG_PLL_ENABLE(icl_pll_id_to_tc_port(pll->info->id)); 
> > > > > > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > > > > > +	   (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5 ||
> > > > > > +	    I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6)) {
> > > > > > +		DRM_ERROR("Cant enable DPLL4 when DC5 or DC6
> > > > > > are enabled\n");
> > > > > > +		return;
> > > > > > +	}    
> > > > > 
> > > > > This looks like dead code, or we messed up earlier already (in
> > > > > which case it should just be some kind of assert IMO).    
> > > Are you suggesting that I put this in a ->prepare() hook or much
> > > earlier in the atomic check/prepare phase? FYI, I am just adding a
> > > restriction mentioned in the spec concerning this DPLL.
> > >   
> > > > 
> > > > Does DMC handle other MG PLLs? I would have thought not, so if we
> > > > keep the assert then it should perhaps be unconditional. Hmm. But
> > > > aren't we still hodling the modeset power domain when this gets
> > > > called? If so this is definitely dead code.  
> > > I am not sure whether DMC handles other MG PLLs but the spec
> > > explicitly says that DMC will not handle this PLL. Where do you
> > > suggest is an appropriate place to put this assert?  
> > 
> > DC5/6 enable probably is the best, if we even want the assert. And I'm
> > not convinced that we do since we don't have it for any other PLL
> > either.
> As I mentioned earlier, the spec says we cannot enable this DPLL for EHL
> if DC5 or DC6 is enabled. Do you think we should disable DC5/DC6 on EHL
> in order to enable this DPLL on EHL? Should the user/system integrator 
> be making that decision via a module parameter?

If there is some way that could happen currently, then we need to
prevent DC5/6 via some power domain stuff.

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

[PATCH] drm/i915/ehl: Add support for DPLL4 (v4)

[Vivek Kasireddy]

This patch adds support for DPLL4 on EHL that include the
following restrictions:

- DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
  DPLL4 can be used with other DDIs, including DDID
  (combo port A external usage).

- DPLL4 cannot be enabled when DC5 or DC6 are enabled.

- The DPLL4 enable, lock, power enabled, and power state are connected
  to the MGPLL1_ENABLE register.

v2: (suggestions from Bob Paauwe)
- Rework ehl_get_dpll() function to call intel_find_shared_dpll() and
  iterate twice: once for Combo plls and once for MG plls.

- Use MG pll funcs for DPLL4 instead of creating new ones and modify
  mg_pll_enable to include the restrictions for EHL.

v3: Fix compilation error

v4: (suggestions from Lucas and Ville)
- Treat DPLL4 as a combo phy PLL and not as MG PLL
- Disable DC states when this DPLL is being enabled
- Reuse icl_get_dpll instead of creating a separate one for EHL

Cc: Lucas De Marchi <lucas.demarchi@intel.com>
Cc: José Roberto de Souza <jose.souza@intel.com>
Cc: Bob Paauwe <bob.j.paauwe@intel.com>
Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
---
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 35 ++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_dpll_mgr.h |  4 ++++
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index e01c057ce50b..207af4af4978 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2825,6 +2825,12 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
 	if (intel_port_is_combophy(dev_priv, port)) {
 		min = DPLL_ID_ICL_DPLL0;
 		max = DPLL_ID_ICL_DPLL1;
+
+		if (IS_ELKHARTLAKE(dev_priv)) {
+			if (encoder->type != INTEL_OUTPUT_EDP)
+				max = DPLL_ID_EHL_DPLL4;
+		}
+
 		ret = icl_calc_dpll_state(crtc_state, encoder);
 	} else if (intel_port_is_tc(dev_priv, port)) {
 		if (encoder->type == INTEL_OUTPUT_DP_MST) {
@@ -2964,8 +2970,14 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
 				   struct intel_shared_dpll *pll,
 				   struct intel_dpll_hw_state *hw_state)
 {
-	return icl_pll_get_hw_state(dev_priv, pll, hw_state,
-				    CNL_DPLL_ENABLE(pll->info->id));
+	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+
+	if (IS_ELKHARTLAKE(dev_priv) &&
+	    pll->info->id == DPLL_ID_EHL_DPLL4) {
+		enable_reg = MG_PLL_ENABLE(0);
+	}
+
+	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
 }
 
 static bool tbt_pll_get_hw_state(struct drm_i915_private *dev_priv,
@@ -3076,6 +3088,14 @@ static void combo_pll_enable(struct drm_i915_private *dev_priv,
 {
 	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
 
+	if (IS_ELKHARTLAKE(dev_priv) &&
+	    pll->info->id == DPLL_ID_EHL_DPLL4) {
+		enable_reg = MG_PLL_ENABLE(0);
+
+		/* Need to disable DC states when this DPLL is enabled. */
+		bxt_disable_dc9(dev_priv);
+	}
+
 	icl_pll_power_enable(dev_priv, pll, enable_reg);
 
 	icl_dpll_write(dev_priv, pll);
@@ -3171,7 +3191,15 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
 static void combo_pll_disable(struct drm_i915_private *dev_priv,
 			      struct intel_shared_dpll *pll)
 {
-	icl_pll_disable(dev_priv, pll, CNL_DPLL_ENABLE(pll->info->id));
+	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
+
+	if (IS_ELKHARTLAKE(dev_priv) &&
+	    pll->info->id == DPLL_ID_EHL_DPLL4) {
+		enable_reg = MG_PLL_ENABLE(0);
+		bxt_enable_dc9(dev_priv);
+	}
+
+	icl_pll_disable(dev_priv, pll, enable_reg);
 }
 
 static void tbt_pll_disable(struct drm_i915_private *dev_priv,
@@ -3249,6 +3277,7 @@ static const struct intel_dpll_mgr icl_pll_mgr = {
 static const struct dpll_info ehl_plls[] = {
 	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
 	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
+	{ "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
 	{ },
 };
 
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index bd8124cc81ed..f3f99929cee8 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -113,6 +113,10 @@ enum intel_dpll_id {
 	 * @DPLL_ID_ICL_DPLL1: ICL combo PHY DPLL1
 	 */
 	DPLL_ID_ICL_DPLL1 = 1,
+	/**
+	 * @DPLL_ID_EHL_DPLL4: EHL combo PHY DPLL4
+	 */
+	DPLL_ID_EHL_DPLL4 = 2,
 	/**
 	 * @DPLL_ID_ICL_TBTPLL: ICL TBT PLL
 	 */
-- 
2.14.5

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

✓ Fi.CI.BAT: success for drm/i915/ehl: Add support for DPLL4 (v3) (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/ehl: Add support for DPLL4 (v3) (rev2)
URL   : https://patchwork.freedesktop.org/series/59078/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5917 -> Patchwork_12771
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59078/revisions/2/mbox/

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_basic@gtt-bsd2:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] +52

  * igt@gem_exec_basic@readonly-bsd1:
    - fi-snb-2520m:       NOTRUN -> SKIP [fdo#109271] +52

  * igt@gem_exec_store@basic-bsd2:
    - fi-hsw-4770:        NOTRUN -> SKIP [fdo#109271] +41

  * igt@kms_busy@basic-flip-c:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] / [fdo#109278]
    - fi-snb-2520m:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     NOTRUN -> FAIL [fdo#103167]

  
#### Possible fixes ####

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-y:           INCOMPLETE [fdo#108569] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (47 -> 42)
------------------------------

  Additional (3): fi-hsw-4770 fi-byt-clapper fi-snb-2520m 
  Missing    (8): fi-kbl-soraka fi-hsw-4200u fi-byt-j1900 fi-bsw-n3050 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5917 -> Patchwork_12771

  CI_DRM_5917: b01c0e68e8d1092c436dbba4d03b260c828f37c9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4944: 9b74b8226e8c108db091bd3b1d105a71dc0fb861 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12771: aad97f450c0946316db4497022256b0b37633c6f @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

aad97f450c09 drm/i915/ehl: Add support for DPLL4 (v4)

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915/ehl: Add support for DPLL4 (v3) (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915/ehl: Add support for DPLL4 (v3) (rev2)
URL   : https://patchwork.freedesktop.org/series/59078/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5917_full -> Patchwork_12771_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         PASS -> FAIL [fdo#108686]

  * igt@i915_pm_rpm@debugfs-read:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#107807]

  * igt@i915_pm_rpm@gem-execbuf-stress:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#107803] / [fdo#107807]

  * igt@i915_selftest@live_workarounds:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#108954]

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@kms_atomic_transition@5x-modeset-transitions:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@extended-modeset-hang-newfb-render-d:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_chamelium@hdmi-edid-change-during-suspend:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +23

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#104108]

  * igt@kms_fbcon_fbt@psr:
    - shard-skl:          NOTRUN -> FAIL [fdo#103833]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          PASS -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-suspend:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665] +1
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566] +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-shrfb-msflip-blt:
    - shard-apl:          NOTRUN -> INCOMPLETE [fdo#103927]

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-indfb-msflip-blt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +17

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-indfb-plflip-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +73

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-cpu:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +11

  * igt@kms_lease@atomic_implicit_crtc:
    - shard-skl:          NOTRUN -> FAIL [fdo#110279]

  * igt@kms_lease@cursor_implicit_plane:
    - shard-skl:          NOTRUN -> FAIL [fdo#110278]

  * igt@kms_lease@page_flip_implicit_plane:
    - shard-skl:          NOTRUN -> FAIL [fdo#110281]

  * igt@kms_panel_fitting@legacy:
    - shard-skl:          NOTRUN -> FAIL [fdo#105456]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-f:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +20

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271] +1

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] +4

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         PASS -> SKIP [fdo#109642]

  * igt@kms_psr@no_drrs:
    - shard-iclb:         PASS -> FAIL [fdo#108341]

  * igt@kms_psr@primary_mmap_cpu:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215] +3

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +2

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          PASS -> FAIL [fdo#109016]

  * igt@kms_setmode@basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#99912]

  * igt@kms_universal_plane@disable-primary-vs-flip-pipe-d:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +11

  * igt@perf_pmu@busy-accuracy-50-vcs1:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +236

  * igt@tools_test@tools_test:
    - shard-iclb:         PASS -> SKIP [fdo#109352]

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS +4

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS

  * igt@kms_color@pipe-b-ctm-max:
    - shard-skl:          FAIL [fdo#108147] -> PASS

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-skl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] -> PASS

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-skl:          FAIL [fdo#103232] -> PASS

  * igt@kms_dp_dsc@basic-dsc-enable-edp:
    - shard-iclb:         SKIP [fdo#109349] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +3

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +11

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          FAIL [fdo#108145] -> PASS +1

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          FAIL [fdo#110403] -> PASS +1

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         SKIP [fdo#109642] -> PASS

  * igt@kms_psr@cursor_blt:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103833]: https://bugs.freedesktop.org/show_bug.cgi?id=103833
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
  [fdo#109352]: https://bugs.freedesktop.org/show_bug.cgi?id=109352
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110278]: https://bugs.freedesktop.org/show_bug.cgi?id=110278
  [fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279
  [fdo#110281]: https://bugs.freedesktop.org/show_bug.cgi?id=110281
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

    * Linux: CI_DRM_5917 -> Patchwork_12771

  CI_DRM_5917: b01c0e68e8d1092c436dbba4d03b260c828f37c9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4944: 9b74b8226e8c108db091bd3b1d105a71dc0fb861 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12771: aad97f450c0946316db4497022256b0b37633c6f @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v4)

[Bob Paauwe]

On Thu, 11 Apr 2019 16:36:00 -0700
Vivek Kasireddy <vivek.kasireddy@intel.com> wrote:

> This patch adds support for DPLL4 on EHL that include the
> following restrictions:
> 
> - DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
>   DPLL4 can be used with other DDIs, including DDID
>   (combo port A external usage).
> 
> - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> 
> - The DPLL4 enable, lock, power enabled, and power state are connected
>   to the MGPLL1_ENABLE register.
> 
> v2: (suggestions from Bob Paauwe)
> - Rework ehl_get_dpll() function to call intel_find_shared_dpll() and
>   iterate twice: once for Combo plls and once for MG plls.
> 
> - Use MG pll funcs for DPLL4 instead of creating new ones and modify
>   mg_pll_enable to include the restrictions for EHL.
> 
> v3: Fix compilation error
> 
> v4: (suggestions from Lucas and Ville)
> - Treat DPLL4 as a combo phy PLL and not as MG PLL
> - Disable DC states when this DPLL is being enabled
> - Reuse icl_get_dpll instead of creating a separate one for EHL
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Bob Paauwe <bob.j.paauwe@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 35 ++++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_dpll_mgr.h |  4 ++++
>  2 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index e01c057ce50b..207af4af4978 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2825,6 +2825,12 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
>  	if (intel_port_is_combophy(dev_priv, port)) {
>  		min = DPLL_ID_ICL_DPLL0;
>  		max = DPLL_ID_ICL_DPLL1;
> +
> +		if (IS_ELKHARTLAKE(dev_priv)) {
> +			if (encoder->type != INTEL_OUTPUT_EDP)
> +				max = DPLL_ID_EHL_DPLL4;
> +		}
> +
>  		ret = icl_calc_dpll_state(crtc_state, encoder);
>  	} else if (intel_port_is_tc(dev_priv, port)) {
>  		if (encoder->type == INTEL_OUTPUT_DP_MST) {
> @@ -2964,8 +2970,14 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  				   struct intel_shared_dpll *pll,
>  				   struct intel_dpll_hw_state *hw_state)
>  {
> -	return icl_pll_get_hw_state(dev_priv, pll, hw_state,
> -				    CNL_DPLL_ENABLE(pll->info->id));
> +	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +
> +	if (IS_ELKHARTLAKE(dev_priv) &&
> +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> +		enable_reg = MG_PLL_ENABLE(0);
> +	}
> +
> +	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
>  }
>  
>  static bool tbt_pll_get_hw_state(struct drm_i915_private *dev_priv,
> @@ -3076,6 +3088,14 @@ static void combo_pll_enable(struct drm_i915_private *dev_priv,
>  {
>  	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>  
> +	if (IS_ELKHARTLAKE(dev_priv) &&
> +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> +		enable_reg = MG_PLL_ENABLE(0);
> +
> +		/* Need to disable DC states when this DPLL is enabled. */
> +		bxt_disable_dc9(dev_priv);
> +	}
> +
>  	icl_pll_power_enable(dev_priv, pll, enable_reg);
>  
>  	icl_dpll_write(dev_priv, pll);
> @@ -3171,7 +3191,15 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
>  static void combo_pll_disable(struct drm_i915_private *dev_priv,
>  			      struct intel_shared_dpll *pll)
>  {
> -	icl_pll_disable(dev_priv, pll, CNL_DPLL_ENABLE(pll->info->id));
> +	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +
> +	if (IS_ELKHARTLAKE(dev_priv) &&
> +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> +		enable_reg = MG_PLL_ENABLE(0);
> +		bxt_enable_dc9(dev_priv);

dc9 is disabled before the DPLL is enabled and here, you're also
enabling before the DPLL has been disabled.  Is that OK?

If the order here is fine then
Reviewed-by: Bob Paauwe <bob.j.paauwe@intel.com>

> +	}
> +
> +	icl_pll_disable(dev_priv, pll, enable_reg);
>  }
>  
>  static void tbt_pll_disable(struct drm_i915_private *dev_priv,
> @@ -3249,6 +3277,7 @@ static const struct intel_dpll_mgr icl_pll_mgr = {
>  static const struct dpll_info ehl_plls[] = {
>  	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
>  	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> +	{ "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
>  	{ },
>  };
>  
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> index bd8124cc81ed..f3f99929cee8 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> @@ -113,6 +113,10 @@ enum intel_dpll_id {
>  	 * @DPLL_ID_ICL_DPLL1: ICL combo PHY DPLL1
>  	 */
>  	DPLL_ID_ICL_DPLL1 = 1,
> +	/**
> +	 * @DPLL_ID_EHL_DPLL4: EHL combo PHY DPLL4
> +	 */
> +	DPLL_ID_EHL_DPLL4 = 2,
>  	/**
>  	 * @DPLL_ID_ICL_TBTPLL: ICL TBT PLL
>  	 */



-- 
--
Bob Paauwe                  
Bob.J.Paauwe@intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193    

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v4)

[Ville Syrjälä]

On Thu, Apr 11, 2019 at 04:36:00PM -0700, Vivek Kasireddy wrote:
> This patch adds support for DPLL4 on EHL that include the
> following restrictions:
> 
> - DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
>   DPLL4 can be used with other DDIs, including DDID
>   (combo port A external usage).
> 
> - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> 
> - The DPLL4 enable, lock, power enabled, and power state are connected
>   to the MGPLL1_ENABLE register.
> 
> v2: (suggestions from Bob Paauwe)
> - Rework ehl_get_dpll() function to call intel_find_shared_dpll() and
>   iterate twice: once for Combo plls and once for MG plls.
> 
> - Use MG pll funcs for DPLL4 instead of creating new ones and modify
>   mg_pll_enable to include the restrictions for EHL.
> 
> v3: Fix compilation error
> 
> v4: (suggestions from Lucas and Ville)
> - Treat DPLL4 as a combo phy PLL and not as MG PLL
> - Disable DC states when this DPLL is being enabled
> - Reuse icl_get_dpll instead of creating a separate one for EHL
> 
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Cc: José Roberto de Souza <jose.souza@intel.com>
> Cc: Bob Paauwe <bob.j.paauwe@intel.com>
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 35 ++++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_dpll_mgr.h |  4 ++++
>  2 files changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index e01c057ce50b..207af4af4978 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2825,6 +2825,12 @@ icl_get_dpll(struct intel_crtc_state *crtc_state,
>  	if (intel_port_is_combophy(dev_priv, port)) {
>  		min = DPLL_ID_ICL_DPLL0;
>  		max = DPLL_ID_ICL_DPLL1;
> +
> +		if (IS_ELKHARTLAKE(dev_priv)) {
> +			if (encoder->type != INTEL_OUTPUT_EDP)
> +				max = DPLL_ID_EHL_DPLL4;
> +		}
> +
>  		ret = icl_calc_dpll_state(crtc_state, encoder);
>  	} else if (intel_port_is_tc(dev_priv, port)) {
>  		if (encoder->type == INTEL_OUTPUT_DP_MST) {
> @@ -2964,8 +2970,14 @@ static bool combo_pll_get_hw_state(struct drm_i915_private *dev_priv,
>  				   struct intel_shared_dpll *pll,
>  				   struct intel_dpll_hw_state *hw_state)
>  {
> -	return icl_pll_get_hw_state(dev_priv, pll, hw_state,
> -				    CNL_DPLL_ENABLE(pll->info->id));
> +	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +
> +	if (IS_ELKHARTLAKE(dev_priv) &&
> +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> +		enable_reg = MG_PLL_ENABLE(0);
> +	}
> +
> +	return icl_pll_get_hw_state(dev_priv, pll, hw_state, enable_reg);
>  }
>  
>  static bool tbt_pll_get_hw_state(struct drm_i915_private *dev_priv,
> @@ -3076,6 +3088,14 @@ static void combo_pll_enable(struct drm_i915_private *dev_priv,
>  {
>  	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
>  
> +	if (IS_ELKHARTLAKE(dev_priv) &&
> +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> +		enable_reg = MG_PLL_ENABLE(0);
> +
> +		/* Need to disable DC states when this DPLL is enabled. */
> +		bxt_disable_dc9(dev_priv);

You can't simply call that from random places. It needs to be handled by
the power domain stuff.

> +	}
> +
>  	icl_pll_power_enable(dev_priv, pll, enable_reg);
>  
>  	icl_dpll_write(dev_priv, pll);
> @@ -3171,7 +3191,15 @@ static void icl_pll_disable(struct drm_i915_private *dev_priv,
>  static void combo_pll_disable(struct drm_i915_private *dev_priv,
>  			      struct intel_shared_dpll *pll)
>  {
> -	icl_pll_disable(dev_priv, pll, CNL_DPLL_ENABLE(pll->info->id));
> +	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> +
> +	if (IS_ELKHARTLAKE(dev_priv) &&
> +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> +		enable_reg = MG_PLL_ENABLE(0);
> +		bxt_enable_dc9(dev_priv);
> +	}
> +
> +	icl_pll_disable(dev_priv, pll, enable_reg);
>  }
>  
>  static void tbt_pll_disable(struct drm_i915_private *dev_priv,
> @@ -3249,6 +3277,7 @@ static const struct intel_dpll_mgr icl_pll_mgr = {
>  static const struct dpll_info ehl_plls[] = {
>  	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
>  	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> +	{ "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
>  	{ },
>  };
>  
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> index bd8124cc81ed..f3f99929cee8 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
> @@ -113,6 +113,10 @@ enum intel_dpll_id {
>  	 * @DPLL_ID_ICL_DPLL1: ICL combo PHY DPLL1
>  	 */
>  	DPLL_ID_ICL_DPLL1 = 1,
> +	/**
> +	 * @DPLL_ID_EHL_DPLL4: EHL combo PHY DPLL4
> +	 */
> +	DPLL_ID_EHL_DPLL4 = 2,
>  	/**
>  	 * @DPLL_ID_ICL_TBTPLL: ICL TBT PLL
>  	 */
> -- 
> 2.14.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [PATCH] drm/i915/ehl: Add support for DPLL4 (v4)

[Vivek Kasireddy]

On Wed, 17 Apr 2019 16:06:11 +0300
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
Hi Ville,

> On Thu, Apr 11, 2019 at 04:36:00PM -0700, Vivek Kasireddy wrote:
> > This patch adds support for DPLL4 on EHL that include the
> > following restrictions:
> > 
> > - DPLL4 cannot be used with DDIA (combo port A internal eDP usage).
> >   DPLL4 can be used with other DDIs, including DDID
> >   (combo port A external usage).
> > 
> > - DPLL4 cannot be enabled when DC5 or DC6 are enabled.
> > 
> > - The DPLL4 enable, lock, power enabled, and power state are
> > connected to the MGPLL1_ENABLE register.
> > 
> > v2: (suggestions from Bob Paauwe)
> > - Rework ehl_get_dpll() function to call intel_find_shared_dpll()
> > and iterate twice: once for Combo plls and once for MG plls.
> > 
> > - Use MG pll funcs for DPLL4 instead of creating new ones and modify
> >   mg_pll_enable to include the restrictions for EHL.
> > 
> > v3: Fix compilation error
> > 
> > v4: (suggestions from Lucas and Ville)
> > - Treat DPLL4 as a combo phy PLL and not as MG PLL
> > - Disable DC states when this DPLL is being enabled
> > - Reuse icl_get_dpll instead of creating a separate one for EHL
> > 
> > Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> > Cc: José Roberto de Souza <jose.souza@intel.com>
> > Cc: Bob Paauwe <bob.j.paauwe@intel.com>
> > Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dpll_mgr.c | 35
> > ++++++++++++++++++++++++++++++++---
> > drivers/gpu/drm/i915/intel_dpll_mgr.h |  4 ++++ 2 files changed, 36
> > insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.c index
> > e01c057ce50b..207af4af4978 100644 ---
> > a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -2825,6 +2825,12 @@
> > icl_get_dpll(struct intel_crtc_state *crtc_state, if
> > (intel_port_is_combophy(dev_priv, port)) { min = DPLL_ID_ICL_DPLL0;
> >  		max = DPLL_ID_ICL_DPLL1;
> > +
> > +		if (IS_ELKHARTLAKE(dev_priv)) {
> > +			if (encoder->type != INTEL_OUTPUT_EDP)
> > +				max = DPLL_ID_EHL_DPLL4;
> > +		}
> > +
> >  		ret = icl_calc_dpll_state(crtc_state, encoder);
> >  	} else if (intel_port_is_tc(dev_priv, port)) {
> >  		if (encoder->type == INTEL_OUTPUT_DP_MST) {
> > @@ -2964,8 +2970,14 @@ static bool combo_pll_get_hw_state(struct
> > drm_i915_private *dev_priv, struct intel_shared_dpll *pll,
> >  				   struct intel_dpll_hw_state
> > *hw_state) {
> > -	return icl_pll_get_hw_state(dev_priv, pll, hw_state,
> > -
> > CNL_DPLL_ENABLE(pll->info->id));
> > +	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > +
> > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > +		enable_reg = MG_PLL_ENABLE(0);
> > +	}
> > +
> > +	return icl_pll_get_hw_state(dev_priv, pll, hw_state,
> > enable_reg); }
> >  
> >  static bool tbt_pll_get_hw_state(struct drm_i915_private *dev_priv,
> > @@ -3076,6 +3088,14 @@ static void combo_pll_enable(struct
> > drm_i915_private *dev_priv, {
> >  	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> >  
> > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > +		enable_reg = MG_PLL_ENABLE(0);
> > +
> > +		/* Need to disable DC states when this DPLL is
> > enabled. */
> > +		bxt_disable_dc9(dev_priv);  
> 
> You can't simply call that from random places. It needs to be handled
> by the power domain stuff.
The only other places in the driver, the functions bxt_disable/enable_dc9
are called are intel_runtime_suspend/resume and
i915_drm_suspend_late/resume_early. Are you suggesting that I call one
of these functions instead? Or, do you simply want me to pair
bxt_*able_dc9 with intel_power_domains_suspend/resume and/or other
functions similar to what the above mentioned functions do?

Thanks,
Vivek

> 
> > +	}
> > +
> >  	icl_pll_power_enable(dev_priv, pll, enable_reg);
> >  
> >  	icl_dpll_write(dev_priv, pll);
> > @@ -3171,7 +3191,15 @@ static void icl_pll_disable(struct
> > drm_i915_private *dev_priv, static void combo_pll_disable(struct
> > drm_i915_private *dev_priv, struct intel_shared_dpll *pll)
> >  {
> > -	icl_pll_disable(dev_priv, pll,
> > CNL_DPLL_ENABLE(pll->info->id));
> > +	i915_reg_t enable_reg = CNL_DPLL_ENABLE(pll->info->id);
> > +
> > +	if (IS_ELKHARTLAKE(dev_priv) &&
> > +	    pll->info->id == DPLL_ID_EHL_DPLL4) {
> > +		enable_reg = MG_PLL_ENABLE(0);
> > +		bxt_enable_dc9(dev_priv);
> > +	}
> > +
> > +	icl_pll_disable(dev_priv, pll, enable_reg);
> >  }
> >  
> >  static void tbt_pll_disable(struct drm_i915_private *dev_priv,
> > @@ -3249,6 +3277,7 @@ static const struct intel_dpll_mgr
> > icl_pll_mgr = { static const struct dpll_info ehl_plls[] = {
> >  	{ "DPLL 0", &combo_pll_funcs, DPLL_ID_ICL_DPLL0, 0 },
> >  	{ "DPLL 1", &combo_pll_funcs, DPLL_ID_ICL_DPLL1, 0 },
> > +	{ "DPLL 4", &combo_pll_funcs, DPLL_ID_EHL_DPLL4, 0 },
> >  	{ },
> >  };
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.h index
> > bd8124cc81ed..f3f99929cee8 100644 ---
> > a/drivers/gpu/drm/i915/intel_dpll_mgr.h +++
> > b/drivers/gpu/drm/i915/intel_dpll_mgr.h @@ -113,6 +113,10 @@ enum
> > intel_dpll_id {
> >  	 * @DPLL_ID_ICL_DPLL1: ICL combo PHY DPLL1
> >  	 */
> >  	DPLL_ID_ICL_DPLL1 = 1,
> > +	/**
> > +	 * @DPLL_ID_EHL_DPLL4: EHL combo PHY DPLL4
> > +	 */
> > +	DPLL_ID_EHL_DPLL4 = 2,
> >  	/**
> >  	 * @DPLL_ID_ICL_TBTPLL: ICL TBT PLL
> >  	 */
> > -- 
> > 2.14.5
> > 
> > _______________________________________________
> > 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