public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable
@ 2015-11-27 10:21 Jani Nikula
  2015-11-27 10:21 ` [PATCH 2/3] drm/i915: remove pre_pll_enable hook from DDI/gen9+ crtc enable Jani Nikula
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jani Nikula @ 2015-11-27 10:21 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

For DSI, the pre_pll_enable and the pre_enable hooks are called
back-to-back on all platforms that support DSI. The distinction is
artificial for DSI, for which we enable the DSI PLL in the encoder
hooks. Do everything in pre_enable, and remove DSI pre_pll_enable hook.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index efb5a27dd49c..08c46c49c481 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -462,6 +462,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
 	intel_panel_enable_backlight(intel_dsi->attached_connector);
 }
 
+static void intel_dsi_prepare(struct intel_encoder *intel_encoder);
+
 static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 {
 	struct drm_device *dev = encoder->base.dev;
@@ -474,6 +476,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
 
 	DRM_DEBUG_KMS("\n");
 
+	intel_dsi_prepare(encoder);
+	intel_enable_dsi_pll(encoder);
+
 	/* Panel Enable over CRC PMIC */
 	if (intel_dsi->gpio_panel)
 		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
@@ -1026,15 +1031,6 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
 	}
 }
 
-static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
-{
-	DRM_DEBUG_KMS("\n");
-
-	intel_dsi_prepare(encoder);
-	intel_enable_dsi_pll(encoder);
-
-}
-
 static enum drm_connector_status
 intel_dsi_detect(struct drm_connector *connector, bool force)
 {
@@ -1154,9 +1150,7 @@ void intel_dsi_init(struct drm_device *dev)
 
 	drm_encoder_init(dev, encoder, &intel_dsi_funcs, DRM_MODE_ENCODER_DSI);
 
-	/* XXX: very likely not all of these are needed */
 	intel_encoder->compute_config = intel_dsi_compute_config;
-	intel_encoder->pre_pll_enable = intel_dsi_pre_pll_enable;
 	intel_encoder->pre_enable = intel_dsi_pre_enable;
 	intel_encoder->enable = intel_dsi_enable_nop;
 	intel_encoder->disable = intel_dsi_pre_disable;
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/3] drm/i915: remove pre_pll_enable hook from DDI/gen9+ crtc enable
  2015-11-27 10:21 [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable Jani Nikula
@ 2015-11-27 10:21 ` Jani Nikula
  2015-11-27 10:21 ` [PATCH 3/3] drm/i915: add has_dsi_encoder to crtc state Jani Nikula
  2015-11-27 11:26 ` [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable Ville Syrjälä
  2 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2015-11-27 10:21 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

The hook was added to cater for DSI, but with the hooks rearranged on
the DSI encoder side, this is no longer needed. It was a bit silly
anyway to have two hooks called back-to-back.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0743337ac9e8..e18ad2582794 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4981,8 +4981,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
-		if (encoder->pre_pll_enable)
-			encoder->pre_pll_enable(encoder);
 		if (encoder->pre_enable)
 			encoder->pre_enable(encoder);
 	}
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 3/3] drm/i915: add has_dsi_encoder to crtc state
  2015-11-27 10:21 [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable Jani Nikula
  2015-11-27 10:21 ` [PATCH 2/3] drm/i915: remove pre_pll_enable hook from DDI/gen9+ crtc enable Jani Nikula
@ 2015-11-27 10:21 ` Jani Nikula
  2015-11-27 11:26 ` [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable Ville Syrjälä
  2 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2015-11-27 10:21 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

DSI has quite a few special cases, like DP, so add it to crtc
state. This way we can get rid of a number of intel_pipe_has_type()
checks for DSI. This isn't necessarily the prettiest way, but it's a
step towards being aligned with what's being done with other encoders.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 49 ++++++++++++------------------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 +++
 drivers/gpu/drm/i915/intel_dsi.c     |  8 ++++--
 3 files changed, 25 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e18ad2582794..8dbe5b398af4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2130,7 +2130,7 @@ static void intel_enable_pipe(struct intel_crtc *crtc)
 	 * need the check.
 	 */
 	if (HAS_GMCH_DISPLAY(dev_priv->dev))
-		if (intel_pipe_has_type(crtc, INTEL_OUTPUT_DSI))
+		if (crtc->config->has_dsi_encoder)
 			assert_dsi_pll_enabled(dev_priv);
 		else
 			assert_pll_enabled(dev_priv, pipe);
@@ -4636,7 +4636,7 @@ static void intel_crtc_load_lut(struct drm_crtc *crtc)
 		return;
 
 	if (HAS_GMCH_DISPLAY(dev_priv->dev)) {
-		if (intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI))
+		if (intel_crtc->config->has_dsi_encoder)
 			assert_dsi_pll_enabled(dev_priv);
 		else
 			assert_pll_enabled(dev_priv, pipe);
@@ -4946,7 +4946,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	int pipe = intel_crtc->pipe, hsw_workaround_pipe;
 	struct intel_crtc_state *pipe_config =
 		to_intel_crtc_state(crtc->state);
-	bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
 
 	if (WARN_ON(intel_crtc->active))
 		return;
@@ -4988,7 +4987,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	if (intel_crtc->config->has_pch_encoder)
 		dev_priv->display.fdi_link_train(crtc);
 
-	if (!is_dsi)
+	if (!intel_crtc->config->has_dsi_encoder)
 		intel_ddi_enable_pipe_clock(intel_crtc);
 
 	if (INTEL_INFO(dev)->gen >= 9)
@@ -5003,7 +5002,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	intel_crtc_load_lut(crtc);
 
 	intel_ddi_set_pipe_settings(crtc);
-	if (!is_dsi)
+	if (!intel_crtc->config->has_dsi_encoder)
 		intel_ddi_enable_transcoder_func(crtc);
 
 	intel_update_watermarks(crtc);
@@ -5012,7 +5011,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	if (intel_crtc->config->has_pch_encoder)
 		lpt_pch_enable(crtc);
 
-	if (intel_crtc->config->dp_encoder_is_mst && !is_dsi)
+	if (intel_crtc->config->dp_encoder_is_mst)
 		intel_ddi_set_vc_payload_alloc(crtc, true);
 
 	assert_vblank_disabled(crtc);
@@ -5123,7 +5122,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
-	bool is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
 
 	if (intel_crtc->config->has_pch_encoder)
 		intel_set_pch_fifo_underrun_reporting(dev_priv, TRANSCODER_A,
@@ -5142,7 +5140,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	if (intel_crtc->config->dp_encoder_is_mst)
 		intel_ddi_set_vc_payload_alloc(crtc, false);
 
-	if (!is_dsi)
+	if (!intel_crtc->config->has_dsi_encoder)
 		intel_ddi_disable_transcoder_func(dev_priv, cpu_transcoder);
 
 	if (INTEL_INFO(dev)->gen >= 9)
@@ -5150,7 +5148,7 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	else
 		ironlake_pfit_disable(intel_crtc, false);
 
-	if (!is_dsi)
+	if (!intel_crtc->config->has_dsi_encoder)
 		intel_ddi_disable_pipe_clock(intel_crtc);
 
 	if (intel_crtc->config->has_pch_encoder) {
@@ -6152,13 +6150,10 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_encoder *encoder;
 	int pipe = intel_crtc->pipe;
-	bool is_dsi;
 
 	if (WARN_ON(intel_crtc->active))
 		return;
 
-	is_dsi = intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI);
-
 	if (intel_crtc->config->has_dp_encoder)
 		intel_dp_set_m_n(intel_crtc, M1_N1);
 
@@ -6181,7 +6176,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 		if (encoder->pre_pll_enable)
 			encoder->pre_pll_enable(encoder);
 
-	if (!is_dsi) {
+	if (!intel_crtc->config->has_dsi_encoder) {
 		if (IS_CHERRYVIEW(dev)) {
 			chv_prepare_pll(intel_crtc, intel_crtc->config);
 			chv_enable_pll(intel_crtc, intel_crtc->config);
@@ -6307,7 +6302,7 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 		if (encoder->post_disable)
 			encoder->post_disable(encoder);
 
-	if (!intel_pipe_has_type(intel_crtc, INTEL_OUTPUT_DSI)) {
+	if (!intel_crtc->config->has_dsi_encoder) {
 		if (IS_CHERRYVIEW(dev))
 			chv_disable_pll(dev_priv, pipe);
 		else if (IS_VALLEYVIEW(dev))
@@ -7920,8 +7915,6 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	int refclk, num_connectors = 0;
 	intel_clock_t clock;
 	bool ok;
-	bool is_dsi = false;
-	struct intel_encoder *encoder;
 	const intel_limit_t *limit;
 	struct drm_atomic_state *state = crtc_state->base.state;
 	struct drm_connector *connector;
@@ -7931,26 +7924,14 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	memset(&crtc_state->dpll_hw_state, 0,
 	       sizeof(crtc_state->dpll_hw_state));
 
-	for_each_connector_in_state(state, connector, connector_state, i) {
-		if (connector_state->crtc != &crtc->base)
-			continue;
-
-		encoder = to_intel_encoder(connector_state->best_encoder);
-
-		switch (encoder->type) {
-		case INTEL_OUTPUT_DSI:
-			is_dsi = true;
-			break;
-		default:
-			break;
-		}
+	if (crtc_state->has_dsi_encoder)
+		return 0;
 
-		num_connectors++;
+	for_each_connector_in_state(state, connector, connector_state, i) {
+		if (connector_state->crtc == &crtc->base)
+			num_connectors++;
 	}
 
-	if (is_dsi)
-		return 0;
-
 	if (!crtc_state->clock_set) {
 		refclk = i9xx_get_refclk(crtc_state, num_connectors);
 
@@ -12599,6 +12580,8 @@ intel_pipe_config_compare(struct drm_device *dev,
 	} else
 		PIPE_CONF_CHECK_M_N_ALT(dp_m_n, dp_m2_n2);
 
+	PIPE_CONF_CHECK_I(has_dsi_encoder);
+
 	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hdisplay);
 	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_htotal);
 	PIPE_CONF_CHECK_I(base.adjusted_mode.crtc_hblank_start);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8fae824e3cd3..1ffd8d5c3235 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -393,6 +393,9 @@ struct intel_crtc_state {
 	 * accordingly. */
 	bool has_dp_encoder;
 
+	/* DSI has special cases */
+	bool has_dsi_encoder;
+
 	/* Whether we should send NULL infoframes. Required for audio. */
 	bool has_hdmi_sink;
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 08c46c49c481..230957f9f663 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -266,16 +266,18 @@ static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
 }
 
 static bool intel_dsi_compute_config(struct intel_encoder *encoder,
-				     struct intel_crtc_state *config)
+				     struct intel_crtc_state *pipe_config)
 {
 	struct intel_dsi *intel_dsi = container_of(encoder, struct intel_dsi,
 						   base);
 	struct intel_connector *intel_connector = intel_dsi->attached_connector;
 	struct drm_display_mode *fixed_mode = intel_connector->panel.fixed_mode;
-	struct drm_display_mode *adjusted_mode = &config->base.adjusted_mode;
+	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
 
 	DRM_DEBUG_KMS("\n");
 
+	pipe_config->has_dsi_encoder = true;
+
 	if (fixed_mode)
 		intel_fixed_panel_mode(fixed_mode, adjusted_mode);
 
@@ -704,6 +706,8 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
 	u32 pclk = 0;
 	DRM_DEBUG_KMS("\n");
 
+	pipe_config->has_dsi_encoder = true;
+
 	/*
 	 * DPLL_MD is not used in case of DSI, reading will get some default value
 	 * set dpll_md = 0
-- 
2.1.4

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable
  2015-11-27 10:21 [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable Jani Nikula
  2015-11-27 10:21 ` [PATCH 2/3] drm/i915: remove pre_pll_enable hook from DDI/gen9+ crtc enable Jani Nikula
  2015-11-27 10:21 ` [PATCH 3/3] drm/i915: add has_dsi_encoder to crtc state Jani Nikula
@ 2015-11-27 11:26 ` Ville Syrjälä
  2015-11-30  8:33   ` Daniel Vetter
  2 siblings, 1 reply; 6+ messages in thread
From: Ville Syrjälä @ 2015-11-27 11:26 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

On Fri, Nov 27, 2015 at 12:21:44PM +0200, Jani Nikula wrote:
> For DSI, the pre_pll_enable and the pre_enable hooks are called
> back-to-back on all platforms that support DSI. The distinction is
> artificial for DSI, for which we enable the DSI PLL in the encoder
> hooks. Do everything in pre_enable, and remove DSI pre_pll_enable hook.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

I was eyeing this same thing at some point.

For the series:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index efb5a27dd49c..08c46c49c481 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -462,6 +462,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>  	intel_panel_enable_backlight(intel_dsi->attached_connector);
>  }
>  
> +static void intel_dsi_prepare(struct intel_encoder *intel_encoder);
> +
>  static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  {
>  	struct drm_device *dev = encoder->base.dev;
> @@ -474,6 +476,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>  
>  	DRM_DEBUG_KMS("\n");
>  
> +	intel_dsi_prepare(encoder);
> +	intel_enable_dsi_pll(encoder);
> +
>  	/* Panel Enable over CRC PMIC */
>  	if (intel_dsi->gpio_panel)
>  		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
> @@ -1026,15 +1031,6 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>  	}
>  }
>  
> -static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
> -{
> -	DRM_DEBUG_KMS("\n");
> -
> -	intel_dsi_prepare(encoder);
> -	intel_enable_dsi_pll(encoder);
> -
> -}
> -
>  static enum drm_connector_status
>  intel_dsi_detect(struct drm_connector *connector, bool force)
>  {
> @@ -1154,9 +1150,7 @@ void intel_dsi_init(struct drm_device *dev)
>  
>  	drm_encoder_init(dev, encoder, &intel_dsi_funcs, DRM_MODE_ENCODER_DSI);
>  
> -	/* XXX: very likely not all of these are needed */
>  	intel_encoder->compute_config = intel_dsi_compute_config;
> -	intel_encoder->pre_pll_enable = intel_dsi_pre_pll_enable;
>  	intel_encoder->pre_enable = intel_dsi_pre_enable;
>  	intel_encoder->enable = intel_dsi_enable_nop;
>  	intel_encoder->disable = intel_dsi_pre_disable;
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable
  2015-11-27 11:26 ` [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable Ville Syrjälä
@ 2015-11-30  8:33   ` Daniel Vetter
  2015-11-30 10:59     ` Jani Nikula
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2015-11-30  8:33 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Jani Nikula, intel-gfx

On Fri, Nov 27, 2015 at 01:26:40PM +0200, Ville Syrjälä wrote:
> On Fri, Nov 27, 2015 at 12:21:44PM +0200, Jani Nikula wrote:
> > For DSI, the pre_pll_enable and the pre_enable hooks are called
> > back-to-back on all platforms that support DSI. The distinction is
> > artificial for DSI, for which we enable the DSI PLL in the encoder
> > hooks. Do everything in pre_enable, and remove DSI pre_pll_enable hook.
> > 
> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> I was eyeing this same thing at some point.
> 
> For the series:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yeah, all the pretties in here!

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Btw about bxt dsi I wonder whether we should partially merge this with the
cpu transcoder (since it's only either that or a dsi encoder aparrently).
At least I remember that we still had some issues with bxt's
dsi_get_hw_state?

Cheers, Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/intel_dsi.c | 16 +++++-----------
> >  1 file changed, 5 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index efb5a27dd49c..08c46c49c481 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -462,6 +462,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
> >  	intel_panel_enable_backlight(intel_dsi->attached_connector);
> >  }
> >  
> > +static void intel_dsi_prepare(struct intel_encoder *intel_encoder);
> > +
> >  static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> >  {
> >  	struct drm_device *dev = encoder->base.dev;
> > @@ -474,6 +476,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
> >  
> >  	DRM_DEBUG_KMS("\n");
> >  
> > +	intel_dsi_prepare(encoder);
> > +	intel_enable_dsi_pll(encoder);
> > +
> >  	/* Panel Enable over CRC PMIC */
> >  	if (intel_dsi->gpio_panel)
> >  		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
> > @@ -1026,15 +1031,6 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
> >  	}
> >  }
> >  
> > -static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
> > -{
> > -	DRM_DEBUG_KMS("\n");
> > -
> > -	intel_dsi_prepare(encoder);
> > -	intel_enable_dsi_pll(encoder);
> > -
> > -}
> > -
> >  static enum drm_connector_status
> >  intel_dsi_detect(struct drm_connector *connector, bool force)
> >  {
> > @@ -1154,9 +1150,7 @@ void intel_dsi_init(struct drm_device *dev)
> >  
> >  	drm_encoder_init(dev, encoder, &intel_dsi_funcs, DRM_MODE_ENCODER_DSI);
> >  
> > -	/* XXX: very likely not all of these are needed */
> >  	intel_encoder->compute_config = intel_dsi_compute_config;
> > -	intel_encoder->pre_pll_enable = intel_dsi_pre_pll_enable;
> >  	intel_encoder->pre_enable = intel_dsi_pre_enable;
> >  	intel_encoder->enable = intel_dsi_enable_nop;
> >  	intel_encoder->disable = intel_dsi_pre_disable;
> > -- 
> > 2.1.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable
  2015-11-30  8:33   ` Daniel Vetter
@ 2015-11-30 10:59     ` Jani Nikula
  0 siblings, 0 replies; 6+ messages in thread
From: Jani Nikula @ 2015-11-30 10:59 UTC (permalink / raw)
  To: Daniel Vetter, Ville Syrjälä; +Cc: intel-gfx

On Mon, 30 Nov 2015, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Fri, Nov 27, 2015 at 01:26:40PM +0200, Ville Syrjälä wrote:
>> On Fri, Nov 27, 2015 at 12:21:44PM +0200, Jani Nikula wrote:
>> > For DSI, the pre_pll_enable and the pre_enable hooks are called
>> > back-to-back on all platforms that support DSI. The distinction is
>> > artificial for DSI, for which we enable the DSI PLL in the encoder
>> > hooks. Do everything in pre_enable, and remove DSI pre_pll_enable hook.
>> > 
>> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> I was eyeing this same thing at some point.
>> 
>> For the series:
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Yeah, all the pretties in here!
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> Btw about bxt dsi I wonder whether we should partially merge this with the
> cpu transcoder (since it's only either that or a dsi encoder aparrently).
> At least I remember that we still had some issues with bxt's
> dsi_get_hw_state?

One step at a time, maybe as a follow-up. I'm kind of hesitant to
conflate this with cpu transcoder, in case some new platform changes the
rules.

So for now, I've pushed this to drm-intel-next-queued, thanks for the
reviews.

BR,
Jani.


>
> Cheers, Daniel
>
>> 
>> > ---
>> >  drivers/gpu/drm/i915/intel_dsi.c | 16 +++++-----------
>> >  1 file changed, 5 insertions(+), 11 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> > index efb5a27dd49c..08c46c49c481 100644
>> > --- a/drivers/gpu/drm/i915/intel_dsi.c
>> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> > @@ -462,6 +462,8 @@ static void intel_dsi_enable(struct intel_encoder *encoder)
>> >  	intel_panel_enable_backlight(intel_dsi->attached_connector);
>> >  }
>> >  
>> > +static void intel_dsi_prepare(struct intel_encoder *intel_encoder);
>> > +
>> >  static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>> >  {
>> >  	struct drm_device *dev = encoder->base.dev;
>> > @@ -474,6 +476,9 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder)
>> >  
>> >  	DRM_DEBUG_KMS("\n");
>> >  
>> > +	intel_dsi_prepare(encoder);
>> > +	intel_enable_dsi_pll(encoder);
>> > +
>> >  	/* Panel Enable over CRC PMIC */
>> >  	if (intel_dsi->gpio_panel)
>> >  		gpiod_set_value_cansleep(intel_dsi->gpio_panel, 1);
>> > @@ -1026,15 +1031,6 @@ static void intel_dsi_prepare(struct intel_encoder *intel_encoder)
>> >  	}
>> >  }
>> >  
>> > -static void intel_dsi_pre_pll_enable(struct intel_encoder *encoder)
>> > -{
>> > -	DRM_DEBUG_KMS("\n");
>> > -
>> > -	intel_dsi_prepare(encoder);
>> > -	intel_enable_dsi_pll(encoder);
>> > -
>> > -}
>> > -
>> >  static enum drm_connector_status
>> >  intel_dsi_detect(struct drm_connector *connector, bool force)
>> >  {
>> > @@ -1154,9 +1150,7 @@ void intel_dsi_init(struct drm_device *dev)
>> >  
>> >  	drm_encoder_init(dev, encoder, &intel_dsi_funcs, DRM_MODE_ENCODER_DSI);
>> >  
>> > -	/* XXX: very likely not all of these are needed */
>> >  	intel_encoder->compute_config = intel_dsi_compute_config;
>> > -	intel_encoder->pre_pll_enable = intel_dsi_pre_pll_enable;
>> >  	intel_encoder->pre_enable = intel_dsi_pre_enable;
>> >  	intel_encoder->enable = intel_dsi_enable_nop;
>> >  	intel_encoder->disable = intel_dsi_pre_disable;
>> > -- 
>> > 2.1.4
>> > 
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Ville Syrjälä
>> Intel OTC
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-11-30 10:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-27 10:21 [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable Jani Nikula
2015-11-27 10:21 ` [PATCH 2/3] drm/i915: remove pre_pll_enable hook from DDI/gen9+ crtc enable Jani Nikula
2015-11-27 10:21 ` [PATCH 3/3] drm/i915: add has_dsi_encoder to crtc state Jani Nikula
2015-11-27 11:26 ` [PATCH 1/3] drm/i915/dsi: merge pre_pll_enable hook to pre_enable Ville Syrjälä
2015-11-30  8:33   ` Daniel Vetter
2015-11-30 10:59     ` Jani Nikula

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox