[PATCH 0/6] ICL DSI PLL enable

[PATCH 0/6] ICL DSI PLL enable

[Vandita Kulkarni]

ICL DSI uses DPLL.
As per the discussion with hw team, the same
sequence can be used for enabling DPLL for mipi dsi
as well. Hence reusing the dpll functions from icl pll manager.
In addition to that we need to program
the esc clock register before enabling dsi.

This has been tested on git://people.freedesktop.org/~jani/drm
and the patches are rebased on this.

Madhav Chauhan (3):
  drm/i915/icl: Calculate DPLL params for DSI
  drm/i915/icl: Gate clocks for DSI
  drm/i915/icl: Ungate DSI clocks

Vandita Kulkarni (3):
  drm/i915/icl: Use the same pll functions for dsi
  drm/i915/icl: Get port clock from pll.
  drm/i915/icl: Update port clock in compute config

 drivers/gpu/drm/i915/icl_dsi.c        | 62 ++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_ddi.c      |  2 +-
 drivers/gpu/drm/i915/intel_display.c  |  4 ++-
 drivers/gpu/drm/i915/intel_dpll_mgr.c |  6 ++--
 drivers/gpu/drm/i915/intel_dsi.h      |  4 +++
 5 files changed, 68 insertions(+), 10 deletions(-)

-- 
1.9.1

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

[PATCH 1/6] drm/i915/icl: Calculate DPLL params for DSI

[Vandita Kulkarni]

From: Madhav Chauhan <madhav.chauhan@intel.com>

This patch calculates various DPLL dividers and
parameters for DSI encoder and adjust AFE clock
for DSI. For DSI, 8x clock is AFE clock.

v2: Extend haswell_crtc_compute_clock() for Gen11 DSI

v3: Rebase

v4: use port clock instead of bitrate.

v5: Reabse and remove divide by 5

Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c  | 4 +++-
 drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 27bdf91..1318faf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9303,10 +9303,12 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
 static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
 				      struct intel_crtc_state *crtc_state)
 {
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_atomic_state *state =
 		to_intel_atomic_state(crtc_state->base.state);
 
-	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)) {
+	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) ||
+	     IS_ICELAKE(dev_priv)) {
 		struct intel_encoder *encoder =
 			intel_get_crtc_new_encoder(state, crtc_state);
 
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
index 901e150..e3cb0db 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
@@ -2523,10 +2523,10 @@ static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
 
 	if (intel_port_is_tc(dev_priv, encoder->port))
 		ret = icl_calc_tbt_pll(dev_priv, clock, &pll_params);
-	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
-		ret = cnl_ddi_calculate_wrpll(clock, dev_priv, &pll_params);
-	else
+	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP))
 		ret = icl_calc_dp_combo_pll(dev_priv, clock, &pll_params);
+	else
+		ret = cnl_ddi_calculate_wrpll(clock, dev_priv, &pll_params);
 
 	if (!ret)
 		return false;
-- 
1.9.1

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

[PATCH 2/6] drm/i915/icl: Use the same pll functions for dsi

[Vandita Kulkarni]

The same pll manager functions can be used to enable
dpll for mipi. Hence enabling the IO power and
esc clock as part of pre pll enable call.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/icl_dsi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index 89ede31..8590825 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -924,17 +924,22 @@ static void gen11_dsi_powerup_panel(struct intel_encoder *encoder)
 	wait_for_cmds_dispatched_to_panel(encoder);
 }
 
-static void gen11_dsi_pre_enable(struct intel_encoder *encoder,
+static void gen11_dsi_pre_pll_enable(struct intel_encoder *encoder,
 				 const struct intel_crtc_state *pipe_config,
 				 const struct drm_connector_state *conn_state)
 {
-	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
-
 	/* step2: enable IO power */
 	gen11_dsi_enable_io_power(encoder);
 
 	/* step3: enable DSI PLL */
 	gen11_dsi_program_esc_clk_div(encoder);
+}
+
+static void gen11_dsi_pre_enable(struct intel_encoder *encoder,
+				 const struct intel_crtc_state *pipe_config,
+				 const struct drm_connector_state *conn_state)
+{
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 
 	/* step4: enable DSI port and DPHY */
 	gen11_dsi_enable_port_and_phy(encoder, pipe_config);
@@ -1305,6 +1310,7 @@ void icl_dsi_init(struct drm_i915_private *dev_priv)
 			 DRM_MODE_ENCODER_DSI, "DSI %c", port_name(port));
 
 	encoder->pre_enable = gen11_dsi_pre_enable;
+	encoder->pre_pll_enable = gen11_dsi_pre_pll_enable;
 	encoder->disable = gen11_dsi_disable;
 	encoder->port = port;
 	encoder->get_config = gen11_dsi_get_config;
-- 
1.9.1

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

[PATCH 3/6] drm/i915/icl: Get port clock from pll.

[Vandita Kulkarni]

Use the same method to get port clock
like other encoders.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/icl_dsi.c   | 10 ++++++++--
 drivers/gpu/drm/i915/intel_ddi.c |  2 +-
 drivers/gpu/drm/i915/intel_dsi.h |  4 ++++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index 8590825..ec22b2e 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -1105,12 +1105,18 @@ static void gen11_dsi_get_config(struct intel_encoder *encoder,
 {
 	struct intel_dsi *intel_dsi = container_of(encoder, struct intel_dsi,
 						   base);
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	int link_clock = 0;
+	uint32_t pll_id;
 	u32 pixel_clk;
 
-	//FIXME: Calculate pixel clock using PLL functions once implemented.
+	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config->shared_dpll);
+	link_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
+	pipe_config->port_clock = link_clock;
+
 	pixel_clk = intel_dsi->pclk;
 	pipe_config->base.adjusted_mode.crtc_clock = pixel_clk;
-	pipe_config->port_clock = pixel_clk;
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
 }
 
 static bool gen11_dsi_compute_config(struct intel_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ad11540..3783f79 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1363,7 +1363,7 @@ static int skl_calc_wrpll_link(struct drm_i915_private *dev_priv,
 	return dco_freq / (p0 * p1 * p2 * 5);
 }
 
-static int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
+int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
 			       enum intel_dpll_id pll_id)
 {
 	uint32_t cfgcr0, cfgcr1;
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index ee93137..80805bf 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -189,4 +189,8 @@ void intel_dsi_vbt_exec_sequence(struct intel_dsi *intel_dsi,
 				 enum mipi_seq seq_id);
 void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec);
 
+/* icl dsi uses combophy pll which can be used by any ddi */
+int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
+			enum intel_dpll_id pll_id);
+
 #endif /* _INTEL_DSI_H */
-- 
1.9.1

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

[PATCH 4/6] drm/i915/icl: Gate clocks for DSI

[Vandita Kulkarni]

From: Madhav Chauhan <madhav.chauhan@intel.com>

As per BSPEC, depending on the DSI transcoder being used,
DDI clock for the associated port should be gated. This
patch does the same.

Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/icl_dsi.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index ec22b2e..fe4efc7 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -536,6 +536,23 @@ static void gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder)
 	}
 }
 
+static void gen11_dsi_gate_clocks(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	u32 tmp;
+	enum port port;
+
+	mutex_lock(&dev_priv->dpll_lock);
+	tmp = I915_READ(DPCLKA_CFGCR0_ICL);
+	for_each_dsi_port(port, intel_dsi->ports) {
+		tmp |= DPCLKA_CFGCR0_DDI_CLK_OFF(port);
+	}
+
+	I915_WRITE(DPCLKA_CFGCR0_ICL, tmp);
+	mutex_unlock(&dev_priv->dpll_lock);
+}
+
 static void
 gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
 			       const struct intel_crtc_state *pipe_config)
@@ -883,6 +900,9 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder)
 
 	/* Step (4h, 4i, 4j, 4k): Configure transcoder */
 	gen11_dsi_configure_transcoder(encoder, pipe_config);
+
+	/* Step 4l: Gate DDI clocks */
+	gen11_dsi_gate_clocks(encoder);
 }
 
 static void gen11_dsi_powerup_panel(struct intel_encoder *encoder)
-- 
1.9.1

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

[PATCH 5/6] drm/i915/icl: Ungate DSI clocks

[Vandita Kulkarni]

From: Madhav Chauhan <madhav.chauhan@intel.com>

Ungate the clocks on the selected port.

Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/icl_dsi.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index fe4efc7..80382fb 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -553,6 +553,23 @@ static void gen11_dsi_gate_clocks(struct intel_encoder *encoder)
 	mutex_unlock(&dev_priv->dpll_lock);
 }
 
+static void gen11_dsi_ungate_clocks(struct intel_encoder *encoder)
+{
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
+	u32 tmp;
+	enum port port;
+
+	mutex_lock(&dev_priv->dpll_lock);
+	tmp = I915_READ(DPCLKA_CFGCR0_ICL);
+	for_each_dsi_port(port, intel_dsi->ports) {
+		tmp &= ~DPCLKA_CFGCR0_DDI_CLK_OFF(port);
+	}
+
+	I915_WRITE(DPCLKA_CFGCR0_ICL, tmp);
+	mutex_unlock(&dev_priv->dpll_lock);
+}
+
 static void
 gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
 			       const struct intel_crtc_state *pipe_config)
@@ -1061,6 +1078,7 @@ static void gen11_dsi_disable_port(struct intel_encoder *encoder)
 	u32 tmp;
 	enum port port;
 
+	gen11_dsi_ungate_clocks(encoder);
 	for_each_dsi_port(port, intel_dsi->ports) {
 		tmp = I915_READ(DDI_BUF_CTL(port));
 		tmp &= ~DDI_BUF_CTL_ENABLE;
@@ -1072,6 +1090,7 @@ static void gen11_dsi_disable_port(struct intel_encoder *encoder)
 			DRM_ERROR("DDI port:%c buffer not idle\n",
 				  port_name(port));
 	}
+	gen11_dsi_ungate_clocks(encoder);
 }
 
 static void gen11_dsi_disable_io_power(struct intel_encoder *encoder)
-- 
1.9.1

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

[PATCH 6/6] drm/i915/icl: Update port clock in compute config

[Vandita Kulkarni]

For DSI 8X clock is AFE clock which is
is 5 times port clock.

Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
---
 drivers/gpu/drm/i915/icl_dsi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
index 80382fb..2812129 100644
--- a/drivers/gpu/drm/i915/icl_dsi.c
+++ b/drivers/gpu/drm/i915/icl_dsi.c
@@ -1183,6 +1183,7 @@ static bool gen11_dsi_compute_config(struct intel_encoder *encoder,
 		pipe_config->cpu_transcoder = TRANSCODER_DSI_0;
 
 	pipe_config->clock_set = true;
+	pipe_config->port_clock = intel_dsi_bitrate(intel_dsi)/5;
 
 	//TODO: Add check if DSI PLL calculation is done
 
-- 
1.9.1

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

✗ Fi.CI.BAT: failure for ICL DSI PLL enable (rev2)

[Patchwork]

== Series Details ==

Series: ICL DSI PLL enable (rev2)
URL   : https://patchwork.freedesktop.org/series/51373/
State : failure

== Summary ==

Applying: drm/i915/icl: Calculate DPLL params for DSI
Applying: drm/i915/icl: Use the same pll functions for dsi
error: sha1 information is lacking or useless (drivers/gpu/drm/i915/icl_dsi.c).
error: could not build fake ancestor
Patch failed at 0002 drm/i915/icl: Use the same pll functions for dsi
Use 'git am --show-current-patch' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

Re: [PATCH 1/6] drm/i915/icl: Calculate DPLL params for DSI

[Jani Nikula]

On Tue, 27 Nov 2018, Vandita Kulkarni <vandita.kulkarni@intel.com> wrote:
> From: Madhav Chauhan <madhav.chauhan@intel.com>
>
> This patch calculates various DPLL dividers and
> parameters for DSI encoder and adjust AFE clock
> for DSI. For DSI, 8x clock is AFE clock.
>
> v2: Extend haswell_crtc_compute_clock() for Gen11 DSI
>
> v3: Rebase
>
> v4: use port clock instead of bitrate.
>
> v5: Reabse and remove divide by 5
>
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c  | 4 +++-
>  drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 27bdf91..1318faf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9303,10 +9303,12 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
>  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>  				      struct intel_crtc_state *crtc_state)
>  {
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	struct intel_atomic_state *state =
>  		to_intel_atomic_state(crtc_state->base.state);
>  
> -	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)) {
> +	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) ||
> +	     IS_ICELAKE(dev_priv)) {
>  		struct intel_encoder *encoder =
>  			intel_get_crtc_new_encoder(state, crtc_state);
>  
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 901e150..e3cb0db 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2523,10 +2523,10 @@ static bool icl_calc_dpll_state(struct intel_crtc_state *crtc_state,
>  
>  	if (intel_port_is_tc(dev_priv, encoder->port))
>  		ret = icl_calc_tbt_pll(dev_priv, clock, &pll_params);
> -	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> -		ret = cnl_ddi_calculate_wrpll(clock, dev_priv, &pll_params);
> -	else
> +	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP))

I think this breaks EDP and DP MST. Probably just safest to add

	|| intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)

alongside the HDMI branch.

BR,
Jani.

>  		ret = icl_calc_dp_combo_pll(dev_priv, clock, &pll_params);
> +	else
> +		ret = cnl_ddi_calculate_wrpll(clock, dev_priv, &pll_params);
>  
>  	if (!ret)
>  		return false;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/6] drm/i915/icl: Calculate DPLL params for DSI

[Chauhan, Madhav]

> -----Original Message-----
> From: Kulkarni, Vandita
> Sent: Tuesday, November 27, 2018 3:09 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Kulkarni,
> Vandita <vandita.kulkarni@intel.com>
> Subject: [PATCH 1/6] drm/i915/icl: Calculate DPLL params for DSI
> 
> From: Madhav Chauhan <madhav.chauhan@intel.com>
> 
> This patch calculates various DPLL dividers and parameters for DSI encoder
> and adjust AFE clock for DSI. For DSI, 8x clock is AFE clock.
> 
> v2: Extend haswell_crtc_compute_clock() for Gen11 DSI
> 
> v3: Rebase
> 
> v4: use port clock instead of bitrate.
> 
> v5: Reabse and remove divide by 5
> 
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c  | 4 +++-
> drivers/gpu/drm/i915/intel_dpll_mgr.c | 6 +++---
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 27bdf91..1318faf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9303,10 +9303,12 @@ void hsw_disable_pc8(struct drm_i915_private
> *dev_priv)  static int haswell_crtc_compute_clock(struct intel_crtc *crtc,
>  				      struct intel_crtc_state *crtc_state)  {
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	struct intel_atomic_state *state =
>  		to_intel_atomic_state(crtc_state->base.state);
> 
> -	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI)) {
> +	if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI) ||
> +	     IS_ICELAKE(dev_priv)) {

Please align IS_ICELAKE(dev_priv) to "(".

Regards,
Madhav

>  		struct intel_encoder *encoder =
>  			intel_get_crtc_new_encoder(state, crtc_state);
> 
> diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> index 901e150..e3cb0db 100644
> --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c
> +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c
> @@ -2523,10 +2523,10 @@ static bool icl_calc_dpll_state(struct
> intel_crtc_state *crtc_state,
> 
>  	if (intel_port_is_tc(dev_priv, encoder->port))
>  		ret = icl_calc_tbt_pll(dev_priv, clock, &pll_params);
> -	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> -		ret = cnl_ddi_calculate_wrpll(clock, dev_priv, &pll_params);
> -	else
> +	else if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP))
>  		ret = icl_calc_dp_combo_pll(dev_priv, clock, &pll_params);
> +	else
> +		ret = cnl_ddi_calculate_wrpll(clock, dev_priv, &pll_params);
> 
>  	if (!ret)
>  		return false;
> --
> 1.9.1

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

Re: [PATCH 2/6] drm/i915/icl: Use the same pll functions for dsi

[Chauhan, Madhav]

> -----Original Message-----
> From: Kulkarni, Vandita
> Sent: Tuesday, November 27, 2018 3:09 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Kulkarni,
> Vandita <vandita.kulkarni@intel.com>
> Subject: [PATCH 2/6] drm/i915/icl: Use the same pll functions for dsi
> 
> The same pll manager functions can be used to enable dpll for mipi. Hence
> enabling the IO power and esc clock as part of pre pll enable call.
> 
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/icl_dsi.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> index 89ede31..8590825 100644
> --- a/drivers/gpu/drm/i915/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/icl_dsi.c
> @@ -924,17 +924,22 @@ static void gen11_dsi_powerup_panel(struct
> intel_encoder *encoder)
>  	wait_for_cmds_dispatched_to_panel(encoder);
>  }
> 
> -static void gen11_dsi_pre_enable(struct intel_encoder *encoder,
> +static void gen11_dsi_pre_pll_enable(struct intel_encoder *encoder,
>  				 const struct intel_crtc_state *pipe_config,
>  				 const struct drm_connector_state

Correct the alignment of pipe_config and connector_state.
With that fix,
Reviewed-by: Madhav Chauhan <madhav.chauhan@intel.com>

Regards,
Madhav

> *conn_state)  {
> -	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> -
>  	/* step2: enable IO power */
>  	gen11_dsi_enable_io_power(encoder);
> 
>  	/* step3: enable DSI PLL */
>  	gen11_dsi_program_esc_clk_div(encoder);
> +}
> +
> +static void gen11_dsi_pre_enable(struct intel_encoder *encoder,
> +				 const struct intel_crtc_state *pipe_config,
> +				 const struct drm_connector_state
> *conn_state) {
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> 
>  	/* step4: enable DSI port and DPHY */
>  	gen11_dsi_enable_port_and_phy(encoder, pipe_config); @@ -
> 1305,6 +1310,7 @@ void icl_dsi_init(struct drm_i915_private *dev_priv)
>  			 DRM_MODE_ENCODER_DSI, "DSI %c",
> port_name(port));
> 
>  	encoder->pre_enable = gen11_dsi_pre_enable;
> +	encoder->pre_pll_enable = gen11_dsi_pre_pll_enable;
>  	encoder->disable = gen11_dsi_disable;
>  	encoder->port = port;
>  	encoder->get_config = gen11_dsi_get_config;
> --
> 1.9.1

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

Re: [PATCH 3/6] drm/i915/icl: Get port clock from pll.

[Chauhan, Madhav]

> -----Original Message-----
> From: Kulkarni, Vandita
> Sent: Tuesday, November 27, 2018 3:09 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Kulkarni,
> Vandita <vandita.kulkarni@intel.com>
> Subject: [PATCH 3/6] drm/i915/icl: Get port clock from pll.
> 
> Use the same method to get port clock
> like other encoders.
> 
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/icl_dsi.c   | 10 ++++++++--
>  drivers/gpu/drm/i915/intel_ddi.c |  2 +-  drivers/gpu/drm/i915/intel_dsi.h |
> 4 ++++
>  3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> index 8590825..ec22b2e 100644
> --- a/drivers/gpu/drm/i915/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/icl_dsi.c
> @@ -1105,12 +1105,18 @@ static void gen11_dsi_get_config(struct
> intel_encoder *encoder,  {
>  	struct intel_dsi *intel_dsi = container_of(encoder, struct intel_dsi,
>  						   base);
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	int link_clock = 0;

We are directly overwriting this field without reading first.
We might not need initialization.

> +	uint32_t pll_id;
>  	u32 pixel_clk;
> 
> -	//FIXME: Calculate pixel clock using PLL functions once implemented.
> +	pll_id = intel_get_shared_dpll_id(dev_priv, pipe_config-
> >shared_dpll);
> +	link_clock = cnl_calc_wrpll_link(dev_priv, pll_id);
> +	pipe_config->port_clock = link_clock;
> +
Remove extra blank line.

>  	pixel_clk = intel_dsi->pclk;

We can directly use intel_dsi->pclk without using local variable.

>  	pipe_config->base.adjusted_mode.crtc_clock = pixel_clk;
> -	pipe_config->port_clock = pixel_clk;
> +	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
>  }
> 
>  static bool gen11_dsi_compute_config(struct intel_encoder *encoder, diff --
> git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index ad11540..3783f79 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1363,7 +1363,7 @@ static int skl_calc_wrpll_link(struct
> drm_i915_private *dev_priv,
>  	return dco_freq / (p0 * p1 * p2 * 5);
>  }
> 
> -static int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
> +int cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
>  			       enum intel_dpll_id pll_id)
>  {
>  	uint32_t cfgcr0, cfgcr1;
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h
> b/drivers/gpu/drm/i915/intel_dsi.h
> index ee93137..80805bf 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -189,4 +189,8 @@ void intel_dsi_vbt_exec_sequence(struct intel_dsi
> *intel_dsi,
>  				 enum mipi_seq seq_id);
>  void intel_dsi_msleep(struct intel_dsi *intel_dsi, int msec);
> 
> +/* icl dsi uses combophy pll which can be used by any ddi */ int
> +cnl_calc_wrpll_link(struct drm_i915_private *dev_priv,
> +			enum intel_dpll_id pll_id);
> +

All other intel_ddi.c functions has prototype added inside intel_drv.h,
why here for cnl_calc_wrpll_link??

Regards,
Madhav

>  #endif /* _INTEL_DSI_H */
> --
> 1.9.1

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

Re: [PATCH 6/6] drm/i915/icl: Update port clock in compute config

[Chauhan, Madhav]

> -----Original Message-----
> From: Kulkarni, Vandita
> Sent: Tuesday, November 27, 2018 3:09 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Kulkarni,
> Vandita <vandita.kulkarni@intel.com>
> Subject: [PATCH 6/6] drm/i915/icl: Update port clock in compute config
> 
> For DSI 8X clock is AFE clock which is
> is 5 times port clock.

This description need to be rewritten atleast removal of "is" duplication.

> 
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/icl_dsi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> index 80382fb..2812129 100644
> --- a/drivers/gpu/drm/i915/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/icl_dsi.c
> @@ -1183,6 +1183,7 @@ static bool gen11_dsi_compute_config(struct
> intel_encoder *encoder,
>  		pipe_config->cpu_transcoder = TRANSCODER_DSI_0;
> 
>  	pipe_config->clock_set = true;
> +	pipe_config->port_clock = intel_dsi_bitrate(intel_dsi)/5;
> 
>  	//TODO: Add check if DSI PLL calculation is done

We won't need this "TODO" now.

With these fixes,
Reviewed-by: Madhav Chauhan <madhav.chauhan@intel.com>

Regards,
Madhav

> 
> --
> 1.9.1

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

Re: [PATCH 0/6] ICL DSI PLL enable

[Chauhan, Madhav]

Vandita,
As we discussed earlier, we need to select clock for PORT_B as well 
in dual link video scenario even though we are going to use same DPLL for both the
Ports. This will be done by DPCLKA_CFGCR0_DDI_CLK_SEL.
icl_map_plls_to_ports inside haswell_crtc_enable() doesn't take care of that
as its DSI specific requirement.

Regards,
Madhav

> -----Original Message-----
> From: Kulkarni, Vandita
> Sent: Tuesday, November 27, 2018 3:09 PM
> To: intel-gfx@lists.freedesktop.org
> Cc: Chauhan, Madhav <madhav.chauhan@intel.com>; Nikula, Jani
> <jani.nikula@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Kulkarni,
> Vandita <vandita.kulkarni@intel.com>
> Subject: [PATCH 0/6] ICL DSI PLL enable
> 
> ICL DSI uses DPLL.
> As per the discussion with hw team, the same sequence can be used for
> enabling DPLL for mipi dsi as well. Hence reusing the dpll functions from icl
> pll manager.
> In addition to that we need to program
> the esc clock register before enabling dsi.
> 
> This has been tested on git://people.freedesktop.org/~jani/drm
> and the patches are rebased on this.
> 
> Madhav Chauhan (3):
>   drm/i915/icl: Calculate DPLL params for DSI
>   drm/i915/icl: Gate clocks for DSI
>   drm/i915/icl: Ungate DSI clocks
> 
> Vandita Kulkarni (3):
>   drm/i915/icl: Use the same pll functions for dsi
>   drm/i915/icl: Get port clock from pll.
>   drm/i915/icl: Update port clock in compute config
> 
>  drivers/gpu/drm/i915/icl_dsi.c        | 62
> ++++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_ddi.c      |  2 +-
>  drivers/gpu/drm/i915/intel_display.c  |  4 ++-
> drivers/gpu/drm/i915/intel_dpll_mgr.c |  6 ++--
>  drivers/gpu/drm/i915/intel_dsi.h      |  4 +++
>  5 files changed, 68 insertions(+), 10 deletions(-)
> 
> --
> 1.9.1

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

Re: [PATCH 4/6] drm/i915/icl: Gate clocks for DSI

[Imre Deak]

On Tue, Nov 27, 2018 at 03:09:06PM +0530, Vandita Kulkarni wrote:
> From: Madhav Chauhan <madhav.chauhan@intel.com>
> 
> As per BSPEC, depending on the DSI transcoder being used,
> DDI clock for the associated port should be gated. This
> patch does the same.
> 
> Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> ---
>  drivers/gpu/drm/i915/icl_dsi.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> index ec22b2e..fe4efc7 100644
> --- a/drivers/gpu/drm/i915/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/icl_dsi.c
> @@ -536,6 +536,23 @@ static void gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder)
>  	}
>  }
>  
> +static void gen11_dsi_gate_clocks(struct intel_encoder *encoder)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> +	u32 tmp;
> +	enum port port;
> +
> +	mutex_lock(&dev_priv->dpll_lock);
> +	tmp = I915_READ(DPCLKA_CFGCR0_ICL);
> +	for_each_dsi_port(port, intel_dsi->ports) {
> +		tmp |= DPCLKA_CFGCR0_DDI_CLK_OFF(port);
> +	}
> +
> +	I915_WRITE(DPCLKA_CFGCR0_ICL, tmp);
> +	mutex_unlock(&dev_priv->dpll_lock);
> +}
> +
>  static void
>  gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
>  			       const struct intel_crtc_state *pipe_config)
> @@ -883,6 +900,9 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder)
>  
>  	/* Step (4h, 4i, 4j, 4k): Configure transcoder */
>  	gen11_dsi_configure_transcoder(encoder, pipe_config);
> +
> +	/* Step 4l: Gate DDI clocks */
> +	gen11_dsi_gate_clocks(encoder);

I think the default case should be that DDI clocks are gated and here we
shouldn't do anything.

The DDI clocks will get then ungated only when enabling a DDI encoder in
icl_map_plls_to_ports(). There we should also make sure we don't ungate
the clock for a DSI encoder.

I also have a patch to sanity check the clock gating wrt. DSI too during
HW readout, will send that today.

>  }
>  
>  static void gen11_dsi_powerup_panel(struct intel_encoder *encoder)
> -- 
> 1.9.1
> 
> _______________________________________________
> 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

Re: [PATCH 4/6] drm/i915/icl: Gate clocks for DSI

[Imre Deak]

On Tue, Nov 27, 2018 at 04:06:43PM +0200, Imre Deak wrote:
> On Tue, Nov 27, 2018 at 03:09:06PM +0530, Vandita Kulkarni wrote:
> > From: Madhav Chauhan <madhav.chauhan@intel.com>
> > 
> > As per BSPEC, depending on the DSI transcoder being used,
> > DDI clock for the associated port should be gated. This
> > patch does the same.
> > 
> > Signed-off-by: Madhav Chauhan <madhav.chauhan@intel.com>
> > Signed-off-by: Vandita Kulkarni <vandita.kulkarni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/icl_dsi.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/icl_dsi.c b/drivers/gpu/drm/i915/icl_dsi.c
> > index ec22b2e..fe4efc7 100644
> > --- a/drivers/gpu/drm/i915/icl_dsi.c
> > +++ b/drivers/gpu/drm/i915/icl_dsi.c
> > @@ -536,6 +536,23 @@ static void gen11_dsi_setup_dphy_timings(struct intel_encoder *encoder)
> >  	}
> >  }
> >  
> > +static void gen11_dsi_gate_clocks(struct intel_encoder *encoder)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> > +	u32 tmp;
> > +	enum port port;
> > +
> > +	mutex_lock(&dev_priv->dpll_lock);
> > +	tmp = I915_READ(DPCLKA_CFGCR0_ICL);
> > +	for_each_dsi_port(port, intel_dsi->ports) {
> > +		tmp |= DPCLKA_CFGCR0_DDI_CLK_OFF(port);
> > +	}
> > +
> > +	I915_WRITE(DPCLKA_CFGCR0_ICL, tmp);
> > +	mutex_unlock(&dev_priv->dpll_lock);
> > +}
> > +
> >  static void
> >  gen11_dsi_configure_transcoder(struct intel_encoder *encoder,
> >  			       const struct intel_crtc_state *pipe_config)
> > @@ -883,6 +900,9 @@ static void gen11_dsi_setup_timeouts(struct intel_encoder *encoder)
> >  
> >  	/* Step (4h, 4i, 4j, 4k): Configure transcoder */
> >  	gen11_dsi_configure_transcoder(encoder, pipe_config);
> > +
> > +	/* Step 4l: Gate DDI clocks */
> > +	gen11_dsi_gate_clocks(encoder);
> 
> I think the default case should be that DDI clocks are gated and here we
> shouldn't do anything.
> 
> The DDI clocks will get then ungated only when enabling a DDI encoder in
> icl_map_plls_to_ports().

Err, scratch the above. Reading the spec again and trying it on a DSI
machine it seems we do have to keep the DDI clock ungated up to this
point. So that's fine, we just have to make sure not to ungate/gate it
also in icl_map_plls_to_ports/icl_unmap_plls_to_ports.

> There we should also make sure we don't ungate the clock for a DSI
> encoder.
> 
> I also have a patch to sanity check the clock gating wrt. DSI too during
> HW readout, will send that today.
> 
> >  }
> >  
> >  static void gen11_dsi_powerup_panel(struct intel_encoder *encoder)
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx